GDExtension: Always run shutdown callback before deinitializing any levels

[dsnopek]

This is a small fix to PR #106030

When normal shutdown occurs, the shutdown callbacks are run before any of the initialization levels are deinitialized. So, when an extension is unloaded before Godot is shutdown, it should also run it before any of the initialization levels are deinitialized, which is what this PR does.

(The diff is a little weird, because it removes and adds code that was already there, not the code added by the referenced PR. But the result is the same: the order of the two things are swapped.)

I also snuck in a couple tiny comments to document these callbacks in gdextension_interface.h, which was discussed at the last GDExtension team meeting. If it'd be better to move that to its own PR, I can, but these changes are minor fix ups to the same new feature so I thought it may be OK :-)

[Bromeon]

I know this code just moved, but something I noticed is the use of level >= 0 (ordinal number), but then
i >= GDExtension::INITIALIZATION_LEVEL_CORE (constant, but with implicit knowledge that CORE == 0).

The if is not really necessary at all, is it?

Values are defined as follows:

godot/core/extension/gdextension_interface.h

Lines 729 to 735 in 46c495c

	typedef enum {
	GDEXTENSION_INITIALIZATION_CORE,
	GDEXTENSION_INITIALIZATION_SERVERS,
	GDEXTENSION_INITIALIZATION_SCENE,
	GDEXTENSION_INITIALIZATION_EDITOR,
	GDEXTENSION_MAX_INITIALIZATION_LEVEL,
	} GDExtensionInitializationLevel;

[dsnopek]

GDExtensionManager::level defaults to -1:

godot/core/extension/gdextension_manager.h

Line 38 in 46c495c

int32_t level = -1;

... which is used to indicate that we haven't been initialized up to any level yet. So, I think this if is still necessary.

I don't know that this is done in the clearest possible way, but it's not something I think we need to address in this PR :-)

[Bromeon]

But in the case of level == -1, this for loop:

for (int32_t i = level; i >= GDExtension::INITIALIZATION_LEVEL_CORE; i--)

would run as:

for (int32_t i = -1; i >= 0; i--)

i.e. never -- so I don't think the if is needed.

I agree it doesn't have to be part of this PR, just noticed it during review.

[dsnopek]

Oh, I see what you're saying now! Yeah, I think you're right :-)

Let's leave it be in this PR, though

[Bromeon]

I think you mentioned it @dsnopek, but I don't remember: what was the reason again for frame_func to exist, in addition to _process()?

Also in light of the godotengine/godot-proposals#12622 initiative, which wants to move hardcoded .h functionality to the .json file 🙂

[dsnopek]

I think you mentioned it @dsnopek, but I don't remember: what was the reason again for frame_func to exist, in addition to _process()?

It's to make a GDExtension equivalent to ScriptLanguage::frame() (that's why the comment makes reference to ScriptServer::frame() which is what calls that method on all the registered script languages). If your GDExtension is just binding a language, like the C# GDExtension binding, there is no Node to put a _process() method on.

There are some workarounds, but they are awkward.

For example, with the the startup_func, you can now access the main loop, and if it's SceneTree, you can connect to the process_frame signal. However, it's not uncommon to replace the main loop in automated tests, and you wouldn't want your language binding to stop working in that case. :-) There's also a workaround involving RenderingServer, but it's weird for a language binding to depend on RenderingServer when it has nothing to do with rendering.

[Bromeon]

Thank you very much! 👍
Do you think it makes sense to add a short version of this to the frame_func docs as well?

[dsnopek]

Do you think it makes sense to add a short version of this to the frame_func docs as well?

Sure, I added this additional comment:

// This is intended to be the equivalent of `ScriptLanguage::frame()` for GDExtension language bindings that don't use the script API.

[Bromeon]

Thanks a lot! 🙂

[Repiteo]

Thanks!