Skip to content

fix(agents): advance travel step when walk() detects arrival#95

Merged
Maudfer merged 1 commit into
mainfrom
fix/person-walk-arrival
Jul 12, 2026
Merged

fix(agents): advance travel step when walk() detects arrival#95
Maudfer merged 1 commit into
mainfrom
fix/person-walk-arrival

Conversation

@Maudfer

@Maudfer Maudfer commented Jul 12, 2026

Copy link
Copy Markdown
Owner

The bug

Person.walk() clears currentTarget/currentDestination the instant it detects final arrival (path exhausted + target reached). But processTravel()'s WalkingToCar and WalkingToDestination cases called walk() and then immediately re-checked isDestinationReached() — which now reads the just-wiped state (isCurrentTargetReached() returns false with a null currentTarget) 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) force travelStep manually instead of letting arrival emerge from movement — they were working around the stall.

The fix

walk() now returns true on the frame it detects arrival (and false otherwise). The two travel cases advance on that return value instead of re-querying the state walk() just cleared:

case TravelStep.WalkingToCar:
    if (this.walk(currentTile, timeDelta)) {
        this.travelStep = TravelStep.EnteringCar;
    }
    break;

This is minimal and preserves every other path:

  • Wandering (update()'s non-travel branch) still calls walk() and ignores the return — clearing semantics unchanged, so debug wanderers still re-roll a destination.
  • Manually-driven travel tests (personTravel.test.ts, commute.test.ts) are untouched and still green.
  • walk() is only called from Person.ts + the agents tests, so the voidboolean change is fully backward compatible.

Test

Rewrote the person.test.ts "walk(): real per-frame movement" test to drive arrival through update()/processTravel() with real movement only (no manual travelStep poking) and assert the observable state transitions:

  • WalkingToDestinationArrived
  • WalkingToCarEnteringCar

Revert-dance (test guards the fix)

Confirmed the fail→pass transition:

  • Source fix stashed (Person.ts reverted), test kept: 2 failed, 94 passed — exactly the two new regression tests fail, nothing else regresses.
  • Fix restored: 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 — clean
  • npx eslint test/agents src/app/game/agents --max-warnings 0 — clean
  • npx jest --selectProjects agents — 96 passed (incl. personTravel.test.ts + commute.test.ts)
  • agents module coverage: 93.26% stmts (Person.ts 92.54%) — above the 80% floor

Base is chore/game-test-reorg (not main): the src/app/game/agents/ layout, per-module Jest projects, and the blocking coverage gate this task targets only exist on the reorg branch. Rebase onto main once the reorg lands.

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>
@Maudfer Maudfer changed the base branch from chore/game-test-reorg to main July 12, 2026 01:52
@Maudfer Maudfer closed this Jul 12, 2026
@Maudfer Maudfer reopened this Jul 12, 2026
@Maudfer Maudfer merged commit a18471b into main Jul 12, 2026
18 checks passed
@Maudfer Maudfer self-assigned this Jul 12, 2026
@Maudfer Maudfer added the bug Something isn't working label Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant