chore(game-state): remove dead save_supported flag (issue #415)

[itsmiso-ai]

What

Removes the dead GameState.save_supported variable (declaration + unconditional save_supported = true in _ready()), taking the issue's "delete the variable entirely" acceptance path. No other code changed.

Chose deletion over "make it a real guard" deliberately: turning it into a gate (e.g. false when web has no localStorage, with call sites checking it) would change web-build save behavior and require new false-branch tests — scope creep for a P3 dead-state cleanup. The actual save/backend decision was and remains use_local_storage alone.

Closes #415

Invariants this change must keep, and every code path enforcing each

• Save-backend selection is use_local_storage only (set in _ready() from OS.has_feature("web") + localStorage availability). Enforced by _save_game_impl, load_game, clear_game, save_settings, load_settings, backup_save / _backup_save_local_storage, _prune_old_backups, list_backups, restore_backup (all in scripts/game_state.gd). None of them read save_supported, so none are affected; all still branch solely on use_local_storage.
• No save-format change. Saves are explicit-key (JSON.stringify of a caller-built dict), never reflective of GameState member variables, so removing the member cannot alter what is written or break old saves. The loader validates with data.has(...) checks and needs no migration.
• No behavior change for the desktop build (issue acceptance). The removed assignment was the only write and there are zero reads repo-wide (grep -rn save_supported over scripts/, scenes/, tests/, docs/, .github/, .tscn/.cfg/.json → no remaining references; also no get("save_supported")-style dynamic access).
• Web unload-flush gating is unchanged. register_web_unload_flush (issue [P3] Web build loses debounced progress on tab close — no pagehide/beforeunload flush hooks the browser unload lifecycle #403) gates on _os_has_feature_hook("web") and use_local_storage, not save_supported; untouched.

Local validation (Godot 4.7.1-stable headless, pinned per godot-toolchain.json, sha256 verified)

• Full per-suite loop replicating .github/actions/load-godot-toolchain run_tests: 33/33 suites OK, no SCRIPT ERROR in any log (includes test_runner: 119 passed, 0 failed).
• Smoke test replicating run_smoke (--fixed-fps 60 --quit-after 300): clean, no SCRIPT ERROR / ERROR: lines.
• Independent review pass over the pinned diff: GO, zero findings (repo-wide reference sweep, diff-scope check, save/load failure-mode check, docs staleness check).

CI (Test workflow: smoke, script-tests, macOS, exports) not run locally beyond the suite + smoke replication above — checks are pending on this PR; operator verifies per repo process.

Head

a30bafb493547465ef5755e0363e67c6e89e7b5d

[its-saffron]

AI Automated Review

Full PR review.

Analysis engine: MiniMax-M3 (anthropic) — primary route · pr-reviewer-action v3.4.1

Findings (1 info)

Severity	Location	Finding
Info		PR body mentions CI was not run locally beyond suite + smoke replication; CI Check Results show all gates green on the head commit, so this is informational only.

Recommendation: Approve

This is a 2-line, dead-state cleanup that maps cleanly to issue PR 415's first acceptance path ("delete the variable entirely") and does not change runtime behavior. Verified via diff and independent repo-wide reference sweep.

Change-by-change

• scripts/game_state.gd:21 — var save_supported := false removed.
• scripts/game_state.gd:38 (now :37) — save_supported = true removed from _ready().

Both removals are exactly the two write/read sites of save_supported in the diff. Repo Impact Scan term save_supported returned no git grep hits — confirms zero remaining references and no dynamic get("save_supported") access.

PR's quantified claims, falsification pass

• Claim 1 (invariants keep): held. Removing a write-only variable cannot affect save-backend selection, save format, or any call site.
• Claim 2 (use_local_storage-only gating): unverified at item level. The runtime check needs consumer-side tools I don't have here, but the save_supported symbol was independent of use_local_storage in the original code (parallel vars set independently), so deleting one cannot affect the other. Repo Impact Scan term use_local_storage not run; the claim is structural rather than behavioral.
• Claim 3 (no save-format change, explicit-key JSON.stringify): unverified at the stringify call site (would need git grep of JSON.stringify in scripts/game_state.gd), but the symbolic claim holds — save_supported was never written to a save dict.
• Claim 4 (loader validates with data.has(...)): unverified — same caveat, claim is structural.
• Claim 5 (zero reads of save_supported repo-wide): held per Repository Impact Scan (term save_supported → no git grep hits). Author's grep claim about save-backend selection citing _save_game_impl, load_game, clear_game, save_settings, load_settings, backup_save, _backup_save_local_storage, _prune_old_backups, list_backups, restore_backup in scripts/game_state.gd is plausible; I cannot falsify it without those grep results, but it is consistent with the symbol being unused.
• Loading-state safety: removed variable had default false and was flipped to true in _ready(). No code path read it (per the issue's evidence and the repo-wide sweep), so removing the default and the assignment cannot leave a stale reader.

Required check disposition

• verify audit findings are addressed: satisfied. Issue PR 415's audit finding is that save_supported is write-only, dead state. The PR deletes the declaration and its sole assignment, closes the dead-state audit finding, and the choice ("delete" vs. "make it a real guard") is explicitly defended in the PR body as deliberate scope discipline for a P3 chore. CI Check Results show all eight gates (Headless smoke test, macOS validation, Export validation (Web), Script test suite, three more export validations, Release tag validation) succeeded on head a30bafb4. No behavior change for the desktop build (issue acceptance) — confirmed by removing the only write site and having zero readers.

Standards Compliance

• The change is a minimal, focused chore on a single file (scripts/game_state.gd), matches the issue's expected-file list, and follows AGENTS.md's "make one focused change" implicit discipline.
• Validation matches the AGENTS.md-canonical invocation (godot --headless --path . --script res://tests/<suite>.gd); the report mentions Pinned Godot 4.7.1-stable (matches AGENTS.md and CONTRIBUTING.md).
• CI workflow gates (smoke, script-tests, macOS, three exports) all green on the head commit per CI Check Results.

Linked Issue Fit

Issue PR 415 acceptance criteria:

• "Either delete the variable entirely, or use it as the real guard …" — ✅ chose deletion; explicit, documented choice.
• "If kept as a real guard, tests/ covers the false branch" — N/A (variable deleted, not kept).
• "No behavior change for the desktop build" — ✅ satisfied: removed assignment was the only write; zero reads; desktop save path runs unchanged via use_local_storage branch logic in _ready().

The PR closes all three acceptance items. Scope is limited to the issue's named file (scripts/game_state.gd), with no scope drift.

Tool Harness Findings

No tool calls were issued; the corpus was reviewed directly. The save_supported reference sweep via Repository Impact Scan came back empty across scripts/scenes/tests/docs/.github/.tscn/.cfg/.json — consistent with the PR's claim of zero remaining references.

Unknowns or Needs Verification

• CI was green on the head commit at PR open, but the PR body acknowledges CI was not run locally beyond suite + smoke replication. Since CI Check Results show success on the actual head SHA, this is moot.
• I did not run git grep -n JSON.stringify scripts/game_state.gd to fully mechanically verify Claim 3 at the call-site level; the claim is structural and well-supported by the diff regardless.