Add #[export_group] Attribute for Creating Property Groups

[gg-yb]

This makes Godot's property groups available to exported properties in Rust.

On the itest side there may be possibilities to further test the success of creating the property, right now itest only uses the attribute, i.e. checks that code using it compiles. However, I am unsure how to properly query this in a way like HasProperty is tested.

Feedback is more than welcome!

Without this PR

With this PR

[Bromeon]

Thanks a lot for this pull request! 🙂

We should probably consider how we represent the variety of GDScript's @export_* annotations in gdext. Your current proposed syntax is this one (at least according to the test):

#[export_group(name = "My group", prefix = "my_")]
#[export]
my_number: i32,

#[export]
my_string: GodotString,

Does that first #[export_group] statement carry over to my_string as well, like in GDScript? If yes, we should change that -- it's very unintuitive in Rust (and I think it's not great in GDScript either, they should have added a block or at least indentation).

I don't think a separate #[export_group] annotation is needed -- it can be part of #[export]. Having it separate suggests that it can be used independently, which is not the case -- you always need #[export] in addition to #[export_group].

---

Furthermore, we had such a feature request a year ago in gdnative, and I added some comments there:
godot-rust/gdnative#855 (comment)

Regarding syntax suggestions, it may be helpful to define "group properties" (name, prefix) in one place and then refer to them from other places. There are multiple ways to achieve that. Two are outlined in gdnative#855, but there are more:

1. Require a separate #[derive(ExportGroup)] struct, per group.

Quite verbose and non-local.
However, it might be similar in syntax to other ideas like sub-tree projections (godot-rust/gdnative#980).

#[derive(ExportGroup)]
#[group(name="Offset", prefix="offset_")]
struct OffsetGroup {
    #[export]
    x: f32,

    #[export]
    y: f32,
}

#[derive(GodotClass)]
#[class(init, base=Node)]
struct MyNode {
    #[export(group)]
    offsets: OffsetGroup,
}

2. Allow definition in the parent struct, but define the group somewhere outside.

Slightly non-local, but type-safe. Renaming a group makes sure all references are updated.

#[derive(ExportGroup)]
#[group(name="Offset", prefix="offset_")]
struct OffsetGroup; // just a tag

#[derive(GodotClass)]
#[class(init, base=Node)]
struct MyNode {
    #[export(group = OffsetGroup)]
    offset_x: f32,

    #[export(group = OffsetGroup)]
    offset_y: f32,
}

3. Allow definition in the parent struct, and define the group as a struct attribute.

String-based, but could still be verified at proc-macro evaluation time.
Might need multiple #[group] attributes (something our proc-macro parser doesn't yet support -- it assumes a map).

#[derive(GodotClass)]
#[class(init, base=Node)]
#[group(name="Offset", prefix="offset_")]
struct MyNode {
    #[export(group = "Offset")]
    offset_x: f32,

    #[export(group = "Offset")]
    offset_y: f32,
}

[Bromeon]

Also, we don't need to find the perfect, super-complex design right now. We can start experimenting and then iteratively improve based on user feedback.

[Bromeon]

Any reason why these variables are renamed here?
Probably easier to understand if the same name is used as on the call site.

[Bromeon]

Also, the extracting into function makes it a bit hard to see changes 🙈
But I agree with splitting it 👍

[gg-yb]

No reason but being lazy. Will change the name back.

Re: extracting
Yup, hard to see, but a separate commit would have been overkill I think.

[Bromeon]

This doesn't test the group association -- is there a way to check that programmatically at all?

[gg-yb]

That's what I do not (yet) know. Will experiment a bit with Gdscript property groups and introspection and then revisit this issue.

[RealFloorIsJava]

I really like option 1 you mentioned: It feels like the natural thing to do, as property groups are intended to keep struct-like data close together. The non-locality in this case could even be an advantage, as common property groups are reusable that way. Think of how we can right now edit Vectors in a concise way in the editor as single properties instead of having an "ABC x" and "ABC y" property each time we wanna use a vector. There's a lot of potential here.

This last point wouldn't really be viable with options 2 and 3, which treat property groups not as grouped data, but as relations on data of a larger, encompassing struct. So I am definitely in favor of 1.

[RealFloorIsJava]

Expanding on that: As those reusable structs may be more than exports, I am in favor of naming and deriving the trait something like "ExportGroupable", with helper attributes for prefix and name.

[gg-yb]

Thank you for your review, you're right, my proposed export_group syntax has a bunch of shortcomings.
However, it's a long bank holiday weekend here, so I will only be able to come back to this issue next Tuesday - maybe until then a better syntax for the immediate future has been decided upon ;)
Once I'm back I will explore syntax alternatives and keep you all posted.

[gg-yb]

So, after thinking about this, and looking into gdnative, which I hadn't used before, I think jumping into implementation for more than checking feasibility of a syntax option at this stage is premature. For this reason I will close this PR for now and create an issue instead, where we can develop the idea more. Once a solid, informed choice on the syntax has been made, we can then move on with getting this into gdext.