From d30ce4dbb28fc877a2f7bf775e565673523a4e0d Mon Sep 17 00:00:00 2001 From: Rekai Musuka Date: Mon, 15 Mar 2021 19:19:40 -0500 Subject: [PATCH] chore: improve code quailty --- src/bus.rs | 8 ++-- src/cartridge.rs | 108 ++++++++++++++++++++++++++--------------------- src/cpu.rs | 64 ++++++++++++++++------------ src/ppu.rs | 49 ++++++++++----------- src/sound.rs | 6 +-- 5 files changed, 128 insertions(+), 107 deletions(-) diff --git a/src/bus.rs b/src/bus.rs index 20283da..61cf715 100644 --- a/src/bus.rs +++ b/src/bus.rs @@ -107,7 +107,7 @@ impl Bus { unimplemented!("Unable to read {:#06X} in Restricted Mirror", addr); } 0xFE00..=0xFE9F => { - // Sprite Attrbute Table + // Sprite Attribute Table unimplemented!("Unable to read {:#06X} in the Sprite Attribute Table", addr); } 0xFEA0..=0xFEFF => unimplemented!("{:#06X} is not allowed to be used", addr), @@ -138,7 +138,7 @@ impl Bus { self.hram.read_byte((addr - 0xFF80) as usize) } 0xFFFF => { - // Interupts Enable Register + // Interrupts Enable Register self.interrupt.enable.into() } } @@ -184,7 +184,7 @@ impl Bus { unimplemented!("Unable to write to {:#06X} in Restricted Mirror", addr); } 0xFE00..=0xFE9F => { - // Sprite Attrbute Table + // Sprite Attribute Table unimplemented!( "Unable to write to {:#06X} in the Sprite Attribute Table", addr @@ -225,7 +225,7 @@ impl Bus { self.hram.write_byte((addr - 0xFF80) as usize, byte); } 0xFFFF => { - // Interupts Enable Register + // Interrupts Enable Register self.interrupt.enable = byte.into(); } } diff --git a/src/cartridge.rs b/src/cartridge.rs index be7dfa5..f8b863e 100644 --- a/src/cartridge.rs +++ b/src/cartridge.rs @@ -125,13 +125,15 @@ impl MBC1 { } fn apply_rom_size_bitmask(&self, byte: u8) -> u8 { + use BankCount::*; + match self.bank_count { - BankCount::Four => byte & 0b00000011, - BankCount::Eight => byte & 0b00000111, - BankCount::Sixteen => byte & 0b00001111, - BankCount::ThirtyTwo => byte & 0b00011111, - BankCount::SixtyFour => byte & 0b00011111, - BankCount::OneHundredTwentyEight => byte & 0b00011111, + Four => byte & 0b00000011, + Eight => byte & 0b00000111, + Sixteen => byte & 0b00001111, + ThirtyTwo => byte & 0b00011111, + SixtyFour => byte & 0b00011111, + OneHundredTwentyEight => byte & 0b00011111, _ => unreachable!("{#:?} is not a valid ROM Bank Number for MBC1"), } } @@ -139,18 +141,20 @@ impl MBC1 { impl MBC for MBC1 { fn handle_read(&self, addr: u16) -> MBCResult { + use MBCResult::*; + match addr { 0x0000..=0x3FFF => { if self.mode { let zero_bank_number = self.calc_zero_bank_number() as u16; - MBCResult::Address(0x4000 * zero_bank_number + addr) + Address(0x4000 * zero_bank_number + addr) } else { - MBCResult::Address(addr) + Address(addr) } } 0x4000..=0x7FFF => { let high_bank_number = self.calc_high_bank_number() as u16; - MBCResult::Address(0x4000 * high_bank_number * (addr - 0x4000)) + Address(0x4000 * high_bank_number * (addr - 0x4000)) } 0xA000..=0xBFFF => { if self.ram_enabled { @@ -168,9 +172,9 @@ impl MBC for MBC1 { _ => unreachable!(""), }; - MBCResult::RamValue(self.ram[ram_addr as usize]) + RamValue(self.ram[ram_addr as usize]) } else { - MBCResult::Address(0x00FF) + Address(0x00FF) } } _ => unimplemented!(), @@ -261,13 +265,15 @@ enum RAMSize { impl RAMSize { pub fn to_byte_count(&self) -> u32 { + use RAMSize::*; + match *self { - RAMSize::None => 0, - RAMSize::_2KB => 2_048, - RAMSize::_8KB => 8_192, - RAMSize::_32KB => 32_768, - RAMSize::_128KB => 131_072, - RAMSize::_64KB => 65_536, + None => 0, + _2KB => 2_048, + _8KB => 8_192, + _32KB => 32_768, + _128KB => 131_072, + _64KB => 65_536, } } } @@ -280,13 +286,15 @@ impl Default for RAMSize { impl From for RAMSize { fn from(byte: u8) -> Self { + use RAMSize::*; + match byte { - 0x00 => Self::None, - 0x01 => Self::_2KB, - 0x02 => Self::_8KB, - 0x03 => Self::_32KB, - 0x04 => Self::_64KB, - 0x05 => Self::_128KB, + 0x00 => None, + 0x01 => _2KB, + 0x02 => _8KB, + 0x03 => _32KB, + 0x04 => _64KB, + 0x05 => _128KB, _ => unreachable!("{:#04X} is an invalid value for RAMSize"), } } @@ -317,45 +325,49 @@ impl Default for BankCount { impl BankCount { // https://hacktix.github.io/GBEDG/mbcs/#rom-size pub fn to_byte_count(&self) -> u32 { + use BankCount::*; + match *self { - BankCount::None => 32_768, - BankCount::Four => 65_536, - BankCount::Eight => 131_072, - BankCount::Sixteen => 262_144, - BankCount::ThirtyTwo => 524_288, - BankCount::SixtyFour => 1_048_576, - BankCount::OneHundredTwentyEight => 2_097_152, - BankCount::TwoHundredFiftySix => 4_194_304, - BankCount::FiveHundredTwelve => 8_388_608, - BankCount::SeventyTwo => 1_179_648, - BankCount::Eighty => 1_310_720, - BankCount::NinetySix => 1_572_864, + None => 32_768, + Four => 65_536, + Eight => 131_072, + Sixteen => 262_144, + ThirtyTwo => 524_288, + SixtyFour => 1_048_576, + OneHundredTwentyEight => 2_097_152, + TwoHundredFiftySix => 4_194_304, + FiveHundredTwelve => 8_388_608, + SeventyTwo => 1_179_648, + Eighty => 1_310_720, + NinetySix => 1_572_864, } } } impl From for BankCount { fn from(byte: u8) -> Self { + use BankCount::*; + match byte { - 0x00 => Self::None, - 0x01 => Self::Four, - 0x02 => Self::Eight, - 0x03 => Self::Sixteen, - 0x04 => Self::ThirtyTwo, - 0x05 => Self::SixtyFour, - 0x06 => Self::OneHundredTwentyEight, - 0x07 => Self::TwoHundredFiftySix, - 0x08 => Self::FiveHundredTwelve, - 0x52 => Self::SeventyTwo, - 0x53 => Self::Eighty, - 0x54 => Self::NinetySix, + 0x00 => None, + 0x01 => Four, + 0x02 => Eight, + 0x03 => Sixteen, + 0x04 => ThirtyTwo, + 0x05 => SixtyFour, + 0x06 => OneHundredTwentyEight, + 0x07 => TwoHundredFiftySix, + 0x08 => FiveHundredTwelve, + 0x52 => SeventyTwo, + 0x53 => Eighty, + 0x54 => NinetySix, _ => unreachable!("{:#04X} is an invalid value for BankCount"), } } } impl std::fmt::Debug for Box { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + fn fmt(&self, _f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { todo!("Implement Debug for Box Trait Object"); } } diff --git a/src/cpu.rs b/src/cpu.rs index cbd05ed..5786254 100644 --- a/src/cpu.rs +++ b/src/cpu.rs @@ -149,65 +149,73 @@ impl Default for State { impl Cpu { pub fn set_register(&mut self, register: Register, value: u8) { + use Register::*; + match register { - Register::A => self.reg.a = value, - Register::B => self.reg.b = value, - Register::C => self.reg.c = value, - Register::D => self.reg.d = value, - Register::E => self.reg.e = value, - Register::H => self.reg.h = value, - Register::L => self.reg.l = value, - Register::Flag => self.flags = value.into(), + A => self.reg.a = value, + B => self.reg.b = value, + C => self.reg.c = value, + D => self.reg.d = value, + E => self.reg.e = value, + H => self.reg.h = value, + L => self.reg.l = value, + Flag => self.flags = value.into(), } } pub fn register(&self, register: Register) -> u8 { + use Register::*; + match register { - Register::A => self.reg.a, - Register::B => self.reg.b, - Register::C => self.reg.c, - Register::D => self.reg.d, - Register::E => self.reg.e, - Register::H => self.reg.h, - Register::L => self.reg.l, - Register::Flag => self.flags.into(), + A => self.reg.a, + B => self.reg.b, + C => self.reg.c, + D => self.reg.d, + E => self.reg.e, + H => self.reg.h, + L => self.reg.l, + Flag => self.flags.into(), } } pub fn register_pair(&self, pair: RegisterPair) -> u16 { + use RegisterPair::*; + match pair { - RegisterPair::AF => (self.reg.a as u16) << 8 | u8::from(self.flags) as u16, - RegisterPair::BC => (self.reg.b as u16) << 8 | self.reg.c as u16, - RegisterPair::DE => (self.reg.d as u16) << 8 | self.reg.e as u16, - RegisterPair::HL => (self.reg.h as u16) << 8 | self.reg.l as u16, - RegisterPair::SP => self.reg.sp, - RegisterPair::PC => self.reg.pc, + AF => (self.reg.a as u16) << 8 | u8::from(self.flags) as u16, + BC => (self.reg.b as u16) << 8 | self.reg.c as u16, + DE => (self.reg.d as u16) << 8 | self.reg.e as u16, + HL => (self.reg.h as u16) << 8 | self.reg.l as u16, + SP => self.reg.sp, + PC => self.reg.pc, } } pub fn set_register_pair(&mut self, pair: RegisterPair, value: u16) { + use RegisterPair::*; + let high = (value >> 8) as u8; let low = value as u8; match pair { - RegisterPair::AF => { + AF => { self.reg.a = high; self.flags = low.into(); } - RegisterPair::BC => { + BC => { self.reg.b = high; self.reg.c = low; } - RegisterPair::DE => { + DE => { self.reg.d = high; self.reg.e = low; } - RegisterPair::HL => { + HL => { self.reg.h = high; self.reg.l = low; } - RegisterPair::SP => self.reg.sp = value, - RegisterPair::PC => self.reg.pc = value, + SP => self.reg.sp = value, + PC => self.reg.pc = value, } } } diff --git a/src/ppu.rs b/src/ppu.rs index 0800c5c..b095157 100644 --- a/src/ppu.rs +++ b/src/ppu.rs @@ -19,8 +19,8 @@ impl PPU { pub fn step(&mut self, cycles: Cycles) { self.cycles += cycles; - // let smth: u32 = self.cycles.into(); - // println!("Mode: {:?} | Cycles: {}", self.mode, smth); + // let tmp: u32 = self.cycles.into(); + // println!("Mode: {:?} | Cycles: {}", self.mode, tmp); match self.mode { Mode::OAMScan => { @@ -164,15 +164,15 @@ impl From for LCDControl { } impl From for u8 { - fn from(lcdc: LCDControl) -> Self { - (lcdc.lcd_enable as u8) << 7 - | (lcdc.window_tile_map_select as u8) << 6 - | (lcdc.window_enable as u8) << 5 - | (lcdc.tile_data_select as u8) << 4 - | (lcdc.bg_tile_map_select as u8) << 3 - | (lcdc.sprite_size as u8) << 2 - | (lcdc.sprite_enable as u8) << 1 - | lcdc.display_priority as u8 + fn from(ctrl: LCDControl) -> Self { + (ctrl.lcd_enable as u8) << 7 + | (ctrl.window_tile_map_select as u8) << 6 + | (ctrl.window_enable as u8) << 5 + | (ctrl.tile_data_select as u8) << 4 + | (ctrl.bg_tile_map_select as u8) << 3 + | (ctrl.sprite_size as u8) << 2 + | (ctrl.sprite_enable as u8) << 1 + | ctrl.display_priority as u8 } } @@ -204,30 +204,31 @@ impl From for GrayShade { #[derive(Debug, Clone, Copy, Default)] pub struct BackgroundPalette { - color3: GrayShade, - color2: GrayShade, - color1: GrayShade, - color0: GrayShade, + colour: GrayShade, + colour2: GrayShade, + colour3: GrayShade, + colour0: GrayShade, // FIXME: Is this supposed to be colour0? } impl From for BackgroundPalette { fn from(byte: u8) -> Self { Self { - color3: (byte >> 6).into(), - color2: ((byte >> 4) & 0x03).into(), - color1: ((byte >> 2) & 0x03).into(), - color0: (byte & 0x03).into(), + colour: (byte >> 6).into(), + colour2: ((byte >> 4) & 0x03).into(), + colour3: ((byte >> 2) & 0x03).into(), + colour0: (byte & 0x03).into(), } } } impl From for u8 { fn from(palette: BackgroundPalette) -> Self { - let color0: u8 = palette.color0 as u8; - let color1: u8 = palette.color1 as u8; - let color2: u8 = palette.color2 as u8; - let color3: u8 = palette.color0 as u8; + // FIXME: There is a bug here, see the above FIXME + let colour0: u8 = palette.colour0 as u8; + let colour1: u8 = palette.colour3 as u8; + let colour2: u8 = palette.colour2 as u8; + let colour3: u8 = palette.colour0 as u8; - color3 << 6 | color2 << 4 | color1 << 2 | color0 + colour3 << 6 | colour2 << 4 | colour1 << 2 | colour0 } } diff --git a/src/sound.rs b/src/sound.rs index 1b29138..03a280d 100644 --- a/src/sound.rs +++ b/src/sound.rs @@ -30,15 +30,15 @@ pub struct Frequency { #[derive(Debug, Clone, Copy)] enum FrequencyType { Counter = 0, - Consequtive = 1, + Consecutive = 1, } impl From for FrequencyType { fn from(byte: u8) -> Self { match byte { 0b00 => Self::Counter, - 0b01 => Self::Consequtive, - _ => unreachable!("{} is not a valid number for FreuquencyType"), + 0b01 => Self::Consecutive, + _ => unreachable!("{} is not a valid number for FrequencyType"), } } }