BSNES 115: Input polling event should happen when controller is latched

[alyosha-tas]

Currently the input poll callback happens along with snes_run, but it should be when input is latched instead. I don't know where in the code this happens.

[Morilli]

On that core there are currently two calls; the input_poll call, which checks the controller state and saves it, and an input_state call which retrieves this information. The input_poll call is currently executed just before a frame gets emulated in the actual core. If I'm understanding you correctly, the current input_poll call should be removed and put in place of the current input_state call? Does this accomplish something important, or would this merely be helpful for trying to input accurate real-time inputs?

[alyosha-tas]

The input_state part is fine, it's just that the input_poll part should only happen on actual latches from the core.

This avoids situations where the console has latched inputs on one frame, but does not read them until the next frame (the old inputs should still be there.)

It is also important for console verification, where we need to match latches. The oninputpoll event is used for this (which is part of the InputCallbacks in IInputPollable with the ??? comment.) That part is just a 3 line copy-paste from NESHawk, but I need the poll events to happen at the right time first.

I don't know where in BSNES this happens though, presumably somewhere it has some code like: if (reg_4200.Bit(0)) then latch_controllers. And similarly for reg_4016. Even if i found where, I don't know how to build the core.

[Morilli]

The part where a latch is performed in bsnes is at line 135 of io.cpp (writing to 0x4016). There is also code here (on reading from 4016/4017) which seems to retrieve stored controller data but doesn't (necessarily? see this poll call) perform any input poll calls.

I believe bsnes's current logic exists to not poll the underlying (real) hardware device too often (there's an article from near about the conceptual design) which I believe I have completely ignored when implementing input polling for BizHawk.

If I'm understanding things correctly right now, the best path would be

1. to either implement full just-in-time polling or do the timing-based approach if possible (it does clash a bit with bizhawk's current workings, so it's probably not too helpful anyway)
2. on every latch, tell the frontend somehow (most likely via the inputpoll function) that a latch call was done in the core

What confuses me a bit is that apparently reads trigger a data call which does not poll inputs, and writes do poll input. Not sure what to make of this.

[alyosha-tas]

Thanks for the info, it looks like the other latch (automatic one) happens in cpu/timing.cpp. I'm also a bit confused by the extra possible poll in the read you pointed out, I'll look at it a bit more carefully later.

It is correct that reads shouldn't poll the input. The poll (either automatic or write triggered by 1->0 transition when writing to 4016) latches the state of the buttons . The button state polled will persist even if the results aren't read ( and even if the state of the physical buttons changes)

[Morilli]

So just to confirm for now, the logic inside bsnes core is correct, and the only thing that needs to change is that instead of having the actual input state that will be read get set via snes_input_poll before the emulator->run() function, it should instead be updated on every inputPoll call but not be able to change inbetween them (multiple reads should always return the same input state, until another inputPoll is performed)?

[alyosha-tas]

Yup, that's correct.

[Morilli]

Okay, have pushed the changes to the bsnes_input_poll branch, if you wanna test that out.

Note that I don't really know how bizhawk handles actual hardware input, so inputs appearing in bizhawk may still be incorrect and updated less than they should or whatever, but at least the actual snes_input_poll calls should be correct now and (theoretically) be polling updated input states.

Let me know whether there's anything else you need with this or there is something wrong about it.

[Morilli]

Looking at the inputcallbacksystem, is there a chance that that one should be called inside the snes_input_poll function, which doesn't happen at all at the moment (and not in the old bsnes either)?

[CasualPokePlayer]

The input callback system is intended to called the same time an input poll happens, so yes it would be inside the snes_input_poll. There's already API for checking if the frame is lag so non-midframe input callbacks are practically useless for the input callback system.

[alyosha-tas]

That extensive change isn't really what I had in mind, (maybe there is some confusion because the word 'poll' is used in so many different places.)

Basically the only change that needs to happen is snesCallbacks.snes_input_poll(); being removed from snes_run and if (port == 0) snesCallbacks.snes_input_poll(); added right above this line for the gamepad:

https://github.com/TASEmulators/BizHawk/blob/bb226f694aa9a3e2322c03dbdeb07f93e0774204/waterbox/bsnescore/bsnes/sfc/controller/gamepad/gamepad.cpp#L36

and right above here for the mouse:

https://github.com/TASEmulators/BizHawk/blob/bb226f694aa9a3e2322c03dbdeb07f93e0774204/waterbox/bsnescore/bsnes/sfc/controller/mouse/mouse.cpp#L67

(and similarly for other controllers but I don't see them selectable in the controller menu.)

Everything else works correctly as is.

[Morilli]

This "extensive change" was basically just removing the snes_input_state call and replacing it with snes_input_poll calls, which had a bit of logic implications due to a different function signature, but should generally not be a real change.

What I'm seeing is that apparently you want to get one notification/input poll call for every latch, and not see the snes_input_poll function get called for each button/whatever that bsnes wants (having it called two times, once per port, is still fine, yes?)

Seeing as how having the state cached in the C# code doesn't seem to have any usefulness (I checked and fps were exactly the same, even though on the branch there are probably more calls, also bsnes itself stores the button state on inputPoll anyway), I would opt to leaving it removed and making a separate callback in the lines you mentioned (inside the latch function) that basically just tells the C#-side that a latch was performed and should be unambiguous in terms of what it does.

similarly for other controllers but I don't see them selectable in the controller menu.

That's because only the mouse and gamepad are selectable in the first port, and everything is selectable in the second port. I'm following bsnes's specification there, even though I'm not 100% sure whether that is accurate to real hardware (I would assume so, because I don't see another reason for this specification; every Device seems to work in both ports).

Lastly, I'm wondering how everything else works correctly, or more specifically, whether the InputCallbackSystem is what you use. As far as I'm seeing, its Call function would need to be called on every latch from the core to get the callbacks to fire, and it currently isn't at all. How else are you seeing the oninputpoll callback fire?

[alyosha-tas]

Each latch effects both controllers at the same time , so the callback only needs to happen once, hence the if (port==0) part.

Yes a separate callback as you describe would work just fine.

In my local code I put in the input callback on the C# side because I was trying to dump some resynced tases to verify them. Once I realized the callback was happening every frame I opened this issue. But that's only a couple lines that can be put in at any time.

[Morilli]

Hm, I'm a bit confused right now by which which functions should call an input latch callback and which shouldn't.

For example, in gamepad.cpp, this line seems to guard against updating the controller state when latched goes from 0->1.

However, such a check does not exist in the mouse code. Does this mean that whether latches are performed depends on which hardware device is connected?

Also, there is this port2 latch call which also seems to update the controller state, but only for the second port device? Ideally I would be able to put one callback call before each of the latch calls, but I'm not so sure that would be entirely correct regarding the above points...

[alyosha-tas]

Good point, I'm not sure what's going on with the mouse. It seems like the same guard should be there as the gamepad (it is correct that the latch should only occur on the 1->0 transition and not 0->1.) Maybe it's just an oversight? Documentation I could find seems to indicate so: https://www.repairfaq.org/REPAIR/F_SNES.html

That other poll looks like it's goal is just to get the lightgun position latch() instead of latch(bool). Gamepad for example doesn't implement just latch(). In NESHawk this is an entirely different callback altogether (again specifically for the lightgun) so maybe it should have it's own path here too, it would simplify the situation a bit at least.

[Morilli]

The mouse thing may be a thing worth reporting upstream.

Aside from that, let me know how https://github.com/TASEmulators/BizHawk/commit/a9adc1319b9e8925a52f7e06fadccdb92854d0d6 looks, especially now that the InputCallbackSystem is actually called.

I've ignored the latch() calls for now and just handled the latch(bool) calls, but that shouldn't be an issue I believe.

[alyosha-tas]

Looks like its working correctly except that the callback happens 4 times per frame when it should only be once.

Haven't tracked down why yet. everything looks good to me.

EDIT: looks like the callbacks need to be created in the correct order in BsnesApi (currently no_lag and controller_latch are swapped.)

[Morilli]

Dumb oversight on my part. Should be correct now.

[alyosha-tas]

It works!

Awesome, now I can do more SNES console verification testing.

Thanks for your help!

I think this can be PR'd now.