Improve code quality

src/emu.rs

@@ -251,8 +251,7 @@ impl Chip8 {
 
     fn and_vx_vy(&mut self, x: u8, y: u8) {
         // calc bitwise AND on Vx and Vy, then store in Vx
-        let x = x as usize;
+        self.v[x as usize] &= self.v[y as usize];
-        self.v[x] = self.v[x] & self.v[y as usize];
     }
 
     fn xor(&mut self, x: u8, y: u8) {
@@ -272,13 +271,16 @@ impl Chip8 {
     }
 
     fn sub_vx_vy(&mut self, x: u8, y: u8) {
+        // TODO: Confirm that the default behaviour
+        // for this opcode includes a wrapping subtraction
+
         // subtract Vx and Vy, if Vx > Vy VF is set to 1, otherwise 0
         // then set Vx to Vx - Vy
         let vx = self.v[x as usize];
         let vy = self.v[y as usize];
 
         self.v[0xF] = if vx > vy { 1 } else { 0 };
-        self.v[x as usize] = vx - vy;
+        self.v[x as usize] = vx.wrapping_sub(vy);
     }
 
     fn shr(&mut self, x: u8) {
@@ -297,7 +299,7 @@ impl Chip8 {
         let vy = self.v[y as usize];
 
         self.v[0xF] = if vy > vx { 1 } else { 0 };
-        self.v[x as usize] = vy - vx;
+        self.v[x as usize] = vy.wrapping_sub(vx);
     }
 
     fn shl(&mut self, x: u8) {
@@ -305,8 +307,8 @@ impl Chip8 {
         // then shift Vx one to the left;
         let x = x as usize;
 
-        self.v[0xF] = if (self.v[x] & 0x80) == 1 { 1 } else { 0 };
+        self.v[0xF] = if (self.v[x] >> 7) == 1 { 1 } else { 0 };
-        self.v[x] = self.v[x] << 1;
+        self.v[x] <<= 1;
     }
 
     fn sne_vx_vy(&mut self, x: u8, y: u8) {
@@ -416,7 +418,7 @@ impl Chip8 {
 
     fn add_vx_to_i(&mut self, x: u8) {
         // set I to be I + Vx
-        self.i = self.i + self.v[x as usize] as u16;
+        self.i += self.v[x as usize] as u16;
     }
 
     fn set_i_to_hex_sprite_loc(&mut self, x: u8) {
@@ -708,10 +710,11 @@ mod test {
 
         chip8.add_vx_vy(0xA, 0x8);
         assert_eq!(chip8.v[0xA], 0x01 + 0xC8);
+        assert_eq!(chip8.v[0xF], 0);
     }
 
     #[test]
-    fn opcode_add_vx_vy_overflows_as_expected() {
+    fn opcode_add_vx_vy_works_vf_set() {
         let mut chip8: Chip8 = Default::default();
 
         chip8.v[0xE] = 0xFF;
@@ -722,4 +725,144 @@ mod test {
         assert_eq!(chip8.v[0xA], (0xFF as u8).wrapping_add(0x01));
         assert_eq!(chip8.v[0xF], 1);
     }
+
+    #[test]
+    fn opcode_sub_vx_vy_works_vf_set() {
+        let mut chip8: Chip8 = Default::default();
+
+        chip8.v[0x3] = 0xD5;
+        chip8.v[0xD] = 0x71;
+
+        chip8.sub_vx_vy(0x3, 0xD);
+
+        assert_eq!(chip8.v[0xF], 1);
+        assert_eq!(chip8.v[0x3], 0xD5 - 0x71);
+    }
+
+    #[test]
+    fn opcode_sub_vx_vy_works() {
+        let mut chip8: Chip8 = Default::default();
+
+        chip8.v[0xB] = 0x23;
+        chip8.v[0x5] = 0xCA;
+
+        chip8.sub_vx_vy(0xB, 0x5);
+
+        assert_eq!(chip8.v[0xF], 0);
+        assert_eq!(chip8.v[0xB], (0x23 as u8).wrapping_sub(0xCA))
+    }
+
+    #[test]
+    fn opcode_shr_works() {
+        let mut chip8: Chip8 = Default::default();
+
+        // LSB == 1
+        chip8.v[0x6] = 0xC8;
+        chip8.shr(0x6);
+
+        assert_eq!(chip8.v[0xF], 0);
+        assert_eq!(chip8.v[0x6], 0xC8 >> 1);
+    }
+
+    #[test]
+    fn opcode_shr_works_vf_set() {
+        let mut chip8: Chip8 = Default::default();
+
+        chip8.v[0xD] = 0xB5;
+        chip8.shr(0xD);
+
+        assert_eq!(chip8.v[0xF], 1);
+        assert_eq!(chip8.v[0xD], 0xB5 >> 1);
+    }
+
+    #[test]
+    fn opcode_subn_vx_vy_works() {
+        let mut chip8: Chip8 = Default::default();
+
+        chip8.v[0xC] = 0x87;
+        chip8.v[0x9] = 0x04;
+
+        chip8.subn_vx_vy(0xC, 0x9);
+        assert_eq!(chip8.v[0xC], (0x04 as u8).wrapping_sub(0x87));
+        assert_eq!(chip8.v[0xF], 0);
+    }
+
+    #[test]
+    fn opcode_subn_vx_vy_works_vf_set() {
+        let mut chip8: Chip8 = Default::default();
+
+        chip8.v[0x6] = 0x4C;
+        chip8.v[0x4] = 0xE8;
+
+        chip8.subn_vx_vy(0x6, 0x4);
+
+        assert_eq!(chip8.v[0xF], 1);
+        assert_eq!(chip8.v[0x6], 0xE8 - 0x4C);
+    }
+
+    #[test]
+    fn opcode_shl_works() {
+        let mut chip8: Chip8 = Default::default();
+
+        chip8.v[0xC] = 0x47;
+        chip8.shl(0xC);
+
+        assert_eq!(chip8.v[0xC], 0x47 << 1);
+        assert_eq!(chip8.v[0xF], 0);
+    }
+
+    #[test]
+    fn opcode_shl_works_vf_set() {
+        let mut chip8: Chip8 = Default::default();
+
+        chip8.v[0x7] = 0xC7;
+        chip8.shl(0x7);
+
+        assert_eq!(chip8.v[0x7], 0xC7 << 1);
+        assert_eq!(chip8.v[0xF], 1);
+    }
+
+    #[test]
+    fn opcode_sne_vx_vy_works() {
+        let mut chip8: Chip8 = Default::default();
+
+        // Vx != Vy
+        chip8.pc = 0x200;
+        chip8.v[0x4] = 0x5F;
+        chip8.v[0x3] = 0xF8;
+
+        chip8.sne_vx_vy(0x4, 0x3);
+
+        assert_eq!(chip8.pc, 0x202);
+
+        // Vx == Vy
+        chip8.pc = 0x200;
+        chip8.v[0x4] = 0x5F;
+        chip8.v[0x3] = 0x5F;
+
+        chip8.sne_vx_vy(0x4, 0x3);
+
+        assert_ne!(chip8.pc, 0x202);
+    }
+
+    #[test]
+    fn opcode_load_addr_to_i_works() {
+        let mut chip8: Chip8 = Default::default();
+
+        chip8.i = 0;
+        chip8.load_addr_to_i(0x1ABC & 0x0FFF);
+
+        assert_eq!(chip8.i, 0x0ABC);
+    }
+
+    #[test]
+    fn opcode_jmp_addr_with_offset_works() {
+        let mut chip8: Chip8 = Default::default();
+
+        chip8.pc = 0x200;
+        chip8.v[0x0] = 0xA2;
+        chip8.jmp_addr_with_offset(0x05D2);
+
+        assert_eq!(chip8.pc, 0x05D2 + 0xA2);
+    }
 }

src/periph.rs

@@ -72,7 +72,7 @@ impl Keypad {
         let mut found: Option<u8> = None;
 
         for (index, key) in self.keys.iter().enumerate() {
-            if *key == true {
+            if *key {
                 found = Some(index as u8);
                 break;
             }