Skip to content

Add "init" lifecycle hook #7779

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

Closed
wants to merge 2 commits into from

Conversation

quinton-ashley
Copy link
Contributor

@quinton-ashley quinton-ashley commented Apr 25, 2025

Added two lines of code that implement the "init" lifecycle hook in p5 v2.

This is needed for preload system compatibility. Issue #7742 can't be fixed by moving the current "presetup" hook.

Added two lines of code that implement the "init" lifecycle hook in p5 v2.

This is needed for preload compatibility.
@davepagurek
Copy link
Contributor

Hey @quinton-ashley, thanks for making a PR! I think because we aren't awaiting the hooks, there are some bugs currently. Since we currently await every lifecycle hook, if there exists more than one addon using init, it looks like only the first one gets run before globals are initialized: https://editor.p5js.org/davepagurek/sketches/zZy1xer18

Arguably we shouldn't be awaiting non-async hooks though -- see some discussion here: #7770 (comment) If we end up implementing that, then this implementation will work as-is though (assuming init hooks are never async, which I think is reasonable if this is intended only to be used to initialize globals.)

@quinton-ashley
Copy link
Contributor Author

quinton-ashley commented Apr 27, 2025

@davepagurek ah yes you're right

assuming init hooks are never async, which I think is reasonable if this is intended only to be used to initialize globals.

Yes and for now I'll edit the PR based on that assumption.

@quinton-ashley
Copy link
Contributor Author

quinton-ashley commented Apr 27, 2025

I've tested this PR and it works as a fix for #7742

https://editor.p5js.org/quinton-ashley/sketches/hO-XUOV9K

I'd like if it could be merged before other lifecycle changes are finalized, as that could take a while.

@ksen0
Copy link
Member

ksen0 commented May 7, 2025

Copied from the issue thread for visibility:

I'll be closing this issue (and the associated PR) because based on this discussion, the problem is more general than this issue; that PR does not fully resolve the more general problem. The proposed new init lifecycle hook introduces a relatively major change (the existence of a lifecycle hook that cannot be async). Since there do exist solutions that do not require this, I will close this for now.

@ksen0 ksen0 closed this May 7, 2025
@quinton-ashley quinton-ashley deleted the patch-1 branch May 8, 2025 01:40
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