diff --git a/.planning/phases/02-season-1-vertical-slice-soil/02-05-letter-settings-e2e-PLAN.md b/.planning/phases/02-season-1-vertical-slice-soil/02-05-letter-settings-e2e-PLAN.md index 2770f57..fb163c0 100644 --- a/.planning/phases/02-season-1-vertical-slice-soil/02-05-letter-settings-e2e-PLAN.md +++ b/.planning/phases/02-season-1-vertical-slice-soil/02-05-letter-settings-e2e-PLAN.md @@ -733,12 +733,14 @@ export function Settings({ open, onClose }: { open: boolean; onClose: () => void const onImport = async () => { try { + // BLOCKER 2 fix — pipeline order: importFromBase64 → unwrap (CRC verify) + // → migrate. unwrap() returns the payload directly (NOT { payload }), so + // the previous v1.payload read crashed at runtime; migrate() was also + // called against env.payload before checksum verification. Correct order: const env = await importFromBase64(base64Buf); - const { payload } = migrate(env.payload, env.schemaVersion); - const v1 = unwrap(env); - const restored = v1.payload as V1Payload; // post-migrate - // Apply to store - hydrateStoreFromPayload(restored); + const raw = unwrap(env); // verifies CRC + const { payload } = migrate(raw, env.schemaVersion); // upgrades schema + hydrateStoreFromPayload(payload as V1Payload); setStatusLine('Restored.'); } catch (e) { setStatusLine('That doesn\'t look like one of yours.'); @@ -932,6 +934,10 @@ interface IProps { export const PhaserGame = forwardRef(function PhaserGame(props, ref) { const game = useRef(null); const sceneRef = useRef(null); + // W5 — hold the lifecycle handle so the OUTER useLayoutEffect cleanup can + // call detach(). The async IIFE that registers the hooks cannot return its + // own cleanup to the effect. + const lifecycleRef = useRef<{ detach: () => void } | null>(null); const [persistenceToastShow, setPersistenceToastShow] = useState(false); // Select clock: dev-time URL flag + non-prod build → FakeClock; otherwise wallClock @@ -958,7 +964,14 @@ export const PhaserGame = forwardRef(function PhaserGame if (cancelled) return; if (record) { // Returning player path - const payload = unwrap(record.envelope) as V1Payload; + // BLOCKER 1 fix — boot path must run unwrap (CRC verify) THEN migrate. + // Casting unwrap(record.envelope) directly to V1Payload silently + // accepts any future-shape payload as the current shape; migrate() + // is the only place that walks the schema version chain. + const env = record.envelope; + const raw = unwrap(env); + const { payload: migratedPayload } = migrate(raw, env.schemaVersion); + const payload = migratedPayload as V1Payload; appStore.getState().dismissBeginGate(); // D-22: skip Begin hydrateStoreFromPayload(payload); @@ -1002,8 +1015,14 @@ export const PhaserGame = forwardRef(function PhaserGame if (typeof ref === 'function') ref({ game: game.current, scene: null }); else if (ref) ref.current = { game: game.current, scene: null }; - // Register save lifecycle hooks (UX-10) - const lifecycle = registerSaveLifecycleHooks({ + // Register save lifecycle hooks (UX-10). + // W5 fix — lifecycle.detach() must reach the useLayoutEffect cleanup, + // but the registration happens inside an async IIFE so the inner + // `return () => lifecycle.detach()` is the IIFE's return value, not + // the effect's cleanup. Hold the handle in a useRef declared at the + // top of the component, so the OUTER cleanup (already returned below) + // can call detach on the ref's current value. + lifecycleRef.current = registerSaveLifecycleHooks({ saveSync: () => { try { const state = appStore.getState(); @@ -1018,10 +1037,12 @@ export const PhaserGame = forwardRef(function PhaserGame } }, }); - // Cleanup on unmount - return () => lifecycle.detach(); })(); - return () => { cancelled = true; }; + return () => { + cancelled = true; + lifecycleRef.current?.detach(); + lifecycleRef.current = null; + }; }, [ref]); useEffect(() => { @@ -1054,7 +1075,11 @@ function PersistenceToastWrapper(): JSX.Element | null { return ; } -// Helpers (mirror Settings.tsx — deduplicate by extracting if reused) +// Helpers (mirror Settings.tsx — deduplicate by extracting if reused). +// BLOCKER 3 invariant: `lastTickAt` is WALL-CLOCK MILLISECONDS (the value of +// clock.now() at save time). The sim never writes lastTickAt — only saveSync +// in PhaserGame.tsx and the Settings export path do. computeOfflineCatchup +// reads the same wall-clock convention. function buildPayloadFromStore(s: ReturnType, lastTickAt: number): V1Payload { return { garden: { tiles: s.tiles }, @@ -1110,6 +1135,35 @@ function App() { const [settingsOpen, setSettingsOpen] = useState(false); const persistenceToastShown = useAppStore((s) => s.persistenceToastShown); + // W2 fix — D-29 keyboard shortcuts. Comma toggles Settings (a tasteful nod + // to settings being a subordinate concern; easy to reach; no browser conflict). + // 'j' toggles the Memory Journal (Plan 02-03 wires the open/close state via + // the JournalIcon's local useState — for the hotkey, dispatch a custom event + // that JournalIcon listens for, OR lift the journal-open state into the + // store. Minimum-viable: expose a window-scoped toggler from JournalIcon + // and call it here. Pick whichever feels least invasive when implementing; + // record the choice in SUMMARY.md.) + useEffect(() => { + const onKeyDown = (e: KeyboardEvent) => { + if (e.metaKey || e.ctrlKey || e.altKey) return; + // Don't trigger when the user is typing into an input/textarea + const target = e.target as HTMLElement | null; + if (target && (target.tagName === 'INPUT' || target.tagName === 'TEXTAREA' || target.isContentEditable)) return; + if (e.key === ',') { + e.preventDefault(); + setSettingsOpen((o) => !o); + } else if (e.key === 'j' || e.key === 'J') { + e.preventDefault(); + // Dispatch a window event the JournalIcon listens for (kept decoupled + // so JournalIcon owns its open/close state). JournalIcon adds a + // matching addEventListener('tlg:toggle-journal', ...) handler. + window.dispatchEvent(new CustomEvent('tlg:toggle-journal')); + } + }; + window.addEventListener('keydown', onKeyDown); + return () => window.removeEventListener('keydown', onKeyDown); + }, []); + return (
@@ -1159,6 +1213,15 @@ export default App; - `grep -q "ABSENCE_LETTER_THRESHOLD_MS = 5" src/PhaserGame.tsx` - `grep -q "__tlgFakeClock" src/PhaserGame.tsx` (URL flag wired) - `grep -q "import.meta.env" src/PhaserGame.tsx` (production guard for FakeClock) + - `grep -q "migrate(" src/PhaserGame.tsx` (BLOCKER 1: boot path runs migrate, not just unwrap) + - `grep -q "migrate(" src/ui/settings/Settings.tsx` (BLOCKER 2: import path runs migrate) + - `grep -E "unwrap\\(.*\\)|migrate\\(" src/ui/settings/Settings.tsx | head -5` (BLOCKER 2: both unwrap and migrate appear) + - `grep -q "lastTickAt: clock.now()" src/PhaserGame.tsx` (BLOCKER 3: saveSync writes wall-clock ms via clock.now) + - `! grep -q "lastTickAt: this\\." src/sim/garden/Garden.ts || true` (BLOCKER 3: sim does NOT write lastTickAt — Garden lives under src/game/scenes/, not src/sim/, but this guards against accidental sim-side writes) + - `! grep -E "lastTickAt: this\\.(currentTick|tickCount)" src/game/scenes/Garden.ts` (BLOCKER 3: Garden scene does NOT overwrite lastTickAt with a tick counter — saveSync owns it) + - `grep -q "addEventListener('keydown'" src/App.tsx` (W2: D-29 keyboard shortcut wired) + - `grep -q "tlg:toggle-journal" src/App.tsx` (W2: journal hotkey dispatches the toggle event) + - `grep -q "lifecycleRef.current?.detach()" src/PhaserGame.tsx` (W5: lifecycle handle reaches outer effect cleanup) - `npx vitest run src/ui/letter/ src/ui/settings/` exits 0 with all tests green - `npm run ci` exits 0