diff --git a/godot-core/src/builtin/collections/array.rs b/godot-core/src/builtin/collections/array.rs index 5698260f0..8f50d5c38 100644 --- a/godot-core/src/builtin/collections/array.rs +++ b/godot-core/src/builtin/collections/array.rs @@ -1137,6 +1137,10 @@ impl ParamType for Array { fn arg_to_ref<'r>(arg: &'r Self::Arg<'_>) -> &'r Self { arg.cow_as_ref() } + + fn arg_into_owned(arg: Self::Arg<'_>) -> Self { + arg.cow_into_owned() + } } impl GodotConvert for Array { diff --git a/godot-core/src/meta/args/as_arg.rs b/godot-core/src/meta/args/as_arg.rs index b5556e48d..7eef8db96 100644 --- a/godot-core/src/meta/args/as_arg.rs +++ b/godot-core/src/meta/args/as_arg.rs @@ -90,9 +90,17 @@ macro_rules! arg_into_ref { #[macro_export] macro_rules! arg_into_owned { ($arg_variable:ident) => { + // Non-generic version allows type inference. Only applicable for CowArg types. let $arg_variable = $arg_variable.into_arg(); let $arg_variable = $arg_variable.cow_into_owned(); - // cow_into_owned() is not yet used generically; could be abstracted in ParamType::arg_to_owned() as well. + }; + ($arg_variable:ident: $T:ty) => { + let $arg_variable = $arg_variable.into_arg(); + let $arg_variable: $T = $crate::meta::ParamType::arg_into_owned($arg_variable); + }; + (infer $arg_variable:ident) => { + let $arg_variable = $arg_variable.into_arg(); + let $arg_variable = $crate::meta::ParamType::arg_into_owned($arg_variable); }; } @@ -116,6 +124,10 @@ macro_rules! impl_asarg_by_value { fn arg_to_ref<'r>(arg: &'r Self::Arg<'_>) -> &'r Self { arg } + + fn arg_into_owned(arg: Self::Arg<'_>) -> Self { + arg + } } }; } @@ -149,6 +161,10 @@ macro_rules! impl_asarg_by_ref { fn arg_to_ref<'r>(arg: &'r Self::Arg<'_>) -> &'r Self { arg.cow_as_ref() } + + fn arg_into_owned(arg: Self::Arg<'_>) -> Self { + arg.cow_into_owned() + } } }; } @@ -280,4 +296,10 @@ pub trait ParamType: sealed::Sealed + Sized + 'static /// Useful in generic contexts where you need to extract a reference of an argument, independently of how it is passed. #[doc(hidden)] // for now, users are encouraged to use only call-site of impl AsArg; declaration-site may still develop. fn arg_to_ref<'r>(arg: &'r Self::Arg<'_>) -> &'r Self; + + /// Clones an argument into an owned value. + /// + /// Useful in generic contexts where you need to extract a value of an argument, independently of how it is passed. + #[doc(hidden)] // for now, users are encouraged to use only call-site of impl AsArg; declaration-site may still develop. + fn arg_into_owned(arg: Self::Arg<'_>) -> Self; } diff --git a/godot-core/src/meta/mod.rs b/godot-core/src/meta/mod.rs index 17bb82953..c5f2c72f7 100644 --- a/godot-core/src/meta/mod.rs +++ b/godot-core/src/meta/mod.rs @@ -72,9 +72,10 @@ pub(crate) use traits::{ use crate::registry::method::MethodParamOrReturnInfo; pub(crate) use crate::{ - arg_into_owned, arg_into_ref, declare_arg_method, impl_asarg_by_ref, impl_asarg_by_value, - impl_godot_as_self, + arg_into_ref, declare_arg_method, impl_asarg_by_ref, impl_asarg_by_value, impl_godot_as_self, }; +// Public due to signals emit() needing it. Should be made pub(crate) again if that changes. +pub use crate::arg_into_owned; #[doc(hidden)] pub use signature::*; diff --git a/godot-core/src/obj/dyn_gd.rs b/godot-core/src/obj/dyn_gd.rs index e8d2edc9a..45d8e18e5 100644 --- a/godot-core/src/obj/dyn_gd.rs +++ b/godot-core/src/obj/dyn_gd.rs @@ -543,6 +543,23 @@ where } } +/* +// See `impl AsArg for Gd` for why this isn't yet implemented. +impl<'r, T, TBase, D> meta::AsArg> for &'r DynGd +where + T: Inherits, + TBase: GodotClass, + D: ?Sized + 'static, +{ + fn into_arg<'cow>(self) -> meta::CowArg<'cow, DynGd> + where + 'r: 'cow, + { + meta::CowArg::Owned(self.clone().upcast::()) + } +} +*/ + impl meta::ParamType for DynGd where T: GodotClass, @@ -557,6 +574,10 @@ where fn arg_to_ref<'r>(arg: &'r Self::Arg<'_>) -> &'r Self { arg.cow_as_ref() } + + fn arg_into_owned(arg: Self::Arg<'_>) -> Self { + arg.cow_into_owned() + } } impl meta::ArrayElement for DynGd diff --git a/godot-core/src/obj/gd.rs b/godot-core/src/obj/gd.rs index 811f6c9ca..f6b848052 100644 --- a/godot-core/src/obj/gd.rs +++ b/godot-core/src/obj/gd.rs @@ -852,6 +852,30 @@ impl<'r, T: GodotClass> AsArg> for &'r Gd { } } +/* +// TODO find a way to generalize AsArg to derived->base conversions without breaking type inference in array![]. +// Possibly we could use a "canonical type" with unambiguous mapping (&Gd -> &Gd, not &Gd -> &Gd). +// See also regression test in array_test.rs. + +impl<'r, T, TBase> AsArg> for &'r Gd +where + T: Inherits, + TBase: GodotClass, +{ + #[doc(hidden)] // Repeated despite already hidden in trait; some IDEs suggest this otherwise. + fn into_arg<'cow>(self) -> CowArg<'cow, Gd> + where + 'r: 'cow, // Original reference must be valid for at least as long as the returned cow. + { + // Performance: clones unnecessarily, which has overhead for ref-counted objects. + // A result of being generic over base objects and allowing T: Inherits rather than just T == Base. + // Was previously `CowArg::Borrowed(self)`. Borrowed() can maybe be specialized for objects, or combined with AsObjectArg. + + CowArg::Owned(self.clone().upcast::()) + } +} +*/ + impl ParamType for Gd { type Arg<'v> = CowArg<'v, Gd>; @@ -862,6 +886,10 @@ impl ParamType for Gd { fn arg_to_ref<'r>(arg: &'r Self::Arg<'_>) -> &'r Self { arg.cow_as_ref() } + + fn arg_into_owned(arg: Self::Arg<'_>) -> Self { + arg.cow_into_owned() + } } impl AsArg>> for Option<&Gd> { @@ -884,6 +912,10 @@ impl ParamType for Option> { fn arg_to_ref<'r>(arg: &'r Self::Arg<'_>) -> &'r Self { arg.cow_as_ref() } + + fn arg_into_owned(arg: Self::Arg<'_>) -> Self { + arg.cow_into_owned() + } } impl Default for Gd diff --git a/godot-macros/src/class/data_models/signal.rs b/godot-macros/src/class/data_models/signal.rs index d46a6e7ae..3912b4987 100644 --- a/godot-macros/src/class/data_models/signal.rs +++ b/godot-macros/src/class/data_models/signal.rs @@ -320,8 +320,35 @@ impl SignalCollection { } } +fn make_asarg_params(params: &venial::Punctuated) -> TokenStream { + // Could be specialized by trying to parse types, but won't be 100% accurate due to lack of semantics (AsArg could be a safe fallback). E.g.: + // if ty.tokens.iter().any(|tk| matches!(tk, TokenTree::Ident(ident) if ident == "Gd")) { + // quote! { impl ::godot::meta::AsObjectArg<#some_inner_ty> } + // } + + let mut tokens = TokenStream::new(); + + for (param, _punct) in params.iter() { + match param { + venial::FnParam::Typed(param) => { + let param_name = ¶m.name; + let param_type = ¶m.ty; + + tokens.extend(quote! { + #param_name: impl ::godot::meta::AsArg<#param_type>, + }); + } + venial::FnParam::Receiver(_) => { + unreachable!("signals have no receivers; already checked") + } + }; + } + + tokens +} + fn make_signal_individual_struct(details: &SignalDetails) -> TokenStream { - let emit_params = &details.fn_signature.params; + let emit_params = make_asarg_params(&details.fn_signature.params); let SignalDetails { // class_name, @@ -360,6 +387,12 @@ fn make_signal_individual_struct(details: &SignalDetails) -> TokenStream { #(#signal_cfg_attrs)* impl #individual_struct_name<'_, C> { pub fn emit(&mut self, #emit_params) { + use ::godot::meta::AsArg; + #( + ::godot::meta::arg_into_owned!(infer #param_names); + //let #param_names = #param_names.into_arg(); + )* + self.__typed.emit_tuple((#( #param_names, )*)); } } diff --git a/itest/rust/src/builtin_tests/containers/array_test.rs b/itest/rust/src/builtin_tests/containers/array_test.rs index d780eff5e..e5f1902cc 100644 --- a/itest/rust/src/builtin_tests/containers/array_test.rs +++ b/itest/rust/src/builtin_tests/containers/array_test.rs @@ -570,6 +570,18 @@ fn array_resize() { assert_eq!(a, Array::new()); } +// Tests that arrays of objects can be declared without explicit type annotations. A similar test exists for DynGd in dyn_gd_test.rs. +// This has deliberately been added to guard against regressions in case `AsArg` is extended (T: Inherits support broke this). +fn __array_type_inference() { + let a = Node::new_alloc(); + let b = Node::new_alloc(); + let _array = array![&a, &b]; + + let c = ArrayTest::new_gd(); + let d = ArrayTest::new_gd(); + let _array = array![&c, &d]; +} + #[derive(GodotClass, Debug)] #[class(init, base=RefCounted)] struct ArrayTest; diff --git a/itest/rust/src/builtin_tests/containers/signal_test.rs b/itest/rust/src/builtin_tests/containers/signal_test.rs index 02c022ece..75b0ffcb5 100644 --- a/itest/rust/src/builtin_tests/containers/signal_test.rs +++ b/itest/rust/src/builtin_tests/containers/signal_test.rs @@ -122,6 +122,28 @@ fn signal_symbols_external() { emitter.free(); } +// "External" means connect/emit happens from outside the class, via Gd::signals(). +#[cfg(since_api = "4.2")] +#[itest] +fn signal_symbols_complex_emit() { + let mut emitter = Emitter::new_alloc(); + let arg = emitter.clone(); + let mut sig = emitter.signals().signal_obj(); + + let tracker = Rc::new(RefCell::new(None)); + { + let tracker = tracker.clone(); + sig.connect(move |obj: Gd, name: GString| { + *tracker.borrow_mut() = Some((obj, name)); + }); + } + + // Forward compat: .upcast() here becomes a breaking change if we generalize AsArg to include derived->base conversions. + sig.emit(&arg.upcast(), "hello"); + + emitter.free(); +} + // "External" means connect/emit happens from outside the class, via Gd::signals(). #[cfg(since_api = "4.2")] #[itest] @@ -396,7 +418,7 @@ mod emitter { pub fn signal_int(arg1: i64); #[signal] - fn signal_obj(arg1: Gd, arg2: GString); + pub(super) fn signal_obj(arg1: Gd, arg2: GString); #[func] pub fn self_receive(&mut self, arg1: i64) { diff --git a/itest/rust/src/engine_tests/async_test.rs b/itest/rust/src/engine_tests/async_test.rs index 48650db4e..3c8fce580 100644 --- a/itest/rust/src/engine_tests/async_test.rs +++ b/itest/rust/src/engine_tests/async_test.rs @@ -229,7 +229,10 @@ fn async_typed_signal_with_array() -> TaskHandle { assert_eq!(result, array![1, 2, 3]); }); - object.signals().custom_signal_array().emit(array![1, 2, 3]); + object + .signals() + .custom_signal_array() + .emit(&array![1, 2, 3]); task_handle } diff --git a/itest/rust/src/object_tests/dyn_gd_test.rs b/itest/rust/src/object_tests/dyn_gd_test.rs index b7ec060ba..a23f06f1c 100644 --- a/itest/rust/src/object_tests/dyn_gd_test.rs +++ b/itest/rust/src/object_tests/dyn_gd_test.rs @@ -4,6 +4,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ + use crate::framework::{expect_panic, itest}; // Test that all important dyn-related symbols are in the prelude. use godot::prelude::*; @@ -346,12 +347,19 @@ fn dyn_gd_store_in_godot_array() { let a = Gd::from_object(RefcHealth { hp: 33 }).into_dyn(); let b = foreign::NodeHealth::new_alloc().into_dyn(); + // Forward compat: .upcast() here becomes a breaking change if we generalize AsArg to include derived->base conversions. let array: Array> = array![&a.upcast(), &b.upcast()]; assert_eq!(array.at(0).dyn_bind().get_hitpoints(), 33); assert_eq!(array.at(1).dyn_bind().get_hitpoints(), 100); array.at(1).free(); + + // Tests also type inference of array![]. Independent variable c. + let c: DynGd = Gd::from_object(RefcHealth { hp: 33 }).into_dyn(); + let c = c.upcast::(); + let array_inferred /*: Array>*/ = array![&c]; + assert_eq!(array_inferred.at(0).dyn_bind().get_hitpoints(), 33); } #[itest] diff --git a/itest/rust/src/object_tests/object_test.rs b/itest/rust/src/object_tests/object_test.rs index 947641486..ebbbfb240 100644 --- a/itest/rust/src/object_tests/object_test.rs +++ b/itest/rust/src/object_tests/object_test.rs @@ -1040,6 +1040,7 @@ pub mod object_test_gd { #[func] fn return_nested_self() -> Array::Base>> { + // Forward compat: .upcast() here becomes a breaking change if we generalize AsArg to include derived->base conversions. array![&Self::return_self().upcast()] } }