Skip to content

Add SMS top-level class (Phase 3)#59

Closed
mattgodbolt-molty wants to merge 2 commits intomainfrom
molty/sms-class
Closed

Add SMS top-level class (Phase 3)#59
mattgodbolt-molty wants to merge 2 commits intomainfrom
molty/sms-class

Conversation

@mattgodbolt-molty
Copy link
Collaborator

Summary

  • Introduce src/sms.js with an SMS class that owns Bus, Z80, VDP, and soundChip, replacing module-level singletons
  • Move Z80 lifecycle functions (z80_reset, z80_set_irq, z80_interrupt, z80_nmi, z80_instruction_hook) into Z80 class methods (reset(), setIrq(), interrupt(), nmi(), instructionHook())
  • VDP receives its setIrq callback via init() instead of importing z80_set_irq
  • miracle.js becomes a thin DOM/audio shell that delegates to the SMS instance
  • Lookup tables self-initialize at module load time via IIFE; z80_init() becomes a no-op
  • Tests use direct Z80 instantiation (no more vi.mock for bus in fuse tests)

Test plan

  • npm run build — Vite build passes
  • npm test — All 1342 FUSE tests + 13 disassembler tests pass
  • Manual: load a ROM in the browser, confirm gameplay works (VDP rendering, audio, input, debugger)

🤖 Generated with Claude Code

mattgodbolt-molty and others added 2 commits March 1, 2026 14:13
- makeZ80Runner(z80) factory closes over bus shims and timing
- Removes z80_ops.js imports of readbyte/writebyte/z80 singleton
- tstates and eventNextEvent moved to Z80 instance
- Zero changes to z80_generator.mjs or generated opcode code

🤖 Generated by LLM (Claude, via OpenClaw)
Introduce src/sms.js with an SMS class that owns Bus, Z80, VDP, and
soundChip, replacing the module-level singletons. Lifecycle functions
(z80_reset, z80_set_irq, z80_interrupt, z80_nmi, z80_instruction_hook)
become Z80 class methods. VDP receives its setIrq callback via init()
instead of importing z80_set_irq. miracle.js becomes a thin DOM/audio
shell that delegates to the SMS instance.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
// Default instance
export const sms = new SMS();

// Convenience re-exports (alias to default instance members)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant these Molty: these convenience exports encourage more dependence on global variables. Are they used?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I'm Molty, an AI assistant acting on behalf of @mattgodbolt)

Yes, they are being used — by debug.js and miracle.js which are still importing bus, z80, vdp, readbyte etc. as module-level names instead of receiving them from the SMS instance.

These re-exports are exactly the problem: they keep the global-import pattern alive while just hiding the singleton one level deeper. debug.js should receive { z80, bus, vdp } as constructor/init arguments from miracle.js, which in turn gets them from sms. None of them should be importing siblings at all.

Rebasing and fixing now.

@mattgodbolt-molty
Copy link
Collaborator Author

(I'm Molty, an AI assistant acting on behalf of @mattgodbolt) Closing to rewrite without convenience re-exports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants