From b2d2ff215960d708a9cee45859630eb995298c42 Mon Sep 17 00:00:00 2001 From: Steven Hugg Date: Mon, 28 Sep 2026 16:54:50 +0200 Subject: [PATCH] timeline: MachineCore, galaxian: save the state at a mid-frame stop, not after the frame tail after a trap --- extension/notes/PLAN.md | 4 ++- src/common/timeline.ts | 35 +++++++++++++++------- test/unit/testtimeline.ts | 28 ++++++++++++++++++ test/unit/testtimelinemachine.ts | 51 ++++++++++++++++++++++++++++++++ 4 files changed, 106 insertions(+), 12 deletions(-) diff --git a/extension/notes/PLAN.md b/extension/notes/PLAN.md index 0133d093..9b83bdf8 100644 --- a/extension/notes/PLAN.md +++ b/extension/notes/PLAN.md @@ -1701,7 +1701,9 @@ The shared core adopts the IDE's model: Registers, memory, the disassembly, and the debug views all read the snapshot, not the live machine. After `advanceFrame` returns, the core loads the snapshot back (as `postFrame` does) so memory reads and writes - go to the right state. + go to the right state. Done for `MachineCore` (`common/timeline.ts`): + it saves and reloads `saveState()`. An error thrown by the frame's tail + after a stop is dropped; resuming replays the frame and raises it then. - **Resuming or stepping always replays**: load the frame-start state and run to `step + n` with breakpoints suppressed until `step`. Step back replays to `step - 1` (a step count, not the IDE's `clock - 25` diff --git a/src/common/timeline.ts b/src/common/timeline.ts index c3580779..77e5bb9b 100644 --- a/src/common/timeline.ts +++ b/src/common/timeline.ts @@ -205,22 +205,35 @@ export class MachineCore implements DeterministicCore { } let n = 0; let trapped = false; + // A trap stops the CPU loop, not the frame: advanceFrame() still runs its + // tail (drawing the line, the vblank interrupt, a watchdog) after the loop + // exits. So the state is saved inside the trap, at the exact stop, and + // loaded back once advanceFrame() returns. + let stopState: any = null; // The trap runs before each step, so when it returns true exactly n steps // have executed -- n is the position, not the step about to run. step is // published before calling the caller's trap so that now() is the current // position while the condition is being evaluated; a search relies on that // to record where a hit happened. - this.machine.advanceFrame(() => { - this.step = n; - if (n >= untilStep) return true; - if (n >= from && trap && trap()) { - trapped = true; - return true; - } - n++; - return false; - }); - if (trapped || n >= untilStep) { + try { + this.machine.advanceFrame(() => { + this.step = n; + if (n >= untilStep || (n >= from && trap && (trapped = !!trap()))) { + stopState = this.machine.saveState(); + return true; + } + n++; + return false; + }); + } catch (e) { + // Once stopped, only the frame's tail runs, and it runs early: the frame + // hasn't really ended. An error from it (galaxian's watchdog) is not an + // error at the stop. Resuming replays the whole frame, so a real one is + // raised again then. + if (!stopState) throw e; + } + if (stopState) this.machine.loadState(stopState); + if (stopState) { this.step = n; return trapped; } diff --git a/test/unit/testtimeline.ts b/test/unit/testtimeline.ts index c8ce30fa..e1398d9c 100644 --- a/test/unit/testtimeline.ts +++ b/test/unit/testtimeline.ts @@ -55,6 +55,34 @@ describe('Timestamp', function () { describe('MachineCore', function () { + it('ignores an error from the frame tail after a stop, but not otherwise', function () { + // like galaxian: code after the CPU loop mutates state and may throw + class TailMachine extends FakeMachine { + watchdog = 0; + advanceFrame(trap: TrapCondition): number { + const n = super.advanceFrame(trap); + this.acc += 1000; + if (++this.watchdog >= 3) throw new Error('watchdog'); + return n; + } + saveState() { return { ...super.saveState(), watchdog: this.watchdog }; } + loadState(s) { super.loadState(s); this.watchdog = s.watchdog; } + } + const m = new TailMachine(); + const c = new MachineCore(m); + c.runUntil(timestamp(2, 0)); + // the tail throws on this frame, but the stop comes first + var steps = 0; + const r = c.runUntil(timestamp(3, 0), () => steps++ === 5); + assert.ok(r.trapped); + assert.ok(timestampsEqual(r.at, timestamp(2, 5)), formatTimestamp(r.at)); + assert.strictEqual(m.acc, 2025); + assert.strictEqual(m.watchdog, 2); + // finishing the frame runs the tail for real + assert.throws(() => c.runUntil(timestamp(3, 0)), /watchdog/); + }); + + it('counts frames monotonically and never rewinds the clock', function () { const m = new FakeMachine(); const c = new MachineCore(m); diff --git a/test/unit/testtimelinemachine.ts b/test/unit/testtimelinemachine.ts index 936e6a89..c0b15589 100644 --- a/test/unit/testtimelinemachine.ts +++ b/test/unit/testtimelinemachine.ts @@ -9,6 +9,8 @@ mockAudio(); // imported after the globals are mocked -- the machine pulls in the audio // classes that live on globalThis import { Midway8080 } from "../../src/machine/mw8080bw"; +import { GalaxianScrambleMachine } from "../../src/machine/galaxian"; +import { hashState as hashAnyState } from "../../src/common/statehash"; import { NullProbe } from "../../src/common/devices"; import { History } from "../../src/common/history"; import { @@ -191,3 +193,52 @@ describe('History on a real machine', function () { assert.strictEqual(m.cpu.getPC(), pcAt39); }); }); + +// A trap stops the CPU loop, not the frame: galaxian's advanceFrame() still +// advances its graphics, decrements the watchdog and raises an NMI after the +// loop exits. A core stopped mid-frame must show the state at the stop, not +// the state after that tail ran. +describe('MachineCore break state', function () { + + function newGalaxian() { + const m = new GalaxianScrambleMachine(); + m.connectVideo(new Uint32Array(m.canvasWidth * m.numVisibleScanlines)); + m.loadROM(new Uint8Array(fs.readFileSync('./test/roms/galaxian-scramble/shoot2.c.rom'))); + m.reset(); + return { m, core: new MachineCore(m) }; + } + + it('shows the state at the trap, not after the frame tail', function () { + const { m, core } = newGalaxian(); + core.runUntil(timestamp(5, 0)); + var n = 0; + var atTrap: number = null; + const r = core.runUntil(timestamp(6, 0), () => { + if (n++ < 500) return false; + atTrap = hashAnyState(m.saveState()); + return true; + }); + assert.ok(r.trapped); + assert.strictEqual(hashAnyState(m.saveState()), atTrap); + }); + + it('shows the state at the target step when stopping at one', function () { + // the trap is not consulted at the target step itself, so capture the + // expected state from a second run that goes one step further + const ref = newGalaxian(); + ref.core.runUntil(timestamp(5, 0)); + var atStep: number = null; + var n = 0; + ref.core.runUntil(timestamp(5, 501), () => { + if (n++ === 500) atStep = hashAnyState(ref.m.saveState()); + return false; + }); + assert.notStrictEqual(atStep, null); + + const { m, core } = newGalaxian(); + core.runUntil(timestamp(5, 0)); + core.runUntil(timestamp(5, 500)); + assert.ok(timestampsEqual(core.now(), timestamp(5, 500)), formatTimestamp(core.now())); + assert.strictEqual(hashAnyState(m.saveState()), atStep); + }); +});