Add a way to safely read from controllers

First of all, move some reading utilities into the new `shared`
directory, so controllers can be read by more than one example. Second,
even if we keep the example on `basics/input.s` simple, let's provide a
safe alternative for the other examples.

Signed-off-by: Miquel Sabaté Solà <msabate@suse.com>

4 files changed, 74 insertions, 104 deletions

diff --git a/basics/flicker.s b/basics/flicker.s
index b170d5e..640817a 100644
--- a/basics/flicker.s
+++ b/basics/flicker.s
@@ -36,7 +36,7 @@
 @main_game_loop:
     ;; NOTE: the logic is pretty simply: read the pad, move the player
     ;; accordingly, and apply the flickering effect.
-    jsr Joypad::read
+    jsr joypad_read
     jsr Diskun::update
 
     ;; NOTE: comment this `jsr` out if you want to see what happens if no
diff --git a/basics/input.s b/basics/input.s
index dbb9ee6..6918487 100644
--- a/basics/input.s
+++ b/basics/input.s
@@ -8,6 +8,11 @@
 ;;         we are constantly polling it and filling it, the value will move constantly).
 ;;  - $21: the previous status of the right arrow.
 ;;  - $42: the counter which is incremented on each press of the right arrow button.
+;;
+;; Whenever you are done with this example, hop into the `shared/joypad.s` file,
+;; which brings some other considerations when reading from controllers. That
+;; is, the algorithm shown below is not entirely "safe" due to a hardware bug
+;; which might give unreliable inputs on some spikes.
 ;;;
 
 ;;;
diff --git a/scroll/include/joypad.s b/scroll/include/joypad.s
deleted file mode 100644
index 8711c28..0000000
--- a/scroll/include/joypad.s
+++ /dev/null
@@ -1,63 +0,0 @@
-.segment "CODE"
-
-;;;
-;; Joypad controller code. The following memory addresses are reserved: $21-$23.
-;;
-;; Memory address $21 is used for internal purposes, whereas $22 and $23 contain
-;; the bitmask of the buttons that are pressed from each controller.
-;;;
-
-.scope Joypad
-    ;; Button masks.
-    BUTTON_A      = 1 << 7
-    BUTTON_B      = 1 << 6
-    BUTTON_SELECT = 1 << 5
-    BUTTON_START  = 1 << 4
-    BUTTON_UP     = 1 << 3
-    BUTTON_DOWN   = 1 << 2
-    BUTTON_LEFT   = 1 << 1
-    BUTTON_RIGHT  = 1 << 0
-
-    ;; Port addresses for controllers.
-    JOYPAD1 = $4016
-    JOYPAD2 = $4017
-
-    ;; We keep all the information from controller from mainly two variables:
-    ;; m_buttons1 and m_buttons2; containing respectively the buttons pressed
-    ;; for this frame for both controllers. The m_inv_buttons ($21) is an
-    ;; internal variable and should not be used for anything outside of this
-    ;; usage.
-    m_inv_buttons = $21
-    m_buttons1    = $22
-    m_buttons2    = $23
-
-    ;; READ_CONTROLLER reads the input from the controller mapped into the given
-    ;; port, and saves the state into the given `buttons` address.
-    ;; Implementation taken from NESHacker's example of smb3-like movement.
-    .macro READ_CONTROLLER port, buttons
-        lda m_inv_buttons
-        tay
-        lda #1
-        sta port
-        sta m_inv_buttons
-        lsr
-        sta port
-    :
-        lda port
-        lsr
-        rol m_inv_buttons
-        bcc :-
-        tya
-        eor m_inv_buttons
-        and m_inv_buttons
-        sta buttons
-    .endmacro
-
-    ;; read sets the values for m_buttons1 and m_buttons2 as read from both
-    ;; controllers.
-    .proc read
-        READ_CONTROLLER JOYPAD1, m_buttons1
-        READ_CONTROLLER JOYPAD2, m_buttons2
-        rts
-    .endproc
-.endscope
diff --git a/shared/joypad.s b/shared/joypad.s
index 2abf959..7c3e44c 100644
--- a/shared/joypad.s
+++ b/shared/joypad.s
@@ -1,5 +1,3 @@
-;;; TODO: safe read
-
 .scope Joypad
     ;; Button masks.
     BUTTON_A      = 1 << 7
@@ -15,42 +13,72 @@
     JOYPAD1 = $4016
     JOYPAD2 = $4017
 
-    ;; We keep all the information from controller from mainly two variables:
-    ;; m_buttons1 and m_buttons2; containing respectively the buttons pressed
-    ;; for this frame for both controllers. The m_inv_buttons ($21) is an
-    ;; internal variable and should not be used for anything outside of this
-    ;; usage.
-    m_inv_buttons = $21
-    m_buttons1    = $22
-    m_buttons2    = $23
-
-    ;; READ_CONTROLLER reads the input from the controller mapped into the given
-    ;; port, and saves the state into the given `buttons` address.
-    ;; Implementation taken from NESHacker's example of smb3-like movement.
-    .macro READ_CONTROLLER port, buttons
-        lda m_inv_buttons
-        tay
-        lda #1
-        sta port
-        sta m_inv_buttons
-        lsr
-        sta port
-    :
-        lda port
-        lsr
-        rol m_inv_buttons
-        bcc :-
-        tya
-        eor m_inv_buttons
-        and m_inv_buttons
-        sta buttons
-    .endmacro
-
-    ;; read sets the values for m_buttons1 and m_buttons2 as read from both
-    ;; controllers.
-    .proc read
-        READ_CONTROLLER JOYPAD1, m_buttons1
-        READ_CONTROLLER JOYPAD2, m_buttons2
-        rts
-    .endproc
+    ;; After running a `joypad_read_*` function these two variables will contain
+    ;; the given result.
+    m_buttons1 = $22
+    m_buttons2 = $23
 .endscope
+
+;;;
+;; Read the first joypad. This method is fast but it might be vulnerable to the
+;; DPCM bug (see: https://www.nesdev.org/wiki/Controller_reading_code).
+joypad_unsafe_read:
+    ldx #$00
+    ;; NOTE: fallthrough
+
+;;;
+;; Read the joypad as indexed by the X register (0 for controller 1; 1 for
+;; controller 2). This method is fast but it might be vulnerable to the DPCM bug
+;; (see: https://www.nesdev.org/wiki/Controller_reading_code).
+joypad_unsafe_read_x:
+    ;; Start the latch process.
+    lda #$01
+    sta Joypad::JOYPAD1
+    sta Joypad::m_buttons1, x   ; Bit as a guard for the loop below.
+    lsr a
+    sta Joypad::JOYPAD1
+
+    ;; Now the joypad is ready to accept reads.
+@joypad_unsafe_read_x_loop:
+    lda Joypad::JOYPAD1, x
+    and #%00000011              ; Ignore bits other than controller.
+    cmp #$01                    ; Set carry if and only if nonzero.
+    rol Joypad::m_buttons1, x   ; Carry -> bit 0; bit 7 -> Carry
+    bcc @joypad_unsafe_read_x_loop
+    rts
+
+;;;
+;; Safely read the first controller via a re-read algorithm.
+joypad_read:
+    ldx #$00
+
+    ;; NOTE: uncomment these two lines to also read safely the second
+    ;; controller.
+    ;;
+    ;; jsr joypad_read_x
+    ;; inx
+
+    ;; NOTE: fallthrough
+
+;;;
+;; Safely read via a re-read algorithm the joypad as indexed by the X register
+;; (0 for controller 1; 1 for controller 2).
+joypad_read_x:
+    jsr joypad_unsafe_read_x
+
+    ;; The main idea around a re-read algorithm is that you read the controller
+    ;; "unsafely" once, then you do it again and compare both reads. If they
+    ;; were the same then we are on the safe side. Otherwise we would need to
+    ;; loop until we get two identical reads. This sounds bad but in practice
+    ;; it's not so much (and hey, if it worked for Super Mario Bros. 3, it
+    ;; should work for us too :P). Otherwise there is the algorithm via OAM DMA,
+    ;; but it sure is tricky.
+@joypad_read_x_reread:
+    lda Joypad::m_buttons1, x
+    pha
+    jsr joypad_unsafe_read_x
+    pla
+    cmp Joypad::m_buttons1, x
+    bne @joypad_read_x_reread
+
+    rts