godotengine / godot-docs

Godot Engine official documentation
https://docs.godotengine.org
Other
3.92k stars 3.2k forks source link

`unitialize_example_model` in GDExtension example looks suspiciously like a no-op #9660

Open TV4Fun opened 3 months ago

TV4Fun commented 3 months ago

Your Godot version: master Issue description: The GDExtension C++ example gives this rather odd example for uninitialize_example_model in register_types.cpp:

void uninitialize_example_module(ModuleInitializationLevel p_level) {
    if (p_level != MODULE_INITIALIZATION_LEVEL_SCENE) {
        return;
    }
}

This looks a lot like a function that checks for a specific module initialization level, but then returns without doing anything either way? Is this intentional? Is this function actually necessary here? If it is, the documentation should really say something about it, as it's rather confusing as it is now. If there is some uninitialization code that would be needed in other circumstances but isn't needed here, the documentation should really include some examples of when it would be needed.

URL to the documentation page (if already existing): https://github.com/godotengine/godot-docs/blob/d0fde25f53d99d7b4aa0ed49f915d3e2e7efee55/tutorials/scripting/gdextension/gdextension_cpp_example.rst?plain=1#L275

AThousandShips commented 3 months ago

The function must be registered, but it could just be empty instead, but there's nothing wrong about an empty method here, there's nothing to uninitialize here, as you don't unregister classes etc.

There is a description of what's done right below:

All we're doing here is parse through the functions in our bindings module to initialize them, but you might have to set up more things depending on your needs.

But we could add some further elaborate example for de-initialization, like some singleton, but it feels unnecessary, we could just remove the:

if (p_level != MODULE_INITIALIZATION_LEVEL_SCENE) {
    return;
}

The same exact code is used in several engine modules though, see: https://github.com/godotengine/godot/blob/607b230ffe120b2757c56bd3d52a7a0d4e502cfe/modules/csg/register_types.cpp#L60-L64 https://github.com/godotengine/godot/blob/607b230ffe120b2757c56bd3d52a7a0d4e502cfe/modules/etcpak/register_types.cpp#L45-L49 https://github.com/godotengine/godot/blob/607b230ffe120b2757c56bd3d52a7a0d4e502cfe/modules/gridmap/register_types.cpp#L54-L58 https://github.com/godotengine/godot/blob/607b230ffe120b2757c56bd3d52a7a0d4e502cfe/modules/jsonrpc/register_types.cpp#L45-L49