diff --git a/godot-core/src/builtin/array.rs b/godot-core/src/builtin/array.rs index f6f2ac568..755fa0b79 100644 --- a/godot-core/src/builtin/array.rs +++ b/godot-core/src/builtin/array.rs @@ -762,14 +762,12 @@ impl TypeStringHint for VariantArray { } impl Var for Array { - type Intermediate = Self; - - fn get_property(&self) -> Self::Intermediate { - self.clone() + fn get_property(&self) -> Self::Via { + self.to_godot() } - fn set_property(&mut self, value: Self::Intermediate) { - *self = value; + fn set_property(&mut self, value: Self::Via) { + *self = FromGodot::from_godot(value) } #[cfg(since_api = "4.2")] diff --git a/godot-core/src/builtin/dictionary.rs b/godot-core/src/builtin/dictionary.rs index c99852a7e..a5b54b821 100644 --- a/godot-core/src/builtin/dictionary.rs +++ b/godot-core/src/builtin/dictionary.rs @@ -341,14 +341,12 @@ impl Clone for Dictionary { } impl Var for Dictionary { - type Intermediate = Self; - - fn get_property(&self) -> Self::Intermediate { - self.clone() + fn get_property(&self) -> Self::Via { + self.to_godot() } - fn set_property(&mut self, value: Self::Intermediate) { - *self = value; + fn set_property(&mut self, value: Self::Via) { + *self = FromGodot::from_godot(value) } } diff --git a/godot-core/src/obj/gd.rs b/godot-core/src/obj/gd.rs index 5d0ffdde4..4baea9969 100644 --- a/godot-core/src/obj/gd.rs +++ b/godot-core/src/obj/gd.rs @@ -656,15 +656,15 @@ impl TypeStringHint for Gd { } } +// TODO: Do we even want to implement `Var` and `Export` for `Gd`? You basically always want to use `Option>` because the editor +// may otherwise try to set the object to a null value. impl Var for Gd { - type Intermediate = Self; - - fn get_property(&self) -> Self { - self.clone() + fn get_property(&self) -> Self::Via { + self.to_godot() } - fn set_property(&mut self, value: Self) { - *self = value; + fn set_property(&mut self, value: Self::Via) { + *self = FromGodot::from_godot(value) } } diff --git a/godot-core/src/obj/onready.rs b/godot-core/src/obj/onready.rs index 35ae04013..d77f96b97 100644 --- a/godot-core/src/obj/onready.rs +++ b/godot-core/src/obj/onready.rs @@ -5,6 +5,7 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ +use crate::builtin::meta::GodotConvert; use crate::property::{PropertyHintInfo, Var}; use std::mem; @@ -190,15 +191,17 @@ impl std::ops::DerefMut for OnReady { } } -impl Var for OnReady { - type Intermediate = T::Intermediate; +impl GodotConvert for OnReady { + type Via = T::Via; +} - fn get_property(&self) -> Self::Intermediate { +impl Var for OnReady { + fn get_property(&self) -> Self::Via { let deref: &T = self; deref.get_property() } - fn set_property(&mut self, value: Self::Intermediate) { + fn set_property(&mut self, value: Self::Via) { let deref: &mut T = self; deref.set_property(value); } diff --git a/godot-core/src/property.rs b/godot-core/src/property.rs index ee40b2ba3..9d751fd94 100644 --- a/godot-core/src/property.rs +++ b/godot-core/src/property.rs @@ -7,6 +7,7 @@ //! Registration support for property types. +use crate::builtin::meta::{FromGodot, GodotConvert, ToGodot}; use crate::builtin::GString; use crate::engine::global::PropertyHint; @@ -17,11 +18,12 @@ use crate::engine::global::PropertyHint; /// /// This creates a copy of the value, according to copy semantics provided by `Clone`. For example, `Array`, `Dictionary` and `Gd` are /// returned by shared reference instead of copying the actual data. -pub trait Var { - type Intermediate; - - fn get_property(&self) -> Self::Intermediate; - fn set_property(&mut self, value: Self::Intermediate); +/// +/// This does not require [`FromGodot`] or [`ToGodot`], so that something can be used as a property even if it can't be used in function +/// arguments/return types. +pub trait Var: GodotConvert { + fn get_property(&self) -> Self::Via; + fn set_property(&mut self, value: Self::Via); fn property_hint() -> PropertyHintInfo { PropertyHintInfo::with_hint_none("") @@ -59,21 +61,20 @@ impl TypeStringHint for Option { impl Var for Option where - T: Var + From<::Intermediate>, + T: Var + FromGodot, + Option: GodotConvert>, { - type Intermediate = Option; - - fn get_property(&self) -> Self::Intermediate { + fn get_property(&self) -> Self::Via { self.as_ref().map(Var::get_property) } - fn set_property(&mut self, value: Self::Intermediate) { + fn set_property(&mut self, value: Self::Via) { match value { Some(value) => { if let Some(current_value) = self { current_value.set_property(value) } else { - *self = Some(T::from(value)) + *self = Some(FromGodot::from_godot(value)) } } None => *self = None, @@ -83,7 +84,8 @@ where impl Export for Option where - T: Export + From<::Intermediate>, + T: Export, + Option: Var, { fn default_export_info() -> PropertyHintInfo { T::default_export_info() @@ -314,39 +316,26 @@ mod export_impls { use crate::builtin::*; use godot_ffi as sys; - macro_rules! impl_property_by_clone { - ($Ty:ty => $variant_type:ident, no_export) => { - impl_property_by_clone!(@property $Ty => $variant_type); - impl_property_by_clone!(@type_string_hint $Ty, $variant_type); - }; - - ($Ty:ty => $variant_type:ident, no_export; $type_string_name:ident) => { - impl_property_by_clone!(@property $Ty => $variant_type); - impl_property_by_clone!(@type_string_hint $Ty, $type_string_name); - }; - - ($Ty:ty => $variant_type:ident) => { - impl_property_by_clone!(@property $Ty => $variant_type); - impl_property_by_clone!(@export $Ty); - impl_property_by_clone!(@type_string_hint $Ty, $variant_type); + macro_rules! impl_property_by_godot_convert { + ($Ty:ty, no_export) => { + impl_property_by_godot_convert!(@property $Ty); + impl_property_by_godot_convert!(@type_string_hint $Ty); }; - ($Ty:ty => $variant_type:ident; $type_string_name:ident) => { - impl_property_by_clone!(@property $Ty => $variant_type); - impl_property_by_clone!(@export $Ty); - impl_property_by_clone!(@type_string_hint $Ty, $type_string_name); + ($Ty:ty) => { + impl_property_by_godot_convert!(@property $Ty); + impl_property_by_godot_convert!(@export $Ty); + impl_property_by_godot_convert!(@type_string_hint $Ty); }; - (@property $Ty:ty => $variant_type:ident) => { + (@property $Ty:ty) => { impl Var for $Ty { - type Intermediate = Self; - - fn get_property(&self) -> Self { - self.clone() + fn get_property(&self) -> Self::Via { + self.to_godot() } - fn set_property(&mut self, value: Self) { - *self = value; + fn set_property(&mut self, value: Self::Via) { + *self = FromGodot::from_godot(value); } } }; @@ -359,82 +348,83 @@ mod export_impls { } }; - (@type_string_hint $Ty:ty, $type_string_name:ident) => { + (@type_string_hint $Ty:ty) => { impl TypeStringHint for $Ty { fn type_string() -> String { use sys::GodotFfi; let variant_type = <$Ty as $crate::builtin::meta::GodotType>::Ffi::variant_type(); - format!("{}:{}", variant_type as i32, stringify!($type_string_name)) + let type_name = <$Ty as $crate::builtin::meta::GodotType>::godot_type_name(); + format!("{}:{}", variant_type as i32, type_name) } } } } // Bounding Boxes - impl_property_by_clone!(Aabb => Aabb; AABB); - impl_property_by_clone!(Rect2 => Rect2); - impl_property_by_clone!(Rect2i => Rect2i); + impl_property_by_godot_convert!(Aabb); + impl_property_by_godot_convert!(Rect2); + impl_property_by_godot_convert!(Rect2i); // Matrices - impl_property_by_clone!(Basis => Basis); - impl_property_by_clone!(Transform2D => Transform2D); - impl_property_by_clone!(Transform3D => Transform3D); - impl_property_by_clone!(Projection => Projection); + impl_property_by_godot_convert!(Basis); + impl_property_by_godot_convert!(Transform2D); + impl_property_by_godot_convert!(Transform3D); + impl_property_by_godot_convert!(Projection); // Vectors - impl_property_by_clone!(Vector2 => Vector2); - impl_property_by_clone!(Vector2i => Vector2i); - impl_property_by_clone!(Vector3 => Vector3); - impl_property_by_clone!(Vector3i => Vector3i); - impl_property_by_clone!(Vector4 => Vector4); - impl_property_by_clone!(Vector4i => Vector4i); + impl_property_by_godot_convert!(Vector2); + impl_property_by_godot_convert!(Vector2i); + impl_property_by_godot_convert!(Vector3); + impl_property_by_godot_convert!(Vector3i); + impl_property_by_godot_convert!(Vector4); + impl_property_by_godot_convert!(Vector4i); // Misc Math - impl_property_by_clone!(Quaternion => Quaternion); - impl_property_by_clone!(Plane => Plane); + impl_property_by_godot_convert!(Quaternion); + impl_property_by_godot_convert!(Plane); // Stringy Types - impl_property_by_clone!(GString => String); - impl_property_by_clone!(StringName => StringName); - impl_property_by_clone!(NodePath => NodePath); + impl_property_by_godot_convert!(GString); + impl_property_by_godot_convert!(StringName); + impl_property_by_godot_convert!(NodePath); - impl_property_by_clone!(Color => Color); + impl_property_by_godot_convert!(Color); // Arrays - impl_property_by_clone!(PackedByteArray => PackedByteArray); - impl_property_by_clone!(PackedInt32Array => PackedInt32Array); - impl_property_by_clone!(PackedInt64Array => PackedInt64Array); - impl_property_by_clone!(PackedFloat32Array => PackedFloat32Array); - impl_property_by_clone!(PackedFloat64Array => PackedFloat64Array); - impl_property_by_clone!(PackedStringArray => PackedStringArray); - impl_property_by_clone!(PackedVector2Array => PackedVector2Array); - impl_property_by_clone!(PackedVector3Array => PackedVector3Array); - impl_property_by_clone!(PackedColorArray => PackedColorArray); + impl_property_by_godot_convert!(PackedByteArray); + impl_property_by_godot_convert!(PackedInt32Array); + impl_property_by_godot_convert!(PackedInt64Array); + impl_property_by_godot_convert!(PackedFloat32Array); + impl_property_by_godot_convert!(PackedFloat64Array); + impl_property_by_godot_convert!(PackedStringArray); + impl_property_by_godot_convert!(PackedVector2Array); + impl_property_by_godot_convert!(PackedVector3Array); + impl_property_by_godot_convert!(PackedColorArray); // Primitives - impl_property_by_clone!(f64 => Float; float); - impl_property_by_clone!(i64 => Int; int); - impl_property_by_clone!(bool => Bool; bool); + impl_property_by_godot_convert!(f64); + impl_property_by_godot_convert!(i64); + impl_property_by_godot_convert!(bool); // Godot uses f64 internally for floats, and if Godot tries to pass an invalid f32 into a rust property // then the property will just round the value or become inf. - impl_property_by_clone!(f32 => Float; float); + impl_property_by_godot_convert!(f32); // Godot uses i64 internally for integers, and if Godot tries to pass an invalid integer into a property // accepting one of the below values then rust will panic. In the editor this will appear as the property // failing to be set to a value and an error printed in the console. During runtime this will crash the // program and print the panic from rust stating that the property cannot store the value. - impl_property_by_clone!(i32 => Int; int); - impl_property_by_clone!(i16 => Int; int); - impl_property_by_clone!(i8 => Int; int); - impl_property_by_clone!(u32 => Int; int); - impl_property_by_clone!(u16 => Int; int); - impl_property_by_clone!(u8 => Int; int); + impl_property_by_godot_convert!(i32); + impl_property_by_godot_convert!(i16); + impl_property_by_godot_convert!(i8); + impl_property_by_godot_convert!(u32); + impl_property_by_godot_convert!(u16); + impl_property_by_godot_convert!(u8); // Callables and Signals are useless when exported to the editor, so we only need to make them available as // properties. - impl_property_by_clone!(Callable => Callable, no_export); - impl_property_by_clone!(Signal => Signal, no_export); + impl_property_by_godot_convert!(Callable, no_export); + impl_property_by_godot_convert!(Signal, no_export); // RIDs when exported act slightly weird. They are largely read-only, however you can reset them to their // default value. This seems to me very unintuitive. Since if we are storing an RID we would likely not @@ -443,7 +433,7 @@ mod export_impls { // // Additionally, RIDs aren't persistent, and can sometimes behave a bit weirdly when passed from the // editor to the runtime. - impl_property_by_clone!(Rid => Rid, no_export; RID); + impl_property_by_godot_convert!(Rid, no_export); - // impl_property_by_clone!(Signal => Signal); + // impl_property_by_godot_convert!(Signal); } diff --git a/godot-macros/src/class/data_models/field_var.rs b/godot-macros/src/class/data_models/field_var.rs index 02bce7973..7923514db 100644 --- a/godot-macros/src/class/data_models/field_var.rs +++ b/godot-macros/src/class/data_models/field_var.rs @@ -168,7 +168,7 @@ impl GetterSetterImpl { match kind { GetSet::Get => { signature = quote! { - fn #function_name(&self) -> <#field_type as ::godot::register::property::Var>::Intermediate + fn #function_name(&self) -> <#field_type as ::godot::builtin::meta::GodotConvert>::Via }; function_body = quote! { <#field_type as ::godot::register::property::Var>::get_property(&self.#field_name) @@ -176,7 +176,7 @@ impl GetterSetterImpl { } GetSet::Set => { signature = quote! { - fn #function_name(&mut self, #field_name: <#field_type as ::godot::register::property::Var>::Intermediate) + fn #function_name(&mut self, #field_name: <#field_type as ::godot::builtin::meta::GodotConvert>::Via) }; function_body = quote! { <#field_type as ::godot::register::property::Var>::set_property(&mut self.#field_name, #field_name); diff --git a/godot-macros/src/derive/derive_godot_convert.rs b/godot-macros/src/derive/derive_godot_convert.rs index 7d04af69e..b0aad029f 100644 --- a/godot-macros/src/derive/derive_godot_convert.rs +++ b/godot-macros/src/derive/derive_godot_convert.rs @@ -9,7 +9,7 @@ use proc_macro2::TokenStream; use quote::quote; use venial::Declaration; -use crate::util::{decl_get_info, DeclInfo}; +use crate::util::{decl_get_info, via_type, DeclInfo}; use crate::ParseResult; pub fn derive_godot_convert(decl: Declaration) -> ParseResult { @@ -22,9 +22,11 @@ pub fn derive_godot_convert(decl: Declaration) -> ParseResult { let gen = generic_params.as_ref().map(|x| x.as_inline_args()); + let via_type = via_type(&decl)?; + Ok(quote! { impl #generic_params ::godot::builtin::meta::GodotConvert for #name #gen #where_ { - type Via = ::godot::builtin::Variant; + type Via = #via_type; } }) } diff --git a/godot-macros/src/derive/derive_var.rs b/godot-macros/src/derive/derive_var.rs index 135ec59c9..469954beb 100644 --- a/godot-macros/src/derive/derive_var.rs +++ b/godot-macros/src/derive/derive_var.rs @@ -6,10 +6,10 @@ */ use proc_macro2::TokenStream as TokenStream2; -use quote::{quote, ToTokens}; +use quote::quote; use venial::{Declaration, StructFields}; -use crate::util::{bail, decl_get_info, ident, DeclInfo}; +use crate::util::{bail, decl_get_info, via_type, DeclInfo}; use crate::ParseResult; pub fn derive_var(decl: Declaration) -> ParseResult { @@ -19,7 +19,7 @@ pub fn derive_var(decl: Declaration) -> ParseResult { let body_get; let body_set; - let intermediate; + let via_type = via_type(&decl)?; let enum_ = match decl { Declaration::Enum(e) => e, @@ -40,18 +40,6 @@ pub fn derive_var(decl: Declaration) -> ParseResult { } else { let mut matches_get = quote! {}; let mut matches_set = quote! {}; - intermediate = if let Some(attr) = enum_ - .attributes - .iter() - .find(|attr| attr.get_single_path_segment() == Some(&ident("repr"))) - { - attr.value.to_token_stream() - } else { - return bail!( - name, - "Property can only be derived on enums with an explicit `#[repr(i*/u*)]` type" - ); - }; for (enum_v, _) in enum_.variants.inner.iter() { let v_name = enum_v.name.clone(); @@ -99,7 +87,7 @@ pub fn derive_var(decl: Declaration) -> ParseResult { body_set = quote! { *self = match value { #matches_set - _ => panic!("Incorrect conversion from {} to {}", stringify!(#intermediate), #name_string), + _ => panic!("Incorrect conversion from {} to {}", stringify!(#via_type), #name_string), } }; } @@ -107,13 +95,11 @@ pub fn derive_var(decl: Declaration) -> ParseResult { let out = quote! { #[allow(unused_parens)] impl godot::register::property::Var for #name { - type Intermediate = #intermediate; - - fn get_property(&self) -> #intermediate { + fn get_property(&self) -> #via_type { #body_get } - fn set_property(&mut self, value: #intermediate) { + fn set_property(&mut self, value: #via_type) { #body_set } } diff --git a/godot-macros/src/lib.rs b/godot-macros/src/lib.rs index df3a8eb65..9f5128053 100644 --- a/godot-macros/src/lib.rs +++ b/godot-macros/src/lib.rs @@ -601,6 +601,8 @@ pub fn derive_from_godot(input: TokenStream) -> TokenStream { /// Derive macro for [`Var`](../register/property/trait.Var.html) on enums. /// +/// This also requires deriving `GodotConvert`. +/// /// Currently has some tight requirements which are expected to be softened as implementation expands: /// - Only works for enums, structs aren't supported by this derive macro at the moment. /// - The enum must have an explicit `#[repr(u*/i*)]` type. @@ -612,7 +614,7 @@ pub fn derive_from_godot(input: TokenStream) -> TokenStream { /// /// ```no_run /// # use godot::prelude::*; -/// #[derive(Var)] +/// #[derive(Var, GodotConvert)] /// #[repr(i32)] /// # #[derive(Eq, PartialEq, Debug)] /// enum MyEnum { diff --git a/godot-macros/src/util/mod.rs b/godot-macros/src/util/mod.rs index 1e6ac7ebd..3f89f6744 100644 --- a/godot-macros/src/util/mod.rs +++ b/godot-macros/src/util/mod.rs @@ -35,12 +35,12 @@ pub fn class_name_obj(class: &impl ToTokens) -> TokenStream { pub fn property_variant_type(property_type: &impl ToTokens) -> TokenStream { let property_type = property_type.to_token_stream(); - quote! { <<<#property_type as ::godot::register::property::Var>::Intermediate as ::godot::builtin::meta::GodotConvert>::Via as ::godot::builtin::meta::GodotType>::Ffi::variant_type() } + quote! { <<#property_type as ::godot::builtin::meta::GodotConvert>::Via as ::godot::builtin::meta::GodotType>::Ffi::variant_type() } } pub fn property_variant_class_name(property_type: &impl ToTokens) -> TokenStream { let property_type = property_type.to_token_stream(); - quote! { <<<#property_type as ::godot::register::property::Var>::Intermediate as ::godot::builtin::meta::GodotConvert>::Via as ::godot::builtin::meta::GodotType>::class_name() } + quote! { <<#property_type as ::godot::builtin::meta::GodotConvert>::Via as ::godot::builtin::meta::GodotType>::class_name() } } pub fn bail_fn(msg: impl AsRef, tokens: T) -> ParseResult @@ -288,3 +288,74 @@ pub fn make_virtual_tool_check() -> TokenStream { } } } + +pub enum ViaType { + Struct, + EnumWithRepr { int_ty: Ident }, + Enum, +} + +impl ToTokens for ViaType { + fn to_tokens(&self, tokens: &mut TokenStream) { + match self { + ViaType::Struct | ViaType::Enum => { + quote! { ::godot::builtin::Variant }.to_tokens(tokens) + } + ViaType::EnumWithRepr { int_ty } => int_ty.to_tokens(tokens), + } + } +} + +pub fn via_type(declaration: &venial::Declaration) -> ParseResult { + use venial::Declaration; + + match declaration { + Declaration::Enum(enum_) => enum_repr(enum_), + Declaration::Struct(_) => Ok(ViaType::Struct), + other => bail!( + other, + "cannot get via type for {:?}, only structs and enums are supported currently", + other.name() + ), + } +} + +pub fn enum_repr(enum_: &venial::Enum) -> ParseResult { + let Some(repr) = enum_ + .attributes + .iter() + .find(|attr| attr.get_single_path_segment() == Some(&ident("repr"))) + else { + return Ok(ViaType::Enum); + }; + + let venial::AttributeValue::Group(_, repr_value) = &repr.value else { + // `repr` is always going to look like `#[repr(..)]` + unreachable!() + }; + + let Some(repr_type) = repr_value.first() else { + // `#[repr()]` is just a warning apparently, so we're gonna give an error if that's provided. + return bail!(&repr.value, "expected non-empty `repr` list"); + }; + + let TokenTree::Ident(repr_type) = &repr_type else { + // all valid non-empty `#[repr(..)]` will have an ident as its first element. + unreachable!(); + }; + + if !matches!( + repr_type.to_string().as_str(), + "i8" | "u8" | "i16" | "u16" | "i32" | "u32" | "i64" + ) { + return bail!( + &repr_type, + "enum with repr #[repr({})] cannot implement `GodotConvert`, repr must be one of: i8, i16, i32, i64, u8, u16, u32", + repr_type.to_string(), + ); + } + + Ok(ViaType::EnumWithRepr { + int_ty: repr_type.clone(), + }) +} diff --git a/itest/rust/src/object_tests/property_test.rs b/itest/rust/src/object_tests/property_test.rs index 32adfeb51..814a113bb 100644 --- a/itest/rust/src/object_tests/property_test.rs +++ b/itest/rust/src/object_tests/property_test.rs @@ -5,15 +5,14 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ -use godot::{ - engine::{ - global::{PropertyHint, PropertyUsageFlags}, - Texture, - }, - prelude::*, - register::property::PropertyHintInfo, - test::itest, -}; +use godot::builtin::meta::{GodotConvert, ToGodot}; +use godot::builtin::{dict, Color, Dictionary, GString, Variant, VariantType}; +use godot::engine::global::{PropertyHint, PropertyUsageFlags}; +use godot::engine::{INode, IRefCounted, Node, Object, RefCounted, Texture}; +use godot::obj::{Base, EngineBitfield, EngineEnum, Gd, NewAlloc, NewGd}; +use godot::register::property::{Export, PropertyHintInfo, Var}; +use godot::register::{godot_api, Export, GodotClass, GodotConvert, Var}; +use godot::test::itest; // No tests currently, tests using these classes are in Godot scripts. @@ -155,14 +154,16 @@ enum SomeCStyleEnum { C = 2, } -impl Var for SomeCStyleEnum { - type Intermediate = i64; +impl GodotConvert for SomeCStyleEnum { + type Via = i64; +} - fn get_property(&self) -> Self::Intermediate { +impl Var for SomeCStyleEnum { + fn get_property(&self) -> Self::Via { (*self) as i64 } - fn set_property(&mut self, value: Self::Intermediate) { + fn set_property(&mut self, value: Self::Via) { match value { 0 => *self = Self::A, 1 => *self = Self::B, @@ -187,17 +188,19 @@ struct NotExportable { b: i64, } -impl Var for NotExportable { - type Intermediate = Dictionary; +impl GodotConvert for NotExportable { + type Via = Dictionary; +} - fn get_property(&self) -> Self::Intermediate { +impl Var for NotExportable { + fn get_property(&self) -> Self::Via { dict! { "a": self.a, "b": self.b } } - fn set_property(&mut self, value: Self::Intermediate) { + fn set_property(&mut self, value: Self::Via) { let a = value.get("a").unwrap().to::(); let b = value.get("b").unwrap().to::(); @@ -300,8 +303,8 @@ struct CheckAllExports { color_no_alpha: Color, } +#[derive(GodotConvert, Var, Export, Eq, PartialEq, Debug)] #[repr(i64)] -#[derive(Var, Export, Eq, PartialEq, Debug)] pub enum TestEnum { A = 0, B = 1,