Integrate with gdrust macro for easy property and signal exports

[wyattjsmith1]

I wrote gdrust not thinking it would fit in well here, but it turns out it would. This ticket is for discussing how gdrust fits into this project.

Properties

Properties in gdrust use a very gdscript-like syntax:

#[gdrust(extends = Node)]
#[derive(Debug)]
struct HelloWorld {
    #[export_range(0.0, 10.0)]
    simple_range: f32,

    #[export_range(0, 10, 10, "or_greater")]
    #[default(10)]
    simple_range_step_or_greater: u64,

    #[export_enum("This", "will", "be", "enum", "ordinals")]
    int_enum: u32,

    #[export_file("*.png")]
    png_file: String,
}

In general, I think we should try to keep this syntax as it maps well to Godot's syntax. I discussed some syntax ideas with @toasteater on discord, and there are a few things we should keep in mind:

• This should ideally be merged with #[property]
• We should try to retain some functionality of #[property], namely the path and accessor hooks:

#[property(path = "foo/bar", after_set = "Self::_update_stuff")]
foo_bar: i32

One difference between gdrust and the #[property] syntax is the way other information such as default and getters and setters are defined. #[property] favors grouping them all in one attribute like the example above, whereas gdrust tries to split them into multiple attributes. The reason gdrust splits them is that godot's export documentation defines an "annotation-style" syntax. Export hint information is added as annotation arguments. default and getters/setters were placed in separate attributes to prevent interfering with godot's syntax. As a result, there are more lines of code, but it is easier to read and we aren't cramming everything in one big attribute. We don't have to bring this to gdnative-rust, but it is worth considering.

Things to consider

• Should fields be exported by default? Probably not.
• Should we put all the property information in one attribute, or split it out into many (export_*, default, path, setter, getter, etc.)?
• Will this replace property, or is there still a need for property?
• Should "or_greater" and "or_lesser" not be strings? Parsing a list of Lits is easy, but there's no reason they couldn't be keywords: #[export_range(0, 10, 10, or_lesser, or_greater)].
• gdrust requires all properties to have a default value. This allows us to generate a new method. This doesn't work in 100% of cases, but I've always found the new method annoying, so I added it.

Signals

Signals in gdrust are defined with an attribute on a struct:

#[gdrust(extends = Node)]
#[signal(my_signal(arg1: F64, arg2: GodotString = "test".to_string()))]
#[signal(simple_signal(arg:I64))]
#[derive(Debug)]
struct HelloWorld;

The syntax was chosen to look like gdscript. You may be wondering why the types are weird; they are required to be (VariantTypes)[https://docs.rs/gdnative/0.9.3/gdnative/core_types/enum.VariantType.html]. I suppose this is not a hard requirement, but it makes it much easier to export the signal as it requires a VariantType instead of a generic.

Things to consider

• Should we change the VariantType to a rust type?
• Is the syntax good?
• gdrust exports a const with the signal name. This is good for ensuring anyone emitting on the signal is emitting with the correct name at compile time, but it is a bit of magic.
• Is there a better place for this attribute than on the struct? It works, and seems to be the best place, but there may be a better option.

unsafe_functions

A collection of extension functions that make coding easier at the risk of crashing the program. I wasn't asked to include this, but I'm throwing it in for clarity. It sounds like the unsafe nature of these functions means they should not be a part of this library and should maybe live in gdrust. Let me know if there are other opinions.

[ghost]

Regarding unsafe_functions: we now have a module gdnative::api::utils for helper functions that depend on generated bindings, but yes they do need to be correctly marked as unsafe if necessary. We already have something similar to expect_node in NodeExt::get_node_as, albeit correctly unsafe and without the final expect. An expect version can be nice to have, too.

[phenax]

+1. Need this!