godot-cpp icon indicating copy to clipboard operation
godot-cpp copied to clipboard

TypedArray and TypedDictionary do not allow enums as template parameters

Open BenLubar opened this issue 11 months ago • 4 comments

Godot version

4.4.dev7.official.46c8f8c5c

godot-cpp version

befe3ee2f2a129faa321c08e814ccfbd8cd6651c

System information

Debian Trixie, i7-1360P

Issue description

Related to #1584, although the introduction of typed dictionaries makes this much more complicated.

TypedArray and TypedDictionary should be able to take enum types that have been registered with VARIANT_ENUM_CAST as the element or key or value type, treating it as an integer with whatever metadata is needed to make the editor show the correct type name in the brackets.

Steps to reproduce

Contrived toy example:

class Foo : public Object {
    GDCLASS(Foo, Object);

protected:
    static void _bind_methods();

public:
    enum Animal {
        CAT,
        DOG,
        HORSE,
    };

    TypedArray<Animal> get_animals_that_should_go_in_a_house() const;
    TypedDictionary<Animal, ArrayMesh> get_animal_meshes() const;
};

VARIANT_ENUM_CAST(Foo::Animal);

Actual project where I am using typed arrays and dictionaries of enum types: https://github.com/BenLubar/godot4-spy-cards-online (In the actual project, src/dry.h contains a lot of helper macros.)

Minimal reproduction project

N/A

BenLubar avatar Jan 13 '25 21:01 BenLubar

I don't think enums are allowed as types for TypedArray or TypedDictionary in a Godot module either, although, I'm not 100% sure about that. If that is the case, it'll need to get fixed in Godot first, because we try to maintain API compatibility with Godot.

However, if I'm wrong about that, then that's a bug to fix in godot-cpp.

dsnopek avatar Mar 13 '25 17:03 dsnopek

I naively copied the example into a test module and get this error:

.\core/variant/typed_array.h(60): error C2838: 'get_class_static': illegal qualified name in member declaration
.\core/variant/typed_array.h(60): note: the template instantiation context (the oldest one first) is
modules\tracy\foo.cpp(12): note: see reference to class template instantiation 'TypedArray<Foo::Animal>' being compiled
.\core/variant/typed_array.h(59): note: while compiling class template member function 'TypedArray<Foo::Animal>::TypedArray(void)'
modules\tracy\foo.cpp(13): note: see the first reference to 'TypedArray<Foo::Animal>::TypedArray' in 'Foo::get_animals_that_should_go_in_a_house'
.\core/variant/typed_array.h(60): error C3861: 'get_class_static': identifier not found
scons: *** [modules\tracy\foo.windows.editor.x86_64.obj] Error 2

enetheru avatar Mar 14 '25 04:03 enetheru

I don't think enums are allowed as types for TypedArray or TypedDictionary in a Godot module either, although, I'm not 100% sure about that. If that is the case, it'll need to get fixed in Godot first, because we try to maintain API compatibility with Godot.

However, if I'm wrong about that, then that's a bug to fix in godot-cpp.

This might not be something doable in a module but it is soemthing that is doable in other scripting languages

enum TEST {
	TEST_1,
	TEST_2,
	TEST_3,
	TEST_4,
	TEST_5,
}
@export var test2 : Dictionary[float,TEST]

because of that I think you should be able to do it through godot-cpp too. For arrays I was able to do it with the trick found in #1584. Which still need to remove the undef in godot-cpp (which should be modified in source)

MAKE_TYPED_ARRAY_INFO(Test, Variant::INT);
MAKE_TYPED_ARRAY(Test, Variant::INT);

but for typed dictionarries I tried doing the same but couldn't make it work yet. I'll update if I find a way to make it work.

ajreckof avatar Apr 28 '25 10:04 ajreckof

I don't disagree that it would be useful!

But as a matter of policy, we want to avoid merging new things into godot-cpp that are API incompatible with Godot modules. We already have enough existing incompatibilities that we're trying to fix, we shouldn't add any more new ones. :-)

So, if this doesn't work in Godot modules, it should be fixed there first, so that we know what we add here will be compatible with it

dsnopek avatar Apr 28 '25 14:04 dsnopek