ci.persist() does not respect to removing files from fs. Save/Load functionality fails to Load in some cases

alorello commented 2 years ago

From an unpacked v7.3.9.zip and run with the attached bundle.jsdos ("Hexen" packaged using doszone studio).

Problem

Loading from indexedDB does nothing when using the following code in index.html:

Dos(document.getElementById("jsdos-root")).run("bundle.jsdos");

nor does it work when I attempt to use the second argument to dos.run():

Dos(document.getElementById("jsdos-root"))
                  .run("bundle.jsdos",
                       URL.createObjectURL(new Blob(<<bufferFromMyIndexedDB>>)));

No errors or warnings in the dev console that I noticed.

Steps to recreate

Load from an in-game save file (there are save files present in original bundle)
Do something in the game and save (in game)
Click Save UI button to call ci.persist()
Close browser tab and reopen to page

Things tried

I used the dev console application storage feature to verify the Save UI does in fact store an ArrayBuffer to js-dos-cache (ui-emulated-saves) under files store with key bundle.jsdos.changes. I downloaded the ArrayBuffer from the console as a blob to my hard drive, turned it back into a ZIP file, unpacked it and inspected the files. The "Hexndata" folder correctly has the changed save files, and when I add these files to my local DosBOX folder and run it, the save files show the changes. Thus, I believe the problem lies with loading and not saving.

Full index.html file

<!DOCTYPE html>
  <head>
    <style>
      #jsdos-root {
          width:  960px;
          height: 720px;
      }
    </style>
    <link rel="stylesheet" href="js-dos.css">
    <script src="js-dos.js"></script>
  </head>
  <body>
    <div id="jsdos-root"></div>
    <script>
      let db, buf;
      indexedDB.open("js-dos-cache (emulators-ui-saves)").onsuccess = (e) => {
          db = e.target.result;
          const tx = db.transaction("files");
          const store = tx.objectStore("files");
          store.getAll().onsuccess = (e) => {
              buf = e.target.result;
              console.log("Buf is", buf); // you can see the buf is defined here in the console
              Dos(document.getElementById("jsdos-root"))
                  .run("bundle.jsdos",
                       URL.createObjectURL(new Blob(buf)));
          };
      };
    </script>
  </body>
</html>

caiiiycuk commented 2 years ago

Why you doing so? Saving to indexed db is working out of the box...

caiiiycuk commented 2 years ago

You can force saving by calling dos.layers.save()

caiiiycuk commented 2 years ago

If you want to use builtin support for indexedb you just need to call Dos(...).run("bundle.jsdos") without second argument, it will load changes from indexed db if they are exists. You just need to have unique first url to not overwrite changes from different bundles.

caiiiycuk commented 2 years ago

The second argument in run is needed if you want to load changes from some server not from indexed db. Or, if you want to implement own indexed db storage, then you need also override save function with dos.layers.setOnSave. But again, I don't see any cases why you need to do it. just use run(<bundle url>) also you can provide custom key inside indexed db by calling run(<bundle url>, undefined, <key>)

alorello commented 2 years ago

Ah. Sorry, but I should have mentioned that Dos(...).run("bundle.jsdos") is not working for me either, which is what started me on this journey. I can play the game, but if I save using the UI and then close, nothing persists.

alorello commented 2 years ago

I can see that the saving does indeed store into the indexedDB, as I mentioned above, but loading doesn't seem to respect the DB, even though I see it in the dev console.

caiiiycuk commented 2 years ago

Does it work for you on dos.zone?

caiiiycuk commented 2 years ago

Can you share your test page?

alorello commented 2 years ago

I will try dos.zone. Though I believe that would then use the cloud, no? I will try in any case and comment back here.

caiiiycuk commented 2 years ago

It uses cloud only if you logged in, for anonymous users it uses indexedb

alorello commented 2 years ago

It does not seem to be working in dos.zone either. I upload jsdos archive, click "run original file", load the game, mess around and save. Then I X out and go through the process again and it doesn't persist.

caiiiycuk commented 2 years ago

In studio saving should not work, please test on dos.zone game like doom

alorello commented 2 years ago

Doom works. So the problem is specific to either Hexen, or the bundle.jdos I attached to the issue?

alorello commented 2 years ago

I did test that the bundle.jdos.changes buffer in my local DB does in fact contain the save files, so it has to do with the loading and I'm not sure why, as a direct paste of those files into my locally installed DosBox shows the changes I saved in the browser.

alorello commented 2 years ago

Edited issue description to reflect Dos(...).run("bundle.jsdos") attempt.

caiiiycuk commented 2 years ago

For sure it's not related to the bundle. I think you do something unexpected with js-dos integration. I tired build empty project with your bundle and it works for me. Try it, you need to serve _site directory: hex.zip

alorello commented 2 years ago

Thank you! I will try it and reply. If I can figure out what I did wrong in the first place I'll post it here.

alorello commented 2 years ago

Okay, after I tried hosting your package and still had the same problem, I decided to take another look at how I was testing it. I had been overwriting the same save file in all my tests. I load a slot, play around, save in the same slot. Even renaming the slot experiences the same bug. But if I save to a different slot, it keeps it.

Looks like we've found an interesting edge case with how the changes.bundle.jsdos is overlaid on the original filesystem, as I suspect it has to do with either how Hexen saves its data, or how the application logic decides what changes should be overlaid.

I'll dig further into the save files for Hexen tomorrow. Heck, I'll break out hexdump if I have to, as I'm pretty curious.

caiiiycuk commented 2 years ago

Strange for me it also working for overriding same slot. Do you have public url to test?

caiiiycuk commented 2 years ago

But when I tested I always start a new game and overwrite existing slot...

caiiiycuk commented 2 years ago

Just checked again, even opening save game, playing a bit, saving to same slot works for me. Very strange

alorello commented 2 years ago

Hmm okay I will definitely try to throw up a public facing instance tomorrow after work to test with. Thanks for all your help thus far.

alorello commented 2 years ago

All right. The test site is here at http://hexen-test.ddns.net

I'm not crazy, just extremely unlucky. You're right in that most of the save files can be overwritten just fine, it seems. Its just the ONE file I happened to be using that has this problem. I tried all the others and saves persist as expected.

If you want to try it, its the second save slot called lolo. Load it, mess around and resave it and it never persists.

alorello commented 2 years ago

Figured it out. Its related to case sensitivity for the data file names.

The problematic save slot is associated with the only data files in the bundle with lower-case filenames. (I don't know why they were lower-case in the first place):

Using my local DosBox emulator, these lowercase files are deleted and replaced with uppercase versions when I overwrite this slot
Using dos-js, the indexedDB contains the replacement files as uppercase named in memory, which do not replace the old lowercase files in the bundle

Given old DOS games usually use uppercase for data file names, I suppose this could be an issue for other games too, but I bet its a pretty rare problem to encounter. Whether you consider this a "bug" or not is totally up to you of course.

Thanks once more for your assistance, I leave the fate of this issue to you!

alorello commented 2 years ago

Also I'll leave the test site up for a while in case you're curious.

alorello commented 2 years ago

And by the way, the test site is hosted with your hex.zip file serving _site as instructed before.

caiiiycuk commented 2 years ago

Wow, @alorello thank you for deep investigation of this bug. I think that problem is not in upper-case/lower-case filenames. I think it's happened because persist() didn't check if file was deleted, this is something that not trivial to implement. Because of that lower-case saves are not deleted after changes are applied and probably they have bigger priority for hexen. I also agree that it should happen rarely, and because it's not easy to fix, I will let it open until other responses.

alorello commented 2 years ago

You're welcome! And what you said about file priority definitely explains it.

By the way, thank you for this project @caiiiycuk! Being able to run DOS games through Web Assembly is a dream, and the ability to get our games up and running with a smart and simple front-end framework is like this is exciting, to say the least. I plan to use it well.

caiiiycuk / js-dos