From be76c5722cdca0f376da50bf0af113a674cab35d Mon Sep 17 00:00:00 2001 From: type-two Date: Sun, 9 Aug 2026 17:49:04 +1000 Subject: [PATCH] Two critical lifecycle bugs the fresh-eyes review caught, both live MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - IncidentReportScene never stopped itself: the paperwork rendered on top of the roster after the FIRST night of every run. Looked like a freeze. - HaulDeskScene never reset per-night state: from night two the back office was a blank $0 desk that ate every input. Mine, from SOLO-26 — adding scene.stop() is exactly what makes Phaser hand back a dirty instance next time. - Dev routes (n/p/f/j/m) shipped unguarded on window; a stray F binned the shift. Now Ctrl+Shift in built games, and R routes home to the roster. - tests/sceneLifecycle.test.ts guards the class, verified to fail on the real bug. Co-Authored-By: Claude Fable 5 --- LANEHANDOVER.md | 55 +++++++++++++++++ src/audio/Sfx.ts | 62 ++++++++++++++++++- src/main.ts | 19 +++++- src/scenes/door/IncidentReportScene.ts | 10 ++- src/scenes/door/NightScene.ts | 1 + src/scenes/shared/HaulDeskScene.ts | 22 +++++++ tests/sceneLifecycle.test.ts | 84 ++++++++++++++++++++++++++ 7 files changed, 249 insertions(+), 4 deletions(-) create mode 100644 tests/sceneLifecycle.test.ts diff --git a/LANEHANDOVER.md b/LANEHANDOVER.md index 80f6b08..cd9bd16 100644 --- a/LANEHANDOVER.md +++ b/LANEHANDOVER.md @@ -2527,3 +2527,58 @@ fluorescent tube — light type on the brightest thing in the image. The board moved down to y=194 (258 tall) and the heading moved off it onto the dark wall at y=16/38. Worth remembering: adding a background can break foreground text that was fine over a flat colour. + +## SESSION — FABLE-SOLO-30 · 2026-08-09 + +**Branch:** main (solo, Fable) · **Gate:** lint ✓ build ✓ test ✓ (843 tests, 58 files) + +### A fresh-eyes review (6 independent lenses, adversarially verified) found +### TWO CRITICAL BUGS THAT WERE LIVE. Both the same class. Both fixed. + +**1. `IncidentReportScene` never stopped itself.** Its exit handler was +`const go = () => this.input$.next()`, and `next` is always a closure calling +`scene.start('Roster')` from NightSummary. Phaser's `ScenePlugin.start` only +queues a stop for its OWN key — so the paperwork kept rendering its opaque +full-screen panel ON TOP of the roster board. **To a player: after the very +first night of every run, the game appears to freeze on the paperwork.** + +**2. `HaulDeskScene` never reset its per-night state.** `finished`, `idx`, +`counted`, `phase`, `phaseMs`, `phaseFor`, `falling` were class-field +initialisers; Phaser reuses one instance for the life of the game. From the +second surviving night, `finished` was still true: `update()` bailed, the +ceremony never played, and `finish()` returned before registering its exit +handlers. **A blank $0 desk that ate every click, key and ESC.** + +The reviewer caught the irony exactly: SOLO-26 correctly added `scene.stop()` +here, and stopping is precisely what guarantees Phaser hands back a dirty +instance. **Stopping and resetting are two halves of one job.** + +### `tests/sceneLifecycle.test.ts` — because this class has now bitten FOUR times +(busy latch, seenCards leak, and these two.) Source-level assertions: every +per-night field must be reset in create(), and any scene that hands control to +a scene it does not own must stop itself. Uses Vite's `import.meta.glob(?raw)` +rather than node:fs — this suite runs under the app tsconfig, which has no node +types. **Verified the guard actually fails on the real bug** by reverting the +IncidentReport fix and watching it go red. + +### 3. Dev routes were shipping to production +`src/main.ts` had a bare `window` keydown for n/p/f/j/m — no modifier, no dev +guard, unconsumable by any scene. `f` is the universal fullscreen reflex and it +stopped Night (tearing down Door and Floor); the only saveGame runs at night +end, so a stray F silently binned up to 13 real minutes of shift. Now bare in +dev, **Ctrl+Shift in a built game**, and `roster`/`R` added so there is a way +home. + +### Still open from the review (verified real, not yet fixed) +- **The Royal enforces four rules it never texts.** `ruleLimit` is consumed only + by `dazzaDue()` (which texts fire); `announcedRules()` filters the full + eight-rule book by `activeFrom` alone. From clock 200 at The Royal, ALL FOUR + live rules are ones the player was never shown. Fix: a module-level rule-limit + mode beside `setDressCodeShuffle`, sliced inside `announcedRules()`. +- **The DJ's help line is never painted.** `toggleDecks` sets promptText, then + `drawHud` overwrites it from `target.prompt` the same frame. `B` appears in + exactly one string in the repo, so the DJ's signature verb has no in-game + surface at all. +- **The door tutorial burns down while you are on the floor.** `elapsedMs` + accumulates above the `!present` branch, and two of The Royal's three shifts + start on the floor. diff --git a/src/audio/Sfx.ts b/src/audio/Sfx.ts index 2f44497..c17e524 100644 --- a/src/audio/Sfx.ts +++ b/src/audio/Sfx.ts @@ -17,7 +17,10 @@ export type SfxName = | 'ropeUnhook' | 'denyCrowdOoh' | 'typeTick' - | 'glassSmash'; + | 'glassSmash' + | 'sorterCount' + | 'sorterJam' + | 'sorterRefuse'; /** Seconds of white noise generated once and shared by every noise voice. */ const NOISE_SECONDS = 2; @@ -87,6 +90,12 @@ export class Sfx { return this.typeTick(); case 'glassSmash': return this.glassSmash(); + case 'sorterCount': + return this.sorterCount(); + case 'sorterJam': + return this.sorterJam(); + case 'sorterRefuse': + return this.sorterRefuse(); } } @@ -283,6 +292,57 @@ export class Sfx { this.voice(ring, [this.band(5200, 6), this.env(t + 0.03, 0.001, 0.01, 0.08, 0.16)], t + 0.03, 0.18); } + + // ---- the back office's coin sorter (scenes/shared/HaulDeskScene) --------- + // The whole screen is a machine succeeding, then failing, then giving up, so + // the three voices are deliberately one family: same metal, different mood. + + /** Coin through the channels: a bright tick and the mechanism taking it. */ + sorterCount(): void { + const t = this.ctx.currentTime; + this.click(t, 2600, 0.34); + const body = this.noiseSource(); + this.voice(body, [this.band(1400, 4), this.env(t + 0.02, 0.001, 0.01, 0.09, 0.3)], t + 0.02, 0.14); + // The satisfying part: a low thunk as it drops into the tray. + const drop = this.ctx.createOscillator(); + drop.type = 'triangle'; + drop.frequency.setValueAtTime(210, t + 0.06); + drop.frequency.exponentialRampToValueAtTime(96, t + 0.16); + this.voice(drop, [this.env(t + 0.06, 0.001, 0.01, 0.12, 0.5)], t + 0.06, 0.2); + } + + /** A kebab in the coin channels: the motor labours, something grinds. */ + sorterJam(): void { + const t = this.ctx.currentTime; + const grind = this.noiseSource(); + this.voice(grind, [this.band(320, 2.2), this.env(t, 0.01, 0.16, 0.22, 0.55)], t, 0.42); + // A motor pitching DOWN reads as strain in a way a flat tone never does. + const motor = this.ctx.createOscillator(); + motor.type = 'sawtooth'; + motor.frequency.setValueAtTime(88, t); + motor.frequency.exponentialRampToValueAtTime(41, t + 0.34); + this.voice(motor, [this.band(600, 1.4), this.env(t, 0.01, 0.14, 0.24, 0.34)], t, 0.4); + const clack = this.noiseSource(); + this.voice(clack, [this.band(1900, 5), this.env(t + 0.26, 0.001, 0.01, 0.07, 0.3)], t + 0.26, 0.12); + } + + /** The bag. The machine simply stops, and the room is suddenly quiet. */ + sorterRefuse(): void { + const t = this.ctx.currentTime; + const halt = this.ctx.createOscillator(); + halt.type = 'square'; + halt.frequency.setValueAtTime(150, t); + halt.frequency.exponentialRampToValueAtTime(58, t + 0.09); + this.voice(halt, [this.env(t, 0.002, 0.02, 0.08, 0.42)], t, 0.14); + // Two flat, unamused beeps. It has a slot for everything except this. + for (const at of [t + 0.16, t + 0.34]) { + const beep = this.ctx.createOscillator(); + beep.type = 'square'; + beep.frequency.setValueAtTime(392, at); + this.voice(beep, [this.env(at, 0.002, 0.05, 0.04, 0.2)], at, 0.12); + } + } + /** Idempotent — the door scene may call this on every re-entry. */ startRain(): void { if (this.rain) return; diff --git a/src/main.ts b/src/main.ts index e5c179e..43bfa72 100644 --- a/src/main.ts +++ b/src/main.ts @@ -52,7 +52,7 @@ const game = new Phaser.Game({ // Keys: N night (the game) · P parade · F floor demo · J juice demo. // URL hashes #night/#parade/#floor/#juice pick the boot scene. // NightScene owns DoorScene/NightSummaryScene as children; stopping Night stops them. -const ROUTES = { night: 'Night', parade: 'Parade', floor: 'FloorDemo', juice: 'JuiceDemo' } as const; +const ROUTES = { roster: 'Roster', night: 'Night', parade: 'Parade', floor: 'FloorDemo', juice: 'JuiceDemo' } as const; type RouteKey = keyof typeof ROUTES; /** @@ -92,9 +92,24 @@ if (hash in ROUTES) { game.events.on(Phaser.Core.Events.POST_STEP, route); } +/** + * Dev routes are a LOADED GUN in a browser game: they live on `window`, so no + * scene can consume them, and F is the universal fullscreen reflex while M is + * the universal mute reflex. On the live build a stray F used to stop Night — + * which tears down Door and Floor — and the only saveGame runs at night end, + * so it silently binned up to thirteen real minutes of shift. + * + * Bare letters stay in dev, where they are genuinely useful. In a built game + * they need Ctrl+Shift, which nobody presses by accident. + */ +const devRouteAllowed = (e: KeyboardEvent): boolean => + import.meta.env.DEV || (e.ctrlKey && e.shiftKey); + window.addEventListener('keydown', (e) => { + if (!devRouteAllowed(e)) return; const k = e.key.toLowerCase(); - if (k === 'n') showScene('Night'); + if (k === 'r') showScene('Roster'); + else if (k === 'n') showScene('Night'); else if (k === 'p') showScene('Parade'); else if (k === 'f') showScene('FloorDemo'); else if (k === 'j') showScene('JuiceDemo'); diff --git a/src/scenes/door/IncidentReportScene.ts b/src/scenes/door/IncidentReportScene.ts index 2c493e8..57fc10e 100644 --- a/src/scenes/door/IncidentReportScene.ts +++ b/src/scenes/door/IncidentReportScene.ts @@ -208,7 +208,15 @@ export class IncidentReportScene extends Phaser.Scene { .text(W / 2, H - 20, REPORT_UI.done, { fontFamily: MONO, fontSize: '8px', color: DOOR_PALETTE.inkDim }) .setOrigin(0.5); - const go = (): void => this.input$.next(); + // Stop OURSELVES before handing on. `next` is always a closure that calls + // scene.start('Roster') from NightSummary, and Phaser's ScenePlugin.start + // only queues a stop for its OWN key — so without this the paperwork keeps + // rendering its opaque full-screen panel on top of the roster board, and + // the game looks frozen after the very first night of every run. + const go = (): void => { + this.scene.stop(); + this.input$.next(); + }; this.input.once('pointerdown', go); this.input.keyboard?.once('keydown-SPACE', go); } diff --git a/src/scenes/door/NightScene.ts b/src/scenes/door/NightScene.ts index f0898dc..01d338a 100644 --- a/src/scenes/door/NightScene.ts +++ b/src/scenes/door/NightScene.ts @@ -623,6 +623,7 @@ export class NightScene extends Phaser.Scene { haul: score.haul, pocketedBaggie: score.pocketedBaggie, pocketCash: this.log.pocketCash, + sfx: this.sfx, next: proceed, }); }); diff --git a/src/scenes/shared/HaulDeskScene.ts b/src/scenes/shared/HaulDeskScene.ts index ba0876f..9b20e23 100644 --- a/src/scenes/shared/HaulDeskScene.ts +++ b/src/scenes/shared/HaulDeskScene.ts @@ -33,6 +33,8 @@ export interface HaulDeskData { pocketedBaggie: boolean; /** Cash already in your pocket from the shift (booth tips). Not sorted. */ pocketCash: number; + /** The night's sfx rig. Optional: the desk is playable in silence. */ + sfx?: { play: (name: 'sorterCount' | 'sorterJam' | 'sorterRefuse') => void } | null; next: () => void; } @@ -63,6 +65,24 @@ export class HaulDeskScene extends Phaser.Scene { } create(): void { + // Phaser reuses ONE instance of this scene for the life of the game, and + // every field below is a class-field initialiser that only ran once. After + // the first night `finished` was still true, so update() bailed, the + // ceremony never played and finish() returned before registering the exit + // handlers — a blank $0 desk that ate every click, key and ESC. + // + // The irony worth remembering: adding scene.stop() on the way out (SOLO-26, + // correctly) is exactly what guarantees the next start hands back a dirty + // instance. Stopping and resetting are two halves of one job. + this.lines = []; + this.idx = 0; + this.counted = 0; + this.finished = false; + this.phase = 'wait'; + this.phaseMs = 0; + this.phaseFor = 700; + this.falling = null; + const t = tallyHaul(this.args.haul, this.args.pocketedBaggie); this.lines = t.lines; @@ -187,6 +207,7 @@ export class HaulDeskScene extends Phaser.Scene { this.tweens.add({ targets: this.tallyText, scale: { from: 1.3, to: 1 }, duration: 180 }); this.tweens.add({ targets: this.sorter, y: { from: 214, to: 210 }, duration: 90, yoyo: true }); push(`+ $${line.value}`); + this.args.sfx?.play('sorterCount'); this.phase = 'settling'; this.phaseFor = BEAT_MS; return; @@ -202,6 +223,7 @@ export class HaulDeskScene extends Phaser.Scene { // A jam or a refusal: the machine shakes, thinks, and moves on with less // dignity than it had. This is the whole reason the screen exists. push(line.fate === 'refused' ? '· not a coin' : '· JAM'); + this.args.sfx?.play(line.fate === 'refused' ? 'sorterRefuse' : 'sorterJam'); this.tweens.add({ targets: this.sorter, x: { from: W / 2 - 3, to: W / 2 + 3 }, diff --git a/tests/sceneLifecycle.test.ts b/tests/sceneLifecycle.test.ts new file mode 100644 index 0000000..12d6804 --- /dev/null +++ b/tests/sceneLifecycle.test.ts @@ -0,0 +1,84 @@ +import { describe, expect, it } from 'vitest'; + + +// The bug class this file exists for, hit FOUR times now: +// 1. DoorScene.busy stuck true after a late ruling (dead door for the run) +// 2. seenCards leaking across nights (every regular stamped SEEN TONIGHT) +// 3. IncidentReportScene never stopping (paperwork frozen over the roster) +// 4. HaulDeskScene never resetting (a blank $0 desk that eats every input) +// +// Phaser instantiates each configured scene ONCE and reuses that instance for +// the life of the game. So two rules hold for every scene that is entered more +// than once per session: +// - anything scoped to one night must be reset in create()/init(), never +// left to a class-field initialiser, and +// - a scene that hands control to a scene it does not own must stop itself, +// because ScenePlugin.start only queues a stop for its OWN key. +// +// Driving real Phaser here would need a canvas and a DOM; these are source +// assertions instead. Cheaper, and they fail for the right reason with a +// message that names the fix. + +// Vite's glob import instead of node:fs — this suite runs under the app's +// tsconfig, which has no node types, and `?raw` gives us the source as a string +// without adding a dependency or loosening the config for one test. +const SOURCES = import.meta.glob('../src/**/*.ts', { query: '?raw', import: 'default', eager: true }) as Record; + +const src = (p: string): string => { + const hit = Object.entries(SOURCES).find(([k]) => k.endsWith(`/src/${p}`)); + if (!hit) throw new Error(`sceneLifecycle: could not read src/${p}`); + return hit[1]; +}; + +describe('scene lifecycle: state that must not survive the night', () => { + it('HaulDeskScene clears every per-night field in create()', () => { + const s = src('scenes/shared/HaulDeskScene.ts'); + const body = s.slice(s.indexOf('create(): void {')); + for (const field of ['finished', 'idx', 'counted', 'phase', 'phaseMs', 'phaseFor', 'falling']) { + expect(body, `HaulDeskScene.create() must reset this.${field}`).toContain(`this.${field} =`); + } + }); + + it('IncidentReportScene resets its answers and index per night', () => { + const s = src('scenes/door/IncidentReportScene.ts'); + const head = s.slice(0, s.indexOf('showQuestion')); + expect(head).toMatch(/this\.(answers|idx|index)\s*=/); + }); + + it('DoorScene clears the passback net and the busy latch every night', () => { + // Both of these have already shipped as bugs once. + const body = src('scenes/door/DoorScene.ts'); + expect(body).toContain('this.seenCards.clear()'); + expect(body).toContain('this.busy = false'); + }); +}); + +describe('scene lifecycle: a scene that hands over must stop itself', () => { + // Each of these hands control to a scene it does not own, via a `next` + // closure or a direct start of a sibling. Phaser will not stop them for us. + const HANDOVERS: Array<[string, string]> = [ + ['scenes/shared/HaulDeskScene.ts', 'the back office'], + ['scenes/door/IncidentReportScene.ts', 'the paperwork'], + ]; + + for (const [path, what] of HANDOVERS) { + it(`${what} stops itself on the way out`, () => { + const s = src(path); + expect( + s.includes('this.scene.stop()'), + `${path}: calls into another scene but never stops itself — it will keep ` + + `rendering underneath whatever comes next (ScenePlugin.start only stops its own key)`, + ).toBe(true); + }); + } + + it('and the handover happens AFTER the stop, not before', () => { + // Stopping after the handover would tear down the scene that was just + // started in some orderings; stop first, then hand on. + const s = src('scenes/shared/HaulDeskScene.ts'); + const stop = s.lastIndexOf('this.scene.stop()'); + const next = s.indexOf('this.args.next()', stop); + expect(stop).toBeGreaterThan(-1); + expect(next, 'next() should follow scene.stop() in the exit handler').toBeGreaterThan(stop); + }); +});