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

Add support for preMainLoop/postMainLoop functions #22621

Merged
merged 1 commit into from
Sep 26, 2024

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Sep 25, 2024

Rather than having code from e.g SDL, etc, in event loop code use a callback registration system.

@@ -149,12 +149,21 @@ LibraryJSEventLoop = {
clearInterval(id);
},

$registerPostMainLoop: (f) => {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add a comment that as an optimization, this does not include MainLoop but instead checks for it at runtime and does nothing?

Separately, I wonder if we can do better than this. We could in theory add a "weak linking" mechanism, that would add some code only if MainLoop was actually included. Though if it's complicated it might not be worth it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sadly I don't think we have such a weak linking mechanism today. The way I did it here I think is the closest thing we have to weak linking in JS code.

Rather than having code from e.g SDL, etc, in event loop code use a
callback registration system.
@@ -149,12 +149,23 @@ LibraryJSEventLoop = {
clearInterval(id);
},

$registerPostMainLoop: (f) => {
// Does nothing unless $MainLoop is included/used.
typeof MainLoop != 'undefined' && MainLoop.postMainLoop.push(f);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
typeof MainLoop != 'undefined' && MainLoop.postMainLoop.push(f);
MainLoop?.postMainLoop.push(f);

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sadly I don't think the ? operator works like that (i.e. it doesn't work on top level things):

$ node
Welcome to Node.js v18.20.3.
Type ".help" for more information.
> a?.foo
Uncaught ReferenceError: a is not defined
> 

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining:

Optional chaining cannot be used on a non-declared root object

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh sorry. That is unfortunate, I wonder why it has that restriction.

@sbc100 sbc100 merged commit f508b43 into emscripten-core:main Sep 26, 2024
28 checks passed
@sbc100 sbc100 deleted the eventloop_register branch September 26, 2024 23:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants