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

Send SDL_APP_TERMINATING before unload (#88) #89

Merged
merged 2 commits into from Aug 19, 2019

Conversation

@Beuc
Copy link
Contributor

commented Aug 16, 2019

No description provided.

@@ -652,6 +652,16 @@ Emscripten_HandleVisibilityChange(int eventType, const EmscriptenVisibilityChang
return 0;
}

static EM_BOOL
Emscripten_HandleBeforeUnload(int eventType, const void* reserved, void *userData)

This comment has been minimized.

Copy link
@Daft-Freak

Daft-Freak Aug 16, 2019

Member

void *reserved

@Daft-Freak

This comment has been minimized.

Copy link
Member

commented Aug 16, 2019

Looks good otherwise, though not sure we need the comment about using SDL_AddEventWatch as the SDL_APP_* events always need to be handled like that.

@Beuc

This comment has been minimized.

Copy link
Contributor Author

commented Aug 16, 2019

Ren'Py for instance seems to get away with handling SDL_APP_* in the main thread on Android & iOS, so I guess it's worth making clear this callback is handled differently than the others (to the least my future self will be grateful if I have to work on this code again ;)).
Also I fixed the return value with didn't match the callback signature.

@Beuc

This comment has been minimized.

Copy link
Contributor Author

commented Aug 16, 2019

Fun fact: in both Firefox and Chromium the main loop may receive SDL_APP_TERMINATING.
Similarly FS.sync is not guaranteed to complete (as it's asynchronous, sadly).
Typically if I spam the click button and reload meanwhile, though in practice this is rare.
It seems the browsers still process a random number of events before finalizing the unload. Only the SDL_AddEventWatch callback itself is guaranteed to run.

@Beuc

This comment has been minimized.

Copy link
Contributor Author

commented Aug 18, 2019

Let me know if you need something else :)

@Daft-Freak Daft-Freak merged commit 5b79532 into emscripten-ports:master Aug 19, 2019

@Daft-Freak

This comment has been minimized.

Copy link
Member

commented Aug 19, 2019

Thanks. (Didn't notice the different return type myself...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.