Gate all suites in CI, add _on_tick integration test, finish deferred refactors#283
Conversation
Replaces the four hand-listed suite steps (linux + macOS jobs) with a loop over tests/test_*.gd so new suites are gated automatically. The TestCase base now fails a run in which zero assertions executed, so a suite whose preloads fail to compile can't exit 0 silently.
Drives ColonySim.process_tick() directly — no scene, UI, or autoloads — covering worker movement, gather/haul cycles, food upkeep cadence, goal completion/rotation, the dirty-flag contract, starvation build stalls, idle behavior, and build completion with milestone advance. Requires the sim to be autoload-free: the reservation rebuild is now a static ColonySim function that GameState delegates to, instead of the sim reaching into the GameState autoload. Also flattens do_haul into _deliver_carried/_pick_up_for_build now that the path is covered.
- Popup visibility is a single MenuMode state machine (set_menu_mode); the four panel flags can no longer desync across open/close paths. - apply_anchor_layout's mirrored reparenting branches collapse onto an ordered (node, parent, index) table with a shared tail. - Structure/tile icons and short labels move to constants.gd tables; next_unlock_text walks BUILD_UNLOCKS instead of hardcoding the chain. - LayoutMath exports ALL_ANCHOR_FAMILIES and BOTTOM_DOCK_MIN_HEIGHT; save validation and main.gd consume them (removes the mirrored anchor list and the duplicated 460px minimum); drop dead tile_is_square. - main.gd uses Constants.X uniformly instead of local const re-aliases.
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 is well-executed and addresses its stated goals comprehensively. The new tick integration test (issue PR 234), CI test discovery loop, autoload-free refactor, and popup state machine are all high-quality additions that align with repository conventions.
Change-by-Change Findings
CI / test discovery (.github/workflows/test.yml, .justfile)
- Replaces 4 hand-listed test steps with a
for suite in tests/test_*.gdloop that automatically gates all 19 suites (excludingtest_casebase). This is strictly better than the previous approach — suites added later are automatically caught. .justfilegains atest-allcommand that mirrors the CI matrix exactly, improving local validation fidelity.- Minor behavior change (acknowledged in PR notes): the
test_runner.gdsuite is now run alongside 18 others rather than as a wrapper. This is fine — the test logic is unchanged.
New integration test (tests/test_tick_integration.gd, 202 lines)
- Instantiates
ColonySimdirectly (no scene tree, no autoloads) and drivesprocess_tick()programmatically. - Covers all required acceptance criteria: worker movement, gather/haul cycle, food upkeep cadence, goal completion/rotation, dirty flag contract, starvation stalls, idle behavior, and build completion with milestone advance.
- Acceptance criteria deviation: The issue explicitly requires inclusion in
test_runner.gd. The PR runs it as a standalone suite via the CI loop instead, citing the post-PR 281 one-suite-per-file convention. Functionally the test runs in CI (all test_*.gd except test_case are auto-discovered), so the spirit of the criterion is met. This is acceptable.
Autoload-free ColonySim (scripts/colony_sim.gd, scripts/game_state.gd)
rebuild_reservations_from_workers()is now a staticColonySimmethod.GameStatedelegates to it. This enablesColonySimto be instantiated in--scriptmode without triggering autoload initialization — a prerequisite for the integration test.do_haul()is flattened into_deliver_carried()and_pick_up_for_build(), with improved inline documentation. The logic is unchanged.
Popup state machine (scripts/main.gd)
MenuModeenum (CLOSED,MENU,MANAGEMENT,SETTINGS) replaces scatteredsidebar_scroll.visible/menu_actions.visible/management_panels.visible/settings_panel.visibleflag juggling.set_menu_mode()is the single mutating entry point; all open/close paths (toggle_menu,close_menu,open_build_popup,open_settings,close_settings,save_game,_load_failed,load_saved_game,begin_build_placement,queue_structure_at,cancel_build_placement,_on_dock_side_selected) delegate to it.- Minor behavior change (acknowledged in PR notes): closing the popup now always reapplies dock sizing (previously skipped on some paths), and
_load_failedcloses the popup instead of leaving it half-open. These are correct fixes. _apply_header_tree()consolidates the mirrored reparenting branches inapply_anchor_layoutonto an ordered[node, parent, index]table. Identical style properties for both anchors are now shared after the branch.
Constants consolidation (scripts/constants.gd, scripts/main.gd)
STRUCTURE_ICONS,TILE_ICONS,TILE_SHORT_LABELSadded toConstants.tile_icon()andtile_amount_text()inmain.gdnow delegate to these tables instead of inlinematchstatements.next_unlock_text()now walksConstants.BUILD_UNLOCKS— adding a new structure tier only requires a table entry, not editing function logic.main.gdpreloads cleaned up: individual const extractions removed; code consistently usesConstants.X.
LayoutMath exports (scripts/layout_math.gd, scripts/game_state.gd, scripts/main.gd)
ALL_ANCHOR_FAMILIESandBOTTOM_DOCK_MIN_HEIGHTexported as public constants.game_state.gd's_expected_grid_sizes()now iteratesLayoutMath.ALL_ANCHOR_FAMILIESinstead of hardcoding["bottom", "side"].main.gdusesLayoutMath.BOTTOM_DOCK_MIN_HEIGHTinstead of the literal460.
Test base hardening (tests/test_case.gd)
- Added zero-assertion guard: if
test_pass == 0 and test_fail == 0, exits with code 1. This catches preload compile failures that would silently exit 0.
Standards Compliance
- Autoload-free sim:
ColonySimis aRefCountedclass with no autoload dependencies, enabling standalone--scriptmode testing. ✅ - Save/version migration: Reservation rebuild is a static function callable from both load path (
GameState) and runtime (ColonySim.rebuild_reservations()). ✅ - Constants over scattered literals: Icon tables,
BOTTOM_DOCK_MIN_HEIGHT,ALL_ANCHOR_FAMILIESare centralized. ✅ - Popup as overlay: The state machine keeps the sidebar transient; dock sizing is reapplied on every mode change. ✅
- No
tile_is_squarestub: Removed the no-optile_is_squarefunction (always returnedtrueby comment; body was "by construction"). This was dead code. ✅ - test_case exclusion: Both CI and
.justfileskiptest_case(the base class), matching the expected pattern. ✅
Linked Issue Fit
Issue PR 234 — "Add full-cycle integration test for _on_tick"
| Criterion | Status |
|---|---|
tests/test_tick_integration.gd exists and runs headless |
✅ Added; gated by CI loop |
| Covers worker movement, resource gathering | ✅ test_tick_advances_and_worker_moves, test_gather_and_haul_cycle |
| Covers food upkeep | ✅ test_food_upkeep_interval |
| Covers goal tick | ✅ test_goal_completion_and_rotation |
| Runs without full UI scene | ✅ Instantiates ColonySim directly; no Control/autoload |
Included in test_runner.gd |
test_runner.gd. Functionally included (CI runs all test_*.gd except test_case), and matches post-PR 281 convention. Acceptable. |
Tool Harness Findings
No tool harness output present in corpus.
CI Check Results
All CI checks passed (6/6): Export validation (Linux/macOS/Windows), Script test suite, macOS validation, Headless smoke test. Test suite exited 0 with 724 passing assertions. CI is green.
Unknowns / Needs Verification
None. All evidence is complete and consistent.
… regression (#284) * Fix tile theme dictionary keys corrupted by the constants-alias sweep The #283 alias rewrite also renamed the string KEYS inside main.gd's cached TileRender theme dicts ('TILE_BACKDROPS' became 'Constants.TILE_BACKDROPS'), so every tile silently fell back to default colors. Fix the keys, move the accent palette into constants.gd (passed through TileRender's theme context, fallbacks unchanged), add a regression test for the main→TileRender wiring, and make CI/just test-all fail any suite whose log contains SCRIPT ERROR — runtime script errors inside a passing suite previously went unnoticed. * Polish sweep: close out the remaining review findings - WorkerCapLogic.count_active_workers is the one definition of an active worker (HUD count + milestone progress). - Crew list computes the colony-wide idle reason once per render; worker_intent_text accepts it as an optional precomputed arg. - ColonySim caches the current milestone catalog entry between advances and indexes builds by id (self-verifying against state swaps). - GoalReward magnitudes live in REWARD_MAGNITUDES next to the catalog. - game_state.gd persistence I/O consolidated into four shared helpers (localStorage double-stringify and file read/write live in one place). - Focus-mode multiplier and zoom bounds move to constants.gd; RESOURCE_TILES table defines resource yields/seed/drop amounts and drop messages in one place (gatherability derives from membership).
Closes #234. Follow-ups from #282.
Summary
just test-allloop over everytests/test_*.gdinstead of four hand-listed suites — the other 15 suites are now actually gated. The TestCase base fails any run where zero assertions executed, so a suite whose preload fails to compile can't silently exit 0.tests/test_tick_integration.gd(Add full-cycle integration test for _on_tick #234): drivesColonySim.process_tick()with no scene/UI/autoloads — worker movement, gather/haul cycles, food upkeep cadence, goal rotation, dirty-flag contract, starvation build stalls, idle behavior, build completion + milestone advance. Deviation from the issue text: it's a standalone suite gated by the CI loop rather than folded intotest_runner.gd, matching the post-Refactor test suite onto a shared TestCase base with a canonical assertion API #281 one-suite-per-file convention.ColonySimfunctionGameStatedelegates to (this was required for the integration test to compile in--scriptmode).do_haulflattened into_deliver_carried/_pick_up_for_buildunder that new coverage.MenuModestate machine;apply_anchor_layout's mirrored reparenting branches collapse onto an ordered node/parent/index table; structure/tile icons + short labels move toconstants.gd;next_unlock_textwalksBUILD_UNLOCKS;LayoutMathexportsALL_ANCHOR_FAMILIESandBOTTOM_DOCK_MIN_HEIGHT(save validation and main.gd stop mirroring them);main.gdusesConstants.Xuniformly.*.logignored;smoke.log/smoke_ci.loguntracked.Verification
--fixed-fps 60 --quit-after 300): no script errors.Notes