Skip to content

Conversation

@dsnopek
Copy link
Contributor

@dsnopek dsnopek commented Aug 4, 2025

It was brought up on chat.godotengine.org by @Yarwin that we're currently calling a GDExtension's startup callback before the extension has been fully reloaded, when any objects that use its classes are in an inconsistent state.

Instead, we'd want the startup callback to be called after reload is fully finished, so that the state is roughly the same as if the extension was being loaded for the first time.

This PR attempts to do that, by moving the "after load" stuff into a _finish_load_extension() method, which won't be called automatically on reload, so that the reload process can call it at the end

This startup callback was functionality added for Godot 4.5, it'd be good to get the order correct before release

@dsnopek dsnopek added this to the 4.5 milestone Aug 4, 2025
@dsnopek dsnopek requested a review from a team as a code owner August 4, 2025 18:31
Copy link
Member

@Ivorforce Ivorforce left a comment

Choose a reason for hiding this comment

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

Looks good to me.
Just an implementation thought; since _load_extension_internal is called with p_first_load=true only from load_extension_with_loader, maybe the _finish_load_extension should be called from load_extension_with_loader instead?

@dsnopek dsnopek force-pushed the gdextension-startup-callback-after-reload branch from 9c1947b to 92d9227 Compare August 5, 2025 13:31
@dsnopek
Copy link
Contributor Author

dsnopek commented Aug 5, 2025

@Ivorforce Thanks!

Just an implementation thought; since _load_extension_internal is called with p_first_load=true only from load_extension_with_loader, maybe the _finish_load_extension should be called from load_extension_with_loader instead?

Yeah, I had thought about that. My thinking was that I wanted to make it clear that _finish_load_extension() should follow _load_extension_internal(). But since this is all internal stuff, and it's all very close together, it's probably clear regardless of how it's done.

In my latest push, I've switched to _finish_load_extension() being called "manually" after _load_extension_internal() in both cases

Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

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

Looks good.

It's internal method so doesn't ready matter, but maybe it should be called something like notify_extension_loaded or similar, since it's only sending signal / callback after actual (re)loading is done.

@Repiteo Repiteo merged commit b1792e5 into godotengine:master Aug 7, 2025
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Aug 7, 2025

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants