Fix Pokemon Pinball

common/src/initialization.rs

@@ -73,6 +73,7 @@ pub fn init_and_run_gameboy(
     EMULATOR_STATE.running.store(true, std::sync::atomic::Ordering::Relaxed);
     while EMULATOR_STATE.running.load(std::sync::atomic::Ordering::Relaxed){
         if !EMULATOR_STATE.pause.load(std::sync::atomic::Ordering::SeqCst){
+            // Locking the state mutex in order to signal the menu that we are cycling a frame now
             let state = &EMULATOR_STATE;
             let _mutex_ctx = state.state_mutex.lock().unwrap();
             gameboy.cycle_frame();

core/src/debugger/mod.rs

@@ -1,6 +1,7 @@
 mod disassembler;
 
-use std::collections::HashSet;
+use std::fmt::{Formatter, Display, Result};
+use std::collections::{HashSet, HashMap};
 
 use crate::{*, machine::gameboy::*, cpu::gb_cpu::GbCpu, utils::vec2::Vec2, ppu::{ppu_state::PpuState, gb_ppu::GbPpu}};
 use self::disassembler::{OpcodeEntry, disassemble};
@@ -12,18 +13,51 @@ pub enum PpuLayer{
     Sprites
 }
 
+#[derive(Clone, Copy, PartialEq, Eq, Hash)]
+pub struct Address{
+    pub mem_addr: u16,
+    pub bank: u16
+}
+
+impl Address{
+    pub fn new(mem_addr:u16, bank:u16)->Self { Self { mem_addr, bank} }
+}
+
+impl Display for Address{
+    fn fmt(&self, f: &mut Formatter<'_>) -> Result {
+        f.write_fmt(format_args!("{:#X}:{}", self.mem_addr, self.bank))
+    }
+}
+
+#[derive(Clone, Copy)]
+pub enum WatchMode{
+    Read, 
+    Write,
+    ReadWrite
+}
+
+impl PartialEq for WatchMode{
+    fn eq(&self, other: &Self) -> bool {
+        let self_val = core::mem::discriminant(self);
+        let other_val = core::mem::discriminant(other);
+        let read_write_val = core::mem::discriminant(&WatchMode::ReadWrite);
+
+        return self_val == other_val || read_write_val == self_val || read_write_val == other_val;
+    }
+}
+
 pub enum DebuggerCommand{
     Stop,
     Step,
     Continue,
     SkipHalt,
     Registers,
-    Break(u16, u16),
-    RemoveBreak(u16, u16),
+    Break(Address),
+    RemoveBreak(Address),
     DumpMemory(u16, u16),
     Disassemble(u16),
-    Watch(u16),
-    RemoveWatch(u16),
+    Watch(Address, WatchMode, Option<u8>),
+    RemoveWatch(Address),
     PpuInfo,
     GetPpuLayer(PpuLayer)
 }
@@ -34,20 +68,20 @@ pub const PPU_BUFFER_SIZE:usize = PPU_BUFFER_HEIGHT * PPU_BUFFER_WIDTH;
 
 pub enum DebuggerResult{
     Registers(Registers),
-    AddedBreak(u16),
+    AddedBreak(Address),
     HitBreak(u16, u16),
-    RemovedBreak(u16),
-    BreakDoNotExist(u16),
+    RemovedBreak(Address),
+    BreakDoNotExist(Address),
     Continuing,
     HaltWakeup,
     Stepped(u16, u16),
     Stopped(u16, u16),
     MemoryDump(u16, u16, Vec<u8>),
     Disassembly(u16, u16, Vec<OpcodeEntry>),
-    AddedWatch(u16),
-    HitWatch(u16, u16),
-    RemovedWatch(u16),
-    WatchDoNotExist(u16),
+    AddedWatch(Address),
+    HitWatch(u16, u16, u16, u16, u8),
+    RemovedWatch(Address),
+    WatchDoNotExist(Address),
     PpuInfo(PpuInfo),
     PpuLayer(PpuLayer, Box<[Pixel;PPU_BUFFER_SIZE]>)
 }
@@ -97,7 +131,7 @@ pub trait DebuggerInterface{
 
 pub struct Debugger<UI:DebuggerInterface>{
     ui:UI,
-    breakpoints:HashSet<(u16, u16)>,
+    breakpoints:HashSet<Address>,
     skip_halt: bool
 }
 
@@ -113,45 +147,45 @@ impl<UI:DebuggerInterface> Debugger<UI>{
         (self.check_for_break(cpu.program_counter, bank) || self.ui.should_stop() || hit_watch) && !(cpu.halt && self.skip_halt)
     }
 
-    fn check_for_break(&self, pc:u16, bank:u16)->bool{self.breakpoints.contains(&(pc, bank))}
+    fn check_for_break(&self, pc:u16, bank:u16)->bool{self.breakpoints.contains(&Address::new(pc, bank))}
 
-    fn add_breakpoint(&mut self, address:u16, bank:u16){_ = self.breakpoints.insert((address, bank))}
+    fn add_breakpoint(&mut self, address:Address){_ = self.breakpoints.insert(address)}
 
-    fn try_remove_breakpoint(&mut self, address:u16, bank:u16)->bool{self.breakpoints.remove(&(address, bank))}
+    fn try_remove_breakpoint(&mut self, address:Address)->bool{self.breakpoints.remove(&address)}
 }
 
 impl_gameboy!{{
     pub fn run_debugger(&mut self){
-        while self.debugger.should_halt(&self.cpu, self.get_current_bank(self.cpu.program_counter), self.mmu.mem_watch.hit_addr.is_some()) {
+        while self.debugger.should_halt(&self.cpu, self.mmu.get_current_bank(self.cpu.program_counter), self.mmu.mem_watch.hit_addr.is_some()) {
             if !self.cpu.halt && self.debugger.skip_halt{
                 self.debugger.send(DebuggerResult::HaltWakeup);
                 self.debugger.skip_halt = false;
             }
-            if self.debugger.check_for_break(self.cpu.program_counter, self.get_current_bank(self.cpu.program_counter)){
-                self.debugger.send(DebuggerResult::HitBreak(self.cpu.program_counter, self.get_current_bank(self.cpu.program_counter)));
+            if self.debugger.check_for_break(self.cpu.program_counter, self.mmu.get_current_bank(self.cpu.program_counter)){
+                self.debugger.send(DebuggerResult::HitBreak(self.cpu.program_counter, self.mmu.get_current_bank(self.cpu.program_counter)));
             }
-            if let Some(addr) = self.mmu.mem_watch.hit_addr{
-                self.debugger.send(DebuggerResult::HitWatch(addr, self.cpu.program_counter));
+            if let Some((addr, bank, val)) = self.mmu.mem_watch.hit_addr{
+                self.debugger.send(DebuggerResult::HitWatch(addr, bank, self.cpu.program_counter, self.mmu.get_current_bank(self.cpu.program_counter), val));
                 self.mmu.mem_watch.hit_addr = None;
             }
             match self.debugger.recv(){
-                DebuggerCommand::Stop=>self.debugger.send(DebuggerResult::Stopped(self.cpu.program_counter, self.get_current_bank(self.cpu.program_counter))),
+                DebuggerCommand::Stop=>self.debugger.send(DebuggerResult::Stopped(self.cpu.program_counter, self.mmu.get_current_bank(self.cpu.program_counter))),
                 DebuggerCommand::Step=>{
                     self.step();
-                    self.debugger.send(DebuggerResult::Stepped(self.cpu.program_counter, self.get_current_bank(self.cpu.program_counter)));
+                    self.debugger.send(DebuggerResult::Stepped(self.cpu.program_counter, self.mmu.get_current_bank(self.cpu.program_counter)));
                 }
                 DebuggerCommand::Continue=>{
                     self.debugger.send(DebuggerResult::Continuing);
                     break;
                 },
                 DebuggerCommand::SkipHalt => self.debugger.skip_halt = true,
                 DebuggerCommand::Registers => self.debugger.send(DebuggerResult::Registers(Registers::new(&self.cpu))),
-                DebuggerCommand::Break(address, bank) => {
-                    self.debugger.add_breakpoint(address, bank);
+                DebuggerCommand::Break(address) => {
+                    self.debugger.add_breakpoint(address);
                     self.debugger.send(DebuggerResult::AddedBreak(address));
                 },
-                DebuggerCommand::RemoveBreak(address, bank)=>{
-                    let result = match self.debugger.try_remove_breakpoint(address, bank) {
+                DebuggerCommand::RemoveBreak(address)=>{
+                    let result = match self.debugger.try_remove_breakpoint(address) {
                         true => DebuggerResult::RemovedBreak(address),
                         false => DebuggerResult::BreakDoNotExist(address)
                     };
@@ -163,14 +197,14 @@ impl_gameboy!{{
                         buffer[i as usize] = self.mmu.dbg_read(address + i);
                     }
 
-                    self.debugger.send(DebuggerResult::MemoryDump(address, self.get_current_bank(address), buffer));
+                    self.debugger.send(DebuggerResult::MemoryDump(address, self.mmu.get_current_bank(address), buffer));
                 }
                 DebuggerCommand::Disassemble(len)=>{
                     let result = disassemble(&self.cpu, &mut self.mmu, len);
-                    self.debugger.send(DebuggerResult::Disassembly(len, self.get_current_bank(self.cpu.program_counter), result));
+                    self.debugger.send(DebuggerResult::Disassembly(len, self.mmu.get_current_bank(self.cpu.program_counter), result));
                 },
-                DebuggerCommand::Watch(address)=>{
-                    self.mmu.mem_watch.add_address(address);
+                DebuggerCommand::Watch(address, mode, value)=>{
+                    self.mmu.mem_watch.add_address(address, mode, value);
                     self.debugger.send(DebuggerResult::AddedWatch(address));
                 },
                 DebuggerCommand::RemoveWatch(address)=>{
@@ -187,27 +221,17 @@ impl_gameboy!{{
             }
         }
     }
-
-    fn get_current_bank(&self, address:u16)->u16{
-        return match address{
-            0..=0x3FFF => 0,
-            0x4000..=0x7FFF => self.mmu.mem_watch.current_rom_bank_number,
-            0x8000..=0x9FFF => self.mmu.get_ppu().vram.get_bank_reg() as u16,
-            0xC000..=0xFDFF => self.mmu.mem_watch.current_ram_bank_number as u16,
-            _=>0
-        };
-    }
 }}
 
 pub struct MemoryWatcher{
-    pub watching_addresses: HashSet<u16>,
-    pub hit_addr:Option<u16>,
+    pub watching_addresses: HashMap<Address, (WatchMode, Option<u8>)>,
+    pub hit_addr:Option<(u16, u16, u8)>,
     pub current_rom_bank_number: u16,
     pub current_ram_bank_number: u8,
 }
 
 impl MemoryWatcher{
-    pub fn new()->Self{Self { watching_addresses: HashSet::new(), hit_addr: None, current_rom_bank_number: 0, current_ram_bank_number: 0 }}
-    pub fn add_address(&mut self, address:u16){_ = self.watching_addresses.insert(address)}
-    pub fn try_remove_address(&mut self, address:u16)->bool{self.watching_addresses.remove(&address)}
+    pub fn new()->Self{Self { watching_addresses: HashMap::new(), hit_addr: None, current_rom_bank_number: 0, current_ram_bank_number: 0 }}
+    pub fn add_address(&mut self, address:Address, mode: WatchMode, value:Option<u8>){_ = self.watching_addresses.insert(address, (mode, value))}
+    pub fn try_remove_address(&mut self, address:Address)->bool{self.watching_addresses.remove(&address).is_some()}
 }

core/src/machine/mbc_initializer.rs

@@ -28,7 +28,8 @@ pub fn initialize_mbc(program:&[u8], save_data:Option<&[u8]>)->&'static mut dyn
         0x12 => static_alloc(Mbc3::new(program_clone,false,None)),
         0x19 | 
         0x1A => static_alloc(Mbc5::new(program_clone, false, save_data_clone)),
-        0x1B => static_alloc(Mbc5::new(program_clone, true, save_data_clone)),
+        0x1B |
+        0x1E => static_alloc(Mbc5::new(program_clone, true, save_data_clone)),
         _=> core::panic!("not supported cartridge: {:#X}",mbc_type)
     };
 

core/src/mmu/carts/mbc1.rs

@@ -43,14 +43,19 @@ impl<'a> Mbc for Mbc1<'a>{
         if self.ram.is_empty(){
             return 0xFF;
         }
+
         let bank:u16 = self.get_current_ram_bank() as u16;
-        return self.ram[(bank as usize * RAM_BANK_SIZE) + address as usize];
+        let address = (bank as usize * RAM_BANK_SIZE) + address as usize;
+        let address = get_external_ram_valid_address(address, &self.ram);
+        return self.ram[address];
     }
 
     fn write_external_ram(&mut self, address: u16, value: u8){
         if self.ram.len() > 0{
             let bank:u16 = self.get_current_ram_bank() as u16;
-            self.ram[(bank as usize * RAM_BANK_SIZE) + address as usize] = value;
+            let address = (bank as usize * RAM_BANK_SIZE) + address as usize;
+            let address = get_external_ram_valid_address(address, &self.ram);
+            self.ram[address] = value;
         }
     }
 

core/src/mmu/carts/mbc3.rs

@@ -56,7 +56,8 @@ impl<'a> Mbc for Mbc3<'a>{
         return match self.ram_rtc_select{
             0..=3=>{
                 let internal_address = self.ram_rtc_select as usize * RAM_BANK_SIZE as usize +  address as usize;
-                return self.ram[internal_address];
+                let address = get_external_ram_valid_address(internal_address, &self.ram);
+                return self.ram[address];
             },
             0x8..=0xC=>self.rtc_registers[(self.ram_rtc_select - 8) as usize],
             _=>EXTERNAL_RAM_READ_ERROR_VALUE
@@ -68,7 +69,8 @@ impl<'a> Mbc for Mbc3<'a>{
             match self.ram_rtc_select{
                 0..=3=>{
                     let internal_address = self.ram_rtc_select as usize * RAM_BANK_SIZE as usize +  address as usize;
-                    self.ram[internal_address] = value;
+                    let address = get_external_ram_valid_address(internal_address, &self.ram);
+                    self.ram[address] = value;
                 },
                 0x8..=0xC=>self.rtc_registers[(self.ram_rtc_select - 8) as usize] = value,
                 _=>{}

core/src/mmu/carts/mbc5.rs

@@ -38,15 +38,16 @@ impl<'a> Mbc for Mbc5<'a> {
             2=>self.rom_bank_number_register = (self.rom_bank_number_register & 0xFF00) | value as u16,
             // high bit 9
             3=>self.rom_bank_number_register = (self.rom_bank_number_register & 0x00FF) | ((value as u16) << 8),
-            4|5=>self.ram_bank_number = value,
+            4|5=>self.ram_bank_number = value & 0xF,
             _=>{}
         }
     }
 
     fn read_external_ram(&self, address:u16)->u8 {
         if self.ram_enable_register == ENABLE_RAM_VALUE{
-            let bank = (self.ram_bank_number & 0xF) as usize * RAM_BANK_SIZE;
-            return self.ram[address as usize + bank];
+            let bank = self.ram_bank_number as usize * RAM_BANK_SIZE;
+            let address= get_external_ram_valid_address(address as usize + bank, &self.ram);
+            return self.ram[address];
         }
 
         // ram is disabled
@@ -55,11 +56,9 @@ impl<'a> Mbc for Mbc5<'a> {
 
     fn write_external_ram(&mut self, address:u16, value:u8) {
         if self.ram_enable_register == ENABLE_RAM_VALUE{
-            let bank = (self.ram_bank_number & 0xF) as usize * RAM_BANK_SIZE;
-            self.ram[address as usize + bank] = value;
-        }
-        else{
-            log::warn!("MBC5 write while ram is not enabled. ram_address: {}, value: {}", address, value);
+            let bank = self.ram_bank_number as usize * RAM_BANK_SIZE;
+            let address= get_external_ram_valid_address(address as usize + bank, &self.ram);
+            self.ram[address] = value;
         }
     }
 

core/src/mmu/carts/mod.rs

@@ -16,11 +16,11 @@ pub const RAM_BANK_SIZE:usize = 0x2000;
 pub const CGB_FLAG_ADDRESS:usize = 0x143;
 pub const MBC_RAM_SIZE_LOCATION:usize = 0x149;
 
-pub fn get_ram_size(ram_size_register:u8)->usize{
+fn get_ram_size(ram_size_register:u8)->usize{
     match ram_size_register{
         0x0=>0,
         0x1=>0x800,     // Unofficial - Undefined according to official docs
-        0x2=>0x4000,
+        0x2=>0x2000,
         0x3=>0x8000,
         0x4=>0x2_0000,
         0x5=>0x1_0000,
@@ -43,6 +43,13 @@ pub fn init_ram(ram_reg:u8, external_ram:Option<&'static mut[u8]>)->&'static mut
     }
 }
 
+/// Almost all MBC's external ram access are clipped to the ram size by masking with the relevent bits
+/// (Notice that all the avaliable sizes are left shifts of 1 and we assume `init_ram` initialized it).
+/// This emulates the bus to the chip with the amount of bits from the address the ram chip reads.
+pub(self) fn get_external_ram_valid_address(address:usize, external_ram:&[u8])->usize{
+    address as usize & (external_ram.len() - 1)
+}
+
 pub trait Mbc{
     fn get_ram(&mut self)->&mut [u8];
     fn has_battery(&self)->bool;

core/src/mmu/carts/rom.rs

@@ -29,11 +29,11 @@ impl<'a> Mbc for Rom<'a>{
     }
 
     fn read_external_ram(&self, address:u16)->u8{
-        self.external_ram[address as usize]
+        self.external_ram[get_external_ram_valid_address(address as usize, &self.external_ram)]
     }
 
     fn write_external_ram(&mut self, address:u16, value:u8){
-        self.external_ram[address as usize] = value
+        self.external_ram[get_external_ram_valid_address(address as usize, &self.external_ram)] = value
     }
 
     #[cfg(feature = "dbg")]