cpu/system: Improve auto-joypad poll timing, modify state handling

Backport: bsnes-emu/bsnes#351 by Morilli

This commit breaks state compatibility with standalone bsnes. Previous
to this commit, the raw state data was still compatible, but would
require some manipulation on the user's part as the standalone creates
compressed states and encapsulates them in .bsz format. As standalone
bsnes also broke the state format for this change, it made sense to use
this opportunity to version the states differently in bsnes-jg. Effort
has been taken to allow loading states created in previous versions of
the emu, with the caveat that there may be some unexpected behaviour,
though this is unlikely in nearly all cases.

Author: carmiker

src/cpu.cpp

@@ -478,13 +478,21 @@ void CPU::writeCPU(unsigned addr, uint8_t data) {
     //bit 0 is shared between JOYSER0 and JOYSER1:
     //strobing $4016.d0 affects both controller port latches.
     //$4017 bit 0 writes are ignored.
-    controllerPort1.device->latch(data & 1);
-    controllerPort2.device->latch(data & 1);
+    status.cpuLatch = data & 1;
+    controllerPort1.device->latch(status.autoJoypadLatch | status.cpuLatch);
+    controllerPort2.device->latch(status.autoJoypadLatch | status.cpuLatch);
     return;
 
   case 0x4200:  //NMITIMEN
     io.autoJoypadPoll = data & 1;
-    if(!io.autoJoypadPoll) status.autoJoypadCounter = 33; // Disable auto-joypad read
+    if(status.autoJoypadCounter == 0) {
+      // allow controller latches during this time
+      status.autoJoypadLatch = io.autoJoypadPoll;
+      controllerPort1.device->latch(status.autoJoypadLatch | status.cpuLatch);
+      controllerPort2.device->latch(status.autoJoypadLatch | status.cpuLatch);
+    } else if (!io.autoJoypadPoll && status.autoJoypadCounter >= 2) {
+      status.autoJoypadCounter = 33;
+    }
     nmitimenUpdate(data);
     return;
 
@@ -820,47 +828,56 @@ void CPU::dmaEdge() {
 
 //called every 128 clocks from inside the CPU::stepOnce() function
 void CPU::joypadEdge() {
-  //it is not yet confirmed if polling can be stopped early and/or (re)started later
-  if(!io.autoJoypadPoll) return;
-
-  if(vcounter() == ppu.vdisp() && hcounter() >= 130 && hcounter() <= 256) {
+  if(vcounter() == ppu.vdisp() && (counter.cpu & 255) == 0 && hcounter() >= 130 && hcounter() <= 384) {
     //begin new polling sequence
     status.autoJoypadCounter = 0;
-  }
+  } else {
+    //stop after polling has been completed for this frame
+    if(status.autoJoypadCounter >= 33) return;
 
-  //stop after polling has been completed for this frame
-  if(status.autoJoypadCounter >= 33) return;
+    status.autoJoypadCounter++;
+  }
 
   if(status.autoJoypadCounter == 0) {
     //latch controller states on the first polling cycle
-    controllerPort1.device->latch(1);
-    controllerPort2.device->latch(1);
+    status.autoJoypadLatch = io.autoJoypadPoll;
+    controllerPort1.device->latch(status.autoJoypadLatch | status.cpuLatch);
+    controllerPort2.device->latch(status.autoJoypadLatch | status.cpuLatch);
+    if(io.autoJoypadPoll) {
+      //shift registers are cleared to zero at start of auto-joypad polling
+      io.joy1 = 0;
+      io.joy2 = 0;
+      io.joy3 = 0;
+      io.joy4 = 0;
+    }
   }
 
   if(status.autoJoypadCounter == 1) {
     //release latch and begin reading on the second cycle
-    controllerPort1.device->latch(0);
-    controllerPort2.device->latch(0);
-
-    //shift registers are cleared to zero at start of auto-joypad polling
-    io.joy1 = 0;
-    io.joy2 = 0;
-    io.joy3 = 0;
-    io.joy4 = 0;
+    status.autoJoypadLatch = 0;
+    controllerPort1.device->latch(status.autoJoypadLatch | status.cpuLatch);
+    controllerPort2.device->latch(status.autoJoypadLatch | status.cpuLatch);
   }
 
-  if(status.autoJoypadCounter >= 2 && !(status.autoJoypadCounter & 1)) {
-    //sixteen bits are shifted into joy{1-4}, one bit per 256 clocks
-    uint8_t port0 = controllerPort1.device->data();
-    uint8_t port1 = controllerPort2.device->data();
-
-    io.joy1 = io.joy1 << 1 | (port0 & 1);
-    io.joy2 = io.joy2 << 1 | (port1 & 1);
-    io.joy3 = io.joy3 << 1 | ((port0 & 2) >> 1);
-    io.joy4 = io.joy4 << 1 | ((port1 & 2) >> 1);
+  if(status.autoJoypadCounter != 1 && !io.autoJoypadPoll) {
+    // if auto-joypad polling is disabled at this point skip the rest of the polling
+    status.autoJoypadCounter = 33;
+    return;
   }
 
-  ++status.autoJoypadCounter;
+  if(status.autoJoypadCounter >= 2) {
+    //sixteen bits are shifted into joy{1-4}, one bit per 256 clocks
+    //the bits are read on one 128-clock cycle and written on the next
+    if ((status.autoJoypadCounter & 1) == 0) {
+      status.autoJoypadPort1 = controllerPort1.device->data();
+      status.autoJoypadPort2 = controllerPort2.device->data();
+    } else {
+      io.joy1 = io.joy1 << 1 | (status.autoJoypadPort1 & 1);
+      io.joy2 = io.joy2 << 1 | (status.autoJoypadPort2 & 1);
+      io.joy3 = io.joy3 << 1 | ((status.autoJoypadPort1 & 2) >> 1);
+      io.joy4 = io.joy4 << 1 | ((status.autoJoypadPort2 & 2) >> 1);
+    }
+  }
 }
 
 //nmiPoll() and irqPoll() are called once every four clock cycles;
@@ -1005,6 +1022,14 @@ void CPU::serialize(serializer& s) {
 
   s.integer(status.autoJoypadCounter);
 
+  if (s.version) {
+    s.integer(status.autoJoypadPort1);
+    s.integer(status.autoJoypadPort2);
+
+    s.boolean(status.cpuLatch);
+    s.boolean(status.autoJoypadLatch);
+  }
+
   s.integer(io.wramAddress);
 
   s.boolean(io.hirqEnable);

src/cpu.hpp

@@ -145,6 +145,12 @@ struct CPU : Processor::WDC65816, Thread, PPUcounter {
     bool hdmaMode = 0;  //0 = init, 1 = run
 
     unsigned autoJoypadCounter = 33;  //state machine; 4224 / 128 = 33 (inactive)
+
+    unsigned autoJoypadPort1 = 0;
+    unsigned autoJoypadPort2 = 0;
+
+    bool cpuLatch = 0;
+    bool autoJoypadLatch = 0;
   } status;
 
   struct IO {

src/serializer.hpp

@@ -36,6 +36,16 @@
 
 struct serializer;
 
+/* This value is used by bsnes-jg as a state version number, since the state
+   format broke during the 2.0.2 development cycle. The number is a hex
+   representation of the bsnes-jg version when the state format last changed,
+   where each byte represents the patch, minor, and major version from the
+   least to most significant byte. The previously used "signature" in the state
+   is used to determine whether the state was created previous to the first
+   post-fork state format break.
+*/
+static const unsigned serializerVersion = 0x00020002;
+
 template<typename T>
 struct has_serialize {
   template<typename C> static char test(decltype(std::declval<C>().serialize(std::declval<serializer&>()))*);
@@ -46,6 +56,8 @@ struct has_serialize {
 struct serializer {
   enum Mode : unsigned { Load, Save, Size };
 
+  unsigned version = serializerVersion;
+
   Mode mode() const;
   const uint8_t* data() const;
   unsigned size() const;

src/system.cpp

@@ -49,6 +49,7 @@
 #include "dsp.hpp"
 #include "emulator.hpp"
 #include "expansion/expansion.hpp"
+#include "logger.hpp"
 #include "random.hpp"
 #include "settings.hpp"
 #include "sufamiturbo.hpp"
@@ -66,11 +67,13 @@ serializer System::serialize(bool synchronize) {
   if(!information.serializeSize[synchronize]) return {};  //should never occur
   if(synchronize) runToSave();
 
-  unsigned signature = 0x31545342;
+  unsigned signature = serializerVersion; // previously 0x31545342
   unsigned serializeSize = information.serializeSize[synchronize];
   char version[16] = {};
   char description[512] = {};
   bool placeholder = false;
+
+  // Note: This is here for legacy compatibility only. This version is forever frozen at 115
   std::memcpy(&version, (const char*)SerializerVersion.c_str(), SerializerVersion.size());
 
   serializer s(serializeSize);
@@ -99,10 +102,19 @@ bool System::unserialize(serializer& s) {
   s.boolean(synchronize);
   s.boolean(placeholder);
 
-  if((signature != 0x31545342)
-      || (serializeSize != information.serializeSize[synchronize])
-      || (std::string{version} != SerializerVersion))
+  if (signature == 0x31545342) {
+    /* When the state format changed during the 2.0.2 dev cycle, the size
+       increased by 10. Do update this whenever the state format changes!!
+    */
+    if ((serializeSize + 10) != information.serializeSize[synchronize]
+       || (std::string{version} != SerializerVersion))
+      return false;
+    logger.log(Logger::WRN, "Loading deprecated state version, success not guaranteed.\n");
+    s.version = 0;
+  }
+  else if ((signature != serializerVersion) || (serializeSize != information.serializeSize[synchronize])) {
     return false;
+  }
 
   if(synchronize) power(/* reset = */ false);
   serializeAll(s, synchronize);