feat: studio-quality Sprint J polish - #44
Conversation
Act 4-5 live quest hints, industrial damp sheen, photo session gallery, travel confirm quest context, stuck NPC ring refocus, board-journal deep-links, and env LOD/locomotion feel.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds damp industrial scene materials, quest navigation and contextual hints, stuck-interaction recovery feedback, photo capture history with gallery selection, environment LOD adjustments, and player movement tuning. ChangesScene rendering and environment tuning
Quest navigation and contextual hints
Interaction stuck recovery
Photo capture gallery
Player feel tuning
Estimated code review effort: 4 (Complex) | ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
🔧 Fix failing CI
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
volodka2 | b5a9411 | Jul 28 2026, 09:48 PM |
![coderabbitai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/game/photoMode/PhotoModeViewfinder.tsx`:
- Around line 340-342: Update the capture history gallery container in
PhotoModeViewfinder so it uses a different vertical position from the scene-name
block anchored at bottom-6. Preserve its existing horizontal positioning and
behavior while assigning a separate bottom spacing slot that prevents overlap.
- Around line 341-355: Remove the conflicting role="listitem" from each native
thumbnail button in the captureHistory mapping within PhotoModeViewfinder; keep
the buttons’ native semantics and existing click behavior intact.
In `@src/components/game/photoMode/usePhotoModeController.ts`:
- Around line 126-130: Update selectHistoryCapture to store entry.filter in
PhotoPreviewData alongside the dataUrl and timestamp, then update
PhotoModeViewfinder and this hook’s download/share paths to use the selected
preview filter instead of the current filterPreset. Preserve current-filter
behavior for newly captured photos and ensure historical captures preview,
download, and share with their stored filter.
- Around line 56-57: Update the capture flow around capturePhotoStill and
captureScreenshot to snapshot filterPreset and sceneName before the async
capture begins, then use those snapshots when creating the history entry. Add
sceneName to captureScreenshot’s dependency list, and remove the render-time
sceneNameRef.current assignment and any metadata reads that can observe newer
values.
In `@src/engine/interaction/stuckRecoveryNpcFocus.test.ts`:
- Around line 42-52: Update the test for resolveNearestNpcForRingFocus to pass a
non-null, unknown recovered target ID that is absent from groups, while
retaining the assertions that Maria is selected as the nearest NPC and has the
expected label.
In `@src/engine/interaction/stuckRecoveryNpcFocus.ts`:
- Around line 69-76: Update the interaction hint emission in the stuck-recovery
NPC focus flow to use a re-approach message when focus.distance exceeds
NPC_INTERACTION_QUERY_RANGE, rather than reporting the NPC as being in the
interaction zone. Preserve the existing in-range message for distances within
the query range and keep the remaining hint fields unchanged.
In `@src/engine/photo/photoCaptureHistory.ts`:
- Around line 23-25: Update the capture ID generation in the photo capture
history function to use a collision-resistant identifier independent of
history.length, such as a monotonic counter or UUID, while preserving the
existing cap-${timestamp}- style only if uniqueness remains guaranteed. Add a
regression test covering two captures created within the same millisecond when
history is full, verifying their IDs differ.
In `@src/engine/player/playerConstants.ts`:
- Around line 58-62: Update RUN_FOV_SPEED_MIN in the player movement constants
to match the actual sprint threshold, preventing normal movement from receiving
FOV boost; retain RUN_FOV_SPEED_FULL above this minimum so full intensity
remains reached only at higher sprint speeds.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 06a508a5-492e-4ad0-8638-8527277faddd
📒 Files selected for processing (29)
src/components/3d/AbandonedFactoryVisual.tsxsrc/components/3d/ChkForestZorgeVisual.tsxsrc/components/3d/SceneColliderSelector.tsxsrc/components/game/QuestsPanel.tsxsrc/components/game/WorldMap.tsxsrc/components/game/hud/parts/InteractionDistanceRing.tsxsrc/components/game/photoMode/PhotoModePanel.tsxsrc/components/game/photoMode/PhotoModeViewfinder.tsxsrc/components/game/photoMode/usePhotoModeController.tssrc/components/game/questBoard/QuestBoardPanel.tsxsrc/engine/graphics/wetStreetScenes.test.tssrc/engine/graphics/wetStreetScenes.tssrc/engine/guidedStory/act4QuestHints.test.tssrc/engine/guidedStory/act4QuestHints.tssrc/engine/guidedStory/act5QuestHints.test.tssrc/engine/guidedStory/act5QuestHints.tssrc/engine/interaction/emergencyInteractionReset.tssrc/engine/interaction/interactionSession.tssrc/engine/interaction/stuckRecoveryNpcFocus.test.tssrc/engine/interaction/stuckRecoveryNpcFocus.tssrc/engine/lod/distanceLod.test.tssrc/engine/lod/distanceLod.tssrc/engine/photo/photoCaptureHistory.test.tssrc/engine/photo/photoCaptureHistory.tssrc/engine/photo/photoModeConstants.tssrc/engine/player/playerConstants.tssrc/engine/questBoard/questBoardConstants.tssrc/hooks/questJournalHint.test.tssrc/hooks/questJournalHint.ts
| {captureHistory.length > 0 && onSelectHistory && ( | ||
| <div | ||
| className="absolute bottom-6 left-6 sm:left-10 pointer-events-auto flex flex-col gap-1.5" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Move the gallery away from the scene label.
The gallery uses bottom-6 left-6, the same anchor as the existing scene-name block at Lines 198–208. It will cover the scene name in the viewfinder; give the gallery a separate vertical slot.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/game/photoMode/PhotoModeViewfinder.tsx` around lines 340 -
342, Update the capture history gallery container in PhotoModeViewfinder so it
uses a different vertical position from the scene-name block anchored at
bottom-6. Preserve its existing horizontal positioning and behavior while
assigning a separate bottom spacing slot that prevents overlap.
| <div | ||
| className="absolute bottom-6 left-6 sm:left-10 pointer-events-auto flex flex-col gap-1.5" | ||
| role="list" | ||
| aria-label={PHOTO_MODE_LABELS.galleryTitle} | ||
| > | ||
| <span className="text-[8px] font-mono uppercase tracking-wider" style={{ color: accent(0.55) }}> | ||
| {PHOTO_MODE_LABELS.galleryTitle} | ||
| </span> | ||
| <div className="flex items-center gap-1.5"> | ||
| {captureHistory.slice(0, 6).map((entry) => ( | ||
| <button | ||
| key={entry.id} | ||
| type="button" | ||
| role="listitem" | ||
| onClick={() => onSelectHistory(entry)} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
fd -a 'PhotoModeViewfinder.tsx' . || true
echo "== relevant lines =="
file="$(fd 'PhotoModeViewfinder.tsx' . | head -n1 || true)"
if [ -n "${file:-}" ]; then
nl -ba "$file" | sed -n '320,375p'
fi
echo "== search role/listitem usage in file =="
if [ -n "${file:-}" ]; then
rg -n 'role="listitem"|<button|role="list"|onSelectHistory|captureHistory' "$file"
fi
echo "== package/accessibility deps =="
for f in package.json deno.json pnpm-lock.yaml .npmrc .eslintrc* eslint.config.*; do
[ -f "$f" ] && { echo "--- $f"; sed -n '1,220p' "$f"; }
doneRepository: IETGLIM/Volodka
Length of output: 306
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="/home/jailuser/git/src/components/game/photoMode/PhotoModeViewfinder.tsx"
echo "== relevant lines =="
awk '{printf "%6d\t%s\n", NR, $0}' "$file" | sed -n '320,375p'
echo "== search relevant symbols in file =="
grep -nE 'role="listitem"|role="list"|<button|onSelectHistory|captureHistory' "$file" || true
echo "== package/accessibility deps =="
for f in /home/jailuser/git/package.json /home/jailuser/git/deno.json /home/jailuser/git/pnpm-lock.yaml /home/jailuser/git/.npmrc /home/jailuser/git/.eslintrc /home/jailuser/git/.eslintrc.js /home/jailuser/git/.eslintrc.json /home/jailuser/git/eslint.config.js; do
if [ -f "$f" ]; then
echo "--- $f"
sed -n '1,220p' "$f"
fi
doneRepository: IETGLIM/Volodka
Length of output: 12913
🌐 Web query:
WAI-ARIA listitem role button role allowed ancestors implicit semantics listitem invalid container
💡 Result:
Under WAI-ARIA and HTML accessibility specifications, the listitem role requires specific ancestral context, and the button role imposes restrictions on its descendants. Ancestral Requirements for listitem: The listitem role must be contained within, or owned by, an element with the role list or group [1]. In native HTML, an li element is implicitly exposed as a listitem only when it is a direct child of a ul, ol, or menu element [2][3]. If an li element is used outside of these required parent elements, it is typically exposed as a generic role (non-semantic) [2][3]. Implicit Semantics and Invalid Containers: If a listitem is placed in a container that does not provide the required list or group context, it fails to fulfill the semantic requirement of a list item [1]. Conversely, if an li element is assigned an explicit role (e.g., role=button) while inside a properly structured list (a parent element with role=list), this is generally disallowed by the ARIA in HTML specification because the list element only permits specific accessibility children (typically listitem or group) [4][5]. Interaction with the button Role: 1. Ancestors: There are no specific "listitem-like" ancestral requirements for a button, but a button cannot be a direct child of certain elements that do not support interactive content or where it would violate the accessibility tree structure [4]. 2. Descendants: A critical restriction exists for the button role: it cannot contain other interactive elements or semantic children that the accessibility API cannot represent [6]. Browsers automatically apply role=presentation to all descendants of a button element, effectively stripping them of their native or explicit semantic roles [6]. Invalid Usage Patterns: Attempting to nest a button inside a listitem (e.g.,
Citations:
- 1: https://www.w3.org/TR/wai-aria-1.0/roles
- 2: https://www.w3.org/TR/2023/REC-html-aria-20230531/
- 3: https://www.w3.org/TR/2025/REC-html-aria-20250123/
- 4: Clarify li element role allowances w3c/html-aria#410
- 5: https://www.w3.org/TR/wai-aria-1.3/
- 6: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Roles/button_role
- 7: https://rocketvalidator.com/html-validation/the-listitem-role-is-unnecessary-for-element-li
- 8: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Roles/listitem_role
🌐 Web query:
ARIA listitem role native button semantics aria-role on button listitem invalid
💡 Result:
Applying role=button directly to an li element is invalid and semantically contradictory [1][2][3]. According to the W3C ARIA in HTML specification, an li element functioning as a list item (within a ul, ol, or menu) must not be assigned the button role [1][4]. Assigning role=button to an li creates a semantic conflict where the browser treats the element as a list item, but screen readers may incorrectly announce it as a button, leading to unpredictable or degraded user experiences [2][3]. Furthermore, a native li lacks the built-in keyboard interactivity (focus management and activation via Space/Enter) expected of a button, requiring developers to manually implement these features—an approach that is error-prone and violates best practices [5][2]. To implement this correctly, follow these recommended patterns: 1. Place a native button inside the li: This preserves the list semantics while providing standard button functionality [2].
Citations:
- 1: https://www.w3.org/TR/2026/REC-html-aria-20260415/
- 2: https://rocketvalidator.com/html-validation/bad-value-button-for-attribute-role-on-element-li
- 3: https://rocketvalidator.com/accessibility-validation/axe/4.12/aria-allowed-role
- 4: https://www.w3.org/TR/2024/REC-html-aria-20241217/
- 5: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Roles/button_role
- 6: Clarifying the allowed roles on an li element validator/validator#1368
- 7: https://stackoverflow.com/questions/56458808/accessibility-wrapping-list-items-in-buttons-for-better-keyboard-navigation-acc
🌐 Web query:
MDN ARIA button role descendants semantic children presentation descendants button role
💡 Result:
In the context of WAI-ARIA, the button role is defined as a role that does not support semantic children [1][2][3]. Because accessibility APIs cannot represent semantic elements (such as headings, lists, or tables) contained within a button, browsers automatically apply the presentation role to all descendant elements of any button [1][2][3]. As a result of this automatic transformation, any semantic elements nested inside a button are hidden from assistive technology, effectively becoming presentational [1][2]. For example, a heading element nested inside a div with role="button" is treated as if it had an explicit role="presentation," meaning it is ignored by screen readers and other accessibility tools [1][2]. Furthermore, according to the ARIA in HTML specification, buttons (including elements with an explicit role="button") must not contain interactive content or elements with a tabindex attribute specified [4][5][6]. While buttons are permitted to contain phrasing content, the underlying accessibility tree flattens the hierarchy, removing the semantic structure of any children [1][2]. Developers are strongly advised against nesting interactive elements or complex structures within buttons to avoid unexpected behavior and accessibility issues, as these descendants will not be properly exposed to users of assistive technologies [7][8].
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Roles/button_role
- 2: https://github.com/mdn/content/blob/main/files/en-us/web/accessibility/aria/reference/roles/button_role/index.md
- 3: https://www.w3.org/WAI/ARIA/apg/practices/hiding-semantics/
- 4: https://www.w3.org/TR/2024/REC-html-aria-20241213/
- 5: https://www.w3.org/TR/2023/REC-html-aria-20230827/
- 6: https://www.w3.org/TR/2025/REC-html-aria-20250123/
- 7: https://www.w3.org/WAI/standards-guidelines/act/rules/307n5z/
- 8: Descendants of buttons with "Children Presentational: True"? w3c/aria#1174
🌐 Web query:
WAI-ARIA listitem role allowed parent roles button element native HTML aria role listitem on button
💡 Result:
The WAI-ARIA listitem role requires an accessibility parent role of list or group [1][2][3]. Specifically, an element with role listitem is only meaningful when it is contained inside or owned by an element with the role list [1][2]. Regarding the use of aria roles on specific elements: 1. Native HTML LI Elements: If an li element is a child of a list element (ul, ol, or menu) that is exposed as a list, the li element has an implicit role of listitem [4][5]. Under the ARIA in HTML specification, no role other than listitem is permitted on an li in this context [6][5]. Specifying role=button on an li that is correctly nested within a list is invalid and creates a semantic conflict, as the browser continues to treat it as a list item while assistive technologies are instructed to treat it as a button [7][8]. This practice is strongly discouraged [7][5]. 2. If the parent list element has had its implicit list role removed or overwritten (e.g., via role=none or role=presentation), the li element may be permitted to have other roles, provided those roles are allowed by its ancestors [9][6]. 3. General Best Practice: If you need a button, you should use the native HTML button element [7][10][8]. If you must use a different element, you should generally avoid overriding list items with button roles [7][8]. Instead, place a button element inside the list item, or use a structure that natively supports the desired role [7][8].
Citations:
- 1: https://www.w3.org/TR/wai-aria-1.3/
- 2: https://www.w3.org/TR/aria-role/roles
- 3: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Roles/listitem_role
- 4: https://www.w3.org/TR/2023/REC-html-aria-20230705/
- 5: https://www.w3.org/TR/2024/REC-html-aria-20241217/
- 6: Clarifying the allowed roles on an li element validator/validator#1368
- 7: https://rocketvalidator.com/html-validation/bad-value-button-for-attribute-role-on-element-li
- 8: https://rocketvalidator.com/accessibility-validation/axe/4.12/aria-allowed-role
- 9: Clarify li element role allowances w3c/html-aria#410
- 10: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Roles/button_role
🌐 Web query:
"role=listitem" "<button>" button semantics
💡 Result:
Using role="button" on an element with role="listitem" (or an
- or
- ). Applying role="button" to a list item confuses assistive technologies, as the element attempts to convey two different, incompatible semantics: being a structural item in a list and being an interactive trigger [2]. Key reasons to avoid this pattern include: 1. Semantic Conflict: Screen readers may struggle to interpret the element correctly. You are telling the browser the element is a button, yet it remains structurally trapped within a list context [2]. 2. Lack of Native Functionality: Unlike a native
- /
- structure and put a
- with role="none") and mark up the buttons appropriately [2][4]. - Native elements first: Always prefer native HTML elements (
Citations:
- 1: Why does an <li role=button... fail... seems to read ok with screen reader dequelabs/axe-core#1942
- 2: https://rocketvalidator.com/html-validation/bad-value-button-for-attribute-role-on-element-li
- 3: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Roles/button_role
- 4: Clarifying the allowed roles on an li element validator/validator#1368
- 5: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Roles/listitem_role
Keep gallery thumbnails exposed as buttons, not list items.
role="listitem" is invalid on a native <button> and can suppress/override button semantics. Wrap each thumbnail in an <li role="listitem"> with a native button inside, or remove the conflicting ARIA role from the buttons.
🧰 Tools
🪛 React Doctor (0.9.1)
[warning] 343-343: Screen reader users get more reliable semantics from <ul> than role="list", so use <ul> instead.
Use the matching HTML element when one exists so browsers and assistive tech get native semantics.
(prefer-tag-over-role)
[warning] 354-354: Screen reader users can't operate this interactive <button> because role listitem says it isn't, so remove the role or use a different element.
Do not give an interactive element a role that says it is not interactive.
(no-interactive-element-to-noninteractive-role)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/game/photoMode/PhotoModeViewfinder.tsx` around lines 341 -
355, Remove the conflicting role="listitem" from each native thumbnail button in
the captureHistory mapping within PhotoModeViewfinder; keep the buttons’ native
semantics and existing click behavior intact.
Source: Linters/SAST tools
| const sceneNameRef = useRef(sceneName); | ||
| sceneNameRef.current = sceneName; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)usePhotoModeController\.ts$|PhotoMode|photoMode' || true
echo "== target outline =="
ast-grep outline src/components/game/photoMode/usePhotoModeController.ts || true
echo "== target relevant source =="
cat -n src/components/game/photoMode/usePhotoModeController.ts | sed -n '1,180p'
echo "== related photo capture usages/tests =="
rg -n "capturePhotoStill|dataUrl|history|sceneNameRef|filterPreset|sceneName" src -S || trueRepository: IETGLIM/Volodka
Length of output: 28394
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p=Path('src/components/game/photoMode/usePhotoModeController.ts')
s=p.read_text()
checks={
"capture_photo_still_call": "const result = await capturePhotoStill(filterPreset);" in s,
"result_data_in_snapshot": "const historyEntry: PhotoHistoryEntry = { dataUrl: result.dataUrl, title: sceneName, filter: filterPreset }" in s,
"scene_ref_render_assignment": "sceneNameRef.current = sceneName;" in s,
"scene_ref_callback_dependency": ", sceneNameRef" in s or ", sceneName" in s,
}
for k,v in checks.items():
print(f"{k}: {v}")
PYRepository: IETGLIM/Volodka
Length of output: 287
Snapshot capture metadata before the async operation.
capturePhotoStill(filterPreset) captures with the filter at invocation time, so pairing result.dataUrl with mutable filterPresetRef.current and sceneNameRef.current in the history entry can attach newer metadata to an older capture. Snapshot filterPreset and sceneName before starting the capture, pass those snapshots into the history entry, add sceneName to captureScreenshot dependencies, and remove the render-time sceneNameRef.current = sceneName.
Also applies to lines 101-110.
🧰 Tools
🪛 React Doctor (0.9.1)
[error] 57-57: This ref is mutated during render. React can replay or discard render work, so the mutation can leak from UI that never commits.
Move ref writes into an event handler or effect. Render must stay pure because React can replay or discard it. The predictable null-guarded lazy initialization pattern remains supported.
(no-ref-current-in-render)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/game/photoMode/usePhotoModeController.ts` around lines 56 -
57, Update the capture flow around capturePhotoStill and captureScreenshot to
snapshot filterPreset and sceneName before the async capture begins, then use
those snapshots when creating the history entry. Add sceneName to
captureScreenshot’s dependency list, and remove the render-time
sceneNameRef.current assignment and any metadata reads that can observe newer
values.
Source: Linters/SAST tools
| const selectHistoryCapture = useCallback((entry: PhotoCaptureHistoryEntry) => { | ||
| setPreview({ dataUrl: entry.dataUrl, timestamp: entry.timestamp }); | ||
| bumpPreviewTimer(); | ||
| setLiveAnnouncement(`${PHOTO_MODE_LABELS.gallerySelect}: ${entry.sceneName}`); | ||
| }, [bumpPreviewTimer]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Preserve the selected capture’s filter.
Selecting an older entry stores only its URL and timestamp. PhotoModeViewfinder.tsx:304 and this hook’s download/share paths then use the current filterPreset, so selecting a noir capture while the mode is neon previews and exports it with the wrong filter. Store entry.filter in PhotoPreviewData and use that value for preview, download, and share.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/game/photoMode/usePhotoModeController.ts` around lines 126 -
130, Update selectHistoryCapture to store entry.filter in PhotoPreviewData
alongside the dataUrl and timestamp, then update PhotoModeViewfinder and this
hook’s download/share paths to use the selected preview filter instead of the
current filterPreset. Preserve current-filter behavior for newly captured photos
and ensure historical captures preview, download, and share with their stored
filter.
| it('falls back to nearest registered npc', () => { | ||
| groups.set('albert', { | ||
| getWorldPosition: (v) => { v.set(5, 0, 0); }, | ||
| }); | ||
| groups.set('maria', { | ||
| getWorldPosition: (v) => { v.set(1, 0, 0); }, | ||
| }); | ||
| const focus = resolveNearestNpcForRingFocus(null); | ||
| expect(focus?.npcId).toBe('maria'); | ||
| expect(focus?.label).toBe('Мария'); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Exercise the missing recovered-target fallback.
Line 49 passes null, so this does not verify the documented case where a recovered target ID is present but no longer registered. Pass an unknown ID here (or add a case) and retain the nearest-NPC assertion.
- const focus = resolveNearestNpcForRingFocus(null);
+ const focus = resolveNearestNpcForRingFocus('missing-npc');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('falls back to nearest registered npc', () => { | |
| groups.set('albert', { | |
| getWorldPosition: (v) => { v.set(5, 0, 0); }, | |
| }); | |
| groups.set('maria', { | |
| getWorldPosition: (v) => { v.set(1, 0, 0); }, | |
| }); | |
| const focus = resolveNearestNpcForRingFocus(null); | |
| expect(focus?.npcId).toBe('maria'); | |
| expect(focus?.label).toBe('Мария'); | |
| }); | |
| it('falls back to nearest registered npc', () => { | |
| groups.set('albert', { | |
| getWorldPosition: (v) => { v.set(5, 0, 0); }, | |
| }); | |
| groups.set('maria', { | |
| getWorldPosition: (v) => { v.set(1, 0, 0); }, | |
| }); | |
| const focus = resolveNearestNpcForRingFocus('missing-npc'); | |
| expect(focus?.npcId).toBe('maria'); | |
| expect(focus?.label).toBe('Мария'); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/engine/interaction/stuckRecoveryNpcFocus.test.ts` around lines 42 - 52,
Update the test for resolveNearestNpcForRingFocus to pass a non-null, unknown
recovered target ID that is absent from groups, while retaining the assertions
that Maria is selected as the nearest NPC and has the expected label.
| eventBus.emit('interaction:hint', { | ||
| label: focus.label, | ||
| key: 'E', | ||
| description: 'Снова в зоне взаимодействия', | ||
| type: 'npc', | ||
| distance: focus.distance, | ||
| maxRange: Math.max(focus.distance + 0.5, NPC_INTERACTION_QUERY_RANGE), | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not report an out-of-range NPC as interactable.
The fallback has no distance limit, and Line 75 explicitly supports distant NPCs; therefore Line 72 can falsely say the player is “in the interaction zone.” Use a re-approach message when focus.distance > NPC_INTERACTION_QUERY_RANGE.
Proposed fix
+ const inRange = focus.distance <= NPC_INTERACTION_QUERY_RANGE;
eventBus.emit('interaction:hint', {
label: focus.label,
key: 'E',
- description: 'Снова в зоне взаимодействия',
+ description: inRange
+ ? 'Снова в зоне взаимодействия'
+ : 'Подойдите ближе для взаимодействия',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| eventBus.emit('interaction:hint', { | |
| label: focus.label, | |
| key: 'E', | |
| description: 'Снова в зоне взаимодействия', | |
| type: 'npc', | |
| distance: focus.distance, | |
| maxRange: Math.max(focus.distance + 0.5, NPC_INTERACTION_QUERY_RANGE), | |
| }); | |
| const inRange = focus.distance <= NPC_INTERACTION_QUERY_RANGE; | |
| eventBus.emit('interaction:hint', { | |
| label: focus.label, | |
| key: 'E', | |
| description: inRange | |
| ? 'Снова в зоне взаимодействия' | |
| : 'Подойдите ближе для взаимодействия', | |
| type: 'npc', | |
| distance: focus.distance, | |
| maxRange: Math.max(focus.distance + 0.5, NPC_INTERACTION_QUERY_RANGE), | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/engine/interaction/stuckRecoveryNpcFocus.ts` around lines 69 - 76, Update
the interaction hint emission in the stuck-recovery NPC focus flow to use a
re-approach message when focus.distance exceeds NPC_INTERACTION_QUERY_RANGE,
rather than reporting the NPC as being in the interaction zone. Preserve the
existing in-range message for distances within the query range and keep the
remaining hint fields unchanged.
| id: `cap-${entry.timestamp}-${history.length}`, | ||
| }; | ||
| return [next, ...history].slice(0, Math.max(1, max)); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make capture IDs collision-resistant.
Once history is full, two captures with the same millisecond timestamp both generate cap-${timestamp}-6. Duplicate React keys can make gallery selection ambiguous. Use a monotonic counter or UUID independent of history.length, and add a same-timestamp regression test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/engine/photo/photoCaptureHistory.ts` around lines 23 - 25, Update the
capture ID generation in the photo capture history function to use a
collision-resistant identifier independent of history.length, such as a
monotonic counter or UUID, while preserving the existing cap-${timestamp}- style
only if uniqueness remains guaranteed. Add a regression test covering two
captures created within the same millisecond when history is full, verifying
their IDs differ.
| export const RUN_FOV_BOOST = 3.5; | ||
| /** Player speed (m/s) at which FOV boost is at full intensity. */ | ||
| export const RUN_FOV_SPEED_FULL = 5.5; | ||
| export const RUN_FOV_SPEED_FULL = 5.25; | ||
| /** Player speed (m/s) at which FOV boost begins. */ | ||
| export const RUN_FOV_SPEED_MIN = 2.5; | ||
| export const RUN_FOV_SPEED_MIN = 2.35; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep the FOV boost sprint-only.
With RUN_FOV_SPEED_MIN = 2.35, normal movement around 4 m/s already receives approximately a 2° boost, while src/engine/camera/strategies/explorationStrategy.ts documents this effect as engaging only during sprinting. Set the minimum threshold to the actual sprint threshold and keep the full-intensity threshold above it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/engine/player/playerConstants.ts` around lines 58 - 62, Update
RUN_FOV_SPEED_MIN in the player movement constants to match the actual sprint
threshold, preventing normal movement from receiving FOV boost; retain
RUN_FOV_SPEED_FULL above this minimum so full intensity remains reached only at
higher sprint speeds.
1 participant
Summary
last_poem,blind_spot,final_code,machine_confession,echo_of_vladimir,night_before_dawn)chk_campfire_nightash ring); Photo Mode session gallery (in-memory, max 6)Test plan
npm run typechecknpm run lintnpm run test:unitBase:
main(post #43). No merge unless green and simple — user will merge.