refactor panics to error handling

internal/testroms/nestest/nestest_test.go

@@ -32,7 +32,7 @@ func TestNestest(t *testing.T) {
 		nes.WithDisabledGUI(),
 		nes.WithTracingTarget(trace),
 	}
-	nes.Start(options...)
+	assert.NoError(t, nes.Start(options...))
 
 	assert.NoError(t, trace.Flush())
 

main.go

@@ -101,6 +101,8 @@ func emulateFile(options optionFlags) error {
 		opts = append(opts, nes.WithDisabledGUI())
 	}
 
-	nes.Start(opts...)
+	if err := nes.Start(opts...); err != nil {
+		return fmt.Errorf("starting emulator: %w", err)
+	}
 	return nil
 }

pkg/mapper/mapper.go

@@ -10,7 +10,7 @@ import (
 	"github.com/retroenv/nesgoemu/pkg/mapper/mapperdb"
 )
 
-type mapperInitializer func(base mapperdb.Base) bus.Mapper
+type mapperInitializer func(base mapperdb.Base) (bus.Mapper, error)
 
 var mappers = map[byte]mapperInitializer{
 	0:   mapperdb.NewNROM,
@@ -33,6 +33,9 @@ func New(bus *bus.Bus) (bus.Mapper, error) {
 	}
 
 	base := mapperbase.New(bus)
-	mapper := initializer(base)
+	mapper, err := initializer(base)
+	if err != nil {
+		return nil, err
+	}
 	return mapper, nil
 }

pkg/mapper/mapperbase/base.go

@@ -92,7 +92,10 @@ func (b *Base) Read(address uint16) uint8 {
 
 	for _, hook := range b.readHooks {
 		if address >= hook.startAddress && address <= hook.endAddress {
-			value = hook.hookFunc(address)
+			value, err := hook.hookFunc(address)
+			if err != nil {
+				panic(fmt.Sprintf("read hook error: %v", err))
+			}
 			if !hook.onlyProxy {
 				return value
 			}
@@ -128,7 +131,9 @@ func (b *Base) Read(address uint16) uint8 {
 func (b *Base) Write(address uint16, value uint8) {
 	for _, hook := range b.writeHooks {
 		if address >= hook.startAddress && address <= hook.endAddress {
-			hook.hookFunc(address, value)
+			if err := hook.hookFunc(address, value); err != nil {
+				panic(fmt.Errorf("write hook error: %w", err))
+			}
 			if !hook.onlyProxy {
 				return
 			}

pkg/mapper/mapperbase/hook.go

@@ -5,6 +5,10 @@ type Hook interface {
 	SetProxyOnly(proxy bool)
 }
 
+type ReadHookFunc func(address uint16) (uint8, error)
+
+type WriteHookFunc func(address uint16, value uint8) error
+
 type hook struct {
 	startAddress uint16
 	endAddress   uint16
@@ -15,21 +19,21 @@ type hook struct {
 type readHook struct {
 	hook
 
-	hookFunc func(address uint16) uint8
+	hookFunc ReadHookFunc
 }
 
 type writeHook struct {
 	hook
 
-	hookFunc func(address uint16, value uint8)
+	hookFunc WriteHookFunc
 }
 
 func (h *hook) SetProxyOnly(proxy bool) {
 	h.onlyProxy = proxy
 }
 
 // AddReadHook adds an address range read hook that gets called when a read from given range is made.
-func (b *Base) AddReadHook(startAddress, endAddress uint16, hookFunc func(address uint16) uint8) Hook {
+func (b *Base) AddReadHook(startAddress, endAddress uint16, hookFunc ReadHookFunc) Hook {
 	hook := readHook{
 		hook: hook{
 			startAddress: startAddress,
@@ -42,7 +46,7 @@ func (b *Base) AddReadHook(startAddress, endAddress uint16, hookFunc func(addres
 }
 
 // AddWriteHook adds an address range write hook that gets called when a write into the given range is made.
-func (b *Base) AddWriteHook(startAddress, endAddress uint16, hookFunc func(address uint16, value uint8)) Hook {
+func (b *Base) AddWriteHook(startAddress, endAddress uint16, hookFunc WriteHookFunc) Hook {
 	hook := writeHook{
 		hook: hook{
 			startAddress: startAddress,

pkg/mapper/mapperbase/nametable.go

@@ -43,8 +43,9 @@ func (b *Base) NameTable(bank int) []byte {
 }
 
 // SetNameTableMirrorMode sets the nametable mirror mode.
-func (b *Base) SetNameTableMirrorMode(mirrorMode cartridge.MirrorMode) {
+func (b *Base) SetNameTableMirrorMode(mirrorMode cartridge.MirrorMode) error {
 	b.bus.NameTable.SetMirrorMode(mirrorMode)
+	return nil
 }
 
 // MirrorMode returns the set mirror mode.
@@ -59,10 +60,11 @@ func (b *Base) SetMirrorModeTranslation(translation MirrorModeTranslation) {
 
 // SetNameTableMirrorModeIndex sets the nametable mirror mode based on the value of the mapper based
 // translation map from index to mirror mode.
-func (b *Base) SetNameTableMirrorModeIndex(index uint8) {
+func (b *Base) SetNameTableMirrorModeIndex(index uint8) error {
 	mode, ok := b.mirrorModeTranslation[index]
 	if !ok {
-		panic(fmt.Sprintf("invalid nametable mirror mode index %d", index))
+		return fmt.Errorf("invalid nametable mirror mode index %d", index)
 	}
 	b.bus.NameTable.SetMirrorMode(mode)
+	return nil
 }

pkg/mapper/mapperdb/axrom.go

@@ -18,7 +18,7 @@ type mapperAxROM struct {
 }
 
 // NewAxROM returns a new mapper instance.
-func NewAxROM(base Base) bus.Mapper {
+func NewAxROM(base Base) (bus.Mapper, error) {
 	m := &mapperAxROM{
 		Base: base,
 	}
@@ -33,13 +33,13 @@ func NewAxROM(base Base) bus.Mapper {
 	m.SetMirrorModeTranslation(translation)
 
 	m.AddWriteHook(0x8000, 0xFFFF, m.setPrgWindow)
-	return m
+	return m, nil
 }
 
-func (m *mapperAxROM) setPrgWindow(_ uint16, value uint8) {
+func (m *mapperAxROM) setPrgWindow(_ uint16, value uint8) error {
 	value &= 0b0000_0111
 	m.SetPrgWindow(0, int(value)) // select 32 KB PRG ROM bank for CPU $8000-$FFFF
 
 	mirrorMode := (value >> 4) & 1
-	m.SetNameTableMirrorModeIndex(mirrorMode)
+	return m.SetNameTableMirrorModeIndex(mirrorMode)
 }

pkg/mapper/mapperdb/axrom_test.go

@@ -20,7 +20,8 @@ func TestMapperAxROM(t *testing.T) {
 		},
 		NameTable: nametable.New(cartridge.MirrorHorizontal),
 	})
-	m := NewAxROM(base)
+	m, err := NewAxROM(base)
+	assert.NoError(t, err)
 
 	prg[0x0010] = 0x03 // bank 0
 	prg[0x8010] = 0x04 // bank 1

pkg/mapper/mapperdb/base.go

@@ -24,12 +24,12 @@ type Base interface {
 	NameTable(bank int) []byte
 	SetMirrorModeTranslation(translation mapperbase.MirrorModeTranslation)
 	SetNameTableCount(count int)
-	SetNameTableMirrorMode(mirrorMode cartridge.MirrorMode)
-	SetNameTableMirrorModeIndex(index uint8)
+	SetNameTableMirrorMode(mirrorMode cartridge.MirrorMode) error
+	SetNameTableMirrorModeIndex(index uint8) error
 	SetNameTableWindow(bank int)
 
-	AddReadHook(startAddress, endAddress uint16, hookFunc func(address uint16) uint8) mapperbase.Hook
-	AddWriteHook(startAddress, endAddress uint16, hookFunc func(address uint16, value uint8)) mapperbase.Hook
+	AddReadHook(startAddress, endAddress uint16, hookFunc mapperbase.ReadHookFunc) mapperbase.Hook
+	AddWriteHook(startAddress, endAddress uint16, hookFunc mapperbase.WriteHookFunc) mapperbase.Hook
 	Cartridge() *cartridge.Cartridge
 	Initialize()
 	SetName(name string)

pkg/mapper/mapperdb/cnrom.go

@@ -16,17 +16,18 @@ type mapperCNROM struct {
 }
 
 // NewCNROM returns a new mapper instance.
-func NewCNROM(base Base) bus.Mapper {
+func NewCNROM(base Base) (bus.Mapper, error) {
 	m := &mapperCNROM{
 		Base: base,
 	}
 	m.SetName("CNROM")
 	m.Initialize()
 
 	m.AddWriteHook(0x8000, 0xFFFF, m.setChrWindow)
-	return m
+	return m, nil
 }
 
-func (m *mapperCNROM) setChrWindow(_ uint16, value uint8) {
+func (m *mapperCNROM) setChrWindow(_ uint16, value uint8) error {
 	m.SetChrWindow(0, int(value)) // select 8 KB CHR ROM bank for PPU $0000-$1FFF
+	return nil
 }

pkg/mapper/mapperdb/cnrom_test.go

@@ -20,7 +20,8 @@ func TestMapperCNROM(t *testing.T) {
 		},
 		NameTable: nametable.New(cartridge.MirrorHorizontal),
 	})
-	m := NewCNROM(base)
+	m, err := NewCNROM(base)
+	assert.NoError(t, err)
 
 	chr[0x0010] = 0x03 // bank 0
 	chr[0x2010] = 0x04 // bank 1

pkg/mapper/mapperdb/gtrom.go

@@ -19,7 +19,7 @@ type mapperGTROM struct {
 }
 
 // NewGTROM returns a new mapper instance.
-func NewGTROM(base Base) bus.Mapper {
+func NewGTROM(base Base) (bus.Mapper, error) {
 	m := &mapperGTROM{
 		Base: base,
 	}
@@ -34,14 +34,14 @@ func NewGTROM(base Base) bus.Mapper {
 	m.AddWriteHook(0x5000, 0x5FFF, m.setBanks)
 	m.AddWriteHook(0x7000, 0x7FFF, m.setBanks)
 
-	return m
+	return m, nil
 }
 
-func (m *mapperGTROM) getControl(_ uint16) uint8 {
-	return 0 // TODO should return open bus value
+func (m *mapperGTROM) getControl(_ uint16) (uint8, error) {
+	return 0, nil // TODO should return open bus value
 }
 
-func (m *mapperGTROM) setBanks(_ uint16, value uint8) {
+func (m *mapperGTROM) setBanks(_ uint16, value uint8) error {
 	prgBank := value & 0b0000_1111
 
 	m.SetPrgWindow(0, int(prgBank)) // select 32 KB PRG ROM bank for CPU $8000-$FFFF
@@ -51,4 +51,5 @@ func (m *mapperGTROM) setBanks(_ uint16, value uint8) {
 
 	nameTableBank := int(value>>5) & 1
 	m.SetNameTableWindow(nameTableBank)
+	return nil
 }

pkg/mapper/mapperdb/gtrom_test.go

@@ -20,7 +20,8 @@ func TestMapperGTROM(t *testing.T) {
 		},
 		NameTable: nameTable,
 	})
-	m := NewGTROM(base)
+	m, err := NewGTROM(base)
+	assert.NoError(t, err)
 
 	chr := make([]byte, 0x4000)
 	base.SetChrRAM(chr)

pkg/mapper/mapperdb/mmc1.go

@@ -31,7 +31,7 @@ type mapperMMC1 struct {
 }
 
 // NewMMC1 returns a new mapper instance.
-func NewMMC1(base Base) bus.Mapper {
+func NewMMC1(base Base) (bus.Mapper, error) {
 	m := &mapperMMC1{
 		Base: base,
 		ram:  make([]byte, 0x8000), // 32K
@@ -55,20 +55,22 @@ func NewMMC1(base Base) bus.Mapper {
 
 	// TODO support mmc1 variants
 
-	return m
+	return m, nil
 }
 
-func (m *mapperMMC1) resetShift() {
+func (m *mapperMMC1) resetShift() error {
 	m.shiftCount = 0
 	m.shiftRegister = 0
-	m.applyControl()
+	if err := m.applyControl(); err != nil {
+		return err
+	}
 	m.control |= 0x0C
+	return nil
 }
 
-func (m *mapperMMC1) writeShiftBit(address uint16, value uint8) {
+func (m *mapperMMC1) writeShiftBit(address uint16, value uint8) error {
 	if value&0x80 != 0 {
-		m.resetShift()
-		return
+		return m.resetShift()
 	}
 
 	// the shift register gets written from lowest to highest bit
@@ -77,7 +79,7 @@ func (m *mapperMMC1) writeShiftBit(address uint16, value uint8) {
 
 	m.shiftCount++
 	if m.shiftCount < 5 {
-		return
+		return nil
 	}
 
 	switch {
@@ -94,12 +96,14 @@ func (m *mapperMMC1) writeShiftBit(address uint16, value uint8) {
 		m.prgBank = int(m.shiftRegister) & 0b0000_1111
 	}
 
-	m.resetShift()
+	return m.resetShift()
 }
 
-func (m *mapperMMC1) applyControl() {
+func (m *mapperMMC1) applyControl() error {
 	mirrorMode := m.control & 0b0000_0011
-	m.SetNameTableMirrorModeIndex(mirrorMode)
+	if err := m.SetNameTableMirrorModeIndex(mirrorMode); err != nil {
+		return err
+	}
 
 	prgMode := (m.control >> 2) & 0b0000_0011
 	switch prgMode {
@@ -129,4 +133,5 @@ func (m *mapperMMC1) applyControl() {
 		// switch two separate 4 KB banks
 		m.SetChrWindow(1, m.chrBank1)
 	}
+	return nil
 }

pkg/mapper/mapperdb/mmc1_test.go

@@ -21,7 +21,8 @@ func TestMapperMMC1(t *testing.T) {
 		},
 		NameTable: nametable.New(cartridge.MirrorHorizontal),
 	})
-	m := NewMMC1(base)
+	m, err := NewMMC1(base)
+	assert.NoError(t, err)
 
 	chr[0x0000] = 0x01
 	chr[0x2000] = 0x02

pkg/mapper/mapperdb/nrom.go

@@ -15,11 +15,11 @@ type mapperNROM struct {
 }
 
 // NewNROM returns a new mapper instance.
-func NewNROM(base Base) bus.Mapper {
+func NewNROM(base Base) (bus.Mapper, error) {
 	m := &mapperNROM{
 		Base: base,
 	}
 	m.SetName("NROM")
 	m.Initialize()
-	return m
+	return m, nil
 }

pkg/mapper/mapperdb/nrom_test.go

@@ -21,7 +21,8 @@ func TestMapperNROMPrg16k(t *testing.T) {
 		},
 		NameTable: nametable.New(cartridge.MirrorHorizontal),
 	})
-	m := NewNROM(base)
+	m, err := NewNROM(base)
+	assert.NoError(t, err)
 
 	chr[0x0001] = 0x02 // bank 0
 	assert.Equal(t, 0x02, m.Read(0x0001))
@@ -42,7 +43,8 @@ func TestMapperNROMPrg32k(t *testing.T) {
 		},
 		NameTable: nametable.New(cartridge.MirrorHorizontal),
 	})
-	m := NewNROM(base)
+	m, err := NewNROM(base)
+	assert.NoError(t, err)
 
 	chr[0x0001] = 0x02 // bank 0
 	assert.Equal(t, 0x02, m.Read(0x0001))