godot-rust / gdext

Rust bindings for Godot 4
https://mastodon.gamedev.place/@GodotRust
Mozilla Public License 2.0
3.08k stars 193 forks source link

Seemingly incorrect TypeStringHint for Array[Enum] #353

Open gg-yb opened 1 year ago

gg-yb commented 1 year ago

The relevant type string hint is generated here: https://github.com/godot-rust/gdext/blob/3ab6b2ae32b090c004586dd19dd2918d8546cdf7/godot-core/src/builtin/array.rs#L645

Example code:

...
pub struct Textbox {
  ...
  #[export]
  font_types: Array<FontType>,
  ...
}
...

vs.

extends Control

enum Foo {
    Bar = 0,
    Baz = 1,
    Qux = 2,
}

@export
var lol: Array[Foo] = []

When inspected via

func _ready():
    for prop in get_property_list():
        if prop.name == "lol":
            print(prop)
    var mt = Textbox.new()
    for prop in mt.get_property_list():
        if prop.name == "font_types":
            print(prop)

the following output is obtained:

{ "name": "lol", "class_name": &"", "type": 28, "hint": 23, "hint_string": "2/2:Bar:0,Baz:1,Qux:2", "usage": 4102 }
{ "name": "font_types", "class_name": &"Textbox", "type": 28, "hint": 23, "hint_string": "Generic33x8:0,Greek33x3:1,Cyrillic33x4:2,Compact6x6:3", "usage": 6 }

Note the missing "2/2:" prefix. After some digging around, the first 2 apparently stands for VariantType::Int, the second 2 stands for PROPERTY_HINT_ENUM, though I am not 100% sure of this.

Note that the gdext version does not work in the editor, it simply shows up as unspecified ints, while the gdscript variant shows the enum labels.

Note that I implement TypeStringHint for FontType with what is essentially the "hint_string" from the output. From my understanding, there should still be a prefix added to the hint from the line referenced above, but it appears to not be added.

gg-yb commented 1 year ago

Upon further inspection, the behavior for no prefix at all comes from here: https://github.com/godot-rust/gdext/blob/3ab6b2ae32b090c004586dd19dd2918d8546cdf7/godot-core/src/builtin/array.rs#L665

Even then, gdscript behavior implies that simply prefixing with VariantType::Array is not the solution here.

lilizoey commented 1 year ago

Array<T> will have the hint string be whatever the TypeStringHint of T is, so you should implement the TypeStringHint for your type to have the 2/2: prefix. a/b:c means, "values of type a, with hint b, and hint string c".

gg-yb commented 1 year ago

Wouldn't that be the wrong type string if I use T without an array?

lilizoey commented 1 year ago

Wouldn't that be the wrong type string if I use T without an array?

type strings are specific to when a type is contained in some other type that use the PROPERTY_HINT_TYPE_STRING property hint. which is mainly when a type is contained in an Array. they aren't used in other cases.

gg-yb commented 1 year ago

So I figured out a way to get my enum work both as

#[export]
foo: Foo,

and

#[export]
foo: Array<Foo>,

The trick seems to be to yield "Bar:0,Baz:1,Qux:2" as the type string in the export info in impl Export for Foo (hint is PROPERTY_HINT_ENUM), and at the same time yielding "2/2:Bar:0,Baz:1,Qux:2" in impl TypeStringHint for Foo.

If I add "2/2:" to the one, it adds a "2/2" element in the dropdown for the enum. If I remove "2/2:" on the other, it doesn't work for arrays.

This feels unintuitive, so this should be documented somewhere prominent IMO.

gg-yb commented 1 year ago

Demo code can be found here: https://github.com/gg-yb/gdext/commit/6192ac2af6507deedb113eeeb2e36273532804de

lilizoey commented 1 year ago

So I figured out a way to get my enum work both as

#[export]
foo: Foo,

and

#[export]
foo: Array<Foo>,

The trick seems to be to yield "Bar:0,Baz:1,Qux:2" as the type string in the export info in impl Export for Foo (hint is PROPERTY_HINT_ENUM), and at the same time yielding "2/2:Bar:0,Baz:1,Qux:2" in impl TypeStringHint for Foo.

To clarify, this hint string in export is not a type string. Type string is specifically a hint string used with PROPERTY_HINT_TYPE_STRING. since youre using PROPERTY_HINT_ENUM, then the hint string isn't a type string.

If I add "2/2:" to the one, it adds a "2/2" element in the dropdown for the enum. If I remove "2/2:" on the other, it doesn't work for arrays.

This feels unintuitive, so this should be documented somewhere prominent IMO.

That's fair. Could explain a bit better what the type string is in the trait. But Export and TypeStringHint are different traits because they should be different values.

Bromeon commented 9 months ago

There have been quite a few changes to exporting in the last half a year, is this still a problem?