California Games not advancing past BMX event

[sa666666]

Bug has apparently been present since Stella 3.9.3, and is still present in master!

ROM: California Games (1987) (Epyx).zip
State file from 6.5.3 (submitted by bug reporter): California Games (1987) (Epyx)_state_6.5.3.zip
State file from master: California Games (1987) (Epyx)_state_6.7pre.zip

This was reported by PM to AtariAge; I will include the relevant info here.

---

When playing California Games in normal mode (meaning that you're progressing from completing one event then moving on to the next), the game will not go past the BMX downhill event once complete. It will just stay at the screen with the bike at the finish line and never move to the next screen. It should display the team with the high score, then move on to the Surfing event. I tried with other emulators and it behaves as expected, but when playing with Stella, it "hangs" after the BMX event. Is this a known issue?

This save state0 will put you right before the last screen on the BMX event. Once you cross the finish line, you won’t be able to continue. It’s supposed to show the event winner screen and then go to the surfing event when you press the button.

I also added a save state for the end of the footbag event (state1). You'll see that when you press the button, it behaves as it should for this event.

---

I tried in Stella 6.5.3, and confirmed it there. And also in latest master (as of Dec. 29), with the same results. I set emulation speed to 1000%, to get to the desired point very quickly.

Some oddities:

• The state save files provided for 6.5.3 are over 6MB each! They should only be ~18-20KB. But they still worked in 6.5.3. Perhaps there's a truncation bug or something in the state save mechanism. Maybe this should be split out as a separate issue.
• If I play around with the emulation speed, it starts working! So if I set the speed to, say, 25%, or 300%, pressing Space (aka, Fire button) allows the event to proceed. But if I set it back to 100%, it doesn't work again 😕

[thrust26]

Adding @DirtyHairy because the fact that it is working at other speeds than 100% makes me wondering.

[sa666666]

Yeah, that makes sense.

[thrust26]

Ideas:
The code (non intentionally?) also reads INPT4 via PLA multiple times during kernel display. I wonder if that affects Stella.
The code checks the fire button twice per frame, once before and once after the kernel. It checks if the buttons is pressed and released (or vice versa). The button press is only detected if the button is pressed when the 2nd check happens. Does Stella maybe update the button state not frequently enough? Somehow this reminds me at #689.

[thrust26]

Attached is a simple test program, which checks the fire button twice per frame just like CG does. The background color should change when the button is pressed or released. This does not happen in Stella but works for other emulators.

test_button.zip

[sa666666]

It doesn't work in MAME or z26 either, but does in Stellerator, gopher2600 and clksignal.

[thrust26]

Findings:

• my test ROM checks at scanlines 0 and 228 (CG at 3 and 221).
• on a real console this would mean, about 12.5% of state changes (between scanlines 229 and 262) would be missed
• when testing with gopher2600 and Stellerator, it also misses a small fraction of state changes
• after adapting my test ROM to CG check timings (V2), I found that gopher2600 changes behavior. Sometimes a lot of changes are missed consecutively (this does not happen with Stellerator)

test_button_v2.zip

Some thoughts:

• CG checks the fire button at the very beginning of a frame and after the display kernel
• the code expects an input state change somewhere in the middle of a frame
• in Stella all state changes happen between two frames
• if Stella would poll events while the EmulationWorker is running, CG would work. The 1st check would take the state from the previous poll and the 2nd from the current one (I suppose that's what the other, "good" emulators do)
• BUT: The faster the CPU and emulation, the less real time the EmulationWorker requires (theoretically zero). And then there will be no state changes possible during one frame.
• so this only works by coincidence and only with slow enough emulation (may that's why the "good" emulators are working?)

Ideas for solutions:

1. We could "slow down" emulation, e.g. by emulating only fractions of a frame each step, poll events, sleep and then continue with the next step. Each step should take about the same amount of time. For CG two steps per frame would be sufficient, and I cannot think of any realistic scenario which would require more.
2. We could poll events parallel to the EmulationWorker and cross our fingers, hoping that the first (INPT4) state check happens before we have polled new events. (tested, this works in debug mode on my machine)
3. We could move the beginning of a frame e.g. to the start of the display kernel. So the VBlank code reads the result from the current, and Overscan the result of the next poll.
4. ...?

[thrust26]

@DirtyHairy @sa666666 How shall we continue here?

[sa666666]

How about, in addition to the normal polling, we poll at each read of INPTx too? Would this cause too much slowdown, I wonder?

If there are no pending events, this is essentially a small check (check returns false, so method returns immediately).

[thrust26]

We are doing this for paddle and trackball games anyway, no?

[sa666666]

No, currently all controllers are querying myEvent, which is essentially a cached state of the events from the last time polling was done in EventHandler. What I am proposing is to directly call EventHandler::pollEvent at each INPTx read. If there are no pending events from SDL, then this is a simple check (while(SDL_PollEvent(&myEvent))) that exits immediately. Otherwise it will get the pending events and put them in myEvent, from which the current code can then read it and continue as it is now.

[thrust26]

Hm, paddles are read multiple times per frame. So here Stella must update multiple times per frame. We should do the same with joystick controllers.

[sa666666]

Events are only grabbed from SDL once per frame. If the paddles are checking multiple times per frame, they are checking a cached event stored in myEvent. They are not polling SDL directly multiple times per frame. The latter is what I'm proposing.

[thrust26]

Got you. Polling with each INPTx might become quite slow though. Maybe we should define a minimum interval between polls, e.g 1/2 frame.

[sa666666]

Well, first I would try it on every read, to see if it even fixes the issue. Then if that works, we can further optimize it.

[thrust26]

If we limit the change to INPT4/5, then we should be fine. Makes no sense for analog input anyway.

[thrust26]

@sa666666 Do you have an idea how to add this extra polling? Usually the controller code has no access to event handling.

[sa666666]

It's going to require some refactoring. New method(s) need to be added to EventHandler, and then the object needs to be accessible from the Controller class. I suggest whatever we do, get it test "quick and dirty" first, to see if it will even help. Then we can worry about doing it cleanly.

[thrust26]

Wouldn't a change in OSystem::mainLoop() be much less invasive and more versatile, too?

[sa666666]

I don't know. And I guess we need to experiment. I'm not even entirely sure my suggestion will fix the problem. I think this work needs to be done in a branch first, since it has the potential to be very invasive either way.

[DirtyHairy]

Be careful with this. Afaik accessing input is only legal on the main thread on some platforms, and emulation runs on thread.

[thrust26]

How about a separate thread which only polls the input multiple times per frame?

Have you seen SDL_AddTimer?

[thrust26]

Be careful with this. Afaik accessing input is only legal on the main thread on some platforms, and emulation runs on thread.

Two ideas here:

1. interrupt emulation when INPT4/5 are accessed, sleep*, poll and continue emulation
2. sleep* emulation when INPT4/5 are accessed, poll multiple times per frame in a separate thread

* sleep until current real time

[DirtyHairy]

If I understand (1) correctly (terminate timeslice prematurely -> return to main thread -> poll -> go for next timeslice), this should work.

I don't think (2) is viable; afaik polling has to happen on the main thread in order to be safe across platforms --- this is not just about race conditions, but about assumptions made by the API of windowing systems.

[thrust26]

If I understand (1) correctly (terminate timeslice prematurely -> return to main thread -> poll -> go for next timeslice), this should work.

Not quite. After polling, the current timeslice has to be continued before we go to the next one.

I don't think (2) is viable; afaik polling has to happen on the main thread in order to be safe across platforms --- this is not just about race conditions, but about assumptions made by the API of windowing systems.

Then it must be made possible in the main thread to continue running (and polling) while emulation is dispatched.

[thrust26]

@DirtyHairy @sa666666 How should we proceed here?

How would you interrupt emulation? I suppose this has to be triggered in TIA class? Since I still have problems to follow the code here, I suggest someone how knows the code, should do it.

[thrust26]

@DirtyHairy @sa666666 Bump. IMO we should fix this for 7.0.

[thrust26]

@DirtyHairy @sa666666 2nd Bump