Skip to content
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

Implement the #[export] for Array<DynGd<D, T>> #1057

Closed
Yarwin opened this issue Feb 23, 2025 · 1 comment · Fixed by #1056
Closed

Implement the #[export] for Array<DynGd<D, T>> #1057

Yarwin opened this issue Feb 23, 2025 · 1 comment · Fixed by #1056
Labels
bug c: core Core components c: register Register classes, functions and other symbols to GDScript

Comments

@Yarwin
Copy link
Contributor

Yarwin commented Feb 23, 2025

Overwiew

Currently #[export] for Array<DynGd<T, D>> is totally broken and should be fixed. Additionally, #[export] for DynGd<Node, D> is broken too.

Given library such as:

Example Library
trait ExampleTrait {}

#[derive(GodotClass)]
#[class(init, base = Node)]
pub struct Collector {
    #[export]
    nodes: Array<DynGd<Node, dyn ExampleTrait>>,
    #[export]
    resources: Array<DynGd<Node, dyn ExampleTrait>>,
}

#[derive(GodotClass)]
#[class(init, base = Node)]
pub struct A {}

#[godot_dyn]
impl ExampleTrait for A {}

#[derive(GodotClass)]
#[class(init, base = Resource)]
pub struct AR {}

#[godot_dyn]
impl ExampleTrait for AR {}

#[derive(GodotClass)]
#[class(init, base = Node)]
pub struct B {}

#[godot_dyn]
impl ExampleTrait for B {}

#[derive(GodotClass)]
#[class(init, base = Resource)]
pub struct BR {}

#[godot_dyn]
impl ExampleTrait for BR {}

#[derive(GodotClass)]
#[class(init, base = Node)]
pub struct C {}

#[godot_dyn]
impl ExampleTrait for C {}

#[derive(GodotClass)]
#[class(init, base = Resource)]
pub struct CR {}

#[godot_dyn]
impl ExampleTrait for CR {}

The resulting export in the editor doesn't work at all – the inspector behaves irrationally and exporting nodes is pretty much impossible:

Image Image

It is not surprising, since currently our Array<DynGd<D, T>> lacks proper hint_string. One can learn more about expected type hint from Godot Docs – https://docs.godotengine.org/en/stable/classes/[email protected]#enum-globalscope-propertyhint:

PROPERTY_HINT_TYPE_STRING = 23

If a property is Array, hints the editor how to show elements. The hint_string must encode nested types using ":" and "/".

Examples:

# Array of elem_type.
hint_string = "%d:" % [elem_type]
hint_string = "%d/%d:%s" % [elem_type, elem_hint, elem_hint_string]
# Two-dimensional array of elem_type (array of arrays of elem_type).
hint_string = "%d:%d:" % [TYPE_ARRAY, elem_type]
hint_string = "%d:%d/%d:%s" % [TYPE_ARRAY, elem_type, elem_hint, elem_hint_string]
# Three-dimensional array of elem_type (array of arrays of arrays of elem_type).
hint_string = "%d:%d:%d:" % [TYPE_ARRAY, TYPE_ARRAY, elem_type]
hint_string = "%d:%d:%d/%d:%s" % [TYPE_ARRAY, TYPE_ARRAY, elem_type, elem_hint, elem_hint_string]
hint_string = "%d:" % [TYPE_INT] # Array of integers.
hint_string = "%d/%d:1,10,1" % [TYPE_INT, PROPERTY_HINT_RANGE] # Array of integers (in range from 1 to 10).
hint_string = "%d/%d:Zero,One,Two" % [TYPE_INT, PROPERTY_HINT_ENUM] # Array of integers (an enum).
hint_string = "%d/%d:Zero,One,Three:3,Six:6" % [TYPE_INT, PROPERTY_HINT_ENUM] # Array of integers (an enum).
hint_string = "%d/%d:*.png" % [TYPE_STRING, PROPERTY_HINT_FILE] # Array of strings (file paths).
hint_string = "%d/%d:Texture2D" % [TYPE_OBJECT, PROPERTY_HINT_RESOURCE_TYPE] # Array of textures.

hint_string = "%d:%d:" % [TYPE_ARRAY, TYPE_FLOAT] # Two-dimensional array of floats.
hint_string = "%d:%d/%d:" % [TYPE_ARRAY, TYPE_STRING, PROPERTY_HINT_MULTILINE_TEXT] # Two-dimensional array of multiline strings.
hint_string = "%d:%d/%d:-1,1,0.1" % [TYPE_ARRAY, TYPE_FLOAT, PROPERTY_HINT_RANGE] # Two-dimensional array of floats (in range from -1 to 1).
hint_string = "%d:%d/%d:Texture2D" % [TYPE_ARRAY, TYPE_OBJECT, PROPERTY_HINT_RESOURCE_TYPE] # Two-dimensional array of textures.

Naive Approach

I added proper type hint with #1056. The first implementation is very naive – the resulting hint_string is TYPE_OBJECT:(RESOURCE/NODE)_TYPE:all_dyn_trait_implementors_including_objects.

  1. Export for DynGd<Resource, dyn Trait> wrongly assumes that all dyn_gd implementors are resources too!

Image

It is impossible to create nodes and objects using this widget and outside the jank everything works fine:

Image

So that's a minor and bearable issue (which should be fixed nonetheless 😅 ).

  1. Godot doesn't allow to specify multiple exports for Nodes.

This one is much more annoying. Given hint_string for array such as TYPE_OBJECT:NODE_TYPE:A,B,C…. Godot will allow to export only the very first class specified in type hint.

Example

Image

Image

Image

The order of classes in hint_string is not guaranteed and pretty much randomized, so one can get around this issue by restarting the editor multiple times 🧐.

Manual testing
trait ExampleTrait {
    fn dumb_method(&self) {
        godot_print!("Hello, I'm implementor of Example Trait");
    }
}

#[derive(GodotClass)]
#[class(init, base = Node)]
pub struct Collector {
    #[export]
    nodes: Array<DynGd<Node, dyn ExampleTrait>>,
    #[export]
    resources: Array<DynGd<Resource, dyn ExampleTrait>>,
}

#[godot_api]
impl INode for Collector {
    fn ready(&mut self) {
        godot_print!("{}", self.nodes);
        godot_print!("{}", self.resources);

        for n in self.nodes.iter_shared() {
            n.dyn_bind().dumb_method();
        }
        for r in self.resources.iter_shared() {
            r.dyn_bind().dumb_method();
        }
    }
}

Image

Godot Engine v4.3.stable.official.77dcf97d8 - https://godotengine.org
OpenGL API 4.6 (Core Profile) Mesa 24.3.4 - kisak-mesa PPA - Compatibility - Using Device: AMD - AMD Radeon RX 5700 XT (radeonsi, navi10, LLVM 15.0.7, DRM 3.59, 6.12.3-061203-generic)

[A:<A#25165825290>, B:<B#25182602510>, C:<C#25199379723>]
[<AR#-9223372011823168240>, <BR#-9223372011806391022>, <CR#-9223372011789613805>, <AR#-9223372012074826483>, <BR#-9223372011974163185>, <CR#-9223372011873499887>]
Hello, I'm implementor of Example Trait
Hello, I'm implementor of Example Trait
Hello, I'm implementor of Example Trait
Hello, I'm implementor of Example Trait
Hello, I'm implementor of Example Trait
Hello, I'm implementor of Example Trait
Hello, I'm implementor of Example Trait
Hello, I'm implementor of Example Trait
Hello, I'm implementor of Example Trait
--- Debugging process stopped ---

Solution

Exports for DynGd are broken, and not only for nodes inside Arrays, but for #[export]ed nodes too. The question is what should be done about it:

Resources can be exported as they are; the types of given implementors of dyntrait can be checked on runtime with ClassDb; we can store some additional data during class registration as well. Godot has no issue with exporting multiple types of resources.

Nodes are more tricky – exporting Array<DynGd<Node, dyn D>> just as it would be Array<Gd<Node>> seems like obvious solution at first.

Obvious solution

Image

and causes proper error:

E 0:00:00:0345   godot_core::private::report_call_error: godot-rust function call failed: Collector::ready()
    Reason: [panic]  FromGodot::from_variant() failed -- none of the classes derived from `Node` have been linked to trait `dyn ExampleTrait` with #[godot_dyn]: Gd { id: 25216156940, class: Node }
  at /home/irwin/apps/godot/opensource-contr/missing_docs/gdext/godot-core/src/meta/godot_convert/mod.rs:106
  <C++ Source>   /home/irwin/apps/godot/opensource-contr/missing_docs/gdext/godot-core/src/private.rs:336 @ godot_core::private::report_call_error()

So I guess this is fine?

questions about implementation details

Array<DynGd<Resource, D>> should be exported with hint_string such as TYPE_OBJECT:(RESOURCE_TYPE:all_dyn_trait_implementors_including_objects. #[export] DynGd<Resource, D> should be exported with hint string such as all_dyn_trait_implementors_including_objects.

Should we check for ClassDb to include only Resource-based implementors, or should we store this data during class registration? (Personally I think ClassDb is better tbqh) EDIT: I found out is_derived_base_cached
EDIT2: It seems that not all classes are enabled while getting property list, which leads to jank – gotta store this info on registration

Array<DynGd<Node, D>> should be exported with hint_string such as TYPE_OBJECT:NODE_TYPE:base_inherited_node_such_as_node2d_or_node. #[export] DynGd<Node, D> should be exported with hint string such as Node. In other word – for editor #[export] Gd<Node> and #[export] DynGd<Node, D> would be identical (same goes for Array).
Should we add some extra checks or are we fine with godot ones? (Personally I'm fine with godots ones tbh – I tried hacking around and it falls apart fast 😅 )

@Yarwin Yarwin added bug c: core Core components labels Feb 23, 2025
@Yarwin Yarwin added the c: register Register classes, functions and other symbols to GDScript label Feb 23, 2025
@Yarwin
Copy link
Contributor Author

Yarwin commented Feb 23, 2025

Final Implementation in linked PR

  • #[export] DynGd<T, D> works the same as it would for Gd<T> for T inheriting Node; identical for Array of these. Exporting incompatible value in such case results in runtime error.
  • #[export] for DynGd<T, D> for T inheriting Resource creates hint_string which limits selection only to valid DynTrait implementors which inherit T.
  • #[export] DynGd<MyGdextensionClass, D> is not implemented, same goes for arrays. Just use #[export] Gd<MyGdextensionClass> in such cases (needs to be documented). Might be revisited in the future after inheritance will be added.
  • Exports of DynGd with tool remain ultra-quirky and must be documented in the book 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug c: core Core components c: register Register classes, functions and other symbols to GDScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant