fix(agents): advance travel step when walk() detects arrival#95
Merged
Conversation
Person.walk() cleared currentTarget/currentDestination the instant it detected final arrival, but processTravel()'s WalkingToCar and WalkingToDestination cases then called isDestinationReached() — which read that already-wiped state and returned a false negative. Driven by real per-frame walk() movement alone, those steps could stall forever (existing travel tests only passed by forcing travelStep manually). Fix: walk() now returns true on the frame it detects arrival, and the two travel cases advance on that return value instead of re-querying the state walk() just cleared. Wandering (update()) and the manually-driven travel paths are unchanged. Updated the person.test.ts "real per-frame movement" test to drive arrival through update()/processTravel() (no manual travelStep poking) and assert the observable WalkingToDestination->Arrived and WalkingToCar->EnteringCar transitions. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
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.
The bug
Person.walk()clearscurrentTarget/currentDestinationthe instant it detects final arrival (path exhausted + target reached). ButprocessTravel()'sWalkingToCarandWalkingToDestinationcases calledwalk()and then immediately re-checkedisDestinationReached()— which now reads the just-wiped state (isCurrentTargetReached()returns false with a nullcurrentTarget) and returns a false negative.Net effect: driven by real per-frame
walk()movement alone, those two travel steps could never advance to the next step. This is why the existing travel tests (test/agents/personTravel.test.ts,commute.test.ts) forcetravelStepmanually instead of letting arrival emerge from movement — they were working around the stall.The fix
walk()now returnstrueon the frame it detects arrival (andfalseotherwise). The two travel cases advance on that return value instead of re-querying the statewalk()just cleared:This is minimal and preserves every other path:
update()'s non-travel branch) still callswalk()and ignores the return — clearing semantics unchanged, so debug wanderers still re-roll a destination.personTravel.test.ts,commute.test.ts) are untouched and still green.walk()is only called fromPerson.ts+ the agents tests, so thevoid→booleanchange is fully backward compatible.Test
Rewrote the
person.test.ts"walk(): real per-frame movement" test to drive arrival throughupdate()/processTravel()with real movement only (no manualtravelSteppoking) and assert the observable state transitions:WalkingToDestination→ArrivedWalkingToCar→EnteringCarRevert-dance (test guards the fix)
Confirmed the fail→pass transition:
Person.tsreverted), test kept:2 failed, 94 passed— exactly the two new regression tests fail, nothing else regresses.96 passed, 96 total.A test that guards nothing would pass both ways; these fail without the fix and pass with it.
Gates
npx tsc --noEmit -p tsconfig.json— cleannpx eslint test/agents src/app/game/agents --max-warnings 0— cleannpx jest --selectProjects agents— 96 passed (incl.personTravel.test.ts+commute.test.ts)agentsmodule coverage: 93.26% stmts (Person.ts 92.54%) — above the 80% floor