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] Synchronous main, better persistence, handlers fixes, optional full screen. #42178

Merged
merged 8 commits into from
Sep 23, 2020

Conversation

Faless
Copy link
Collaborator

@Faless Faless commented Sep 18, 2020

This PR has most of the things in #41097 (excluding the virtual keyboard support):

Plus, a rework of the initialization process to better handle file system synchronization (last commit, fixes #39643):

The engine now expects to emscripten FS to be setup and sync-ed before
main is called. This is exposed via Module["initFS"] which also allows
to setup multiple persistence paths (internal use only for now).

Additionally, FS syncing is done once for every loop if at least one
file in a persistent path was open for writing and closed, and if the FS
is not syncing already.

This should potentially fix issues reported by users where "autosave"
would not work on the web (never calling syncfs because of too many
writes).

There is a 3.2 branch of all these changes for testing and easy cherry picking: https://github.com/Faless/godot/tree/js/3.0_sync_fs_size_handlers

EDIT: Converted to draft, there is a bug when de-registering events. Will update soonish Fixed

Add override keyword to RasterizerDummy methods.
Plus cleanup unused methods, remove virtual keyword.
@Faless
Copy link
Collaborator Author

Faless commented Sep 19, 2020

Fixed silly bug in event listeners add/remove helpers.

@Faless Faless marked this pull request as ready for review September 19, 2020 16:41

#include <emscripten/emscripten.h>
#include <stdlib.h>

static OS_JavaScript *os = nullptr;
static uint64_t target_ticks = 0;

extern "C" EMSCRIPTEN_KEEPALIVE void _request_quit_callback(char *p_filev[], int p_filec) {
if (os && os->get_main_loop()) {
os->get_main_loop()->notification(Node::NOTIFICATION_WM_CLOSE_REQUEST);
Copy link
Member

@akien-mga akien-mga Sep 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There seems to be DisplayServer::WINDOW_EVENT_CLOSE_REQUEST, which is what triggers emitting NOTIFICATION_WM_CLOSE_REQUEST in Window. Maybe this is better suited here than emitting a Node notification?

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left one tiny comment but otherwise this is good to merge.

Crated helper class in native/utils.js.
Simplify code in OS/DisplayServer.
The engine now expects to emscripten FS to be setup and sync-ed before
main is called. This is exposed via `Module["initFS"]` which also allows
to setup multiple persistence paths (internal use only for now).

Additionally, FS syncing is done **once** for every loop if at least one
file in a persistent path was open for writing and closed, and if the FS
is not syncing already.

This should potentially fix issues reported by users where "autosave"
would not work on the web (never calling `syncfs` because of too many
writes).
@akien-mga akien-mga merged commit c10e8ac into godotengine:master Sep 23, 2020
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment