Break io into chirp-minifb, and refactor to use Results in more places

justfile

@@ -1,10 +1,17 @@
 # Some common commands for working on this stuff
 
-test:
+# Run All Tests
+rat:
     cargo test --doc && cargo nextest run
 
+test:
+    cargo nextest run
+
 chirp:
     cargo run --bin chirp-minifb -- tests/chip8-test-suite/bin/chip8-test-suite.ch8
+# Run at 2100000 instructions per frame, and output per-frame runtime statistics
+bench:
+    cargo run --bin chirp-minifb --release -- chip-8/1dcell.ch8 -xP -s10 -S2100000
 
 cover:
     cargo llvm-cov --open --doctests

src/bin/chirp-minifb/io.rs

@@ -10,7 +10,7 @@ use std::{
     time::Instant,
 };
 
-use crate::{
+use chirp::{
     bus::{Bus, Region},
     error::Result,
     Chip8,
@@ -27,17 +27,15 @@ pub struct UIBuilder {
 }
 
 impl UIBuilder {
-    pub fn new(height: usize, width: usize) -> Self {
+    #[allow(dead_code)] // this code is used in tests thank you
+    pub fn new(width: usize, height: usize, rom: impl AsRef<Path>) -> Self {
         UIBuilder {
             width,
             height,
+            rom: Some(rom.as_ref().to_owned()),
             ..Default::default()
         }
     }
-    pub fn rom(&mut self, path: impl AsRef<Path>) -> &mut Self {
-        self.rom = Some(path.as_ref().into());
-        self
-    }
     pub fn build(&self) -> Result<UI> {
         let ui = UI {
             window: Window::new(
@@ -147,8 +145,8 @@ impl UI {
                 self.window.set_title("Chirp  ⏸")
             } else {
                 self.window.set_title(&format!(
-                    "Chirp  ▶ {:2?}",
+                    "Chirp  ▶ {:02.02}",
-                    (1.0 / self.time.elapsed().as_secs_f64()).trunc()
+                    (1.0 / self.time.elapsed().as_secs_f64())
                 ));
             }
             if !self.window.is_open() {
@@ -177,7 +175,9 @@ impl UI {
         };
         use crate::io::Region::*;
         for key in get_keys_released() {
-            ch8.cpu.release(identify_key(key));
+            if let Some(key) = identify_key(key) {
+                ch8.cpu.release(key)?;
+            }
         }
         // handle keybinds for the UI
         for key in get_keys_pressed() {
@@ -227,7 +227,11 @@ impl UI {
                     ch8.bus.clear_region(Screen);
                 }
                 Escape => return Ok(None),
-                key => ch8.cpu.press(identify_key(key)),
+                key => {
+                    if let Some(key) = identify_key(key) {
+                        ch8.cpu.press(key)?;
+                    }
+                }
             }
         }
         self.keyboard = self.window.get_keys();
@@ -235,48 +239,28 @@ impl UI {
     }
 }
 
-pub const KEYMAP: [Key; 16] = [
+pub fn identify_key(key: Key) -> Option<usize> {
-    Key::X,
-    Key::Key1,
-    Key::Key2,
-    Key::Key3,
-    Key::Q,
-    Key::W,
-    Key::E,
-    Key::A,
-    Key::S,
-    Key::D,
-    Key::Z,
-    Key::C,
-    Key::Key4,
-    Key::R,
-    Key::F,
-    Key::V,
-];
-
-pub fn identify_key(key: Key) -> usize {
     match key {
-        Key::Key1 => 0x1,
+        Key::Key1 => Some(0x1),
-        Key::Key2 => 0x2,
+        Key::Key2 => Some(0x2),
-        Key::Key3 => 0x3,
+        Key::Key3 => Some(0x3),
-        Key::Key4 => 0xc,
+        Key::Key4 => Some(0xc),
-        Key::Q => 0x4,
+        Key::Q => Some(0x4),
-        Key::W => 0x5,
+        Key::W => Some(0x5),
-        Key::E => 0x6,
+        Key::E => Some(0x6),
-        Key::R => 0xD,
+        Key::R => Some(0xD),
-        Key::A => 0x7,
+        Key::A => Some(0x7),
-        Key::S => 0x8,
+        Key::S => Some(0x8),
-        Key::D => 0x9,
+        Key::D => Some(0x9),
-        Key::F => 0xE,
+        Key::F => Some(0xE),
-        Key::Z => 0xA,
+        Key::Z => Some(0xA),
-        Key::X => 0x0,
+        Key::X => Some(0x0),
-        Key::C => 0xB,
+        Key::C => Some(0xB),
-        Key::V => 0xF,
+        Key::V => Some(0xF),
-        _ => 0x10,
+        _ => None,
     }
 }
 
-#[cfg_attr(feature = "unstable", no_coverage)]
 pub fn debug_dump_screen(ch8: &Chip8, rom: &Path) -> Result<()> {
     let path = PathBuf::new()
         .join("src/cpu/tests/screens/")

src/bin/chirp-minifb/main.rs

@@ -4,8 +4,13 @@
 //! Chirp: A chip-8 interpreter in Rust
 //! Hello, world!
 
+mod io;
+#[cfg(test)]
+mod tests;
+
 use chirp::{error::Result, prelude::*};
 use gumdrop::*;
+use io::*;
 use owo_colors::OwoColorize;
 use std::fs::read;
 use std::{
@@ -95,7 +100,7 @@ impl State {
             ch8: Chip8 {
                 bus: bus! {
                     // Load the charset into ROM
-                    Charset [0x0050..0x00A0] = include_bytes!("../mem/charset.bin"),
+                    Charset [0x0050..0x00A0] = include_bytes!("../../mem/charset.bin"),
                     // Load the ROM file into RAM
                     Program [0x0200..0x1000] = &read(&options.file)?,
                     // Create a screen
@@ -125,7 +130,7 @@ impl State {
                     },
                 ),
             },
-            ui: UIBuilder::default().rom(&options.file).build()?,
+            ui: UIBuilder::new(64, 32, &options.file).build()?,
             ft: Instant::now(),
         };
         state.ch8.bus.write(0x1feu16, options.data);

src/bin/chirp-minifb/tests.rs

@@ -0,0 +1,146 @@
+//! Tests for chirp-minifb
+
+use super::io::*;
+use chirp::prelude::*;
+use std::{collections::hash_map::DefaultHasher, hash::Hash};
+
+mod ui_builder {
+    use super::*;
+    #[test]
+    fn ui_builder() -> Result<()> {
+        let builder = UIBuilder::new(32, 64, "dummy.ch8").build()?;
+        println!("{builder:?}");
+        Ok(())
+    }
+    #[test]
+    fn default() {
+        let ui_builder = UIBuilder::default();
+        println!("{ui_builder:?}");
+    }
+    #[test]
+    #[allow(clippy::redundant_clone)]
+    fn clone_debug() {
+        let ui_builder_clone = UIBuilder::default().clone();
+        println!("{ui_builder_clone:?}");
+    }
+}
+mod ui {
+    use super::*;
+    fn new_chip8() -> Chip8 {
+        Chip8 {
+            cpu: CPU::default(),
+            bus: bus! {},
+        }
+    }
+    #[test]
+    fn frame() -> Result<()> {
+        let mut ui = UIBuilder::new(32, 64, "dummy.ch8").build()?;
+        let mut ch8 = new_chip8();
+        ui.frame(&mut ch8).unwrap();
+        Ok(())
+    }
+    #[test]
+    fn keys() -> Result<()> {
+        let mut ui = UIBuilder::new(32, 64, "dummy.ch8").build()?;
+        let mut ch8 = new_chip8();
+        let ch8 = &mut ch8;
+        ui.frame(ch8).unwrap();
+        ui.keys(ch8).unwrap();
+        Ok(())
+    }
+    #[test]
+    fn debug() -> Result<()> {
+        println!("{:?}", UIBuilder::new(32, 64, "dummy.ch8").build()?);
+        Ok(())
+    }
+}
+
+mod framebuffer_format {
+
+    use super::*;
+    #[test]
+    fn default() {
+        let _fbf = FrameBufferFormat::default();
+    }
+    #[test]
+    fn clone() {
+        let fbf = FrameBufferFormat {
+            fg: 0x12345678,
+            bg: 0x90abcdef,
+        };
+        let fbf2 = fbf.clone();
+        assert_eq!(fbf, fbf2);
+    }
+    #[test]
+    fn debug() {
+        println!("{:?}", FrameBufferFormat::default());
+    }
+    #[test]
+    fn eq() {
+        assert_eq!(FrameBufferFormat::default(), FrameBufferFormat::default());
+        assert_ne!(
+            FrameBufferFormat {
+                fg: 0xff00ff,
+                bg: 0x00ff00
+            },
+            FrameBufferFormat {
+                fg: 0x00ff00,
+                bg: 0xff00ff
+            },
+        );
+    }
+    #[test]
+    fn ord() {
+        assert!(
+            FrameBufferFormat::default()
+                == FrameBufferFormat {
+                    fg: 0xffffff,
+                    bg: 0xffffff,
+                }
+                .min(FrameBufferFormat::default())
+        );
+    }
+    #[test]
+    fn hash() {
+        let mut hasher = DefaultHasher::new();
+        FrameBufferFormat::default().hash(&mut hasher);
+        println!("{hasher:?}");
+    }
+}
+
+mod framebuffer {
+    use super::*;
+    // [derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
+    #[test]
+    fn new() {
+        assert_eq!(FrameBuffer::new(64, 32), FrameBuffer::default());
+    }
+    #[test]
+    fn clone() {
+        let fb1 = FrameBuffer::default();
+        let fb2 = fb1.clone();
+        assert_eq!(fb1, fb2);
+    }
+
+    #[test]
+    fn debug() {
+        println!("{:?}", FrameBuffer::default());
+    }
+
+    #[test]
+    fn eq() {
+        assert_eq!(FrameBuffer::new(64, 32), FrameBuffer::default());
+    }
+
+    #[test]
+    fn ord() {
+        assert!(FrameBuffer::new(21, 12) == FrameBuffer::new(21, 12).min(FrameBuffer::new(34, 46)));
+    }
+
+    #[test]
+    fn hash() {
+        let mut hasher = DefaultHasher::new();
+        FrameBuffer::default().hash(&mut hasher);
+        println!("{hasher:?}");
+    }
+}

src/cpu.rs

@@ -17,7 +17,7 @@ pub mod disassembler;
 use self::disassembler::{Dis, Insn};
 use crate::{
     bus::{Bus, Read, Region, Write},
-    error::Result,
+    error::{Error, Result},
 };
 use imperative_rs::InstructionSet;
 use owo_colors::OwoColorize;
@@ -81,10 +81,10 @@ pub struct ControlFlags {
     /// Set when the emulator is waiting for a keypress
     pub keypause: bool,
     /// Set when the emulator is waiting for a frame to be drawn
-    pub vbi_wait: bool,
+    pub draw_wait: bool,
     /// Set to the last key that's been *released* after a keypause
     pub lastkey: Option<usize>,
-    /// Represents the set of emulator "[Quirks]" to enable
+    /// Represents the set of emulator [Quirks] to enable
     pub quirks: Quirks,
     /// Represents the number of instructions to run per tick of the internal timer
     pub monotonic: Option<usize>,
@@ -95,14 +95,12 @@ impl ControlFlags {
     ///
     /// # Examples
     /// ```rust
-    ///# use chirp::prelude::*;
+    /// # use chirp::prelude::*;
-    ///# fn main() -> Result<()> {
     /// let mut cpu = CPU::default();
     /// assert_eq!(true, cpu.flags.debug);
+    /// // Toggle debug mode
     /// cpu.flags.debug();
     /// assert_eq!(false, cpu.flags.debug);
-    ///#    Ok(())
-    ///# }
     /// ```
     pub fn debug(&mut self) {
         self.debug = !self.debug
@@ -112,14 +110,12 @@ impl ControlFlags {
     ///
     /// # Examples
     /// ```rust
-    ///# use chirp::prelude::*;
+    /// # use chirp::prelude::*;
-    ///# fn main() -> Result<()> {
     /// let mut cpu = CPU::default();
     /// assert_eq!(false, cpu.flags.pause);
+    /// // Pause the cpu
     /// cpu.flags.pause();
     /// assert_eq!(true, cpu.flags.pause);
-    ///#    Ok(())
-    ///# }
     /// ```
     pub fn pause(&mut self) {
         self.pause = !self.pause
@@ -206,63 +202,90 @@ impl CPU {
         }
     }
 
-    /// Presses a key
+    /// Presses a key, and reports whether the key's state changed.  
+    /// If key does not exist, returns [Error::InvalidKey].
+    ///
     /// # Examples
     /// ```rust
-    ///# use chirp::prelude::*;
+    /// # use chirp::prelude::*;
-    ///# fn main() -> Result<()> {
     /// let mut cpu = CPU::default();
-    ///     cpu.press(0x7);
+    ///
-    ///     cpu.press(0xF);
+    /// // press key `7`
-    ///#    Ok(())
+    /// let did_press = cpu.press(0x7).unwrap();
-    ///# }
+    /// assert!(did_press);
+    ///
+    /// // press key `7` again, even though it's already pressed
+    /// let did_press = cpu.press(0x7).unwrap();
+    /// // it was already pressed, so nothing's changed.
+    /// assert!(!did_press);
     /// ```
-    pub fn press(&mut self, key: usize) {
+    pub fn press(&mut self, key: usize) -> Result<bool> {
-        if (0..16).contains(&key) {
+        if let Some(keyref) = self.keys.get_mut(key) {
-            self.keys[key] = true;
+            if !*keyref {
+                *keyref = true;
+                return Ok(true);
+            } // else do nothing
+        } else {
+            return Err(Error::InvalidKey { key });
         }
+        Ok(false)
     }
 
-    /// Releases a key
+    /// Releases a key, and reports whether the key's state changed.  
+    /// If key is outside range `0..=0xF`, returns [Error::InvalidKey].
     ///
-    /// If keypause is enabled, this disables keypause and records the last released key.
+    /// If [ControlFlags::keypause] was enabled, it is disabled,
+    /// and the [ControlFlags::lastkey] is recorded.
     /// # Examples
     /// ```rust
-    ///# use chirp::prelude::*;
+    /// # use chirp::prelude::*;
-    ///# fn main() -> Result<()> {
     /// let mut cpu = CPU::default();
-    ///     cpu.press(0x7);
+    /// // press key `7`
-    ///     cpu.release(0x7);
+    /// cpu.press(0x7).unwrap();
-    ///#    Ok(())
+    /// // release key `7`
-    ///# }
+    /// let changed = cpu.release(0x7).unwrap();
+    /// assert!(changed); // key released
+    /// // try releasing `7` again
+    /// let changed = cpu.release(0x7).unwrap();
+    /// assert!(!changed); // key was not held
     /// ```
-    pub fn release(&mut self, key: usize) {
+    pub fn release(&mut self, key: usize) -> Result<bool> {
-        if (0..16).contains(&key) {
+        if let Some(keyref) = self.keys.get_mut(key) {
-            self.keys[key] = false;
+            if *keyref {
+                *keyref = false;
                 if self.flags.keypause {
                     self.flags.lastkey = Some(key);
-            }
                     self.flags.keypause = false;
                 }
+                return Ok(true);
+            }
+        } else {
+            return Err(Error::InvalidKey { key });
+        }
+        Ok(false)
     }
 
-    /// Sets a general purpose register in the CPU
+    /// Sets a general purpose register in the CPU.  
+    /// If the register doesn't exist, returns [Error::InvalidRegister]
     /// # Examples
     /// ```rust
     /// # use chirp::prelude::*;
     /// // Create a new CPU, and set v4 to 0x41
     /// let mut cpu = CPU::default();
-    /// cpu.set_v(0x4, 0x41);
+    /// cpu.set_v(0x4, 0x41).unwrap();
     /// // Dump the CPU registers
     /// cpu.dump();
     /// ```
-    pub fn set_v(&mut self, gpr: Reg, value: u8) {
+    pub fn set_v(&mut self, reg: Reg, value: u8) -> Result<()> {
-        if let Some(gpr) = self.v.get_mut(gpr) {
+        if let Some(gpr) = self.v.get_mut(reg) {
             *gpr = value;
+            Ok(())
+        } else {
+            Err(Error::InvalidRegister { reg })
         }
     }
 
-    /// Gets a slice of the entire general purpose register field
+    /// Gets a slice of the entire general purpose registers
     /// # Examples
     /// ```rust
     /// # use chirp::prelude::*;
@@ -281,12 +304,9 @@ impl CPU {
     /// Gets the program counter
     /// # Examples
     /// ```rust
-    ///# use chirp::prelude::*;
+    /// # use chirp::prelude::*;
-    ///# fn main() -> Result<()> {
     /// let mut cpu = CPU::default();
     /// assert_eq!(0x200, cpu.pc());
-    ///#    Ok(())
-    ///# }
     /// ```
     pub fn pc(&self) -> Adr {
         self.pc
@@ -295,12 +315,9 @@ impl CPU {
     /// Gets the I register
     /// # Examples
     /// ```rust
-    ///# use chirp::prelude::*;
+    /// # use chirp::prelude::*;
-    ///# fn main() -> Result<()> {
     /// let mut cpu = CPU::default();
     /// assert_eq!(0, cpu.i());
-    ///#    Ok(())
-    ///# }
     /// ```
     pub fn i(&self) -> Adr {
         self.i
@@ -309,12 +326,9 @@ impl CPU {
     /// Gets the value in the Sound Timer register
     /// # Examples
     /// ```rust
-    ///# use chirp::prelude::*;
+    /// # use chirp::prelude::*;
-    ///# fn main() -> Result<()> {
     /// let mut cpu = CPU::default();
     /// assert_eq!(0, cpu.sound());
-    ///#    Ok(())
-    ///# }
     /// ```
     pub fn sound(&self) -> u8 {
         self.sound as u8
@@ -323,12 +337,9 @@ impl CPU {
     /// Gets the value in the Delay Timer register
     /// # Examples
     /// ```rust
-    ///# use chirp::prelude::*;
+    /// # use chirp::prelude::*;
-    ///# fn main() -> Result<()> {
     /// let mut cpu = CPU::default();
     /// assert_eq!(0, cpu.delay());
-    ///#    Ok(())
-    ///# }
     /// ```
     pub fn delay(&self) -> u8 {
         self.delay as u8
@@ -340,12 +351,9 @@ impl CPU {
     /// updated even when the CPU is in drawpause or keypause
     /// # Examples
     /// ```rust
-    ///# use chirp::prelude::*;
+    /// # use chirp::prelude::*;
-    ///# fn main() -> Result<()> {
     /// let mut cpu = CPU::default();
     /// assert_eq!(0x0, cpu.cycle());
-    ///#    Ok(())
-    ///# }
     /// ```
     pub fn cycle(&self) -> usize {
         self.cycle
@@ -355,8 +363,7 @@ impl CPU {
     /// reinitializing the program counter to 0x200
     /// # Examples
     /// ```rust
-    ///# use chirp::prelude::*;
+    /// # use chirp::prelude::*;
-    ///# fn main() -> Result<()> {
     /// let mut cpu = CPU::new(
     ///     0xf00,
     ///     0x50,
@@ -367,19 +374,17 @@ impl CPU {
     ///     ControlFlags::default()
     /// );
     /// cpu.flags.keypause = true;
-    ///     cpu.flags.vbi_wait = true;
+    /// cpu.flags.draw_wait = true;
     /// assert_eq!(0x340, cpu.pc());
     /// cpu.soft_reset();
     /// assert_eq!(0x200, cpu.pc());
     /// assert_eq!(false, cpu.flags.keypause);
-    ///     assert_eq!(false, cpu.flags.vbi_wait);
+    /// assert_eq!(false, cpu.flags.draw_wait);
-    ///#    Ok(())
-    ///# }
     /// ```
     pub fn soft_reset(&mut self) {
         self.pc = 0x200;
         self.flags.keypause = false;
-        self.flags.vbi_wait = false;
+        self.flags.draw_wait = false;
     }
 
     /// Set a breakpoint
@@ -411,12 +416,9 @@ impl CPU {
     /// Gets a slice of breakpoints
     /// # Examples
     /// ```rust
-    ///# use chirp::prelude::*;
+    /// # use chirp::prelude::*;
-    ///# fn main() -> Result<()> {
     /// let mut cpu = CPU::default();
     /// assert_eq!(cpu.breakpoints(), &[]);
-    ///#    Ok(())
-    ///# }
     /// ```
     pub fn breakpoints(&self) -> &[Adr] {
         self.breakpoints.as_slice()
@@ -425,11 +427,13 @@ impl CPU {
     /// Unpauses the emulator for a single tick,
     /// even if cpu.flags.pause is set.
     ///
+    /// Like with [CPU::tick], this returns [Error::UnimplementedInstruction]
+    /// if the instruction is unimplemented.
+    ///
     /// NOTE: does not synchronize with delay timers
     /// # Examples
     /// ```rust
-    ///# use chirp::prelude::*;
+    /// # use chirp::prelude::*;
-    ///# fn main() -> Result<()> {
     /// let mut cpu = CPU::default();
     /// let mut bus = bus!{
     ///     Program [0x0200..0x0f00] = &[
@@ -438,16 +442,14 @@ impl CPU {
     ///     ],
     ///     Screen  [0x0f00..0x1000],
     /// };
-    ///     cpu.singlestep(&mut bus)?;
+    /// cpu.singlestep(&mut bus).unwrap();
     /// assert_eq!(0x202, cpu.pc());
     /// assert_eq!(1, cpu.cycle());
-    ///#    Ok(())
-    ///# }
     /// ```
     pub fn singlestep(&mut self, bus: &mut Bus) -> Result<&mut Self> {
         self.flags.pause = false;
         self.tick(bus)?;
-        self.flags.vbi_wait = false;
+        self.flags.draw_wait = false;
         self.flags.pause = true;
         Ok(self)
     }
@@ -457,8 +459,7 @@ impl CPU {
     /// Ticks the timers every `rate` ticks
     /// # Examples
     /// ```rust
-    ///# use chirp::prelude::*;
+    /// # use chirp::prelude::*;
-    ///# fn main() -> Result<()> {
     /// let mut cpu = CPU::default();
     /// let mut bus = bus!{
     ///     Program [0x0200..0x0f00] = &[
@@ -467,11 +468,10 @@ impl CPU {
     ///     ],
     ///     Screen  [0x0f00..0x1000],
     /// };
-    ///     cpu.multistep(&mut bus, 0x20)?;
+    /// cpu.multistep(&mut bus, 0x20)
+    ///     .expect("The program should only have valid opcodes.");
     /// assert_eq!(0x202, cpu.pc());
     /// assert_eq!(0x20, cpu.cycle());
-    ///#    Ok(())
-    ///# }
     /// ```
     pub fn multistep(&mut self, bus: &mut Bus, steps: usize) -> Result<&mut Self> {
         for _ in 0..steps {
@@ -497,8 +497,8 @@ impl CPU {
         }
         // Use a monotonic counter when testing
         if let Some(speed) = self.flags.monotonic {
-            if self.flags.vbi_wait {
+            if self.flags.draw_wait {
-                self.flags.vbi_wait = self.cycle % speed != 0;
+                self.flags.draw_wait = self.cycle % speed != 0;
             }
             let speed = 1.0 / speed as f64;
             self.delay -= speed;
@@ -510,7 +510,7 @@ impl CPU {
         let time = self.timers.frame.elapsed().as_secs_f64() * 60.0;
         self.timers.frame = Instant::now();
         if time > 1.0 {
-            self.flags.vbi_wait = false;
+            self.flags.draw_wait = false;
         }
         if self.delay > 0.0 {
             self.delay -= time;
@@ -524,8 +524,7 @@ impl CPU {
     /// Executes a single instruction
     /// # Examples
     /// ```rust
-    ///# use chirp::prelude::*;
+    /// # use chirp::prelude::*;
-    ///# fn main() -> Result<()> {
     /// let mut cpu = CPU::default();
     /// let mut bus = bus!{
     ///     Program [0x0200..0x0f00] = &[
@@ -534,20 +533,18 @@ impl CPU {
     ///     ],
     ///     Screen  [0x0f00..0x1000],
     /// };
-    ///     cpu.tick(&mut bus)?;
+    /// cpu.tick(&mut bus)
+    ///     .expect("0x00e0 (cls) should be a valid opcode.");
     /// assert_eq!(0x202, cpu.pc());
     /// assert_eq!(1, cpu.cycle());
-    ///#    Ok(())
-    ///# }
     /// ```
-    /// # Panics
+    /// Returns [Error::UnimplementedInstruction] if the instruction is not implemented.
-    /// Will panic if an invalid instruction is executed
+    /// ```rust
-    /// ```rust,should_panic
+    /// # use chirp::prelude::*;
-    ///# use chirp::prelude::*;
+    /// # use chirp::error::Error;
-    ///# fn main() -> Result<()> {
     /// let mut cpu = CPU::default();
-    ///#     cpu.flags.debug = true;        // enable live disassembly
+    /// # cpu.flags.debug = true;        // enable live disassembly
-    ///#     cpu.flags.monotonic = Some(8); // enable monotonic/test timing
+    /// # cpu.flags.monotonic = Some(8); // enable monotonic/test timing
     /// let mut bus = bus!{
     ///     Program [0x0200..0x0f00] = &[
     ///         0xff, 0xff, // invalid!
@@ -555,13 +552,15 @@ impl CPU {
     ///     ],
     ///     Screen  [0x0f00..0x1000],
     /// };
-    ///     cpu.tick(&mut bus)?; // panics!
+    /// match cpu.tick(&mut bus) {
-    ///#    Ok(())
+    ///     Err(Error::UnimplementedInstruction {word})
-    ///# }
+    ///         => assert_eq!(0xffff, word),
+    ///     _ => panic!(),
+    /// }
     /// ```
     pub fn tick(&mut self, bus: &mut Bus) -> Result<&mut Self> {
         // Do nothing if paused
-        if self.flags.pause || self.flags.vbi_wait || self.flags.keypause {
+        if self.flags.pause || self.flags.draw_wait || self.flags.keypause {
             // always tick in test mode
             if self.flags.monotonic.is_some() {
                 self.cycle += 1;
@@ -574,7 +573,7 @@ impl CPU {
         {
             slice.try_into()?
         } else {
-            return Err(crate::error::Error::InvalidBusRange {
+            return Err(Error::InvalidBusRange {
                 range: self.pc as usize..self.pc as usize + 2,
             });
         };
@@ -631,7 +630,7 @@ impl CPU {
                 Insn::dmai { x } => self.load_dma(x, bus),
             }
         } else {
-            return Err(crate::error::Error::UnimplementedInstruction {
+            return Err(Error::UnimplementedInstruction {
                 word: u16::from_be_bytes(*opcode),
             });
         }
@@ -646,12 +645,9 @@ impl CPU {
     /// Dumps the current state of all CPU registers, and the cycle count
     /// # Examples
     /// ```rust
-    ///# use chirp::prelude::*;
+    /// # use chirp::prelude::*;
-    ///# fn main() -> Result<()> {
     /// let mut cpu = CPU::default();
     /// cpu.dump();
-    ///#    Ok(())
-    ///# }
     /// ```
     /// outputs
     /// ```text
@@ -992,7 +988,7 @@ impl CPU {
     fn draw(&mut self, x: Reg, y: Reg, n: Nib, bus: &mut Bus) {
         let (x, y) = (self.v[x] as u16 % 64, self.v[y] as u16 % 32);
         if self.flags.quirks.draw_wait {
-            self.flags.vbi_wait = true;
+            self.flags.draw_wait = true;
         }
         self.v[0xf] = 0;
         for byte in 0..n as u16 {
@@ -1003,7 +999,8 @@ impl CPU {
             let addr = (y + byte) * 8 + (x & 0x3f) / 8 + self.screen;
             // Read a byte of sprite data into a u16, and shift it x % 8 bits
             let sprite: u8 = bus.read(self.i + byte);
-            let sprite = (sprite as u16) << (8 - (x & 7)) & if x % 64 > 56 { 0xff00 } else { 0xffff };
+            let sprite =
+                (sprite as u16) << (8 - (x & 7)) & if x % 64 > 56 { 0xff00 } else { 0xffff };
             // Read a u16 from the bus containing the two bytes which might need to be updated
             let mut screen: u16 = bus.read(addr);
             // Save the bits-toggled-off flag if necessary
@@ -1144,12 +1141,7 @@ impl CPU {
     #[inline(always)]
     fn load_dma(&mut self, x: Reg, bus: &mut Bus) {
         let i = self.i as usize;
-        for (reg, value) in bus
+        for (reg, value) in bus.get(i..=i + x).unwrap_or_default().iter().enumerate() {
-            .get(i..=i + x)
-            .unwrap_or_default()
-            .iter()
-            .enumerate()
-        {
             self.v[reg] = *value;
         }
         if self.flags.quirks.dma_inc {

src/cpu/tests.rs

@@ -43,47 +43,49 @@ fn setup_environment() -> (CPU, Bus) {
 }
 
 fn print_screen(bytes: &[u8]) {
-    bus! {Screen [0..0x100] = bytes}.print_screen().unwrap()
+    bus! {Screen [0..0x100] = bytes}
+        .print_screen()
+        .expect("Printing screen should not fail if Screen exists.")
 }
 
 /// Unused instructions
 mod unimplemented {
     use super::*;
     #[test]
-    #[should_panic]
     fn ins_5xyn() {
         let (mut cpu, mut bus) = setup_environment();
-        bus.write(0x200u16, 0x500fu16); // 0x500f is not an instruction
+        bus.write(0x200u16, 0x500fu16);
-        cpu.tick(&mut bus).unwrap();
+        cpu.tick(&mut bus)
+            .expect_err("0x500f is not an instruction");
     }
     #[test]
-    #[should_panic]
     fn ins_8xyn() {
         let (mut cpu, mut bus) = setup_environment();
-        bus.write(0x200u16, 0x800fu16); // 0x800f is not an instruction
+        bus.write(0x200u16, 0x800fu16);
-        cpu.tick(&mut bus).unwrap();
+        cpu.tick(&mut bus)
+            .expect_err("0x800f is not an instruction");
     }
     #[test]
-    #[should_panic]
     fn ins_9xyn() {
         let (mut cpu, mut bus) = setup_environment();
-        bus.write(0x200u16, 0x900fu16); // 0x800f is not an instruction
+        bus.write(0x200u16, 0x900fu16);
-        cpu.tick(&mut bus).unwrap();
+        cpu.tick(&mut bus)
+            .expect_err("0x900f is not an instruction");
     }
     #[test]
-    #[should_panic]
     fn ins_exbb() {
         let (mut cpu, mut bus) = setup_environment();
-        bus.write(0x200u16, 0xe00fu16); // 0xe00f is not an instruction
+        bus.write(0x200u16, 0xe00fu16);
-        cpu.tick(&mut bus).unwrap();
+        cpu.tick(&mut bus)
+            .expect_err("0xe00f is not an instruction");
     }
     // Fxbb
     #[test]
-    #[should_panic]
     fn ins_fxbb() {
         let (mut cpu, mut bus) = setup_environment();
-        bus.write(0x200u16, 0xf00fu16); // 0xf00f is not an instruction
+        bus.write(0x200u16, 0xf00fu16);
-        cpu.tick(&mut bus).unwrap();
+        cpu.tick(&mut bus)
+            .expect_err("0xf00f is not an instruction");
     }
 }
 
@@ -122,6 +124,7 @@ mod sys {
 ///
 /// Basically anything that touches the program counter
 mod cf {
+
     use super::*;
     /// 1aaa: Sets the program counter to an absolute address
     #[test]
@@ -253,6 +256,24 @@ mod cf {
             }
         }
     }
+    /// Tests `stupid_jumps` Quirk behavior
+    #[test]
+    fn jump_stupid() {
+        let (mut cpu, _) = setup_environment();
+        cpu.flags.quirks.stupid_jumps = true;
+
+        //set v[0..F] to 0123456789abcdef
+        for i in 0..0x10 {
+            cpu.v[i] = i as u8;
+        }
+        // just WHY
+        for reg in 0..0x10 {
+            // attempts to jump to 0x`reg`00 + 0
+            cpu.jump_indexed(reg * 0x100);
+            // jumps to 0x`reg`00 + v`reg` instead
+            assert_eq!(cpu.pc, reg * 0x101);
+        }
+    }
 }
 
 mod math {
@@ -635,12 +656,18 @@ mod io {
                 bus = bus.load_region(Program, test.program);
                 // Run the test program for the specified number of steps
                 while cpu.cycle() < test.steps {
-                    cpu.multistep(&mut bus, test.steps - cpu.cycle()).unwrap();
+                    cpu.multistep(&mut bus, test.steps - cpu.cycle())
+                        .expect("Draw tests should not contain undefined instructions");
                 }
                 // Compare the screen to the reference screen buffer
-                bus.print_screen().unwrap();
+                bus.print_screen()
+                    .expect("Printing screen should not fail if screen exists");
                 print_screen(test.screen);
-                assert_eq!(bus.get_region(Screen).unwrap(), test.screen);
+                assert_eq!(
+                    bus.get_region(Screen)
+                        .expect("Getting screen should not fail if screen exists"),
+                    test.screen
+                );
             }
         }
     }
@@ -719,7 +746,8 @@ mod io {
                     assert_eq!(0xff, cpu.v[x]);
                     // There are three parts to a button press
                     // When the button is pressed
-                    cpu.press(key);
+                    assert!(cpu.press(key).expect("Key should be pressed"));
+                    assert!(!cpu.press(key).expect("Key shouldn't be pressed again"));
                     assert!(cpu.flags.keypause);
                     assert_eq!(0xff, cpu.v[x]);
                     // When the button is held
@@ -727,7 +755,8 @@ mod io {
                     assert!(cpu.flags.keypause);
                     assert_eq!(0xff, cpu.v[x]);
                     // And when the button is released!
-                    cpu.release(key);
+                    assert!(cpu.release(key).expect("Key should be released"));
+                    assert!(!cpu.release(key).expect("Key shouldn't be released again"));
                     assert!(!cpu.flags.keypause);
                     assert_eq!(Some(key), cpu.flags.lastkey);
                     cpu.wait_for_key(x);
@@ -825,7 +854,11 @@ mod io {
                 cpu.load_sprite(reg);
 
                 let addr = cpu.i as usize;
-                assert_eq!(bus.get(addr..addr.wrapping_add(5)).unwrap(), test.output,);
+                assert_eq!(
+                    bus.get(addr..addr.wrapping_add(5))
+                        .expect("Region at addr should exist!"),
+                    test.output,
+                );
             }
         }
     }
@@ -880,17 +913,22 @@ mod io {
         const DATA: &[u8] = b"ABCDEFGHIJKLMNOP";
         // Load some test data into memory
         let addr = 0x456;
-        cpu.v.as_mut_slice().write_all(DATA).unwrap();
+        cpu.v
+            .as_mut_slice()
+            .write_all(DATA)
+            .expect("Loading test data should succeed");
         for len in 0..16 {
             // Perform DMA store
             cpu.i = addr as u16;
             cpu.store_dma(len, &mut bus);
             // Check that bus grabbed the correct data
-            let mut bus = bus.get_mut(addr..addr + DATA.len()).unwrap();
+            let bus = bus
+                .get_mut(addr..addr + DATA.len())
+                .expect("Getting a mutable slice at addr 0x0456 should not fail");
             assert_eq!(bus[0..=len], DATA[0..=len]);
             assert_eq!(bus[len + 1..], [0; 16][len + 1..]);
             // clear
-            bus.write_all(&[0; 16]).unwrap();
+            bus.fill(0);
         }
     }
 
@@ -903,7 +941,7 @@ mod io {
         // Load some test data into memory
         let addr = 0x456;
         bus.get_mut(addr..addr + DATA.len())
-            .unwrap()
+            .expect("Getting a mutable slice at addr 0x0456..0x0466 should not fail")
             .write_all(DATA)
             .unwrap();
         for len in 0..16 {
@@ -914,13 +952,14 @@ mod io {
             assert_eq!(cpu.v[0..=len], DATA[0..=len]);
             assert_eq!(cpu.v[len + 1..], [0; 16][len + 1..]);
             // clear
-            cpu.v = [0; 16];
+            cpu.v.fill(0);
         }
     }
 }
 
 mod behavior {
     use super::*;
+
     mod realtime {
         use super::*;
         use std::time::Duration;
@@ -930,7 +969,8 @@ mod behavior {
             cpu.flags.monotonic = None;
             cpu.delay = 10.0;
             for _ in 0..2 {
-                cpu.multistep(&mut bus, 8).unwrap();
+                cpu.multistep(&mut bus, 8)
+                    .expect("Running valid instructions should always succeed");
                 std::thread::sleep(Duration::from_secs_f64(1.0 / 60.0));
             }
             // time is within 1 frame deviance over a theoretical 2 frame pause
@@ -942,7 +982,8 @@ mod behavior {
             cpu.flags.monotonic = None; // disable monotonic timing
             cpu.sound = 10.0;
             for _ in 0..2 {
-                cpu.multistep(&mut bus, 8).unwrap();
+                cpu.multistep(&mut bus, 8)
+                    .expect("Running valid instructions should always succeed");
                 std::thread::sleep(Duration::from_secs_f64(1.0 / 60.0));
             }
             // time is within 1 frame deviance over a theoretical 2 frame pause
@@ -952,13 +993,14 @@ mod behavior {
         fn vbi_wait() {
             let (mut cpu, mut bus) = setup_environment();
             cpu.flags.monotonic = None; // disable monotonic timing
-            cpu.flags.vbi_wait = true;
+            cpu.flags.draw_wait = true;
             for _ in 0..2 {
-                cpu.multistep(&mut bus, 8).unwrap();
+                cpu.multistep(&mut bus, 8)
+                    .expect("Running valid instructions should always succeed");
                 std::thread::sleep(Duration::from_secs_f64(1.0 / 60.0));
             }
             // Display wait is disabled after a 1 frame pause
-            assert!(!cpu.flags.vbi_wait);
+            assert!(!cpu.flags.draw_wait);
         }
     }
     mod breakpoint {
@@ -967,7 +1009,8 @@ mod behavior {
         fn hit_break() {
             let (mut cpu, mut bus) = setup_environment();
             cpu.set_break(0x202);
-            cpu.multistep(&mut bus, 10).unwrap();
+            cpu.multistep(&mut bus, 10)
+                .expect("Running valid instructions should always succeed");
             assert!(cpu.flags.pause);
             assert_eq!(0x202, cpu.pc);
         }

src/error.rs

@@ -32,6 +32,18 @@ pub enum Error {
         /// The offending [Range]
         range: Range<usize>,
     },
+    /// Tried to press a key that doesn't exist
+    #[error("Invalid key: {key:X}")]
+    InvalidKey {
+        /// The offending key
+        key: usize,
+    },
+    /// Tried to get/set an out-of-bounds register
+    #[error("Invalid register: v{reg:X}")]
+    InvalidRegister {
+        /// The offending register
+        reg: usize,
+    },
     /// Error originated in [std::io]
     #[error(transparent)]
     IoError(#[from] std::io::Error),

src/lib.rs

@@ -11,7 +11,6 @@
 pub mod bus;
 pub mod cpu;
 pub mod error;
-pub mod io;
 
 /// Common imports for Chirp
 pub mod prelude {
@@ -21,7 +20,6 @@ pub mod prelude {
     pub use bus::{Bus, Read, Region::*, Write};
     pub use cpu::{disassembler::Dis, ControlFlags, CPU};
     pub use error::Result;
-    pub use io::{UIBuilder, *};
 }
 
 /// Holds the state of a Chip-8

tests/integration.rs

@@ -96,23 +96,28 @@ mod cpu {
     fn press_invalid_key() {
         let mut cpu = CPU::default();
         let cpu2 = cpu.clone();
-        cpu.press(0x21345134);
+        cpu.press(0x21345134)
-        // no change has been made
+            .expect_err("This should produce an Error::InvalidKey");
+        // no change has been made, everything is safe.
         assert_eq!(cpu, cpu2);
     }
+
     #[test]
     fn release_invalid_key() {
         let mut cpu = CPU::default();
         let cpu2 = cpu.clone();
-        cpu.release(0x21345134);
+        cpu.release(0x21345134)
-        // no change has been made
+            .expect_err("This should produce an Error::InvalidKey");
+        // no change has been made, everything is safe.
         assert_eq!(cpu, cpu2);
     }
+
     #[test]
-    fn set_invalid_gpr() {
+    fn set_invalid_reg() {
         let mut cpu = CPU::default();
         let cpu2 = cpu.clone();
-        cpu.set_v(0x21345134, 0xff);
+        cpu.set_v(0x21345134, 0xff)
+            .expect_err("This should produce an Error::InvalidRegister");
         // no change has been made
         assert_eq!(cpu, cpu2);
     }
@@ -125,7 +130,7 @@ mod cpu {
                 debug: false,
                 pause: false,
                 keypause: false,
-                vbi_wait: false,
+                draw_wait: false,
                 lastkey: None,
                 quirks: Default::default(),
                 monotonic: None,
@@ -145,7 +150,7 @@ mod cpu {
                     debug: false,
                     pause: false,
                     keypause: false,
-                    vbi_wait: false,
+                    draw_wait: false,
                     lastkey: Default::default(),
                     quirks: Default::default(),
                     monotonic: Default::default()
@@ -159,7 +164,7 @@ mod cpu {
                 debug: true,
                 pause: true,
                 keypause: true,
-                vbi_wait: true,
+                draw_wait: true,
                 lastkey: Default::default(),
                 quirks: Default::default(),
                 monotonic: Default::default(),
@@ -173,7 +178,7 @@ mod cpu {
                 debug: true,
                 pause: true,
                 keypause: true,
-                vbi_wait: true,
+                draw_wait: true,
                 lastkey: Default::default(),
                 quirks: Default::default(),
                 monotonic: Default::default(),
@@ -214,146 +219,6 @@ fn error() {
     println!("{error} {error:?}");
 }
 
-mod ui_builder {
-    use super::*;
-    #[test]
-    fn ui_builder() -> Result<()> {
-        let builder = UIBuilder::new(32, 64).rom("dummy.ch8").build()?;
-        println!("{builder:?}");
-        Ok(())
-    }
-    #[test]
-    fn default() {
-        let ui_builder = UIBuilder::default();
-        println!("{ui_builder:?}");
-    }
-    #[test]
-    #[allow(clippy::redundant_clone)]
-    fn clone_debug() {
-        let ui_builder_clone = UIBuilder::default().clone();
-        println!("{ui_builder_clone:?}");
-    }
-}
-mod ui {
-    use super::*;
-    fn new_chip8() -> Chip8 {
-        Chip8 {
-            cpu: CPU::default(),
-            bus: bus! {},
-        }
-    }
-    #[test]
-    fn frame() -> Result<()> {
-        let mut ui = UIBuilder::new(32, 64).build()?;
-        let mut ch8 = new_chip8();
-        ui.frame(&mut ch8).unwrap();
-        Ok(())
-    }
-    #[test]
-    fn keys() -> Result<()> {
-        let mut ui = UIBuilder::new(32, 64).build()?;
-        let mut ch8 = new_chip8();
-        let ch8 = &mut ch8;
-        ui.frame(ch8).unwrap();
-        ui.keys(ch8).unwrap();
-        Ok(())
-    }
-    #[test]
-    fn debug() -> Result<()> {
-        println!("{:?}", UIBuilder::new(32, 64).build()?);
-        Ok(())
-    }
-}
-
-mod framebuffer_format {
-    use super::*;
-    #[test]
-    fn default() {
-        let _fbf = FrameBufferFormat::default();
-    }
-    #[test]
-    fn clone() {
-        let fbf = FrameBufferFormat {
-            fg: 0x12345678,
-            bg: 0x90abcdef,
-        };
-        let fbf2 = fbf.clone();
-        assert_eq!(fbf, fbf2);
-    }
-    #[test]
-    fn debug() {
-        println!("{:?}", FrameBufferFormat::default());
-    }
-    #[test]
-    fn eq() {
-        assert_eq!(FrameBufferFormat::default(), FrameBufferFormat::default());
-        assert_ne!(
-            FrameBufferFormat {
-                fg: 0xff00ff,
-                bg: 0x00ff00
-            },
-            FrameBufferFormat {
-                fg: 0x00ff00,
-                bg: 0xff00ff
-            },
-        );
-    }
-    #[test]
-    fn ord() {
-        assert!(
-            FrameBufferFormat::default()
-                == FrameBufferFormat {
-                    fg: 0xffffff,
-                    bg: 0xffffff,
-                }
-                .min(FrameBufferFormat::default())
-        );
-    }
-    #[test]
-    fn hash() {
-        let mut hasher = DefaultHasher::new();
-        FrameBufferFormat::default().hash(&mut hasher);
-        println!("{hasher:?}");
-    }
-}
-
-mod framebuffer {
-    use super::*;
-    // [derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
-    #[test]
-    fn new() {
-        assert_eq!(FrameBuffer::new(64, 32), FrameBuffer::default());
-    }
-    #[test]
-    fn clone() {
-        let fb1 = FrameBuffer::default();
-        let fb2 = fb1.clone();
-        assert_eq!(fb1, fb2);
-    }
-
-    #[test]
-    fn debug() {
-        println!("{:?}", FrameBuffer::default());
-    }
-
-    #[test]
-    fn eq() {
-        assert_eq!(FrameBuffer::new(64, 32), FrameBuffer::default());
-    }
-
-    #[test]
-    fn ord() {
-        assert!(FrameBuffer::new(21, 12) == FrameBuffer::new(21, 12).min(FrameBuffer::new(34, 46)));
-    }
-
-    #[test]
-    fn hash() {
-        let mut hasher = DefaultHasher::new();
-        FrameBuffer::default().hash(&mut hasher);
-        println!("{hasher:?}");
-    }
-}
-
 mod quirks {
     use super::*;
     use chirp::cpu::Quirks;