gdext icon indicating copy to clipboard operation
gdext copied to clipboard

Seemingly incorrect TypeStringHint for Array[Enum]

Open gg-yb opened this issue 1 year ago • 8 comments

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 avatar Jul 21 '23 13:07 gg-yb

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.

gg-yb avatar Jul 21 '23 13:07 gg-yb

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".

lilizoey avatar Jul 21 '23 15:07 lilizoey

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

gg-yb avatar Jul 21 '23 15:07 gg-yb

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.

lilizoey avatar Jul 21 '23 17:07 lilizoey

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 avatar Jul 24 '23 06:07 gg-yb

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

gg-yb avatar Jul 24 '23 06:07 gg-yb

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.

lilizoey avatar Jul 24 '23 13:07 lilizoey

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

Bromeon avatar Jan 05 '24 12:01 Bromeon