chore: imrpove private and public APIs

Cargo.lock

@@ -810,6 +810,7 @@ dependencies = [
  "pollster",
  "rodio",
  "rtrb",
+ "thiserror",
  "tracing",
  "tracing-subscriber",
  "wgpu",

Cargo.toml

@@ -22,6 +22,7 @@ rtrb = "0.2"
 directories-next = "2.0"
 tracing = "0.1"
 tracing-subscriber = { version = "0.3", features = ["std", "env-filter"] }
+thiserror = "1.0.30"
 
 [profile.release]
 debug = true

src/apu.rs

@@ -30,7 +30,7 @@ pub struct Apu {
     fs: FrameSequencer,
     div_prev: Option<u16>,
 
-    prod: Option<SampleProducer<f32>>,
+    pub(crate) prod: Option<SampleProducer<f32>>,
     sample_counter: u64,
 
     cap: f32,
@@ -188,10 +188,6 @@ impl Apu {
         }
     }
 
-    pub fn attach_producer(&mut self, prod: SampleProducer<f32>) {
-        self.prod = Some(prod);
-    }
-
     fn reset(&mut self) {
         self.ch1.sweep = Default::default();
         self.ch1.duty = Default::default();

src/bus.rs

@@ -19,7 +19,7 @@ pub struct Bus {
     var_ram: VariableWorkRam,
     timer: Timer,
     int: Interrupt,
-    apu: Apu,
+    pub(crate) apu: Apu,
     high_ram: HighRam,
     serial: Serial,
     pub(crate) joyp: Joypad,
@@ -51,14 +51,6 @@ impl Bus {
         }
     }
 
-    pub(crate) fn load_cart(&mut self, rom: Vec<u8>) {
-        self.cart = Some(Cartridge::new(rom));
-    }
-
-    pub(crate) fn cart_title(&self) -> Option<&str> {
-        self.cart.as_ref()?.title()
-    }
-
     #[allow(dead_code)]
     pub(crate) fn boot_mapped(&self) -> bool {
         self.boot.is_some()
@@ -89,11 +81,6 @@ impl Bus {
             self.oam_write_byte(dest_addr, byte);
         }
     }
-
-    #[inline]
-    pub(crate) fn apu_mut(&mut self) -> &mut Apu {
-        &mut self.apu
-    }
 }
 
 impl Bus {

src/cartridge.rs

@@ -13,7 +13,7 @@ const ROM_TITLE_MAX_SIZE: usize = 16;
 #[derive(Debug, Default)]
 pub(crate) struct Cartridge {
     memory: Vec<u8>,
-    title: Option<String>,
+    pub(crate) title: Option<String>,
     mbc: Box<dyn MBCIo>,
 }
 
@@ -80,10 +80,6 @@ impl Cartridge {
         }
     }
 
-    pub(crate) fn title(&self) -> Option<&str> {
-        self.title.as_deref()
-    }
-
     fn detect_ram_info(memory: &[u8]) -> RamSize {
         let id = memory[RAM_SIZE_ADDRESS];
         id.into()

src/cpu.rs

@@ -5,7 +5,7 @@ use crate::Cycle;
 use bitfield::bitfield;
 use std::fmt::{Display, Formatter, Result as FmtResult};
 
-#[derive(Debug, Default)]
+#[derive(Debug)]
 pub struct Cpu {
     pub(crate) bus: Bus,
     reg: Registers,
@@ -15,29 +15,13 @@ pub struct Cpu {
 }
 
 impl Cpu {
-    #[allow(dead_code)]
+    pub(crate) fn new(rom: [u8; BOOT_SIZE]) -> Self {
-    pub(crate) fn without_boot() -> Self {
-        Self {
-            reg: Registers {
-                a: 0x01,
-                b: 0x00,
-                c: 0x13,
-                d: 0x00,
-                e: 0xD8,
-                h: 0x01,
-                l: 0x4D,
-                sp: 0xFFFE,
-                pc: 0x0100,
-            },
-            flags: 0xb0.into(),
-            ..Default::default()
-        }
-    }
-
-    pub(crate) fn with_boot(rom: [u8; BOOT_SIZE]) -> Self {
         Self {
             bus: Bus::with_boot(rom),
-            ..Default::default()
+            reg: Default::default(),
+            flags: Default::default(),
+            ime: Default::default(),
+            state: Default::default(),
         }
     }
 
@@ -69,6 +53,12 @@ impl Cpu {
     }
 }
 
+impl Default for Cpu {
+    fn default() -> Self {
+        Cpu::new(*include_bytes!("../bin/bootix_dmg.bin"))
+    }
+}
+
 impl Cpu {
     /// Fetch an [Instruction] from the memory bus
     /// (4 cycles)

src/emu.rs

@@ -1,5 +1,6 @@
 use crate::apu::gen::SampleProducer;
 use crate::bus::BOOT_SIZE;
+use crate::cartridge::Cartridge;
 use crate::cpu::Cpu;
 use crate::{Cycle, GB_HEIGHT, GB_WIDTH};
 use clap::crate_name;
@@ -8,132 +9,146 @@ use std::fs::File;
 use std::io::{Read, Write};
 use std::path::{Path, PathBuf};
 use std::time::Duration;
+use thiserror::Error;
 use winit::event::KeyboardInput;
+use winit::event_loop::ControlFlow;
 
 pub const SM83_CYCLE_TIME: Duration = Duration::from_nanos(1_000_000_000 / SM83_CLOCK_SPEED);
 pub const CYCLES_IN_FRAME: Cycle = 456 * 154; // 456 Cycles times 154 scanlines
 pub(crate) const SM83_CLOCK_SPEED: u64 = 0x40_0000; // Hz which is 4.194304Mhz
 const DEFAULT_TITLE: &str = "DMG-01 Emulator";
 
-pub fn run_frame(emu: &mut Emulator, gamepad: &mut Gilrs, key: KeyboardInput) -> Cycle {
+pub fn run_frame(cpu: &mut Cpu, gamepad: &mut Gilrs, key: KeyboardInput) -> Cycle {
     let mut elapsed = 0;
 
     if let Some(event) = gamepad.next_event() {
-        crate::joypad::handle_gamepad_input(&mut emu.cpu.bus.joyp, event);
+        crate::joypad::handle_gamepad_input(&mut cpu.bus.joyp, event);
     }
-    crate::joypad::handle_keyboard_input(&mut emu.cpu.bus.joyp, key);
+    crate::joypad::handle_keyboard_input(&mut cpu.bus.joyp, key);
 
     while elapsed < CYCLES_IN_FRAME {
-        elapsed += emu.step();
+        elapsed += cpu.step();
     }
 
     elapsed
 }
 
-pub fn pixel_buf(emu: &Emulator) -> &[u8; GB_HEIGHT * GB_WIDTH * 4] {
+pub fn save_and_exit(cpu: &Cpu, control_flow: &mut ControlFlow) {
-    emu.cpu.bus.ppu.frame_buf.as_ref()
+    write_save(cpu);
+    *control_flow = ControlFlow::Exit;
 }
 
-pub struct Emulator {
+#[inline]
-    cpu: Cpu,
+pub fn pixel_buf(cpu: &Cpu) -> &[u8; GB_HEIGHT * GB_WIDTH * 4] {
-    timestamp: Cycle,
+    cpu.bus.ppu.frame_buf.as_ref()
 }
 
-impl Default for Emulator {
+pub fn from_boot_rom<P: AsRef<Path>>(path: P) -> std::io::Result<Cpu> {
-    fn default() -> Self {
+    Ok(Cpu::new(read_boot(path)?))
-        Self::new()
+}
+
+pub fn read_game_rom<P: AsRef<Path>>(cpu: &mut Cpu, path: P) -> std::io::Result<()> {
+    cpu.bus.cart = Some(Cartridge::new(std::fs::read(path.as_ref())?));
+    Ok(())
+}
+
+pub fn set_audio_prod(cpu: &mut Cpu, prod: SampleProducer<f32>) {
+    cpu.bus.apu.prod = Some(prod);
+}
+
+pub fn rom_title(cpu: &Cpu) -> &str {
+    cpu.bus
+        .cart
+        .as_ref()
+        .map(|c| c.title.as_deref())
+        .flatten()
+        .unwrap_or(DEFAULT_TITLE)
+}
+
+pub fn write_save(cpu: &Cpu) {
+    match cpu.bus.cart.as_ref() {
+        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::Io(err)) => tracing::error!("{:?}", err),
+        },
+        None => tracing::error!("No cartridge is currently present"),
     }
 }
 
-impl Emulator {
+pub fn load_save(cpu: &mut Cpu) {
-    pub fn new() -> Self {
+    match cpu.bus.cart.as_mut() {
-        Self {
+        Some(cart) => match read_save_from_file(cart) {
-            cpu: Cpu::with_boot(*include_bytes!("../bin/bootix_dmg.bin")),
+            Ok(path) => tracing::info!("Loaded save from {:?}", path),
-            timestamp: Default::default(),
+            Err(err @ SaveError::NotApplicable) => tracing::warn!("Unable to load save: {:?}", err),
-        }
+            Err(SaveError::Io(err)) => match err.kind() {
-    }
+                std::io::ErrorKind::NotFound => tracing::warn!("Save not found"),
-
+                _ => tracing::error!("{:?}", err),
-    pub fn from_boot_rom<P: AsRef<Path>>(path: P) -> std::io::Result<Self> {
+            },
-        Ok(Self {
+        },
-            cpu: Cpu::with_boot(Self::read_boot(path)?),
+        None => tracing::error!("No cartridge is currently present"),
-            timestamp: Default::default(),
-        })
-    }
-
-    fn read_boot<P: AsRef<Path>>(path: P) -> std::io::Result<[u8; BOOT_SIZE]> {
-        let mut buf = [0; BOOT_SIZE];
-        let mut file = File::open(path.as_ref())?;
-
-        file.read_exact(&mut buf)?;
-        Ok(buf)
-    }
-
-    fn step(&mut self) -> Cycle {
-        let cycles = self.cpu.step();
-        self.timestamp += cycles;
-        cycles
-    }
-
-    pub fn read_game_rom<P: AsRef<Path>>(&mut self, path: P) -> std::io::Result<()> {
-        self.load_rom(std::fs::read(path.as_ref())?);
-        Ok(())
-    }
-
-    fn load_rom(&mut self, rom: Vec<u8>) {
-        self.cpu.bus.load_cart(rom);
-    }
-
-    pub fn set_prod(&mut self, prod: SampleProducer<f32>) {
-        self.cpu.bus.apu_mut().attach_producer(prod)
-    }
-
-    pub fn title(&self) -> &str {
-        self.cpu.bus.cart_title().unwrap_or(DEFAULT_TITLE)
-    }
-
-    pub fn try_write_sav(&self) -> std::io::Result<()> {
-        if let Some(ext_ram) = self.cpu.bus.cart.as_ref().map(|c| c.ext_ram()).flatten() {
-            if let Some(title) = self.cpu.bus.cart_title() {
-                let mut save_path = Self::data_path().unwrap_or_else(|| PathBuf::from("."));
-                save_path.push(title);
-                save_path.set_extension("sav");
-
-                let mut file = File::create(save_path)?;
-                file.write_all(ext_ram)?;
-            }
-        }
-
-        Ok(())
-    }
-
-    pub fn try_load_sav(&mut self) -> std::io::Result<()> {
-        if let Some(cart) = &mut self.cpu.bus.cart {
-            if let Some(title) = cart.title() {
-                let mut save_path = Self::data_path().unwrap_or_else(|| PathBuf::from("."));
-                save_path.push(title);
-                save_path.set_extension("sav");
-
-                if let Ok(mut file) = File::open(&save_path) {
-                    tracing::info!("Load {:?}", save_path);
-
-                    let mut memory = Vec::new();
-                    file.read_to_end(&mut memory)?;
-                    cart.write_ext_ram(memory);
-                }
-            }
-        }
-
-        Ok(())
-    }
-
-    fn data_path() -> Option<PathBuf> {
-        match directories_next::ProjectDirs::from("dev", "musuka", crate_name!()) {
-            Some(dirs) => {
-                let data_local = dirs.data_local_dir();
-                std::fs::create_dir_all(data_local).ok()?;
-                Some(data_local.to_path_buf())
-            }
-            None => None,
-        }
     }
 }
+
+fn write_save_to_file(cart: &Cartridge) -> Result<PathBuf, SaveError> {
+    match cart.title.as_ref().zip(cart.ext_ram()) {
+        Some((title, ram)) => {
+            let mut save_path = data_path().unwrap_or_else(|| PathBuf::from("."));
+            save_path.push(title);
+            save_path.set_extension("sav");
+
+            let mut file = File::create(&save_path)?;
+            file.write_all(ram)?;
+            Ok(save_path)
+        }
+        None => Err(SaveError::NotApplicable),
+    }
+}
+
+fn read_save_from_file(cart: &mut Cartridge) -> Result<PathBuf, SaveError> {
+    match cart.title.as_deref() {
+        Some(title) => {
+            let mut save_path = data_path().unwrap_or_else(|| PathBuf::from("."));
+            save_path.push(title);
+            save_path.set_extension("sav");
+
+            let mut file = File::open(&save_path)?;
+            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);
+            Ok(save_path)
+        }
+        None => Err(SaveError::NotApplicable),
+    }
+}
+
+fn read_boot<P: AsRef<Path>>(path: P) -> std::io::Result<[u8; BOOT_SIZE]> {
+    let mut buf = [0; BOOT_SIZE];
+    let mut file = File::open(path.as_ref())?;
+
+    file.read_exact(&mut buf)?;
+    Ok(buf)
+}
+
+fn data_path() -> Option<PathBuf> {
+    match directories_next::ProjectDirs::from("dev", "musuka", crate_name!()) {
+        Some(dirs) => {
+            let data_local = dirs.data_local_dir();
+            std::fs::create_dir_all(data_local).ok()?;
+            Some(data_local.to_path_buf())
+        }
+        None => None,
+    }
+}
+
+#[derive(Debug, Error)]
+pub enum SaveError {
+    #[error("cartridge lacks title and/or external ram")]
+    NotApplicable,
+    #[error(transparent)]
+    Io(#[from] std::io::Error),
+}

src/main.rs

@@ -2,13 +2,13 @@ use std::time::Instant;
 
 use clap::{crate_authors, crate_description, crate_name, crate_version, App, Arg};
 use egui_wgpu_backend::RenderPass;
-use gb::emu::Emulator;
+use gb::emu;
 use gb::gui::GuiState;
 use gilrs::Gilrs;
 use rodio::{OutputStream, Sink};
 use tracing_subscriber::EnvFilter;
 use winit::event::{Event, WindowEvent};
-use winit::event_loop::{ControlFlow, EventLoop};
+use winit::event_loop::EventLoop;
 
 const AUDIO_ENABLED: bool = true;
 
@@ -63,33 +63,32 @@ fn main() {
 
     // We interrupt your boiler plate to initialize the emulator so that
     // we can copy it's empty pixel buffer to the GPU
-    let mut emu = match m.value_of("boot") {
+    let mut cpu = match m.value_of("boot") {
         Some(path) => {
             tracing::info!("User-provided boot ROM");
-            Emulator::from_boot_rom(path).expect("initialize emulator with custom boot rom")
+            emu::from_boot_rom(path).expect("initialize emulator with custom boot rom")
         }
         None => {
             tracing::info!("Built-in boot ROM");
-            Emulator::new()
+            Default::default()
         }
     };
 
     // Set up the WGPU (and then EGUI) texture we'll be working with.
     let texture_size = gb::gui::texture_size();
     let texture = gb::gui::create_texture(&device, texture_size);
-    gb::gui::write_to_texture(&queue, &texture, gb::emu::pixel_buf(&emu), texture_size);
+    gb::gui::write_to_texture(&queue, &texture, gb::emu::pixel_buf(&cpu), texture_size);
     let texture_id = gb::gui::expose_texture_to_egui(&mut render_pass, &device, &texture);
 
     // Load ROM if filepath was provided
     if let Some(path) = m.value_of("rom") {
         tracing::info!("User-provided cartridge ROM");
-        emu.read_game_rom(path).expect("read game rom from path");
+        emu::read_game_rom(&mut cpu, path).expect("read game rom from path");
     }
 
-    // Load Save File if it exists
+    emu::load_save(&mut cpu);
-    // FIXME: Shouldn't the API be better than this?
+
-    emu.try_load_sav().expect("Load save if exists");
+    let rom_title = emu::rom_title(&cpu).to_string();
-    let rom_title = emu.title().to_string();
 
     tracing::info!("Initialize Gamepad");
     let mut gamepad = Gilrs::new().expect("Initialize Controller Support");
@@ -106,7 +105,7 @@ fn main() {
             s
         };
 
-        emu.set_prod(prod);
+        emu::set_audio_prod(&mut cpu, prod);
 
         tracing::info!("Spawn Audio Thread");
         std::thread::spawn(move || {
@@ -127,17 +126,17 @@ fn main() {
         match event {
             Event::MainEventsCleared => {
                 if app.quit {
-                    *control_flow = ControlFlow::Exit;
+                    emu::save_and_exit(&cpu, control_flow);
                 }
 
-                gb::emu::run_frame(&mut emu, &mut gamepad, last_key);
+                gb::emu::run_frame(&mut cpu, &mut gamepad, last_key);
 
                 window.request_redraw();
             }
             Event::RedrawRequested(..) => {
                 platform.update_time(start_time.elapsed().as_secs_f64());
 
-                let data = gb::emu::pixel_buf(&emu);
+                let data = gb::emu::pixel_buf(&cpu);
                 gb::gui::write_to_texture(&queue, &texture, data, texture_size);
 
                 let output_frame = match surface.get_current_texture() {
@@ -187,7 +186,7 @@ fn main() {
                     surface.configure(&device, &config);
                 }
                 WindowEvent::CloseRequested => {
-                    *control_flow = ControlFlow::Exit;
+                    emu::save_and_exit(&cpu, control_flow);
                 }
                 WindowEvent::KeyboardInput { input, .. } => last_key = input,
                 _ => {}