Fix some bugs in memory

src/memory.rs

@@ -19,7 +19,6 @@
 use self::{
     banked::{Banked, UpperBanked},
     io::BusIO,
-    mapper::Mapper,
 };
 pub mod banked;
 pub mod io;
@@ -44,25 +43,21 @@ impl Mode {
     }
 }
 
+/// The [Bus] controls the memory map from the CPU's perspective
 #[derive(Debug)]
 pub struct Bus {
     mode: Mode,
-    // TODO: define a way to have an arbitrary memory map
+    // TODO: Replace the cart entirely
+    // TODO: Separate into external and vram bus
+    // TODO: Implement multiple views into memory
     cart: Cart,
     // VRAM is a 0x2000 B window from 0x8000..0xa000, with two banks
     vram: Banked<0x8000, 0x2000, 2>,
     // WRAM is a 0x2000 B window from 0xc000..0xe000. The upper half is banked.
     wram: UpperBanked<0xc000, 0x1000, 8>,
+    // Memory mapped IO registers, HRAM
     // HRAM is a 0x80 B window from 0xff80..0xffff
-    hram: [u8; 0x80],
+    hmem: [u8; 0x100],
-    // Joypad driver
-    // Serial driver
-    // Timer and divider
-    // Interrupt controller
-    // Audio controller
-    // PPU/LCD controller
-    mmio: [u8; 0x80],
-    //
 }
 
 impl BusIO for Bus {
@@ -82,10 +77,7 @@ impl BusIO for Bus {
             0xfe00..=0xfe9f => Some(0x0a),
             // Illegal
             0xfea0..=0xfeff => None,
-            // Memory mapped IO
+            0xff00..=0xffff => self.hmem.read(addr & 0xff),
-            0xff00..=0xff7f => self.mmio.read(addr & 0x7f),
-            // HiRAM
-            0xff80..=0xffff => self.hram.read(addr & 0x7f),
             _ => None,
         }
     }
@@ -106,10 +98,8 @@ impl BusIO for Bus {
             0xfe00..=0xfe9f => Some(()),
             // Illegal
             0xfea0..=0xfeff => None,
-            // Memory mapped IO
+            // Memory mapped IO & HRAM
-            0xff00..=0xff7f => self.mmio.write(addr & 0x7f, data),
+            0xff00..=0xffff => self.hmem.write(addr & 0xff, data),
-            // HiRAM
-            0xff80..=0xffff => self.hram.write(addr & 0x7f, data),
             _ => None,
         }
     }
@@ -122,8 +112,7 @@ impl Bus {
             mode: Mode::CGB,
             vram: Default::default(),
             wram: Default::default(),
-            mmio: [0; 128],
+            hmem: [0; 256],
-            hram: [0; 128],
         }
     }
 
@@ -156,7 +145,7 @@ impl Bus {
                 Mode::DMG => {}
             },
         }
-        self.mmio.get(addr % 0x80).copied()
+        self.hmem.get(addr % 0x80 + 0x80).copied()
     }
 
     pub fn mmio_read_cgb(&self, addr: usize) -> Option<u8> {
@@ -209,7 +198,7 @@ impl Bus {
                 Mode::DMG => {}
             },
         }
-        self.mmio.write(addr % 0x80, data)
+        self.hmem.write(addr % 0x80 + 0x80, data)
     }
 
     pub fn mmio_write_cgb(&mut self, addr: usize, data: u8) -> Option<()> {
@@ -242,17 +231,22 @@ impl Bus {
     }
 }
 
+pub use cart::Cart;
+
+mod cart {
+    use super::{mapper::*, BusIO};
+    use crate::{
+        constants::{RAM_BANK, ROM_BANK},
+        memory::io::BusAux,
+    };
+    use std::fmt::Debug;
+
     pub struct Cart {
         pub rom: Vec<u8>,
         pub ram: Vec<u8>,
-    mapper: Box<dyn Mapper>,
+        mapper: MBC,
     }
 
-mod cart {
-    use crate::memory::io::BusAux;
-
-    use super::{mapper::*, BusIO, Cart};
-    use std::fmt::Debug;
     impl BusIO for Cart {
         fn read(&self, addr: usize) -> Option<u8> {
             match self.mapper.read(addr) {
@@ -276,23 +270,28 @@ mod cart {
     }
     /// Read cartridge header, as defined in the [Pan docs](https://gbdev.io/pandocs/The_Cartridge_Header.html)
     impl Cart {
-        pub fn new(rom: Vec<u8>) -> Self {
+        pub fn new(mut rom: Vec<u8>, mut ram: Vec<u8>) -> Self {
-            let rom_size = rom.romsize().expect("ROM should have cart header!");
+            let rom_size = ROM_BANK * rom.romsize().expect("ROM should have cart header!");
-            let ram_size = rom.ramsize().expect("ROM should have cart header!");
+            let ram_size = RAM_BANK * rom.ramsize().expect("ROM should have cart header!");
+            if rom_size > rom.len() {
+                eprintln!("Rom claims to be {rom_size}, but is actually {}", rom.len());
+            }
+
+            rom.resize(rom_size, 0);
+            ram.resize(ram_size, 0);
+
             let cart_type = rom.carttype().expect("ROM should have cart header!");
             Self {
-                mapper: match cart_type {
+                mapper: MBC::from_cart_type(cart_type).expect("MAPPER NOT FOUND: {cart_type}"),
-                    0 => Box::<no_mbc::NoMBC>::default(),
-                    1 => Box::new(mbc1::MBC1::new(rom_size, 0)), // ROM
-                    2 => Box::new(mbc1::MBC1::new(rom_size, ram_size)), // ROM + RAM
-                    3 => Box::new(mbc1::MBC1::new(rom_size, ram_size)), // ROM + RAM + Battery
-                    // _ => Box::<NoMBC>::default(),
-                    n => todo!("Mapper 0x{n:02x}"),
-                },
                 rom,
-                ram: vec![],
+                ram,
             }
         }
+
+        pub fn load_save(&mut self, save: &[u8]) {
+            self.ram.resize(save.len(), 0);
+            self.ram.copy_from_slice(save)
+        }
     }
 
     impl Debug for Cart {
@@ -306,19 +305,19 @@ mod cart {
 
     impl From<Vec<u8>> for Cart {
         fn from(value: Vec<u8>) -> Self {
-            Self::new(value)
+            Self::new(value, vec![])
         }
     }
 
     impl From<&[u8]> for Cart {
         fn from(value: &[u8]) -> Self {
-            Self::new(Vec::from(value))
+            Self::new(Vec::from(value), vec![])
         }
     }
 
     impl<Item: AsRef<u8>> FromIterator<Item> for Cart {
         fn from_iter<T: IntoIterator<Item = Item>>(iter: T) -> Self {
-            Self::new(iter.into_iter().map(|item| *(item.as_ref())).collect())
+            Self::new(iter.into_iter().map(|item| *(item.as_ref())).collect(), vec![])
         }
     }
 
@@ -368,6 +367,48 @@ pub mod mapper {
         fn write(&mut self, addr: usize, val: u8) -> Response;
     }
 
+    pub enum MBC {
+        None,
+        MBC1(mbc1::MBC1),
+        // TODO: MBC2, 3, 5
+    }
+
+    impl MBC {
+        pub fn from_cart_type(cart_type: u8) -> Option<Self> {
+            Some(match cart_type {
+                0 => Self::None,
+                1 => Self::MBC1(mbc1::MBC1::new()), // ROM
+                2 => Self::MBC1(mbc1::MBC1::new()), // ROM + RAM
+                3 => Self::MBC1(mbc1::MBC1::new()), // ROM + RAM + Battery
+                n => {
+                    eprintln!("Mapper 0x{n:02x}");
+                    None?
+                }
+            })
+        }
+
+        pub fn read(&self, addr: usize) -> Response {
+            match self {
+                MBC::None => match addr {
+                    0x0000..=0x7fff => Response::Rom(addr),
+                    0xa000..=0xbfff => Response::Ram(addr - 0xa000),
+                    _ => Response::None,
+                },
+                MBC::MBC1(mbc) => mbc.read(addr),
+            }
+        }
+
+        pub fn write(&mut self, addr: usize, val: u8) -> Response {
+            match self {
+                MBC::None => match addr {
+                    0xa000..=0xbfff => Response::Ram(addr),
+                    _ => Response::None,
+                },
+                MBC::MBC1(mbc) => mbc.write(addr, val),
+            }
+        }
+    }
+
     pub mod no_mbc {
         use super::*;
 
@@ -378,14 +419,14 @@ pub mod mapper {
             fn read(&self, addr: usize) -> Response {
                 match addr {
                     0x0000..=0x7fff => Response::Rom(addr),
-                    0xa000..=0xbfff => Response::Ram(addr - 0x8000),
+                    0xa000..=0xbfff => Response::Ram(addr - 0xa000),
                     _ => Response::None,
                 }
             }
 
             fn write(&mut self, addr: usize, _val: u8) -> Response {
                 match addr {
-                    0xa000..=0xbfff => Response::Ram(addr - 0xA000),
+                    0xa000..=0xbfff => Response::Ram(addr - 0xa000),
                     _ => Response::None,
                 }
             }
@@ -394,71 +435,66 @@ pub mod mapper {
 
     pub mod mbc1 {
         use super::*;
+        use crate::constants::*;
 
         #[derive(Clone, Copy, Debug, Default)]
         pub struct MBC1 {
-            rom_mask: u8,
+            bank_enable: bool,
-            ram_mask: u8,
-            mode: u8,
-            rom_bank: u8,
-            ram_bank: u8,
             ram_enable: bool,
+            bank_lo: u8,
+            bank_hi: u8,
         }
 
         impl MBC1 {
-            pub fn new(rom_size: usize, ram_size: usize) -> Self {
+            pub fn new() -> Self {
-                Self {
+                Self { bank_lo: 1, ..Default::default() }
-                    rom_mask: (rom_size).wrapping_sub(1) as u8,
+            }
-                    ram_mask: (ram_size).wrapping_sub(1) as u8,
+
-                    rom_bank: 1,
+            pub fn rom_lower(&self) -> usize {
-                    ..Default::default()
+                match self.bank_enable {
+                    true => ((self.bank_hi << 5) as usize) * ROM_BANK,
+                    false => 0,
                 }
             }
-            pub fn rom_bank_lower(&self) -> usize {
+
-                (((self.ram_bank << 5) & self.rom_mask) as usize) << 14
+            pub fn rom_upper(&self) -> usize {
+                ((self.bank_lo | (self.bank_hi << 5)) as usize) * ROM_BANK
             }
-            pub fn rom_bank_upper(&self) -> usize {
+
-                ((self.rom_bank | (self.ram_bank << 5) & self.rom_mask) as usize) << 14
+            pub fn ram_base(&self) -> usize {
+                match self.bank_enable {
+                    true => (self.bank_hi as usize) * RAM_BANK,
+                    false => 0,
                 }
-            pub fn ram_bank_base(&self) -> usize {
-                ((self.ram_bank & self.ram_mask) as usize) << 13
             }
         }
 
         impl Mapper for MBC1 {
             fn read(&self, addr: usize) -> Response {
-                match (addr, self.mode) {
+                match addr >> 12 {
-                    (0x0000..=0x3fff, 0) => Response::Rom(addr),
+                    0..=3 => Response::Rom(self.rom_lower() | addr),
-                    (0x0000..=0x3fff, _) => Response::Rom(self.rom_bank_lower() | (addr & 0x3fff)),
+                    4..=7 => Response::Rom(self.rom_upper() | (addr % ROM_BANK)),
-
+                    0xa | 0xb => Response::Ram(self.ram_base() | (addr % RAM_BANK)),
-                    (0x4000..=0x7fff, _) => Response::Rom(self.rom_bank_upper() | (addr & 0x3fff)),
-
-                    (0xa000..=0xbfff, 0) => Response::Ram(addr & 0x1fff),
-                    (0xa000..=0xbfff, _) => Response::Ram(self.ram_bank_base() | (addr & 0x1fff)),
-
                     _ => Response::None,
                 }
             }
 
             fn write(&mut self, addr: usize, val: u8) -> Response {
-                match (addr, self.mode) {
+                match addr >> 12 {
-                    (0x0000..=0x1fff, _) => {
+                    0x0 | 0x1 => {
                         self.ram_enable = val & 0xf == 0xa;
                     }
-                    (0x2000..=0x3fff, _) => {
+                    0x2 | 0x3 => {
-                        self.rom_bank = (val & 0x1f).max(1);
+                        self.bank_lo = (val & 0x1f).max(1);
                     }
-                    (0x4000..=0x5fff, _) => {
+                    0x4 | 0x5 => {
-                        self.ram_bank = val & 0x3;
+                        self.bank_hi = val & 0x3;
                     }
-                    (0x6000..=0x7fff, _) => {
+                    0x6 | 0x7 => {
-                        self.mode = val & 1;
+                        self.bank_enable = val & 1 != 0;
                     }
-                    (0xa000..=0xbfff, 0) if self.ram_enable => {
+                    0xa | 0xb if self.ram_enable => {
-                        return Response::Ram(addr & 0x1fff);
+                        return Response::Ram(self.ram_base() | (addr % RAM_BANK))
-                    }
-                    (0xa000..=0xbfff, 1) if self.ram_enable => {
-                        return Response::Ram((self.ram_bank as usize & 0b11 << 13) | addr & 0x1fff)
                     }
                     _ => return Response::None,
                 }