Add macro for manually registering properties

[you-win]

Adds a new struct-level macro for manually registering properties.

This is primarily useful for cases like this:

// Struct containing some shared data not exposed to Godot
struct Foo {
    data: Whatever
}

#[derive(GodotClass)]
#[property(name = data, type = i32, get = get_data)]
struct Bar {
    foo: Foo
}

... impl RefCountedVirtual ...

#[godot_api]
impl Bar {
    #[func]
    fn get_data(&self) -> i32 {
        self.foo.data.some_func()
    }
}

The macro syntax was chosen to (kind of) mirror how godot-cpp handles property registration.

TODO:

• Usage flags and field hints aren't implemented yet
• Automatically generated getters/setters ignore this, brain was confused. This is not possible since manually registered properties are not associated with any struct field

For the above todos, I think it should be possible to pass all manually registered properties as FieldVars to reduce code duplication as the logic is mostly the same. I will need to investigate further.

Refactored to treat manually registered properties as FieldVars. It might be worth it to rename some types since a #[property] is not actually associated with any struct fields, thus the field name and type need to be explicitly provided.

[GodotRust]

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-400

[lilizoey]

Couldn't we do this with something like this (we'd need to add a type field to the #[var] attribute):

// Struct containing some shared data not exposed to Godot
struct Foo {
    data: Whatever
}

#[derive(GodotClass)]
struct Bar {
    foo: Foo,
    #[var(type = i32, get = get_data)]
    data: (),
}

... impl RefCountedVirtual ...

#[godot_api]
impl Bar {
    #[func]
    fn get_data(&self) -> i32 {
        self.foo.data.some_func()
    }
}

[you-win]

That's a good point. I didn't consider that a field with a unit type could be used.

I'm still in favor of the #[property] attribute because:

1. I think it will make #[var] parsing more complicated because of reasons given in the next section
2. #[property] makes it obvious that the field exists on the type purely on the Godot side
3. I already wrote the feature, so I'm biased

---

The following constraints would need to be followed for unit types:

• automatically generated getters/setters would need to be disallowed
• type = <value> would need to be required

The following question would need to be answered:

• should type = <value> be allowed for struct fields with an existing type?

#[var(type = Vector2, get = get_my_int)]
my_int: i32

Finally, purely subjectively for my use case:

• I would like to potentially register the same methods multiple times under different names (readonly variable, r/w private variable)
• I would prefer to not have many struct fields with a unit type, since I also interact with these structs from Rust

Real WIP example from my project:

#[derive(Debug, GodotClass)]
#[class(base = Node3D)]
#[property(name = head_bone, type = GodotString, get = get_head_bone, set = set_head_bone)]
#[property(
    name = additional_movement_bones,
    type = Array<i32>, get = get_additional_movement_bones,
    set = set_additional_movement_bones
)] // Fixed in latest squashed commit
#[property(name = blink_threshold, type = f32, get = get_blink_threshold, set = set_blink_threshold)]
pub struct VrmPuppet {
    puppet3d: model::puppet::Puppet3d,
    vrm_puppet: model::puppet::VrmPuppet,

    ... omitted for brevity ...
}

... getters/setters impl ...

[lilizoey]

That's a good point. I didn't consider that a field with a unit type could be used.

I'm still in favor of the #[property] attribute because:

1. I think it will make `#[var]` parsing more complicated because of reasons given in the next section

I dont think we need to add anything other than supporting type = <value>

2. `#[property]` makes it obvious that the field exists on the type purely on the Godot side

I mean if it's a field in godot it usually makes sense to also have it be a field in rust. Otherwise you can just manually write a getter/setter for use in both rust and godot.

3. I already wrote the feature, so I'm biased

understandable :p

The following constraints would need to be followed for unit types:

* automatically generated getters/setters would need to be disallowed

Pretty sure this is already the case. () doesn't implement Property so you can't auto-generate a setter/getter, you'll get a type error.

* `type = <value>` would need to be required

Will also already be the case if we add support for type = <value>, because () doesn't implement Property it can't know what type to use without manually specifying it.

The following question would need to be answered:

* should `type = <value>` be allowed for struct fields with an existing type?

#[var(type = Vector2, get = get_my_int)]
my_int: i32

Probably for consistency

Finally, purely subjectively for my use case:

* I would like to potentially register the same methods multiple times under different names (readonly variable, r/w private variable)

This is already possible.

* I would prefer to not have many struct fields with a unit type, since I also interact with these structs from Rust

You dont have to use a unit type, you can use a field of any type you want.

Real WIP example from my project:

#[derive(Debug, GodotClass)]
#[class(base = Node3D)]
#[property(name = head_bone, type = GodotString, get = get_head_bone, set = set_head_bone)]
#[property(
    name = additional_movement_bones,
    type = Array<i32>,
    get = get_additional_movement_bones,
    set = set_additional_movement_bones,
)]
#[property(name = blink_threshold, type = f32, get = get_blink_threshold, set = set_blink_threshold)]
pub struct VrmPuppet {
    puppet3d: model::puppet::Puppet3d,
    vrm_puppet: model::puppet::VrmPuppet,

    ... omitted for brevity ...
}

... getters/setters impl ...

Are you here basically exposing fields internal to Puppet3d and VrmPuppet to godot? Im not sure if manually registering properties using this macro would then be the best way of doing that. Honestly manually registering properties should probably be done in the builder-api when it's made instead imo.

[you-win]

Pretty sure this is already the case. () doesn't implement Property so you can't auto-generate a setter/getter, you'll get a type error.

This would simplify things a lot.

Finally, purely subjectively for my use case:

• I would like to potentially register the same methods multiple times under different names (readonly variable, r/w private variable)

This is already possible.

Would it need multiple struct fields that use the same getters/setters?

If so, my PR removes the need for multiple redundant fields.

• I would prefer to not have many struct fields with a unit type, since I also interact with these structs from Rust

You dont have to use a unit type, you can use a field of any type you want.

My point was more along the lines of "I don't want redundant fields in my struct". I'm composing Godot structs out of smaller data structs.

---

To explain my example further, I have multiple types: GlbPuppet, VrmPuppet, PngPuppet, etc. Each type has common fields + common functionality, which I'm solving via data structs and traits (e.g. model::puppet::Puppet3D, trait Puppet3D, etc). I need fields exposed to Godot to make the Godot GUI -> Rust flow easier.

They are not just private fields, it would be clunky to re-expose the inner fields on the outer struct, and registering every data struct with Godot is not a great solution since these structs are just data.

[lilizoey]

To explain my example further, I have multiple types: GlbPuppet, VrmPuppet, PngPuppet, etc. Each type has common fields + common functionality, which I'm solving via data structs and traits (e.g. model::puppet::Puppet3D, trait Puppet3D, etc). I need fields exposed to Godot to make the Godot GUI -> Rust flow easier.

They are not just private fields, it would be clunky to re-expose the inner fields on the outer struct, and registering every data struct with Godot is not a great solution since these structs are just data.

i mean that sounds like the usecase for Resource in godot. is there a reason you can't just make these structs inherit from Resource and then expose them directly through an export?

[you-win]

In my experience, I've found Godot Resources to be easily breakable + can contain arbitrary code, so all my data is stored in alternative formats. The example was just to showcase that I have an actual use case for this PR.

Either way, using Resource or not using Resource, this PR adds a feature that godot-cpp has and gdext currently only has via a workaround.

[lilizoey]

In my experience, I've found Godot Resources to be easily breakable + can contain arbitrary code, so all my data is stored in alternative formats. The example was just to showcase that I have an actual use case for this PR.

that's fair, i havent really had the same issue with resource since 4.0 came out and made them so much nicer to use in the editor.

Either way, using Resource or not using Resource, this PR adds a feature that godot-cpp has and gdext currently only has via a workaround.

i do agree it should exist, but i dont know if we should add another macro solution to this problem when there exists a workaround and we're already planning on making a builder api that will support this much more nicely. the current implementation also just seems kinda noisy to me.

[you-win]

I'm open to syntax changes. I just want an escape hatch for what I want to do.

Would reusing the #[class] annotation like this be better?

#[class(
    init,
    property = (name = foo, type = i32, get = get_foo, set = set_foo)
)]

or this (I'm not sure if KvParser::handle_list -> ListParser::pop_next -> KvValue::into_tokens -> ListParser::new_from_tree can handle this)

#[class(
    init,
    properties = (
        (name = foo, type = i32, get = get_foo, set = set_foo),
        (name = bar, ... etc ...)
    )
)]

or potentially shorter syntax for #[property]? I'm not sure if this will work with the current KvParser.

#[property(blink_threshold: f32, get = get_blink_threshold, set = set_blink_threshold)]

[you-win]

Pushed a new commit that leaves this PR with 2 options for manual property registration:

1. Standalone #[property] macro

#[property(name = readonly_int, type = i32, get = get_integer)]

2. Reusing #[class] macro

#[class(
    properties = [
        (name = first_int, type = i32, get = get_integer),
        (name = second_int, type = i32, get = get_integer)
    ]
)]

Both examples taken from property_test.rs.

My preference is still for option 1. because the implementation is cleaner.

[Bromeon]

Without reading the whole discussion, why not Property<T> like in gdnative?

Class-based attributes seem a bit off for something that's semantically a property (field). It also means that you cannot use Rust's type system and have to resort to proc-macro magic instead.

[you-win]

I have Godot structs composed out of pure data structs. The pure data structs cannot be bound to Godot as a Resource.

The Godot structs already have many other implementation-specific fields on them, so I don't want to add extra fields just to bind them to Godot. On the Rust side, they are self.common.foo. On the Godot side, I want these to look like self.foo.

What I'm essentially looking for is this function from GDNative's ClassBuilder which is a feature that's also present in godot-cpp.

I will note that type = <ty> is actually checked by Rust to be a valid type.

[Bromeon]

What I'm essentially looking for is this function from GDNative's ClassBuilder which is a feature that's also present in godot-cpp.

But then that would rather be something to make part of the builder API (#4).
Essentially, I imagine it to look something like this:

impl NodeVirtual for MyClass {
    fn register_class(builder: &mut ClassBuilder) {
        builder.add_property("my_thing")
            .getter(get_thing)
            .setter(set_thing)
            .done();
    }
}

The API you suggest was the one gdext started with, and we moved towards field-based attributes, as that feels much more natural. And that is currently the API to register properties -- so we should either expand on that one (Property<T>) or address it differently (builders), but not write a competing attribute on a class level.

Furthermore, I don't think everything provided in a future builder API should additionally be available in a tense proc-macro form. The proc-macro API is for convenience in common cases, but it is a custom, difficult-to-discover DSL with manual validation and compile-time impacts.

I'm aware that the builder API is not yet there, but to me, the solution to that problem is not to fill the proc-macro API with builder functionality as a workaround.

[Bromeon]

As a general remark, I would recommend to discuss larger changes in an issue first, as mentioned in the Contribution Guidelines. Otherwise it can happen that a lot of effort is spent on something that is not directly mergeable. If you're fine with that risk and like to try out different approaches as a proof-of-concept, that's of course OK 🙂

[you-win]

Okay, I will close this PR then. Thanks for considering it.

[Bromeon]

To be clear, I'm not against having this as a feature, but I think it could be provided in a slightly different API.

In gdnative, we had provided custom properties in two ways:

1. the builder API (as you mentioned yourself)
2. the Property<T> ZST which could delegate to custom getters/setters

What is your opinion on those two approaches?

[you-win]

I'm for option 1, the builder API. Option 2 leads to (in my opinion) redundant struct fields that need to be maintained along with their associated getters/setters + the actual struct field wherever it might be.

However, option 1 looks like it will involve a lot of work to implement, whereas option 2 could honestly just use this suggestion from earlier in the thread to achieve the same result.