chore: improve code quality

Removed a lot of magic constants. Gave them descriptive variable names

src/bus.rs

@@ -9,9 +9,11 @@ use super::timer::Timer;
 use super::work_ram::{VariableWorkRam, WorkRam};
 use std::{convert::TryInto, fs::File, io::Read};
 
+const BOOT_ROM_SIZE: usize = 256;
+
 #[derive(Debug, Clone)]
 pub struct Bus {
-    boot: Option<[u8; 256]>, // Boot ROM is 256b long
+    boot: Option<[u8; BOOT_ROM_SIZE]>, // Boot ROM is 256b long
     cartridge: Option<Cartridge>,
     pub ppu: Ppu,
     wram: WorkRam,
@@ -43,10 +45,10 @@ impl Default for Bus {
 impl Bus {
     pub fn with_boot(path: &str) -> Self {
         let mut file = File::open(path).unwrap();
-        let mut buf = Vec::with_capacity(256);
+        let mut buf = Vec::with_capacity(BOOT_ROM_SIZE);
         file.read_to_end(&mut buf).unwrap();
 
-        let boot_rom: [u8; 256] = buf.try_into().unwrap();
+        let boot_rom: [u8; BOOT_ROM_SIZE] = buf.try_into().unwrap();
 
         Self {
             boot: Some(boot_rom),

src/cartridge.rs

@@ -2,10 +2,14 @@ use std::fs::File;
 use std::io::{self, Read};
 use std::path::Path;
 
+const RAM_SIZE_ADDRESS: usize = 0x0149;
+const ROM_SIZE_ADDRESS: usize = 0x0148;
+const MBC_TYPE_ADDRESS: usize = 0x0147;
+
 #[derive(Debug, Clone, Default)]
 pub struct Cartridge {
     memory: Vec<u8>,
-    mbc: Box<dyn Mbc>,
+    mbc: Box<dyn MemoryBankController>,
 }
 
 impl Cartridge {
@@ -20,7 +24,7 @@ impl Cartridge {
         })
     }
 
-    fn detect_mbc(memory: &[u8]) -> Box<dyn Mbc> {
+    fn detect_mbc(memory: &[u8]) -> Box<dyn MemoryBankController> {
         let ram_size = Self::find_ram_size(&memory);
         let bank_count = Self::find_bank_count(&memory);
         let mbc_kind = Self::find_mbc(&memory);
@@ -41,17 +45,17 @@ impl Cartridge {
     }
 
     fn find_ram_size(memory: &[u8]) -> RamSize {
-        let id = memory[0x0149];
+        let id = memory[RAM_SIZE_ADDRESS];
         id.into()
     }
 
     fn find_bank_count(memory: &[u8]) -> BankCount {
-        let id = memory[0x0148];
+        let id = memory[ROM_SIZE_ADDRESS];
         id.into()
     }
 
     fn find_mbc(memory: &[u8]) -> MBCKind {
-        let id = memory[0x0147];
+        let id = memory[MBC_TYPE_ADDRESS];
 
         // TODO: Refactor this to match the other enums in this module
         match id {
@@ -67,7 +71,7 @@ impl Cartridge {
     pub fn read_byte(&self, addr: u16) -> u8 {
         match self.mbc.handle_read(addr) {
             MBCResult::Address(addr) => self.memory[addr as usize],
-            MBCResult::RamValue(byte) => byte,
+            MBCResult::Value(byte) => byte,
         }
     }
     pub fn write_byte(&mut self, addr: u16, byte: u8) {
@@ -139,7 +143,7 @@ impl MBC1 {
     }
 }
 
-impl Mbc for MBC1 {
+impl MemoryBankController for MBC1 {
     fn handle_read(&self, addr: u16) -> MBCResult {
         use MBCResult::*;
 
@@ -172,7 +176,7 @@ impl Mbc for MBC1 {
                         _ => unreachable!(""),
                     };
 
-                    RamValue(self.ram[ram_addr as usize])
+                    Value(self.ram[ram_addr as usize])
                 } else {
                     Address(0x00FF)
                 }
@@ -217,27 +221,27 @@ impl Mbc for MBC1 {
     }
 }
 
-trait Mbc: CloneMBC {
+trait MemoryBankController: CloneMBC {
     fn handle_read(&self, addr: u16) -> MBCResult;
     fn handle_write(&mut self, addr: u16, byte: u8);
 }
 
 trait CloneMBC {
-    fn clone_mbc(&self) -> Box<dyn Mbc>;
+    fn clone_mbc(&self) -> Box<dyn MemoryBankController>;
 }
 
 impl<T> CloneMBC for T
 where
-    T: Mbc + Clone + 'static,
+    T: MemoryBankController + Clone + 'static,
 {
-    fn clone_mbc<'a>(&self) -> Box<dyn Mbc> {
+    fn clone_mbc<'a>(&self) -> Box<dyn MemoryBankController> {
         Box::new(self.clone())
     }
 }
 
 enum MBCResult {
     Address(u16),
-    RamValue(u8),
+    Value(u8),
 }
 
 #[derive(Debug, Clone, Copy)]
@@ -366,19 +370,19 @@ impl From<u8> for BankCount {
     }
 }
 
-impl std::fmt::Debug for Box<dyn Mbc> {
+impl std::fmt::Debug for Box<dyn MemoryBankController> {
     fn fmt(&self, _f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         todo!("Implement Debug for Box<dyn MBC> Trait Object");
     }
 }
 
-impl std::clone::Clone for Box<dyn Mbc> {
+impl std::clone::Clone for Box<dyn MemoryBankController> {
     fn clone(&self) -> Self {
         self.clone_mbc()
     }
 }
 
-impl std::default::Default for Box<dyn Mbc> {
+impl std::default::Default for Box<dyn MemoryBankController> {
     fn default() -> Self {
         Box::new(MBC1::default())
     }

src/high_ram.rs

@@ -1,22 +1,25 @@
+const HIGH_RAM_SIZE: usize = 127;
+const HIGH_RAM_START_ADDRESS: usize = 0xFF80;
+
 #[derive(Debug, Clone)]
 pub struct HighRam {
-    buf: Box<[u8; 127]>,
+    buf: Box<[u8; HIGH_RAM_SIZE]>,
 }
 
 impl Default for HighRam {
     fn default() -> Self {
         Self {
-            buf: Box::new([0u8; 127]),
+            buf: Box::new([0u8; HIGH_RAM_SIZE]),
         }
     }
 }
 
 impl HighRam {
     pub fn write_byte(&mut self, addr: u16, byte: u8) {
-        self.buf[addr as usize - 0xFF80] = byte;
+        self.buf[addr as usize - HIGH_RAM_START_ADDRESS] = byte;
     }
 
     pub fn read_byte(&self, addr: u16) -> u8 {
-        self.buf[addr as usize - 0xFF80]
+        self.buf[addr as usize - HIGH_RAM_START_ADDRESS]
     }
 }

src/interrupt.rs

@@ -75,11 +75,3 @@ impl From<InterruptFlag> for u8 {
         flag.0
     }
 }
-
-enum InterruptType {
-    VBlank,
-    LCDStat,
-    Timer,
-    Serial,
-    Joypad,
-}

src/lib.rs

@@ -1,8 +1,12 @@
+pub use cpu::Cpu as LR35902;
 pub use instruction::Cycles;
 
+pub const GB_WIDTH: usize = 160;
+pub const GB_HEIGHT: usize = 144;
+
 mod bus;
 mod cartridge;
-pub mod cpu;
+mod cpu;
 mod high_ram;
 mod instruction;
 mod interrupt;

src/main.rs

@@ -1,6 +1,5 @@
 use anyhow::{anyhow, Result};
-use gb::cpu::Cpu as LR35902;
+use gb::{Cycles, LR35902};
-use gb::Cycles;
 use pixels::{Pixels, SurfaceTexture};
 use std::env::args;
 use std::time::{Duration, Instant};
@@ -85,7 +84,7 @@ fn main() -> Result<()> {
 pub fn create_window(event_loop: &EventLoop<()>) -> Result<Window> {
     let size = LogicalSize::new((GB_WIDTH as f64) * SCALE, (GB_HEIGHT as f64) * SCALE);
     Ok(WindowBuilder::new()
-        .with_title("DMG-1 Game Boy Emulator")
+        .with_title("DMG-1 Emulator")
         .with_inner_size(size)
         .with_min_inner_size(size)
         .build(&event_loop)?)

src/ppu.rs

@@ -1,16 +1,25 @@
-use crate::instruction::Cycles;
+use crate::Cycles;
+use crate::GB_HEIGHT;
+use crate::GB_WIDTH;
 use bitfield::bitfield;
 
-const GB_WIDTH: usize = 160;
+const VRAM_SIZE: usize = 8192;
-const GB_HEIGHT: usize = 144;
+const OAM_SIZE: usize = 160;
+const PPU_START_ADDRESS: usize = 0x8000;
+
+const WHITE: [u8; 4] = [0xFF, 0xFF, 0xFF, 0xFF];
+const LIGHT_GRAY: [u8; 4] = [0xCC, 0xCC, 0xCC, 0xFF];
+const DARK_GRAY: [u8; 4] = [0x77, 0x77, 0x77, 0xFF];
+const BLACK: [u8; 4] = [0x00, 0x00, 0x00, 0x00];
+
 #[derive(Debug, Clone)]
 pub struct Ppu {
     pub interrupt: Interrupt,
     pub lcd_control: LCDControl,
     pub monochrome: Monochrome,
     pub pos: ScreenPosition,
-    pub vram: Box<[u8; 8192]>,
+    pub vram: Box<[u8; VRAM_SIZE]>,
-    pub oam: Box<[u8; 160]>,
+    pub oam: Box<[u8; OAM_SIZE]>,
     frame_buf: [u8; GB_WIDTH * GB_HEIGHT * 4],
     pub stat: LCDStatus,
     cycles: Cycles,
@@ -18,11 +27,11 @@ pub struct Ppu {
 
 impl Ppu {
     pub fn read_byte(&self, addr: u16) -> u8 {
-        self.vram[addr as usize - 0x8000]
+        self.vram[addr as usize - PPU_START_ADDRESS]
     }
 
     pub fn write_byte(&mut self, addr: u16, byte: u8) {
-        self.vram[addr as usize - 0x8000] = byte;
+        self.vram[addr as usize - PPU_START_ADDRESS] = byte;
     }
 }
 
@@ -145,8 +154,8 @@ impl Default for Ppu {
             monochrome: Default::default(),
             pos: Default::default(),
             stat: Default::default(),
-            vram: Box::new([0u8; 8192]),
+            vram: Box::new([0u8; VRAM_SIZE]),
-            oam: Box::new([0u8; 160]),
+            oam: Box::new([0u8; OAM_SIZE]),
             cycles: 0.into(),
             frame_buf: [0; GB_WIDTH * GB_HEIGHT * 4],
         }
@@ -394,10 +403,10 @@ pub enum GrayShade {
 impl GrayShade {
     pub fn into_rgba(self) -> [u8; 4] {
         match self {
-            GrayShade::White => [0xFF, 0xFF, 0xFF, 0xFF],
+            GrayShade::White => WHITE,
-            GrayShade::LightGray => [0xCC, 0xCC, 0xCC, 0xFF],
+            GrayShade::LightGray => LIGHT_GRAY,
-            GrayShade::DarkGray => [0x77, 0x77, 0x77, 0xFF],
+            GrayShade::DarkGray => DARK_GRAY,
-            GrayShade::Black => [0x00, 0x00, 0x00, 0x00],
+            GrayShade::Black => BLACK,
         }
     }
 }
@@ -499,5 +508,3 @@ impl From<ObjectPalette> for u8 {
         palette.0
     }
 }
-
-struct BackgroundMap([u8; 32]);

src/work_ram.rs

@@ -1,22 +1,27 @@
+const WORK_RAM_SIZE: usize = 4096;
+const VARIABLE_WORK_RAM_SIZE: usize = WORK_RAM_SIZE;
+const WORK_RAM_START_ADDRESS: usize = 0xC000;
+const VARIABLE_WORK_RAM_START_ADDRESS: usize = 0xD000;
+
 #[derive(Debug, Clone)]
 pub struct WorkRam {
-    bank: Box<[u8; 4096]>,
+    bank: Box<[u8; WORK_RAM_SIZE]>,
 }
 
 impl WorkRam {
     pub fn write_byte(&mut self, addr: u16, byte: u8) {
-        self.bank[addr as usize - 0xC000] = byte;
+        self.bank[addr as usize - WORK_RAM_START_ADDRESS] = byte;
     }
 
     pub fn read_byte(&self, addr: u16) -> u8 {
-        self.bank[addr as usize - 0xC000]
+        self.bank[addr as usize - WORK_RAM_START_ADDRESS]
     }
 }
 
 impl Default for WorkRam {
     fn default() -> Self {
         Self {
-            bank: Box::new([0u8; 4096]),
+            bank: Box::new([0u8; WORK_RAM_SIZE]),
         }
     }
 }
@@ -35,14 +40,14 @@ pub enum BankNumber {
 #[derive(Debug, Clone)]
 pub struct VariableWorkRam {
     current: BankNumber,
-    bank_n: Box<[[u8; 4096]; 7]>, // 4K for Variable amount of Banks (Banks 1 -> 7) in Game Boy Colour
+    bank_n: Box<[[u8; VARIABLE_WORK_RAM_SIZE]; 7]>, // 4K for Variable amount of Banks (Banks 1 -> 7) in Game Boy Colour
 }
 
 impl Default for VariableWorkRam {
     fn default() -> Self {
         Self {
             current: BankNumber::One,
-            bank_n: Box::new([[0u8; 4096]; 7]),
+            bank_n: Box::new([[0u8; VARIABLE_WORK_RAM_SIZE]; 7]),
         }
     }
 }
@@ -57,10 +62,10 @@ impl VariableWorkRam {
     }
 
     pub fn write_byte(&mut self, addr: u16, byte: u8) {
-        self.bank_n[self.current as usize][addr as usize - 0xD000] = byte;
+        self.bank_n[self.current as usize][addr as usize - VARIABLE_WORK_RAM_START_ADDRESS] = byte;
     }
 
     pub fn read_byte(&self, addr: u16) -> u8 {
-        self.bank_n[self.current as usize][addr as usize - 0xD000]
+        self.bank_n[self.current as usize][addr as usize - VARIABLE_WORK_RAM_START_ADDRESS]
     }
 }