Add SMS top-level class; remove all module singletons (Phase 3)#60
Add SMS top-level class; remove all module singletons (Phase 3)#60mattgodbolt merged 7 commits intomainfrom
Conversation
- SMS class owns z80, bus, vdp, soundChip — no module-level singletons - miracle.js becomes DOM shell only - debug.js receives deps via init args, not imports - All convenience re-exports removed 🤖 Generated by LLM (Claude, via OpenClaw)
There was a problem hiding this comment.
Pull request overview
This PR refactors the emulator architecture to center around a new SMS top-level class that owns/wires CPU, bus, VDP, and opcode execution, removing prior module-level singletons and shifting callers to dependency-injected access patterns.
Changes:
- Introduces
SMSas the top-level owner/wiring point forBus,Z80,VDP, and the Z80 opcode runner. - Removes
bus/vdp/z80singletons and updatesmiracle.js,main.js, anddebug.jsto use theSMSinstance + injected callbacks. - Wraps Z80 disassembly behind
makeDisassembler(readbyte, addressHtml)and updates tests accordingly.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/fuse.test.js | Updates FUSE CPU tests to use a locally constructed Z80 wired to a mock bus (no module mocks). |
| tests/disassemble.test.js | Updates disassembler tests to use makeDisassembler() with injected memory/address formatting. |
| src/z80/z80_ops.js | Switches opcode runner to call lifecycle hooks as Z80 instance methods. |
| src/z80/z80_dis.js | Converts disassembler into a factory (makeDisassembler) with injected dependencies. |
| src/z80/z80.js | Removes singleton export, adds lifecycle methods on Z80, and self-initializes lookup tables. |
| src/vdp.js | Removes singleton export and injects IRQ assertion via callback passed to init(). |
| src/sms.js | Adds new SMS class that owns/wires Bus/Z80/VDP and exposes a small public API. |
| src/miracle.js | Becomes a thin DOM/audio shell that drives emulation via sms.runLine(). |
| src/main.js | Routes ROM loading and debugger setup through sms.loadRom() + injected debug_init(...). |
| src/debug.js | Removes direct imports of singletons; initializes with injected sms instance + callbacks. |
| src/bus.js | Exports Bus class and removes singleton + convenience re-exports. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- VDP buffers now allocated in constructor (field initializers), not init() - SMS.runLine/execOpcodes guard against pre-init calls 🤖 Generated by LLM (Claude, via OpenClaw)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Resolve main.js conflict: preserve sms.loadRom() from SMS branch, incorporate updateUrl/sanitizeRomName/?load= from #61 - SMS instance created in main.js (const sms = new SMS()) and passed to miracle_init(sms) — miracle.js no longer owns the singleton - miracle.js: keeps SMS import for static constants (FRAMES_PER_SECOND etc.) but does not create an instance Addresses Copilot review: PR description now matches reality — no module-level singleton exports anywhere. 🤖 Generated by LLM (Claude, via OpenClaw)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Introduced by conflict resolution merge — updateUrl was called both before and after start(). Keep only the post-start call. 🤖 Generated by LLM (Claude, via OpenClaw)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- miracle.js: restore SoundChip creation comment, floor-vs-ceil explanation, drain-every-frame block, only-push-while-running block, ArrayBuffer transfer comment, audio_enable resume comment, ResizeObserver comment, Unsupported... note, inputMode TODO - debug.js: restore TODO(#18) re-enable block in updateDisassembly 🤖 Generated by LLM (Claude, via OpenClaw)
Previously the scanline loop stopped early when breakpointHit was set, so paintScreen() was never called. SMS.runLine() was calling it unconditionally, rendering one extra frame after a breakpoint fires. Gate the paint call on !#breakpointHit to restore original behaviour. 🤖 Generated by LLM (Claude, via OpenClaw)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
🤖 Generated by LLM (Claude, via OpenClaw)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
src/sms.js) privately ownsBus,Z80,VDP,soundChip, andz80_do_opcodes— no module-level singletons remaingetZ80(),getBus(),getVdp(),execOpcodes()) or clean public API (runLine(),reset(),loadRom(),nmi(),joystick,pc)sms.runLine(cycleCallback)in the game loopdebug_init(romName, sms, callbacks)— no imports from bus/vdp/z80/miraclemakeDisassembler(readbyte, addressHtml)factory closure (same pattern asmakeZ80Runner)Test plan
npm run build— Vite build passesnpm test— All 1342 FUSE tests + 13 disassembler tests pass🤖 Generated by LLM (Claude, via OpenClaw)