Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Html5 saving #112

Closed
fireworx opened this issue Dec 5, 2022 · 12 comments
Closed

Html5 saving #112

fireworx opened this issue Dec 5, 2022 · 12 comments
Labels
bug Something isn't working
Milestone

Comments

@fireworx
Copy link

fireworx commented Dec 5, 2022

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 ;-)

@fireworx fireworx added the bug Something isn't working label Dec 5, 2022
@2shady4u
Copy link
Owner

2shady4u commented Dec 5, 2022

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
Copy link
Author

fireworx commented Dec 6, 2022

testsql.zip

@fireworx
Copy link
Author

fireworx commented Dec 6, 2022

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
Copy link
Owner

2shady4u commented Dec 6, 2022

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
Copy link
Author

fireworx commented Dec 6, 2022

its your own democode ;-)

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

@2shady4u
Copy link
Owner

2shady4u commented Dec 8, 2022

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 2shady4u added this to the v3.5 milestone Dec 8, 2022
@2shady4u
Copy link
Owner

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
Copy link
Owner

2shady4u commented Dec 26, 2022

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
Copy link
Owner

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
Copy link
Owner

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
Copy link
Owner

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
Copy link
Owner

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants