Merge pull request #944 from godot-rust/feature/callable-by-ref

`Callable` is now passed by-ref

Author: Bromeon

examples/dodge-the-creeps/rust/src/hud.rs

@@ -26,7 +26,7 @@ impl Hud {
         self.show_message("Game Over".into());
 
         let mut timer = self.base().get_tree().unwrap().create_timer(2.0).unwrap();
-        timer.connect("timeout", self.base().callable("show_start_button"));
+        timer.connect("timeout", &self.base().callable("show_start_button"));
     }
 
     #[func]

examples/dodge-the-creeps/rust/src/main_scene.rs

@@ -102,7 +102,7 @@ impl Main {
         mob.set_linear_velocity(Vector2::new(range, 0.0).rotated(real::from_f32(direction)));
 
         let mut hud = self.base().get_node_as::<Hud>("Hud");
-        hud.connect("start_game", mob.callable("on_start_game"));
+        hud.connect("start_game", &mob.callable("on_start_game"));
     }
 
     fn music(&mut self) -> &mut AudioStreamPlayer {

godot-codegen/src/context.rs

@@ -243,14 +243,17 @@ impl<'a> Context<'a> {
         self.builtin_types.contains(ty_name)
     }
 
-    pub fn get_builtin_arg_passing(&self, ty_name: &str) -> ArgPassing {
+    pub fn get_builtin_arg_passing(&self, godot_ty: &GodotTy) -> ArgPassing {
         // Already handled separately.
-        debug_assert!(!ty_name.starts_with("Packed"));
+        debug_assert!(!godot_ty.ty.starts_with("Packed"));
+
+        // IMPORTANT: Keep this in sync with impl_ffi_variant!() macros taking `ref` or not.
 
         // Arrays are also handled separately, and use ByRef.
-        match ty_name {
-            "Variant" | "VariantArray" | "Dictionary" => ArgPassing::ByRef,
-            "GString" | "NodePath" | "StringName" => ArgPassing::ImplAsArg,
+        match godot_ty.ty.as_str() {
+            // Note: Signal is currently not used in any parameter, but this may change.
+            "Variant" | "Array" | "Dictionary" | "Callable" | "Signal" => ArgPassing::ByRef,
+            "String" | "StringName" | "NodePath" => ArgPassing::ImplAsArg,
             _ => ArgPassing::ByValue,
         }
     }

godot-codegen/src/conv/type_conversions.rs

@@ -179,7 +179,7 @@ fn to_rust_type_uncached(full_ty: &GodotTy, ctx: &mut Context) -> RustTy {
         if let Some(hardcoded) = to_hardcoded_rust_ident(full_ty) {
             return RustTy::BuiltinIdent {
                 ty: ident(hardcoded),
-                arg_passing: ctx.get_builtin_arg_passing(hardcoded),
+                arg_passing: ctx.get_builtin_arg_passing(full_ty),
             };
         }
     }
@@ -224,7 +224,7 @@ fn to_rust_type_uncached(full_ty: &GodotTy, ctx: &mut Context) -> RustTy {
         // Native structures might not all be Copy, but they should have value semantics.
         RustTy::BuiltinIdent {
             ty: rustify_ty(ty),
-            arg_passing: ctx.get_builtin_arg_passing(ty),
+            arg_passing: ctx.get_builtin_arg_passing(full_ty),
         }
     } else {
         let ty = rustify_ty(ty);

godot-codegen/src/models/domain.rs

@@ -663,28 +663,6 @@ impl RustTy {
             other => quote! { -> #other },
         }
     }
-
-    pub fn is_pass_by_ref(&self) -> bool {
-        matches!(
-            self,
-            RustTy::BuiltinIdent {
-                arg_passing: ArgPassing::ByRef,
-                ..
-            } | RustTy::BuiltinArray { .. }
-                | RustTy::EngineArray { .. }
-        )
-    }
-
-    pub fn needs_lifetime(&self) -> bool {
-        matches!(
-            self,
-            RustTy::BuiltinIdent {
-                arg_passing: ArgPassing::ByRef | ArgPassing::ImplAsArg,
-                ..
-            } | RustTy::BuiltinArray { .. }
-                | RustTy::EngineArray { .. }
-        )
-    }
 }
 
 impl ToTokens for RustTy {

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

@@ -614,7 +614,7 @@ impl<T: ArrayElement> Array<T> {
     ///
     /// Consider using `sort_custom()` to ensure the sorting order is compatible with
     /// your callable's ordering
-    pub fn bsearch_custom(&self, value: impl AsArg<T>, func: Callable) -> usize {
+    pub fn bsearch_custom(&self, value: impl AsArg<T>, func: &Callable) -> usize {
         meta::arg_into_ref!(value: T);
 
         to_usize(
@@ -646,7 +646,7 @@ impl<T: ArrayElement> Array<T> {
     /// Note: The sorting algorithm used is not [stable](https://en.wikipedia.org/wiki/Sorting_algorithm#Stability).
     /// This means that values considered equal may have their order changed when using `sort_unstable_custom`.
     #[doc(alias = "sort_custom")]
-    pub fn sort_unstable_custom(&mut self, func: Callable) {
+    pub fn sort_unstable_custom(&mut self, func: &Callable) {
         // SAFETY: We do not write any values that don't already exist in the array, so all values have the correct type.
         unsafe { self.as_inner_mut() }.sort_custom(func);
     }

godot-core/src/builtin/signal.rs

@@ -74,7 +74,7 @@ impl Signal {
     /// returns [`Error::ERR_INVALID_PARAMETER`] and
     /// pushes an error message, unless the signal is connected with [`ConnectFlags::REFERENCE_COUNTED`](crate::classes::object::ConnectFlags::REFERENCE_COUNTED).
     /// To prevent this, use [`Self::is_connected`] first to check for existing connections.
-    pub fn connect(&self, callable: Callable, flags: i64) -> Error {
+    pub fn connect(&self, callable: &Callable, flags: i64) -> Error {
         let error = self.as_inner().connect(callable, flags);
 
         Error::from_godot(error as i32)
@@ -83,7 +83,7 @@ impl Signal {
     /// Disconnects this signal from the specified [`Callable`].
     ///
     /// If the connection does not exist, generates an error. Use [`Self::is_connected`] to make sure that the connection exists.
-    pub fn disconnect(&self, callable: Callable) {
+    pub fn disconnect(&self, callable: &Callable) {
         self.as_inner().disconnect(callable);
     }
 
@@ -142,7 +142,7 @@ impl Signal {
     }
 
     /// Returns `true` if the specified [`Callable`] is connected to this signal.
-    pub fn is_connected(&self, callable: Callable) -> bool {
+    pub fn is_connected(&self, callable: &Callable) -> bool {
         self.as_inner().is_connected(callable)
     }
 

godot-core/src/builtin/variant/impls.rs

@@ -130,8 +130,10 @@ macro_rules! impl_ffi_variant {
 #[allow(clippy::module_inception)]
 mod impls {
     use super::*;
+    
+    // IMPORTANT: the presence/absence of `ref` here should be aligned with the ArgPassing variant
+    // used in codegen get_builtin_arg_passing().
 
-    // Also implements ArrayType.
     impl_ffi_variant!(bool, bool_to_variant, bool_from_variant);
     impl_ffi_variant!(i64, int_to_variant, int_from_variant; int);
     impl_ffi_variant!(f64, float_to_variant, float_from_variant; float);

godot-core/src/meta/rpc_config.rs

@@ -12,7 +12,7 @@ use crate::classes::Node;
 use crate::meta::{AsArg, ToGodot};
 use crate::{arg_into_ref, dict};
 
-/// Configuration for a remote procedure call, typically used with `#[rpc(config = ...)]`.
+/// Configuration for a remote procedure call, used with `#[rpc(config = ...)]`.
 ///
 /// See [Godot documentation](https://docs.godotengine.org/en/stable/tutorials/networking/high_level_multiplayer.html#remote-procedure-calls).
 #[derive(Copy, Clone, Debug)]

itest/rust/src/builtin_tests/containers/array_test.rs

@@ -479,7 +479,7 @@ fn array_should_format_with_display() {
 fn array_sort_custom() {
     let mut a = array![1, 2, 3, 4];
     let func = backwards_sort_callable();
-    a.sort_unstable_custom(func);
+    a.sort_unstable_custom(&func);
     assert_eq!(a, array![4, 3, 2, 1]);
 }
 
@@ -488,8 +488,8 @@ fn array_sort_custom() {
 fn array_binary_search_custom() {
     let a = array![5, 4, 2, 1];
     let func = backwards_sort_callable();
-    assert_eq!(a.bsearch_custom(1, func.clone()), 3);
-    assert_eq!(a.bsearch_custom(3, func), 2);
+    assert_eq!(a.bsearch_custom(1, &func), 3);
+    assert_eq!(a.bsearch_custom(3, &func), 2);
 }
 
 #[cfg(since_api = "4.2")]