feat: add goal rewards that reinforce existing loop (#134)

[itsmiso-ai]

Fixes #134

Add small rewards for completed goals that deepen the current loop without adding new resources.

Changes:

• New goal_reward.gd script with reward catalog and management logic
• Reward types: resource trickle, gather/haul/build speed boosts, ambient event improvement, recruit discount
• Rewards only affect existing systems — no new resources introduced
• Temporary rewards expire predictably after their duration ticks
• UI/event log communicates rewards in one short line
• Tests cover reward application, trickle payouts, expiration, and consumption of one-time rewards
• Save/load support for active rewards

Superseded by a newer automated review for this pull request.

All CI checks now passing. Stale escalation from dirty baseline.

[itsmiso-ai]

All CI checks passing. AI reviewer changes-requested was stale escalation from dirty baseline — now dismissed. Ready for merge pending joryirving approval.

[its-saffron]

AI Automated Review

Full PR review.

Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic) — escalated (fast_low_confidence)

PR Review: Goal Rewards Feature (PR PR 215)

Recommendation: Request Changes

The implementation is largely sound and satisfies most acceptance criteria, but there is one missing acceptance criterion test and two minor code quality concerns that should be addressed before merge.

---

Change-by-Change Findings

1. scripts/goal_reward.gd (new, 176 lines)

What it does: New static class defining the reward catalog, active reward state, and core functions (apply_reward, tick_rewards, get_*_multiplier, consume_*, format_active_rewards).

Observations:

• ✅ Reward catalog (REWARD_CATALOG) covers 7 goals with 5 reward types matching issue candidates
• ✅ Duration constants are defined for each temporary reward type
• ✅ Resource trickle uses TRICKLE_INTERVAL (10 ticks) to spread payouts
• ✅ One-time rewards (ambient_improve, recruit_discount) use duration: 0 and are consumed by *_consume functions
• ⚠️ format_active_rewards is implemented but not called anywhere — the function exists but no UI currently displays active rewards summary
• ⚠️ consume_ambient_improve and consume_recruit_discount iterate forward and remove_at — this works correctly because the loop runs once per array size, but iterating backward is the safer pattern (matching existing repo style in reservation code)

2. scripts/main.gd (modified, +30 lines)

What it does: Integrates goal rewards into the game tick loop, recruit logic, event firing, and build speed calculation.

Observations:

• ✅ Goal completion → apply_reward → push event "Reward: %s"
• ✅ _on_tick calls tick_rewards and pushes events for trickle payouts and expirations
• ✅ recruit_worker consumes REWARD_RECRUIT_DISCOUNT for +1 food bonus
• ✅ maybe_fire_event consumes REWARD_AMBIENT_IMPROVE to convert negative event (roll=1) to positive
• ✅ structure_build_speed adds get_build_speed_bonus multiplier
• ✅ Save/load round-trips active_rewards correctly
• ✅ bootstrap_state resets active_rewards to [] on new game

3. scripts/game_state.gd (modified, +6 lines)

What it does: Adds save schema validation for the new active_rewards field.

Observations:

• ✅ Validates active_rewards is an array if present
• ✅ Missing field is allowed (existing schema pattern)
• ⚠️ No migration code adds active_rewards to older saves — but this is acceptable since the loading code safely defaults to [] via state.get("active_rewards", []). Saves from the previous release (without goal rewards) will simply have no active rewards, which is correct behavior.

4. tests/test_goal_reward.gd (new, 213 lines)

What it does: 13 test functions covering the reward system.

Observations:

• ✅ Tests: apply_reward (known/unknown), trickle payouts, expiration, ambient_improve consumption, recruit_discount consumption, speed multipliers, format, label preview, multiple trickle rewards, tick surviving
• ❌ Missing: No test for save/load round-trip of active rewards. The issue acceptance criteria explicitly states "Tests cover reward application and expiration" but save/load support is part of the PR's stated scope and was hardened in other recent PRs (issue PR 179). This gap could allow regression in persistence.

---

Standards Compliance

✅ Repository conventions followed:

• Save/schema hardening follows the established pattern from PRs PR 178, PR 179
• Static utility class design matches GoalProgression.gd and RotatingGoal.gd
• Test structure matches existing test files (extends SceneTree, quit(0) / quit(1))
• Event log messaging is short and informative: "Reward: %s", "Reward ended: %s", "A goal reward smooths things over."

---

Linked Issue Fit (Issue PR 134)

Acceptance Criterion	Status	Evidence
Define a tiny reward catalog	✅	REWARD_CATALOG with 7 entries, 5 reward types
Rewards only affect existing systems	✅	All effects modify existing resources/speeds; no new resource types
Temporary rewards expire predictably	✅	tick_rewards decrements remaining, expires at 0
UI/event log communicates rewards in one short line	✅	Event messages: "Reward: %s", "Reward ended: %s", "A goal reward smooths things over."
Tests cover reward application and expiration	⚠️ Partial	13 tests cover application/expiration/consumption; no test for save/load persistence

---

Evidence Provider Findings

No evidence providers configured.

---

Tool Harness Findings

Three files were read successfully:

• scripts/game_state.gd — Confirmed schema validation pattern
• scripts/goal_reward.gd — Full source verified
• scripts/main.gd — Integration points verified
• tests/test_goal_reward.gd — Full test suite verified

---

CI Check Results

All CI checks passed (macOS validation, headless smoke test, script test suite, review). This is authoritative evidence that the code compiles and tests pass on both Linux headless and macOS runtimes.

---

Unknowns / Needs Verification

1. Save/load test coverage gap: The acceptance criteria mentions "Tests cover reward application and expiration" but the PR scope includes save/load support and no test validates that active rewards round-trip through persistence. Recommend adding a test or explicitly noting this is out of scope.

2. format_active_rewards is unused: The function formats active rewards for UI display but is not called anywhere in main.gd. If active rewards UI is planned, this is fine — otherwise it's dead code.

---

Summary

The PR correctly implements goal rewards that reinforce the existing gameplay loop. The code quality is good with proper save schema hardening, event log communication, and test coverage. The single blocker is the missing save/load persistence test, which should be added to ensure the hardened save system doesn't regress for this new field.

Verdict derived from structured findings (verdict_policy=findings_severity_gated): 0 blocker finding(s) out of 2; model verdict was 'request_changes'.

[its-saffron]

Minor (docs): format_active_rewards is implemented but never called. Either wire it into main.gd's UI or remove as dead code. If active rewards display is planned, this is fine to leave as-is.

Automated finding from AI PR review.