fix(colony-sim): make haul-speed reward actually move workers faster (issue #414)

[itsmiso-ai]

Issue: [P2] build_hut goal grants a haul-speed reward that no code applies — workers always move one tile per tick (#414)

What changed

build_hut granted a haul-speed reward with magnitude 1.5, but
ColonySim.step_worker always advanced a worker exactly one tile per
tick and GoalReward.get_haul_speed_multiplier() had no production
caller. The event feed promised "Reward: haul speed +10%" and the
player got nothing.

This PR closes that loop two ways:

• step_worker now consults the multiplier. It advances
  ceil(multiplier) tiles per tick toward the worker's target, breaking
  early once the worker reaches the target. With the catalogued 1.5x
  multiplier a worker closes 2 tiles of Manhattan distance per tick
  instead of 1, giving the goal reward a measurable effect on movement.
• The catalog label is fixed. "haul speed +10%" → "haul speed +50%"
  so the wording matches the documented 1.5x magnitude.

Also drops the dead REWARD_GATHER_SPEED type: the constant, its
get_gather_speed_multiplier getter, the DURATION_GATHER_SPEED /
REWARD_MAGNITUDES[gather_speed] entries, and its slot in
_known_reward_types(). The issue notes this reward had no catalog grant
and no consumer — the same pattern #335 used to justify removing
RotatingGoal.update_resource_progress. Keeping the parallel,
untested-in-production API invited the same confusion about which path
was authoritative.

Tests

• tests/test_goal_reward.gd
  • Drop test_gather_speed_multiplier alongside the getter it tested.
  • Add test_step_worker_advances_one_tile_per_tick_without_reward as
    a regression guard against breaking baseline movement.
  • Add test_step_worker_advances_two_tiles_per_tick_with_reward which
    drives step_worker directly with the haul-speed reward active and
    asserts the worker closes >=2 tiles per tick — the issue's
    acceptance criterion for the multiplier being consumed by the sim
    path, not just exposed by the getter.
  • Update format_active_rewards label reference to match the new
    +50% wording.
• tests/test_rotating_goal.gd: update test_reward_preview_text's
  label reference.

All 33 script test suites pass under headless Godot 4.7.1.

Local validation

• bash-driven run of every tests/test_*.gd (33 suites) under
  headless Godot 4.7.1: every suite reports 0 failed.
• Smoke test (godot --headless --fixed-fps 60 --quit-after 300) exits
  clean with no SCRIPT ERROR / ERROR: lines.
• No parse errors anywhere in the touched files.

External verification still pending

• CI on courier/misospace/windowstead/issue-414 (Script test suite,
  macOS validation, three export validation jobs).
• AI review via pr-reviewer-action.

Files

• scripts/colony_sim.gd — step_worker now advances N tiles per tick.
• scripts/goal_reward.gd — label fix; drop dead REWARD_GATHER_SPEED
  code paths.
• scripts/game_state.gd — drop REWARD_GATHER_SPEED from
  _known_reward_types().
• tests/test_goal_reward.gd — drop gather test, add sim-path tests,
  update label reference.
• tests/test_rotating_goal.gd — update label reference.

Fixes #414

[its-saffron]

AI Automated Review

Full PR review.

Analysis engine: MiniMax-M3 (anthropic) — primary route · pr-reviewer-action v3.4.1

Findings (1 minor)

Severity	Location	Finding
Minor	docs/SPEC.md	Docs/SPEC milestone table still lists `haul speed +10%` for `build_hut`; the PR changed the catalog label to `haul speed +50%` to match the 1.5x magnitude, so the spec row is now stale relative to the catalog.

Recommendation

Approve. The PR closes the audit loop from PR 414: the haul-speed reward now measurably changes worker movement (worker closes 2 tiles of Manhattan distance per tick under the catalogued 1.5x multiplier), the label/magnitude mismatch is fixed (+10% → +50% to match the documented 1.5x), and the dead REWARD_GATHER_SPEED half of the API is removed in the spirit of the PR 335 / PR 342 cleanup that the issue itself cites as precedent. Production callers and consumers in the diff are internally consistent and the citation chain (step_worker → GoalReward.get_haul_speed_multiplier → REWARD_HAUL_SPEED magnitude) matches what the tests drive.

Change-by-change findings

• scripts/colony_sim.gd:443-453 — step_worker consumes the multiplier. The new loop calls step_toward(current, target) up to ceil(speed_multiplier) times, breaks early if step_toward returns current (i.e., we've hit the target), and writes the final position back. With the catalogued 1.5x multiplier and a worker that is more than one tile from its target, this advances the worker up to 2 Manhattan tiles per tick — exactly the acceptance criterion in PR 414 ("N >= 2 when the reward is active"). The maxi(..., 1) floor keeps baseline movement at 1 tile per tick when the multiplier is at its no-reward default of 1.0, preserving the regression test.
• scripts/goal_reward.gd — REWARD_GATHER_SPEED, DURATION_GATHER_SPEED, REWARD_MAGNITUDES[REWARD_GATHER_SPEED], and get_gather_speed_multiplier are removed (no remaining references anywhere in the repo: git_grep confirmed zero matches for REWARD_GATHER_SPEED and get_gather_speed_multiplier after the diff). The catalog label for build_hut is updated from "haul speed +10%" to "haul speed +50%" so the wording matches the 1.5 magnitude.
• scripts/game_state.gd:606 — _known_reward_types() drops REWARD_GATHER_SPEED, keeping the set aligned with what REWARD_CATALOG and REWARD_MAGNITUDES actually grant.
• tests/test_goal_reward.gd — adds two new sim-path tests that fulfill the issue's "tests must exercise the sim path, not just the getter" requirement. Both tests assert against the worker's Manhattan distance before/after one step_worker call:
  • test_step_worker_advances_one_tile_per_tick_without_reward → distance == 1 (regression guard).
  • test_step_worker_advances_two_tiles_per_tick_with_reward → distance >= 2. The 5×5 grid with the worker at (0,0) and target at (4,4) gives 8 Manhattan distance, so step_toward will keep advancing across both axes until it's exhausted the budget — the >= 2 lower bound is satisfied deterministically.
    The dropped test_gather_speed_multiplier and updated format_active_rewards label reference are consistent with the production changes.
• tests/test_rotating_goal.gd:251-256 — updates the test_reward_preview_text expectation to the new "Reward: haul speed +50%" wording.

Sources

• Diff (5 files): scripts/colony_sim.gd, scripts/goal_reward.gd, scripts/game_state.gd, tests/test_goal_reward.gd, tests/test_rotating_goal.gd.
• Issue PR 414 body and acceptance criteria (cross-referenced above).
• Tool harness evidence: read_file of the touched files, plus repo-wide git_grep for REWARD_GATHER_SPEED, get_gather_speed_multiplier, speed_multiplier, step_toward/step_worker, and the old/new label strings. The speed_multiplier grep returns exactly the two new lines in scripts/colony_sim.gd:447-448 (production) and the 1.5x getter assertion in tests/test_goal_reward.gd:124. No stale callers of get_gather_speed_multiplier remain.
• CI on the head commit (4545e2a) — Script test suite, macOS validation, the four export validation jobs, headless smoke test, and release-tag validation all report success.

Standards Compliance

• AGENTS.md — validation gate: the diff names the canonical files (scripts/colony_sim.gd, scripts/goal_reward.gd, tests/test_goal_reward.gd) that PR 414 specifies as expected scope. The PR also touches scripts/game_state.gd and tests/test_rotating_goal.gd, both reasonable neighbors for the label fix and _known_reward_types() cleanup.
• AGENTS.md — testing trap (assert_eq(actual, expected, name) required): every assert_eq and assert_true call in the diff and the surrounding test_goal_reward.gd passes the third "name" argument; no parse-error pattern.
• AGENTS.md — claim-and-scope style: the PR body states the ask plainly in the title and opens with the issue, satisfies the convention.
• Repo collaboration preferences: changes a goal-reward consumer; medium-risk but well-scoped to the audit findings, no release/version-bump concern.

Linked Issue Fit

Each of PR 414's four acceptance bullets is met:

1. "Worker movement is measurably affected … step_worker advances up to N tiles per tick using get_haul_speed_multiplier(), with N >= 2 when the reward is active" — scripts/colony_sim.gd:447-453 does exactly this; tests/test_goal_reward.gd asserts distance >= 2.
2. "Reward label and magnitude agree" — +50% label now matches 1.5x magnitude.
3. "Either wire REWARD_GATHER_SPEED into a catalog goal and the sim, or remove the dead type/getter/magnitude" — the dead half is removed entirely (constant, duration, magnitude entry, getter, and _known_reward_types() slot all gone).
4. "tests/test_goal_reward.gd gains a test that the multiplier is consumed by the sim path, not just the getter" — test_step_worker_advances_two_tiles_per_tick_with_reward does exactly that.

Tool Harness Findings

Twelve tool calls (six rounds) executed without errors. Highlights:

• read_file against scripts/goal_reward.gd, the step_worker neighborhood of scripts/colony_sim.gd, the _known_reward_types() block in scripts/game_state.gd, the new sim-path test helpers in tests/test_goal_reward.gd, and the updated test_reward_preview_text block in tests/test_rotating_goal.gd — all matched the diff verbatim.
• git_grep (REWARD_GATHER_SPEED|get_gather_speed_multiplier, repo-wide) returned zero matches, confirming complete removal of the gather-speed surface.
• git_grep (step_toward|step_worker, scripts/) returned only the one expected site in colony_sim.gd; no other sim paths duplicate movement that the PR could have missed.
• git_grep for the old and new label substrings surfaced exactly one stale-looking match: docs/SPEC.md:132 still says haul speed +10% in the milestone table for build_hut. This is a small documentation follow-up worth noting — see findings.

Unknowns or Needs Verification

None material. The diff is internally consistent; the PR's own "Local validation" claim of a passing headless smoke test and a bash-driven tests/test_*.gd run is reinforced by the green CI rows on the same head commit. Issue PR 414's acceptance is fully covered by the new sim-path tests.