Menu + render polish bundle: pause menu, save/load dialog, pheromone toggle, round entrances#118
Conversation
Replaces the square hole + 2-px dirt rim at colony entrances with a round hole sitting in a piled-dirt mound. Three concentric fillCircles per entrance: outer mound rim (lighter spoil tone), mound body (damp earth), and the dark hole interior. Mound spills 2 px onto each adjacent tile so the entrance reads as a 3-D bump rather than a flat painted disc; the viewport cull margin widens accordingly. Enemy entrances now ring the mound with a 1-px COLOR_ENEMY_COLONY strokeCircle (issue #14 cue) instead of a four-rect rectangle, so the "invade-here" cue follows the new circular silhouette. The Phase 8.5 right-click pending-entrance preview keeps its gold tile frame — the explicit affordance reads as "you haven't placed it yet" more clearly than a preview of the final mound shape would. - New COLOR_BARREN_EARTH_MOUND in terrain-atlas.ts (between DAMP and BARREN_EARTH). - Updated #14 perimeter test to assert the new strokeCircle contract. - Added a positive concentric-circles test pinning radii (mound 10 / body 9 / hole 6 at TILE_SIZE_PX = 16). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Esc now opens a centered pause-menu overlay on UIScene that pauses the sim via gameLoop.pause() and presents Resume / Save / Load / Settings / Download debug log. Replaces the bare P-pause keybinding so P is free for the pheromone toggle in issue #114; speed multipliers (1/2/4) and the pan + Tab + X bindings are unchanged. Architecture - pause-menu-layout.ts — pure-TypeScript layout + hit-test helpers (CANVAS coords, button stack geometry, two-page state machine: main / settings). Mirrors context-menu-layout's pattern. - platform/settings.ts — versioned localStorage-backed Settings store (subterrans:settings:v1). Permissive merge: unknown keys ignored, malformed values fall back to per-field defaults so a single corrupt field can't wipe valid neighbors. Distinct namespace from save.ts so cosmetic preferences survive deleteSave(). - ui-scene.ts — show/hide/isVisible methods + dispatcher; reuses the existing Phaser-overlay pattern (input-blocking bg rect at depth 20, per-button interactive rectangles, setActiveOverlay for Playwright). Esc handler wired to consume when the menu is visible so it closes the menu instead of falling through to ant-activity hide. - game-scene.ts — Esc binds to openPauseMenu / closePauseMenu helpers that flip GamePhase between Playing and Paused and pump gameLoop.pause/resume. Early-returns under SavePrompt and GameOver so those phases keep their existing input precedence. Pheromone overlay flag - ViewState gains showPheromoneOverlay (issue #114 prep). Hydrated from settings on createViewState / resetViewState; the menu's Settings sub-screen toggle persists via saveSettings and writes back through ViewState. Issue #114 will land the per-frame draw skip and the P keybinding. Save / Load row - Renders disabled until issue #115 wires the dialog. The row stays visible (clear "coming soon" affordance) but does nothing on click. Tests - Full settings.ts round-trip + corrupted-input fallback coverage. - pause-menu-layout: page composition, Save/Load enabled gating, geometry invariants, hit-test (incl. disabled rows are skipped). - camera.test.ts: showPheromoneOverlay default + reset behavior. - ViewState fixtures across input tests updated for the new field. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
P now toggles the player's pheromone overlay visibility for the session AND persists the choice to settings (subterrans:settings:v1) so it survives reloads. The render loop in game-scene.update() gates the drawPheromoneOverlay call on viewState.showPheromoneOverlay (the field landed in issue #116's prep work, hydrated from settings on boot). The pause menu's Settings sub-screen toggle, the P key, and persisted storage all share the same write path: read settings → flip flag → saveSettings → mirror to viewState. Either input affects the next rendered frame. Player colony only — enemy pheromones stay hidden regardless of toggle state (PRD §7b / T-08-05). The toggle is render-only; simulation is unaffected, and the flag never round-trips through save.ts. Inert during SavePrompt so a stray P press while the boot prompt is visible can't quietly flip the setting before the player has even chosen Continue or New Game. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a Save/Load dialog reachable from the pause menu's Save / Load row.
Renders on top of the pause menu (menu hides while dialog is open;
Back re-shows it). Game stays paused throughout — Continue / New Game
are the only paths that resume the loop.
Buttons (with confirm flow on destructive actions):
- Continue — load existing save and resume play (enabled iff hasSave)
- Save Now — manualSave; info line refreshes with the new tick
- Delete Save — first click arms; second click commits. Stays enabled
when an incompatible save sits in storage so issue
#66 leftovers can be cleared.
- New Game — confirm gate only when a save exists (no progress at
risk otherwise)
- Back — close dialog, return to pause menu
save.ts API additions
- manualSave(seed, inputLog, world): boolean — explicit write that does
NOT touch the autosave timer. Returns false on quota / blocked storage.
- hasIncompatibleSave(): boolean — distinguishes "no save" from "save
bytes present, this build can't read them" so the dialog can surface
the issue #66 case instead of silently booting fresh.
- getSaveInfo(): SaveInfo | null — pulls tick + player worker count +
player food (human units) WITHOUT a full deserialize. Safe to call on
every dialog open.
UI plumbing
- save-load-dialog-layout.ts mirrors pause-menu-layout: pure-TypeScript
layout + hit-test, two-flag confirm state machine, formatSaveInfoLine
composer (also pure).
- UIScene gains show/hide/isVisible methods + a pointerdown precedence
rule: dialog > pause menu > HUD. Esc on the dialog is "Back"; Esc on
the pause menu underneath is "Resume" (consistent "close topmost" UX).
- GameScene now supplies onOpenSaveLoad to the pause menu (lighting up
the row landed disabled in #116) and threads onContinue / onNewGame /
onSaveNow / onBack through to the dialog. onContinue replays
bootFromSave; onNewGame goes through restartGame's existing paths.
- ActiveOverlay enum gains 'save-load' for Playwright observability.
- Test fixtures unaffected: 1998 vitest tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef32386074
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Issue-context review, comparing this PR against #114, #115, #116, and #117:
The broader approach still looks sound to me: no |
…ke test + round-2 self-review) Round 1 (Codex automated review + repo owner smoke test) flagged: - P1: Esc pause menu unreachable in runtime — GameScene's keydown-ESC and UIScene's escKey handler both fired on each press; GameScene opened the menu, UIScene immediately closed it via the ant-activity hide path, __phase9_ui.activeOverlay landed at 'none'. - P1: scene-level + per-button pointerdown both dispatched on a click, bypassing the 2-click confirm gate (one-click delete) and double-flipping the pheromone toggle (apparent no-op). - P2: Save info missing timestamp; Save Now had no success/failure feedback. Round 2 self-review surfaced: - HIGH: hide…Overlay() unconditionally published activeOverlay='none' even when other overlays were still up — observability lie that round 1 was supposed to kill. - HIGH: keydown-P / keydown-X / keydown-1/2/4 all fired while the pause menu was open. Pressing P with the Settings sub-screen open desynced the on-screen "ON/OFF" label from the persisted setting. - HIGH: openPauseMenu callback payload was duplicated inline inside openSaveLoadDialog's onBack — drift waiting to happen. - MEDIUMs: cb capture pattern in dispatchSaveLoadDialogItem was undocumented; formatSaveTime returned "NaN:NaN" for tampered huge timestamps; manualSave didn't bump the autosave cooldown so an autosave seconds later would overwrite the manual save's displayed timestamp; hasIncompatibleSave didn't run the legacy-save purge that hasSave does; new-game success path didn't explicitly reset confirming.newGame; hidePauseMenuOverlay didn't cascade to hideSaveLoadDialogOverlay. Fixes ----- UIScene - Removed the GameScene-side keydown-ESC binding; UIScene is now the single owner of Esc with a documented precedence ladder (dialog > menu > ant-activity > onEscape callback). GameScene supplies onEscape via the scene.launch payload. - Removed per-button pointerdown handlers in renderPauseMenuPage and renderSaveLoadDialog. Scene-level pointerdown's hit-test is the sole dispatcher; button rectangles keep setInteractive() for pointer absorption / cursor styling. - Added recomputeActiveOverlay() — every hide…/show… method now publishes the actual topmost overlay rather than unconditionally 'none'/the shown one. - hidePauseMenuOverlay now defensively cascades to hideSaveLoadDialogOverlay. - dispatchSaveLoadDialogItem captures `cb` at the top with an explicit comment block. - new-game success path explicitly resets confirming.newGame. - Save Now feedback: a brief "Saved" / "Save failed" flash above the info line, driven by a Phaser delayedCall (SAVE_FLASH_MS = 1500ms). Cancelled on hide; the delayedCall callback also has a visibility guard against scene-shutdown races. GameScene - keydown-P, keydown-X, keydown-1/2/4 all early-return if gamePhase !== Playing. F9 (debug snapshot) and Tab (view toggle) remain unguarded because they're inspection-only during pause. - Extracted buildPauseMenuCallbacks() helper; openPauseMenu and the dialog's onBack both call it (no inline duplication). - onSaveNow callback bumps lastAutosaveMs after a successful manualSave so the next autosave window doesn't fire seconds later and overwrite the manual save's displayed timestamp. save.ts - SaveFile envelope gains optional savedAtMs (Date.now() in buildSaveFile). Pre-fix saves load fine — the field is optional; getSaveInfo returns 0 when absent or malformed. - SaveInfo gains savedAtMs. - hasIncompatibleSave now calls purgeLegacySaves (symmetry with hasSave / loadSave). save-load-dialog-layout.ts - formatSaveInfoLine includes "Saved HH:MM" when savedAtMs > 0. - formatSaveTime returns "—" for 0, NaN, and huge values that construct an Invalid Date (tampered envelope guard). Tests ----- - src/platform/save.test.ts: 5 new tests for manualSave timestamp round-trip, getSaveInfo's savedAtMs (present, absent, malformed), and the round-2 fix coverage. - src/render/save-load-dialog-layout.test.ts: 4 new tests for formatSaveTime (zero, normal, late-night, NaN guard) and formatSaveInfoLine timestamp inclusion. - tests/menu-and-dialog.spec.ts (new Playwright spec): 9 runtime regressions covering Esc opens menu in single press, single-click dispatch, P-key toggle persists, Save/Load dialog reachable, 2-click delete confirm, round-2 review's "Esc back doesn't flash activeOverlay = none", and "P pressed during pause is no-op." Verification ------------ - npm run typecheck: clean - npm run lint: 6 errors all pre-existing on main (PR introduces none) - npx vitest run: 2008/2008 passing (10 new vs round 1) - npx playwright test (smoke + menu-and-dialog): 15/15 passing - npm run build: clean Closes the Codex P1 review findings on #118. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Round-2 fixes pushed in 62726e8. Addressed all P1 / P2 findings from your review and Codex's automated review: P1 — Esc unreachable — UIScene now owns Esc with documented precedence (dialog > pause-menu > ant-activity > GameScene P1 — Duplicate dispatch — Per-button P2 — Save info timestamp + Save Now confirmation — Round-2 self-review caught + fixed before push:
Verification:
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62726e837f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… P2) Codex round-2 review (commit 62726e8) flagged that hasIncompatibleSave only validated the envelope via parseSaveFile and never checked snapshot.simVersion against LATEST_SIM_VERSION. A save written by a newer build (envelope shape unchanged, simVersion bumped) would: - parseSaveFile succeed → hasIncompatibleSave returns false - Save/Load dialog enables Continue - Player clicks Continue - bootFromSave's deserializeWorldState throws FutureSimVersionError - Falls through to bootFresh with autosave suspended (issue #66 path) Net: Continue was a UI lie — it looked actionable but couldn't actually load the save the player was looking at. Fixes ----- - save.ts hasIncompatibleSave now also rejects when snapshot.simVersion exceeds LATEST_SIM_VERSION. parseSaveFile still ONLY validates the envelope; the dialog-layer compatibility check is layered on top. Defensive type-check: only fires when simVersion is present and a number (legacy saves omit the field, default to LEGACY on load). - ui-scene.ts dialog context now derives hasCompatibleSave as `hasSave() && !hasIncompatibleSave()`. Continue stays disabled for future-sim saves; the existing Delete + New Game paths cover recovery. - save-load-dialog-layout.ts formatSaveInfoLine now shows the warning preferentially when hasIncompatibleSave is true, even if getSaveInfo returned a parseable summary. Without that ordering the info line read "Saved 14:30 · Tick 100 · …" while Continue was grayed out — visually contradictory. The boot-path hasSave keeps its envelope-only check so the SavePrompt still appears on cold boot for future-sim saves (preserving the issue #66 recovery flow). The dialog uses the layered check. Tests ----- - save.test.ts: 3 new tests for hasIncompatibleSave covering simVersion > LATEST (incompatible), simVersion === LATEST (loadable), and missing simVersion (legacy default — loadable). - save-load-dialog-layout.test.ts: updated the "info vs incompatible" ordering test to verify the warning wins when both signals are present. Verification ------------ - npm run typecheck: clean - npx vitest run: 2011/2011 passing (3 new) - npm run lint: matches main's pre-existing baseline - npx playwright test: 15/15 passing - npm run build: clean Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Round-3 fix in 9e25e4a addresses the P2: @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e25e4afb4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…(Codex round-3 P1) Codex round-3 review (commit 9e25e4a) flagged two paths that could throw when the dialog opened against a parseable envelope with a malformed snapshot: 1. hasIncompatibleSave() dereferenced file.snapshot.simVersion. With snapshot: null or any non-object, this throws TypeError and crashes the dialog open path before the user can click Delete to recover. 2. getSaveInfo() dereferenced file.snapshot.colonies[playerKey]. Same shape: a snapshot missing `colonies` would throw, again crashing the dialog open instead of falling through to the "incompatible save" warning + Delete/New Game recovery. Both functions now defensively check that `snapshot` is a non-null object before dereferencing. hasIncompatibleSave returns true (the save IS incompatible — bootFromSave would reject it too); getSaveInfo returns null so formatSaveInfoLine falls back to the warning branch. getSaveInfo also defensively type-checks tick / colonies / foodStored / workerCount in case the snapshot has SOME fields but not others — surfaces 0 for missing or malformed numerics rather than rendering "Tick undefined" in the dialog. The recovery flow is now: - Cold-boot SavePrompt: hasSave returns true (envelope parses), Continue attempts bootFromSave, deserialize throws, falls through to bootFresh. Existing path, unchanged. - Mid-session pause menu → Save/Load: dialog opens cleanly, info line shows "Incompatible save in storage — start a new game or delete it", Continue is disabled, Delete is enabled, New Game with confirm is enabled. The user has a clear recovery path. Tests ----- - 2 new hasIncompatibleSave tests (snapshot=null, snapshot=string) - 3 new getSaveInfo tests (snapshot=null, missing colonies, missing tick) - All 5 cover behaviors that pre-fix would have thrown TypeError Verification ------------ - npm run typecheck: clean - npx vitest run: 2016/2016 passing (5 new) - npm run lint: matches main's pre-existing baseline - npx playwright test: 15/15 passing - npm run build: clean Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Round-4 fix in 6d840ef addresses both P1s:
Net: the dialog open path is now crash-proof for any parseable envelope, no matter how malformed the snapshot. The "incompatible save → Delete/New Game" recovery path stays reachable. 5 new save.test.ts cases lock in the no-throw behavior for snapshot=null, snapshot=string, missing colonies, and missing tick. @codex review |
|
Codex Review: Didn't find any major issues. Keep it up! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
One remaining issue-context nit after rechecking latest
This is no longer a crash thanks to the latest defensive checks, but it preserves the same UI-lie class as the future-sim case: Continue looks actionable for a save this build cannot load. I’d either make |
…lity boundary (Rob P2) Rob's manual review of 6d840ef flagged that hasIncompatibleSave still permitted UI-lie states. The prior implementation rejected only: 1. envelope parse failures 2. non-object snapshots 3. simVersion > LATEST_SIM_VERSION It still accepted parseable envelopes whose snapshot was shape-valid at the top level but missing required deeper fields, e.g.: - snapshot: {} - snapshot: { tick: 0, rngState: 0 } // no ants / colonies - snapshot: { ..., simVersion: LEGACY_SIM_VERSION - 1 } In those cases the dialog's `hasCompatibleSave: hasSave() && !hasIncompatibleSave()` evaluated to true, Continue appeared clickable, but bootFromSave's deserializeWorldState would throw and silently fall through to bootFresh. Same UI-lie class as the future-sim case round-3 fixed. Fix: align the dialog's compatibility boundary with the canonical "is this loadable?" check by attempting the full deserialize. Any throw (FutureSimVersionError, validation errors from missing/malformed fields, SimVersion-below-LEGACY tampering, etc.) means Continue would not actually load the save, so we surface as incompatible. The narrower simVersion guard is removed — deserializeWorldState's own validateSimVersion handles both bounds (< LEGACY → plain Error, > LATEST → FutureSimVersionError), plus every other field check. UIScene caches hasIncompatibleSave() once per renderSaveLoadDialog() in a local variable so the round-trip's single dialog render doesn't pay the deserialize cost twice for the two ctx fields that read it. Cost: one WorldState allocation (~8192 ants worth of typed arrays plus colonies / grids) per dialog open. The dialog is user-initiated and the deserialize is a one-shot probe — acceptable. Tests ----- - Pre-existing 5 hasIncompatibleSave tests (parse-fail, future-sim, equal-LATEST, missing simVersion legacy, null/string snapshot) all still pass — the deserialize boundary subsumes the prior checks. - 3 new tests cover Rob's specific scenarios: - snapshot: {} (empty object → deserialize throws) - snapshot.simVersion = LEGACY - 1 (tampered down → plain Error) - snapshot missing colonies field → deserialize throws Verification ------------ - npm run typecheck: clean - npx vitest run: 2019/2019 passing (3 new) - npm run lint: matches main's pre-existing baseline - npx playwright test: 15/15 passing - npm run build: clean Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Round-5 fix in b115a5f addresses your latest P2.
3 new save.test.ts cases lock in @codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b115a5f5c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ound-5 P1) bootFromSave catches FutureSimVersionError and sets autosaveSuspended=true so the preserved newer-build save bytes survive in localStorage for recovery (user reloads on a newer build that knows the simVersion). tickAutosave already respects this flag at update() time, but the Save/Load dialog's Save Now path called manualSave unconditionally — silently overwriting the protected bytes with the fall-through fresh session and destroying the recovery point. Fix: gate manualSave on `!this.autosaveSuspended` inside the onSaveNow callback. When suspended, return false → the dialog renders the "Save failed" flash so the player knows the action didn't take. Recovery paths remain: explicit Delete clears the preserved bytes; reloading on a newer build restores them. Tests ----- - tests/menu-and-dialog.spec.ts: new e2e seeds a future-sim save (simVersion > LATEST), boots via SavePrompt → Continue, which lands in autosaveSuspended state, then opens the dialog and clicks Save Now. Asserts the preserved localStorage bytes are unchanged after the click. Pre-fix this would have overwritten them. Verification ------------ - npm run typecheck: clean - npx vitest run: 2019/2019 passing - npm run lint: matches main's pre-existing baseline - npx playwright test (menu-and-dialog): 10/10 passing (1 new) - npm run build: clean Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Round-5b fix in 727ab48 addresses your latest P1. New Playwright e2e in tests/menu-and-dialog.spec.ts seeds a future-sim save → boots through SavePrompt → Continue (lands in suspended state) → opens the Save/Load dialog → clicks Save Now → asserts the preserved localStorage bytes are unchanged. Pre-fix this would have silently overwritten them. @codex review |
|
Codex Review: Didn't find any major issues. Keep it up! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…tings menu Two UAT findings on PR #118 round 5b: 1. PHEROMONES NEVER RENDERED VISIBLY (pre-existing P1 since 9f5b23f) GameScene called drawPheromoneOverlay BEFORE drawSurface, but the drawSurface orchestrator paints opaque terrain THEN entities. The pheromone overlay landed on canvas first, then the terrain pass wiped it out. The docstring in draw-pheromone.ts said the overlay was "called between terrain and entities" but the actual call site has been wrong since the Phase 9.06 wiring. Issue #114's toggle key surfaced the bug — both ON and OFF rendered identically because ON was always being overpainted. Fix: call drawSurfaceTerrain / drawUndergroundTerrain explicitly, then drawPheromoneOverlay (if flag), then drawSurfaceEntities / drawUndergroundEntities. Pheromones now land between layers as the docstring intended. Verified via the debug snapshot Rob captured at tick 13577 — 46 non-zero FoodTrail cells, peak 2253 (above the PHEROMONE_VISUAL_MAX of 2048, so full alpha 0.6). Screenshots confirm a clearly visible green trail from the entrance to food piles after 600 sim-seconds of foraging. 2. SPEED MULTIPLIER ADDED TO SETTINGS SUB-SCREEN (Rob UAT) The 1/2/4 keyboard shortcuts had no HUD readout — players had no way to discover them. Settings page now has a "Speed: N×" row that cycles 1→2→4→1 on click. Same write path as the keys: both set speedMultiplier on GameScene. Session-only: speed does NOT persist to settings (matches the Phase 4 fresh-boot contract that resets to 1× on restart). The menu reads/writes via getSpeedMultiplier / onCycleSpeed callbacks so the layout module stays free of GameScene type knowledge. GameScene.speedMultiplier narrowed from `number` to `1 | 2 | 4` matching the SpeedMultiplier union exported from pause-menu-layout. Tests ----- - src/render/pause-menu-layout.test.ts: speed-cycle row position, label reflects current speed (1×/2×/4×), nextSpeedMultiplier cycles 1→2→4→1 (3 new tests). - tests/menu-and-dialog.spec.ts: clicking the speed row through the cycle produces three visibly distinct labels then returns to the initial 1× label (new Playwright spec). - tests/menu-and-dialog.spec.ts: ON-vs-OFF pheromone overlay PNG screenshots differ after 600 sim-seconds at 4× (new Playwright spec). Pre-fix this asserted byte-equality (the bug). - Existing pheromone-toggle test updated for the new 3-item Settings page (was 2 items). Verification ------------ - npm run typecheck: clean - npx vitest run: 2024/2024 passing (5 new) - npm run lint: matches main's pre-existing baseline - npx playwright test (menu-and-dialog + smoke): 18/18 passing (3 new including the round-trip speed cycle and pheromone visibility) - npm run build: clean Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Round-6 fix in 4cd9e0e addresses two findings from your UAT: Pheromone trails were invisible — pre-existing P1 since 9f5b23f. GameScene called I verified against the debug snapshot you captured at tick 13577 — 46 non-zero FoodTrail cells, peak value 2253 (above Fix: split the orchestrator call into Speed multiplier in Settings. Added a "Speed: N×" row to the Settings sub-screen that cycles 1→2→4→1 on click. Session-only (matches the Phase 4 fresh-boot contract). Same write path as the existing 1/2/4 keyboard shortcuts — both converge on Tests: 2024/2024 vitest, 18/18 Playwright (3 new — speed-cycle round-trip, pheromone ON/OFF pixel diff after foraging, plus an @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4cd9e0e274
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…(Codex round-6 P2) When localStorage writes are blocked (quota / private-mode), saveSettings silently drops the write. Pre-fix the keydown-P handler and the pause- menu's pheromone-toggle dispatch both did: const next = loadSettings(); next.pheromoneOverlay = !next.pheromoneOverlay; saveSettings(next); this.viewState.showPheromoneOverlay = next.pheromoneOverlay; After the first press, loadSettings returns DEFAULT_SETTINGS (= true) because the prior save never landed; `!true = false` flips viewState to OFF. On every subsequent press loadSettings STILL returns true, `!true = false` again, viewState stays OFF — the toggle gets stuck. The pause-menu's Settings page also reads the label from `loadSettings()`, so it gets frozen at "ON" while ViewState says OFF — visible desync. Fix: flip from in-memory state (`viewState.showPheromoneOverlay`), then best-effort persist. ViewState is the authoritative current value; persistence only survives reloads when storage cooperates. Same refactor applies to the menu label. PauseMenuRenderContext now carries `currentPheromoneOverlay: boolean` (sourced from ViewState by the caller) instead of a full Settings snapshot. The layout module no longer imports the Settings type at all — it's pure ctx-driven display. Tests ----- - New Playwright spec: with localStorage.setItem stubbed to throw, clicking the pause-menu's pheromone-toggle row three times produces visibly distinct labels (ON → OFF → ON) and returns to the initial state. Pre-fix every click would have ended at OFF because the menu label recomputed from loadSettings. - Pause-menu-layout tests updated to use the new ctx shape. - Pheromone overlay render Playwright spec: foraging window extended from 30s to 90s wall-clock at 4× speed. The shorter window was RNG-flaky — slow-seeded ants didn't accumulate enough FoodTrail for visible alpha within 30s. 90s = 1800 sim seconds covers the observed worst-case seed comfortably. Verification ------------ - npm run typecheck: clean - npx vitest run: 2024/2024 passing - npm run lint: matches main's pre-existing baseline (6, all preexisting) - npx playwright test (menu-and-dialog + smoke): 19/19 passing (1 new degraded-storage spec) - npm run build: clean Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Round-6 P2 fix in 6a0f85d. Both the keydown-P handler and the pause-menu's pheromone-toggle dispatch now flip from The pause-menu Settings page label also moved off New Playwright regression: with @codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
Bundles four small/medium feature requests into a single PR — three of them pivot off the new pause-menu shell, so they're naturally cohesive and share test/lint sweeps.
Per-issue commits are atomic and reviewable independently:
574df7bfeat(render): round entrance holes with dirt mound ([FEAT] Surface entrances should render as round holes with a dirt mound #117)79deeadfeat(render): pause menu overlay (Esc) ([FEAT] Pause menu UI (Esc) — settings / save+load / debug snapshot #116)673abedfeat(render): pheromone overlay toggle key (P) ([FEAT] Toggle key to show/hide pheromone trail overlay #114)ef32386feat(save,render): in-game Save / Load dialog ([FEAT] In-game Save/Load dialog (continue, manual save, delete, info) #115)Highlights
Esc replaces P-pause — Esc now opens the pause menu (which itself pauses the game). P frees up for the pheromone toggle. Single mental model: "menu = pause."
Pheromone overlay flag lives on
ViewState, hydrated from a newsubterrans:settings:v1localStorage namespace. The settings store is a separate module fromsave.tsso cosmetic preferences survivedeleteSave().Save/Load dialog respects issue #66 path —
hasIncompatibleSave()detects "save bytes present but this build can't read them" and surfaces a warning info line. Delete stays enabled so the player can clear the leftovers without losing the dialog.Mound spills 2 px onto neighbor tiles — viewport cull margin widened so the entrance reads as a 3-D bump, not a flat painted disc. Issue #14 enemy ring becomes a
strokeCirclefollowing the round silhouette.Architecture notes
pause-menu-layout.ts,save-load-dialog-layout.ts,platform/settings.ts— all Phaser-free, fully unit-tested.gameLoop.pause()from menu open through dialog flows.src/sim/changes. No determinism / replay impact.ActiveOverlayenum gains'pause-menu'and'save-load'for Playwright observability.Test plan
npm run typecheck— cleannpm run lint— matches main's pre-existing baseline (6 errors all pre-existing on main; PR introduces none)npx vitest run— 1998/1998 passing (added 45 new tests across settings.ts, pause-menu-layout, save-load-dialog-layout, save.ts, draw-surface.ts, and camera.ts)npm run build— production bundle compilesOut of scope
🤖 Generated with Claude Code