Implement SWAP r8 instructions

The previous refactor implemented it implicitly

src/cpu/instructions/manip.rs

@@ -1,12 +1,29 @@
-use crate::cpu::register;
+use crate::register;
+
+#[derive(Debug, Copy, Clone)]
+pub enum IndividualBitOperation {
+    Set,
+    Clear,
+}
+
+#[derive(Debug, Copy, Clone)]
+pub enum Operand {
+    Register(register::SingleEightBit),
+    HLValue,
+}
 
 #[derive(Debug, Copy, Clone)]
 pub enum BitManipulationInstruction {
-    SetRegisterBit {
+    ManipulateIndividualBit {
-        register: register::SingleEightBit,
+        operation: IndividualBitOperation,
+        operand: Operand,
         bit: u8,
     },
-    SetHLValueBit {
+    TestIndividualBit {
+        operand: Operand,
         bit: u8,
     },
+    SwapNibbles {
+        operand: Operand,
+    },
 }

src/cpu/parse/manip.rs

@@ -1,7 +1,12 @@
-use super::{unparsed::Opcode, OpcodeParser, ParseResult};
+use std::ops::RangeInclusive;
+
+use super::{unparsed::Opcode, OpcodeParser, OrParse, ParseResult};
 use crate::{
     cpu::{
-        instructions::{manip::BitManipulationInstruction, Instruction},
+        instructions::{
+            manip::{BitManipulationInstruction, IndividualBitOperation, Operand},
+            Instruction,
+        },
         parse,
     },
     memory::View,
@@ -10,61 +15,104 @@ use crate::{
 
 pub struct Parser;
 
-enum OpcodeOperand {
-    Register(register::SingleEightBit),
-    HLValue,
-}
-
 impl OpcodeParser for Parser {
     fn parse_opcode(data: &View) -> ParseResult {
         let opcode = parse::get_opcode_from_data(data);
 
         parse_set_opcode(opcode)
+            .or_parse(parse_clear_opcode)
+            .or_parse(parse_bit_test_opcode)
+            .or_parse(parse_swap_opcode)
     }
 }
 
 fn parse_set_opcode(opcode: Opcode) -> ParseResult {
+    parse_individual_bit_operation(opcode, IndividualBitOperation::Set, 0xCB_C0..=0xCB_FF)
+}
+
+fn parse_clear_opcode(opcode: Opcode) -> ParseResult {
+    parse_individual_bit_operation(opcode, IndividualBitOperation::Clear, 0xCB_80..=0xCB_BF)
+}
+
+fn parse_bit_test_opcode(opcode: Opcode) -> ParseResult {
+    let (operand, bit) = decompose_bit_action_opcode(opcode, 0xCB_40..=0xCB_7F)?;
+    let ins = BitManipulationInstruction::TestIndividualBit { operand, bit };
+
+    Ok((Instruction::BitManipulation(ins), 2))
+}
+
+fn parse_swap_opcode(opcode: Opcode) -> ParseResult {
     let Opcode::SixteenBit(sixteen_bit_opcode) = opcode else {
         return Err(parse::Error::UnknownOpcode(opcode));
     };
 
-    if !(0xCB_C0..=0xCB_FF).contains(&sixteen_bit_opcode) {
+    if !(0xCB_30..=0xCB_37).contains(&sixteen_bit_opcode) {
         return Err(parse::Error::UnknownOpcode(opcode));
     }
 
-    let bit = bit_for_set_opcode(sixteen_bit_opcode)?;
     let operand = operand_for_opcode(sixteen_bit_opcode)?;
-    let ins = match operand {
+    let ins = BitManipulationInstruction::SwapNibbles { operand };
-        OpcodeOperand::HLValue => BitManipulationInstruction::SetHLValueBit { bit },
+
-        OpcodeOperand::Register(register) => {
+    Ok((Instruction::BitManipulation(ins), 2))
-            BitManipulationInstruction::SetRegisterBit { register, bit }
+}
-        }
+
+fn parse_individual_bit_operation(
+    opcode: Opcode,
+    operation: IndividualBitOperation,
+    opcode_range: RangeInclusive<u16>,
+) -> ParseResult {
+    let (operand, bit) = decompose_bit_action_opcode(opcode, opcode_range)?;
+    let ins = BitManipulationInstruction::ManipulateIndividualBit {
+        operation,
+        operand,
+        bit,
     };
 
     Ok((Instruction::BitManipulation(ins), 2))
 }
 
-fn operand_for_opcode(opcode: u16) -> Result<OpcodeOperand, parse::Error> {
+fn decompose_bit_action_opcode(
+    opcode: Opcode,
+    opcode_range: RangeInclusive<u16>,
+) -> Result<(Operand, u8), parse::Error> {
+    let Opcode::SixteenBit(sixteen_bit_opcode) = opcode else {
+        return Err(parse::Error::UnknownOpcode(opcode));
+    };
+
+    if !opcode_range.contains(&sixteen_bit_opcode) {
+        return Err(parse::Error::UnknownOpcode(opcode));
+    }
+
+    let operand = operand_for_opcode(sixteen_bit_opcode)?;
+    let bit = bit_for_opcode(sixteen_bit_opcode, opcode_range)?;
+
+    Ok((operand, bit))
+}
+
+fn operand_for_opcode(opcode: u16) -> Result<Operand, parse::Error> {
     let register_nibble = opcode & 0x000F;
     match register_nibble {
-        0x0 | 0x8 => Ok(OpcodeOperand::Register(register::SingleEightBit::B)),
+        0x0 | 0x8 => Ok(Operand::Register(register::SingleEightBit::B)),
-        0x1 | 0x9 => Ok(OpcodeOperand::Register(register::SingleEightBit::C)),
+        0x1 | 0x9 => Ok(Operand::Register(register::SingleEightBit::C)),
-        0x2 | 0xA => Ok(OpcodeOperand::Register(register::SingleEightBit::D)),
+        0x2 | 0xA => Ok(Operand::Register(register::SingleEightBit::D)),
-        0x3 | 0xB => Ok(OpcodeOperand::Register(register::SingleEightBit::E)),
+        0x3 | 0xB => Ok(Operand::Register(register::SingleEightBit::E)),
-        0x4 | 0xC => Ok(OpcodeOperand::Register(register::SingleEightBit::H)),
+        0x4 | 0xC => Ok(Operand::Register(register::SingleEightBit::H)),
-        0x5 | 0xD => Ok(OpcodeOperand::Register(register::SingleEightBit::L)),
+        0x5 | 0xD => Ok(Operand::Register(register::SingleEightBit::L)),
-        0x7 | 0xF => Ok(OpcodeOperand::Register(register::SingleEightBit::A)),
+        0x7 | 0xF => Ok(Operand::Register(register::SingleEightBit::A)),
-        0x6 | 0xE => Ok(OpcodeOperand::HLValue),
+        0x6 | 0xE => Ok(Operand::HLValue),
         _ => Err(super::Error::UnknownOpcode(opcode.into())),
     }
 }
 
-fn bit_for_set_opcode(opcode: u16) -> Result<u8, parse::Error> {
+fn bit_for_opcode(
-    if !(0xCB_C0..=0xCB_FF).contains(&opcode) {
+    opcode: u16,
+    opcode_variant_range: RangeInclusive<u16>,
+) -> Result<u8, parse::Error> {
+    if !opcode_variant_range.contains(&opcode) {
         return Err(parse::Error::UnknownOpcode(opcode.into()));
     }
 
-    let bit = (opcode - 0xCB_C0) / 8;
+    let bit = (opcode - opcode_variant_range.start()) / 8;
 
     // Given the range we've constrained to, this has to be able to fit in 8 bits
     Ok(bit.try_into().unwrap())

src/cpu/run/manip.rs

@@ -1,6 +1,10 @@
 use crate::{
-    cpu::{instructions::manip::BitManipulationInstruction, run, Processor},
+    cpu::{
-    memory, register,
+        instructions::manip::{BitManipulationInstruction, IndividualBitOperation, Operand},
+        run, Processor,
+    },
+    memory,
+    register::{self, SingleEightBit},
 };
 
 use super::{Cycles, Run};
@@ -8,62 +12,211 @@ use super::{Cycles, Run};
 impl Run for BitManipulationInstruction {
     fn run_on(&self, processor: &mut Processor) -> Result<Cycles, run::Error> {
         match *self {
-            BitManipulationInstruction::SetRegisterBit { register, bit } => {
+            BitManipulationInstruction::ManipulateIndividualBit {
-                let register_value = processor.registers.get_single_8bit_register(register);
+                operand: Operand::Register(register),
-                let upd_value = set_nth_bit(register_value, bit);
+                operation,
-
+                bit,
-                processor
+            } => {
-                    .registers
+                transform_register_value(processor, register, |value| {
-                    .set_single_8bit_register(register, upd_value);
+                    operate_on_bit(operation, value, bit)
+                });
 
                 Ok(Cycles(8))
             }
 
-            BitManipulationInstruction::SetHLValueBit { bit } => {
+            BitManipulationInstruction::ManipulateIndividualBit {
-                // TODO: wow this code kinda sucks, I kinda wanna make a better to deal with the HL values
+                operand: Operand::HLValue,
-
+                operation,
-                let hl_addr = processor
+                bit,
-                    .registers
+            } => {
-                    .get_combined_register(register::Combined::HL);
+                transform_hl_value(processor, |value| operate_on_bit(operation, value, bit))?;
-
-                #[allow(clippy::match_wildcard_for_single_variants)]
-                let hl_value = processor
-                    .memory
-                    .get(hl_addr.into())
-                    .map_err(|err| match err {
-                        memory::Error::GetInvalidAddress(addr) => {
-                            run::Error::InvalidRegisterAddress(
-                                register::SixteenBit::Combined(register::Combined::HL),
-                                addr,
-                            )
-                        }
-                        err => run::Error::Unknown(Box::new(err)),
-                    })?;
-
-                let upd_value = set_nth_bit(hl_value, bit);
-
-                #[allow(clippy::match_wildcard_for_single_variants)]
-                processor
-                    .memory
-                    .set(hl_addr.into(), upd_value)
-                    .map_err(|err| match err {
-                        memory::Error::SetInvalidAddress(addr, _value) => {
-                            run::Error::InvalidRegisterAddress(
-                                register::SixteenBit::Combined(register::Combined::HL),
-                                addr,
-                            )
-                        }
-                        err => run::Error::Unknown(Box::new(err)),
-                    })?;
 
                 Ok(Cycles(16))
             }
+
+            BitManipulationInstruction::TestIndividualBit {
+                operand: Operand::Register(register),
+                bit,
+            } => {
+                test_bit_and_set_flags(processor, Operand::Register(register), bit)?;
+
+                Ok(Cycles(8))
+            }
+
+            BitManipulationInstruction::TestIndividualBit {
+                operand: Operand::HLValue,
+                bit,
+            } => {
+                test_bit_and_set_flags(processor, Operand::HLValue, bit)?;
+
+                Ok(Cycles(12))
+            }
+
+            BitManipulationInstruction::SwapNibbles {
+                operand: Operand::Register(register),
+            } => {
+                transform_register_value(processor, register, swap_nibbles);
+                set_zero_flag_after_swap(processor, register);
+
+                Ok(Cycles(8))
+            }
+
+            _ => todo!(),
         }
     }
 }
 
+fn operate_on_bit(operation: IndividualBitOperation, value: u8, bit: u8) -> u8 {
+    match operation {
+        IndividualBitOperation::Set => set_nth_bit(value, bit),
+        IndividualBitOperation::Clear => clear_nth_bit(value, bit),
+    }
+}
+
 fn set_nth_bit(value: u8, bit: u8) -> u8 {
     assert!(bit < 8, "Cannot set a bit greater than 7");
 
     value | (0x1 << bit)
 }
+
+fn clear_nth_bit(value: u8, bit: u8) -> u8 {
+    assert!(bit < 8, "Cannot clear a bit greater than 7");
+
+    let clear_mask = !(0x1 << bit);
+    value & clear_mask
+}
+
+fn swap_nibbles(value: u8) -> u8 {
+    let top = value & 0xF0;
+    let bottom = value & 0x0F;
+
+    (top >> 4) | (bottom << 4)
+}
+
+fn transform_register_value<F: Fn(u8) -> u8>(
+    processor: &mut Processor,
+    register: SingleEightBit,
+    transform: F,
+) {
+    let register_value = processor.registers.get_single_8bit_register(register);
+    let upd_value = transform(register_value);
+
+    processor
+        .registers
+        .set_single_8bit_register(register, upd_value);
+}
+
+fn transform_hl_value<F: Fn(u8) -> u8>(
+    processor: &mut Processor,
+    transform: F,
+) -> Result<(), run::Error> {
+    let hl_addr = processor
+        .registers
+        .get_combined_register(register::Combined::HL);
+
+    #[allow(clippy::match_wildcard_for_single_variants)]
+    let hl_value = processor
+        .memory
+        .get(hl_addr.into())
+        .map_err(|err| match err {
+            memory::Error::GetInvalidAddress(addr) => run::Error::InvalidRegisterAddress(
+                register::SixteenBit::Combined(register::Combined::HL),
+                addr,
+            ),
+            err => run::Error::Unknown(Box::new(err)),
+        })?;
+
+    let upd_value = transform(hl_value);
+
+    #[allow(clippy::match_wildcard_for_single_variants)]
+    processor
+        .memory
+        .set(hl_addr.into(), upd_value)
+        .map_err(|err| match err {
+            memory::Error::SetInvalidAddress(addr, _value) => run::Error::InvalidRegisterAddress(
+                register::SixteenBit::Combined(register::Combined::HL),
+                addr,
+            ),
+            err => run::Error::Unknown(Box::new(err)),
+        })?;
+
+    Ok(())
+}
+
+fn set_zero_flag_after_swap(processor: &mut Processor, register: register::SingleEightBit) {
+    let value = processor.registers.get_single_8bit_register(register);
+    // Clear all flags
+    processor
+        .registers
+        .set_raw_flag_bits(0x00)
+        .expect("lower four bits of register aren't set");
+
+    if value == 0 {
+        processor.registers.set_flag_bit(register::Flag::Zero, 1);
+    } else {
+        processor.registers.set_flag_bit(register::Flag::Zero, 0);
+    }
+}
+
+fn test_bit_and_set_flags(
+    processor: &mut Processor,
+    operand: Operand,
+    bit: u8,
+) -> Result<(), run::Error> {
+    assert!(bit < 8, "Cannot test a bit greater than 7");
+
+    let operand_val = get_operand_value(processor, operand)?;
+    let compare_mask = 0x1 << bit;
+    let zero_flag = (compare_mask & operand_val) == 0;
+
+    processor
+        .registers
+        .set_flag_bit(register::Flag::Zero, zero_flag.into());
+
+    processor
+        .registers
+        .set_flag_bit(register::Flag::Subtract, 0);
+    processor
+        .registers
+        .set_flag_bit(register::Flag::HalfCarry, 1);
+
+    Ok(())
+}
+
+fn get_operand_value(processor: &mut Processor, operand: Operand) -> Result<u8, run::Error> {
+    let val = match operand {
+        Operand::Register(register) => processor.registers.get_single_8bit_register(register),
+        Operand::HLValue => {
+            let hl_addr = processor
+                .registers
+                .get_combined_register(register::Combined::HL);
+
+            #[allow(clippy::match_wildcard_for_single_variants)]
+            processor
+                .memory
+                .get(hl_addr.into())
+                .map_err(|err| match err {
+                    memory::Error::GetInvalidAddress(addr) => run::Error::InvalidRegisterAddress(
+                        register::SixteenBit::Combined(register::Combined::HL),
+                        addr,
+                    ),
+                    err => run::Error::Unknown(Box::new(err)),
+                })?
+        }
+    };
+
+    Ok(val)
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use test_case::test_case;
+
+    #[test_case(0xA5, 0x5A)]
+    #[test_case(0x5A, 0xA5)]
+    #[test_case(0xFF, 0xFF)]
+    fn test_swap_nibbles(n: u8, expected: u8) {
+        assert_eq!(expected, swap_nibbles(n));
+    }
+}