godotengine / godot-cpp

C++ bindings for the Godot script API
MIT License
1.7k stars 528 forks source link

`GDCLASS` requires the exposed class name to match the C++ class name. #1428

Closed elenakrittik closed 6 months ago

elenakrittik commented 6 months ago

Godot version

4.1.3-stable

godot-cpp version

The 4.1 branch

System information

Godot v4.1.3.stable - Windows 10.0.19045 - Vulkan (Compatibility) - NVIDIA GeForce RTX 3060 (NVIDIA; 31.0.15.5123) - AMD Ryzen 7 5800X 8-Core Processor (8 Threads)

Issue description

If in the example from docs i do the following change:

#ifndef GDEXAMPLE_H
#define GDEXAMPLE_H

#include <godot_cpp/classes/sprite2d.hpp>

namespace godot {

class GDExample : public Sprite2D {
-    GDCLASS(GDExample, Sprite2D)
+    GDCLASS(AnotherName, Sprite2D)

private:
    double time_passed;

protected:
    static void _bind_methods();

public:
    GDExample();
    ~GDExample();

    void _process(double delta) override;
};

}

#endif

The code stops compiling with the following log:

scons: Reading SConscript files ...
Auto-detected 8 CPU cores available for build parallelism. Using 7 cores by default. You can override it 
with the -j argument.
Building for architecture x86_64 on platform windows
scons: done reading SConscript files.
scons: Building targets ...
scons: `godot-cpp\bin\libgodot-cpp.windows.template_debug.x86_64.lib' is up to date.
cl /Fosrc\gdexample.windows.template_debug.x86_64.obj /c src\gdexample.cpp /TP /EHsc /std:c++17 /nologo /utf-8 /MT /O2 /DTYPED_METHOD_BIND /DNOMINMAX /DWINDOWS_ENABLED /DDEBUG_ENABLED /DDEBUG_METHODS_ENABLED /DNDEBUG /DGDEXTENSION /Igodot-cpp\gdextension /Igodot-cpp\include /Igodot-cpp\gen\include /Isrc
cl /Fosrc\register_types.windows.template_debug.x86_64.obj /c src\register_types.cpp /TP /EHsc /std:c++17 /nologo /utf-8 /MT /O2 /DTYPED_METHOD_BIND /DNOMINMAX /DWINDOWS_ENABLED /DDEBUG_ENABLED /DDEBUG_METHODS_ENABLED /DNDEBUG /DGDEXTENSION /Igodot-cpp\gdextension /Igodot-cpp\include /Igodot-cpp\gen\include /Isrc 
gdexample.cpp
register_types.cpp
C:\Users\lena\Development\Repos\gcppnames\src\gdexample.h(9): error C4430: missing type specifier - int assumed. Note: C++ does not support default-intC:\Users\lena\Development\Repos\gcppnames\src\gdexample.h(9): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int

C:\Users\lena\Development\Repos\gcppnames\src\gdexample.h(9): error C2143: syntax error: missing ',' before '&'C:\Users\lena\Development\Repos\gcppnames\src\gdexample.h(9): error C2143: syntax error: missing ',' before '&'

C:\Users\lena\Development\Repos\gcppnames\src\gdexample.h(9): error C3646: 'self_type': unknown override 
specifierC:\Users\lena\Development\Repos\gcppnames\src\gdexample.h(9): error C3646: 'self_type': unknown 
override specifier

C:\Users\lena\Development\Repos\gcppnames\src\gdexample.h(9): error C2825: 'godot::GDExample::AnotherName': must be a class or namespace when followed by '::'C:\Users\lena\Development\Repos\gcppnames\src\gdexample.h(9): error C2825: 'godot::GDExample::AnotherName': must be a class or namespace when followed by '::'

C:\Users\lena\Development\Repos\gcppnames\src\gdexample.h(9): error C2510: 'AnotherName': left of '::' must be a class/struct/unionC:\Users\lena\Development\Repos\gcppnames\src\gdexample.h(9): error C2510: 'AnotherName': left of '::' must be a class/struct/union

C:\Users\lena\Development\Repos\gcppnames\src\gdexample.h(9): error C2530: 'E': references must be initializedC:\Users\lena\Development\Repos\gcppnames\src\gdexample.h(9): error C2530: 'E': references must be initialized

C:\Users\lena\Development\Repos\gcppnames\src\gdexample.h(9): error C2143: syntax error: missing ';' before ':'C:\Users\lena\Development\Repos\gcppnames\src\gdexample.h(9): error C2143: syntax error: missing ';' before ':'

C:\Users\lena\Development\Repos\gcppnames\src\gdexample.h(9): error C2143: syntax error: missing ';' before ')'C:\Users\lena\Development\Repos\gcppnames\src\gdexample.h(9): error C2143: syntax error: missing ';' before ')'

scons: *** [src\register_types.windows.template_debug.x86_64.obj] Error 2
scons: *** [src\gdexample.windows.template_debug.x86_64.obj] Error 2
scons: building terminated because of errors.

Steps to reproduce

Please see below

Minimal reproduction project

Clone https://github.com/elenakrittik/gcppnames and compile as usual.

dsnopek commented 6 months ago

I'm pretty sure this is also a requirement for the GDCLASS() macro in Godot itself. Since one of the design goals of godot-cpp is to be as API compatible as possible with modules in Godot, this would need to be changed in Godot first.

elenakrittik commented 6 months ago

Does godot-cpp somehow re-use the macro definition from Godot, or does godot-cpp copy it? If the latter, may it be possible to refactor GDCLASS(ExposedName, Ancestor) into something like GDCLASS_CUSTOM(Cppname, ExposedName, Ancestor) and make GDCLASS(ExposedName, Ancestor) = GDCLASS_CUSTOM(ExposedName, ExposedName, Ancestor) to preserve compatibility?

AThousandShips commented 6 months ago

I don't really see why this would be useful? Why would you want to register it in this way? You have namespaces in godot-cpp so it's not a matter of name collision, we register the core binds by their names in that way, see core/core_binds.h in the engine

To me this just adds complexity and fragility to the build system without adding any needed feature

elenakrittik commented 6 months ago

Oh, namespaces, right.. My case was indeed to avoid name collisions, but i forgot about namespaces. Not a day-to-day C++ user as you can see 😅 Case closed, thanks!