-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
base: dev-2.0
Are you sure you want to change the base?
Conversation
@ksen0 @davepagurek For the test fail here, it is to do with a bit of potential bottle neck in the lifecycle system, the 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 Would appreciate any ideas and thoughts on these. |
Sorry for the delay on this! The |
@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. |
@davepagurek @ksen0 The only thing left here to decide is the name of |
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. |
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.