Repository navigation
docs: refresh save-format section against current validation/migration behavior - #426
Conversation
…n behavior (issue #416) - v1 saves are migrated in place, not reset; future/0 versions fail the load - migration bullet lists what migrate_v1_to_v2() actually writes - persisted key list now includes colony_stance, reserved_resources, active_rewards, milestone fields, and migration_log, and points to validate_save_schema() as the source of truth
There was a problem hiding this comment.
AI Automated Review
Full PR review.
Analysis engine: MiniMax-M3 (anthropic) — primary route · pr-reviewer-action v3.4.1
Recommendation: Approve
This is a documentation-only change that updates docs/DESIGN.md to match the current save/load validation and migration behavior. The diff is consistent with the actual code paths and addresses every acceptance criterion in linked issue PR 416.
Change-by-change findings
docs/DESIGN.md
-
Persistence key list (line 49) — Now lists
colony_stance,reserved_resources,active_rewards,current_milestone_id,completed_milestone_ids, andmigration_login addition to the previously documented keys, and namesvalidate_save_schema()inscripts/game_state.gdas the source of truth. Cross-checked againstscripts/main.gd:636-652(bootstrap_state),scripts/colony_sim.gd:48-75(ensure_defaults), andscripts/main.gd:1842-1855(persist) — all keys listed are written by these three code paths. -
Data-model state dictionary — Adds the missing fields
colony_stance(line 76),active_rewards(line 107), milestone ids (line 109),reserved_resources(line 113), andmigration_log(line 114). Marksspawn_tickas optional with a comment that it is set on recruit (scripts/colony_sim.gd:325) and backfilled by migration (scripts/game_state.gd:707-708). Notes thatresourceon reward entries is trickle-only, which matches theapply_rewardlogic inscripts/goal_reward.gdwhereresourceis only assigned when the entry has it (trickle-type rewards). -
Save format section (lines 140-147) — Replaces "version mismatch triggers a colony reset" with the actual lifecycle: v1 saves are migrated in place by
migrate_v1_to_v2()(scripts/game_state.gd:700-709); future versions and version 0 are rejected viamigrate_save()returning{}(scripts/game_state.gd:661-698) andmain.gd:load_saved_game()bailing via_load_failed()before assigningstate(scripts/main.gd:767-774); a missingsave_versionis treated as current (scripts/game_state.gd:667-670); the migration bullet correctly attributesmigration_logandspawn_ticktomigrate_v1_to_v2()and the other newer fields toColonySim.ensure_defaults()(scripts/colony_sim.gd:57-73).
Claim verification
- Claim 1 (persistence key list): Held. 9/9 items verified. The key list at
docs/DESIGN.md:49matches whatbootstrap_state(),ColonySim.ensure_defaults(), andpersist()write, and defers tovalidate_save_schema()for validated shapes. - Claim 2 (data-model state dictionary): Held. 8/8 items verified. All newly documented fields match the code that produces them. The
spawn_tickoptionality andresourcetrickle-only notes are accurate. - Claim 3 (invariants checked on main): Held. The doc text at line 108 accurately describes the v1→v2 migration behavior.
- Claim 4 (clear_game() not on load path): Held. 3/3 items verified.
clear_game()is only called from the New Game flow (scripts/main.gd:285-287backs up before reset), not from any load path.migrate_save()returns{}on version 0 or future versions, andload_saved_game()calls_load_failed()before assigningstate. - Claim 5 (desktop/web share one pipeline; persisted key set exact): Held. 6/6 items verified.
_validate_and_apply_save()(scripts/game_state.gd:256-265) is the single entry point used by both desktop and web load paths (issue PR 313). The persisted key set matchesbootstrap_state()+ColonySim.ensure_defaults()+persist().
Sources
docs/DESIGN.md(the modified file)scripts/game_state.gd:256-265(_validate_and_apply_save)scripts/game_state.gd:284-660(validate_save_schema)scripts/game_state.gd:661-698(migrate_save)scripts/game_state.gd:700-709(migrate_v1_to_v2)scripts/colony_sim.gd:48-75(ensure_defaults)scripts/colony_sim.gd:325(recruit setsspawn_tick)scripts/main.gd:285-290(New Game backs up before reset)scripts/main.gd:630-660(bootstrap_state)scripts/main.gd:761-774(_load_failed,load_saved_game)scripts/main.gd:1842-1855(persist)scripts/goal_reward.gd(reward entry shape,resourceonly on trickle)
Standards Compliance
The repository standards file (AGENTS.md) notes that "Save/version migration should be migration-first because the game auto-loads the latest save on startup." This PR documents exactly that behavior — the doc now states v1 saves are migrated in place, which is consistent with the migration-first principle. No standards violations.
Linked Issue Fit
The PR closes PR 416. All three acceptance criteria from the issue are met:
- ✅ The Save format section states that v1 saves are migrated (not reset) and that future versions are rejected with a load failure.
- ✅ The migration bullet lists what
migrate_v1_to_v2()actually writes (migration_log,spawn_tick). - ✅ The key list includes all schema-validated fields and links to
validate_save_schema()as the source of truth.
Tool Harness Findings
15 tool calls were executed across 9 rounds. Key findings:
validate_save_schema()inscripts/game_state.gd:284is the single validation entry point used by both desktop and web load paths via_validate_and_apply_save()(line 256).migrate_save()(line 661) returns{}for version 0 and future versions (> SAVE_VERSION).migrate_v1_to_v2()(line 700) writesmigration_logand backfillsspawn_tickon workers — it does NOT writeactive_goal,completed_goal_ids,active_rewards, milestone ids,colony_stance, orreserved_resources.ColonySim.ensure_defaults()(line 48) backfillsreserved_resources,events,completed_goal_ids,active_rewards,active_goal,colony_stance, and milestone state.clear_game()is only invoked from the New Game startup path (scripts/main.gd:285-287), which backs up first. No load path calls it.load_saved_game()(line 767) bails via_load_failed()before assigningstatewhen the load result is empty.- The
resourcefield on reward entries is only set forREWARD_RESOURCE_TRICKLEtype rewards inscripts/goal_reward.gd:78-79.
CI Status
All CI checks (Script test suite, Headless smoke test, Export validation for macOS/Linux/Windows/Web, Release tag validation, macOS validation) are reported as success. The PR is markdown-only and touches no script or test file, so the test suite is not directly exercised by this diff but provides context that the surrounding code compiles and passes.
What
Docs-only fix for #416. Updates the stale save-format material in
docs/DESIGN.md:docs/DESIGN.md"Persistence"): now lists every persisted key (colony_stance,reserved_resources,active_rewards,current_milestone_id,completed_milestone_ids, andmigration_logon upgraded saves) and namesvalidate_save_schema()inscripts/game_state.gdas the source of truth.colony_stance,active_rewardswith entry shape, milestone ids,reserved_resources,migration_log), marksspawn_tickoptional (set on recruit, backfilled by migration), and notesresourceon reward entries is trickle-only.migrate_v1_to_v2()and load normally; future versions and version 0 are rejected (load fails, running colony untouched, no reset on any load path); a missingsave_versionis treated as current; the migration bullet lists whatmigrate_v1_to_v2()actually writes (migration_log,spawn_tick), with the other newer fields attributed toColonySim.ensure_defaults().Invariants this change must keep, and the code paths that enforce each
This is a documentation change: the invariant is that the doc describes what the code actually does. Each rule below was checked against the named enforcer on
main:GameState.migrate_save()/migrate_v1_to_v2()(scripts/game_state.gd); the doc now states this.clear_game()is only invoked from New Game (which backs up first), never from a load path — doc makes no reset claim.migrate_save()returning{}andmain.gd:load_saved_game()bailing via_load_failed()before assigningstate. Doc matches._validate_and_apply_save()(issue [P2] Web localStorage load path skips save-schema validation and migration #313); the doc's wording ("shared by the desktop and web paths") reflects that single implementation.bootstrap_state()+ColonySim.ensure_defaults()+persist()write. Verified all three (scripts/main.gd:636-652,scripts/colony_sim.gd:48-75,scripts/main.gd:1842-1855); the doc key list matches, and defers tovalidate_save_schema()for validated shapes.migrate_v1_to_v2()writesmigration_log/spawn_tick; every other newer field is anensure_defaults()backfill. Enforced inscripts/game_state.gd:700-709andscripts/colony_sim.gd:57-73; the doc attributes each field to the right mechanism.Validation
scripts/game_state.gd(validate_save_schema,migrate_save,migrate_v1_to_v2,load_game),scripts/colony_sim.gd(ensure_defaults,recruit_worker),scripts/main.gd(bootstrap_state,persist,load_saved_game), andscripts/goal_reward.gdreward shapes; both findings from that review (optionalresourceon reward entries, optionalspawn_tick) are included in the diff..tools/absent), so the headless test suite was not run; the change is markdown-only and touches no script or test file. CI (test.yml) remains the external gate.Closes #416.