libretro / bsnes2014

Libretro fork of bsnes. As close to upstream as possible.
GNU General Public License v3.0
9 stars 17 forks source link

Lagfix savestate compatibility #17

Closed Alcaro closed 7 years ago

Alcaro commented 7 years ago

I don't really like how savestates are incompatible between lagfix and not-lagfix (that frame_event_performed flag).

Since the serialization code is only be called when within one of those scheduler.exit(Scheduler::ExitReason::FrameEvent) calls, and each of them (other than non-SNES cores, which aren't used by this repo anyways) is followed by setting that flag, I suspect we can replace s.integer(status.frame_event_performed) with status.frame_event_performed=false. @Brunnis, do you agree?

For reference, here's how the lagfix looks after fixing it about ten times: https://github.com/libretro/bsnes-libretro/compare/60398479ee46ad61a7dc1515700f65b0653e589b...libretro

Brunnis commented 7 years ago

I'm not at home right now (not until wednesday), but after a quick look I agree with this. There doesn't currently seem to be any reason to serialize this flag. I vote we make the proposed change (and for bsnes-mercury as well).

Alcaro commented 7 years ago

I just realized that this change is actually not safe.

While it's safe to discard that flag on entry to retro_serialize, the first thing that one does is call System::runtosave(), which executes all threads - including the one setting the frame event flag.

What we could do is always put that flag in savestates and OR in 0x80000000 in the version field, but I'm not sure if that's worth it.

Alcaro commented 7 years ago

Closing per #19 / a839eab.