Re-organize type-level attributes in NativeClass derive macro

[chitoyuu]

The NativeClass macro currently utilizes a large number of type-level attributes, making it tricky to add further expansions to its functionality. The current attributes are:

• inherit
• user_data
• register_with
• no_constructor

... while in the future, we might also like to add:

• Custom type parameter bounds for generics types (Derive NativeClass for known monomorphizations of generic types #601)
• Custom static type name (Rename #[derive(NativeClass)] types #603)

Proposal

My proposal is as follows:

• inherit is to be left as a top-level attribute, because of its significance.
• All other attributes are to be bundled into a single #[nativescript], as key-value pairs when applicable: #[nativescript(user_data = "path::to::Type<Self>", register_with = "path::to::function", no_constructor)]

Under this new arrangement, generic bounds and static renames can easily be introduced as keys under the #[nativescript] attribute: #[nativescript(bound = "T: SomeBound", rename = "SomeNewName")]. Prior art for this could be found in the ecosystem, for example, in Serde.

Unresolved questions

It's a common complaint that the signature of the mandatory new is too verbose, the owner argument being seldom used. However, it isn't obvious how this is best dealt with. Our problem here is the need to distinguish three different semantics at the syntax level:

• Calling a function, possibly a custom one, with no arguments.
• Calling a function, possibly a custom one, with a single owner argument.
• Having no default constructor.

I can see two alternatives to the current situation:

Changing the default constructor to the nullary Default::default, requiring a constructor key for unary constructors

Under this arrangement, the possible annotations are:

• Nothing. Default::default is called to produce a new instance by default.
• #[nativescript(constructor = "Self::new")]. Self::new is called with the owner argument to produce a new instance. This is consistent with the current behavior.
• #[nativescript(no_constructor)]. The NativeScript type would have no default constructor.
• All other cases would become compile errors.

This makes it much less verbose to declare types with constructors that do not use the owner argument, and frees up the new identifier unless requested specifically by the user. The downside is that extra annotation is required to reproduce the current default.

Replacing no_constructor with a nested key-value pair under the constructor meta

Under this arrangement, the possible annotations are:

• #[nativescript(constructor(nullary = "Self::default"))]. Self::default is called with no arguments to produce a new instance.
• #[nativescript(constructor(unary = "Self::new"))]. Self::new is called with the owner argument to produce a new instance.
• #[nativescript(constructor(none))]. The NativeScript type would have no default constructor.
• All other cases would become compile errors.

This has the benefit of allowing custom functions for both the nullary case and the unary case, but is very verbose. It should be noted that Default can always be implemented for types with nullary constructors, even though it could be undesirable if the constructor in question is expensive, or if there are multiple such functions.

[chitoyuu]

Should be considered for #842 if #603 is desirable in 0.10.

[Bromeon]

Thank you for the detailed proposal! 🙂

---

All other attributes are to be bundled into a single #[nativescript] [...]

Since we have #[derive(NativeClass)], wouldn't it be more natural if the attribute were #[nativeclass] or #[native_class] instead of #[nativescript]?

Maybe in the future with GDExtensions, we could even consider something more general. Just brainstorming, e.g. #[derive(GodotClass)] and #[godot(...)].

---

Changing the default constructor to the nullary Default::default, requiring a constructor key for unary constructors

I like this idea. We could benefit of the rusty "default constructor idiom".

I think one limitation is that the derive-macro for NativeClass already needs to know how its constructor looks, no? Because otherwise, an alternative might be something like:

#[methods]
impl MyClass {
    #[constructor]
    fn new() -> Self { ... }

    // or:

    #[constructor(owner)]
    fn new(owner: &Node) -> Self { ... }
}

[chitoyuu]

Maybe in the future with GDExtensions, we could even consider something more general. Just brainstorming, e.g. #[derive(GodotClass)] and #[godot(...)].

It's my opinion that we should instead keep NativeScript and GDExtension classes distinct on the name level, since their behavior differ in many ways, e.g. godotengine/godot#54999. An easy s/NativeClass/GdExtension/g migration might not be possible even if all the external APIs are kept exactly the same, and as such I don't think we should make our identifiers imply that.

I think one limitation is that the derive-macro for NativeClass already needs to know how its constructor looks, no?

Currently yes. I think it can be moved to NativeClassMethods instead, but NativeClassMethods as a whole is just a kludge around the lack of global discovery (#350) -- ideally we want to be able to have an arbitrary number of #[methods] blocks, and don't want that trait to exist at all. In that case, it would become a run-time error to have multiple constructors declared for a single type.

That would have to be left for 0.11 anyway, though, since #350 isn't in the scope for the 0.10 milestone, so I think it would also be acceptable to move it into NativeClassMethods for now.

[Bromeon]

It's my opinion that we should instead keep NativeScript and GDExtension classes distinct on the name level, since their behavior differ in many ways [...]

I see, makes sense.

What about #[nativeclass] or #[native_class], when we have #[derive(NativeClass)]? I.e. instead of #[nativescript].

I'm pretty sure most godot-rust users can't tell the difference between the two terms, so differing here might add more confusion than benefit. Especially since there's also a rather unrelated NativeScript class.

That would have to be left for 0.11 anyway, though, since #350 isn't in the scope for the 0.10 milestone, so I think it would also be acceptable to move it into NativeClassMethods for now.

Okay, let's revisit this later then. There are quite a few pending improvements related to exporting, so some of them could be bundled for v0.11.

[chitoyuu]

What about #[nativeclass] or #[native_class], when we have #[derive(NativeClass)]? I.e. instead of #[nativescript].

I suppose #[native_class] is fine, if we aren't renaming NativeClass to something else. You are right that renaming it to NativeScript outright would be confusing. Sometimes I forget that the Godot NativeScript class existed.

[Bromeon]

Moving to 0.11 milestone, to avoid blocking the 0.10 release, along with #601.
I plan to have the next minor release not so far in the future, so it's less of a "now or only in 1 year" problem.