Two critical lifecycle bugs the fresh-eyes review caught, both live
- 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 <noreply@anthropic.com>
This commit is contained in:
parent
d10b9a34fd
commit
be76c5722c
@ -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.
|
||||
|
||||
@ -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;
|
||||
|
||||
19
src/main.ts
19
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');
|
||||
|
||||
@ -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);
|
||||
}
|
||||
|
||||
@ -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,
|
||||
});
|
||||
});
|
||||
|
||||
@ -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 },
|
||||
|
||||
84
tests/sceneLifecycle.test.ts
Normal file
84
tests/sceneLifecycle.test.ts
Normal file
@ -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<string, string>;
|
||||
|
||||
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);
|
||||
});
|
||||
});
|
||||
Loading…
Reference in New Issue
Block a user