Persist and re-sync reserved_resources on save/load - #241
Conversation
![its-saffron[bot]](https://avatars.githubusercontent.com/in/3901073?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Superseded by a newer automated review for this pull request.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Reason: Failing check: macOS validation (failure) Latest note: foreman fix exhausted 3/3 attempts (prfix-misospace-windowstead-241) Posted automatically by Dispatch on 2026-07-08T15:30:02.848Z |
Reason: Failing check: Script test suite (failure) Latest note: foreman fix exhausted 3/3 attempts (prfix-misospace-windowstead-241) Posted automatically by Dispatch on 2026-07-08T16:30:09.009Z |
Reason: Failing check: Script test suite (failure) Latest note: foreman fix exhausted 3/3 attempts (prfix-misospace-windowstead-241) Posted automatically by Dispatch on 2026-07-08T17:30:06.632Z |
Superseded by a newer automated review for this pull request.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Reason: Failing check: Script test suite (failure) Latest note: foreman fix exhausted 3/3 attempts on ESCALATED (all coder tiers exhausted) (prfix-misospace-windowstead-241) Posted automatically by Dispatch on 2026-07-09T08:30:02.433Z |
Reason: Failing check: Script test suite (failure) Latest note: foreman fix exhausted 3/3 attempts on ESCALATED (all coder tiers exhausted) (prfix-misospace-windowstead-241) Posted automatically by Dispatch on 2026-07-09T09:30:02.508Z |
Reason: Failing check: Script test suite (failure) Latest note: foreman fix exhausted 3/3 attempts on ESCALATED (all coder tiers exhausted) (prfix-misospace-windowstead-241) Posted automatically by Dispatch on 2026-07-09T10:30:02.552Z |
Reason: Failing check: Script test suite (failure) Latest note: foreman fix exhausted 3/3 attempts on ESCALATED (all coder tiers exhausted) (prfix-misospace-windowstead-241) Posted automatically by Dispatch on 2026-07-09T11:30:02.516Z |
Reason: Failing check: Script test suite (failure) Latest note: foreman fix exhausted 3/3 attempts on ESCALATED (all coder tiers exhausted) (prfix-misospace-windowstead-241) Posted automatically by Dispatch on 2026-07-09T12:30:03.053Z |
Reason: Failing check: Script test suite (failure) Latest note: foreman fix exhausted 3/3 attempts on ESCALATED (all coder tiers exhausted) (prfix-misospace-windowstead-241) Posted automatically by Dispatch on 2026-07-09T13:30:02.586Z |
Reason: Failing check: Script test suite (failure) Latest note: foreman fix exhausted 3/3 attempts on ESCALATED (all coder tiers exhausted) (prfix-misospace-windowstead-241) Posted automatically by Dispatch on 2026-07-09T14:30:02.519Z |
Superseded by a newer automated review for this pull request.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Add rebuild_reservations_from_workers() to game_state.gd that rebuilds reserved_resources from active gather/haul worker tasks when the field is missing or empty (old saves). Called after migration in both localStorage and file-based load paths. Add rebuild_reservations() to main.gd that clears and rebuilds reserved_resources from current worker tasks. Called in both load_or_boot() and load_saved_game() after break_ticks init. Add save_game/load_game path parameter for test flexibility. Fix file handle leak in save_game by calling file.close(). Add tests: - test_reserved_resources_save_load: verifies saved reservations persist - test_reserved_resources_resync_on_load: verifies empty reservations are rebuilt from workers Fixes #231 Signed-off-by: Saffron <263493777+itsmiso-ai@users.noreply.github.com>
6be3214 to
07c99f1
Compare
Superseded by a newer automated review for this pull request.
There was a problem hiding this comment.
AI Automated Review
Full PR review.
Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic) — escalated (fast_low_confidence)
Review: Persist and re-sync reserved_resources on save/load (PR 241)
Recommendation: Approve
This PR correctly implements the fix for issue PR 231, adding save/load persistence for reserved_resources with a re-sync mechanism to prevent double-booking after reload. All CI checks pass including macOS validation.
Change-by-Change Findings
scripts/game_state.gd
- Added optional
pathparameter tosave_game()andload_game()— enables testability and is a safe backward-compatible addition. - Added
file.close()after save — correct resource management. - Added
rebuild_reservations_from_workers()— scans workers' active tasks (gather/haul) to rebuildreserved_resourceswhen missing or empty. Guarded withif not existing.is_empty()so it only runs on old saves or saves with missing reservation data. - Called
rebuild_reservations_from_workers()in both localStorage and file-based load paths, after parsing/migration. This directly satisfies issue PR 231's acceptance criteria.
scripts/main.gd
- Added
rebuild_reservations()— mirrors the game_state logic but operates onmain's state reference. Called in bothload_or_boot()andload_saved_game()after state loading completes. - Observation:
rebuild_reservations()is called inmain.gdafterload_game()has already calledrebuild_reservations_from_workers(). This means reservations are rebuilt twice per load. The logic is idempotent so no correctness issue, but it is redundant work. Not a blocker given the consistent behavior.
tests/test_reservations.gd
test_reserved_resources_save_load: Verifies savedreserved_resources({"wood": 2, "stone": 1}) survive a save/load cycle. Directly covers acceptance criterion PR 1.test_reserved_resources_resync_on_load: Saves state withreserved_resources: {}and two workers (one gatherer, one hauler), then verifies reservations are correctly rebuilt (wood=1,stone=1). Directly covers acceptance criterion PR 2.
Standards Compliance
- Save/version migration: Per AGENTS.md, "migration-first" is required because the game auto-loads on startup. The PR respects this by calling the rebuild logic after
migrate_save()completes, not replacing migration. ✓ - No regressions: The rebuild is gated on
if existing.is_empty(), so existing valid saves with populated reservations are trusted as-is. ✓ - Repository conventions: GDScript style is consistent with the codebase. No sidebar UI or other UX regressions introduced. ✓
Linked Issue Fit (PR 231)
| Criterion | Status |
|---|---|
reserved_resources survives save/load cycle |
✓ Verified by test_reserved_resources_save_load |
| On load, reservations are re-synced from active worker tasks | ✓ Verified by test_reserved_resources_resync_on_load |
Test added to tests/test_reservations.gd covering save/load |
✓ Two tests added |
| No double-booking after reload | ✓ Rebuild logic clears then reconstructs; idempotent guard prevents overwrites |
Evidence Provider Findings
No evidence providers configured.
Tool Harness Findings
No tool calls were issued; reviewing from corpus directly.
CI Check Results
| Check | Result |
|---|---|
| Headless smoke test | ✓ |
| Export validation (Windows) | ✓ |
| Export validation (Linux) | ✓ |
| Script test suite | ✓ |
| macOS validation | ✓ |
| Export validation (macOS) | ✓ |
All checks pass. The macOS validation is particularly important per this repository's conventions.
Unknowns / Needs Verification
- Code duplication:
rebuild_reservations()exists in bothgame_state.gd(line 66) andmain.gd(line 2503) with identical logic. This appears intentional given the layered autoload architecture (game_state for persistence, main for runtime state), but worth confirming this is the preferred pattern rather than having one call the other. No functional issue detected.
Summary
The PR is correct, well-tested, and meets all acceptance criteria from issue PR 231. No blockers. Approve for merge.
2 participants
Adds rebuild_reservations() to restore reserved_resources from active worker tasks on load, fixing double-booking after save/load.
Fixes #231
Opened by foreman on review GO (workload wl-misospace-windowstead-231).