Skip to content

TypedArray and TypedDictionary do not allow enums as template parameters #1684

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
BenLubar opened this issue Jan 13, 2025 · 4 comments
Open
Labels
bug This has been identified as a bug
Milestone

Comments

@BenLubar
Copy link
Contributor

Godot version

4.4.dev7.official.46c8f8c5c

godot-cpp version

befe3ee

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

@dsnopek dsnopek added the bug This has been identified as a bug label Jan 21, 2025
@dsnopek dsnopek added this to the 4.x milestone Jan 21, 2025
@dsnopek
Copy link
Collaborator

dsnopek commented Mar 13, 2025

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.

@enetheru
Copy link
Collaborator

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

@ajreckof
Copy link
Member

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.

@dsnopek
Copy link
Collaborator

dsnopek commented Apr 28, 2025

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This has been identified as a bug
Projects
None yet
Development

No branches or pull requests

4 participants