From c1d71be269d46d1fd8b88778ca4bc69238866d98 Mon Sep 17 00:00:00 2001 From: Saffron <263493777+itsmiso-ai@users.noreply.github.com> Date: Wed, 9 Sep 2026 00:11:44 +0000 Subject: [PATCH] fix(save): back-fill state.resources and state.harvested on load MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Saves that omit state.resources or state.harvested passed validate_save_schema ("if present" checks) and then crashed the sim on the first tick: ColonySim.apply_food_upkeep, gather_haul_tasks, and do_gather read state.resources.get(...) / state.harvested[...] with no guard. Take the migration-first option from the issue: - GameState.migrate_save back-fills both dictionaries (zeroed wood/stone/food) before the version branching, so every valid load path — desktop file and web localStorage — returns a save the sim can tick. Existing values are preserved. - ColonySim.ensure_defaults back-fills the same two keys so bootstrap and direct state assignments are covered too. - Main.is_save_compatible rejects a save without a well-typed resources dictionary instead of letting one through. Adds regression tests in tests/test_local_storage_load_validation.gd covering the missing-resources and missing-harvested cases for both the web (localStorage) and desktop (file) branches. Full suite passes headless, including tests/test_local_storage_xss.gd. Fixes #378 Signed-off-by: Saffron <263493777+itsmiso-ai@users.noreply.github.com> --- scripts/colony_sim.gd | 8 ++ scripts/game_state.gd | 20 +++++ scripts/main.gd | 5 ++ tests/test_local_storage_load_validation.gd | 84 +++++++++++++++++++++ 4 files changed, 117 insertions(+) diff --git a/scripts/colony_sim.gd b/scripts/colony_sim.gd index d9529a3..1ce1f9a 100644 --- a/scripts/colony_sim.gd +++ b/scripts/colony_sim.gd @@ -46,6 +46,14 @@ func colony_stance() -> String: ## Fill in any state keys missing from older saves or fresh bootstraps so the ## rest of the sim can mutate them without existence checks. func ensure_defaults() -> void: + # The economy dictionaries are read unconditionally on the first tick + # (apply_food_upkeep, gather_haul_tasks, do_gather); a save that omits + # either would crash the sim (issue #378). migrate_save back-fills them on + # load, but bootstrap and any direct state assignment go through here too. + if not state.has("resources") or not state["resources"] is Dictionary: + state["resources"] = {"wood": 0, "stone": 0, "food": 0} + if not state.has("harvested") or not state["harvested"] is Dictionary: + state["harvested"] = {"wood": 0, "stone": 0, "food": 0} if not state.has("reserved_resources"): state["reserved_resources"] = {} if not state.has("events"): diff --git a/scripts/game_state.gd b/scripts/game_state.gd index 4ac3922..a385329 100644 --- a/scripts/game_state.gd +++ b/scripts/game_state.gd @@ -404,6 +404,17 @@ func validate_save_schema(data: Dictionary) -> Dictionary: # ── Schema helpers ────────────────────────────────────────────────────────── +# Back-fills the economy dictionaries the sim reads unconditionally on the +# first tick after load (issue #378): state.resources and state.harvested. +# bootstrap_state always sets both, but the load path historically did not, +# so a save omitting either crashed ColonySim on the first process_tick. +# Existing values are preserved; only missing keys are filled. +func _backfill_required_state(data: Dictionary) -> void: + if not data.has("resources") or not data["resources"] is Dictionary: + data["resources"] = {"wood": 0, "stone": 0, "food": 0} + if not data.has("harvested") or not data["harvested"] is Dictionary: + data["harvested"] = {"wood": 0, "stone": 0, "food": 0} + # Compute valid tile counts from the LayoutMath anchor family configuration. # Legacy grid sizes (25/36/64/100/150) are still accepted so historical saves # remain loadable until they are migrated. @@ -501,6 +512,15 @@ func _validate_active_rewards(rewards: Array) -> String: return "" func migrate_save(data: Dictionary) -> Dictionary: + # Back-fill the two economy dictionaries the sim reads unconditionally on + # the first tick after load (issue #378). bootstrap_state always sets both, + # but a hand-edited, corrupted, or future-migrated save can omit either; + # without this, ColonySim.apply_food_upkeep / gather_haul_tasks / do_gather + # raise on state.resources.get(...) / state.harvested[...] against a + # missing key. Applied before the version branching so every valid return + # path carries both fields; the invalid paths return a fresh {} and the + # mutation is discarded. + _backfill_required_state(data) # Missing version key means "current" — backward compatible if not data.has("save_version"): data["save_version"] = SAVE_VERSION diff --git a/scripts/main.gd b/scripts/main.gd index c11b144..713a74b 100644 --- a/scripts/main.gd +++ b/scripts/main.gd @@ -1700,6 +1700,11 @@ func next_unlock_text() -> String: return "%s tier unlocked. Keep the tiny settlement fed" % cap(last_kind) func is_save_compatible(loaded: Dictionary) -> bool: + # The sim reads state.resources unconditionally on the first tick after + # load (issue #378); a save without a well-typed resources dictionary is + # incompatible even if its geometry checks out. + if not loaded.has("resources") or not loaded["resources"] is Dictionary: + return false var tiles: Array = loaded.get("tiles", []) if tiles.size() != grid_w * grid_h: return false diff --git a/tests/test_local_storage_load_validation.gd b/tests/test_local_storage_load_validation.gd index bcec0a1..e9ca9c7 100644 --- a/tests/test_local_storage_load_validation.gd +++ b/tests/test_local_storage_load_validation.gd @@ -28,6 +28,10 @@ func run_tests() -> void: flow_v1_is_migrated_to_v2_with_spawn_tick() flow_valid_v2_is_returned_as_is() flow_empty_local_storage_falls_back_to_fresh_start() + flow_missing_resources_backfilled_web() + flow_missing_harvested_backfilled_web() + flow_missing_resources_backfilled_desktop() + flow_missing_harvested_backfilled_desktop() teardown() func setup() -> void: @@ -135,3 +139,83 @@ func flow_empty_local_storage_falls_back_to_fresh_start() -> void: empty.is_empty() or (empty.has("save_version") and empty.get("save_version") is int), "empty localStorage yields a fresh-start save shape" ) + +# 5) A web save missing state.resources is back-filled by migrate_save +# (issue #378) so the sim doesn't crash on the first tick after load. +# Previously validate_save_schema only checked resources "if present", so +# this fixture passed validation and ColonySim.apply_food_upkeep raised on +# state.resources.get(...) against a missing key. +func flow_missing_resources_backfilled_web() -> void: + _stub = { + "save_version": 2, + "tick": 3, + "harvested": {"wood": 1, "stone": 0, "food": 0}, + "workers": [], + } + var loaded: Dictionary = _gs.load_game() + assert_true(loaded.has("resources"), "web save missing resources is back-filled") + assert_true(loaded["resources"] is Dictionary, "back-filled resources is a Dictionary") + assert_eq(int(loaded["resources"].get("wood", -1)), 0, "back-filled resources.wood defaults to 0") + assert_eq(int(loaded["resources"].get("stone", -1)), 0, "back-filled resources.stone defaults to 0") + assert_eq(int(loaded["resources"].get("food", -1)), 0, "back-filled resources.food defaults to 0") + assert_eq(int(loaded.get("harvested", {}).get("wood", -1)), 1, "existing harvested values preserved") + +# 6) A web save missing state.harvested is back-filled the same way. +func flow_missing_harvested_backfilled_web() -> void: + _stub = { + "save_version": 2, + "tick": 3, + "resources": {"wood": 5, "stone": 2, "food": 1}, + "workers": [], + } + var loaded: Dictionary = _gs.load_game() + assert_true(loaded.has("harvested"), "web save missing harvested is back-filled") + assert_true(loaded["harvested"] is Dictionary, "back-filled harvested is a Dictionary") + assert_eq(int(loaded["harvested"].get("wood", -1)), 0, "back-filled harvested.wood defaults to 0") + assert_eq(int(loaded["harvested"].get("stone", -1)), 0, "back-filled harvested.stone defaults to 0") + assert_eq(int(loaded["harvested"].get("food", -1)), 0, "back-filled harvested.food defaults to 0") + assert_eq(int(loaded.get("resources", {}).get("wood", -1)), 5, "existing resources values preserved") + +# 7) Desktop (file) branch: a save file missing state.resources is back-filled +# on load (issue #378). The desktop path shares _validate_and_apply_save +# with the web path, but the file round-trip is exercised here so a +# regression in either branch is caught. +func flow_missing_resources_backfilled_desktop() -> void: + _gs.use_local_storage = false + var path := "user://test_missing_resources.save" + _gs.save_game({ + "save_version": 2, + "tick": 3, + "harvested": {"wood": 2, "stone": 0, "food": 0}, + "workers": [], + }, path) + var loaded: Dictionary = _gs.load_game(path) + assert_true(loaded.has("resources"), "desktop save missing resources is back-filled") + assert_true(loaded["resources"] is Dictionary, "desktop back-filled resources is a Dictionary") + assert_eq(int(loaded["resources"].get("wood", -1)), 0, "desktop back-filled resources.wood defaults to 0") + assert_eq(int(loaded["resources"].get("food", -1)), 0, "desktop back-filled resources.food defaults to 0") + assert_eq(int(loaded.get("harvested", {}).get("wood", -1)), 2, "desktop existing harvested values preserved") + _remove_save_file(path) + +# 8) Desktop (file) branch: a save file missing state.harvested is back-filled. +func flow_missing_harvested_backfilled_desktop() -> void: + _gs.use_local_storage = false + var path := "user://test_missing_harvested.save" + _gs.save_game({ + "save_version": 2, + "tick": 3, + "resources": {"wood": 4, "stone": 1, "food": 2}, + "workers": [], + }, path) + var loaded: Dictionary = _gs.load_game(path) + assert_true(loaded.has("harvested"), "desktop save missing harvested is back-filled") + assert_true(loaded["harvested"] is Dictionary, "desktop back-filled harvested is a Dictionary") + assert_eq(int(loaded["harvested"].get("wood", -1)), 0, "desktop back-filled harvested.wood defaults to 0") + assert_eq(int(loaded["harvested"].get("food", -1)), 0, "desktop back-filled harvested.food defaults to 0") + assert_eq(int(loaded.get("resources", {}).get("wood", -1)), 4, "desktop existing resources values preserved") + _remove_save_file(path) + +# Helper: remove a temporary save file written by the desktop-branch tests. +func _remove_save_file(path: String) -> void: + if FileAccess.file_exists(path): + DirAccess.remove_absolute(ProjectSettings.globalize_path(path))