Fix duration-0 one-time rewards expiring before consumption - #321
Conversation
tick_rewards() now skips the expiration check for rewards with duration == 0 (one-time), allowing REWARD_RECRUIT_DISCOUNT and REWARD_AMBIENT_IMPROVE to persist until consumed by their consume_* helpers. Previously these expired on the very next tick after being awarded, making them unusable in normal gameplay. Adds 5 regression tests covering: - One-time rewards surviving 70 ticks (past EVENT_INTERVAL_TICKS) - consume_recruit_discount and consume_ambient_improve removing rewards - Timed rewards (duration > 0) still expiring on schedule Fixes #311 Signed-off-by: Saffron <263493777+itsmiso-ai@users.noreply.github.com>
There was a problem hiding this comment.
AI Automated Review (incremental)
Incremental review: reviewed the changes since the last managed review; unresolved findings from that review are carried forward.
Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic) — escalated (fast_low_confidence)
Recommendation: Approve
This PR fixes the reported bug where duration-0 one-time rewards (build_workshop → recruit discount, build_garden → ambient improve) expired on the first tick_rewards call before they could be consumed. The incremental review delta contains test-only refinements; the core fix in scripts/goal_reward.gd was introduced in the preceding commit on the branch.
Change-by-Change Findings
tests/test_goal_reward.gd — test refinements
-
test_one_time_ambient_improve_survives_ticks— Added optional assertion messages ("build_garden grants an ambient improve","Ambient improve is a one-time reward") to twoassert_eqcalls. No behavioral change; improves failure readability. -
test_consume_recruit_discount_removes_one_time_rewardandtest_consume_ambient_improve_removes_one_time_reward— Added"Reward is active before consumption"message to the pre-consumption size assertion. Minor clarity improvement; no logic change. -
test_timed_reward_still_expires_on_schedule— Key test correction. The loop boundary changed fromrange(DURATION_RESOURCE_TRICKLE)torange(DURATION_RESOURCE_TRICKLE + 1), with an explanatory comment:"tick_rewards checks
remaining <= 0BEFORE decrementing, so a reward created withremaining == durationsurvivesdurationticks and expires on the next one."This correctly validates that the duration-0 guard does not break the existing timed-reward expiry path. The test now properly observes expiry after
DURATION_RESOURCE_TRICKLE + 1ticks.
Standards Compliance
No AGENTS.md conventions are relevant to this change. The repository standards are satisfied:
- GDScript warnings treated as errors: no new warnings introduced (CI macOS validation passed).
- Tests use the existing
assert_eqpattern with optional message arguments — consistent with the rest of the test suite. - No save/version migration or file format changes.
Linked Issue Fit
Issue PR 311 is referenced in the PR title, body, commit message, and test docstrings. The issue describes duration-0 one-time rewards expiring before consumption — precisely what the test suite now validates:
test_one_time_reward_survives_ticks— REWARD_RECRUIT_DISCOUNT survives 70 tickstest_one_time_ambient_improve_survives_ticks— REWARD_AMBIENT_IMPROVE survives 70 tickstest_consume_recruit_discount_removes_one_time_reward— explicit consumption removes the rewardtest_consume_ambient_improve_removes_one_time_reward— explicit consumption removes the rewardtest_timed_reward_still_expires_on_schedule— timed rewards still expire correctly (no regression)
The linked issue acceptance criteria are met.
Tool Harness Findings
Not applicable — no tool harness output in this review corpus.
Unknowns / Needs Verification
None. All evidence is complete:
- The incremental delta is test-only and self-documenting.
- All 6 CI checks passed at the PR head commit.
- The previous review already validated the
goal_reward.gdimplementation fix via file read.
Tests fail due to missing godot runtime in environment; cannot verify goal reward or progression logic.
Fixes #311
Opened by foreman on review GO (workload wl-misospace-windowstead-311).