Skip to content

Addon Events API and User Defined Functions access #7947

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

Open
wants to merge 8 commits into
base: dev-2.0
Choose a base branch
from

Conversation

limzykenneth
Copy link
Member

Resolves #7946

Changes:

Implements proposed Events API for addon and switching internal events modules to also use the new syntax.

WIP: still need to implement for all other events.

@limzykenneth
Copy link
Member Author

@ksen0 @davepagurek For the test fail here, it is to do with a bit of potential bottle neck in the lifecycle system, the presetup hook can only have a very fixed number of hooked functions before these tests fails (including if the hook function is just a noop function), the limit seems to be since they are all awaited, it likely roll over too much in the event loop. The test will likely need to be rewritten to not import app.js directly at a later point.

However, I'm thinking of changing the way the hooks are executed. Currently it runs like the below:

async _runLifecycleHook(hookName) {
  for(const hook of p5.lifecycleHooks[hookName]){
    await hook.call(this);
  }
}

Each hook function is executed and awaited in the order in which they are added.

I'm thinking of switching to this instead (which fixes tests here):

async _runLifecycleHook(hookName) {
  await Promise.all(p5.lifecycleHooks[hookName].map(hook => {
    return hook.call(this);
  }));
}

I believe in this case the hook functions are still invoked in order with the major difference being there is no guarantee in the case of async hook functions, one will finish executing before the next is invoked, while the non-async hook functions run sequentially within the same event loop. Is this something that is still fine?

I am separately thinking about something that can control the execution order of the hooks, use case being writing a stats.js addon that uses predraw and postdraw hook to get a full picture of execution time. However for it to properly measure the timing of a single frame, it should be the first predraw hook to run and the last postdraw hook to run. I'm not sure yet how this can be achieved without adding a lot of complexity in the lifecycles API, if this is even possible at all. This probably ties in with the addon dependencies idea as well.

Would appreciate any ideas and thoughts on these.

@davepagurek
Copy link
Contributor

Sorry for the delay on this! The Promise.all seems reasonable to me. To clarify, is this problem with awaiting too many promises an issue just in tests, or does this happen in-browser too?

@limzykenneth
Copy link
Member Author

@davepagurek I've not measured this precisely but I have the vague impression that the setup takes longer to start running the more presetup hooks are awaited in series, so it may also affect in browser start up performance too.

@limzykenneth limzykenneth marked this pull request as ready for review July 8, 2025 19:31
@limzykenneth
Copy link
Member Author

@davepagurek @ksen0 The only thing left here to decide is the name of userDefinedFunctions and whether to go with something else. I would need to do one round of clean up and possibly renaming some internal API but the rest should be ready.

@davepagurek
Copy link
Contributor

I'll think a little more about naming. Also I tried a little test here to see if noop async functions make a difference in setup time, for me I don't see any real difference between 0 and 200 of them: https://editor.p5js.org/davepagurek/sketches/D2NK_p_kN

So there might not be a problem in browser based solely on the number of awaits.

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.

2 participants