Merge #198

198: Add an Export trait to use in autogenerated getters r=Bromeon a=ttencate Fixes #197. Note that some types are still not supported, since they're not implemented yet in gdext: `Aabb`, `Plane`, `Rect2` and `Rect2i`. `Signal` and `Callable` don't implement `Export` either. Probably it makes no sense for `Signal`, and should eventually be supported for `Callable`. Co-authored-by: Thomas ten Cate <[email protected]>
godot-rust · Mar 22, 2023 · eb30a8b · eb30a8b
2 parents dbce0d9 + d52c826
commit eb30a8b
Show file tree

Hide file tree

Showing 9 changed files with 109 additions and 13 deletions.
diff --git a/godot-core/src/builtin/array.rs b/godot-core/src/builtin/array.rs
@@ -8,7 +8,7 @@ use godot_ffi as sys;
 
 use crate::builtin::meta::VariantMetadata;
 use crate::builtin::*;
-use crate::obj::Share;
+use crate::obj::{Export, Share};
 use std::fmt;
 use std::marker::PhantomData;
 use sys::{ffi_methods, interface_fn, GodotFfi};
@@ -601,6 +601,12 @@ impl<T: VariantMetadata> Share for Array<T> {
     }
 }
 
+impl<T: VariantMetadata> Export for Array<T> {
+    fn export(&self) -> Self {
+        self.share()
+    }
+}
+
 impl<T: VariantMetadata> Default for Array<T> {
     #[inline]
     fn default() -> Self {

diff --git a/godot-core/src/builtin/dictionary.rs b/godot-core/src/builtin/dictionary.rs
@@ -7,7 +7,7 @@
 use godot_ffi as sys;
 
 use crate::builtin::{inner, FromVariant, ToVariant, Variant};
-use crate::obj::Share;
+use crate::obj::{Export, Share};
 use std::fmt;
 use std::marker::PhantomData;
 use std::ptr::addr_of_mut;
@@ -280,6 +280,12 @@ impl Share for Dictionary {
     }
 }
 
+impl Export for Dictionary {
+    fn export(&self) -> Self {
+        self.share()
+    }
+}
+
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 // Conversion traits
 

diff --git a/godot-core/src/builtin/mod.rs b/godot-core/src/builtin/mod.rs
@@ -318,3 +318,70 @@ macro_rules! real {
         f
     }};
 }
+
+// ----------------------------------------------------------------------------------------------------------------------------------------------
+
+/// Implementations of the `Export` trait for types where it can be done trivially.
+mod export {
+    use crate::builtin::*;
+    use crate::obj::Export;
+
+    macro_rules! impl_export_by_clone {
+        ($ty:path) => {
+            impl Export for $ty {
+                fn export(&self) -> Self {
+                    // If `Self` does not implement `Clone`, this gives a clearer error message
+                    // than simply `self.clone()`.
+                    Clone::clone(self)
+                }
+            }
+        };
+    }
+
+    impl_export_by_clone!(bool);
+    impl_export_by_clone!(isize);
+    impl_export_by_clone!(usize);
+    impl_export_by_clone!(i8);
+    impl_export_by_clone!(i16);
+    impl_export_by_clone!(i32);
+    impl_export_by_clone!(i64);
+    impl_export_by_clone!(u8);
+    impl_export_by_clone!(u16);
+    impl_export_by_clone!(u32);
+    impl_export_by_clone!(u64);
+    impl_export_by_clone!(f32);
+    impl_export_by_clone!(f64);
+
+    // impl_export_by_clone!(Aabb); // TODO uncomment once Aabb implements Clone
+    impl_export_by_clone!(Basis);
+    impl_export_by_clone!(Color);
+    impl_export_by_clone!(GodotString);
+    impl_export_by_clone!(NodePath);
+    impl_export_by_clone!(PackedByteArray);
+    impl_export_by_clone!(PackedColorArray);
+    impl_export_by_clone!(PackedFloat32Array);
+    impl_export_by_clone!(PackedFloat64Array);
+    impl_export_by_clone!(PackedInt32Array);
+    impl_export_by_clone!(PackedInt64Array);
+    impl_export_by_clone!(PackedStringArray);
+    impl_export_by_clone!(PackedVector2Array);
+    impl_export_by_clone!(PackedVector3Array);
+    // impl_export_by_clone!(Plane); // TODO uncomment once Plane implements Clone
+    impl_export_by_clone!(Projection);
+    impl_export_by_clone!(Quaternion);
+    // impl_export_by_clone!(Rect2); // TODO uncomment once Rect2 implements Clone
+    // impl_export_by_clone!(Rect2i); // TODO uncomment once Rect2i implements Clone
+    impl_export_by_clone!(Rid);
+    impl_export_by_clone!(StringName);
+    impl_export_by_clone!(Transform2D);
+    impl_export_by_clone!(Transform3D);
+    impl_export_by_clone!(Vector2);
+    impl_export_by_clone!(Vector2i);
+    impl_export_by_clone!(Vector3);
+    impl_export_by_clone!(Vector3i);
+    impl_export_by_clone!(Vector4);
+
+    // TODO investigate whether these should impl Export at all, and if so, how
+    // impl_export_by_clone!(Callable);
+    // impl_export_by_clone!(Signal);
+}
diff --git a/godot-core/src/obj/gd.rs b/godot-core/src/obj/gd.rs
@@ -18,7 +18,7 @@ use crate::builtin::meta::{ClassName, VariantMetadata};
 use crate::builtin::{FromVariant, ToVariant, Variant, VariantConversionError};
 use crate::obj::dom::Domain as _;
 use crate::obj::mem::Memory as _;
-use crate::obj::{cap, dom, mem, GodotClass, Inherits, Share};
+use crate::obj::{cap, dom, mem, Export, GodotClass, Inherits, Share};
 use crate::obj::{GdMut, GdRef, InstanceId};
 use crate::storage::InstanceStorage;
 use crate::{callbacks, engine, out};
@@ -588,6 +588,12 @@ impl<T: GodotClass> Share for Gd<T> {
     }
 }
 
+impl<T: GodotClass> Export for Gd<T> {
+    fn export(&self) -> Self {
+        self.share()
+    }
+}
+
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 // Trait impls
 

diff --git a/godot-core/src/obj/traits.rs b/godot-core/src/obj/traits.rs
@@ -52,6 +52,14 @@ pub trait Share {
     fn share(&self) -> Self;
 }
 
+/// Trait implemented for types that can be used as `#[export]` fields. This creates a copy of the
+/// value, for some type-specific definition of "copy". For example, `Array` and `Gd` are returned
+/// via `Share::share()` instead of copying the actual data.
+pub trait Export {
+    /// Creates a copy to be returned from a getter.
+    fn export(&self) -> Self;
+}
+
 /// Non-strict inheritance relationship in the Godot class hierarchy.
 ///
 /// `Derived: Inherits<Base>` means that either `Derived` is a subclass of `Base`, or the class `Base` itself (hence "non-strict").

diff --git a/godot-macros/src/derive_godot_class.rs b/godot-macros/src/derive_godot_class.rs
@@ -332,7 +332,7 @@ fn make_exports_impl(class_name: &Ident, fields: &Fields) -> TokenStream {
                 };
                 getter_setter_impls.push(quote! {
                     pub #signature {
-                        self.#field_ident
+                        ::godot::obj::Export::export(&self.#field_ident)
                     }
                 });
                 export_tokens.push(quote! {

diff --git a/godot/src/lib.rs b/godot/src/lib.rs
@@ -136,7 +136,7 @@ pub mod prelude {
     };
     pub use super::init::{gdextension, ExtensionLayer, ExtensionLibrary, InitHandle, InitLevel};
     pub use super::log::*;
-    pub use super::obj::{Base, Gd, GdMut, GdRef, GodotClass, Inherits, InstanceId, Share};
+    pub use super::obj::{Base, Export, Gd, GdMut, GdRef, GodotClass, Inherits, InstanceId, Share};
 
     // Make trait methods available
     pub use super::engine::NodeExt as _;

diff --git a/itest/godot/ManualFfiTests.gd b/itest/godot/ManualFfiTests.gd
@@ -55,10 +55,10 @@ func test_export():
 	assert_eq(obj.object_val, node)
 
 	var texture_val_meta = obj.get_property_list().filter(
-		func(el): return el["name"] == "texture_val"
+		func(el): return el["name"] == "texture_val_rw"
 	).front()
 
-	assert_that(texture_val_meta != null, "'texture_val' is defined")
+	assert_that(texture_val_meta != null, "'texture_val_rw' is defined")
 	assert_eq(texture_val_meta["hint"], PropertyHint.PROPERTY_HINT_RESOURCE_TYPE)
 	assert_eq(texture_val_meta["hint_string"], "Texture")
 

diff --git a/itest/rust/src/export_test.rs b/itest/rust/src/export_test.rs
@@ -31,8 +31,10 @@ struct HasProperty {
     string_val: GodotString,
     #[export(get = "get_object_val", set = "set_object_val")]
     object_val: Option<Gd<Object>>,
+    #[export]
+    texture_val: Gd<Texture>,
     #[export(get = "get_texture_val", set = "set_texture_val", hint = PROPERTY_HINT_RESOURCE_TYPE, hint_desc = "Texture")]
-    texture_val: Option<Gd<Texture>>,
+    texture_val_rw: Option<Gd<Texture>>,
 }
 
 #[godot_api]
@@ -98,17 +100,17 @@ impl HasProperty {
     }
 
     #[func]
-    pub fn get_texture_val(&self) -> Variant {
-        if let Some(texture_val) = self.texture_val.as_ref() {
+    pub fn get_texture_val_rw(&self) -> Variant {
+        if let Some(texture_val) = self.texture_val_rw.as_ref() {
             texture_val.to_variant()
         } else {
             Variant::nil()
         }
     }
 
     #[func]
-    pub fn set_texture_val(&mut self, val: Gd<Texture>) {
-        self.texture_val = Some(val);
+    pub fn set_texture_val_rw(&mut self, val: Gd<Texture>) {
+        self.texture_val_rw = Some(val);
     }
 }
 
@@ -124,7 +126,8 @@ impl NodeVirtual for HasProperty {
             int_val_setter: 0,
             object_val: None,
             string_val: GodotString::new(),
-            texture_val: None,
+            texture_val: Texture::new(),
+            texture_val_rw: None,
             base,
         }
     }