export property with Array typehints

[Taywee]

In gdscript, this is available:

export (Array, PackedScene) var foo

Which gives the editor the ability to resize the passed-in array, and enforces the type of each element. I can't figure out how to do the same thing with this crate. To be honest, I've been digging around and I can't figure out how this is supposed to be done with nativescript in general, or if it's even possible in nativescript at all. There are no examples I can find of property exports above any very simple complexity (of at most a single built-in type, and maybe a specific String or number hint).

If this is possible with this crate, it would be great to have an example that shows how. I'd be glad to build it and open a pull request if I can figure out how to do this at all, but I spent a few hours last night attacking it to no avail.

The library has facility for "complex" export hints in the gdnative::nativescript::init::property module, but array element type hints in particular are indeed not available in the library yet. To implement it, you'll have to dive into the engine source to find out what the export string should be, since engine internal export strings (not equivalent to GDScript notations) have literally zero documentation. Once you have figured out what the hint string should be, support should be as easy as adding a Export implementation.

[Taywee]

Oof. I was afraid (but expecting) that the answer would be something like that. Thanks for the insight. I'll see if I can suss something out, but don't hold your breath on me, because I'm about as far as it gets from a Godot expert.
Wish me luck, I'm going in.

If you need any help with implementation, please feel free to join us on the godot-rust Discord server, which is the main channel of communication for developers of the library.

[Taywee]

So, I dug a little bit into Godot, added some printf-debugging to editor/editor_properties.cpp, line 2983 (in tag 3.2.3-stable):

bool EditorInspectorDefaultPlugin::parse_property(Object *p_object, Variant::Type p_type, const String &p_path, PropertyHint p_hint, const String &p_hint_text, int p_usage) {

	float default_float_step = EDITOR_GET("interface/inspector/default_float_step");
    std::cout << "TAYLOR name: " << p_object->get_class_name() << std::endl;
    std::cout << "TAYLOR type: " << p_type << std::endl;
    std::cout << "TAYLOR path: ";
    print_line(p_path);
    std::cout << "TAYLOR hint: " << p_hint << std::endl;
    std::cout << "TAYLOR hint_text: ";
    print_line(p_hint_text);
    std::cout << "TAYLOR usage: " << p_usage << std::endl;

Then I created a sample project with this as the script for the only node and scene:

extends Node2D

export(Array, PackedScene) var testpackedscenearray

When I ran it, this was in the output:

TAYLOR name: 0x1
TAYLOR type: 19
TAYLOR path: testpackedscenearray
TAYLOR hint: 24
TAYLOR hint_text: 17/17:PackedScene
TAYLOR usage: 8199

That type is ARRAY (as might be expected), and the hint corresponds to PROPERTY_HINT_TYPE_STRING. I'm not sure what the first field of that hint_text corresponds to. I'm assuming we'd have the same result if we put 17/17:PackedScene into the ExportInfo hint_string, but I'm not sure what the 17/17: implies yet. I haven't backtracked that back to its origin yet, and the Godot codebase is very large.

edit: This might be something that would be worth just asking the Godot developers directly, if there's an easy way to get ahold of them.

edit 2: Looks like the 17 corresponds to Variant::Type::OBJECT, via editor/array_property_edit.cpp. Not sure exactly why it's there twice with a slash between it. The first one is set as a subtype (as an int), and the second as a subtype_hint (as a string), whatever the difference may be. Not sure why the triple-redundancy is necessary.

@Taywee You can find Godot devs in #godotengine-devel on irc.freenode.net, if you need to ask them anything directly.

[Taywee]

One last poke for the night, I tried this:

extends Node2D

export(int, "Warrior", "Magician", "Thief") var testsceneintenum
export(Array, int, "Bard", "Barbarian", "Druid") var testsceneintenumarray

And got this:

TAYLOR name: 0x1
TAYLOR type: 2
TAYLOR path: testsceneintenum
TAYLOR hint: 3
TAYLOR hint_text: Warrior,Magician,Thief
TAYLOR usage: 8199
TAYLOR name: 0x1
TAYLOR type: 19
TAYLOR path: testsceneintenumarray
TAYLOR hint: 24
TAYLOR hint_text: 2/3:Bard,Barbarian,Druid
TAYLOR usage: 8199

And also tried:

extends Node2D

export(int, -10, 20) var testsceneintrange
export(Array, int, -10, 20) var testsceneintrangepackedarray

And got

TAYLOR name: 0x1
TAYLOR type: 2
TAYLOR path: testsceneintrange
TAYLOR hint: 1
TAYLOR hint_text: -10,20
TAYLOR usage: 8199
TAYLOR name: 0x1
TAYLOR type: 19
TAYLOR path: testsceneintrangepackedarray
TAYLOR hint: 24
TAYLOR hint_text: 2/1:-10,20
TAYLOR usage: 8199

So it seems the hint_text for type ARRAY hint HINT_TYPE_STRING is {subtype}/{subhint}:{subhint_text}, so it's clearer how that really works. The question is how to safely allow the developer to expose this to the editor from this crate.

edit: and on a whim, doing export(Array, Array, int, "Bard", "Barbarian", "Druid") gave 19:2/3:Bard,Barbarian,Druid, so it appears that extra dimensions just append in order in a simple manner.

The question is how to safely allow the developer to expose this to the editor from this crate.

@Taywee You can do it similar to how it's implemented for Option<T>:

impl<T: Export> Export for Vec<T> {
    type Hint = <T as Export>::Hint;
    #[inline]
    fn export_info(hint: Option<Self::Hint>) -> ExportInfo {
        let inner = T::export_info(hint);
        let hint_string = format!("{}/{}:{}", inner.variant_type, inner.hint_kind, inner.hint_string);
        unimplemented!("make ExportInfo out of new hint_string")
    }
}

You can also add a type-erasing ArrayHint for exporting VariantArrays (breaking change, but a minor version is planned to happen soon anyway, so it's not an important distinction):

struct ArrayHint { /* .. */ }
impl ArrayHint {
    pub fn new<T: Export>(hint: Option<<T as Export>::Hint>) -> Self { /* .. */ }
    fn godot_hint_string(&self) -> GodotString { /* .. */ }
}

impl<T: Export> Export for Vec<T> {
    type Hint = <T as Export>::Hint;
    #[inline]
    fn export_info(hint: Option<Self::Hint>) -> ExportInfo {
        let hint_string = ArrayHint::new(hint).godot_hint_string();
        unimplemented!("make ExportInfo out of new hint_string")
    }
}

impl Export for VariantArray<Shared> {
    // Breaking change because this was `()`
    type Hint = ArrayHint;
    #[inline]
    fn export_info(hint: Option<Self::Hint>) -> ExportInfo {
        hint.map_or_else(|| unimplemented!("default), |hint| {
            let hint_string = hint.godot_hint_string();
            unimplemented!("make ExportInfo out of new hint_string")
        })
    }
}

[Taywee]

The ArrayHint approach seems like it should work well enough, and should nest fine, to boot. Adding an impl for Vec is nice enough (and probably the first place most developers should reach), but allowing to do it with VariantArray might be preferable for people who want more manual control of their setters. I'm not versed enough in nativescript yet to be as useful in this work and discussion as I'd like to be, though, so these are my loose uneducated thoughts. I might take an experimental stab at this work if nobody else does soon.

I also wonder how Dictionary type hints will be handled eventually.

Edit: And also, thank you for your feedback. It's wonderfully refreshing to have this level of responsiveness in an issue tracker.

I'm not versed enough in nativescript yet to be as useful in this work and discussion as I'd like to be, though, so these are my loose uneducated thoughts. I might take an experimental stab at this work if nobody else does soon.

You've done very good research here! Implementing Export and a hint type for a new type is actually quite easy when you know how to format the hint strings. You don't have to touch the C API directly. A PR would be very welcome!

(That's said, it's also fine if you'd prefer to wait. It might take a bit of time before I get around to it, though.)

[Taywee]

I took a quick exploratory poke at it with #639 tonight. I haven't tested it at all yet, and am not even sure yet if it compiles, I just wanted it into an early place where it can be reviewed for potential suitability since I'm not proceeding with testing until tomorrow.

[Taywee]

And one quick last thought on the design of Export for consideration during future major changes, since the current design doesn't work completely naturally with Array's nested hinting. Currently (in my PR draft), it is:

/// Trait for exportable types.
pub trait Export: ToVariant {
    /// A type-specific hint type that is valid for the type being exported.
    type Hint: Hint;

    /// Returns `ExportInfo` given an optional typed hint.
    fn export_info(hint: Option<Self::Hint>) -> ExportInfo;
}

But with something like the Hint trait, I wonder if this might be better expressed as something like:

pub trait Export<T: Hint>: ToVariant {
    // this method or trait might even be unnecessary, due to Hint::export_info
    fn export_info(hint: Option<T>) -> ExportInfo;
}

Which would allow (if desired) splitting up those big IntHint enums and just implementing them individually, and would allow ArrayHint to be parameterized, to allow nesting as something like ArrayHint<ArrayHint<RangeHint<i64>>> for quite natural hint building and possibly avoiding the Box in ArrayHint.

For default hint, maybe give an implementation for impl Hint for () that's used meaningfully for default (returning an empty ExportInfo with 0 types and a blank hint string), or some trait DefaultHint { type Hint: Hint } or something. I'm not sure exactly how it routes from PropertyBuilder through to the final exportinfo in that case. Maybe store the PropertyBuilder::with_hint ExportInfo directly instead of invoking it on done.

This talk might be better suited for a different channel, like the discord, though.

pub trait Export<T: Hint>: ToVariant {
    // this method or trait might even be unnecessary, due to Hint::export_info
    fn export_info(hint: Option<T>) -> ExportInfo;
}

This is not good because we want hints to be correct by construction, so you can't use a hint type that is invalid for the type being exported, e.g. an enum hint for a resource property. At least you want Export<T: Hint<Self>> (if that would compile), but that may make type inference harder and is just a lot more complicated.

This talk might be better suited for a different channel, like the discord, though.

Yes. I suggest that you join the godot-rust Discord server, where discussions can be carried out much more quickly before implementation, so we end up with less wasted effort due to disagreements in design.