feat(26.2): World Alive — A3/B2 roam + Animal Crossing gap fix - #19
Merged
Conversation
- Unlock zone-scale wander (A3 / D-19–D-21); leash becomes escape-only - Keep dual SSOT in lockstep (D-02); coords/facing unchanged Co-authored-by: Cursor <cursoragent@cursor.com>
- Pin disk spawns.json ≡ defaultBeginningFieldsBundle ≡ maxRadius 40 - Drift-sensitive so A3 unlock stays locked in CI (D-02) Co-authored-by: Cursor <cursoragent@cursor.com>
- D-09: less linger standstill so A3 zone roam feels livelier Co-authored-by: Cursor <cursoragent@cursor.com>
- Additive pure gates: wander 55%, linger (stationary/poi) 30%
- Hash domain ambient-step:${npcId}:${gameMinute}; bucket loop untouched
Co-authored-by: Cursor <cursoragent@cursor.com>
- Deterministic 1440-minute scans for ~55% wander / ~30% linger - Co-pass and per-NPC desync assertions (D-22/D-24); stepActiveMinute helper Co-authored-by: Cursor <cursoragent@cursor.com>
- Replace hashNpcBucket gate with shouldStepThisTick (joinVicinity bypass for D-11) - Rewrite multi-mover and maxRadius-0 pin tests; drop distinct-bucket test Co-authored-by: Cursor <cursoragent@cursor.com>
- Add source discriminator on resolveMovementTarget; skip leash when source is join - Cover D-11 gate+leash bypass and A3 in-region radius-40 clamp neutrality Co-authored-by: Cursor <cursoragent@cursor.com>
- Uniform maxRadius 40 table; drop dead per-zone 3/4/5 soft anchors - Document B2 shouldStepThisTick (~55/30), LINGER_PAUSE_PERCENT 15, multi-mover Co-authored-by: Cursor <cursoragent@cursor.com>
- Tick 消费: shouldStepThisTick 55/30 + multi-mover supersedes exclusive bucket - Join: D-11 bypass of probability gate and soft leash; pin still wins - Guardrail #109: forbid reintroducing D-MAP-AMB-03 exclusive buckets Co-authored-by: Cursor <cursoragent@cursor.com>
Close World Alive UAT gaps: walk/pause cadence, zone commute, never-stack targets, full-home wander schedules by persona, gridDebug zone overlays, and harden verify:phase8 dual NL adjacency. Co-authored-by: Cursor <cursoragent@cursor.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 29 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe pull request updates ambient NPC schedules and movement, adds Beginning Fields zone and spawn data, centralizes drawer state, improves NPC animation synchronization, and extracts worker-state, memory, enrichment, caching, and prompt-context logic into dedicated modules. ChangesAmbient movement and Beginning Fields data
Web movement and shell UI
Worker fetch and prompt context
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant GameRoom
participant runAmbientTick
participant zone-wander
participant NPC
GameRoom->>runAmbientTick: execute ambient tick
runAmbientTick->>zone-wander: resolve available movement target
zone-wander-->>runAmbientTick: return target cell
runAmbientTick->>NPC: move at most one grid cell
NPC-->>runAmbientTick: arrive and enter pause state
sequenceDiagram
participant GraphWorker
participant fetch_state_and_memory
participant WorkerStateCache
participant MemoryContext
participant PromptBuilder
GraphWorker->>fetch_state_and_memory: prepare speak state
fetch_state_and_memory->>WorkerStateCache: fetch worker snapshot
fetch_state_and_memory->>MemoryContext: load memory and recall data
WorkerStateCache-->>fetch_state_and_memory: worker state
MemoryContext-->>fetch_state_and_memory: memory context
fetch_state_and_memory->>PromptBuilder: build system context
PromptBuilder-->>GraphWorker: assembled system prompt
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Split worker speak/state helpers out of npc_loop, move ChatPage drawer auto-open into useShellDrawerState, and refresh lpc-npc-9 sprite. Co-authored-by: Cursor <cursoragent@cursor.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (1)
apps/game-server/src/ambient/zone-wander.ts (1)
100-121: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winOptimize cell selection by eliminating the O(N log N) sort and redundant distance calculations.
The current implementation invokes
minDistToOccupiedmultiple times per cell during thesort()operation. Given thatcandidatescan be up to 256 cells andoccupiedup to 12 cells, this generates thousands of redundant iterations per ambient tick.Since the goal is only to pick a random cell among those with the maximum distance, you can compute distances in a single O(N) pass and skip sorting entirely. Because
collectZoneWalkablegenerates the pool already sorted by coordinates, removing the sort maintains the exact same deterministic baseline order prior to the random selection.♻️ Proposed O(N) optimization
export function pickSpaciousCell( pool: readonly GridCell[], occupied: readonly GridCell[], reserved: readonly GridCell[], ): GridCell | null { if (pool.length === 0) return null; const blocked = [...occupied, ...reserved]; - const free = pool.filter((c) => !cellTaken(c, blocked)); - const usable = free.length > 0 ? free : pool; - const spacious = usable.filter((c) => minDistToOccupied(c, occupied) >= PERSONAL_SPACE); - const ranked = (spacious.length > 0 ? spacious : usable).slice(); - ranked.sort((a, b) => { - const da = minDistToOccupied(a, occupied); - const db = minDistToOccupied(b, occupied); - if (db !== da) return db - da; - return a.x - b.x || a.y - b.y; - }); - // Soft random among top-scored peers so destinations aren't identical every tick. - const bestScore = minDistToOccupied(ranked[0]!, occupied); - const top = ranked.filter((c) => minDistToOccupied(c, occupied) === bestScore); - return top[Math.floor(Math.random() * top.length)] ?? null; + + // Precompute distances and filter free cells in a single pass + const poolStats = pool.map(c => ({ + cell: c, + isFree: !cellTaken(c, blocked), + dist: minDistToOccupied(c, occupied), + })); + + const free = poolStats.filter(c => c.isFree); + const usable = free.length > 0 ? free : poolStats; + const spacious = usable.filter(c => c.dist >= PERSONAL_SPACE); + const candidates = spacious.length > 0 ? spacious : usable; + + let bestScore = -1; + for (const c of candidates) { + if (c.dist > bestScore) bestScore = c.dist; + } + + const top = candidates.filter(c => c.dist === bestScore); + return top[Math.floor(Math.random() * top.length)]?.cell ?? null; }🤖 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 `@apps/game-server/src/ambient/zone-wander.ts` around lines 100 - 121, Optimize pickSpaciousCell by replacing the ranked.sort flow and repeated minDistToOccupied calls with a single pass over the selected usable candidates that tracks the maximum distance and the cells sharing it. Preserve the existing pool/free/usable and PERSONAL_SPACE fallback behavior, then randomly return one of the maximum-distance candidates; rely on collectZoneWalkable’s coordinate ordering for deterministic baseline behavior.
🤖 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 `@apps/game-server/src/ambient/tick.test.ts`:
- Around line 471-529: The join-vicinity test currently exercises the legacy
map.player fallback instead of the live player registry. Update the test around
runAmbientTick to register two players through the map’s players registry, use
one as the join initiator, place the other separately, and assert the NPC moves
toward the initiator while preserving the existing bypass and target-selection
setup.
In `@apps/game-server/src/ambient/tick.ts`:
- Around line 39-42: Update runAmbientTick to call shouldStepThisTick before
invoking stepNpcTowardTarget for walking NPCs, skipping movement when the gate
returns false. Preserve join_vicinity as an explicit bypass so that action
continues without applying the deterministic mobility gate.
- Around line 293-295: Update the NPC target-selection flow around
reservedTargets to seed reservations with destinations from active walking
entries in ambientMotion before iterating map.npcs. Ensure persisted walking
destinations are included in the same reservation checks used for newly selected
targets, preserving the never-stack rule while retaining current-loop
reservations.
- Around line 44-52: Update AmbientMotion and the ambient tick handling to
associate each held walk with the current schedule segment using
segmentKey(segment), then invalidate or clear motion when the segment changes so
stale targets are not followed. Preserve existing pause and walk timeout
behavior within the same segment.
In `@apps/web/src/game/roomSceneInput.ts`:
- Around line 62-65: Update zonesAtCell to call getCouncilSpawnSlots inside a
try/catch, returning an empty zone list when it throws while the registry is
still loading. Preserve the existing registry lookup and normal processing when
the call succeeds.
In `@apps/web/src/hooks/useShellDrawerState.ts`:
- Line 1: Remove the redundant openChronicle function from useShellDrawerState
and omit it from the hook return value. In ChatPage, stop destructuring
openChronicle, replace its invocation with openDrawer("chronicle"), and remove
it from the callback dependencies so the existing drawer flow clears chronicle
unread state.
In `@docs/CONTRACTS.md`:
- Line 104: Update the Tick 消费 contract to remove idle from the movement-skip
condition, documenting only resting as gated to zero. Keep the existing
shouldSkipMovement runtime behavior and the per-NPC movement probability rules
unchanged.
In `@workers/agent-worker/src/graph/speak_fetch.py`:
- Around line 312-318: Remove the recalled-memory content from the diagnostic
print in the recent-only miss branch of the memory-context recall flow.
Eliminate the preview extraction and preview field, while retaining only
non-sensitive identifiers and row counts such as room, NPC, player, and recent
length.
In `@workers/agent-worker/src/graph/worker_state_fetch.py`:
- Around line 34-35: Update the worker-state cache flow around
_worker_state_stale_key and its read/write paths to distinguish full snapshots
from skipNearbyLore=1 projections, preventing a partial snapshot from satisfying
a full fetch. Ensure action writes invalidate or update every projection key,
including the additional call sites noted in the diff.
- Line 20: Update the process-global _stale_worker_snapshots cache and its
read/write logic around the affected snapshot helpers to enforce both TTL
expiration and a maximum entry count. Prune expired entries during writes, then
evict the oldest entries when the cache exceeds the configured bound, while
preserving valid snapshot retrieval behavior.
---
Nitpick comments:
In `@apps/game-server/src/ambient/zone-wander.ts`:
- Around line 100-121: Optimize pickSpaciousCell by replacing the ranked.sort
flow and repeated minDistToOccupied calls with a single pass over the selected
usable candidates that tracks the maximum distance and the cells sharing it.
Preserve the existing pool/free/usable and PERSONAL_SPACE fallback behavior,
then randomly return one of the maximum-distance candidates; rely on
collectZoneWalkable’s coordinate ordering for deterministic baseline behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b3736c2c-b5ea-4e53-985f-1af0a161188a
⛔ Files ignored due to path filters (1)
apps/web/public/assets/sprites/lpc-npc-9.pngis excluded by!**/*.png
📒 Files selected for processing (40)
apps/game-server/data/schedules/README.mdapps/game-server/data/schedules/npc-1.jsonapps/game-server/data/schedules/npc-10.jsonapps/game-server/data/schedules/npc-11.jsonapps/game-server/data/schedules/npc-12.jsonapps/game-server/data/schedules/npc-2.jsonapps/game-server/data/schedules/npc-3.jsonapps/game-server/data/schedules/npc-4.jsonapps/game-server/data/schedules/npc-5.jsonapps/game-server/data/schedules/npc-6.jsonapps/game-server/data/schedules/npc-7.jsonapps/game-server/data/schedules/npc-8.jsonapps/game-server/data/schedules/npc-9.jsonapps/game-server/data/world/beginning-fields@v1/spawns.jsonapps/game-server/data/world/beginning-fields@v1/zones.jsonapps/game-server/src/ambient/README.mdapps/game-server/src/ambient/schedule.test.tsapps/game-server/src/ambient/schedule.tsapps/game-server/src/ambient/tick.test.tsapps/game-server/src/ambient/tick.tsapps/game-server/src/ambient/zone-wander.test.tsapps/game-server/src/ambient/zone-wander.tsapps/game-server/src/colyseus/GameRoom.tsapps/game-server/src/world/council-spawn-radius.test.tsapps/web/src/ChatPage.tsxapps/web/src/game/roomSceneInput.tsapps/web/src/hooks/useShellDrawerState.tsdocs/BEGINNING-FIELDS.mddocs/CONTRACTS.mddocs/ISSUE-LOG.mdpackages/shared/src/worldRegion.tsscripts/verify-phase8.mjsworkers/agent-worker/src/graph/nodes/llm_social_turn.pyworkers/agent-worker/src/graph/npc_loop.pyworkers/agent-worker/src/graph/prompt.pyworkers/agent-worker/src/graph/speak_fetch.pyworkers/agent-worker/src/graph/speak_system_context.pyworkers/agent-worker/src/graph/worker_state_fetch.pyworkers/agent-worker/tests/test_fetch_state_and_memory.pyworkers/agent-worker/tests/test_load_memory_recall_fallback.py
Comment on lines
+62
to
+65
| function zonesAtCell(gx: number, gy: number): string[] { | ||
| void getCouncilSpawnSlots(); | ||
| const registry = getWorldRegistry(); | ||
| if (!registry) return []; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Wrap getCouncilSpawnSlots in a try/catch block.
getCouncilSpawnSlots throws an error if the registry is not yet populated. Since zonesAtCell is called frequently on pointermove to update the HUD, an unhandled exception here will crash the input event handler when the map is first loading.
🐛 Proposed fix
function zonesAtCell(gx: number, gy: number): string[] {
- void getCouncilSpawnSlots();
+ try {
+ void getCouncilSpawnSlots();
+ } catch {
+ // registry not ready
+ }
const registry = getWorldRegistry();
if (!registry) return [];📝 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.
Suggested change
| function zonesAtCell(gx: number, gy: number): string[] { | |
| void getCouncilSpawnSlots(); | |
| const registry = getWorldRegistry(); | |
| if (!registry) return []; | |
| function zonesAtCell(gx: number, gy: number): string[] { | |
| try { | |
| void getCouncilSpawnSlots(); | |
| } catch { | |
| // registry not ready | |
| } | |
| const registry = getWorldRegistry(); | |
| if (!registry) return []; |
🤖 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 `@apps/web/src/game/roomSceneInput.ts` around lines 62 - 65, Update zonesAtCell
to call getCouncilSpawnSlots inside a try/catch, returning an empty zone list
when it throws while the registry is still loading. Preserve the existing
registry lookup and normal processing when the call succeeds.
| _FETCH_STATE_ATTEMPTS = 2 | ||
| _FETCH_STATE_HOT_CACHE_TTL_S = 3.0 | ||
| _STALE_SNAPSHOT_TTL_S = 300.0 | ||
| _stale_worker_snapshots: dict[str, tuple[dict[str, Any], float]] = {} |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Bound and evict the process-global snapshot cache.
Expired entries remain in _stale_worker_snapshots forever. A long-lived worker handling new room/player IDs therefore grows memory without limit. Add size-bounded TTL eviction or periodic pruning on writes.
Also applies to: 38-54
🤖 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 `@workers/agent-worker/src/graph/worker_state_fetch.py` at line 20, Update the
process-global _stale_worker_snapshots cache and its read/write logic around the
affected snapshot helpers to enforce both TTL expiration and a maximum entry
count. Prune expired entries during writes, then evict the oldest entries when
the cache exceeds the configured bound, while preserving valid snapshot
retrieval behavior.
Use NPC_GRID_STEP_MS=600 so each cell covers a full LPC walk cycle; enable npcWorldLive from Colyseus roomNpcs; harden ambient spacing, chronicle drawer, and worker-state cache keys from PR CR follow-ups. Co-authored-by: Cursor <cursoragent@cursor.com>
Assert LINGER_PAUSE_PERCENT === 15; leave RED B2 wire canaries that prove shouldStepThisTick is not called from runAmbientTick (45b6455). Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apps/game-server/src/ambient/tick.test.ts (1)
651-709: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExercise join-vicinity through
players.The join-vicinity test appears to exercise the legacy
map.playerfallback instead of the live player registry. As per coding guidelines, human positions live inplayers, not the loneRoomState.player. Update the test aroundrunAmbientTickto register two players through the map'splayersregistry, use one as the join initiator, place the other separately, and assert the NPC moves toward the initiator while preserving the existing bypass and target-selection setup.If this has already been addressed, please disregard. Run the script below to verify the current state of the test.
#!/bin/bash # Description: Verify if the join-vicinity test has been updated to use the player registry. sed -n '651,710p' apps/game-server/src/ambient/tick.test.ts🤖 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 `@apps/game-server/src/ambient/tick.test.ts` around lines 651 - 709, Update the join-vicinity test around runAmbientTick to use the map.players registry instead of map.player: register two players, place one as the join initiator at the existing target location and the other separately, then assert the NPC moves toward the initiator. Preserve the existing schedule bypass, soft-leash, obstacle placement, and distance assertions.Source: Coding guidelines
🤖 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 `@apps/game-server/src/ambient/tick.test.ts`:
- Line 340: Remove the duplicate recentNpcCells declaration in the affected test
setup, keeping only one Map<string, { x: number; y: number }[]> declaration and
preserving all existing references to it.
In `@apps/web/src/game/entitySprites.ts`:
- Around line 369-370: Update playLpcWalkAnim() so repeated calls for player
avatars do not restart the currently playing LPC walk animation; use the
existing playback API behavior that preserves an active matching animation while
still allowing idle-to-walk transitions. Keep spriteProfileForPlayer() and the
lpc-player-1 animation selection unchanged.
In `@apps/web/src/game/roomSceneNpcMotion.ts`:
- Line 191: Update the path-continuation logic around continuing so a queued
pendingGridX/pendingGridY target stops the current path after the active step
completes. Preserve the current-step completion behavior, then exit the
remaining path and invoke resumeFromPendingOrIdle() so the queued target is
handled immediately.
---
Duplicate comments:
In `@apps/game-server/src/ambient/tick.test.ts`:
- Around line 651-709: Update the join-vicinity test around runAmbientTick to
use the map.players registry instead of map.player: register two players, place
one as the join initiator at the existing target location and the other
separately, then assert the NPC moves toward the initiator. Preserve the
existing schedule bypass, soft-leash, obstacle placement, and distance
assertions.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1d71c17d-449a-4790-89fa-0c4ef3cedd63
📒 Files selected for processing (22)
apps/game-server/src/ambient/tick.test.tsapps/game-server/src/ambient/tick.tsapps/game-server/src/colyseus/GameRoom.tsapps/web/src/ChatPage.tsxapps/web/src/game/RoomScene.tsapps/web/src/game/entitySprites.tsapps/web/src/game/gridMovement.npcCatchup.test.tsapps/web/src/game/gridMovement.tsapps/web/src/game/lpcNpc1Sheet.test.tsapps/web/src/game/roomSceneNpcMotion.tsapps/web/src/game/roomSceneSync.tsapps/web/src/game/roomSceneTypes.tsapps/web/src/hooks/useShellDrawerState.test.tsapps/web/src/hooks/useShellDrawerState.tsdocs/CONTRACTS.mddocs/ISSUE-LOG.mdscripts/uat-pr19-cr-screenshots.mjsworkers/agent-worker/src/graph/npc_loop.pyworkers/agent-worker/src/graph/speak_fetch.pyworkers/agent-worker/src/graph/worker_state_fetch.pyworkers/agent-worker/tests/test_fetch_state_and_memory.pyworkers/agent-worker/tests/test_load_memory_recall_fallback.py
💤 Files with no reviewable changes (1)
- apps/web/src/hooks/useShellDrawerState.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- apps/game-server/src/colyseus/GameRoom.ts
- workers/agent-worker/src/graph/worker_state_fetch.py
- apps/web/src/ChatPage.tsx
- workers/agent-worker/src/graph/speak_fetch.py
- workers/agent-worker/tests/test_fetch_state_and_memory.py
- docs/CONTRACTS.md
- apps/game-server/src/ambient/tick.ts
- workers/agent-worker/src/graph/npc_loop.py
|
|
||
| // Linger pool around (10,10) ≤2 includes reservedDest (12,10). Fill recent so | ||
| // the non-reserved alternative peers are less sticky; reservation must still win. | ||
| const recentNpcCells = new Map<string, { x: number; y: number }[]>(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Remove duplicate declaration.
recentNpcCells is declared twice, which will cause a SyntaxError (Identifier has already been declared) and break the build.
🐛 Proposed fix
- const recentNpcCells = new Map<string, { x: number; y: number }[]>();
- const recentNpcCells = new Map<string, { x: number; y: number }[]>();
+ const recentNpcCells = new Map<string, { x: number; y: number }[]>();📝 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.
Suggested change
| const recentNpcCells = new Map<string, { x: number; y: number }[]>(); | |
| const recentNpcCells = new Map<string, { x: number; y: number }[]>(); |
🤖 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 `@apps/game-server/src/ambient/tick.test.ts` at line 340, Remove the duplicate
recentNpcCells declaration in the affected test setup, keeping only one
Map<string, { x: number; y: number }[]> declaration and preserving all existing
references to it.
Comment on lines
+369
to
+370
| // Always (re)start walk — ignoreIfPlaying=false so idle→walk always swaps frames. | ||
| ent.avatar.play(key, false); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== file existence =='
git ls-files 'apps/web/src/game/entitySprites.ts' 'apps/web/src/game' | sed -n '1,120p'
echo
echo '== outline entitySprites.ts =='
ast-grep outline apps/web/src/game/entitySprites.ts --view expanded || true
echo
echo '== targeted search: play(key, false), isNpc, lpc-player-1 =='
rg -n "play\\(key, false\\)|isNpc|lpc-player-1|GRID_STEP_MS|ignoreIfPlaying" apps/web/src/game apps/web/src -g '!**/dist/**' -g '!**/build/**' || true
echo
echo '== line numbers around 350-390 =='
sed -n '340,390p' apps/web/src/game/entitySprites.tsRepository: moyunzero/AetherLife
Length of output: 17485
🏁 Script executed:
#!/bin/bash
set -euo pipefail
pwd
git ls-files 'apps/web/src/game/entitySprites.ts' | cat
sed -n '340,390p' apps/web/src/game/entitySprites.tsRepository: moyunzero/AetherLife
Length of output: 1947
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== entitySprites.ts line map =='
nl -ba apps/web/src/game/entitySprites.ts | sed -n '330,390p'
echo
echo '== references to entitySprites walk helper =='
rg -n -C 3 "ent\.avatar\.play\(key, false\)|ent\.avatar\.play\(key|isNpc|walk" apps/web/src/game/entitySprites.ts apps/web/src/game -g '!**/dist/**' -g '!**/build/**' || trueRepository: moyunzero/AetherLife
Length of output: 230
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== lpc profile mapping =='
rg -n -C 3 "isLpcProfile|lpc-player-1|spriteProfileForPlayer|LpcNpcSpriteProfile|spriteProfileForNpc" apps/web/src/game/lpcNpc1Sheet.ts apps/web/src/game/entitySprites.ts
echo
echo '== playWalkAnim callers =='
rg -n -C 3 "playWalkAnim\\(" apps/web/src/game apps/web/src -g '!**/dist/**' -g '!**/build/**'
echo
echo '== player sprite creation and profile assignment =='
rg -n -C 4 "createPlayerSprite|spriteProfileForPlayer|spriteProfileForNpc|spriteProfile" apps/web/src/game/entitySprites.ts apps/web/src/game/RoomScene.ts apps/web/src/game/roomScene*.tsRepository: moyunzero/AetherLife
Length of output: 25452
Preserve continuous LPC walk playback for players. apps/web/src/game/entitySprites.ts:362-370 — playLpcWalkAnim() is used by spriteProfileForPlayer() (lpc-player-1), so ent.avatar.play(key, false) restarts the same 675ms gait on every 200ms step and will stutter. Keep the loop continuous for player avatars.
🤖 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 `@apps/web/src/game/entitySprites.ts` around lines 369 - 370, Update
playLpcWalkAnim() so repeated calls for player avatars do not restart the
currently playing LPC walk animation; use the existing playback API behavior
that preserves an active matching animation while still allowing idle-to-walk
transitions. Keep spriteProfileForPlayer() and the lpc-player-1 animation
selection unchanged.
Source: Coding guidelines
| } | ||
| }, | ||
| beginNpcStepTween(ctx, ent, cell.x, cell.y, () => { | ||
| const continuing = stepIndex < path.length; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Abort the stale path if a new target is queued mid-step.
The comment explicitly states: "Queue at most one follow-up cell; finish current step first." However, the loop only checks stepIndex < path.length and will stubbornly finish the entire remaining multi-cell path before evaluating the new pendingGridX target.
If a new target is queued, we should abort the remaining path cells and yield to resumeFromPendingOrIdle() immediately after the current step finishes.
🐛 Proposed fix to respect the queued interruption
- const continuing = stepIndex < path.length;
+ const continuing = stepIndex < path.length && ent.pendingGridX == null;📝 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.
Suggested change
| const continuing = stepIndex < path.length; | |
| const continuing = stepIndex < path.length && ent.pendingGridX == null; |
🤖 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 `@apps/web/src/game/roomSceneNpcMotion.ts` at line 191, Update the
path-continuation logic around continuing so a queued pendingGridX/pendingGridY
target stops the current path after the active step completes. Preserve the
current-step completion behavior, then exit the remaining path and invoke
resumeFromPendingOrIdle() so the queued target is handled immediately.
Re-wire shouldStepThisTick for new strolls; mid-walk and join bypass. Align C-06, ambient README, Guardrail #111; keep Nyquist canaries green. Co-authored-by: Cursor <cursoragent@cursor.com>
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
maxRadiusto 40 (A3), replace exclusive 12-bucket with B2 multi-NPC step gating, join_vicinity soft-leash bypass, docs/C-06/Guardrail #109.PERSONAL_SPACE), persona schedules with full-homebeginning-fields@v1:homewander.?gridDebug=1draws home/orchard/plaza/pond zone rects; hardenverify:phase8dual-NL adjacency flake.Test plan
pnpm --filter @aetherlife/shared buildpnpm --filter @aetherlife/game-server test -- src/ambient/pnpm --filter @aetherlife/shared test(if touching spawn/radius)pnpm dev:stack, new roomId, openhttp://localhost:5173/?gridDebug=1— confirm zone overlayspnpm verify:phase8on real stack (noLLM_MOCK) when readyMade with Cursor
Summary by CodeRabbit
New Features
Bug Fixes