Callable: allow R: ToGodot return types

No longer requires pointless Variant::nil() for unit return types.

Author: Bromeon

godot-core/src/builtin/callable.rs

@@ -154,7 +154,6 @@ impl Callable {
     ///
     /// This constructor only allows the callable to be invoked from the same thread as creating it. If you need to invoke it from any thread,
     /// use [`from_sync_fn`][Self::from_sync_fn] instead (requires crate feature `experimental-threads`; only enable if really needed).
-    #[cfg(since_api = "4.2")]
     pub fn from_local_fn<R, F, S>(name: S, rust_function: F) -> Self
     where
         R: ToGodot,
@@ -201,9 +200,10 @@ impl Callable {
     ///
     /// After the first invocation, subsequent calls will panic with a message indicating the callable has already been consumed. This is
     /// useful for deferred operations that should only execute once. For repeated execution, use [`from_local_fn()][Self::from_local_fn].
-    pub(crate) fn from_once_fn<F, S>(name: S, rust_function: F) -> Self
+    pub(crate) fn from_once_fn<R, F, S>(name: S, rust_function: F) -> Self
     where
-        F: 'static + FnOnce(&[&Variant]) -> Variant,
+        R: ToGodot,
+        F: 'static + FnOnce(&[&Variant]) -> R,
         S: meta::AsArg<GString>,
     {
         meta::arg_into_owned!(name);
@@ -213,6 +213,7 @@ impl Callable {
             let rust_fn_once = rust_fn_once
                 .take()
                 .expect("callable created with from_once_fn() has already been consumed");
+
             rust_fn_once(args)
         })
     }
@@ -265,14 +266,15 @@ impl Callable {
     /// });
     /// ```
     #[cfg(feature = "experimental-threads")]
-    pub fn from_sync_fn<F, S>(name: S, rust_function: F) -> Self
+    pub fn from_sync_fn<R, F, S>(name: S, rust_function: F) -> Self
     where
-        F: 'static + Send + Sync + FnMut(&[&Variant]) -> Variant,
+        R: ToGodot,
+        F: 'static + Send + Sync + FnMut(&[&Variant]) -> R,
         S: meta::AsArg<GString>,
     {
         meta::arg_into_owned!(name);
 
-        Self::from_fn_wrapper::<F, Variant>(FnWrapper {
+        Self::from_fn_wrapper::<F, R>(FnWrapper {
             rust_function,
             name,
             thread_id: None,

godot-core/src/obj/base.rs

@@ -14,7 +14,7 @@ use std::fmt::{Debug, Display, Formatter, Result as FmtResult};
 use std::mem::ManuallyDrop;
 use std::rc::Rc;
 
-use crate::builtin::{Callable, Variant};
+use crate::builtin::Callable;
 use crate::obj::{bounds, Gd, GodotClass, InstanceId, PassiveGd};
 use crate::{classes, sys};
 
@@ -226,7 +226,6 @@ impl<T: GodotClass> Base<T> {
         let name = format!("Base<{}> deferred unref", T::class_id());
         let callable = Callable::from_once_fn(&name, move |_args| {
             Self::drop_strong_ref(instance_id);
-            Variant::nil()
         });
 
         // Use Callable::call_deferred() instead of Gd::apply_deferred(). The latter implicitly borrows &mut self,

godot-core/src/obj/gd.rs

@@ -711,7 +711,6 @@ impl<T: GodotClass> Gd<T> {
 
         let callable = Callable::from_once_fn("run_deferred", move |_| {
             gd_function(obj);
-            Variant::nil()
         });
         callable.call_deferred(&[]);
     }

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

@@ -562,9 +562,9 @@ fn array_bsearch_custom() {
 }
 
 fn backwards_sort_callable() -> Callable {
-    Callable::from_local_fn("sort backwards", |args: &[&Variant]| {
-        let res = args[0].to::<i32>() > args[1].to::<i32>();
-        res.to_variant()
+    // No &[&Variant] explicit type in arguments.
+    Callable::from_local_fn("sort backwards", |args| {
+        args[0].to::<i32>() > args[1].to::<i32>()
     })
 }
 

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

@@ -407,7 +407,6 @@ pub mod custom_callable {
 
         let callable = Callable::from_local_fn("change_global", |_args| {
             *GLOBAL.lock() = 777;
-            Variant::nil()
         });
 
         // Note that Callable itself isn't Sync/Send, so we have to transfer it unsafely.
@@ -469,11 +468,9 @@ pub mod custom_callable {
     }
 
     #[itest]
-    fn callable_custom_with_err() {
-        let callable_with_err =
-            Callable::from_local_fn("on_error_doesnt_crash", |_args: &[&Variant]| Variant::nil());
+    fn callable_from_fn_nil() {
+        let callable_with_err = Callable::from_local_fn("returns_nil", |_args: &[&Variant]| {});
 
-        // Causes error in Godot, but should not crash.
         assert_eq!(callable_with_err.callv(&varray![]), Variant::nil());
     }
 
@@ -487,9 +484,9 @@ pub mod custom_callable {
         assert_ne!(a, c, "same function, different instance -> not equal");
     }
 
-    fn sum(args: &[&Variant]) -> Variant {
-        let sum: i32 = args.iter().map(|arg| arg.to::<i32>()).sum();
-        sum.to_variant()
+    // Now non-Variant return type.
+    fn sum(args: &[&Variant]) -> i32 {
+        args.iter().map(|arg| arg.to::<i32>()).sum()
     }
 
     #[itest]
@@ -588,11 +585,10 @@ pub mod custom_callable {
     fn callable_callv_panic_from_fn() {
         let received = Arc::new(AtomicU32::new(0));
         let received_callable = received.clone();
-        let callable = Callable::from_local_fn("test", move |_args| -> Variant {
+        let callable = Callable::from_local_fn("test", move |_args| {
             suppress_panic_log(|| {
                 panic!("TEST: {}", received_callable.fetch_add(1, Ordering::SeqCst))
             });
-            Variant::nil()
         });
 
         assert_eq!(Variant::nil(), callable.callv(&varray![]));

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

@@ -819,6 +819,8 @@ mod custom_callable {
     }
 
     fn connect_signal_panic_from_fn(received: Arc<AtomicU32>) -> Callable {
+        // Explicit `Variant` return type to avoid following warning becoming a hard error in edition 2024.
+        // warning: this function depends on never type fallback being `()`
         Callable::from_local_fn("test", move |_args| -> Variant {
             panic!("TEST: {}", received.fetch_add(1, Ordering::SeqCst))
         })

itest/rust/src/engine_tests/async_test.rs

@@ -7,7 +7,7 @@
 
 use std::ops::Deref;
 
-use godot::builtin::{array, vslice, Array, Callable, Signal, Variant};
+use godot::builtin::{array, vslice, Array, Callable, Signal};
 use godot::classes::{Object, RefCounted};
 use godot::obj::{Base, Gd, NewAlloc, NewGd};
 use godot::prelude::{godot_api, GodotClass};
@@ -211,7 +211,7 @@ fn resolver_callabable_equality() {
 
     let callable = Callable::from_custom(resolver.clone());
     let cloned_callable = Callable::from_custom(resolver.clone());
-    let unrelated_callable = Callable::from_local_fn("unrelated", |_| Variant::nil());
+    let unrelated_callable = Callable::from_local_fn("unrelated", |_| {});
 
     assert_eq!(callable, cloned_callable);
     assert_ne!(callable, unrelated_callable);

itest/rust/src/framework/runner.rs

@@ -483,7 +483,6 @@ fn check_async_test_task(
                 .expect("Callable should not be called multiple times!"),
             &next_ctx,
         );
-        Variant::nil()
     });
 
     ctx.scene_tree