schellingb / dosbox-pure

DOSBox Pure is a new fork of DOSBox built for RetroArch/Libretro aiming for simplicity and ease of use.
GNU General Public License v2.0
752 stars 62 forks source link

Loading a state #194

Open leiradel opened 2 years ago

leiradel commented 2 years ago

Hi,

I have a Libretro frontend that aims to provide debugging facilities to Libretro cores here. It can already run a number a cores, including DOSBox Pure.

I'm trying to implement states now and am facing issues with DOSBox Pure. When I try to load a state in my frontend, I get the following error:

[ERROR] Load State Error: Unable to load state made while DOS was not running.
If using rewind, make sure to modify the related core option.

The state was generated in RetroArch on Windows after the game had started, meaning I could already see it's screen rendered in RetroArch when I hit F1 to create the state.The save file contains a valid AUTOBOOT.DBP file so the core shouldn't be stuck in the Pure Menu:

$ unzip -l dott.save.zip 
Archive:  dott.save.zip
  Length      Date    Time    Name
---------  ---------- -----   ----
       17  2021-09-23 10:25   AUTOBOOT.DBP
        0  2021-09-23 10:25   DOTT.CD/
      231  2021-09-23 10:25   DOTT.CD/DOTT.INI
---------                     -------
      248                     3 files

Debugging my frontend on Linux, I can see that the error comes from this:

Thread 1 "cglibretro" hit Breakpoint 1, retro_unserialize (data=0x55555556b9ac <antstream::Perf::getTimeUsec()+80>, size=140737488345184) at dosbox_pure_libretro.cpp:3311
3311    {
(gdb) n
3312        dbp_overload_count = 0; // don't show overload warning immediately after loading
(gdb) 
3313        DBPArchiveReader ar(data, size);
(gdb) 
3314        bool res = retro_serialize_all(ar, true);
(gdb) s
retro_serialize_all (ar=..., unlock_thread=255) at dosbox_pure_libretro.cpp:3239
3239    {
(gdb) n
3240        if (dbp_serializemode == DBPSERIALIZE_DISABLED) return false;
(gdb) 
3241        DBP_LockThread(true);
(gdb) 
3242        DBPSerialize_All(ar, (dbp_state == DBPSTATE_RUNNING), dbp_game_running);
(gdb) s
DBPSerialize_All (ar=..., dos_running=255, game_running=96) at src/dbp_serialize.cpp:266
266 {
(gdb) n
272     ar.version = 3;
(gdb) p dos_running
$1 = false
(gdb) p game_running
$2 = true
(gdb) n
273     if (ar.mode != DBPArchive::MODE_ZERO)
(gdb) 
275         Bit32u magic = 0xD05B5747;
(gdb) 
276         Bit8u invalid_state = (dos_running ? 0 : 1) | (game_running ? 0 : 2);
(gdb) 
277         ar << magic << ar.version << invalid_state;
(gdb) 
278         if (magic != 0xD05B5747) { ar.had_error = DBPArchive::ERR_LAYOUT; return; }
(gdb) p magic
$3 = 3495647047
(gdb) n
279         if (ar.version < 1 || ar.version > 3) { DBP_ASSERT(false); ar.had_error = DBPArchive::ERR_VERSION; return; }
(gdb) p ar.version
$4 = 3 '\003'
(gdb) n
280         if (ar.mode == DBPArchive::MODE_LOAD || ar.mode == DBPArchive::MODE_SAVE)
(gdb) p ar.mode
$5 = 0 '\000'
(gdb) n
282             if (!dos_running  || (invalid_state & 1)) { ar.had_error = DBPArchive::ERR_DOSNOTRUNNING; return; }
(gdb) p !dos_running
$6 = true
(gdb) p invalid_state & 1
$7 = 0
(gdb) n
366 }

So it doesn't strike me as a corrupted state but the DOSBox Pure internal state being wrong.

How should I setup the core so it's ready to load a state? Right now the sequence of calls in my frontend is:

  1. Load the core's shared object and set up the pointers to the API functions
  2. Call retro_set_environment
  3. Call retro_init
  4. Call retro_get_system_info just to log the core information
  5. Call retro_load_game with the content data loaded from disk (this calls retro_get_system_info again as it needs to query need_fullpath)
  6. Set all the callbacks: video, audio, audio batch, input poll, and input state
  7. Call retro_get_system_av_info to initialize the audio and video components
  8. Here it may call retro_set_controller_port_device depending on the frontend configuration
  9. The frontend now runs one frame via retro_run, I currently do this to try and make sure DOSBox Pure is in the correct state to load the state file, other cores don't need this (and I'm not entirely sure DOSBox Pure needs it)
  10. Call retro_unserialize, which fails with the above trace

Thanks in advance!

leiradel commented 2 years ago

Oh, I'm using version 0.17.

schellingb commented 2 years ago

Hi there First and foremost, I quite like the idea of your frontend! Original DOSBox has some debug tools with a crude UI built in (although everything is disabled in DOSBox Pure builds). It might be fun to expose it to Hackable Console. If I find some time I'd certainly like to tinker with that :-)

Now that you mention it, the error message is wrong there. It shows that same message when the state was serialized while DOS wasn't running (or had a crash), BUT also when it's currently not (yet) running (or crashed).

I'm not sure if calling retro_run once is enough to have the AUTOBOOT start up the DOS process fully. Because as is in version 0.17 the emulation runs in its own thread so this is all a bit wonky. Maybe calling it a few more times would "fix" it?

Once #184 is completed things should be much smoother. The thread should go away and overall things should be a fair bit more deterministic. After that is done, I'd like to fix the state loading so it can load into a game process even when it hasn't been started.

Do you have a Win64 binary of the frontend I could have?

leiradel commented 2 years ago

Ah, calling retro_run 60 times before loading the state did the trick, thanks for the tip!

Maybe quick fix in the core would be to do a busy loop, waiting for the correct internal state to arrive, in retro_unserialize? Just thinking out loud here, as it would be better to be sure the state can be loaded than to run some frames ahead.

As for my frontend, thanks, it's fun to work with it. It's far from what I want it to be though. I don't have a Windows build as my machine runs Linux, but I believe it can be built with MSYS2. The code that loads the state is hammered in a place it shouldn't be, as it's just testing for now.

I'll leave this issue opened in case you think you can fix this before #184 , otherwise feel free to close it as the workaround solved my immediate issue.

leiradel commented 2 years ago

Listen, if you need me to test a fix for this, just shout. I can remove the workaround and try to reproduce the issue.