Derive save-schema grid sizes from LayoutMath and harden cross-field validation#280
Conversation
…validation
- Replace the hardcoded expected_sizes array with sizes computed from
LayoutMath.grid_dims_for_anchor() / the BOTTOM/SIDE_GRID_* constants
so future layout changes propagate automatically. Legacy sizes
(25/36/64/100/150) remain accepted for backward-compatible loads.
- Add internal-consistency check: tiles.size() == grid_w * grid_h when
those fields are present.
- Validate active_goal: {} (no goal) or a dictionary whose id is in
RotatingGoal.GOAL_CATALOG.
- Validate completed_goal_ids: each entry must be a known goal id.
- Validate colony_stance: must be empty or one of ColonyStance.ALL_STANCES.
- Validate active_rewards: each entry is a Dictionary with a recognised
GoalReward type and well-typed numeric/string fields.
- Update tests/test_save_backup.gd with new flows exercising grid-size
consistency, valid/invalid active_goal, completed_goal_ids,
colony_stance, and active_rewards.
Fixes #236
Signed-off-by: Saffron <263493777+itsmiso-ai@users.noreply.github.com>
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)
Recommendation: Approve
This PR fully addresses issue PR 236. All five acceptance criteria are met, tests are comprehensive, CI is green across all platforms, and the implementation is internally consistent.
Change-by-Change Findings
scripts/game_state.gd (+180/-5 lines)
-
Preloads added:
LayoutMath,RotatingGoal,ColonyStance,GoalReward— all referenced by the new validation helpers. -
Grid sizing derivation (
_expected_grid_sizes()): Rather than a hardcoded[25, 36, 64, 100, 150, 160, 240]array, the function now:- Calls
LayoutMath.grid_dims_for_anchor("bottom")andLayoutMath.grid_dims_for_anchor("side")to get per-anchor dimensions - Cross-validates against
LayoutMath.BOTTOM_GRID_W * LayoutMath.BOTTOM_GRID_HandLayoutMath.SIDE_GRID_W * LayoutMath.SIDE_GRID_H - Appends legacy sizes (25/36/64/100/150) for historical save compatibility
- The comment explicitly notes that new anchors added to
layout_math.gdneed to be appended here too — a reasonable maintenance constraint given the preload approach.
- Calls
-
Internal consistency check: When both
grid_wandgrid_hare present, the validator now enforcestiles.size() == grid_w * grid_h. Type checks (int) and positivity checks (> 0) are applied before the comparison. -
active_goalvalidation: Delegates to_validate_active_goal(), which accepts empty dicts (no active goal) and validates thatidis present and exists inRotatingGoal.GOAL_CATALOG. -
completed_goal_idsvalidation: Each entry must be a string and must appear inRotatingGoal.GOAL_CATALOG. -
colony_stancevalidation: Must be a string, and if non-empty must be a member ofColonyStance.ALL_STANCES. -
active_rewardsvalidation: Each entry must be a dict with a knowntype(fromGoalReward.REWARD_CATALOGand theREWARD_*constants), with string fields fortype/label/resourceand non-negative numerics forremaining/duration/trickle_ticks.
tests/test_save_backup.gd (+150/-1 lines)
-
flow_grid_sizing_consistency(): Tests:- Rejects 158 tiles with 32×5 grid (mismatch)
- Rejects a tile count (1) outside LayoutMath-derived sizes
- Accepts 160 tiles without
grid_w/grid_h - Rejects zero
grid_w
-
flow_goal_and_stance_validation(): Tests:- Rejects
active_goalwith unknown id - Accepts
active_goalwith known id (gather_wood) - Rejects
completed_goal_idswith unknown entry - Rejects unknown
colony_stance - Accepts empty
colony_stance - Rejects
active_rewardswith unknown reward type - Rejects non-dict reward entries
- Accepts valid
completed_goal_ids
- Rejects
-
flow_validation_accepts_valid()updated: The base valid state now includesgrid_w/grid_h/anchor_family/active_goal/completed_goal_ids/colony_stance/active_rewards, and the test exercises both bottom (32×5=160) and side (10×24=240) anchors. -
Helper
_minimal_valid_state(): Centralizes the base save structure used across new test flows.
Standards Compliance
- AGENTS.md / Review Notes: Save/version migration is migration-first; this PR correctly adds schema validation before migration in
load_game(). - No regressions flagged: The changes are additive validation only; existing valid saves remain valid.
- No UX impact: This is purely internal save-schema hardening with no gameplay or UI surface changes.
Linked Issue Fit
Issue PR 236 acceptance criteria checklist:
- ✅ Grid size validation derived from layout configuration, not hardcoded —
_expected_grid_sizes()derives fromLayoutMath - ✅ Internal consistency check:
tiles.size() == grid_w * grid_h— implemented with type/positivity guards - ✅ Invalid goal IDs rejected on load —
active_goalandcompleted_goal_idsvalidated againstRotatingGoal.GOAL_CATALOG - ✅ Invalid colony stance rejected on load — validated against
ColonyStance.ALL_STANCES - ✅
tests/test_save_backup.gdupdated with edge cases — two new test flows added covering all new validation paths
Tool Harness Findings
The tool harness ran read_file on scripts/game_state.gd and tests/test_save_backup.gd and git_log. All succeeded. The git log confirms this is the HEAD commit on the feature branch.
CI Check Results
All six CI checks passed:
- Headless smoke test
- macOS validation
- Export validation (macOS)
- Export validation (Linux)
- Script test suite
- Export validation (Windows)
Unknowns / Needs Verification
LayoutMath.grid_dims_for_anchor(anchor: String)— called by_expected_grid_sizes()but not verified present in the truncatedlayout_math.gdview. However, the CI script-test-suite passed, which exercises this code path. Resolved: CI passed.RotatingGoal.GOAL_CATALOGandGoalReward.REWARD_CATALOGstructures — assumed to match the validation logic; confirmed by CI passing._known_reward_keys()returns["type", "remaining", "duration", "label", "trickle_ticks", "resource"]which is a superset check (only validates keys that are present, does not reject missing keys). This matches the existing style in the file for permissive validation and is sufficient for the issue scope.
What
Derives save-schema grid sizing from LayoutMath instead of hard-coded defaults, and hardens
save_game.validate_save_schemawith cross-field consistency checks (active_goal, completed_goal_ids, colony_stance, active_rewards, tile-count vs. grid_w*grid_h).Why
Issu…
Fixes #236
Opened by foreman on review GO (workload wl-misospace-windowstead-236).