diff --git a/godot-codegen/src/generator/enums.rs b/godot-codegen/src/generator/enums.rs index 528bb4495..96ed8a3d4 100644 --- a/godot-codegen/src/generator/enums.rs +++ b/godot-codegen/src/generator/enums.rs @@ -115,16 +115,16 @@ pub fn make_enum_definition_with( let index_enum_impl = make_enum_index_impl(enum_); let bitwise_impls = make_enum_bitwise_operators(enum_, enum_bitmask.as_ref()); - let var_trait_set_property = if enum_.is_exhaustive { + let var_trait_set = if enum_.is_exhaustive { quote! { - fn set_property(&mut self, value: Self::Via) { - *self = ::from_ord(value); + fn var_set(field: &mut Self, value: Self::Via) { + *field = ::from_ord(value); } } } else { quote! { - fn set_property(&mut self, value: Self::Via) { - self.ord = value; + fn var_set(field: &mut Self, value: Self::Via) { + field.ord = value; } } }; @@ -156,14 +156,24 @@ pub fn make_enum_definition_with( } impl crate::registry::property::Var for #name { - fn get_property(&self) -> Self::Via{ - ::ord(*self) + type PubType = Self; + + fn var_get(field: &Self) -> Self::Via { + ::ord(*field) } - #var_trait_set_property + #var_trait_set + + fn var_pub_get(field: &Self) -> Self::PubType { + *field + } + + fn var_pub_set(field: &mut Self, value: Self::PubType) { + *field = value; + } - fn var_hint() -> crate::meta::PropertyHintInfo{ - crate::meta::PropertyHintInfo{ + fn var_hint() -> crate::meta::PropertyHintInfo { + crate::meta::PropertyHintInfo { hint: #property_hint, hint_string: crate::builtin::GString::from(#enum_hint_string), } diff --git a/godot-core/src/builtin/collections/any_array.rs b/godot-core/src/builtin/collections/any_array.rs index 0b7c19b4e..d73c84747 100644 --- a/godot-core/src/builtin/collections/any_array.rs +++ b/godot-core/src/builtin/collections/any_array.rs @@ -17,6 +17,7 @@ use crate::meta::error::ConvertError; use crate::meta::{ ArrayElement, ElementType, FromGodot, GodotConvert, GodotFfiVariant, GodotType, ToGodot, }; +use crate::registry::property::SimpleVar; /// Covariant `Array` that can be either typed or untyped. /// @@ -511,6 +512,8 @@ impl FromGodot for AnyArray { } } +impl SimpleVar for AnyArray {} + impl fmt::Debug for AnyArray { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { self.array.fmt(f) diff --git a/godot-core/src/builtin/collections/array.rs b/godot-core/src/builtin/collections/array.rs index 372db1d1e..9fcc78c16 100644 --- a/godot-core/src/builtin/collections/array.rs +++ b/godot-core/src/builtin/collections/array.rs @@ -1205,12 +1205,22 @@ impl Clone for Array { } impl Var for Array { - fn get_property(&self) -> Self::Via { - self.to_godot_owned() + type PubType = Self; + + fn var_get(field: &Self) -> Self::Via { + field.to_godot_owned() + } + + fn var_set(field: &mut Self, value: Self::Via) { + *field = FromGodot::from_godot(value); + } + + fn var_pub_get(field: &Self) -> Self::PubType { + field.clone() } - fn set_property(&mut self, value: Self::Via) { - *self = FromGodot::from_godot(value) + fn var_pub_set(field: &mut Self, value: Self::PubType) { + *field = value; } fn var_hint() -> PropertyHintInfo { diff --git a/godot-core/src/builtin/collections/packed_array.rs b/godot-core/src/builtin/collections/packed_array.rs index 04650dacd..45615ea77 100644 --- a/godot-core/src/builtin/collections/packed_array.rs +++ b/godot-core/src/builtin/collections/packed_array.rs @@ -22,7 +22,7 @@ use crate::meta; use crate::meta::signed_range::SignedRange; use crate::meta::{AsArg, FromGodot, GodotConvert, PackedArrayElement, ToGodot}; use crate::obj::EngineEnum; -use crate::registry::property::{Export, Var}; +use crate::registry::property::{Export, SimpleVar}; // Many builtin types don't have a #[repr] themselves, but they are used in packed arrays, which assumes certain size and alignment. // This is mostly a problem for as_slice(), which reinterprets the FFI representation into the "frontend" type like GString. @@ -601,15 +601,7 @@ impl ops::IndexMut for PackedArray { // ---------------------------------------------------------------------------------------------------------------------------------------------- // Property trait impls -impl Var for PackedArray { - fn get_property(&self) -> Self::Via { - ToGodot::to_godot_owned(self) - } - - fn set_property(&mut self, value: Self::Via) { - *self = FromGodot::from_godot(value); - } -} +impl SimpleVar for PackedArray {} impl Export for PackedArray { fn export_hint() -> meta::PropertyHintInfo { diff --git a/godot-core/src/obj/dyn_gd.rs b/godot-core/src/obj/dyn_gd.rs index 80f7b2847..f2819ef60 100644 --- a/godot-core/src/obj/dyn_gd.rs +++ b/godot-core/src/obj/dyn_gd.rs @@ -658,13 +658,23 @@ where T: GodotClass, D: ?Sized + 'static, { - fn get_property(&self) -> Self::Via { - self.obj.get_property() + type PubType = Self; + + fn var_get(field: &Self) -> Self::Via { + as Var>::var_get(&field.obj) + } + + fn var_set(field: &mut Self, value: Self::Via) { + // `var_set` can't be delegated to Gd, since we have to set `erased_obj` as well. + *field = ::from_godot(value); + } + + fn var_pub_get(field: &Self) -> Self::PubType { + field.clone() } - fn set_property(&mut self, value: Self::Via) { - // `set_property` can't be delegated to Gd, since we have to set `erased_obj` as well. - *self = ::from_godot(value); + fn var_pub_set(field: &mut Self, value: Self::PubType) { + *field = value; } } @@ -707,13 +717,25 @@ where T: GodotClass, D: ?Sized + 'static, { - fn get_property(&self) -> Self::Via { - Self::get_property_inner(self) + // Not Option<...> -- accessing from Rust through Var trait should not expose larger API than OnEditor itself. + type PubType = as GodotConvert>::Via; + + fn var_get(field: &Self) -> Self::Via { + Self::get_property_inner(field) + } + + fn var_set(field: &mut Self, value: Self::Via) { + // `var_set` can't be delegated to Gd, since we have to set `erased_obj` as well. + Self::set_property_inner(field, value); + } + + fn var_pub_get(field: &Self) -> Self::PubType { + Self::var_get(field) + .expect("generated #[var(pub)] getter: uninitialized OnEditor>") } - fn set_property(&mut self, value: Self::Via) { - // `set_property` can't be delegated to Gd, since we have to set `erased_obj` as well. - Self::set_property_inner(self, value) + fn var_pub_set(field: &mut Self, value: Self::PubType) { + Self::var_set(field, Some(value)) } } diff --git a/godot-core/src/obj/gd.rs b/godot-core/src/obj/gd.rs index f66b0fe1e..31eed82c4 100644 --- a/godot-core/src/obj/gd.rs +++ b/godot-core/src/obj/gd.rs @@ -24,7 +24,7 @@ use crate::obj::{ }; use crate::private::{callbacks, PanicPayload}; use crate::registry::class::try_dynify_object; -use crate::registry::property::{object_export_element_type_string, Export, Var}; +use crate::registry::property::{object_export_element_type_string, Export, SimpleVar, Var}; use crate::{classes, meta, out}; /// Smart pointer to objects owned by the Godot engine. @@ -1045,15 +1045,7 @@ impl Clone for Gd { } } -impl Var for Gd { - fn get_property(&self) -> Self::Via { - self.to_godot_owned() - } - - fn set_property(&mut self, value: Self::Via) { - *self = FromGodot::from_godot(value) - } -} +impl SimpleVar for Gd {} /// See [`Gd` Exporting](struct.Gd.html#exporting) section. impl Export for Option> @@ -1089,12 +1081,23 @@ impl Var for OnEditor> where T: GodotClass, { - fn get_property(&self) -> Self::Via { - Self::get_property_inner(self) + // Not Option<...> -- accessing from Rust through Var trait should not expose larger API than OnEditor itself. + type PubType = as GodotConvert>::Via; + + fn var_get(field: &Self) -> Self::Via { + Self::get_property_inner(field) + } + + fn var_set(field: &mut Self, value: Self::Via) { + Self::set_property_inner(field, value); + } + + fn var_pub_get(field: &Self) -> Self::PubType { + Self::var_get(field).expect("generated #[var(pub)] getter: uninitialized OnEditor>") } - fn set_property(&mut self, value: Self::Via) { - Self::set_property_inner(self, value) + fn var_pub_set(field: &mut Self, value: Self::PubType) { + Self::var_set(field, Some(value)) } } diff --git a/godot-core/src/obj/instance_id.rs b/godot-core/src/obj/instance_id.rs index 6e80531a2..57716abd5 100644 --- a/godot-core/src/obj/instance_id.rs +++ b/godot-core/src/obj/instance_id.rs @@ -10,6 +10,7 @@ use std::num::NonZeroU64; use crate::meta::error::{ConvertError, FromGodotError}; use crate::meta::{FromGodot, GodotConvert, ToGodot}; +use crate::registry::property::SimpleVar; /// Represents a non-zero instance ID. /// @@ -107,3 +108,5 @@ impl FromGodot for InstanceId { Self::try_from_i64(via).ok_or_else(|| FromGodotError::ZeroInstanceId.into_error(via)) } } + +impl SimpleVar for InstanceId {} diff --git a/godot-core/src/obj/on_editor.rs b/godot-core/src/obj/on_editor.rs index 6fdcac04f..63539bef8 100644 --- a/godot-core/src/obj/on_editor.rs +++ b/godot-core/src/obj/on_editor.rs @@ -16,8 +16,7 @@ use crate::registry::property::{BuiltinExport, Export, Var}; /// instead. As a general "maybe initialized" type, `Option>` is always available, even if more verbose. /// /// -/// # What consitutes "initialized"? -/// +/// # What constitutes "initialized"? /// Whether a value is considered initialized or not depends on `T`. /// /// - For objects, a value is initialized if it is not null. Exported object propreties in Godot are nullable, but `Gd` and `DynGd` do @@ -28,7 +27,6 @@ use crate::registry::property::{BuiltinExport, Export, Var}; /// More on this below (see also table-of-contents sidebar). /// /// # Initialization semantics -/// /// Panics during access (`Deref/DerefMut` impls) if uninitialized. /// /// When used inside a node class, `OnEditor` checks if a value has been set before `ready()` is run, and panics otherwise. @@ -40,12 +38,10 @@ use crate::registry::property::{BuiltinExport, Export, Var}; /// /// /// # Using `OnEditor` with classes -/// /// You can wrap class smart pointers `Gd` and `DynGd` inside `OnEditor`, to make them exportable. /// `Gd` itself does not implement the `Export` trait. /// /// ## Example: automatic init -/// /// This example uses the `Default` impl, which expects a non-null value to be provided. /// /// ``` @@ -69,7 +65,6 @@ use crate::registry::property::{BuiltinExport, Export, Var}; /// ``` /// /// ## Example: user-defined init -/// /// Uninitialized `OnEditor>` and `OnEditor>` can be created with `OnEditor::default()`. /// /// ``` @@ -128,7 +123,6 @@ use crate::registry::property::{BuiltinExport, Export, Var}; /// ``` /// /// # Using `OnEditor` with built-in types -/// /// `OnEditor` can be used with any `#[export]`-enabled builtins, to provide domain-specific validation logic. /// An example might be to check whether a game entity has been granted a non-zero ID. /// @@ -169,20 +163,19 @@ use crate::registry::property::{BuiltinExport, Export, Var}; /// } /// ``` /// -/// /// # Custom getters and setters for `OnEditor` -/// -/// Custom setters and/or getters for `OnEditor` are declared as for any other `#[var]`. -/// Use the default `OnEditor` implementation to set/retrieve the value. +/// Custom setters and/or getters for `OnEditor` are declared by accepting/returning the inner type. +/// In their implementation, delegate to the `Var` trait methods, rather than dereferencing directly. /// /// ``` /// # use godot::prelude::*; /// #[derive(GodotClass)] /// #[class(init)] /// struct MyStruct { -/// #[var(get = get_my_node, set = set_my_node)] +/// #[var(get, set)] /// my_node: OnEditor>, -/// #[var(get = get_my_value, set = set_my_value)] +/// +/// #[var(get, set)] /// #[init(sentinel = -1)] /// my_value: OnEditor, /// } @@ -190,40 +183,40 @@ use crate::registry::property::{BuiltinExport, Export, Var}; /// #[godot_api] /// impl MyStruct { /// #[func] -/// pub fn get_my_node(&self) -> Option> { -/// // Write your logic in between here... -/// let ret = self.my_node.get_property(); -/// // ...and there. +/// pub fn get_my_node(&self) -> Gd { +/// let ret = Var::var_pub_get(&self.my_node); +/// // Do something with the value... /// ret /// } /// /// #[func] -/// pub fn set_my_node(&mut self, value: Option>) { -/// self.my_node.set_property(value); +/// pub fn set_my_node(&mut self, value: Gd) { +/// // Validate, pre-process, etc... +/// Var::var_pub_set(&mut self.my_node, value); /// } /// /// #[func] /// pub fn get_my_value(&self) -> i32 { -/// self.my_value.get_property() +/// let ret = Var::var_pub_get(&self.my_value); +/// // Do something with the value... +/// ret /// } /// /// #[func] /// pub fn set_my_value(&mut self, value: i32) { /// if value == 13 { /// godot_warn!("13 is unlucky number."); -/// return +/// return; /// } /// -/// self.my_value.set_property(value); +/// Var::var_pub_set(&mut self.my_value, value); /// } /// } /// ``` /// /// See also: [Register properties -- `#[var]`](../register/derive.GodotClass.html#register-properties--var) /// -/// /// # Using `OnEditor` with `#[class(tool)]` -/// /// When used with `#[class(tool)]`, the before-ready checks are omitted. /// Otherwise, `OnEditor` behaves the same — accessing an uninitialized value will cause a panic. #[derive(Debug)] @@ -296,20 +289,20 @@ impl OnEditor { match &self.state { OnEditorState::UninitNull => None, OnEditorState::UninitSentinel(val) | OnEditorState::Initialized(val) => { - Some(val.get_property()) + Some(T::var_get(val)) } } } - /// [`Var::set_property`] implementation that works both for nullable and non-nullable types. + /// [`Var::var_set`] implementation that works both for nullable and non-nullable types. /// /// All the state transitions are valid, since it is being run only in the editor. - /// See also [`Option::set_property()`]. + /// See also [`Option::var_set()`]. pub(crate) fn set_property_inner(&mut self, value: Option) { match (value, &mut self.state) { (None, _) => self.state = OnEditorState::UninitNull, (Some(value), OnEditorState::Initialized(current_value)) => { - current_value.set_property(value); + T::var_set(current_value, value); } (Some(value), OnEditorState::UninitNull) => { self.state = OnEditorState::Initialized(FromGodot::from_godot(value)) @@ -361,13 +354,24 @@ where T: Var + FromGodot + PartialEq, T::Via: BuiltinExport, { - fn get_property(&self) -> Self::Via { - // Will never fail – `PrimitiveGodotType` can not be represented by the `OnEditorState::UninitNull`. - OnEditor::::get_property_inner(self).expect("DirectExport is not nullable.") + type PubType = T::Via; + + fn var_get(field: &Self) -> Self::Via { + field + .get_property_inner() + .expect("OnEditor field of non-nullable type must be initialized before access") + } + + fn var_set(field: &mut Self, value: Self::Via) { + field.set_property_inner(Some(value)); + } + + fn var_pub_get(field: &Self) -> Self::PubType { + Self::var_get(field) } - fn set_property(&mut self, value: T::Via) { - OnEditor::::set_property_inner(self, Some(value)); + fn var_pub_set(field: &mut Self, value: Self::PubType) { + Self::var_set(field, value) } } diff --git a/godot-core/src/obj/on_ready.rs b/godot-core/src/obj/on_ready.rs index 39e460490..b6c279fd8 100644 --- a/godot-core/src/obj/on_ready.rs +++ b/godot-core/src/obj/on_ready.rs @@ -10,7 +10,7 @@ use std::mem; use crate::builtin::{GString, NodePath}; use crate::classes::{Node, Resource}; -use crate::meta::{arg_into_owned, AsArg, GodotConvert}; +use crate::meta::{arg_into_owned, AsArg, FromGodot, GodotConvert}; use crate::obj::{Gd, Inherits}; use crate::registry::property::Var; @@ -293,15 +293,32 @@ impl GodotConvert for OnReady { type Via = T::Via; } -impl Var for OnReady { - fn get_property(&self) -> Self::Via { - let deref: &T = self; - deref.get_property() +impl Var for OnReady +where + T: Var + FromGodot, +{ + type PubType = T; + + // All following functions: Deref/DerefMut panics if not initialized, preserving the "single init point" invariant. + + fn var_get(field: &Self) -> Self::Via { + let deref: &T = field; + T::var_get(deref) + } + + fn var_set(field: &mut Self, value: Self::Via) { + let deref: &mut T = field; + T::var_set(deref, value); + } + + fn var_pub_get(field: &Self) -> Self::PubType { + let deref: &T = field; + T::var_pub_get(deref) } - fn set_property(&mut self, value: Self::Via) { - let deref: &mut T = self; - deref.set_property(value); + fn var_pub_set(field: &mut Self, value: Self::PubType) { + let deref: &mut T = field; + T::var_pub_set(deref, value); } } diff --git a/godot-core/src/registry/property/mod.rs b/godot-core/src/registry/property/mod.rs index f64b55427..ec3b1617d 100644 --- a/godot-core/src/registry/property/mod.rs +++ b/godot-core/src/registry/property/mod.rs @@ -22,20 +22,28 @@ mod phantom_var; pub use phantom_var::PhantomVar; // ---------------------------------------------------------------------------------------------------------------------------------------------- -// Trait definitions +// Var trait // Note: HTML link for #[var] works if this symbol is inside prelude, but not in register::property. -/// Trait implemented for types that can be used as [`#[var]`](../register/derive.GodotClass.html#properties-and-exports) fields. +/// Trait for types used in [`#[var]`](../register/derive.GodotClass.html#properties-and-exports) fields. /// -/// 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. +/// Defines how a value is passed to/from Godot's property system, through [`var_get()`][Self::var_get] and [`var_set()`][Self::var_set] +/// associated functions. Further customizes how generated Rust getters and setters operate, in fields annotated with `#[var(pub)]`, through +/// [`var_pub_get()`][Self::var_pub_get] and [`var_pub_set()`][Self::var_pub_set]. /// -/// 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. +/// The `Var` trait does not require [`FromGodot`] or [`ToGodot`]: a value can be used as a property even if it can't be used in `#[func]` +/// parameters or return types. /// -/// See also [`Export`], a specialization of this trait for properties exported to the editor UI. +/// See also [`Export`], a subtrait for properties exported to the editor UI using `#[export]`. /// -/// For enums, this trait can be derived using the [`#[derive(Var)]`](../derive.Var.html) macro. +/// # Implementing the trait +/// Most godot-rust types implement `Var` out of the box, so you won't need to do anything. If a type doesn't support it, that's usually a sign +/// that it shouldn't be used in property contexts. +/// +/// For enums, you can use the [`#[derive(Var)]`](../derive.Var.html) macro, in combination with `GodotConvert` as `#[derive(GodotConvert, Var)]`. +/// +/// If you need to manually implement `Var` and your field type already supports `ToGodot` and `FromGodot`, just implement the [`SimpleVar`] +/// trait instead of `Var`. It will automatically provide a reasonable standard implementation of `Var`. #[doc(alias = "property")] // // on_unimplemented: we also mention #[export] here, because we can't control the order of error messages. @@ -46,16 +54,63 @@ pub use phantom_var::PhantomVar; note = "see also: https://godot-rust.github.io/book/register/properties.html" )] pub trait Var: GodotConvert { - fn get_property(&self) -> Self::Via; + /// Type used in generated Rust getters/setters for `#[var(pub)]`. + type PubType; + + /// Get property value. Called when reading a property from Godot. + fn var_get(field: &Self) -> Self::Via; + + /// Set property value. Called when writing a property from Godot. + fn var_set(field: &mut Self, value: Self::Via); + + /// Get property value in a Rust auto-generated getter, for fields annotated with `#[var(pub)]`. + fn var_pub_get(field: &Self) -> Self::PubType; - fn set_property(&mut self, value: Self::Via); + /// Set property value in a Rust auto-generated setter, for fields annotated with `#[var(pub)]`. + fn var_pub_set(field: &mut Self, value: Self::PubType); - /// Specific property hints, only override if they deviate from [`GodotType::property_info`], e.g. for enums/newtypes. + /// Specific property hints. Only override if they deviate from [`GodotType::property_info`], e.g. for enums/newtypes. fn var_hint() -> PropertyHintInfo { Self::Via::property_hint_info() } } +/// Simplified way to implement the `Var` trait, for godot-convertible types. +/// +/// Implementing this trait will auto-implement [`Var`] in a standard way for types supporting [`ToGodot`] and [`FromGodot`]. +/// +/// Types implementing this trait will use `clone()` for the public getter and direct assignment for the public setter, with `PubType = Self`. +/// This is the standard behavior for most types. +pub trait SimpleVar: ToGodot + FromGodot + Clone {} + +/// Blanket impl for types with standard Godot conversion; see [`SimpleVar`] for details. +impl Var for T +where + T: SimpleVar, + T::Via: Clone, +{ + type PubType = Self; + + fn var_get(field: &Self) -> Self::Via { + ::to_godot_owned(field) + } + + fn var_set(field: &mut Self, value: Self::Via) { + *field = ::from_godot(value); + } + + fn var_pub_get(field: &Self) -> Self::PubType { + field.clone() + } + + fn var_pub_set(field: &mut Self, value: Self::PubType) { + *field = value; + } +} + +// ---------------------------------------------------------------------------------------------------------------------------------------------- +// Export trait + // Note: HTML link for #[export] works if this symbol is inside prelude, but not in register::property. /// Trait implemented for types that can be used as [`#[export]`](../register/derive.GodotClass.html#properties-and-exports) fields. /// @@ -89,8 +144,7 @@ pub trait Export: Var { /// Marker trait to identify `GodotType`s that can be directly used with an `#[export]`. /// -/// Implemented pretty much for all the [`GodotTypes`][GodotType] that are not [`GodotClass`]. -/// Provides a few blanket implementations and, by itself, has no implications +/// Implemented pretty much for all [`GodotType`]s that are not [`GodotClass`]. By itself, this trait has no implications /// for the [`Var`] or [`Export`] traits. /// /// Types which don't implement the `BuiltinExport` trait can't be used directly as an `#[export]` @@ -111,6 +165,9 @@ pub trait Export: Var { // this `MarkerTrait` serves as the intended solution to recognize aforementioned types. pub trait BuiltinExport {} +// ---------------------------------------------------------------------------------------------------------------------------------------------- +// Doctests to test compile errors + /// This function only exists as a place to add doc-tests for the `Var` trait and `#[var]` attribute. /// /// The `#[var(no_get, no_set)]` combination is not allowed; if you don't want a property, omit `#[var]` entirely: @@ -199,22 +256,30 @@ where T: Var + FromGodot, Option: GodotConvert>, { - fn get_property(&self) -> Self::Via { - self.as_ref().map(Var::get_property) + type PubType = Option; // Same as Self::Via. + + fn var_get(field: &Self) -> Self::Via { + field.as_ref().map(T::var_get) } - fn set_property(&mut self, value: Self::Via) { + fn var_set(field: &mut Self, value: Self::Via) { match value { - Some(value) => { - if let Some(current_value) = self { - current_value.set_property(value) - } else { - *self = Some(FromGodot::from_godot(value)) - } - } - None => *self = None, + Some(via) => match field { + // If field is already set, delegate to setter (non-null) on field; otherwise assign new value. + Some(ref mut inner) => T::var_set(inner, via), + None => *field = Some(T::from_godot(via)), + }, + None => *field = None, } } + + fn var_pub_get(field: &Self) -> Self::PubType { + Self::var_get(field) + } + + fn var_pub_set(field: &mut Self, value: Self::PubType) { + Self::var_set(field, value) + } } impl Export for Option @@ -557,27 +622,16 @@ mod export_impls { macro_rules! impl_property_by_godot_convert { ($Ty:ty, no_export) => { - impl_property_by_godot_convert!(@property $Ty); + // For types without Export (Callable, Signal, Rid). + impl SimpleVar for $Ty {} }; ($Ty:ty) => { - impl_property_by_godot_convert!(@property $Ty); + impl SimpleVar for $Ty {} impl_property_by_godot_convert!(@export $Ty); impl_property_by_godot_convert!(@builtin $Ty); }; - (@property $Ty:ty) => { - impl Var for $Ty { - fn get_property(&self) -> Self::Via { - self.to_godot_owned() - } - - fn set_property(&mut self, value: Self::Via) { - *self = FromGodot::from_godot(value); - } - } - }; - (@export $Ty:ty) => { impl Export for $Ty { fn export_hint() -> PropertyHintInfo { diff --git a/godot-core/src/registry/property/phantom_var.rs b/godot-core/src/registry/property/phantom_var.rs index 360b229b1..107f8ebb9 100644 --- a/godot-core/src/registry/property/phantom_var.rs +++ b/godot-core/src/registry/property/phantom_var.rs @@ -64,12 +64,22 @@ impl GodotConvert for PhantomVar { // `PhantomVar` supports only part of `Var`, but it has to implement it, otherwise we cannot implement `Export` either. // The `GodotClass` derive macro should ensure that the `Var` implementation is not used. impl Var for PhantomVar { - fn get_property(&self) -> Self::Via { - unreachable!("code generated by GodotClass should call the custom getter") + type PubType = std::convert::Infallible; + + fn var_get(_field: &Self) -> Self::Via { + unreachable!("PhantomVar requires custom getter") + } + + fn var_set(_field: &mut Self, _value: Self::Via) { + unreachable!("PhantomVar requires custom setter") + } + + fn var_pub_get(_field: &Self) -> Self::PubType { + unreachable!("PhantomVar cannot be used with #[var(pub)]") } - fn set_property(&mut self, _value: Self::Via) { - unreachable!("code generated by GodotClass should call the custom setter"); + fn var_pub_set(_field: &mut Self, _value: Self::PubType) { + unreachable!("PhantomVar cannot be used with #[var(pub)]") } fn var_hint() -> PropertyHintInfo { diff --git a/godot-macros/src/class/data_models/field_var.rs b/godot-macros/src/class/data_models/field_var.rs index d005c9ff6..723b4351a 100644 --- a/godot-macros/src/class/data_models/field_var.rs +++ b/godot-macros/src/class/data_models/field_var.rs @@ -226,79 +226,99 @@ impl GetterSetterImpl { .. } = field; - // For #[var] without #[var(pub)], use mangled name for the actual implementation and register that with Godot. + // Use field's span so errors point to the field, not the derive macro. Separate type span for trait errors, e.g.: + // + // 328 | pub my_enum: TestEnum, + // | ^^^^^^^^ the trait `Clone` is not implemented for `TestEnum` + // | + // = note: required for `TestEnum` to implement `Var` (Clone bound for blanket impl) + let field_ty_span = field_type.span(); + let field_span = field_name.span(); + + // Godot-registered function name is always the user-facing name. + let godot_function_name = kind.make_pub_fn_name(field_name, rename); + + // For #[var(pub)], use the nice name and Var::PubType (Clone-based semantics for blanket impl). + // Otherwise, use a mangled internal name, Var::Via (GodotConvert-based semantics), + // and generate a deprecated forwarding function. // TODO(v0.6): remove deprecated forwarding functions. - let (rust_accessor, rust_deprecated_accessor, doc_hidden, godot_function_name); + let rust_accessor; + let doc_hidden; + let deprecated_function; + let signature; + let function_body; + if rust_public { - rust_accessor = kind.make_pub_fn_name(field_name, rename); - godot_function_name = rust_accessor.clone(); - rust_deprecated_accessor = None; + rust_accessor = godot_function_name.clone(); doc_hidden = TokenStream::new(); + deprecated_function = TokenStream::new(); + + match kind { + GetSet::Get => { + signature = quote_spanned! { field_ty_span=> + fn #rust_accessor(&self) -> <#field_type as ::godot::register::property::Var>::PubType + }; + function_body = quote_spanned! { field_ty_span=> + <#field_type as ::godot::register::property::Var>::var_pub_get(&self.#field_name) + }; + } + GetSet::Set => { + signature = quote_spanned! { field_ty_span=> + fn #rust_accessor(&mut self, #field_name: <#field_type as ::godot::register::property::Var>::PubType) + }; + function_body = quote_spanned! { field_ty_span=> + <#field_type as ::godot::register::property::Var>::var_pub_set(&mut self.#field_name, #field_name) + }; + } + } } else { let prefix = kind.prefix(); - - let normal_accessor = kind.make_pub_fn_name(field_name, rename); - godot_function_name = normal_accessor.clone(); // Has field's span. - rust_accessor = format_ident!("__godot_{prefix}{field_name}", span = field_name.span()); - rust_deprecated_accessor = Some(normal_accessor); + rust_accessor = format_ident!("__godot_{prefix}{field_name}", span = field_span); doc_hidden = quote! { #[doc(hidden)] }; - }; - // Separate signature + body, because former is used later for registration. - // Quotes used to be `quote_spanned! { field_span=> `, but didn't seem to make difference in IDE navigation or errors. - // #registered_accessor already has field span set. - let signature; - let function_body; - match kind { - GetSet::Get => { - // Use field's span so errors point to the field, not the derive macro. - signature = quote! { - fn #rust_accessor(&self) -> <#field_type as ::godot::meta::GodotConvert>::Via - }; - function_body = quote! { - <#field_type as ::godot::register::property::Var>::get_property(&self.#field_name) - }; - } - GetSet::Set => { - // Use field's span so errors point to the field, not the derive macro. - signature = quote! { - fn #rust_accessor(&mut self, #field_name: <#field_type as ::godot::meta::GodotConvert>::Via) - }; - function_body = quote! { - <#field_type as ::godot::register::property::Var>::set_property(&mut self.#field_name, #field_name); - }; - } - } - - let deprecated_function = if let Some(deprecated_accessor) = rust_deprecated_accessor { - // Generate both the actual implementation and a deprecated forwarding function. let deprecated_fn = match kind { - GetSet::Get => quote! { - pub fn #deprecated_accessor(&self) -> <#field_type as ::godot::meta::GodotConvert>::Via { + GetSet::Get => quote_spanned! { field_ty_span=> + pub fn #godot_function_name(&self) -> <#field_type as ::godot::meta::GodotConvert>::Via { self.#rust_accessor() } }, - GetSet::Set => quote! { - pub fn #deprecated_accessor(&mut self, #field_name: <#field_type as ::godot::meta::GodotConvert>::Via) { + GetSet::Set => quote_spanned! { field_ty_span=> + pub fn #godot_function_name(&mut self, #field_name: <#field_type as ::godot::meta::GodotConvert>::Via) { self.#rust_accessor(#field_name) } }, }; - quote! { + deprecated_function = quote! { // Deprecated forwarding function with the nice name. #[deprecated = "Auto-generated Rust getters/setters for `#[var]` are being phased out until v0.6.\n\ If you need them, opt in with #[var(pub)]."] #[allow(dead_code)] // These functions are not used for registration; pub fns in private modules remain unused. #deprecated_fn + }; + + match kind { + GetSet::Get => { + signature = quote_spanned! { field_ty_span=> + fn #rust_accessor(&self) -> <#field_type as ::godot::meta::GodotConvert>::Via + }; + function_body = quote_spanned! { field_ty_span=> + <#field_type as ::godot::register::property::Var>::var_get(&self.#field_name) + }; + } + GetSet::Set => { + signature = quote_spanned! { field_ty_span=> + fn #rust_accessor(&mut self, #field_name: <#field_type as ::godot::meta::GodotConvert>::Via) + }; + function_body = quote_spanned! { field_ty_span=> + <#field_type as ::godot::register::property::Var>::var_set(&mut self.#field_name, #field_name) + }; + } } - } else { - TokenStream::new() - }; + } // Assign all tokens to field's span. Supposed to help IDE navigation (get_foo -> foo field), but did not work out in testing. // The function names already have the correct spans, so simple quote! might also work here. - let field_span = field_name.span(); let function_impl = quote_spanned! { field_span=> #doc_hidden pub #signature { diff --git a/godot-macros/src/class/data_models/func.rs b/godot-macros/src/class/data_models/func.rs index cb7ed59e8..a93c78e9c 100644 --- a/godot-macros/src/class/data_models/func.rs +++ b/godot-macros/src/class/data_models/func.rs @@ -155,6 +155,15 @@ pub fn make_method_registration( .into_iter() .collect::>(); + // Statically check that all parameters implement FromGodot + return type implements ToGodot. Span to localize compile error. + let type_and_bounds_check = { + let sig_ret_span = signature_info.params_span; // Alternative: sig_ret's span (first token). + + quote_spanned! { sig_ret_span=> + ::godot::meta::ensure_func_bounds::(); + } + }; + let registration = quote! { #(#cfg_attrs)* { @@ -166,8 +175,7 @@ pub fn make_method_registration( type CallParams = #sig_params; type CallRet = #sig_ret; - // Statically check that all parameters implement FromGodot + return type implements ToGodot. - ::godot::meta::ensure_func_bounds::(); + #type_and_bounds_check let method_name = StringName::from(#method_name_str); diff --git a/godot-macros/src/derive/derive_var.rs b/godot-macros/src/derive/derive_var.rs index d506a5d6a..8ba19dcda 100644 --- a/godot-macros/src/derive/derive_var.rs +++ b/godot-macros/src/derive/derive_var.rs @@ -13,7 +13,7 @@ use crate::ParseResult; /// Derives `Var` for the given declaration. /// -/// This uses `ToGodot` and `FromGodot` for the `get_property` and `set_property` implementations. +/// This uses `ToGodot` and `FromGodot` for the `var_get` and `var_set` implementations. pub fn derive_var(item: venial::Item) -> ParseResult { let convert = GodotConvert::parse_declaration(item)?; @@ -23,12 +23,22 @@ pub fn derive_var(item: venial::Item) -> ParseResult { Ok(quote! { impl ::godot::register::property::Var for #name { - fn get_property(&self) -> ::Via { - ::godot::meta::ToGodot::to_godot(self) + type PubType = Self; + + fn var_get(field: &Self) -> ::Via { + ::godot::meta::ToGodot::to_godot(field) + } + + fn var_set(field: &mut Self, value: ::Via) { + *field = ::godot::meta::FromGodot::from_godot(value); + } + + fn var_pub_get(field: &Self) -> Self::PubType { + field.clone() } - fn set_property(&mut self, value: ::Via) { - *self = ::godot::meta::FromGodot::from_godot(value); + fn var_pub_set(field: &mut Self, value: Self::PubType) { + *field = value; } fn var_hint() -> ::godot::meta::PropertyHintInfo { diff --git a/itest/hot-reload/rust/src/lib.rs b/itest/hot-reload/rust/src/lib.rs index 69d3c9016..9d8b234ab 100644 --- a/itest/hot-reload/rust/src/lib.rs +++ b/itest/hot-reload/rust/src/lib.rs @@ -64,7 +64,7 @@ impl NoDefault { // ---------------------------------------------------------------------------------------------------------------------------------------------- -#[derive(GodotConvert, Var, Export)] +#[derive(Clone, GodotConvert, Var, Export)] #[godot(via = GString)] enum Planet { Earth, diff --git a/itest/rust/src/object_tests/property_test.rs b/itest/rust/src/object_tests/property_test.rs index eeb3cf0c2..5832e8e4c 100644 --- a/itest/rust/src/object_tests/property_test.rs +++ b/itest/rust/src/object_tests/property_test.rs @@ -136,18 +136,28 @@ impl GodotConvert for SomeCStyleEnum { } impl Var for SomeCStyleEnum { - fn get_property(&self) -> Self::Via { - (*self) as i64 + type PubType = Self; + + fn var_get(field: &Self) -> Self::Via { + (*field) as i64 } - fn set_property(&mut self, value: Self::Via) { + fn var_set(field: &mut Self, value: Self::Via) { match value { - 0 => *self = Self::A, - 1 => *self = Self::B, - 2 => *self = Self::C, + 0 => *field = Self::A, + 1 => *field = Self::B, + 2 => *field = Self::C, other => panic!("unexpected variant {other}"), } } + + fn var_pub_get(field: &Self) -> Self::PubType { + *field + } + + fn var_pub_set(field: &mut Self, value: Self::PubType) { + *field = value; + } } impl Export for SomeCStyleEnum { @@ -159,7 +169,7 @@ impl Export for SomeCStyleEnum { } } -#[derive(Default)] +#[derive(Default, Clone)] struct NotExportable { a: i64, b: i64, @@ -170,19 +180,29 @@ impl GodotConvert for NotExportable { } impl Var for NotExportable { - fn get_property(&self) -> Self::Via { + type PubType = Self; + + fn var_get(field: &Self) -> Self::Via { vdict! { - "a": self.a, - "b": self.b + "a": field.a, + "b": field.b } } - fn set_property(&mut self, value: Self::Via) { + fn var_set(field: &mut Self, value: Self::Via) { let a = value.get("a").unwrap().to::(); let b = value.get("b").unwrap().to::(); - self.a = a; - self.b = b; + field.a = a; + field.b = b; + } + + fn var_pub_get(field: &Self) -> Self::PubType { + field.clone() + } + + fn var_pub_set(field: &mut Self, value: Self::PubType) { + *field = value; } } @@ -296,7 +316,9 @@ struct CheckAllExports { color_no_alpha: Color, } -#[derive(GodotConvert, Var, Export, Eq, PartialEq, Debug)] +// TODO(v0.5): consider if the below enums all need Clone -- they didn't in v0.4. +// Reason is that #[derive(Var)] implements Var::var_pub_get() in a way that requires cloning, effectively requiring Clone. +#[derive(GodotConvert, Var, Export, Clone, Eq, PartialEq, Debug)] #[godot(via = i64)] #[repr(i64)] pub enum TestEnum { @@ -305,7 +327,7 @@ pub enum TestEnum { C = 2, } -#[derive(GodotConvert, Var)] +#[derive(Clone, GodotConvert, Var)] #[godot(via = i64)] pub enum Behavior { Peaceful, @@ -313,7 +335,7 @@ pub enum Behavior { Aggressive = (3 + 4), } -#[derive(GodotConvert, Var)] +#[derive(Clone, GodotConvert, Var)] #[godot(via = GString)] pub enum StrBehavior { Peaceful, @@ -323,20 +345,40 @@ pub enum StrBehavior { #[derive(GodotClass)] #[class(no_init)] -pub struct DeriveProperty { +pub struct EnumVars { #[var(pub)] pub my_enum: TestEnum, + + #[var] + pub legacy_enum: TestEnum, +} + +#[itest] +fn property_enum_var() { + let mut obj = EnumVars { + my_enum: TestEnum::B, + legacy_enum: TestEnum::B, + }; + + // From v0.5 and #[var(pub)] getters/setters use the Rust type directly (not Via type). + assert_eq!(obj.get_my_enum(), TestEnum::B); + + obj.set_my_enum(TestEnum::C); + assert_eq!(obj.my_enum, TestEnum::C); } #[itest] -fn derive_property() { - let mut class = DeriveProperty { +#[expect(deprecated)] +fn property_enum_var_legacy() { + let mut obj = EnumVars { my_enum: TestEnum::B, + legacy_enum: TestEnum::B, }; - assert_eq!(class.get_my_enum(), TestEnum::B as i64); - class.set_my_enum(TestEnum::C as i64); - assert_eq!(class.my_enum, TestEnum::C); + assert_eq!(obj.get_legacy_enum(), TestEnum::B as i64); + + obj.set_legacy_enum(TestEnum::A as i64); + assert_eq!(obj.legacy_enum, TestEnum::A); } // Regression test for https://github.com/godot-rust/gdext/issues/1009. diff --git a/itest/rust/src/register_tests/var_test.rs b/itest/rust/src/register_tests/var_test.rs index 3bcca8ceb..2ca537605 100644 --- a/itest/rust/src/register_tests/var_test.rs +++ b/itest/rust/src/register_tests/var_test.rs @@ -7,7 +7,7 @@ use godot::global::godot_str; use godot::prelude::*; -use crate::framework::itest; +use crate::framework::{expect_panic, itest}; #[derive(GodotClass)] #[class(init)] @@ -73,7 +73,6 @@ struct VarAccessors { k_pub: i32, // Custom getter, public generated setter. - // FIXME(v0.5): `get = my_custom_get` compiles despite GString/i32 type mismatch. #[var(pub, get = my_l_get)] l_pub_get: GString, } @@ -230,3 +229,120 @@ fn var_deprecated_accessors() { obj.free(); } + +// ---------------------------------------------------------------------------------------------------------------------------------------------- +// Edge cases for #[var(pub)] + +#[derive(GodotClass)] +#[class(base=Node)] +struct VarPubEdgeCases { + base: Base, + + #[var(pub)] + on_ready_int: OnReady, + + #[var(pub)] + on_ready_node: OnReady>, + + #[var(pub)] + on_editor_int: OnEditor, + + #[var(pub)] + on_editor_node: OnEditor>, +} + +#[godot_api] +impl INode for VarPubEdgeCases { + fn init(base: Base) -> Self { + Self { + base, + on_ready_int: OnReady::manual(), + on_ready_node: OnReady::manual(), + on_editor_int: OnEditor::from_sentinel(-1), + on_editor_node: OnEditor::default(), // Option::None. + } + } +} + +#[itest] +fn var_pub_access_on_ready_builtin() { + let mut obj = VarPubEdgeCases::new_alloc(); + obj.bind_mut().on_ready_int.init(42); + + assert_eq!(obj.bind().get_on_ready_int(), 42); + + obj.bind_mut().set_on_ready_int(100); + assert_eq!(*obj.bind().on_ready_int, 100); + + obj.free(); +} + +#[itest] +fn var_pub_access_on_ready_gd() { + let mut obj = VarPubEdgeCases::new_alloc(); + let first = Node::new_alloc(); + let second = Node::new_alloc(); + + obj.bind_mut().on_ready_node.init(first.clone()); + assert_eq!(obj.bind().get_on_ready_node(), first); + assert_eq!(&*obj.bind().on_ready_node, &first); + + obj.bind_mut().set_on_ready_node(second.clone()); + assert_eq!(obj.bind().get_on_ready_node(), second); + + first.free(); + second.free(); + obj.free(); +} + +#[itest] +fn var_pub_access_on_ready_panics() { + let mut obj = VarPubEdgeCases::new_alloc(); + let node = Node::new_alloc(); + + expect_panic("get - OnReady>", || { + obj.bind().get_on_ready_node(); + }); + expect_panic("get - OnReady", || { + obj.bind().get_on_ready_int(); + }); + expect_panic("set - OnReady>", || { + obj.bind_mut().set_on_ready_node(node.clone()); + }); + expect_panic("set - OnReady", || { + obj.bind_mut().set_on_ready_int(42); + }); + + node.free(); + obj.free(); +} + +#[itest] +fn var_pub_access_on_editor_builtin() { + let mut obj = VarPubEdgeCases::new_alloc(); + + assert_eq!(obj.bind().get_on_editor_int(), -1); + + obj.bind_mut().set_on_editor_int(42); + assert_eq!(obj.bind().get_on_editor_int(), 42); + assert_eq!(*obj.bind().on_editor_int, 42); + + obj.free(); +} + +#[itest] +fn var_pub_access_on_editor_gd() { + let mut obj = VarPubEdgeCases::new_alloc(); + let node = Node::new_alloc(); + + expect_panic("get - OnEditor>", || { + obj.bind().get_on_editor_node(); + }); + + obj.bind_mut().set_on_editor_node(node.clone()); + assert_eq!(obj.bind().get_on_editor_node(), node.clone()); + assert_eq!(&*obj.bind().on_editor_node, &node); + + node.free(); + obj.free(); +}