Polish sweep: close out remaining review findings; fix tile theme key regression

[joryirving]

Final sweep of the review-finding backlog, plus one regression fix.

Regression fix (from #283)

• The constants-alias rewrite in Gate all suites in CI, add _on_tick integration test, finish deferred refactors #283 also renamed the string keys inside main.gd's cached TileRender theme dicts ("TILE_BACKDROPS" → "Constants.TILE_BACKDROPS"), so tiles silently rendered with fallback colors. Fixed, with a new regression test covering the main→TileRender wiring, and CI/just test-all now fail any suite whose log contains SCRIPT ERROR (runtime errors inside a passing suite previously went unnoticed — which is also why the new test initially reported nothing).

Summary

• One definition of "active worker" (WorkerCapLogic.count_active_workers) shared by the HUD and milestone progress.
• Colony-wide idle reason computed once per crew-list render instead of per idle worker.
• ColonySim: current-milestone cache (skips the per-tick catalog scan + deep copy) and an id→index cache for build lookups, self-verifying against wholesale state swaps (builds are append-only).
• GoalReward.REWARD_MAGNITUDES puts reward strength next to the catalog labels.
• game_state.gd persistence I/O consolidated into four shared helpers — the web localStorage double-stringify dance and file read/write each exist once.
• constants.gd gains the tile accent palette (passed through TileRender's theme context; module fallbacks unchanged), focus-mode/zoom tuning values, and a RESOURCE_TILES table (yield, seed/drop amounts, drop messages; gatherability derives from membership).

Verification

• All 19 suites headless: exit 0, 728 passing assertions, zero script errors.
• Full-game headless smoke boot: no script errors.

Notes

• One backlog item intentionally closed without code: threading persist() — the debounce added in Shared TestCase base for all suites; extract ColonySim from main.gd #282 (saves at most every 10 ticks, flushed on quit/explicit actions) removed the per-tick serialization cost that motivated it.
• Minor web-platform behavior tightening: a localStorage save that parses to a non-Dictionary now loads as empty instead of being returned as-is, and the string-wrapped legacy format also gets its reservations rebuilt on load.

[its-saffron]

AI Automated Review

Full PR review.

Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic) — escalated (fast_low_confidence)

Recommendation: Approve

This is a well-scoped polish sweep that closes out the tile theme regression from PR 283, introduces a new regression test that catches the failure mode, hardens the CI/test pipeline to surface runtime errors, and makes several correctness and performance improvements to the simulation and persistence layers.

---

Change-by-Change Findings

Regression Fix (Tile Theme Keys)

The diff between main.gd and tile_render.gd is the heart of the regression fix. The previous rewrite inadvertently stored string keys like "Constants.TILE_BACKDROPS" (the literal text of an identifier) in the theme dict instead of "TILE_BACKDROPS". This caused theme.get("TILE_BACKDROPS", ...) to always miss and fall back to defaults — silently. Verified: The theme dict in main.gd now uses plain string keys ("TILE_BACKDROPS", "DEFAULT_BACKDROP", "RESOURCE_COLORS", "STRUCTURE_COLORS", "TILE_ACCENTS") and tile_style/tile_accent look them up with the same string keys, so the wiring is correct.

New Regression Test (tests/test_tile_render.gd)

The _test_main_theme_wiring() test exercises the exact seam: it creates a main.gd instance, calls tile_accent and tile_style for a tree tile, and asserts the colors equal C.RESOURCE_COLORS["wood"] and C.TILE_BACKDROPS["tree"] directly. This would have caught the PR 283 regression immediately. Also verifies stylebox caching by checking two identical tiles share one object (cached == style). Source: tests/test_tile_render.gd lines 298–328.

CI / Local Test Hardening

Both .github/workflows/test.yml (Linux + macOS jobs) and .justfile now && ! grep -q "SCRIPT ERROR" on the test output log. Previously, a suite could exit 0 with runtime errors inside it. The PR body explicitly calls this out: the new regression test initially reported nothing precisely because runtime errors were swallowed. This is a meaningful improvement to the validation gates.

Shared Active-Worker Definition (WorkerCapLogic.count_active_workers)

main.gd::active_worker_count() and milestone_manager.gd::evaluate_milestone (for MILESTONE_TYPE_WORKER) both previously inlined the same loop over workers checking break_ticks <= 0. The PR extracts this to WorkerCapLogic.count_active_workers(workers) and both call sites now delegate to it. Source: scripts/worker_cap_logic.gd lines 22–29, scripts/main.gd line 928, scripts/milestone_manager.gd lines 106–107. Verified: WorkerCapLogic is declared with class_name at worker_cap_logic.gd:1; milestone_manager.gd preloads it at line 10.

Milestone Lookup Cache (colony_sim.gd::_current_milestone)

The milestone catalog only changes when the chain advances, so per-tick the scan + deep copy was wasteful. The new _milestone_cache / _milestone_cache_id pattern skips the work unless milestone_id changed. Source: scripts/colony_sim.gd lines 134–142. Verified: _current_milestone is called at line 149; cache invalidation on ID change is correct.

Build Index Cache (colony_sim.gd::_build_array_index)

Build lookups were O(n) linear scans on every haul/build step. The new _build_index maps build.id → array index. The self-verification pattern (size check + dual ID verification against the array entry) guards against stale cache when state was swapped wholesale. Source: scripts/colony_sim.gd lines 604–639. Verified: get_build and set_build both use _build_array_index. The comment correctly notes builds are append-only.

Colony-Wide Idle Reason Deduplication (main.gd::_render_crew_list)

worker_idle_reason() was being called per idle worker in the crew list, but the result is colony-wide (not worker-specific). The PR computes it once as idle_text before the worker loop and passes it to worker_intent_text(). Source: scripts/main.gd lines 1553–1565.

Persistence I/O Consolidation (game_state.gd)

Four private helpers (_local_storage_write, _local_storage_read, _write_text_file, _read_json_file) are the sole location for the web localStorage double-stringify quirk and file I/O. save_game, load_game, save_settings, and load_settings all delegate to them. The web load path also now returns empty {} instead of a non-Dictionary value when parsing fails, tightening behavior. Source: scripts/game_state.gd lines 30–83.

RESOURCE_TILES Table (constants.gd)

All three gatherable tile types (tree, rock, berries) are now defined in one place with resource, seed_amount, drop_amount, and drop_message. colony_sim.gd::spawn_resource_drop and seed_tile use this table, and gatherability is derived from membership (Constants.RESOURCE_TILES.has()). Source: scripts/constants.gd lines 102–128.

GoalReward.REWARD_MAGNITUDES

Reward strengths are centralized next to the catalog constants. The four magnitudes (gather speed 1.5, haul speed 1.5, build speed bonus 0.16, trickle 1) were previously inline literals in the getter functions. Source: scripts/goal_reward.gd lines 20–28; all four getters updated to use the table at lines 133, 140, 147.

Tile Accent Palette (constants.gd, tile_render.gd)

TILE_ACCENTS (placement ok/blocked, stockpile, foundation, default border alpha) and TILE_DEFAULT_BACKDROP are now centralized in constants.gd. tile_render.gd::tile_style and tile_accent use theme.get("TILE_ACCENTS", {}) with hardcoded literal fallbacks for minimal-theme callers, so the module boundary is safe. Source: scripts/constants.gd lines 48–55, scripts/tile_render.gd lines 47–75.

---

Standards Compliance

• Save/version migration: Not applicable — no save schema fields added or changed.
• Constants centralization: constants.gd is the single source for tile accent palette, zoom bounds, focus-mode multiplier, and the resource tile table. All consumers read from it.
• Module boundaries: TileRender remains a static utility with explicit theme/context parameters. Fallback literals in tile_accent mean it degrades gracefully if a caller passes a minimal theme dict.
• No main push: This is a PR to main, not a direct push.
• No release after every change: Not applicable (this is a feature PR, not a release workflow).

---

Tool Harness Findings

The tool harness ran read_file on constants.gd and colony_sim.gd, and git_log. All reads succeeded and are consistent with the diff.

---

Unknowns / Needs Verification

No unknown items. All changed code has been verified against the diff and supporting corpus evidence. CI passed all six checks (Linux export validation, macOS validation, macOS export, script test suite, headless smoke test, Windows export) for commit 3ba4158.