From 1eaa9e73f7014cd1467354f854cb4c23e7f0d6db Mon Sep 17 00:00:00 2001 From: tiennm99 Date: Sun, 26 Apr 2026 08:41:43 +0700 Subject: [PATCH] chore(plans): remove superseded reports All three reports are historical artifacts: - v2 brainstorm: shipped (commit 149e07a, journal in docs/journals/) - 260421 level-redesign brainstorm: fully superseded by v2 - 260420 full-project review: pre-v2; findings addressed by v2 Preserved in git history. Removing from working tree. --- ...-260421-2042-level-redesign-solvability.md | 145 --------------- ...rmer-260425-1907-tight-12-design-uplift.md | 166 ------------------ ...eviewer-260420-2203-full-project-review.md | 132 -------------- 3 files changed, 443 deletions(-) delete mode 100644 plans/reports/brainstorm-260421-2042-level-redesign-solvability.md delete mode 100644 plans/reports/brainstormer-260425-1907-tight-12-design-uplift.md delete mode 100644 plans/reports/code-reviewer-260420-2203-full-project-review.md diff --git a/plans/reports/brainstorm-260421-2042-level-redesign-solvability.md b/plans/reports/brainstorm-260421-2042-level-redesign-solvability.md deleted file mode 100644 index fc0c385..0000000 --- a/plans/reports/brainstorm-260421-2042-level-redesign-solvability.md +++ /dev/null @@ -1,145 +0,0 @@ -# Level Redesign & Solvability — Brainstorm Summary - -**Date:** 2026-04-21 -**Scope:** Redesign all 12 levels of Night Ninja: Twilight Voyage. Fix L2 unsolvability. Expand grid sizes. Add more guards. Preserve L12 as intentionally unsolvable. -**Status:** Design approved — ready for implementation plan. - ---- - -## 1. Problem Statement - -- **L2 unsolvable** — BFS from `(0,0)` cannot reach `(5,5)`. Right-side component `{(3,5),(4,4),(4,5),(5,3),(5,4),(5,5)}` is disconnected by permanent light walls from static guards at `(2,4)` and `(4,2)`. -- **No solvability gate** — levels authored manually; no automated proof. Risk of regressions on future edits. -- **Levels feel narrow** — current grids 6x6–9x9 have single corridors; user wants wider, more complex puzzles. -- **Progression imbalanced** — mirror/chaser/patrolling introduced too late (Acts 4–5); early acts feel sparse. - -## 2. Hard Rules (invariants) - -1. **Exactly 12 levels, 6 acts.** No adds, removes, renumbers. -2. **L12 must remain unsolvable by normal play.** Only console easter-egg win (user teleports player to goal via dev tools). Win condition / congratulations screen still fires when goal reached by any means. -3. **Desktop-first.** No mobile responsive work. Large grids handled via scrollable viewport that follows the player. -4. **Preserve** `id`, `name`, `storyKey`, `isFinalLevel` per level — zero progress-persistence migration. - -## 3. Solution — Redesign Philosophy - -### 3.1 Progression (rebalanced acts) - -| Act | Levels | Grid | Guard types (cumulative) | Guard count | -|-----|--------|------|--------------------------|-------------| -| 1 | 1–2 | 8x8 | static | 0 / 3 | -| 2 | 3–4 | 9x9 | + rotating + **blinking** (promoted from Act 3) | 4 / 5 | -| 3 | 5–6 | 10x10 | + **patrolling** (promoted from Act 4) | 6 / 7 | -| 4 | 7–8 | 11x11 | + **mirror** (promoted from Act 5) | 7 / 8 | -| 5 | 9–10 | 12x12 | + chaser | 8 / 9 | -| 6 | 11 | 12x12 | all types combined | 9 | -| 6 | 12 | 13x13 | all types + expanding-wave mechanic | 10 (unsolvable) | - -### 3.2 Design Rules Per Level - -- **Connectivity gate:** at least one reachable path from start→goal in solver (except L12). -- **Widen, don't narrow:** add 2–3 alternative routes so guards force *decisions*, not single chokepoints. -- **More guards ≠ more walls.** Keep wall density ~20–25% of grid area; increase guard count per act tier. -- **Par = solver_optimal + 2** (small slack for undo experimentation). -- **No redundant wall-on-light** (L2 had wall `(3,4)` over guard light `(3,4)` — sloppy overlap, remove). -- **Guard beam pruning:** rotating beams stop at walls — use walls strategically to create safe windows. - -## 4. Solvability Solver (automated validation) - -### 4.1 State & Search - -**State key:** `(player_row, player_col, chaser_positions_tuple, turn_mod_cycle)` -- `cycle = LCM(4 [rotating], 2 [blinking], patrol_path_length_i...)`. Typical cycles 4–24. -- **Chaser** is stateful — include each chaser's position in state key. State space stays tractable (<10M for 12x12 + 2 chasers + cycle 24). -- **Mirror beams** are pure function of rotating-guard directions + mirror positions each turn — not part of state. - -**Transitions:** 5 actions per turn — `up / down / left / right / wait`. -**Pruning:** after applying action + running guard update for next turn, if player cell `isLight` → dead branch. -**Algorithm:** BFS (guarantees shortest). Bail out at state-count cap (~10M) with "unsolvable within budget" → manual check. - -### 4.2 Location & Modules - -- `src/lib/game/level-solver.js` — pure BFS solver, reuses existing `grid-system.js`, `guards.js`, `turn-manager.js`. -- `src/lib/levels/levels.solvability.test.js` — Vitest suite asserting: - - For L1–L11: solver returns a valid path. - - For L12: solver returns no path within budget (unsolvable invariant). - - Par values match `solver_optimal + 2`. -- CI hook: run solvability suite on every PR that touches `levels.js`. - -### 4.3 Known Limitations - -- Chaser path depends on guard AI implementation — solver must use *identical* chaser logic as runtime (import from `guards.js`, don't reimplement). -- Cycle computation must account for patrol path length (may be odd / prime). -- L12's expanding-wave mechanic needs the solver to model princess-mechanic.js state — include `waveRadius` in state key. - -## 5. Scrollable Viewport (prereq for large grids) - -- Current rendering assumes full grid fits on screen. -- Need: camera-follow the player so 12x12 / 13x13 grids are playable on 1080p screens. -- Implementation touch points: - - `src/components/GameBoard.svelte` (or equivalent) — CSS `overflow: auto` + `scroll-behavior: smooth` + programmatic `scrollIntoView` on player position change. - - Viewport: show ~9x9 cells visible; pan on player move. -- Out-of-scope for this redesign brainstorm but **blocker** for Acts 4–6 playability. - -## 6. L12 Easter Egg — Console Win - -- Expose a minimal dev hook on `window` (e.g., `window.__nntvDev = { teleportToGoal() { ... } }`) OR rely on existing game-state reactivity so user can directly mutate player position via devtools. -- Reaching goal by any means → normal win flow (congratulations, progress saved). -- Document the easter egg ONLY in code comments, not in-game UI or README. - -## 7. Migration & Rollout - -- **In-place rewrite** of `src/lib/levels/levels.js`. No new file. -- Preserve each level's `id`, `name`, `storyKey`, `isFinalLevel`, array position. No changes to `src/lib/localization/*` story keys needed. -- Update `parMoves` from solver output for all levels. -- Tests first: land solver + solvability suite **before** level content changes, so each new level design is validated as it's written. - -## 8. Evaluated Alternatives - -| Approach | Pros | Cons | Verdict | -|----------|------|------|---------| -| Fix L2 only, leave others | Minimal churn | Doesn't address "narrow levels" complaint; no regression protection | **Rejected** | -| Keep grids, add guards only | No viewport work | Crowded; still narrow corridors; low decision value | **Rejected** | -| Aggressive grid expansion + more guards + solver | Solves all issues; prevents future regressions | Requires viewport work; solver effort | **Chosen** | -| Expand to 15–18 levels | Richer progression | Breaks progress persistence, story keys, count invariant | **Rejected** (violates hard rule) | - -## 9. Success Criteria - -- [ ] Vitest suite: 11/11 levels solvable, L12 unsolvable. Passes on CI. -- [ ] Every level has ≥2 distinct BFS-discovered paths OR clear single-path justification in code comment. -- [ ] All acts visually wider than before (row/col counts match Section 3.1). -- [ ] Scrollable viewport works on 1080p / 1440p screens for 12x12 grids. -- [ ] Win condition fires when L12 goal is reached via console manipulation. -- [ ] No changes to story keys, level IDs, or progress-persistence schema. - -## 10. Next Steps (Implementation Plan Phases) - -1. **Phase 1:** Implement scrollable viewport (prereq for >9x9 grids). -2. **Phase 2:** Build `level-solver.js` + unit tests on current L1, L3–L11 (should solve) and L12 (should fail). Accept that L2 will fail — that's the baseline bug. -3. **Phase 3:** Solvability test suite — wire into CI. -4. **Phase 4:** Redesign L1–L2 (Act 1, 8x8). Run solver, lock par values. -5. **Phase 5:** Redesign L3–L4 (Act 2, 9x9, introduce blinking earlier). -6. **Phase 6:** Redesign L5–L6 (Act 3, 10x10, introduce patrolling). -7. **Phase 7:** Redesign L7–L8 (Act 4, 11x11, introduce mirror). -8. **Phase 8:** Redesign L9–L10 (Act 5, 12x12, introduce chaser). -9. **Phase 9:** Redesign L11 (Act 6, 12x12, all guards combined). -10. **Phase 10:** Redesign L12 (Act 6, 13x13, preserve unsolvable invariant; expose console win hook). -11. **Phase 11:** Update `docs/` (code-standards, system-architecture) with solver + easter-egg notes. Smoke-test full playthrough. - -## 11. Risks - -| Risk | Mitigation | -|------|------------| -| Solver too slow on chaser+long-cycle levels | State cap 10M; if exceeded, reduce cycle via shorter patrol paths | -| Viewport work reveals rendering bugs | Isolate in Phase 1; land before level changes | -| Solver disagrees with runtime (chaser logic) | Import guard logic from `guards.js` — single source of truth | -| L12 accidentally solvable after redesign | Solver test asserts unsolvability; fails CI | -| Parsing console easter egg as "cheating" concern | Document in code comments only; feature is intentional | - ---- - -## Unresolved Questions - -1. **Console easter-egg API shape** — expose `window.__nntvDev.teleport()` explicitly, or rely on Svelte 5 reactive store mutation? Simplest path TBD in Phase 10. -2. **Viewport implementation owner** — is scrollable viewport a separate brainstorm/plan, or bundled into this redesign's Phase 1? -3. **Story text updates** — do any `storyKey` copy lines need rewriting to match new grid sizes/themes (e.g., "narrow corridor" lines on now-wider levels)? Decision deferred to QA after redesigns land. -4. **Chaser count in Act 5+** — cap at 1 per level, or allow 2 in L10/L11? Affects solver state-space budget. diff --git a/plans/reports/brainstormer-260425-1907-tight-12-design-uplift.md b/plans/reports/brainstormer-260425-1907-tight-12-design-uplift.md deleted file mode 100644 index 82ccf54..0000000 --- a/plans/reports/brainstormer-260425-1907-tight-12-design-uplift.md +++ /dev/null @@ -1,166 +0,0 @@ ---- -type: brainstorm -date: 2026-04-25 -slug: tight-12-design-uplift -status: approved -project: nntv ---- - -# NNTV v2 — "Tight 12" Design Uplift - -Brainstorm summary. Approach **A — Tight 12**: more design density in the same 12-level shape via curated mechanic additions + full overhaul of L1–L11. - -## Problem Statement - -Current build: 12 levels, 6 guard types, lives system, undo/preview, BFS-verified. User wants harder design without ballooning level count or dependencies. - -Brief (resolved): -- Target: all gameplay difficulty (broad) — mechanics + levels + systemic -- Scope: replace + extend -- Constraints: L12 stays unsolvable · BFS solver verifies all solvable · no new deps · ~12 levels exactly - -## Resolved Tensions (Brutal-Honesty Pass) - -| Original signal | Tension | Resolution | -|---|---|---| -| All 4 depth categories | Buffet → kitchen-sink levels | Cut to 6 mechanics across 4 categories | -| Player abilities (stones/dash/freeze) | BFS state explosion | Stones only; per-level cap on count | -| Rogue-lite single life | 10+ min replays = masocore | Softened to level-restart on detection (genre standard) | -| Full overhaul + many mechanics | Intro-budget exhausted | One mechanic per level intro slot, L10–L11 = pure compounding | -| Drop undo+preview | Removes information needed for harder puzzles | Per-level affordance gates; only L9–L11 strip them | - -## Approaches Evaluated - -| | Pros | Cons | -|---|---|---| -| **A — Tight 12** ✅ | Tractable, solver-safe, ships ~2-3 wks, design-legible | Drops 60% of buffet picks | -| B — Layered Hard | Lowest frustration risk, escape valve | Dilutes "make it harder" mandate | -| C — Maximalist v2 | Literally maximal ceiling | ~6 wks, kitchen-sink risk, solver-replacement risk, niche audience | - -**Chosen: A.** Reasoning: depth density is a curation problem, not addition. C burns weeks on solver engineering invisible to player. - -## Final Spec - -### Mechanic Palette (6 additions) - -| Mechanic | Category | Rule | -|---|---|---| -| Sniper guard | Guard | LoS beam from facing dir; stops at first wall/mirror; lethal. Rotates 90° **every 2 turns** | -| Suspicion guard | Guard | 3-tier meter (idle→alerted→firing). Firing-tier lights surrounding cells 1 turn | -| Throwable stone | Player verb | **Variable per-level count**. Target ≤3 Manhattan. All rotating/patrolling/chaser within 2 of target face it 1 turn | -| Door + Key | Environment | Locked door = wall; key collect → permanent open. Bitmask state | -| One-way tile | Environment | Arrow tile, entered only from designated dir. Zero state | -| Light decay | Environment | Lit cell stays "warm" (visible warning, passable) 1 turn after light leaves | - -### Systems Changes -- Lives counter: removed. Detection → restart current level. -- Affordance gates: per-level `{ allowUndo, allowPreview }`. L1–L8 both on, L9–L10 preview-only, L11 both off. -- Stones refresh on level-start (per-level resource). -- Turn order: **stone resolves before guard turn** → guards-react-to-stone → light recalc → detection. - -### Level Plan - -| L | Name | New mechanic | Reuses | Undo | Preview | -|---|---|---|---|---|---| -| 1 | Garden Path | — | movement | ✓ | ✓ | -| 2 | Watchtower | — | static | ✓ | ✓ | -| 3 | Vegetable Patrol | one-way | static, one-way | ✓ | ✓ | -| 4 | Searchlight | rotating + suspicion | static, rotating | ✓ | ✓ | -| 5 | Fortress Gate | doors + keys | suspicion, rotating | ✓ | ✓ | -| 6 | Flickering Corridor | blinking + decay | blinking, decay | ✓ | ✓ | -| 7 | Underground Passage | mirror | rotating, mirror, doors | ✓ | ✓ | -| 8 | The Gauntlet | patrolling + sniper | static, rotating, mirror | ✓ | ✓ | -| 9 | Decoy Path | throwable stones | patrolling, sniper, suspicion | ✗ | ✓ | -| 10 | Hall of Mirrors | combo | mirror, sniper, decay, stones | ✗ | ✓ | -| 11 | Throne Room | chaser + full palette | all | ✗ | ✗ | -| 12 | Princess Chamber | (unsolvable) | princess emanation | n/a | n/a | - -6 mechanic intros across L3–L9. L10–L11 = compounding-only. - -### Engine Architecture Deltas - -| File | Change | -|---|---| -| `src/lib/game/guards.js` | + `SniperGuard`, `SuspicionGuard` classes | -| `src/lib/game/throwable.js` (new) | Stone throw resolver, distraction queue | -| `src/lib/game/grid-system.js` | + cell types `door`, `key`, `oneWay`, `warm` | -| `src/lib/game/level-manager.js` | Parse `doors[]`, `keys[]`, `oneWays[]`, `stones`, `affordances` | -| `src/lib/game/level-solver.js` | State += `(keys_bitmask, stones_left, suspicion[], warm_timers)`; canonicalize hash; per-level node cap | -| `src/lib/game/turn-manager.js` | Throw action ordering; suspicion meter ticks | -| `src/lib/levels/levels.js` | All 11 solvable levels rewritten | -| `src/components/` | StonesCounter, KeyInventory, SuspicionRing overlay, AffordanceBanner | -| `src/lib/i18n/` (or equiv) | EN/VI strings for new mechanics | - -### Data Model Extension - -```js -{ - name, width, height, start, goal, - walls: [...], - guards: [...], - doors: [{ pos, keyId }], - keys: [{ pos, keyId }], - oneWays: [{ pos, dir }], - decayTiles: [pos] | "all", - stones: 0, // per-level budget - affordances: { undo: true, preview: true } -} -``` - -### Solver Scaling Plan -- State canonicalization with new fields hashed. -- Suspicion 0–2, decay 0–1 → bounded per-cell additive blow-up. -- `MAX_BFS_NODES = 2_000_000` per level. CI fails loud → redesign offending level, never relax cap. -- Stone branching mitigated by ≤3 Manhattan target rule + small per-level count. - -### Sacred Constraints (verified honored) -- L12 unsolvable narrative: unchanged. -- BFS CI gate: kept; extended to new state. -- No new deps: pure JS additions only. -- 12 levels exactly. -- EN/VI bilingual + ARIA: preserved on new tiles. - -### Risk Register - -| Risk | Severity | Mitigation | -|---|---|---| -| BFS state explosion | High | Per-level node cap; redesign-not-relax policy | -| Affordance gates confuse players | Med | Pre-level banner; theming aligned w/ act tone | -| Save data breaks (lives→no-lives, new structures) | Med | One-shot progress reset on v2 launch + migration notice | -| Stones make some puzzles single-trick | Med | Multi-stone levels designed around branching distractions; ≥2 valid stone targets where used | -| Soft-rogue (vs original "single life") underdelivers brief | Low | User downgraded in Q3; documented | -| Sniper-every-2-turns trivializes encounters | Low | Compose with rotating/patrolling for overlapping-window puzzles | - -### Out of Scope (Cut) - -Sound guard · dash · freeze-turn · fog-of-war · breakable walls · decoy guards · star scoring · par-move · run-life ironman · A* solver replacement. - -### Success Criteria -- All 11 solvable levels BFS-verified in CI under node cap. -- Each of 6 new mechanics has a dedicated intro level. -- L11 solve attempts on first playthrough: target 5–15. -- v2 run completion 30–50% (vs ~70% current — target zone for "harder"). -- Zero new dependencies in `package.json`. - -### Phase Outline (rough) - -1. Engine: new guards + throwable + door/key/one-way/decay tiles + unit tests -2. Solver: extended state, canonicalization, per-level cap + tests -3. Level redesign: 11 new level definitions + BFS CI green -4. UI: stones counter, suspicion overlay, key inventory, affordance banner -5. i18n: EN/VI strings for new mechanics -6. Polish: pixel art for new entities, audio cues, intro-text per level - -## Unresolved Questions - -1. Cosmetic life counter — strip entirely or replace with "attempts-on-this-level" counter? (defaulting: strip; show only level-restart number) -2. Mirror behavior with sniper beam — also reflects 90°? (defaulting: yes, consistency w/ rotating beam) -3. Suspicion meter — does it decay when player breaks LoS, or only after N idle turns? (recommend: decay 1/turn when player out of range) -4. Stone-throw fizzle — what if no eligible guards within 2 of target? (recommend: stone still consumed; player loses one) -5. Save migration UX — silent reset or modal acknowledgment? (recommend: modal once) -6. Audio for new entities — procedural Web Audio (existing pattern) or punt to art-pass phase? - ---- - -**Status:** DONE -**Summary:** Converged on Approach A — Tight 12. Six curated mechanics (sniper, suspicion, stones, doors+keys, one-way, light-decay), full L1–L11 overhaul, level-restart death model, per-level affordance gates, BFS solver extended with per-level node cap. L12 unsolvable preserved. ~2-3 weeks scope. diff --git a/plans/reports/code-reviewer-260420-2203-full-project-review.md b/plans/reports/code-reviewer-260420-2203-full-project-review.md deleted file mode 100644 index af7a59b..0000000 --- a/plans/reports/code-reviewer-260420-2203-full-project-review.md +++ /dev/null @@ -1,132 +0,0 @@ -# Code Review: Night Ninja: Twilight Voyage (full project) - -## Summary - -Small, well-scoped Svelte 5 + Vite 6 stealth puzzle (~4k LOC). Pure-JS game engine in `lib/game/` is cleanly separated from Svelte rendering. Recent pixel-art integration (commit 9eced8a) introduces `lib/pixel/` with a simple string→SVG renderer; it works but adds ~94KB of inlined art data. Build succeeds with two `state_referenced_locally` warnings (silenced). Docs drifted after pixel integration and still describe the pre-pixel codebase. Zero automated tests; several pure-JS modules are cheaply testable and worth covering. No critical security/correctness bugs; a handful of real but low-impact issues around stale state on bad level ids, undo/redo discarding princess state partially, and preview-simulation side effects on real guard state. - -## Strengths - -- Clean engine/render separation; `renderVersion` pattern is documented and consistently applied. -- All user-facing strings localized via `getText`; XSS-safe by default (Svelte interpolation, no `{@html}`). -- `try/catch` around all `localStorage` reads/writes (`progress.js`, `localization.js`). -- Pure-JS classes (`GridSystem`, `Player`, `TurnManager`, `GameHistory`, `TouchControls`, guard hierarchy) have tight, single-responsibility APIs. -- GUARD_REGISTRY factory pattern in `level-manager.js` removes switch boilerplate. -- File sizes largely under 200 lines; only `Game.svelte` (396), `guards.js` (347), `levels.js` (524, data), and `art-characters.js` (330, data) exceed — data files are acceptable. - -## Issues by severity - -### Critical -None. - -### High - -- **`src/scenes/Game.svelte:74`** — If `loadLevel(currentLevel)` returns `null` (invalid id), `initLevel` returns early leaving `grid`, `player`, `guards` from the previous level (or `null`, triggering crashes when handlers run). No user-facing guard. - Fix: when `state` is null, navigate to `MainMenu` or clamp to 1. - -- **`src/lib/game/turn-manager.js:31-71` (`previewNextTurn`)** — "Preview" mutates real guard state, collects lit cells, then restores from snapshot. If any guard subclass gains state fields not in the snapshot list (e.g., future `cooldown`, `lastSeen`), the preview will silently corrupt the real game. Also `grid.clearAllLight()` + `updateLight(guards)` at the end re-runs mirror reflection but the beam order now differs from a fresh turn-start state. - Fix: deep-clone guards into sandbox copies, run on the sandbox, don't mutate live instances. Or add explicit "all snapshotted fields" contract per guard type. - -- **`src/scenes/Game.svelte:186-208` (`handleUndo`/`handleRedo`)** — After restoring player/guards, the code calls `grid.clearAllLight()` then `g.updateLight(guards)` for every guard. But guards with direction-dependent light (rotating, patrolling, chaser) need their light recomputed based on direction — which IS restored correctly — so this generally works. BUT `princess.alerted` is restored from `state.princessAlerted`, which was **never captured** at push time for move snapshots: `createSnapshot` at line 100-101 passes `princess.alerted, princess.alertRadius`, but the snapshot is created BEFORE the move's effects — so the restored princess state matches the moment before the move, which is correct. Exception: on final level, if princess was alerted BEFORE the move, undoing restores pre-move alerted=true/radius=N; but the `lightRing` is recomputed with that radius. Fine, but `princess.messageShown` is never snapshotted/restored → first re-alert after an undo back across the alert boundary will skip `showMessage: true` because `alerted` is already true. Minor UX bug. - Fix: snapshot `messageShown`, or tie message display strictly to `!prevAlerted && nowAlerted`. - -- **`src/scenes/Game.svelte:74` / `src/lib/game/turn-manager.js:14`** — `nextTurn` checks `isGoal` BEFORE guard update, so reaching goal never triggers the final guard turn. That's by design, but combined with the preview path (which runs real guard turn), the preview and the actual "step onto goal" disagree. Not wrong, just subtle — confirm this is intended behavior. - -- **Build warnings `state_referenced_locally` (`Game.svelte:33, 34`)** — Silenced with `svelte-ignore`, but the underlying pattern (`$state(level)` reading the prop's initial value only) means if the parent remounts Game with new props without scene-key change (unlikely given `{#key currentScene}` pattern), the state would desync. With the current navigate flow each Game instance is a fresh mount, so this is safe. Prefer `$derived(level)` if the goal is "latest prop", or a clarifying comment noting "value is only read on mount; parent remounts scene via {#key}". - -### Medium - -- **`src/lib/game/turn-manager.js:15-16`** — When `player` is at goal, `nextTurn` returns early without clearing lights. Next turn's `grid.clearAllLight()` will run, but intermediate renders show stale lights. Low impact, but combined with the LevelComplete popup it usually isn't visible. - -- **`src/lib/game/princess-mechanic.js:24-30`** — `update` increments `alertRadius` on every call once alerted, even if `update` is called twice in one turn. `Game.svelte` only calls it via `checkFinalLevel` per turn, so fine today, but class is not idempotent. - -- **`src/lib/game/game-history.js:32-47`** — `snapshotGuard` stores `currentPathIndex/isReversing` only for `patrolling`, `isChasing/isReturning/target*` only for `chaser`. If a rotating guard were ever given a state field, snapshot would silently drop it. Fragile. Use `Object.assign({}, g)` filtered to primitives, or have each guard class expose `snapshot()/restore(s)`. - -- **`src/scenes/LevelSelect.svelte:7`** — `const totalLevels = 12;` hardcoded. `getTotalLevels()` already exists. Source of future drift when adding levels. - Fix: `import { getTotalLevels } from '../lib/game/level-manager.js';` and call. - -- **`src/scenes/Game.svelte:59`** — `cellSize = $derived(grid ? Math.min(50, Math.floor(500 / grid.rows)) : 50);` — fixed 500px divisor ignores viewport width and `grid.cols`. On narrow mobile or non-square grids (levels use up to 10×10 but most are ≤7×7), the board can overflow horizontally. No viewport query. - Fix: compute from `Math.min(availableW / cols, availableH / rows)` or read container size. - -- **`src/components/GameBoard.svelte:41` + inline Svelte `{@const}`** — Two `previewCells.has(...)` calls per cell (`class:preview` and `{#if}`). Extract once via `{@const}`. Micro, but cleanup. - -- **`src/scenes/Settings.svelte:7-14`** — `tick++` + `{#key tick}` used to force re-render on language change. With Svelte 5 runes, `getText` is not reactive because translations are pulled from a module-level `currentLanguage` variable. This works by mounting the entire subtree — acceptable but note that other scenes don't pick up the change until remount (MainMenu does via App's `{#key currentScene}`, same for Game). If user flips language mid-game and resumes, loaded story text stays until navigation. Low priority. - -- **`src/lib/audio.js:17`** — `audioCtx.resume()` is unawaited; returns a Promise. Chrome/Safari fine in practice, but on some mobile browsers first few tones may be silent. - Fix: `await` inside an async wrapper, or schedule notes after resume resolves. - -- **`src/lib/pixel/Pixel.svelte:4`** — `scale = 6` prop is unused when `width`/`height` are provided (always, in current callers). Dead default. - -- **`src/lib/pixel/art-scenes.js:5`** — `norm` helper pads with `r.at(-1)` (last char of prior row) if row too short. Silent padding with an unrelated palette char can produce artifacts if an author miscounts. Assert or pad with `.` (transparent). - -### Low - -- **File naming inconsistency** — `components/` mixes PascalCase (`Button.svelte`, `GameBoard.svelte`) with kebab-case (`controls-overlay.svelte`). `docs/code-standards.md` says PascalCase for components. Either rename `controls-overlay.svelte` → `ControlsOverlay.svelte` (already imported as `ControlsOverlay`) or update the doc convention. - -- **`src/scenes/Game.svelte:53-54`** — Comment "class instances are not proxied" accurate; good. Consider extracting the `renderVersion` + `$derived` pattern into a helper once more modules need it (YAGNI for now). - -- **`src/lib/game/guards.js:177-213` (`bfsNextStep`)** — Standard BFS, correct. `queue.shift()` is O(n); for 10×10 grids (<100 cells) fine. - -- **`src/lib/localization.js:9`** — `currentLanguage = 'vi'` hardcoded default; docstring says same. If requirements change to EN default, two places to update. Minor. - -- **`src/scenes/StoryIntro.svelte:9-11`** — `document.querySelector('.story-scroll')` from `onMount`. Works because CSS animation uses `animationend`. Fragile if selector collides; use a `bind:this` ref. - -- **`src/scenes/Game.svelte:180`** — `setTimeout(..., 400)` without ref; if user navigates mid-timeout the component is destroyed and the callback writes to dead state. Svelte 5 is tolerant but use `onMount` cleanup or `$effect` with cleanup. - -- **`src/components/GameHud.svelte:8`** — `lives` default `3`, but `MAX_LIVES` constant is also `3`. If `lives` prop ever exceeds `MAX_LIVES`, hearts are silently truncated. Cosmetic. - -- **Perf (pixel backdrop)** — `LevelIntro` and `Game` each render a `Pixel` backdrop via SVG. After run-length compression ≤1-2KB of rects. Re-renders only on level change (due to `$derived` on `currentLevel`). No action needed. - -- **Bundle size** — 129KB JS (40KB gzip) is fine for a small game. Art data (~8KB compressed) inlined in the JS chunk. Not worth code-splitting given total size. - -## Docs drift - -All three docs pre-date the pixel integration (commit 9eced8a). Required updates: - -- **`docs/codebase-summary.md`** - - Line 28-38 Components table: missing `LevelCompletePopup.svelte`, `controls-overlay.svelte`; `GuardSprite.svelte` described as "Colored circle" (now a Pixel sprite). - - Missing the `src/lib/pixel/` subsystem entirely (5 files, 773 LOC). - - File Dependency Map (line 171) doesn't mention `Pixel`, `art-*`, `palette`. - - Stats (line 193): "Total Source Files ~25" — actually ~35 now. - - "No sprites/images" (line 248 of architecture) is now wrong; pixel-art sprites are present but inlined as JS strings. - -- **`docs/system-architecture.md`** - - Line 83-90 "Svelte Components" still says "PlayerSprite — positioned div", "GuardSprite — colored circle/diamond". Both now wrap ``. - - Line 246-253 "Asset & Resource Management — No sprites/images: Pure CSS rendering". Outdated. - - Missing section on pixel rendering pipeline (string-art + palette + SVG rect merge). - -- **`docs/code-standards.md`** - - Line 41-48 Components table missing `LevelCompletePopup.svelte`, `controls-overlay.svelte`. - - Directory Structure (line 26-73) missing `src/lib/pixel/` entirely. - - File-naming rule says "PascalCase" for components but `controls-overlay.svelte` exists. Resolve one way. - - "No sprites/images" implication in theme section still holds but worth clarifying the pixel pipeline's relationship to theme.css guard colors (duplicated as `NNTV.guard*` constants — document the coupling). - -- **`README.md`** Project Structure table (line 79-91) — does not mention `src/lib/pixel/`. Minor. - -## Test coverage gap - -No tests, no framework installed. Priority targets (pure JS, deterministic, cheap): - -1. **`src/lib/game/grid-system.js`** — bounds, setters, `getAllCells`. Smoke-level only. -2. **`src/lib/game/player.js`** — `move` direction mapping, wall/bounds rejection. -3. **`src/lib/game/turn-manager.js`** — especially `previewNextTurn` (state restoration is the highest-risk code in the engine). Covers the Medium/High fragility noted above. -4. **`src/lib/game/game-history.js`** — undo/redo round-trip for each guard type. Would catch the missing `messageShown` snapshot. -5. **`src/lib/game/guards.js`** — `RotatingGuard.castBeam` with mirror bounce (depth cap, boundary, wall), `ChaserGuard.bfsNextStep` on wall mazes, `PatrollingGuard` circular vs reversing path. Most logic-heavy module, highest defect risk. -6. **`src/lib/game/princess-mechanic.js`** — ring expansion, detection at each radius. -7. **`src/lib/progress.js`** — star calculation, localStorage serialization survives corruption. - -Svelte component tests (via `@testing-library/svelte` or `vitest` + `jsdom`) are worth deferring — the components are thin and mostly props-through. - -Recommended: add `vitest` as a dev dep, create `src/lib/game/*.test.js` for items 1-7. Target: 60-70% statement coverage of `lib/game/`. Skip UI/scene tests initially. - -## Unresolved questions - -- Is `previewNextTurn` mutating real guards intentional (for perf), or a candidate for immediate refactor to sandbox copies? -- Is `currentLanguage = 'vi'` the permanent default, or should it detect `navigator.language`? -- Is `isFinalLevel` behavior of skipping guard detection after princess detect intended? (Currently both code paths can fire in sequence — guard detect wins the popup order.) -- Should `LevelSelect`'s hardcoded `totalLevels = 12` become reactive to `getTotalLevels()` now, anticipating level additions? - ---- - -**Status:** DONE_WITH_CONCERNS -**Summary:** Project is in good shape architecturally; pixel integration is clean but docs weren't updated to match. One high-impact correctness risk in `previewNextTurn` state mutation, one High issue with `initLevel` silent-fail on invalid level, and undo state incompleteness for princess `messageShown`. Docs and tests are the primary gaps. -**Concerns/Blockers:** None blocking; recommend addressing the High items before the next feature phase.