Merge pull request #1150 from godot-rust/qol/emit-asarg

Generated `emit()` functions now take `impl AsArg<T>`

Author: Bromeon

godot-core/src/builtin/collections/array.rs

@@ -1137,6 +1137,10 @@ impl<T: ArrayElement> ParamType for Array<T> {
     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<T: ArrayElement> GodotConvert for Array<T> {

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;
 }

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::*;

godot-core/src/obj/dyn_gd.rs

@@ -543,6 +543,23 @@ where
     }
 }
 
+/*
+// See `impl AsArg for Gd<T>` for why this isn't yet implemented.
+impl<'r, T, TBase, D> meta::AsArg<DynGd<TBase, D>> for &'r DynGd<T, D>
+where
+    T: Inherits<TBase>,
+    TBase: GodotClass,
+    D: ?Sized + 'static,
+{
+    fn into_arg<'cow>(self) -> meta::CowArg<'cow, DynGd<TBase, D>>
+    where
+        'r: 'cow,
+    {
+        meta::CowArg::Owned(self.clone().upcast::<TBase>())
+    }
+}
+*/
+
 impl<T, D> meta::ParamType for DynGd<T, D>
 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<T, D> meta::ArrayElement for DynGd<T, D>

godot-core/src/obj/gd.rs

@@ -852,6 +852,30 @@ impl<'r, T: GodotClass> AsArg<Gd<T>> for &'r Gd<T> {
     }
 }
 
+/*
+// 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<T> -> &Gd<T>, not &Gd<T> -> &Gd<TBase>).
+// See also regression test in array_test.rs.
+
+impl<'r, T, TBase> AsArg<Gd<TBase>> for &'r Gd<T>
+where
+    T: Inherits<TBase>,
+    TBase: GodotClass,
+{
+    #[doc(hidden)] // Repeated despite already hidden in trait; some IDEs suggest this otherwise.
+    fn into_arg<'cow>(self) -> CowArg<'cow, Gd<TBase>>
+    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<Base> 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::<TBase>())
+    }
+}
+*/
+
 impl<T: GodotClass> ParamType for Gd<T> {
     type Arg<'v> = CowArg<'v, Gd<T>>;
 
@@ -862,6 +886,10 @@ impl<T: GodotClass> ParamType for Gd<T> {
     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<T: GodotClass> AsArg<Option<Gd<T>>> for Option<&Gd<T>> {
@@ -884,6 +912,10 @@ impl<T: GodotClass> ParamType for Option<Gd<T>> {
     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<T> Default for Gd<T>

godot-macros/src/class/data_models/signal.rs

@@ -320,8 +320,35 @@ impl SignalCollection {
     }
 }
 
+fn make_asarg_params(params: &venial::Punctuated<venial::FnParam>) -> 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 = &param.name;
+                let param_type = &param.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<C: ::godot::obj::WithSignals> #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, )*));
             }
         }

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<Base> 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;

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<Object>, 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<Object>, arg2: GString);
+        pub(super) fn signal_obj(arg1: Gd<Object>, arg2: GString);
 
         #[func]
         pub fn self_receive(&mut self, arg1: i64) {

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
 }

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<DynGd<Object, _>> = 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<RefcHealth, dyn Health> = Gd::from_object(RefcHealth { hp: 33 }).into_dyn();
+    let c = c.upcast::<RefCounted>();
+    let array_inferred /*: Array<DynGd<RefCounted, _>>*/ = array![&c];
+    assert_eq!(array_inferred.at(0).dyn_bind().get_hitpoints(), 33);
 }
 
 #[itest]