`#[export]` fails to compile for `Gd` types

[KevinThierauf]

The following code fails to compile:

#[derive(GodotClass)]
#[class(base = Node)]
pub struct ExampleNode {
    #[base]
    base: Base<Node>,
    #[export]
    material: Gd<Material>,
}

#[godot_api]
impl ExampleNode {}

#[godot_api]
impl NodeVirtual for ExampleNode {
    fn init(base: Base<Self::Base>) -> Self {
        todo!()
    }
}

Using #[export] compiles fine for other types (e.g. int) but when using Gd the following error is provided: error[E0507]: cannot move out of self.material which is behind a shared reference --> src\example.rs:4:10	23	#[derive(GodotClass)]	^^^^^^^^^^ move occurs because self.material has type godot::prelude::Gd<Material>, which does not implement the Copy trait

= note: this error originates in the derive macro GodotClass (in Nightly builds, run with -Z macro-backtrace for more info)

[ttencate]

I was aware of this while writing the code, but thanks for reporting because now we have a good place to discuss it :)

The autogenerated getters cannot move out of the field, so they need some way to create a copy of the value. For types that implement Copy, this happens automatically. For other types we need to generate code that makes a copy explicitly; these types are at least Gd, Array and Dictionary, but might include Signal and Callable too (would need to check).

Note that derive macro doesn't know the type of the field, only the type's path name in the current scope. In particular, it cannot know whether Copy, Clone or some other trait is supported on that type (unless we match on a fixed list of type names, but this is brittle). Therefore, we need some common trait that's implemented by all types that can be returned from an autogenerated getter.

The standard Rust way to do that is, of course, the Clone trait.

The objection to implementing Clone on Gd is, that users might confuse this with cloning the value that the Gd points to. But there is precedent in the standard library: Rc and Arc implement such a Clone operation, and explicitly recommend that you call it as Rc::clone(&foo) rather than foo.clone() for this very reason. In a similar discussion on gdnative, there were no serious objections to implementing Clone for their Ref type (rough equivalent of Gd in gdext).

The two other types that act as references are Array and Dictionary. Having a Clone implementation that just copies a reference is... unfortunate. But we can document this clearly, and if people are familiar with these types in GDScript, they might already be aware of the fact that it doesn't behave like a native Rust type. One interesting alternative that came up previously is to wrap these types in a sort of smart pointer of their own, e.g. Ref<Array>. That would allow Ref::clone(&array) for explicit cloning of the reference, at the cost of making these types a bit more verbose.

The alternative to Clone is that we create our own trait, let's say Gettable.

Because trait specialization is not yet stable, Gettable would have to be implemented for all core types. This could be done with a macro. For value types it would just invoke Copy or Clone; for reference types it would call the current implementation of Share::share() instead. It's not a lot of work to implement or maintain, but it's yet another concept exposed in our public API that needs to be explained.

[Bromeon]

Great writeup!

But we can document this clearly, and if people are familiar with these types in GDScript, they might already be aware of the fact that it doesn't behave like a native Rust type.

I think that's the key takeaway. One way to see Clone is: it does what GDScript does with = or argument passing. If it's by-reference, then Clone will not do a deep-copy, either.

Apart from that, we're not yet sure if Clone is the correct abstraction (gdnative has Export for it, with supertrait ToVariant, but the exporting mechanism is quite different). It might help keep things simple though, without the need for dedicated Export or Share traits...

[ttencate]

I sent a PR with the Export trait, formerly known as Gettable (#198). But I have a branch that replaces Share by Clone as well.

Having our own trait makes it a bit more controllable, perhaps? For example, Signal might eventually implement Clone but it should not be #[export]ed (we have #[signal] for that).

[Bromeon]

(Relabeled as feature, because part of #[export] was not yet implemented. Export is that feature.)