2shady4u / godot-sqlite

GDExtension wrapper for SQLite (Godot 4.x+)
MIT License
947 stars 82 forks source link

Html5 saving #112

Closed fireworx closed 1 year ago

fireworx commented 1 year ago

closing of db doesn't save in indexdb,

but saving after closing db, a new dummy file will enforce that db is saved

maybe a chance for a generally workaround ;-)

2shady4u commented 1 year ago

Hey @fireworx! Would it be possible to give the code to a minimal reproduction project such that I can better understand the issue?

The database should be automatically saved when you close it and should be stored in the user://-folder (which is the IndexedDB in the case of HTML5).

fireworx commented 1 year ago

testsql.zip

fireworx commented 1 year ago

try it with and without dummysave(), it will not write /userfs/test.db when changes are made

even, if i close the page, no save

2shady4u commented 1 year ago

Hi @fireworx

I currently don't have time to actually run that code. One thing I've noticed is that you are dropping your table each time you restart/reload your application:

    # Throw away any table that was already present
    db.drop_table(table_name)

I'll try to run your code as soon as I can.

fireworx commented 1 year ago

its your own democode ;-)

meantime i checked if it is a browserbased problem, but it isn't. (chrome/firefox)

2shady4u commented 1 year ago

Hi @fireworx,

I finally had some time to try and replicate the issue. As a result, I've successfully replicated the issue; there's indeed something strange going on with the database saving on HTML5.

I'll try to find some time as soon as possible to figure out what is going on and hopefully get it fixed.

2shady4u commented 1 year ago

It seems that Godot doesn't automatically sync the userfs in HTML. The user filesystem only gets synced whenever the close()-method gets called on a File, as found here: https://github.com/godotengine/godot/blob/4e48b855d5cd5dc21e0cabd659386c0d0f425acf/platform/javascript/os_javascript.cpp#L1034

Easiest work-around at the time of writing seems to be to simply open a dummy file as such:

var file := File.new()
file.open("user://whatever", File.WRITE)
file.close()

This will force Godot to sync the filesystem when this file closes.

2shady4u commented 1 year ago

A feasible fix might be to allow the user to select the VFS (default or the godot one); in which case the files sync automatically whenever you use the godot VFS.

This seems to be best approach to tackle this issue:

if OS.get_name() in ["HTML5"]:
    var engine = JavaScript.get_interface("engine")
    var GodotFS = engine.rtenv.GodotFS
    if not GodotFS._syncing:
        GodotFS.sync()

I'll see if I can implement this in the C++ close_db()-method at some point.

2shady4u commented 1 year ago

I've ported the above code snippet and added it to the close_db()-method in the fix-html-saving-branch. Unfortunately it seems that this is actually not supposed to work as the GodotFS-property should actually not be exposed. The so-called closure compiler is supposed to rename/minimize this object in official builds, but, for some reason, it is not enabled in the latest Godot builds.

As a result, there's two options here:

2shady4u commented 1 year ago

Hi @fireworx

I've decided on an entirely different approach, namely to always use the custom VFS for HTML targets. As the custom VFS shim uses Godot's File class internally, the file system will be synced automatically whenever the database gets closed as the internal file will also get closed.

A small concern is the matter of speed (SQLite's default VFS vs. my custom VFS), but I think the difference will be negligible.

2shady4u commented 1 year ago

Posting this here for future reference:

#ifdef __EMSCRIPTEN__
            /* In the case of the web build, we'll have to manually force an update of the file system (IndexedDB) */
            /* The method used here is dangerous as the GodotFS object *might* not be exposed on official builds due to the closure compiler renaming/minimizing the property */
            Ref<JavaScriptObject> engine = JavaScript::get_singleton()->get_interface("engine");
            Ref<JavaScriptObject> rtenv = engine->get("rtenv");
            Ref<JavaScriptObject> GodotFS = rtenv->get("GodotFS");
            if (!GodotFS->get("_syncing"))
            {
                GODOT_LOG(0, "Manually forcing GodotFS to sync!");
                GodotFS->call("sync");
            }
#endif
2shady4u commented 1 year ago

This issue has now been fixed in the latest release (https://github.com/2shady4u/godot-sqlite/releases/tag/v3.5) 🥳

Thank you for reporting this issue and enjoy your coding!