Harden save lifecycle: backup/restore + extended validation

[itsmiso-ai]

Fixes #178

Changes

Timestamped Backup / Restore (game_state.gd)

• backup_save() — creates a timestamped copy of the current save file (e.g. user://windowstead-backup_20260610_014530_1.save). Returns the path or empty string if no save exists.
• restore_backup() — restores from the most recent backup (sorted newest-first). Returns the restored path or empty string.
• list_backups() — returns all backup file paths sorted newest-first.
• _backup_filename() — internal helper that generates unique filenames using system timestamp + counter.

Extended Schema Validation (game_state.gd)

• Tiles: validates amount is numeric, resource is string, build_kind is optional string.
• Workers: validates name is non-empty string, pos is dict, carrying is dict with numeric values, task is dict, break_ticks is optional numeric (non-negative when present for v1 backward compat).
• Builds: validates id is numeric, kind is string, pos is dict, complete is boolean.

Destructive Reset Protection (main.gd)

• _on_startup_new_game() now calls backup_save() before build_world()/bootstrap_state(), so an existing save is preserved as a timestamped backup before being overwritten.
• Added push_event_safe() helper to log the backup action even when events may not yet exist.

Test Updates

• Updated test payloads in test_runner.gd to satisfy new validation schema (added missing worker fields).
• New tests/test_save_backup.gd with 5 flows:
  1. backup_save creates a timestamped file
  2. restore_backup restores from the latest backup
  3. list_backups returns newest-first sorted list
  4. validate_save_schema rejects invalid worker/build/task shapes
  5. validate_save_schema accepts valid worker/build/task shapes

Addresses Audit Findings (weekly tech debt audit 2026-06-03)

• ✅ Timestamped backup before destructive New Game/reset
• ✅ Extended validation to full worker/build/task shape and numeric bounds
• ⏸️ Explicit confirmation dialog — deferred to follow-up (would require UI changes beyond script scope)
• ⏸️ persist() jank mitigation — deferred (would need tick-throttling or async serialization)

[its-saffron]

AI Automated Review

Full PR review.

Analysis engine: review@https://litellm.jory.dev/v1 (openai)

Recommendation

Approve. This PR successfully addresses the technical debt identified in the weekly audit regarding save lifecycle hardening. It implements a robust timestamped backup system and significantly strengthens the schema validation for save files to prevent corruption and ensure data integrity during migrations.

Change-by-Change Findings

scripts/game_state.gd

• Backup/Restore System: Implemented backup_save(), restore_backup(), and list_backups(). Backups are timestamped and stored in user://, providing a safety net before destructive operations.
• Extended Schema Validation: Added comprehensive validation for tiles, workers, and builds. It now checks for correct types (e.g., amount must be numeric, name must be non-empty string) and ensures required dictionary keys are present. This is a significant improvement over the previous loose validation.
• Version Migration: Updated SAVE_VERSION to 2. The schema now includes active_goal, completed_goal_ids, and spawn_tick to support the new goal system.

scripts/main.gd

• Destructive Reset Protection: Modified _on_startup_new_game() to trigger a backup_save() before overwriting existing state, directly addressing the audit finding for issue PR 178.
• Event Safety: Added push_event_safe() to allow logging backup actions even if the event system hasn't been initialized yet.

tests/

• New Test Suite: Added tests/test_save_backup.gd which covers all new functionality: backup creation, restoration, sorting of backups, and various schema validation edge cases (missing keys, invalid types, negative bounds).
• Test Maintenance: Updated tests/test_runner.gd to include the new required fields in existing test payloads, ensuring CI remains green.

docs/

• Documentation Updates: Updated DESIGN.md and SPEC.md to reflect the new save version, the expanded save schema (including goal system fields), and the updated grid dimensions for different dock orientations.

Standards Compliance

• Issue Alignment: Directly addresses the requirements of PR 178 (Timestamped backup and extended validation).
• Testing: High coverage for the new logic via the new test file.
• Convention: Follows the repository's pattern of using user:// for persistence and maintains the existing Godot 4 structure.

Linked Issue Fit

• Issue PR 178: The PR fulfills the P2 recommendation to add backups before destructive resets and extend validation to full worker/build/task shapes.

Unknowns or Needs Verification

• Performance: While the audit mentioned persist() jank, this PR focuses on the safety of the save rather than the frequency of writes. The deferred item regarding persist() jank mitigation is noted as being out of scope for this PR.

[its-saffron]

AI Automated Review

Full PR review.

Analysis engine: review@https://litellm.jory.dev/v1 (openai)

Recommendation

Approved. This PR successfully addresses the technical debt identified in the weekly audit regarding save lifecycle hardening. It implements timestamped backups to prevent data loss during destructive operations (like New Game) and introduces robust schema validation for save data to prevent corruption.

Change-by-Change Findings

scripts/game_state.gd

• Backup System: Implemented backup_save(), restore_backup(), and list_backups(). The use of a timestamped filename with a counter (_backup_filename) ensures unique backups even if multiple backups are triggered in the same second.
• Schema Validation: Significantly expanded validate_save_schema. It now performs deep validation on:
  • Tiles: Ensures amount is numeric, resource is a string, and build_kind (if present) is a string.
  • Workers: Validates name is a non-empty string, pos and carrying are dictionaries, and break_ticks is a non-negative numeric value.
  • Builds: Validates id (numeric), kind (string), pos (dictionary), and complete (boolean).

scripts/main.gd

• Destructive Reset Protection: Modified _on_startup_new_game() to trigger a backup via GameState.backup_save() before proceeding with a new game setup. This directly mitigates the risk of accidental data loss during a reset.
• Event Logging: Added push_event_safe() to allow logging of backup actions even if the game state's event system is not yet initialized.

Tests

• tests/test_save_backup.gd: A comprehensive new test suite covering all new functionality: backup creation, restoration, sorting of backups (newest-first), and various schema validation failure/success cases.
• tests/test_runner.gd: Updated existing test payloads to include the new required fields for workers and builds, ensuring existing tests pass with the stricter validation.

Linked Issue Fit

This PR directly addresses PR 178 (Harden save lifecycle).

• Requirement: Add explicit confirmation for destructive paths.
  • Status: Partially satisfied. The PR implements the technical safety (backups), but as noted in the PR description, the UI confirmation dialog is deferred to a follow-up to keep this change within script scope.
• Requirement: Consider timestamped backup/atomic writes.
  • Status: Satisfied via the new backup system.
• Requirement: Extend validation to full worker/build/task shape and numeric bounds.
  • Status: Satisfied via the expanded validate_save_schema.

Standards Compliance

• Repository Conventions: The implementation follows the established GDScript patterns in the repository. The use of user:// for backups is consistent with Godot best practices for persistent data.
• Testing: The addition of a dedicated test file for the new logic is excellent and follows the pattern of using test_runner.gd for validation.

Unknowns or Needs Verification

• None. The logic for sorting backups using a > b on timestamped strings is correct for achieving a newest-first order.

[its-saffron]

AI Automated Review

Full PR review.

Analysis engine: review@https://litellm.jory.dev/v1 (openai)

Recommendation

Approved. This PR successfully addresses the technical debt identified in the weekly audit regarding save lifecycle hardening. It implements timestamped backups, a restoration mechanism, and robust schema validation for the save file.

Change-by-Change Findings

scripts/game_state.gd

• Backup System: Implemented backup_save(), restore_backup(), and list_backups(). The use of a timestamped filename with a counter (_backup_filename) ensures unique backups even if multiple are triggered in the same second.
• Schema Validation: Significantly expanded validate_save_schema. It now enforces strict types and presence for:
  • Tiles: amount (numeric), resource (string), and optional build_kind (string).
  • Workers: name (non-empty string), pos (dict), carrying (dict with numeric values), task (dict), and optional break_ticks (non-negative numeric).
  • Builds: id (numeric), kind (string), pos (dict), and complete (boolean).

scripts/main.gd

• Destructive Action Protection: Modified _on_startup_new_game() to trigger a backup_save() before initializing a new game state, preventing accidental loss of existing progress during a reset.
• Safety Helper: Added push_event_safe() to allow logging backup actions to the in-game event log even if the event system hasn't been initialized yet.

Tests

• tests/test_save_backup.gd: A comprehensive new test suite covering all new functionality: backup creation, restoration, sorting of backups (newest-first), and various schema validation failure/success cases.
• tests/test_runner.gd: Updated existing test payloads to include the new required fields (e.g., pos, carrying, task) to maintain compatibility with the stricter validation logic.

Linked Issue Fit

This PR directly addresses PR 178 (Harden save lifecycle) by implementing the following requirements from the audit:

• Timestamped backup before destructive New Game/reset.
• Extended validation to full worker/build/task shape and numeric bounds.
• [n/a] Explicit confirmation dialog (deferred to follow-up as noted in PR description).
• [n/a] persist() jank mitigation (deferred to follow-up as noted in PR description).

Standards Compliance

• Implementation Consistency: The implementation follows GDScript conventions and uses the existing user:// pathing for backups.
• Testing: The addition of a dedicated test file for the new logic is excellent practice and ensures the new validation rules don't regress.

Unknowns or Needs Verification

• None. The logic for list_backups relies on reverse alphabetical sorting of timestamped filenames, which is a valid and efficient approach for this use case.