vertical scroll bug fixed, mmc3 fixed, pics, readme

README.md

@@ -1,12 +1,12 @@
 # nestur
 
-Nestur is an NES emulator and a work in progress. There are likely still several bugs, and MMC3 support is not great. There are plenty of full-featured emulators out there; this is primarily an educational project but I do want it to run well.
+Nestur is an NES emulator. There are plenty of full-featured emulators out there; this is primarily an educational project but it is usable. There may still be many bugs, but I'm probably not aware of them so please submit issues.
 - SDL2 is the only dependency
 - no use of `unsafe`
 - NTSC timing
-- may only implement mappers 0-4 as these cover ~85% of games according to http://tuxnes.sourceforge.net/nesmapper.txt
+- supports mappers 0-4 which cover ~85% of [games](http://tuxnes.sourceforge.net/nesmapper.txt)
 
-<img src="pics/smb.png" width=400>  <img src="pics/zelda_dungeon.png" width=400>
+<img src="pics/smb.png" width=300> <img src="pics/zelda_dungeon.png" width=300> <img src="pics/kirby.png" width=300> <img src="pics/metroid.png" width=300> <img src="pics/smb3.png" width=300> <img src="pics/contra.png" width=300>
 
 ## Controls:
 ```
@@ -24,11 +24,11 @@ ___________________
 ```
 The code aims to follow the explanations from the [NES dev wiki](https://wiki.nesdev.com/w/index.php/NES_reference_guide) where possible, especially in the PPU, and the comments quote from it often. Thanks to everyone who contributes to that wiki/forum, and to Michael Fogleman's [NES](https://github.com/fogleman/nes) and Scott Ferguson's [Fergulator](https://github.com/scottferg/Fergulator) for getting me unstuck at several points.
 
-## Compilation and Use
+## Compilation and use
 
-1. Install Rust: https://www.rust-lang.org/tools/install
+1. Install [Rust](https://www.rust-lang.org/tools/install)
 2. Configure SDL2 for your platform:
-    - Windows: `SDL2.dll` is already in the repo so you don't have to do anything.
+    - Windows: `SDL2.dll` is already in the repo so you don't have to do anything
     - macOS: Install [Homebrew](https://brew.sh/) and run `brew install sdl2`
     - Linux: `sudo apt-get install libsdl2-dev` (or whatever your package manager is)
 3. `cd nestur/ && cargo build --release` (be sure to build/run with the release flag or it will run very slowly)
@@ -38,13 +38,17 @@ The code aims to follow the explanations from the [NES dev wiki](https://wiki.ne
 
 ## To do:
 
-- Fix any bugs with mappers 0-3 and improve mapper 4 (MMC3)
-
 - DMC audio channel, high- and low-pass filters
 
-- Save state/load functionality
+- Better GUI and distributable solution
+
+- Save states
 
 - Player 2 controller?
 
+## Known problem games
+
+- Paperboy: input doesn't work
+
 
 Please also check out [Cloaker](https://github.com/spieglt/cloaker) and [Flying Carpet](https://github.com/spieglt/flyingcarpet)!

src/audio.rs

@@ -51,8 +51,8 @@ pub fn initialize(sdl_context: &Sdl, buffer: Arc<Mutex<Vec<f32>>>)
         channels: Some(1), // mono
         samples: Some(SAMPLES_PER_FRAME)
     };
-    audio_subsystem.open_playback(None, &desired_spec, |spec| {
+    audio_subsystem.open_playback(None, &desired_spec, |_spec| {
-        // println!("{:?}", spec);
+        // println!("{:?}", _spec);
         ApuSampler{buffer, sample_ratio: APU_SAMPLE_RATE / (SDL_SAMPLE_RATE as f32)}
     })
 }

src/cartridge/mmc3.rs

@@ -158,7 +158,7 @@ impl Mapper for Mmc3 {
                     0x8000..=0x9FFF => self.bank_select(value),
                     0xA000..=0xBFFF => self.mirroring = if value & 1 == 0 {Mirror::Vertical} else {Mirror::Horizontal},
                     0xC000..=0xDFFF => self.irq_latch = value,
-                    0xE000..=0xFFFF => self.irq_enable = false,
+                    0xE000..=0xFFFF => {self.irq_enable = false; self.trigger_irq = false}, // Writing any value to this register will disable MMC3 interrupts AND acknowledge any pending interrupts.
                     _ => println!("bad address written to MMC3: 0x{:X}", address),
                 }
             },
@@ -186,19 +186,16 @@ impl Mapper for Mmc3 {
     fn load_battery_backed_ram(&mut self) {}
     fn save_battery_backed_ram(&self) {}
 
+    // This function is called by the PPU when the A12 address line changes.
+    // It's supposed to only be called when A12 goes from 0 to 1, but that doesn't work
+    // for my emulator for some reason.
     fn clock(&mut self) {
         if self.reload_counter {
             self.irq_counter = self.irq_latch;
             self.reload_counter = false;
         }
-        // "Should reload and set IRQ every clock when reload is 0."
-        if self.irq_latch == 0 && self.irq_enable {
-            self.irq_counter = self.irq_latch;
-            self.trigger_irq = true;
-        }
         if self.irq_counter == 0 {
             self.irq_counter = self.irq_latch;
-            return;
         } else {
             self.irq_counter -= 1;
         }
@@ -207,16 +204,15 @@ impl Mapper for Mmc3 {
         }
     }
 
+    // This function is called by the CPU every step (which takes more than one CPU clock cycle).
+    // I think I'm supposed to be tracking IRQ delays by the PPU, not letting an IRQ fire if
+    // there was one within the last 15 PPU cycles, but that didn't work and this does.
     fn check_irq(&mut self) -> bool {
-        // if self.trigger_irq {
-        //     self.trigger_irq = false;
-        //     true
-        // } else {
-        //     false
-        // }
         if self.trigger_irq {
             self.trigger_irq = false;
-            self.irq_delay = 15;
+            if self.irq_delay == 0 {
+                self.irq_delay = 5;
+            }
         }
         if self.irq_delay > 0 {
             self.irq_delay -= 1;

src/cartridge/nrom.rs

@@ -2,51 +2,49 @@ use super::{Cartridge, Mapper, Mirror};
 
 pub struct Nrom {
     cart: Cartridge,
+    chr_ram: Vec<u8>,
 }
 
 impl Nrom {
     pub fn new(cart: Cartridge) -> Self {
         Nrom{
             cart: cart,
+            chr_ram: vec![0; 0x2000],
         }
     }
 }
 
 impl Mapper for Nrom {
     fn read(&mut self, address: usize) -> u8 {
-        let cl = self.cart.chr_rom.len();
-        let pl = self.cart.prg_rom.len();
         let addr = address % 0x4000;
         match address {
             0x0000..=0x1FFF => {
-                if cl > 0 {
+                if self.cart.chr_rom_size > 0 {
-                    self.cart.chr_rom[cl-1][address]
+                    self.cart.chr_rom[0][address]
                 } else {
-                    0
+                    self.chr_ram[address]
                 }
             },
             0x8000..=0xBFFF => {
                 self.cart.prg_rom[0][addr]
             },
             0xC000..=0xFFFF => {
-                self.cart.prg_rom[pl-1][addr]
+                self.cart.prg_rom[self.cart.prg_rom_size - 1][addr]
             },
             _ => {println!("bad address read from NROM mapper: 0x{:X}", address); 0},
         }
     }
 
     fn write(&mut self, address: usize, value: u8) {
-        let cl = self.cart.chr_rom.len();
-        let pl = self.cart.prg_rom.len();
-        let addr = address % 0x4000;
         match address {
             0x0000..=0x1FFF => {
-                if cl > 0 {
+                // ROM isn't written to
-                    self.cart.chr_rom[cl-1][address] = value;
+                if self.cart.chr_rom_size == 0 {
+                    self.chr_ram[address] = value;
                 }
             },
-            0x8000..=0xBFFF => self.cart.prg_rom[0][addr] = value,
+            0x8000..=0xBFFF => (),
-            0xC000..=0xFFFF => self.cart.prg_rom[pl-1][addr] = value,
+            0xC000..=0xFFFF => (),
             _ => println!("bad address written to NROM mapper: 0x{:X}", address),
         }
     }

src/cpu/mod.rs

@@ -158,6 +158,8 @@ impl Cpu {
         if self.mapper.borrow_mut().check_irq() && (self.P & INTERRUPT_DISABLE_FLAG == 0) {
             self.irq();
         }
+        // TODO: should checks for APU and MMC3 IRQs be combined and acknowledged together?
+
 
         // back up clock so we know how many cycles we complete
         let clock = self.clock;

src/main.rs

@@ -118,7 +118,6 @@ fn main() -> Result<(), String> {
 /*
 
 TODO:
-- fix MMC3
 - high- and low-pass audio filters
 - DMC audio channel
 - untangle CPU and APU/PPU?
@@ -142,13 +141,4 @@ Failed tests from instr_test-v5/rom_singles/:
     CB AXS #n
 7, abs_xy, 'illegal opcode using abs x: 9c'
 
-
-
-
-
-
-1. A12 stuff controls when IRQs fire. Don't think there are serious problems with when IRQs fire anymore,
-though vertical scroll is still shaky in Kirby.
-2. Don't think the timing stuff is related to the palette and background table issues in SMB2/3.
-How can the palette be wrong? Or is it not, but the attribute bits are reading wrong?
 */

src/ppu/cpu_registers.rs

@@ -110,7 +110,9 @@ impl super::Ppu {
                 set_bit(&mut self.t, 0xC, d, 0x4);
                 set_bit(&mut self.t, 0xD, d, 0x5);
                 // t: X...... ........ = 0
-                set_bit(&mut self.t, 0xF, 0, 0);
+                set_bit(&mut self.t, 0xE, 0, 0);
+                // setting the 16th instead of the 15th bit in the line above was the longest, most frustrating oversight.
+                // caused weird problems with vertical scrolling and backgrounds that I initially thought were bugs with MMC3.
                 self.w = 1;
             },
             1 => { // second write
@@ -169,7 +171,6 @@ impl super::Ppu {
             },
             _ => panic!("reading from invalid PPU address: 0x{:04x}", self.v),
         };
-
         if self.rendering() && (self.scanline < 240 || self.scanline == 261) {
             // During rendering (on the pre-render line and the visible lines 0-239, provided either background or sprite rendering is enabled),
             // it will update v in an odd way, triggering a coarse X increment and a Y increment simultaneously (with normal wrapping behavior).

src/ppu/memory.rs

@@ -2,9 +2,8 @@ use crate::cartridge::Mirror;
 
 impl super::Ppu {
 
-    pub fn read(&mut self, addr: usize) -> u8 {
+    pub fn read(&mut self, address: usize) -> u8 {
-        let address = addr % 0x4000;
+        match address {
-        match addr {
             0x0000..=0x1FFF => self.mapper.borrow_mut().read(address),
             0x2000..=0x3EFF => self.read_nametable(address),
             0x3F00..=0x3FFF => {
@@ -16,9 +15,9 @@ impl super::Ppu {
         }
     }
 
-    pub fn write(&mut self, addr: usize, value: u8) {
+    pub fn write(&mut self, address: usize, value: u8) {
-        let address = addr % 0x4000;
+        // let address = addr % 0x4000;
-        match addr {
+        match address {
             0x0000..=0x1FFF => self.mapper.borrow_mut().write(address, value),
             0x2000..=0x3EFF => self.write_nametable(address, value),
             0x3F00..=0x3FFF => {

src/ppu/mod.rs

@@ -249,13 +249,10 @@ impl Ppu {
         let current_a12 = if self.v & 1 << 12 != 0 { 1 } else { 0 };
         if rendering
             && (0..241).contains(&self.scanline)
-            // && (current_a12 == 1 && self.previous_a12 == 0)
             && current_a12 != self.previous_a12
         {
-            // println!("clocking");
             self.mapper.borrow_mut().clock()
         }
-        // println!("current: {}, previous: {}", current_a12, self.previous_a12);
         self.previous_a12 = current_a12;
 
         (pixel, end_of_frame)