From 2ade899fc6641b0aef80cef28e9c17164a5bc71a Mon Sep 17 00:00:00 2001
From: Nick Krichevsky <nick@ollien.com>
Date: Mon, 1 May 2023 00:12:17 -0400
Subject: [PATCH] Fix 0xF8 instruction flags

---
 src/cpu/run/arithutil.rs | 40 ++++++++++++++++++++++++++++------------
 src/cpu/run/load16.rs    |  1 -
 tests/cpu/load16.rs      |  6 +++---
 3 files changed, 31 insertions(+), 16 deletions(-)

diff --git a/src/cpu/run/arithutil.rs b/src/cpu/run/arithutil.rs
index 30e71be..6451d2f 100644
--- a/src/cpu/run/arithutil.rs
+++ b/src/cpu/run/arithutil.rs
@@ -83,15 +83,23 @@ impl CarryingAdd<u8, u16> for u16 {
 
 impl CarryingAdd<i8, u16> for u16 {
     fn add_with_carry(self, rhs: i8) -> (u16, bool, bool) {
-        if rhs >= 0 {
-            return self.add_with_carry(rhs.unsigned_abs());
-        }
-
-        let (total, carry) = self.overflowing_sub(rhs.unsigned_abs().into());
-        // This is a special case of half carry. Though we could probably twiddle the bits to make one
-        // unified half carry check, I'm gonna trust the guy who has debugged a real gameboy.
-        // https://stackoverflow.com/a/7261149/1103734
-        let half_carry = total & 0xF <= self & 0xF;
+        let [upper_8_bits, lower_8_bits] = self.to_be_bytes();
+        let operand = twos_comp_abs(rhs);
+        let (lower_8_bit_total, half_carry, carry) = operand.add_with_carry(lower_8_bits);
+        // Perform the actual "carry" by hand.
+        //
+        // This comes with heavy inspiration from https://www.reddit.com/r/EmuDev/comments/y51i1c/comment/isiizl4/?utm_source=reddit&utm_medium=web2x&context=3
+        // with some tweaks to handle overflow
+        let carry_adjustment = if !carry && rhs < 0 {
+            // Subtract from the 8th bit
+            !(1_u16 << 8)+1
+        } else if carry && rhs > 0 {
+            // Add to the 8th bit
+            1_u16 << 8
+        } else {
+            0
+        };
+        let (total, _final_carry) = u16::from_be_bytes([upper_8_bits, lower_8_bit_total]).overflowing_add(carry_adjustment);
 
         (total, half_carry, carry)
     }
@@ -151,6 +159,14 @@ fn did_8bit_full_carry_including_carry_bit<L: Into<u16>, R: Into<u16>>(
     ((lhs.into() & 0xff) + (rhs.into() & 0xff) + u16::from(carry_bit)) > 0xff
 }
 
+fn twos_comp_abs(n: i8) -> u8 {
+    if n >= 0 {
+        n.unsigned_abs()
+    } else {
+        (!n.unsigned_abs()) + 1
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -195,9 +211,9 @@ mod tests {
         //    ...:         if ((sp - n) & 0xF) <= (sp & 0xF):
         //    ...:             c.append((sp, n))
         #[test_case(0x1D21, -0x46, 0x1CDB, false, false; "negative; no carry")]
-        #[test_case(0x000F, -0x10, 0xFFFF, true, true; "negative; underflow")]
-        #[test_case(0x000F, -0x7F, 0xFF90, true, true; "negative; underflow 2")]
-        #[test_case(0x249D, -0x08, 0x2495, true, false; "negative; half carry")]
+        #[test_case(0x000F, -0x10, 0xFFFF, false, false; "negative; underflow no carry")]
+        #[test_case(0x000F, -0x7F, 0xFF90, true, false; "negative; underflow 2")]
+        #[test_case(0x249D, -0x08, 0x2495, true, true; "negative; both carries")]
         fn test_i8_u16_carrying_add(a: u16, b: i8, expected: u16, half_carry: bool, full_carry: bool) {
             assert_eq!((expected, half_carry, full_carry), a.add_with_carry(b));
         }
diff --git a/src/cpu/run/load16.rs b/src/cpu/run/load16.rs
index c4797fd..c7f1d01 100644
--- a/src/cpu/run/load16.rs
+++ b/src/cpu/run/load16.rs
@@ -110,7 +110,6 @@ impl InstructionRunner<SixteenBitLoadInstruction> for SixteenBitLoadRunner {
                 ];
 
                 let popped_value = u16::from_le_bytes(popped_bytes);
-                println!("{popped_value:x}");
 
                 processor.registers.set_combined_register(dst, popped_value);
                 processor.registers.set_16bit_register(
diff --git a/tests/cpu/load16.rs b/tests/cpu/load16.rs
index 3648f68..e8b7a52 100644
--- a/tests/cpu/load16.rs
+++ b/tests/cpu/load16.rs
@@ -125,9 +125,9 @@ fn test_load_effective_address(start_sp: u16, value: i8, expected_sp: u16) {
 #[test_case(0x00C0, 0x7f, 0, 1; "overflow 2")]
 #[test_case(0x00FF, 0x01, 1, 1; "overflow with half carry")]
 #[test_case(0x000A, 0x0C, 1, 0; "half carry")]
-#[test_case(0x0018, -0x4, 1, 0; "negative half carry")]
-#[test_case(0x000F, -0x10, 1, 1; "underflow with half carry")]
-#[test_case(0x000E, -0x0F, 0, 1; "underflow with no half carry")]
+#[test_case(0x0018, -0x4, 1, 1; "negative both carry")]
+#[test_case(0x000F, -0x01, 1, 1; "underflow with half carry")]
+#[test_case(0x00FE, -0x0F, 0, 1; "underflow with no half carry")]
 #[test_case(0xFFFF, 0xA, 1, 1; "16 bit overflow results in both carries")]
 fn test_load_effective_address_flags(starting_sp: u16, add_value: i8, half_carry: u8, carry: u8) {
     let mut processor = Processor::default();