From e1e9c61bc6725fe37853364b087a7ba64a234a75 Mon Sep 17 00:00:00 2001
From: Stephen Anthony <sa666666@gmail.com>
Date: Tue, 16 Jun 2020 14:44:44 -0230
Subject: [PATCH] Fix reading from TIA registers when D6 is not used.  Also fix
 randomization with 'tiadriven' to always fully randomize, with no
 relationship to what was on the data bus (fixes #664).

---
 Changes.txt             |  6 ++++-
 src/emucore/System.hxx  | 22 -----------------
 src/emucore/tia/TIA.cxx | 52 +++++++++++++++++++----------------------
 3 files changed, 29 insertions(+), 51 deletions(-)

diff --git a/Changes.txt b/Changes.txt
index 48c78a4b6..3e2c16150 100644
--- a/Changes.txt
+++ b/Changes.txt
@@ -23,7 +23,11 @@
 
   * Make NTSC custom phase shift not affect Yellow anymore.
 
-  * Allow changing custom palette and TV effects adjustables in 1% steps again.
+  * Allow changing custom palette and TV effects adjustables in 1% steps
+    again.
+
+  * Fixed incorrectly setting D6 bit on TIA reads in some cases.  Related
+    to this, improve 'tiadriven' option to randomize only D5..D0 bits.
 
   * Fixed custom palette and TV effects adjustable slider rounding issue.
 
diff --git a/src/emucore/System.hxx b/src/emucore/System.hxx
index dab87b28a..b0660fec7 100644
--- a/src/emucore/System.hxx
+++ b/src/emucore/System.hxx
@@ -168,28 +168,6 @@ class System : public Serializable
     */
     uInt8 getDataBusState() const { return myDataBusState; }
 
-    /**
-      Get the current state of the data bus in the system, taking into
-      account that certain bits are in Z-state (undriven).  In those
-      cases, the bits are floating, but will usually be the same as the
-      last data bus value (the 'usually' is emulated by randomly driving
-      certain bits high).
-
-      However, some CMOS EPROM chips always drive Z-state bits high.
-      This is emulated by hmask, which specifies to push a specific
-      Z-state bit high.
-
-      @param zmask  The bits which are in Z-state
-      @param hmask  The bits which should always be driven high
-      @return  The data bus state
-    */
-    uInt8 getDataBusState(uInt8 zmask, uInt8 hmask = 0x00) const
-    {
-      // For the pins that are floating, randomly decide which are high or low
-      // Otherwise, they're specifically driven high
-      return (myDataBusState | (randGenerator().next() | hmask)) & zmask;
-    }
-
     /**
       Get the byte at the specified address.  No masking of the
       address occurs before it's sent to the device mapped at
diff --git a/src/emucore/tia/TIA.cxx b/src/emucore/tia/TIA.cxx
index 656524eae..6d3461c4c 100644
--- a/src/emucore/tia/TIA.cxx
+++ b/src/emucore/tia/TIA.cxx
@@ -414,85 +414,81 @@ uInt8 TIA::peek(uInt16 address)
 {
   updateEmulation();
 
-  // If pins are undriven, we start with the last databus value
-  // Otherwise, there is some randomness injected into the mix
-  // In either case, we start out with D7 and D6 disabled (the only
-  // valid bits in a TIA read), and selectively enable them
-  uInt8 lastDataBusValue =
-    !myTIAPinsDriven ? mySystem->getDataBusState() : mySystem->getDataBusState(0xFF);
-
-  uInt8 result;
+  // Start with all bits disabled
+  // In some cases both D7 and D6 are used; in other cases only D7 is used
+  uInt8 result = 0b0000000;
 
   switch (address & 0x0F) {
     case CXM0P:
-      result = collCXM0P();
+      result = collCXM0P() & 0b11000000;
       break;
 
     case CXM1P:
-      result = collCXM1P();
+      result = collCXM1P() & 0b11000000;
       break;
 
     case CXP0FB:
-      result = collCXP0FB();
+      result = collCXP0FB() & 0b11000000;
       break;
 
     case CXP1FB:
-      result = collCXP1FB();
+      result = collCXP1FB() & 0b11000000;
       break;
 
     case CXM0FB:
-      result = collCXM0FB();
+      result = collCXM0FB() & 0b11000000;
       break;
 
     case CXM1FB:
-      result = collCXM1FB();
+      result = collCXM1FB() & 0b11000000;
       break;
 
     case CXPPMM:
-      result = collCXPPMM();
+      result = collCXPPMM() & 0b11000000;
       break;
 
     case CXBLPF:
-      result = collCXBLPF();
+      result = collCXBLPF() & 0b10000000;
       break;
 
     case INPT0:
       updatePaddle(0);
-      result = myPaddleReaders[0].inpt(myTimestamp) | (lastDataBusValue & 0x40);
+      result = myPaddleReaders[0].inpt(myTimestamp) & 0b10000000;
       break;
 
     case INPT1:
       updatePaddle(1);
-      result = myPaddleReaders[1].inpt(myTimestamp) | (lastDataBusValue & 0x40);
+      result = myPaddleReaders[1].inpt(myTimestamp) & 0b10000000;
       break;
 
     case INPT2:
       updatePaddle(2);
-      result = myPaddleReaders[2].inpt(myTimestamp) | (lastDataBusValue & 0x40);
+      result = myPaddleReaders[2].inpt(myTimestamp) & 0b10000000;
       break;
 
     case INPT3:
       updatePaddle(3);
-      result = myPaddleReaders[3].inpt(myTimestamp) | (lastDataBusValue & 0x40);
+      result = myPaddleReaders[3].inpt(myTimestamp) & 0b10000000;
       break;
 
     case INPT4:
-      result =
-        myInput0.inpt(!myConsole.leftController().read(Controller::DigitalPin::Six)) |
-        (lastDataBusValue & 0x40);
+      result = myInput0.inpt(!myConsole.leftController().read(Controller::DigitalPin::Six))
+          & 0b10000000;
       break;
 
     case INPT5:
-      result =
-        myInput1.inpt(!myConsole.rightController().read(Controller::DigitalPin::Six)) |
-        (lastDataBusValue & 0x40);
+      result = myInput1.inpt(!myConsole.rightController().read(Controller::DigitalPin::Six))
+          & 0b10000000;
       break;
 
     default:
-      result = 0;
+      break;
   }
 
-  return (result & 0xC0) | (lastDataBusValue & 0x3F);
+  // Bits D5 .. D0 are floating
+  // The options are either to use the last databus value, or use random data
+  return result | ((!myTIAPinsDriven ? mySystem->getDataBusState() :
+    mySystem->randGenerator().next()) & 0b00111111);
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -