Fix the sound timer, sprite wrapping under clip quirks, the quirked jump address, and Fx0A waiting for release

chip8/cpu.py

@@ -132,6 +132,7 @@ def __init__(
 
         self.awaiting_keypress = False
         self.keypress_register = None
+        self.keypress_pending = None
 
         # The operation_lookup table is executed according to the most
         # significant byte of the operand (e.g. operand 8nnn would call
@@ -815,9 +816,11 @@ def jump_to_register_plus_value(self):
                     B         x        n         n
         """
         if self.jump_quirks:
+            # The whole twelve bit address is used; its top nibble also
+            # names the register, so BE9C with vE at 4 jumps to 0xEA0.
             x = (self.operand & 0x0F00) >> 8
-            self.pc = self.v[x] + (self.operand & 0x00FF)
-            self.last_op = f"JUMP V{x:01X} + {self.operand & 0x0FF:03X}"
+            self.pc = self.v[x] + (self.operand & 0x0FFF)
+            self.last_op = f"JUMP V{x:01X} + {self.operand & 0x0FFF:03X}"
         else:
             self.pc = self.v[0] + (self.operand & 0x0FFF)
             self.last_op = f"JUMP V0 + {self.operand & 0x0FFF:03X}"
@@ -908,6 +911,11 @@ def draw_normal(self, x_pos, y_pos, num_bytes, bitplane, index=None):
         if not index:
             index = self.index
 
+        # The starting position always wraps around the screen. Clipping only
+        # decides what happens to the part of the sprite past the edge.
+        x_pos = x_pos % self.screen.get_width()
+        y_pos = y_pos % self.screen.get_height()
+
         for y_index in range(num_bytes):
             color_byte = self.memory[index + y_index]
             y_coord = y_pos + y_index
@@ -940,6 +948,10 @@ def draw_extended(self, x_pos, y_pos, bitplane, index=None):
         if not index:
             index = self.index
 
+        # As in draw_normal: the start wraps, clipping applies to the overhang.
+        x_pos = x_pos % self.screen.get_width()
+        y_pos = y_pos % self.screen.get_height()
+
         for y_index in range(16):
             for x_byte in range(2):
                 color_byte = self.memory[index + (y_index * 2) + x_byte]
@@ -1032,6 +1044,7 @@ def wait_for_keypress(self):
         x = (self.operand & 0x0F00) >> 8
         self.awaiting_keypress = True
         self.keypress_register = x
+        self.keypress_pending = None
         self.last_op = f"KEYD V{x:01X}"
 
     def decode_keypress_and_continue(self, keys_pressed):
@@ -1044,8 +1057,26 @@ def decode_keypress_and_continue(self, keys_pressed):
         """
         for keyval, lookup_key in KEY_MAPPINGS.items():
             if keys_pressed[lookup_key]:
-                self.v[self.keypress_register] = keyval
-                self.awaiting_keypress = False
+                # The original hardware moves on when the key is released,
+                # not when it goes down, so the key is only remembered here.
+                self.keypress_pending = keyval
+                return
+
+    def decode_keyrelease_and_continue(self, keys_pressed):
+        """
+        Given a set of keys pressed, checks whether the key remembered by
+        decode_keypress_and_continue has been released, and if it has,
+        stores it in the register specified by wait_for_keypress and flags
+        the CPU to continue executing.
+
+        :param keys_pressed: the list of keys pressed
+        """
+        if self.keypress_pending is None:
+            return
+        if not keys_pressed[KEY_MAPPINGS[self.keypress_pending]]:
+            self.v[self.keypress_register] = self.keypress_pending
+            self.keypress_pending = None
+            self.awaiting_keypress = False
 
     def move_reg_into_delay_timer(self):
         """
@@ -1284,7 +1315,7 @@ def decrement_timers(self):
         self.tick_counter = 0
 
         self.delay -= 1 if self.delay > 0 else 0
-        self.sound -= 1 if self.delay > 0 else 0
+        self.sound -= 1 if self.sound > 0 else 0
         if self.sound > 0 and not self.sound_playing:
             if self.sound_waveform:
                 self.sound_waveform.play(loops=-1)

chip8/emulator.py

@@ -67,5 +67,8 @@ def main_loop(args):
                     cpu.running = False
                 if cpu.awaiting_keypress:
                     cpu.decode_keypress_and_continue(keys_pressed)
+            if event.type == pygame.KEYUP:
+                if cpu.awaiting_keypress:
+                    cpu.decode_keyrelease_and_continue(pygame.key.get_pressed())
 
 # E N D   O F   F I L E #######################################################

test/test_chip8cpu.py

@@ -508,7 +508,17 @@ def test_jump_to_index_plus_value_quirks(self):
                     self.cpu.operand = value
                     self.cpu.operand |= (register << 8)
                     self.cpu.jump_to_register_plus_value()
-                    self.assertEqual(index + value, self.cpu.pc)
+                    # The whole twelve bit address counts, including the
+                    # nibble that names the register, which is what the
+                    # jump row of Timendus' quirks test checks.
+                    self.assertEqual(index + ((register << 8) | value), self.cpu.pc)
+
+    def test_jump_to_index_plus_value_quirks_keeps_high_nibble(self):
+        self.cpu.jump_quirks = True
+        self.cpu.v[0xE] = 0x9C
+        self.cpu.operand = 0xBE00
+        self.cpu.jump_to_register_plus_value()
+        self.assertEqual(0xE9C, self.cpu.pc)
 
     def test_generate_random_number(self):
         for register in range(0x10):
@@ -718,6 +728,13 @@ def test_decrement_timers_decrements_by_one(self):
         self.assertEqual(1, self.cpu.delay)
         self.assertEqual(1, self.cpu.sound)
 
+    def test_decrement_timers_sound_counts_down_while_delay_is_zero(self):
+        self.cpu.delay = 0
+        self.cpu.sound = 2
+        self.cpu.decrement_timers()
+        self.assertEqual(0, self.cpu.delay)
+        self.assertEqual(1, self.cpu.sound)
+
     def test_decrement_timers_does_not_go_negative(self):
         self.cpu.delay = 0
         self.cpu.sound = 0
@@ -931,6 +948,23 @@ def test_draw_sprite_normal_bitplane_1_integration_correct(self):
         self.assertFalse(self.screen.get_pixel(6, 0, 2))
         self.assertFalse(self.screen.get_pixel(7, 0, 2))
 
+    def test_draw_sprite_normal_start_wraps_even_with_clip_quirks(self):
+        self.screen = Chip8Screen(2)
+        self.screen.init_display()
+        self.cpu = Chip8CPU(self.screen, clip_quirks=True)
+
+        # A sprite at (70, 40) on a 64 by 32 screen starts at (6, 8). Only
+        # the part that hangs past an edge is clipped.
+        self.cpu.memory[0x0200] = 0xD0
+        self.cpu.memory[0x0201] = 0x11
+        self.cpu.memory[0x5000] = 0x80
+        self.cpu.index = 0x5000
+        self.cpu.v[0] = 70
+        self.cpu.v[1] = 40
+        self.cpu.bitplane = 1
+        self.cpu.execute_instruction()
+        self.assertTrue(self.screen.get_pixel(6, 8, 1))
+
     def test_draw_sprite_extended_bitplane_1_integration_correct(self):
         self.screen = Chip8Screen(2)
         self.screen.init_display()
@@ -1354,6 +1388,28 @@ def test_wait_for_keypress_sets_awaiting_keypress(self):
         self.assertEqual(1, self.cpu.keypress_register)
         self.assertTrue(self.cpu.awaiting_keypress)
 
+    def test_wait_for_keypress_continues_on_release_not_press(self):
+        self.cpu.operand = 0x0100
+        self.cpu.wait_for_keypress()
+        pressed = [False] * 512
+        pressed[pygame.K_w] = True
+        self.cpu.decode_keypress_and_continue(pressed)
+        self.assertTrue(self.cpu.awaiting_keypress)
+        self.assertEqual(0, self.cpu.v[1])
+        released = [False] * 512
+        self.cpu.decode_keyrelease_and_continue(released)
+        self.assertFalse(self.cpu.awaiting_keypress)
+        self.assertEqual(0x5, self.cpu.v[1])
+
+    def test_wait_for_keypress_release_of_another_key_does_not_continue(self):
+        self.cpu.operand = 0x0100
+        self.cpu.wait_for_keypress()
+        pressed = [False] * 512
+        pressed[pygame.K_w] = True
+        self.cpu.decode_keypress_and_continue(pressed)
+        self.cpu.decode_keyrelease_and_continue(pressed)
+        self.assertTrue(self.cpu.awaiting_keypress)
+
     def test_store_subset_regs_one_two(self):
         self.cpu.v[1] = 5
         self.cpu.v[2] = 6