godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
89.73k stars 20.84k forks source link

GDExtension: Unable to add_resource_format_loader #87728

Open ksmit799 opened 8 months ago

ksmit799 commented 8 months ago

Tested versions

System information

Godot v4.2.1.stable - Windows 10.0.22631 - Vulkan (Forward+) - integrated Intel(R) Iris(R) Xe Graphics (Intel Corporation; 31.0.101.4889) - 12th Gen Intel(R) Core(TM) i7-1280P (20 Threads)

Issue description

Following this official guide and making note of this proposal, it should be possible to register custom ResourceLoader's within a GDExtension. I've checked and triple checked that I haven't missed anything, following the JSON and Image implementation of ResourceFormatLoader under godot/core/io/ verbatim. The project I'm working on implementing this in is open-source, and the files of interest are as follows:

When attempting to debug this, calling print(ResourceLoader.get_recognized_extensions_for_type("DC")) within a GDScript file returns ["tres"], when it should return ["tres", "dc"]. Calling the same code with "JSON" instead returns ["tres", "res", "json"]

Other functions of the GDExtension work fine, as can be seen here, it's just the custom ResourceLoader I'm having issues with.

Any help would be greatly appreciated!

Steps to reproduce

The MRP contains the compiled extension for windows as well as the demo code that causes the debugger to error with: `E 0:00:00:0541 demo.gd:7 @ _ready(): No loader found for resource: res://data/example.dc (expected type: ) <C++ Error> Method/function failed. Returning: Ref() <C++ Source> core/io/resource_loader.cpp:282 @ _load()

demo.gd:7 @ _ready() ` ### Minimal reproduction project (MRP) [MRP.zip](https://github.com/godotengine/godot/files/14093035/MRP.zip)
rburing commented 8 months ago

Your loader is missing the GDCLASS macro. Does adding that fix it?

ksmit799 commented 8 months ago

Your loader is missing the GDCLASS macro. Does adding that fix it?

Unfortunately not, I get this error in the debugger in addition to the other ones: E 0:00:00:0649 set_object_extension_instance: Cannot get class 'DCFileLoader'. <C++ Error> Parameter "ti" is null. <C++ Source> core/object/class_db.cpp:392 @ set_object_extension_instance()

FWIW the core/io/ resource loaders don't use that macro either, presumably as they have no methods to bind. The example I linked above is also outdated (even though it's tagged latest), using the RES macro instead of Ref<Resource> load();

rburing commented 8 months ago

That's progress. The error hints you that you should register the loader class in ClassDB before creating an instance / calling add_resource_format_loader.

ksmit799 commented 8 months ago

That's progress. The error hints you that you should register the loader class in ClassDB before creating an instance / calling add_resource_format_loader.

Hmm, that's resolved the Cannot get class 'DCFileLoader'. error but I'm still getting the original one.

rburing commented 8 months ago

Just noticed you're following documentation for modules to write a GDExtension. There are slight differences. Take a look at the generated header file godot-cpp/gen/include/godot_cpp/classes/resource_format_loader.hpp (which you correctly include) and override the virtual methods listed there (which start with an underscore and can have slightly different signatures). Not all of them have to be overridden; the underscored versions of the ones you currently have should be enough.

ksmit799 commented 8 months ago

Just noticed you're following documentation for modules to write a GDExtension. There are slight differences. Take a look at the generated header file godot-cpp/gen/include/godot_cpp/classes/resource_format_loader.hpp (which you correctly include) and override the virtual methods listed there (which start with an underscore and can have slightly different signatures). Not all of them have to be overridden; the underscored versions of the ones you currently have should be enough.

Thank you, that worked! I guess the GDVIRTUALxRC macros don't work for extensions? Also, would you be able to quickly make sure this doesn't cause a memory leak?

I can draft up a docs page for custom resource loading within GDExtensions if that's something you would want?

Thank you again!