chore: remove useless default implementations

src/apu.rs

@@ -211,8 +211,8 @@ impl Apu {
         self.ch4.poly = Default::default();
         self.ch4.freq = Default::default();
 
-        self.ctrl.channel = Default::default();
-        self.ctrl.out = Default::default();
+        self.ctrl.channel = ChannelControl(0);
+        self.ctrl.out = SoundOutput(0);
 
         // Disable the Channels
         self.ch1.enabled = Default::default();
@@ -344,7 +344,7 @@ impl Apu {
     }
 }
 
-#[derive(Debug, Default)]
+#[derive(Debug)]
 pub(crate) struct SoundControl {
     /// 0xFF24 | NR50 - Channel Control
     channel: ChannelControl,
@@ -354,6 +354,16 @@ pub(crate) struct SoundControl {
     enabled: bool,
 }
 
+impl Default for SoundControl {
+    fn default() -> Self {
+        Self {
+            channel: ChannelControl(0),
+            out: SoundOutput(0),
+            enabled: Default::default(),
+        }
+    }
+}
+
 impl SoundControl {
     /// 0xFF24 | NR50 - Channel Control
     pub(crate) fn channel(&self) -> u8 {

src/apu/types.rs

@@ -416,12 +416,6 @@ pub(super) mod common {
         }
     }
 
-    impl Default for WavePattern {
-        fn default() -> Self {
-            Self::OneEighth // Rationale: OneEighth is 0x00
-        }
-    }
-
     impl From<WavePattern> for u8 {
         fn from(pattern: WavePattern) -> Self {
             pattern as Self
@@ -479,12 +473,6 @@ impl Clone for SoundOutput {
     }
 }
 
-impl Default for SoundOutput {
-    fn default() -> Self {
-        Self(0)
-    }
-}
-
 impl From<u8> for SoundOutput {
     fn from(byte: u8) -> Self {
         Self(byte)
@@ -523,12 +511,6 @@ impl Clone for ChannelControl {
     }
 }
 
-impl Default for ChannelControl {
-    fn default() -> Self {
-        Self(0)
-    }
-}
-
 impl From<u8> for ChannelControl {
     fn from(byte: u8) -> Self {
         Self(byte)

src/cartridge.rs

@@ -6,66 +6,66 @@ use crate::Cycle;
 
 const RAM_SIZE_ADDRESS: usize = 0x0149;
 const ROM_SIZE_ADDRESS: usize = 0x0148;
-const MBC_TYPE_ADDRESS: usize = 0x0147;
+const MBC_KIND_ADDRESS: usize = 0x0147;
 const ROM_TITLE_START: usize = 0x134;
 const ROM_TITLE_MAX_SIZE: usize = 16;
 const ROM_MANUFACTURER_START: usize = 0x13F;
 
-#[derive(Debug, Default)]
+#[derive(Debug)]
 pub(crate) struct Cartridge {
-    memory: Vec<u8>,
+    mem: Vec<u8>,
     pub(crate) title: Option<String>,
     mbc: Box<dyn MBCIo>,
 }
 
 impl Cartridge {
-    pub(crate) fn new(memory: Vec<u8>) -> Self {
-        let title_mem: &[u8; 16] = memory[ROM_TITLE_START..(ROM_TITLE_START + ROM_TITLE_MAX_SIZE)]
+    pub(crate) fn new(mem: Vec<u8>) -> Self {
+        let title_mem: &[u8; 16] = mem[ROM_TITLE_START..(ROM_TITLE_START + ROM_TITLE_MAX_SIZE)]
             .try_into()
             .expect("coerce slice containing cartridge title from ROM to [u8; 16]");
+
         let title = Self::detect_title(title_mem);
+        let mbc = Self::detect_mbc(&mem);
         tracing::info!("Title: {:?}", title);
 
-        Self {
-            mbc: Self::detect_mbc(&memory),
-            title,
-            memory,
-        }
+        Self { mem, title, mbc }
     }
 
     pub(crate) fn ext_ram(&self) -> Option<&[u8]> {
         self.mbc.ext_ram()
     }
 
-    pub(crate) fn write_ext_ram(&mut self, memory: Vec<u8>) {
-        self.mbc.write_ext_ram(memory)
+    pub(crate) fn ext_ram_mut(&mut self) -> Option<&mut [u8]> {
+        self.mbc.ext_ram_mut()
     }
 
+    #[inline]
     pub(crate) fn tick(&mut self) {
         self.mbc.tick()
     }
 
-    fn detect_mbc(memory: &[u8]) -> Box<dyn MBCIo> {
-        let ram_size = Self::detect_ram_info(memory);
-        let rom_size = Self::detect_rom_info(memory);
-        let mbc_kind = Self::find_mbc(memory);
+    fn detect_mbc(mem: &[u8]) -> Box<dyn MBCIo> {
+        let ram_size: RamSize = mem[RAM_SIZE_ADDRESS].into();
+        let rom_size: RomSize = mem[ROM_SIZE_ADDRESS].into();
+        let mbc_kind = Self::detect_mbc_kind(mem[MBC_KIND_ADDRESS]);
         let ram_cap = ram_size.capacity();
         let rom_cap = rom_size.capacity();
 
         tracing::info!("RAM size: {} bytes", ram_cap);
-        tracing::info!("ROM size: {} bytes", rom_size.capacity());
+        tracing::info!("ROM size: {} bytes", rom_cap);
         tracing::info!("MBC kind: {:?}", mbc_kind);
 
         match mbc_kind {
             MBCKind::None => Box::new(NoMBC),
-            MBCKind::MBC1 => Box::new(MBC1::new(ram_size, rom_size)),
-            MBCKind::MBC1WithBattery => Box::new(MBC1::with_battery(ram_size, rom_size)),
-            MBCKind::MBC2 => Box::new(MBC2::new(rom_cap)),
-            MBCKind::MBC2WithBattery => Box::new(MBC2::with_battery(rom_cap)),
-            MBCKind::MBC3 => Box::new(MBC3::new(ram_cap)),
-            MBCKind::MBC3WithBattery => Box::new(MBC3::with_battery(ram_cap)),
-            MBCKind::MBC5 => Box::new(MBC5::new(ram_cap, rom_cap)),
-            MBCKind::MBC5WithBattery => Box::new(MBC5::with_battery(ram_cap, rom_cap)),
+            MBCKind::MBC1(hw) => Box::new(MBC1::new(hw, ram_size, rom_size)),
+            MBCKind::MBC2(hw) => Box::new(MBC2::new(hw, rom_cap)),
+            MBCKind::MBC3(hw @ MBC3Hardware::RTC) => Box::new(MBC3::new(hw, ram_cap)),
+            MBCKind::MBC3(hw @ MBC3Hardware::RTCAndBatteryRAM) => Box::new(MBC3::new(hw, ram_cap)),
+            MBCKind::MBC5(hw @ MBC5Hardware::None) => Box::new(MBC5::new(hw, ram_cap, rom_cap)),
+            MBCKind::MBC5(hw @ MBC5Hardware::BatteryRAM) => {
+                Box::new(MBC5::new(hw, ram_cap, rom_cap))
+            }
+            kind => todo!("ROMS with {:?} are currently unsupported", kind),
         }
     }
 
@@ -88,30 +88,30 @@ impl Cartridge {
         }
     }
 
-    fn detect_ram_info(memory: &[u8]) -> RamSize {
-        let id = memory[RAM_SIZE_ADDRESS];
-        id.into()
-    }
-
-    fn detect_rom_info(memory: &[u8]) -> RomSize {
-        let id = memory[ROM_SIZE_ADDRESS];
-        id.into()
-    }
-
-    fn find_mbc(memory: &[u8]) -> MBCKind {
+    fn detect_mbc_kind(id: u8) -> MBCKind {
         use MBCKind::*;
 
-        match memory[MBC_TYPE_ADDRESS] {
+        match id {
             0x00 => None,
-            0x01 | 0x02 => MBC1,
-            0x03 => MBC1WithBattery,
-            0x05 => MBC2,
-            0x06 => MBC2WithBattery,
-            0x19 | 0x1A => MBC5,
-            0x1B => MBC5WithBattery,
-            0x13 => MBC3WithBattery,
-            0x11 | 0x12 => MBC3,
-            id => unimplemented!("id {:#04X} is an unsupported MBC", id),
+            0x01 => MBC1(MBC1Hardware::None),
+            0x02 => MBC1(MBC1Hardware::RAM),
+            0x03 => MBC1(MBC1Hardware::BatteryRAM),
+            0x05 => MBC2(MBC2Hardware::None),
+            0x06 => MBC2(MBC2Hardware::BatteryRAM),
+            0x08 | 0x09 => unimplemented!("NoMBC + RAM and NoMBC + Battery unsupported"),
+            0x0B | 0x0C | 0x0D => unimplemented!("MM01 unsupported"),
+            0x0F => MBC3(MBC3Hardware::RTC),
+            0x10 => MBC3(MBC3Hardware::RTCAndBatteryRAM),
+            0x11 => MBC3(MBC3Hardware::None),
+            0x12 => MBC3(MBC3Hardware::RAM),
+            0x13 => MBC3(MBC3Hardware::BatteryRAM),
+            0x19 => MBC5(MBC5Hardware::None),
+            0x1A => MBC5(MBC5Hardware::RAM),
+            0x1B => MBC5(MBC5Hardware::BatteryRAM),
+            0x1C => MBC5(MBC5Hardware::Rumble),
+            0x1D => MBC5(MBC5Hardware::RumbleRAM),
+            0x1E => MBC5(MBC5Hardware::RumbleBatteryRAM),
+            id => unimplemented!("MBC with code {:#04X} is unsupported", id),
         }
     }
 }
@@ -121,8 +121,8 @@ impl BusIo for Cartridge {
         use MBCResult::*;
 
         match self.mbc.handle_read(addr) {
-            Address(addr) => self.memory[addr],
-            Value(byte) => byte,
+            Addr(addr) => self.mem[addr],
+            Byte(byte) => byte,
         }
     }
 
@@ -139,37 +139,24 @@ struct MBC1 {
     ram_bank: u8,
     mode: bool,
     ram_size: RamSize,
-    memory: Vec<u8>,
+    mem: Vec<u8>,
     rom_size: RomSize,
     mem_enabled: bool,
 
-    has_battery: bool,
+    hw: MBC1Hardware,
 }
 
 impl MBC1 {
-    fn new(ram_size: RamSize, rom_size: RomSize) -> Self {
+    fn new(hw: MBC1Hardware, ram_size: RamSize, rom_size: RomSize) -> Self {
         Self {
             rom_bank: 0x01,
-            memory: vec![0; ram_size.capacity() as usize],
+            mem: vec![0; ram_size.capacity() as usize],
             ram_size,
             rom_size,
             ram_bank: Default::default(),
             mode: Default::default(),
             mem_enabled: Default::default(),
-            has_battery: Default::default(),
-        }
-    }
-
-    fn with_battery(ram_size: RamSize, rom_size: RomSize) -> Self {
-        Self {
-            rom_bank: 0x01,
-            memory: vec![0; ram_size.capacity() as usize],
-            ram_size,
-            rom_size,
-            ram_bank: Default::default(),
-            mode: Default::default(),
-            mem_enabled: Default::default(),
-            has_battery: true,
+            hw,
         }
     }
 
@@ -240,15 +227,16 @@ impl MBC1 {
 
 impl Savable for MBC1 {
     fn ext_ram(&self) -> Option<&[u8]> {
-        match self.has_battery {
-            true => Some(&self.memory),
-            false => None,
+        match self.hw {
+            MBC1Hardware::BatteryRAM => Some(&self.mem),
+            _ => None,
         }
     }
 
-    fn write_ext_ram(&mut self, memory: Vec<u8>) {
-        if self.has_battery {
-            self.memory.copy_from_slice(&memory);
+    fn ext_ram_mut(&mut self) -> Option<&mut [u8]> {
+        match self.hw {
+            MBC1Hardware::BatteryRAM => Some(&mut self.mem),
+            _ => None,
         }
     }
 }
@@ -259,16 +247,12 @@ impl MBCIo for MBC1 {
 
         match addr {
             0x0000..=0x3FFF if self.mode => {
-                Address(0x4000 * self.zero_bank() as usize + addr as usize)
+                Addr(0x4000 * self.zero_bank() as usize + addr as usize)
             }
-            0x0000..=0x3FFF => Address(addr as usize),
-            0x4000..=0x7FFF => {
-                Address(0x4000 * self.high_bank() as usize + (addr as usize - 0x4000))
-            }
-            0xA000..=0xBFFF if self.mem_enabled && self.ram_size != RamSize::None => {
-                Value(self.memory[self.ram_addr(addr)])
-            }
-            0xA000..=0xBFFF => Value(0xFF),
+            0x0000..=0x3FFF => Addr(addr as usize),
+            0x4000..=0x7FFF => Addr(0x4000 * self.high_bank() as usize + (addr as usize - 0x4000)),
+            0xA000..=0xBFFF if self.mem_enabled => Byte(self.mem[self.ram_addr(addr)]),
+            0xA000..=0xBFFF => Byte(0xFF),
             _ => unreachable!("A read from {:#06X} should not be handled by MBC1", addr),
         }
     }
@@ -283,9 +267,9 @@ impl MBCIo for MBC1 {
             }
             0x4000..=0x5FFF => self.ram_bank = byte & 0x03,
             0x6000..=0x7FFF => self.mode = (byte & 0x01) == 0x01,
-            0xA000..=0xBFFF if self.mem_enabled && self.ram_size != RamSize::None => {
+            0xA000..=0xBFFF if self.mem_enabled => {
                 let ram_addr = self.ram_addr(addr);
-                self.memory[ram_addr] = byte;
+                self.mem[ram_addr] = byte;
             }
             0xA000..=0xBFFF => {} // Ram isn't enabled, ignored write
             _ => unreachable!("A write to {:#06X} should not be handled by MBC1", addr),
@@ -313,7 +297,7 @@ struct RtClock {
 
 impl RtClock {
     fn inc_day(&mut self) {
-        // TODO: Figure out order of operations, the brackets are a bit too defenseive here
+        // TODO: Figure out order of operations, the brackets are a bit too defensive here
         let days: u16 = (((self.day_high.ninth() as u16) << 8) | self.day_low as u16) + 1;
 
         if days > 0x1FF {
@@ -418,34 +402,20 @@ struct MBC3 {
 
     devs_enabled: bool,
     mapped: Option<MBC3Device>,
-    memory: Vec<u8>,
+    mem: Vec<u8>,
 
     // RTC Data Latch Previous Write
     prev_latch_write: Option<u8>,
 
-    has_battery: bool,
+    hw: MBC3Hardware,
     rtc: RtClock,
     rtc_latch: Option<RtClock>,
 }
 
 impl MBC3 {
-    fn new(ram_cap: usize) -> Self {
+    fn new(hw: MBC3Hardware, ram_cap: usize) -> Self {
         Self {
-            memory: vec![0; ram_cap],
-            rom_bank: Default::default(),
-            ram_bank: Default::default(),
-            devs_enabled: Default::default(),
-            mapped: Default::default(),
-            prev_latch_write: Default::default(),
-            has_battery: Default::default(),
-            rtc: Default::default(),
-            rtc_latch: Default::default(),
-        }
-    }
-
-    fn with_battery(ram_cap: usize) -> Self {
-        Self {
-            memory: vec![0; ram_cap],
+            mem: vec![0; ram_cap],
             rom_bank: Default::default(),
             ram_bank: Default::default(),
             devs_enabled: Default::default(),
@@ -453,22 +423,23 @@ impl MBC3 {
             prev_latch_write: Default::default(),
             rtc: Default::default(),
             rtc_latch: Default::default(),
-            has_battery: true,
+            hw,
         }
     }
 }
 
 impl Savable for MBC3 {
     fn ext_ram(&self) -> Option<&[u8]> {
-        match self.has_battery {
-            true => Some(&self.memory),
-            false => None,
+        match self.hw {
+            MBC3Hardware::BatteryRAM | MBC3Hardware::RTCAndBatteryRAM => Some(&self.mem),
+            _ => None,
         }
     }
 
-    fn write_ext_ram(&mut self, memory: Vec<u8>) {
-        if self.has_battery {
-            self.memory.copy_from_slice(&memory);
+    fn ext_ram_mut(&mut self) -> Option<&mut [u8]> {
+        match self.hw {
+            MBC3Hardware::BatteryRAM | MBC3Hardware::RTCAndBatteryRAM => Some(&mut self.mem),
+            _ => None,
         }
     }
 }
@@ -479,13 +450,13 @@ impl MBCIo for MBC3 {
         use RtcRegister::*;
 
         let res = match addr {
-            0x0000..=0x3FFF => Address(addr as usize),
-            0x4000..=0x7FFF => Address(0x4000 * self.rom_bank as usize + (addr as usize - 0x4000)),
+            0x0000..=0x3FFF => Addr(addr as usize),
+            0x4000..=0x7FFF => Addr(0x4000 * self.rom_bank as usize + (addr as usize - 0x4000)),
             0xA000..=0xBFFF => match self.mapped {
                 Some(MBC3Device::ExternalRam) if self.devs_enabled => {
-                    Value(self.memory[0x2000 * self.ram_bank as usize + (addr as usize - 0xA000)])
+                    Byte(self.mem[0x2000 * self.ram_bank as usize + (addr as usize - 0xA000)])
                 }
-                Some(MBC3Device::Clock(reg)) if self.devs_enabled => Value(
+                Some(MBC3Device::Clock(reg)) if self.devs_enabled => Byte(
                     self.rtc_latch
                         .as_ref()
                         .map(|rtc| match reg {
@@ -497,7 +468,7 @@ impl MBCIo for MBC3 {
                         })
                         .unwrap_or(0xFF),
                 ),
-                _ => Value(0xFF),
+                _ => Byte(0xFF),
             },
             _ => unreachable!("A read from {:#06X} should not be handled by MBC3", addr),
         };
@@ -539,7 +510,7 @@ impl MBCIo for MBC3 {
             }
             0xA000..=0xBFFF => match self.mapped {
                 Some(MBC3Device::ExternalRam) if self.devs_enabled => {
-                    self.memory[0x2000 * self.ram_bank as usize + (addr as usize - 0xA000)] = byte
+                    self.mem[0x2000 * self.ram_bank as usize + (addr as usize - 0xA000)] = byte
                 }
                 Some(MBC3Device::Clock(rtc_reg)) if self.devs_enabled => match rtc_reg {
                     Second => {
@@ -561,7 +532,9 @@ impl MBCIo for MBC3 {
 
 impl RtClockTick for MBC3 {
     fn tick(&mut self) {
-        self.rtc.tick();
+        if let MBC3Hardware::RTCAndBatteryRAM | MBC3Hardware::RTC = self.hw {
+            self.rtc.tick();
+        }
     }
 }
 
@@ -573,32 +546,21 @@ struct MBC5 {
     ram_bank: u8,
 
     rom_cap: usize,
-    memory: Vec<u8>,
+    mem: Vec<u8>,
     mem_enabled: bool,
 
-    has_battery: bool,
+    hw: MBC5Hardware,
 }
 
 impl MBC5 {
-    fn new(ram_cap: usize, rom_cap: usize) -> Self {
+    fn new(hw: MBC5Hardware, ram_cap: usize, rom_cap: usize) -> Self {
         Self {
             rom_bank: 0x01,
-            memory: vec![0; ram_cap],
+            mem: vec![0; ram_cap],
             rom_cap,
             ram_bank: Default::default(),
             mem_enabled: Default::default(),
-            has_battery: Default::default(),
-        }
-    }
-
-    fn with_battery(ram_cap: usize, rom_cap: usize) -> Self {
-        Self {
-            rom_bank: 0x01,
-            memory: vec![0; ram_cap],
-            rom_cap,
-            ram_bank: Default::default(),
-            mem_enabled: Default::default(),
-            has_battery: true,
+            hw,
         }
     }
 
@@ -609,15 +571,16 @@ impl MBC5 {
 
 impl Savable for MBC5 {
     fn ext_ram(&self) -> Option<&[u8]> {
-        match self.has_battery {
-            true => Some(&self.memory),
-            false => None,
+        match self.hw {
+            MBC5Hardware::RumbleBatteryRAM | MBC5Hardware::BatteryRAM => Some(&self.mem),
+            _ => None,
         }
     }
 
-    fn write_ext_ram(&mut self, memory: Vec<u8>) {
-        if self.has_battery {
-            self.memory.copy_from_slice(&memory);
+    fn ext_ram_mut(&mut self) -> Option<&mut [u8]> {
+        match self.hw {
+            MBC5Hardware::RumbleBatteryRAM | MBC5Hardware::BatteryRAM => Some(&mut self.mem),
+            _ => None,
         }
     }
 }
@@ -627,12 +590,12 @@ impl MBCIo for MBC5 {
         use MBCResult::*;
 
         match addr {
-            0x0000..=0x3FFF => Address(addr as usize),
-            0x4000..=0x7FFF => Address(self.bank_addr(addr)),
+            0x0000..=0x3FFF => Addr(addr as usize),
+            0x4000..=0x7FFF => Addr(self.bank_addr(addr)),
             0xA000..=0xBFFF if self.mem_enabled => {
-                Value(self.memory[0x2000 * self.ram_bank as usize + (addr as usize - 0xA000)])
+                Byte(self.mem[0x2000 * self.ram_bank as usize + (addr as usize - 0xA000)])
             }
-            0xA000..=0xBFFF => Value(0xFF),
+            0xA000..=0xBFFF => Byte(0xFF),
             _ => unreachable!("A read from {:#06X} should not be handled by MBC5", addr),
         }
     }
@@ -644,7 +607,7 @@ impl MBCIo for MBC5 {
             0x3000..=0x3FFF => self.rom_bank = (self.rom_bank & 0x00FF) | (byte as u16 & 0x01) << 8,
             0x4000..=0x5FFF => self.ram_bank = byte & 0x0F,
             0xA000..=0xBFFF if self.mem_enabled => {
-                self.memory[0x2000 * self.ram_bank as usize + (addr as usize - 0xA000)] = byte;
+                self.mem[0x2000 * self.ram_bank as usize + (addr as usize - 0xA000)] = byte;
             }
             0xA000..=0xBFFF => {}
             _ => unreachable!("A write to {:#06X} should not be handled by MBC5", addr),
@@ -660,33 +623,23 @@ impl RtClockTick for MBC5 {
 struct MBC2 {
     /// 4-bit number
     rom_bank: u8,
-    memory: Box<[u8; Self::RAM_SIZE]>,
-    mem_enabled: bool,
+    mem: Box<[u8; Self::RAM_SIZE]>,
 
+    mem_enabled: bool,
     rom_cap: usize,
-    has_battery: bool,
+    hw: MBC2Hardware,
 }
 
 impl MBC2 {
     const RAM_SIZE: usize = 0x0200;
 
-    fn new(rom_cap: usize) -> Self {
+    fn new(hw: MBC2Hardware, rom_cap: usize) -> Self {
         Self {
             rom_bank: 0x01,
-            memory: Box::new([0; Self::RAM_SIZE]),
+            mem: Box::new([0; Self::RAM_SIZE]),
             rom_cap,
             mem_enabled: Default::default(),
-            has_battery: Default::default(),
-        }
-    }
-
-    fn with_battery(rom_cap: usize) -> Self {
-        Self {
-            rom_bank: 0x01,
-            memory: Box::new([0; Self::RAM_SIZE]),
-            rom_cap,
-            mem_enabled: Default::default(),
-            has_battery: true,
+            hw,
         }
     }
 
@@ -697,15 +650,16 @@ impl MBC2 {
 
 impl Savable for MBC2 {
     fn ext_ram(&self) -> Option<&[u8]> {
-        match self.has_battery {
-            true => Some(self.memory.as_ref()),
-            false => None,
+        match self.hw {
+            MBC2Hardware::BatteryRAM => Some(self.mem.as_ref()),
+            MBC2Hardware::None => None,
         }
     }
 
-    fn write_ext_ram(&mut self, memory: Vec<u8>) {
-        if self.has_battery {
-            self.memory.copy_from_slice(&memory);
+    fn ext_ram_mut(&mut self) -> Option<&mut [u8]> {
+        match self.hw {
+            MBC2Hardware::BatteryRAM => Some(self.mem.as_mut()),
+            MBC2Hardware::None => None,
         }
     }
 }
@@ -715,13 +669,13 @@ impl MBCIo for MBC2 {
         use MBCResult::*;
 
         match addr {
-            0x0000..=0x3FFF => Address(addr as usize),
-            0x4000..=0x7FFF => Address(self.rom_addr(addr)),
+            0x0000..=0x3FFF => Addr(addr as usize),
+            0x4000..=0x7FFF => Addr(self.rom_addr(addr)),
             0xA000..=0xBFFF if self.mem_enabled => {
                 let mbc2_addr = addr as usize & (Self::RAM_SIZE - 1);
-                Value(self.memory[mbc2_addr] | 0xF0)
+                Byte(self.mem[mbc2_addr] | 0xF0)
             }
-            0xA000..=0xBFFF => Value(0xFF),
+            0xA000..=0xBFFF => Byte(0xFF),
             _ => unreachable!("A read from {:#06X} should not be handled by MBC2", addr),
         }
     }
@@ -736,7 +690,7 @@ impl MBCIo for MBC2 {
             0x0000..=0x3FFF => self.mem_enabled = nybble == 0x0A,
             0xA000..=0xBFFF if self.mem_enabled => {
                 let mbc2_addr = addr as usize & (Self::RAM_SIZE - 1);
-                self.memory[mbc2_addr] = nybble;
+                self.mem[mbc2_addr] = nybble;
             }
             0x4000..=0x7FFF | 0xA000..=0xBFFF => {}
             _ => unreachable!("A write to {:#06X} should not be handled by MBC2", addr),
@@ -756,14 +710,14 @@ impl Savable for NoMBC {
         None
     }
 
-    fn write_ext_ram(&mut self, _memory: Vec<u8>) {
-        // Nothing Happens Here
+    fn ext_ram_mut(&mut self) -> Option<&mut [u8]> {
+        None
     }
 }
 
 impl MBCIo for NoMBC {
     fn handle_read(&self, addr: u16) -> MBCResult {
-        MBCResult::Address(addr as usize)
+        MBCResult::Addr(addr as usize)
     }
 
     fn handle_write(&mut self, _: u16, byte: u8) {
@@ -782,27 +736,49 @@ trait MBCIo: Savable + RtClockTick {
 
 #[derive(Debug, Clone, Copy)]
 enum MBCResult {
-    Address(usize),
-    Value(u8),
+    Addr(usize),
+    Byte(u8),
 }
 
 #[derive(Debug, Clone, Copy)]
 enum MBCKind {
     None,
-    MBC1,
-    MBC1WithBattery,
-    MBC2,
-    MBC2WithBattery,
-    MBC3,
-    MBC3WithBattery,
-    MBC5,
-    MBC5WithBattery,
+    MBC1(MBC1Hardware),
+    MBC2(MBC2Hardware),
+    MBC3(MBC3Hardware),
+    MBC5(MBC5Hardware),
 }
 
-impl Default for MBCKind {
-    fn default() -> Self {
-        Self::None
-    }
+#[derive(Debug, Clone, Copy)]
+enum MBC1Hardware {
+    None,
+    RAM,
+    BatteryRAM,
+}
+
+#[derive(Debug, Clone, Copy)]
+enum MBC2Hardware {
+    None,
+    BatteryRAM,
+}
+
+#[derive(Debug, Clone, Copy)]
+enum MBC3Hardware {
+    RTC,
+    RTCAndBatteryRAM,
+    None,
+    RAM,
+    BatteryRAM,
+}
+
+#[derive(Debug, Clone, Copy)]
+enum MBC5Hardware {
+    None,
+    RAM,
+    BatteryRAM,
+    Rumble,
+    RumbleRAM,
+    RumbleBatteryRAM,
 }
 
 #[derive(Debug, Clone, Copy, PartialEq)]
@@ -830,12 +806,6 @@ impl RamSize {
     }
 }
 
-impl Default for RamSize {
-    fn default() -> Self {
-        Self::None
-    }
-}
-
 impl From<u8> for RamSize {
     fn from(byte: u8) -> Self {
         use RamSize::*;
@@ -910,15 +880,9 @@ impl std::fmt::Debug for Box<dyn MBCIo> {
     }
 }
 
-impl Default for Box<dyn MBCIo> {
-    fn default() -> Self {
-        Box::new(NoMBC)
-    }
-}
-
 trait Savable {
     fn ext_ram(&self) -> Option<&[u8]>;
-    fn write_ext_ram(&mut self, memory: Vec<u8>);
+    fn ext_ram_mut(&mut self) -> Option<&mut [u8]>;
 }
 
 #[cfg(test)]

src/cpu.rs

@@ -19,9 +19,9 @@ impl Cpu {
         Self {
             bus: Bus::with_boot(rom),
             reg: Default::default(),
-            flags: Default::default(),
-            ime: Default::default(),
-            state: Default::default(),
+            flags: Flags(0),
+            ime: ImeState::Disabled,
+            state: State::Execute,
         }
     }
 
@@ -238,12 +238,6 @@ enum State {
     // Stop,
 }
 
-impl Default for State {
-    fn default() -> Self {
-        Self::Execute
-    }
-}
-
 impl Cpu {
     pub(crate) fn set_register(&mut self, register: Register, value: u8) {
         use Register::*;
@@ -437,12 +431,6 @@ impl Clone for Flags {
     }
 }
 
-impl Default for Flags {
-    fn default() -> Self {
-        Self(0)
-    }
-}
-
 impl Display for Flags {
     fn fmt(&self, f: &mut Formatter<'_>) -> FmtResult {
         if self.z() {
@@ -497,9 +485,3 @@ pub(crate) enum ImeState {
     Pending,
     Enabled,
 }
-
-impl Default for ImeState {
-    fn default() -> Self {
-        Self::Disabled
-    }
-}

src/emu.rs

@@ -70,6 +70,7 @@ pub fn write_save(cpu: &Cpu) {
         Some(cart) => match write_save_to_file(cart) {
             Ok(path) => tracing::info!("Wrote to save at {:?}", path),
             Err(err @ SaveError::NotApplicable) => tracing::warn!("Unable to Save: {:?}", err),
+            Err(SaveError::DiffSize) => unreachable!(),
             Err(SaveError::Io(err)) => tracing::error!("{:?}", err),
         },
         None => tracing::error!("No cartridge is currently present"),
@@ -80,7 +81,8 @@ pub fn load_save(cpu: &mut Cpu) {
     match cpu.bus.cart.as_mut() {
         Some(cart) => match read_save_from_file(cart) {
             Ok(path) => tracing::info!("Loaded save from {:?}", path),
-            Err(err @ SaveError::NotApplicable) => tracing::warn!("Unable to load save: {:?}", err),
+            Err(err @ SaveError::NotApplicable) => tracing::warn!("Unable to load save: {}", err),
+            Err(err @ SaveError::DiffSize) => tracing::error!("Unable to load save: {}", err),
             Err(SaveError::Io(err)) => match err.kind() {
                 std::io::ErrorKind::NotFound => tracing::warn!("Save not found"),
                 _ => tracing::error!("{:?}", err),
@@ -106,8 +108,8 @@ fn write_save_to_file(cart: &Cartridge) -> Result<PathBuf, SaveError> {
 }
 
 fn read_save_from_file(cart: &mut Cartridge) -> Result<PathBuf, SaveError> {
-    match cart.title.as_deref() {
-        Some(title) => {
+    match cart.title.clone().zip(cart.ext_ram_mut()) {
+        Some((title, ext_ram)) => {
             let mut save_path = data_path().unwrap_or_else(|| PathBuf::from("."));
             save_path.push(title);
             save_path.set_extension("sav");
@@ -116,10 +118,11 @@ fn read_save_from_file(cart: &mut Cartridge) -> Result<PathBuf, SaveError> {
             let mut memory = Vec::new();
             file.read_to_end(&mut memory)?;
 
-            // FIXME: We call this whether we can write to Ext RAM or not.
-            // We should add a check that ensures that by this point we know whether
-            // the cartridge has external RAM or not.
-            cart.write_ext_ram(memory);
+            if ext_ram.len() != memory.len() {
+                return Err(SaveError::DiffSize);
+            }
+
+            ext_ram.copy_from_slice(&memory);
             Ok(save_path)
         }
         None => Err(SaveError::NotApplicable),
@@ -151,4 +154,6 @@ pub enum SaveError {
     NotApplicable,
     #[error(transparent)]
     Io(#[from] std::io::Error),
+    #[error("save file size differs from external ram")]
+    DiffSize,
 }

src/interrupt.rs

@@ -1,11 +1,20 @@
 use bitfield::bitfield;
 
-#[derive(Debug, Default)]
+#[derive(Debug)]
 pub(crate) struct Interrupt {
     pub(crate) flag: InterruptFlag,
     pub(crate) enable: InterruptEnable,
 }
 
+impl Default for Interrupt {
+    fn default() -> Self {
+        Self {
+            flag: InterruptFlag(0),
+            enable: InterruptEnable(0),
+        }
+    }
+}
+
 bitfield! {
    pub struct InterruptEnable(u8);
     impl Debug;
@@ -23,12 +32,6 @@ impl Clone for InterruptEnable {
     }
 }
 
-impl Default for InterruptEnable {
-    fn default() -> Self {
-        Self(0)
-    }
-}
-
 impl From<u8> for InterruptEnable {
     fn from(byte: u8) -> Self {
         Self(byte)
@@ -58,12 +61,6 @@ impl Clone for InterruptFlag {
     }
 }
 
-impl Default for InterruptFlag {
-    fn default() -> Self {
-        Self(0)
-    }
-}
-
 impl From<u8> for InterruptFlag {
     fn from(byte: u8) -> Self {
         Self(byte)

src/ppu.rs

@@ -470,10 +470,10 @@ impl Default for Ppu {
             dot: Default::default(),
             frame_buf: Box::new([0; GB_WIDTH * GB_HEIGHT * 4]),
             int: Default::default(),
-            ctrl: Default::default(),
+            ctrl: LCDControl(0),
             monochrome: Default::default(),
             pos: Default::default(),
-            stat: Default::default(),
+            stat: LCDStatus(0x80), // bit 7 is always 1
             oam: Default::default(),
             scan_dot: Default::default(),
             fetch: Default::default(),
@@ -528,7 +528,7 @@ pub(crate) struct ScreenPosition {
     pub(crate) window_x: u8,
 }
 
-#[derive(Debug, Default)]
+#[derive(Debug)]
 pub(crate) struct Monochrome {
     /// 0xFF47 | BGP - Background Palette Data
     pub(crate) bg_palette: BackgroundPalette,
@@ -538,6 +538,16 @@ pub(crate) struct Monochrome {
     pub(crate) obj_palette_1: ObjectPalette,
 }
 
+impl Default for Monochrome {
+    fn default() -> Self {
+        Self {
+            bg_palette: BackgroundPalette(0),
+            obj_palette_0: ObjectPalette(0),
+            obj_palette_1: ObjectPalette(0),
+        }
+    }
+}
+
 #[derive(Debug)]
 pub(crate) struct ObjectAttrTable {
     buf: Box<[u8; OAM_SIZE]>,
@@ -575,7 +585,7 @@ impl Default for ObjectAttrTable {
     }
 }
 
-#[derive(Debug, Clone, Copy, Default)]
+#[derive(Debug, Clone, Copy)]
 struct ObjectAttr {
     y: u8,
     x: u8,
@@ -836,7 +846,7 @@ struct BgPixelProperty {
     shade_id: u8,
 }
 
-#[derive(Debug, Default)]
+#[derive(Debug)]
 struct ObjPixelProperty {
     shade_id: u8,
     palette_kind: ObjectPaletteKind,

src/ppu/dma.rs

@@ -1,6 +1,6 @@
 use crate::Cycle;
 
-#[derive(Debug, Default)]
+#[derive(Debug)]
 pub(crate) struct DirectMemoryAccess {
     pub(crate) state: DmaState,
     cycle: Cycle,
@@ -8,6 +8,16 @@ pub(crate) struct DirectMemoryAccess {
     pub(crate) start: DmaAddress,
 }
 
+impl Default for DirectMemoryAccess {
+    fn default() -> Self {
+        Self {
+            state: DmaState::Disabled,
+            cycle: Default::default(),
+            start: Default::default(),
+        }
+    }
+}
+
 impl DirectMemoryAccess {
     pub(crate) fn tick(&mut self) -> Option<(u16, u16)> {
         match self.state {
@@ -69,12 +79,6 @@ pub(crate) enum DmaState {
     Transferring,
 }
 
-impl Default for DmaState {
-    fn default() -> Self {
-        Self::Disabled
-    }
-}
-
 #[derive(Debug, Clone, Copy, Default)]
 pub(crate) struct DmaAddress(Option<u16>);
 

src/ppu/types.rs

@@ -29,12 +29,6 @@ impl Clone for LCDStatus {
     }
 }
 
-impl Default for LCDStatus {
-    fn default() -> Self {
-        Self(0x80) // bit 7 is always 1
-    }
-}
-
 impl From<LCDStatus> for u8 {
     fn from(status: LCDStatus) -> Self {
         status.0
@@ -67,12 +61,6 @@ impl From<PpuMode> for u8 {
     }
 }
 
-impl Default for PpuMode {
-    fn default() -> Self {
-        Self::HBlank
-    }
-}
-
 bitfield! {
     pub struct LCDControl(u8);
     impl Debug;
@@ -93,12 +81,6 @@ impl Clone for LCDControl {
     }
 }
 
-impl Default for LCDControl {
-    fn default() -> Self {
-        Self(0)
-    }
-}
-
 impl From<u8> for LCDControl {
     fn from(byte: u8) -> Self {
         Self(byte)
@@ -142,12 +124,6 @@ impl From<TileMapAddress> for u8 {
     }
 }
 
-impl Default for TileMapAddress {
-    fn default() -> Self {
-        Self::X9800
-    }
-}
-
 #[derive(Debug, Clone, Copy)]
 pub enum TileDataAddress {
     X8800 = 0,
@@ -170,12 +146,6 @@ impl From<TileDataAddress> for u8 {
     }
 }
 
-impl Default for TileDataAddress {
-    fn default() -> Self {
-        Self::X8800
-    }
-}
-
 #[derive(Debug, Clone, Copy)]
 pub enum ObjectSize {
     Eight = 0,
@@ -209,12 +179,6 @@ impl From<ObjectSize> for u8 {
     }
 }
 
-impl Default for ObjectSize {
-    fn default() -> Self {
-        Self::Eight
-    }
-}
-
 bitfield! {
     pub struct BackgroundPalette(u8);
     impl Debug;
@@ -243,12 +207,6 @@ impl Clone for BackgroundPalette {
     }
 }
 
-impl Default for BackgroundPalette {
-    fn default() -> Self {
-        Self(0)
-    }
-}
-
 impl From<u8> for BackgroundPalette {
     fn from(byte: u8) -> Self {
         Self(byte)
@@ -288,12 +246,6 @@ impl Clone for ObjectPalette {
     }
 }
 
-impl Default for ObjectPalette {
-    fn default() -> Self {
-        Self(0)
-    }
-}
-
 impl From<u8> for ObjectPalette {
     fn from(byte: u8) -> Self {
         Self(byte)
@@ -361,24 +313,12 @@ impl From<ObjectFlags> for u8 {
     }
 }
 
-impl Default for ObjectFlags {
-    fn default() -> Self {
-        Self(0)
-    }
-}
-
 #[derive(Debug, Clone, Copy)]
 pub enum ObjectPaletteKind {
     Zero = 0,
     One = 1,
 }
 
-impl Default for ObjectPaletteKind {
-    fn default() -> Self {
-        Self::Zero
-    }
-}
-
 impl From<u8> for ObjectPaletteKind {
     fn from(byte: u8) -> Self {
         match byte & 0b01 {
@@ -417,12 +357,6 @@ impl From<RenderPriority> for u8 {
     }
 }
 
-impl Default for RenderPriority {
-    fn default() -> Self {
-        Self::Object
-    }
-}
-
 #[derive(Debug, Clone, Copy)]
 pub enum GrayShade {
     White = 0,
@@ -442,12 +376,6 @@ impl GrayShade {
     }
 }
 
-impl Default for GrayShade {
-    fn default() -> Self {
-        Self::White
-    }
-}
-
 impl From<u8> for GrayShade {
     fn from(byte: u8) -> Self {
         match byte & 0b11 {

src/serial.rs

@@ -1,6 +1,6 @@
 use bitfield::bitfield;
 
-#[derive(Debug, Default)]
+#[derive(Debug)]
 pub(crate) struct Serial {
     /// 0xFF01 | SB - Serial Transfer Data
     pub(crate) next: u8,
@@ -8,6 +8,15 @@ pub(crate) struct Serial {
     pub(crate) ctrl: SerialControl,
 }
 
+impl Default for Serial {
+    fn default() -> Self {
+        Self {
+            next: Default::default(),
+            ctrl: SerialControl(0),
+        }
+    }
+}
+
 bitfield! {
     pub struct SerialControl(u8);
     impl Debug;
@@ -23,12 +32,6 @@ impl Clone for SerialControl {
     }
 }
 
-impl Default for SerialControl {
-    fn default() -> Self {
-        Self(0)
-    }
-}
-
 impl From<u8> for SerialControl {
     fn from(byte: u8) -> Self {
         Self(byte)
@@ -47,12 +50,6 @@ enum ShiftClock {
     Internal = 1,
 }
 
-impl Default for ShiftClock {
-    fn default() -> Self {
-        Self::External
-    }
-}
-
 impl From<u8> for ShiftClock {
     fn from(byte: u8) -> Self {
         match byte & 0b01 {
@@ -69,12 +66,6 @@ enum ClockSpeed {
     Fast = 1,
 }
 
-impl Default for ClockSpeed {
-    fn default() -> Self {
-        Self::Normal
-    }
-}
-
 impl From<u8> for ClockSpeed {
     fn from(byte: u8) -> Self {
         match byte & 0b01 {

src/timer.rs

@@ -106,7 +106,7 @@ impl Timer {
 impl Default for Timer {
     fn default() -> Self {
         Self {
-            ctrl: Default::default(),
+            ctrl: TimerControl(0),
             counter: 0,
             modulo: 0,
             divider: 0,
@@ -157,12 +157,6 @@ impl Clone for TimerControl {
     }
 }
 
-impl Default for TimerControl {
-    fn default() -> Self {
-        Self(0)
-    }
-}
-
 impl From<u8> for TimerControl {
     fn from(byte: u8) -> Self {
         Self(byte)