diff --git a/scripts/game_state.gd b/scripts/game_state.gd index a385329..1a5dad4 100644 --- a/scripts/game_state.gd +++ b/scripts/game_state.gd @@ -27,6 +27,11 @@ var use_local_storage := false # a browser (see tests/test_local_storage_load_validation.gd). var _local_storage_reader: Callable = Callable(self, "_local_storage_read") +# Function-pointer hook for the save path. Defaults to _save_game_impl (the +# real write). Tests inject a stub here so they can force a failed write and +# count invocations without touching the filesystem (issue #379). +var _save_game_hook: Callable = Callable(self, "_save_game_impl") + var _backup_counter := 0 func _ready() -> void: @@ -101,6 +106,9 @@ func _read_json_file(path: String) -> Variant: return JSON.parse_string(text) func save_game(data: Dictionary, path: String = "") -> bool: + return _save_game_hook.call(data, path) + +func _save_game_impl(data: Dictionary, path: String = "") -> bool: var target_path := path if not path.is_empty() else SAVE_PATH var payload := JSON.stringify(data) if use_local_storage: diff --git a/scripts/main.gd b/scripts/main.gd index 713a74b..6b8ede3 100644 --- a/scripts/main.gd +++ b/scripts/main.gd @@ -1792,17 +1792,23 @@ func push_event(text: String) -> void: ## Save the colony. Debounced on the tick path; pass force=true for explicit ## user actions (save/recruit/placement) and on quit so nothing is lost. func persist(force := false) -> bool: - if not sim.dirty: + # A forced save (explicit Save click, recruit, placement, quit) must + # always re-attempt the write, even if a previous write failed and left + # sim.dirty cleared (issue #379). Only the non-forced tick path may skip + # when nothing has changed since the last persist. + if not force and not sim.dirty: return true if not force and tick - _last_persist_tick < PERSIST_INTERVAL_TICKS: return true - sim.dirty = false _last_persist_tick = tick # Stamp the save version so future migrations can detect the format. state["priority_order"] = priority_order.duplicate() state["dock_anchor"] = String(settings.get("dock_anchor", "bottom")) state["save_version"] = GameState.SAVE_VERSION if not GameState.save_game(state): + # Keep sim.dirty set so the next forced save (or the next tick path + # once the sim mutates) re-attempts the write instead of reporting + # success without writing (issue #379). # Surface the failure to the player exactly once per failed run, so # a long streak of debounced failed ticks doesn't spam the feed. # Subsequent ticks keep trying; we only re-announce when a save has @@ -1811,6 +1817,8 @@ func persist(force := false) -> bool: _persist_failure_announced = true push_event("Save failed: colony progress is not being persisted.") return false + # Only clear the dirty flag once the write actually succeeded. + sim.dirty = false _persist_failure_announced = false return true diff --git a/tests/test_save_failure_surfaced.gd b/tests/test_save_failure_surfaced.gd index 691df68..91a0fc7 100644 --- a/tests/test_save_failure_surfaced.gd +++ b/tests/test_save_failure_surfaced.gd @@ -9,8 +9,15 @@ extends "res://tests/test_case.gd" # 3. _write_text_file returns false on an unwritable path # 4. save_game returns false when the underlying write fails # 5. save_game returns true when the underlying write succeeds +# 6. (issue #379) a second forced Save after a failed write re-attempts the +# write instead of reporting success without saving # ============================================================================= +# Count of GameState.save_game invocations while a forced-failure hook is +# installed (Flow 6). A member (not a local) so the injected lambda can mutate +# it — a lambda captures `self`, not enclosing locals. +var _save_call_count := 0 + func run_tests() -> void: # Autoloads are not running in --script mode — instantiate game_state.gd # manually (same pattern as test_save_backup.gd). @@ -23,6 +30,7 @@ func run_tests() -> void: flow_write_text_file_returns_false_for_unwritable_path(gs) flow_save_game_returns_false_when_write_fails(gs) flow_save_game_returns_true_on_successful_write(gs) + flow_second_save_click_retries_after_failure() # --------------------------------------------------------------------------- @@ -85,4 +93,73 @@ func flow_save_game_returns_true_on_successful_write(gs: Node) -> void: var dir := DirAccess.open("user://") if dir != null and dir.file_exists(tmp): dir.remove(tmp) - gs.use_local_storage = saved_local \ No newline at end of file + gs.use_local_storage = saved_local + + +# --------------------------------------------------------------------------- +# Flow 6: (issue #379) a second forced Save after a failed write re-attempts +# the write instead of reporting success without saving. +# +# persist() used to clear sim.dirty *before* the write, so after a failed +# write a fast second Save click hit the `if not sim.dirty: return true` +# early-return and the caller pushed "Game saved" without writing. Now the +# dirty flag only clears on a successful write, so a forced save always +# re-invokes GameState.save_game. +# --------------------------------------------------------------------------- + +func flow_second_save_click_retries_after_failure() -> void: + print("\n=== Flow 6: second forced Save re-attempts after a failed write ===") + # main.gd references the GameState autoload — load at runtime, not preload. + var main_script: GDScript = load("res://scripts/main.gd") + var main: Control = main_script.new() + # The GameState autoload is registered once the engine has booted, so + # persist() writes through it. Force every write to fail and count calls. + var gs: Node = root.get_node("GameState") + _save_call_count = 0 + gs._save_game_hook = func(_data: Dictionary, _path: String) -> bool: + _save_call_count += 1 + return false + + main.state = {"tick": 0, "events": []} + main._mark_dirty() + + # First explicit Save click: the write fails. + var first: bool = main.persist(true) + assert_false(first, "first forced persist returns false when the write fails") + assert_eq(_save_call_count, 1, "first forced persist invoked GameState.save_game once") + # The core fix: dirty must stay set so the next forced save re-attempts. + assert_true(main.sim.dirty, "sim.dirty stays true after a failed write") + + # Second explicit Save click immediately after the failure. Before the fix + # this hit the dirty-skip early-return and returned true (→ "Game saved"). + var second: bool = main.persist(true) + assert_false(second, "second forced persist returns false (re-attempts, does not report success)") + assert_eq(_save_call_count, 2, "second forced persist re-invoked GameState.save_game") + assert_true(main.sim.dirty, "sim.dirty still true after the second failed write") + + # The success branch (the only place "Game saved" is pushed) must not have + # fired for either click. + assert_false( + _events_contain(main.state.get("events", []), "Game saved"), + "no 'Game saved' event was pushed after failed writes" + ) + + # Once the write succeeds, a forced persist clears the dirty flag. + gs._save_game_hook = func(_data: Dictionary, _path: String) -> bool: + _save_call_count += 1 + return true + var recovered: bool = main.persist(true) + assert_true(recovered, "forced persist returns true once the write succeeds") + assert_false(main.sim.dirty, "sim.dirty clears on a successful write") + + # Restore the real save path for any later flow. + gs._save_game_hook = Callable(gs, "_save_game_impl") + main.free() + + +## True when any event's text contains `needle`. +func _events_contain(events: Array, needle: String) -> bool: + for entry in events: + if String(entry.get("text", "")).contains(needle): + return true + return false \ No newline at end of file