Improve auto-joypad polling timing

[Morilli]

This fixes #322.

In addition, these changes improve results in a number of test roms from #322 (comment).

Most of these test roms aren't "good or bad" and instead show trends and general behavior. In some cases it's clear when the emulator behavior is wrong and in other cases it's not so clear. The displayed values are not fixed and change frame-by-frame. I think it is most important to roughly match the hardware result. In some cases there may not even be one true correct result as even different hardware differs in results.

That being said, on current master the following tests are clearly bad:

01 auto-joy-timing-test
05 blip-autojoy-test-automatic
06 enable-autojoy-late-test-2
07 blip-autojoy-test
08 joypad-latch-set-during-autojoy
09 joypad-latch-clear-during-autojoy
10 joyser0-read-during-autojoy
12 blip-autojoy-timing-test
13 blip-autojoy-latches-joypad-test
14 clear-autojoy-after-autojoy-active

With the changes in this PR, all of these tests seem "fixed", at least mostly.

Notable remaining differences with hardware:

• Test 04 now has more FFFF in the column on the right. Should be 3-6 from the top, is 4-8.
• The right-most column in test 06 is a solid block of FFFF and does not flicker with occasional 0000 (it does on hardware).
• When holding B in test 08, the fourth row under JOY 1 will never show FFFF, it shows BFFF at most.
• Similar to above, in test 09 when holding B the third row under JOY 1 can never show C000, only E000 and F000.
• When holding a key like Y in test 10, the left-most column under JOY 1 may show 0s, but the video from hardware only shows 4s and 8s.

These are mainly minor off-by-1 problems, and roughly half of these also show in Mesen. I think the current results are good enough for now.

[Screwtapello]

If we change what's being serialised, we should also bump the serialisation version.

[Morilli]

You're right, and that's one thing I was contemplating while making the serialization and struct changes. But I looked at the last time serialization was changed (34af95d) and the version wasn't bumped after that either. Should I just bump it to something like 115.1?

[Screwtapello]

I think the previous idea was that the serialisation version only needed to be changed once per release, so if it had been changed since the previous release there was no need to change it again. Given releases are a lot further apart now, it seems prudent to change the number more regularly.

115.1 seems like a good choice, yeah.

[Screwtapello]

Why is the status.autoJoypadCounter != 1 condition here? If the counter is 0 or 2-33, and auto-polling is disabled, we can skip the rest of the process, but if the counter is 1, we just wait until the counter becomes 2 and then disable auto-polling? If a game enables auto-polling after the counter ticks 1, and we've skipped the latching inputs and clearing the shift registers, does the SNES start shifting a new copy of the previously-latched inputs into the shift registers? Does this not happen if the game enables auto-polling on ticks 2 or later?

[Morilli]

You're right that this seems peculiar but it does appear to be accurate. Without this condition the tests 12-blip-autojoy-timing-test and 13-blip-autojoy-latches-joypad-test no longer match hardware results.

The implication of this condition is that disabling auto-joypad polling during this one cycle has no effect, so it could for example be toggled off and toggled on again without affecting the auto-joypad polling sequence at all during this specific timing window.

As far as I understand that's precisely what the blip-autojoy-timing-test intends to test: It waits until HVBJOY reports auto-joypad polling to be active, disables auto-joypad polling per NMITIMEN write, waits a certain amount of cpu cycles and then re-enables it. Of course this depends on HVBJOY and NMITIMEN behavior, but changing those as well causes other tests to break which leads me to believe that this combination is most accurate.

[undisbeliever]

Fiskbit and I noticed that temporary disabling (NMITIMEN = 0) then quickly re-enabling auto-joy (NMITIMEN = 1) would not stop automatic joypad reading.

12-blip-autojoy-timing-test was used to determine if is a 128-clock or 256-clock window from when HVBJOY reports auto-joypad active. My results indicate it is a 256-clock window. If the enable auto-joy NMITIMEN write is > 256-clocks after auto-joypad starts then automatic joypad read stops.

Quoting my notes from that day:

It's matches what I expected for the 3chip. I didn't know if it was a 128 clock or 256 clock before autojoy could be re-enabled.
If my math is right, there's N + 6 fastROM cycles (48 mcycles) between HVBJOY reports auto-joy active and the NMITIMEN=1 write.

• 206 delay = 254 m-cycles
• 208 delay = 256 m-cycles.

For the 13-blip-autojoy-latches-joypad-test, Fiskbit asked for a test that checks if the S-CPU latches the controller twice if the NMITIMEN auto-joypad enable flag is blipped (disabled then quickly enabled). Details about the test can be found in the test's source code.

[Screwtapello]

I'm sorry it's taken me so long to review this PR; it wasn't just a matter of finding time, but finding large blocks of time so that I could give it the attention it deserved.

As best I can understand it, this patch is introducing these changes:

Previously, "sixteen bits are shifted into joy{1-4}, one bit per 256 clocks", so every 256 clocks we'd read a bit and shift it in immediately. However, what seems to happen in hardware is that we read one bit, then 128 clocks later we shift it in, then 128 clocks later we read the next bit. So we introduce autoJoypadPort1 and autoJoypadPort2 variables to store the bits between reading them from the controller, and shifting them into the register, and modify the joypadEdge() logic to use them.

I'm a little surprised that any software would notice the difference, but this seems much more like the sort of thing hardware would do.

Previously, automatic joypad polling was controlled by the autoJoypadPoll ("has the game asked for auto-polling") and autoJoypadCounter ("how far are we through the polling process") variables. There's 16 bits to read, each bit takes two 128-clock phases (read and shift, see above) plus the first two clock phases are used to open and close the latch. That's (16 * 2) + 2 = 34 steps, numbered 0-33, so when autoJoypadCounter finishes step 33 we're done. Now, we've added two new variables: cpuLatch and autoJoypadLatch. cpuLatch is set when the game manually tries to latch the controller ports by setting bit 0 of $4016, and autoJoypadLatch is set when the auto-joypad-polling hardware is trying to latch the controller ports. All the code that used to forcibly latch or unlatch the controller ports has been changed to set the variable that represents the cause of latching, and use status.autoJoypadLatch | status.cpuLatch as the latch expression, so the latch is open when either system (manual or automatic) opens it, and closed when both close it.

That makes me wonder what happens if the CPU keeps the latch open while the auto-poller is trying to read all the buttons. Fullsnes says "Be sure that Out0 in Port 4016h is zero (otherwise the shift register gets stuck on the first bit, ie. all 16bit will be equal to the B-button state." and I think bsnes implements that.

Again, this makes much more sense as a possible hardware implementation than what bsnes previously had.

It looks like the joypadEdge() function has been substantially reworked. It's called every 128 cycles, but previously it would immediately return if auto-polling were disabled, and autoJoypadCounter would never be clocked. Now, it always tries to start the polling process whenever it can (setting autoJoypadCounter to 0 at the appropriate time, trying to open and close the latch, etc.) and clocks the counter. If auto-polling is disabled by the time we get past the latch interactions, we skip the rest of the auto-polling and re-enabling it won't do anything until the next frame. This sounds at least plausible for a hardware implementation, but I don't know.

Previously, when the game wrote to $4200, it would set autoJoypadPoll appropriately (to be checked later). If the game disabled auto-polling, we'd set autoJoypadCounter to 33, ending the sequence regardless of where it currently was. In particular, if auto-polling was disabled while the auto-polling process had opened the latches, they would not be properly closed. In the new version of the code, if auto-polling is enabled (or disabled) at the very start of the auto-poll period (after the auto-poller decided whether to open the latches), we try again to open (or close) the latches, so they'll be in the right state by the time autoJoypadCounter reaches 1. When the counter reaches 1 we do nothing (the latches will be closed if they can be, either way), and if auto-polling is disabled after that, the auto-polling process is complete for the rest of the frame.

Is that more or less correct?

[Screwtapello]

For completeness, checking the list of games in #322:

• ✅ Nuke (PD) works, as well as it ever did
• ✅ The sky looks fine in Secret of Mana
• ✅ In Spellcraft, I can hold B at the main menu to automatically select "New Game"
• ✅ Again in Spellcraft, I can activate the password orb without automatically dismissing it
• ✅ In Super Conflict, no random inputs on the main menu, or the level select, or in-game
• ✅ In Super Star Wars, the game does not automatically unpause itself
• ✅ In Taikyoku Igo - Goliath, the title-screen animation is not skipped, and I can enter the game
• ✅ In Tatakae Genshijin 2, the attract-mode does not immediately exit
• ✅ In Williams Arcade's Greatest Hits, no menu items are skipped
• ✅ In Wolverine - Adamantium Rage, the game gets to the main menu
• ✅ In World Masters Golf, the D-pad does not auto-repeat on the main menu

...and the 01-auto-joy-timing-test gives:

Event	Min	Max
Start	2A	71
End	4B	9C

...which is extremely close to the real-hardware result.

If my understanding in the previous comment is correct, I'm happy to merge this.

[Morilli]

Is that more or less correct?

Yes, that seems like a good description of the changes. I realize I could have explained more in the PR description, but I'm glad it was understandable regardless. Just a couple notes:

what seems to happen in hardware is that we read one bit, then 128 clocks later we shift it in, then 128 clocks later we read the next bit. [...] I'm a little surprised that any software would notice the difference

It's possible this is coincidental due to the fact that the game spams HVBJOY polls and auto-joypad poll result reads. Perhaps someone engineered the mechanism in this way so it would always work on hardware or it was sheer luck.
The way the code seems to work is that HVBJOY is polled, and if it says auto-joypad polling isn't active, 0x4218, 0x4219 and 0x421A reads will be performed.

However there is a delay between checking the auto-joypad polling state and the actual read (TOCTOU problem). In rare cases, the last HVBJOY read will return auto-joypad polling to be inactive, but the next io.joy read isn't until after 3 auto-joypad polling cycles later, at which point autoJoypadCounter is 2. If io.joy was already written to in this cycle the game would read incorrect data and misbehave.

Here's an excerpt of a printf-log I created showcasing the problematic sequence of events:

[...] (hundreds of HVBJOY polls and io.joy reads)
HVBJOY | counter: 33 (reported inactive)
reading io.joy2
autoJoypad: 0
latched active and cleared io.joy
autoJoypad: 1
latched inactive
autoJoypad: 2
read data
reading io.joy1
autoJoypad: 3
wrote data
[...]
HVBJOY | counter: 9 (reported active)

If auto-polling is disabled by the time we get past the latch interactions, we skip the rest of the auto-polling and re-enabling it won't do anything until the next frame.

This is true, but this is also true for the very start of auto-joypad polling (status.autoJoypadCounter == 0). If the sequence was just started and io.autoJoypadPoll is false then the controller latch will be closed (if it isn't already) and status.autoJoypadCounter will be set to 33 immediately.

[Screwtapello]

Thank you to undisbeliever for doing the research, and to Morilli for figuring out how to update bsnes to match!

[Max833]

Looks like Zenkoku Koukou Soccer 2 has issues (ares-emulator/ares#970), but that's good progress. thanks for everyone who was involved!

[Morilli]

Looks like Zenkoku Koukou Soccer 2 still has issues (ares-emulator/ares#970)

That's the corresponding issue in ares, not bsnes. This problem is fixed in bsnes but someone would have to port the changes to ares for it to be fixed there as well.

[Max833]

I've tested this using the latest bsnes nightly version, that's why I mentioned it. ;) It's buggy in bsnes now. Occasionally, it ignores the pause command and just skips the screen.

[Max833]

However, we would probably have to test how the game behaves on original hardware, because:

[Morilli]

Occasionally, it ignores the pause command and just skips the screen.

Interesting, I can reproduce that. I expected it to behave similar to Spellcraft where holding down the B button would eventually dismiss the menu, but here it seems that the game never unpauses from holding the start button if it entered the pause screen properly, but sometimes the pause screen will be immediately dismissed. While I can't reproduce this in snes9x, the same thing happens in mesen. I would definitely be interested in seeing someone test this on hardware to verify what's actually "correct" here.

[Screwtapello]

I would definitely be interested in seeing someone test this on hardware to verify what's actually "correct" here.

I got a friend with a "real" 2/1/3 Super Famicom to test this game... and apparently both behaviours are correct. Sometimes when you press the Start button, it pauses and unpauses instantly, other times it pauses and stays paused. You can literally sit there tapping the Start button and sometimes it'll unpause and sometimes it won't - it's not just uninitialised memory or an accident of clock phase.

I guess there's some more nuance to how often the auto-polling happens, or how fast it works, that we haven't yet captured.

[Screwtapello]

OK, I tested this game in the latest bsnes nightly myself, and once I figured out how to get in-game, I repeatedly held the Start button for about a second, and recorded how many times the game stayed paused ("solid"), and how many times it immediately unpaused ("flaky"):

• total presses: 50
• solid pauses: 36
• flaky pauses: 14

My friend with the real SNES reports that it behaved very similarly.