begin IO Register classification, start with KEYINPUT

.travis.yml

@@ -27,13 +27,15 @@ script:
   - export PATH="$PATH:/opt/devkitpro/devkitARM/bin"
   - export PATH="$PATH:/opt/devkitpro/tools/bin"
   - cd ..
-  # Run all tests, both modes
+  # Run all verificaions, both debug and release
+  - cargo clippy
+  - cargo clippy --release
   - cargo test --no-fail-fast --lib
   - cargo test --no-fail-fast --lib --release
   - cargo test --no-fail-fast --tests
   - cargo test --no-fail-fast --tests --release
-  # cargo make defaults to both debug and release builds of all examples
+  # Let cargo make take over the rest
-  - cargo make
+  - cargo make build-all
   # Test build the book so that a failed book build kills this run
   - cd book && mdbook build
 

Makefile.toml

@@ -55,15 +55,15 @@ fn main() -> std::io::Result<()> {
 '''
 ]
 
-[tasks.build]
-dependencies = ["build-examples-debug", "build-examples-release", "pack-roms"]
-
-[tasks.justrelease]
-dependencies = ["build-examples-release", "pack-roms"]
-
 [tasks.test]
 command = "cargo"
 args = ["test", "--lib"]
 
+[tasks.justrelease]
+dependencies = ["build-examples-release", "pack-roms"]
+
+[tasks.build-all]
+dependencies = ["build-examples-debug", "build-examples-release", "pack-roms"]
+
 [tasks.default]
-alias = "build"
+alias = "build-all"

book/src/04-non-video/01-buttons.md

@@ -1 +1,5 @@
 # Buttons
+
+It's all well and good to just show a picture, even to show an animation, but if
+we want a game we have to let the user interact with something.
+

src/builtins.rs

@@ -28,7 +28,7 @@ pub extern "C" fn __clzsi2(mut x: usize) -> usize {
   {
     y = x >> 32;
     if y != 0 {
-      n = n - 32;
+      n -= 32;
       x = y;
     }
   }
@@ -36,23 +36,23 @@ pub extern "C" fn __clzsi2(mut x: usize) -> usize {
   {
     y = x >> 16;
     if y != 0 {
-      n = n - 16;
+      n -= 16;
       x = y;
     }
   }
   y = x >> 8;
   if y != 0 {
-    n = n - 8;
+    n -= 8;
     x = y;
   }
   y = x >> 4;
   if y != 0 {
-    n = n - 4;
+    n -= 4;
     x = y;
   }
   y = x >> 2;
   if y != 0 {
-    n = n - 2;
+    n -= 2;
     x = y;
   }
   y = x >> 1;

src/fixed.rs

@@ -135,6 +135,7 @@ macro_rules! fixed_point_signed_multiply {
   ($t:ident) => {
     impl<F: Unsigned> Mul for Fx<$t, F> {
       type Output = Self;
+      #[allow(clippy::suspicious_arithmetic_impl)]
       fn mul(self, rhs: Fx<$t, F>) -> Self::Output {
         let pre_shift = (self.num as i32).wrapping_mul(rhs.num as i32);
         if pre_shift < 0 {
@@ -168,6 +169,7 @@ macro_rules! fixed_point_unsigned_multiply {
   ($t:ident) => {
     impl<F: Unsigned> Mul for Fx<$t, F> {
       type Output = Self;
+      #[allow(clippy::suspicious_arithmetic_impl)]
       fn mul(self, rhs: Fx<$t, F>) -> Self::Output {
         Fx {
           num: ((self.num as u32).wrapping_mul(rhs.num as u32) >> F::U8) as $t,
@@ -186,6 +188,7 @@ macro_rules! fixed_point_signed_division {
   ($t:ident) => {
     impl<F: Unsigned> Div for Fx<$t, F> {
       type Output = Self;
+      #[allow(clippy::suspicious_arithmetic_impl)]
       fn div(self, rhs: Fx<$t, F>) -> Self::Output {
         let mul_output: i32 = (self.num as i32).wrapping_mul(1 << F::U8);
         let divide_result: i32 = crate::bios::div(mul_output, rhs.num as i32);
@@ -206,6 +209,7 @@ macro_rules! fixed_point_unsigned_division {
   ($t:ident) => {
     impl<F: Unsigned> Div for Fx<$t, F> {
       type Output = Self;
+      #[allow(clippy::suspicious_arithmetic_impl)]
       fn div(self, rhs: Fx<$t, F>) -> Self::Output {
         let mul_output: i32 = (self.num as i32).wrapping_mul(1 << F::U8);
         let divide_result: i32 = crate::bios::div(mul_output, rhs.num as i32);

src/io.rs

@@ -0,0 +1,13 @@
+//! This module contains definitions and types for the IO Registers.
+//!
+//! ## Naming
+//!
+//! In the interest of making things easy to search for, all io register
+//! constants are given the names used in the
+//! [GBATEK](https://problemkaputt.de/gbatek.htm) technical description.
+
+use super::*;
+
+use gba_proc_macro::register_bit;
+
+pub mod keypad;

src/io/keypad.rs

@@ -0,0 +1,121 @@
+//! Allows access to the keypad.
+
+use super::*;
+
+/// The Key Input Register.
+///
+/// This register follows the "low-active" convention. If you want your code to
+/// follow the "high-active" convention (hint: you probably do, it's far easier
+/// to work with) then call `read_key_input()` rather than reading this register
+/// directly. It will perform the necessary bit flip operation for you.
+pub const KEYINPUT: VolAddress<u16> = unsafe { VolAddress::new_unchecked(0x400_0130) };
+
+/// A "tribool" value helps us interpret the arrow pad.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+#[repr(i32)]
+#[allow(missing_docs)]
+pub enum TriBool {
+  Minus = -1,
+  Neutral = 0,
+  Plus = 1,
+}
+
+newtype! {
+  /// Records a particular key press combination.
+  ///
+  /// Methods here follow the "high-active" convention, where a bit is enabled
+  /// when it's part of the set.
+  #[derive(Debug, Copy, Clone, Default, PartialEq, Eq)]
+  KeyInput, u16
+}
+
+#[allow(missing_docs)]
+impl KeyInput {
+  register_bit!(A_BIT, u16, 1, a_pressed);
+  register_bit!(B_BIT, u16, 1 << 1, b_pressed);
+  register_bit!(SELECT_BIT, u16, 1 << 2, select_pressed);
+  register_bit!(START_BIT, u16, 1 << 3, start_pressed);
+  register_bit!(RIGHT_BIT, u16, 1 << 4, right_pressed);
+  register_bit!(LEFT_BIT, u16, 1 << 5, left_pressed);
+  register_bit!(UP_BIT, u16, 1 << 6, up_pressed);
+  register_bit!(DOWN_BIT, u16, 1 << 7, down_pressed);
+  register_bit!(R_BIT, u16, 1 << 8, r_pressed);
+  register_bit!(L_BIT, u16, 1 << 9, l_pressed);
+
+  /// Takes the set difference between these keys and another set of keys.
+  pub fn difference(self, other: Self) -> Self {
+    KeyInput(self.0 ^ other.0)
+  }
+
+  /// Gives the arrow pad value as a tribool, with Plus being increased column
+  /// value (right).
+  pub fn column_direction(self) -> TriBool {
+    if self.right_pressed() {
+      TriBool::Plus
+    } else if self.left_pressed() {
+      TriBool::Minus
+    } else {
+      TriBool::Neutral
+    }
+  }
+
+  /// Gives the arrow pad value as a tribool, with Plus being increased row
+  /// value (down).
+  pub fn row_direction(self) -> TriBool {
+    if self.down_pressed() {
+      TriBool::Plus
+    } else if self.up_pressed() {
+      TriBool::Minus
+    } else {
+      TriBool::Neutral
+    }
+  }
+}
+
+/// Gets the current state of the keys
+pub fn read_key_input() -> KeyInput {
+  // Note(Lokathor): The 10 used bits are "low when pressed" style, but the 6
+  // unused bits are always low, so we XOR with this mask to get a result where
+  // the only active bits are currently pressed keys.
+  KeyInput(KEYINPUT.read() ^ 0b0000_0011_1111_1111)
+}
+
+newtype! {
+  /// Allows configuration of when a keypad interrupt fires.
+  ///
+  /// * The most important bit here is the `irq_enabled` bit, which determines
+  ///   if an interrupt happens at all.
+  /// * The second most important bit is the `irq_logical_and` bit. If this bit
+  ///   is set, _all_ the selected buttons are required to be set for the
+  ///   interrupt to be fired (logical AND). If it's not set then _any_ of the
+  ///   buttons selected can be pressed to fire the interrupt (logical OR).
+  /// * All other bits select a particular button to be required or not as part
+  ///   of the interrupt firing.
+  ///
+  /// NOTE: This _only_ configures the operation of when keypad interrupts can
+  /// fire. You must still set the `IME` to have interrupts at all, and you must
+  /// further set `IE` for keypad interrupts to be possible.
+  #[derive(Debug, Copy, Clone, Default, PartialEq, Eq)]
+  KeyInterruptSetting, u16
+}
+#[allow(missing_docs)]
+impl KeyInterruptSetting {
+  register_bit!(A_BIT, u16, 1, a_pressed);
+  register_bit!(B_BIT, u16, 1 << 1, b_pressed);
+  register_bit!(SELECT_BIT, u16, 1 << 2, select_pressed);
+  register_bit!(START_BIT, u16, 1 << 3, start_pressed);
+  register_bit!(RIGHT_BIT, u16, 1 << 4, right_pressed);
+  register_bit!(LEFT_BIT, u16, 1 << 5, left_pressed);
+  register_bit!(UP_BIT, u16, 1 << 6, up_pressed);
+  register_bit!(DOWN_BIT, u16, 1 << 7, down_pressed);
+  register_bit!(R_BIT, u16, 1 << 8, r_pressed);
+  register_bit!(L_BIT, u16, 1 << 9, l_pressed);
+  //
+  register_bit!(IRQ_ENABLE_BIT, u16, 1 << 14, irq_enabled);
+  register_bit!(IRQ_AND_BIT, u16, 1 << 15, irq_logical_and);
+}
+
+/// Use this to configure when a keypad interrupt happens.
+///
+/// See the `KeyInterruptSetting` type for more.
+pub const KEYCNT: VolAddress<KeyInterruptSetting> = unsafe { VolAddress::new_unchecked(0x400_0132) };

src/io_registers.rs

@@ -124,73 +124,3 @@ pub fn wait_until_vdraw() {
   // TODO: make this the better version with BIOS and interrupts and such.
   while vcount() >= SCREEN_HEIGHT as u16 {}
 }
-
-/// Key Status
-const KEYINPUT: VolAddress<u16> = unsafe { VolAddress::new_unchecked(0x400_0130) };
-
-/// A "tribool" value helps us interpret the arrow pad.
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
-#[repr(i32)]
-#[allow(missing_docs)]
-pub enum TriBool {
-  Minus = -1,
-  Neutral = 0,
-  Plus = 1,
-}
-
-newtype! {
-  /// Records a particular key press combination.
-  #[derive(Debug, Copy, Clone, Default, PartialEq, Eq)]
-  KeyInput, u16
-}
-
-#[allow(missing_docs)]
-impl KeyInput {
-  register_bit!(A_BIT, u16, 1, a_pressed);
-  register_bit!(B_BIT, u16, 1 << 1, b_pressed);
-  register_bit!(SELECT_BIT, u16, 1 << 2, select_pressed);
-  register_bit!(START_BIT, u16, 1 << 3, start_pressed);
-  register_bit!(RIGHT_BIT, u16, 1 << 4, right_pressed);
-  register_bit!(LEFT_BIT, u16, 1 << 5, left_pressed);
-  register_bit!(UP_BIT, u16, 1 << 6, up_pressed);
-  register_bit!(DOWN_BIT, u16, 1 << 7, down_pressed);
-  register_bit!(R_BIT, u16, 1 << 8, r_pressed);
-  register_bit!(L_BIT, u16, 1 << 9, l_pressed);
-
-  /// Takes the difference between these keys and another set of keys.
-  pub fn difference(self, other: Self) -> Self {
-    KeyInput(self.0 ^ other.0)
-  }
-
-  /// Gives the arrow pad value as a tribool, with Plus being increased column
-  /// value (right).
-  pub fn column_direction(self) -> TriBool {
-    if self.right_pressed() {
-      TriBool::Plus
-    } else if self.left_pressed() {
-      TriBool::Minus
-    } else {
-      TriBool::Neutral
-    }
-  }
-
-  /// Gives the arrow pad value as a tribool, with Plus being increased row
-  /// value (down).
-  pub fn row_direction(self) -> TriBool {
-    if self.down_pressed() {
-      TriBool::Plus
-    } else if self.up_pressed() {
-      TriBool::Minus
-    } else {
-      TriBool::Neutral
-    }
-  }
-}
-
-/// Gets the current state of the keys
-pub fn key_input() -> KeyInput {
-  // Note(Lokathor): The 10 used bits are "low when pressed" style, but the 6
-  // unused bits are always low, so we XOR with this mask to get a result where
-  // the only active bits are currently pressed keys.
-  KeyInput(KEYINPUT.read() ^ 0b0000_0011_1111_1111)
-}

src/lib.rs

@@ -66,10 +66,9 @@ pub mod bios;
 pub mod core_extras;
 pub(crate) use crate::core_extras::*;
 
-pub mod io_registers;
+pub mod io;
 
 pub mod video_ram;
-pub(crate) use crate::video_ram::*;
 
 /// Performs unsigned divide and remainder, gives None if dividing by 0.
 pub fn divrem_u32(numer: u32, denom: u32) -> Option<(u32, u32)> {
@@ -127,12 +126,12 @@ fn divrem_u32_non_restoring(numer: u32, denom: u32) -> (u32, u32) {
     }
     i >>= 1;
   }
-  q = q - !q;
+  q -= !q;
   if r < 0 {
-    q = q - 1;
+    q -= 1;
-    r = r + d;
+    r += d;
   }
-  r = r >> 32;
+  r >>= 32;
   // TODO: remove this once we've done more checks here.
   debug_assert!(r >= 0);
   debug_assert!(r <= core::u32::MAX as i64);
@@ -168,18 +167,11 @@ pub unsafe fn divrem_i32_unchecked(numer: i32, denom: i32) -> (i32, i32) {
   } else {
     divrem_u32_non_restoring(unsigned_numer, unsigned_denom)
   };
-  if opposite_sign {
+  match (opposite_sign, numer < 0) {
-    if numer < 0 {
+    (true, true) => (-(udiv as i32), -(urem as i32)),
-      (-(udiv as i32), -(urem as i32))
+    (true, false) => (-(udiv as i32), urem as i32),
-    } else {
+    (false, true) => (udiv as i32, -(urem as i32)),
-      (-(udiv as i32), urem as i32)
+    (false, false) => (udiv as i32, urem as i32),
-    }
-  } else {
-    if numer < 0 {
-      (udiv as i32, -(urem as i32))
-    } else {
-      (udiv as i32, urem as i32)
-    }
   }
 }