Merge pull request #1332 from godot-rust/qol/callable-no-result

`Callable::from_local_fn()` now returns `R: ToGodot`

Author: Bromeon

godot-core/src/builtin/callable.rs

@@ -72,7 +72,7 @@ impl Callable {
     /// If the builtin type does not have the method, the returned callable will be invalid.
     ///
     /// Static builtin methods (e.g. `String.humanize_size`) are not supported in reflection as of Godot 4.4. For static _class_ functions,
-    /// use [`from_local_static()`][Self::from_local_static] instead.
+    /// use [`from_class_static()`][Self::from_class_static] instead.
     ///
     /// _Godot equivalent: `Callable.create(Variant variant, StringName method)`_
     #[cfg(since_api = "4.3")]
@@ -84,17 +84,15 @@ impl Callable {
         inner::InnerCallable::create(variant, method_name)
     }
 
-    /// Create a callable for the static method `class_name::function` (single-threaded).
+    /// Create a callable for the static method `class_name::function`
     ///
-    /// Allows you to call static functions through `Callable`.
+    /// Allows you to call static functions through `Callable`. Allows both single- and multi-threaded calls; what happens on Godot side
+    /// is your responsibility.
     ///
-    /// Does not support built-in types (such as `String`), only classes.
-    ///
-    /// # Compatibility
-    /// Not available before Godot 4.4. Library versions <0.3 used to provide this, however the polyfill used to emulate it was half-broken
-    /// (not supporting signals, bind(), method_name(), is_valid(), etc).
+    /// Does not support built-in types (such as `String`), only classes. Static functions on built-in types are not supported in Godot's
+    /// reflection APIs at the moment.
     #[cfg(since_api = "4.4")]
-    pub fn from_local_static(
+    pub fn from_class_static(
         class_name: impl meta::AsArg<StringName>,
         function_name: impl meta::AsArg<StringName>,
     ) -> Self {
@@ -103,16 +101,33 @@ impl Callable {
 
         let callable_name = format!("{class_name}.{function_name}");
 
-        Self::from_local_fn(&callable_name, move |args| {
+        let function = move |args: &[&Variant]| {
             let args = args.iter().cloned().cloned().collect::<Vec<_>>();
 
             let result: Variant = classes::ClassDb::singleton().class_call_static(
                 &class_name,
                 &function_name,
                 args.as_slice(),
             );
-            Ok(result)
-        })
+            result
+        };
+
+        #[cfg(feature = "experimental-threads")]
+        let callable = Self::from_sync_fn(&callable_name, function);
+
+        #[cfg(not(feature = "experimental-threads"))]
+        let callable = Self::from_local_fn(&callable_name, function);
+
+        callable
+    }
+
+    #[deprecated = "Renamed to `from_class_static`."]
+    #[cfg(since_api = "4.4")]
+    pub fn from_local_static(
+        class_name: impl meta::AsArg<StringName>,
+        function_name: impl meta::AsArg<StringName>,
+    ) -> Self {
+        Self::from_class_static(class_name, function_name)
     }
 
     fn default_callable_custom_info() -> CallableCustomInfo {
@@ -139,14 +154,15 @@ 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).
-    pub fn from_local_fn<F, S>(name: S, rust_function: F) -> Self
+    pub fn from_local_fn<R, F, S>(name: S, rust_function: F) -> Self
     where
-        F: 'static + FnMut(&[&Variant]) -> Result<Variant, ()>,
+        R: ToGodot,
+        F: 'static + FnMut(&[&Variant]) -> R,
         S: meta::AsArg<GString>,
     {
         meta::arg_into_owned!(name);
 
-        Self::from_fn_wrapper(FnWrapper {
+        Self::from_fn_wrapper::<F, R>(FnWrapper {
             rust_function,
             name,
             thread_id: Some(std::thread::current().id()),
@@ -165,12 +181,12 @@ impl Callable {
     pub fn from_linked_fn<F, T, S>(name: S, linked_object: &Gd<T>, rust_function: F) -> Self
     where
         T: GodotClass,
-        F: 'static + FnMut(&[&Variant]) -> Result<Variant, ()>,
+        F: 'static + FnMut(&[&Variant]) -> Variant,
         S: meta::AsArg<GString>,
     {
         meta::arg_into_owned!(name);
 
-        Self::from_fn_wrapper(FnWrapper {
+        Self::from_fn_wrapper::<F, Variant>(FnWrapper {
             rust_function,
             name,
             thread_id: Some(std::thread::current().id()),
@@ -184,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]) -> Result<Variant, ()>,
+        R: ToGodot,
+        F: 'static + FnOnce(&[&Variant]) -> R,
         S: meta::AsArg<GString>,
     {
         meta::arg_into_owned!(name);
@@ -196,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)
         })
     }
@@ -204,7 +222,7 @@ impl Callable {
     #[doc(hidden)]
     pub fn __once_fn<F, S>(name: S, rust_function: F) -> Self
     where
-        F: 'static + FnOnce(&[&Variant]) -> Result<Variant, ()>,
+        F: 'static + FnOnce(&[&Variant]) -> Variant,
         S: meta::AsArg<GString>,
     {
         Self::from_once_fn(name, rust_function)
@@ -213,12 +231,12 @@ impl Callable {
     pub(crate) fn with_scoped_fn<S, F, Fc, R>(name: S, rust_function: F, callable_usage: Fc) -> R
     where
         S: meta::AsArg<GString>,
-        F: FnMut(&[&Variant]) -> Result<Variant, ()>,
+        F: FnMut(&[&Variant]) -> Variant,
         Fc: FnOnce(&Callable) -> R,
     {
         meta::arg_into_owned!(name);
 
-        let callable = Self::from_fn_wrapper(FnWrapper {
+        let callable = Self::from_fn_wrapper::<F, Variant>(FnWrapper {
             rust_function,
             name,
             thread_id: Some(std::thread::current().id()),
@@ -248,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]) -> Result<Variant, ()>,
+        R: ToGodot,
+        F: 'static + Send + Sync + FnMut(&[&Variant]) -> R,
         S: meta::AsArg<GString>,
     {
         meta::arg_into_owned!(name);
 
-        Self::from_fn_wrapper(FnWrapper {
+        Self::from_fn_wrapper::<F, R>(FnWrapper {
             rust_function,
             name,
             thread_id: None,
@@ -287,9 +306,10 @@ impl Callable {
         Self::from_custom_info(info)
     }
 
-    fn from_fn_wrapper<F>(inner: FnWrapper<F>) -> Self
+    fn from_fn_wrapper<F, R>(inner: FnWrapper<F>) -> Self
     where
-        F: FnMut(&[&Variant]) -> Result<Variant, ()>,
+        F: FnMut(&[&Variant]) -> R,
+        R: ToGodot,
     {
         let object_id = inner.linked_object_id();
 
@@ -298,7 +318,7 @@ impl Callable {
         let info = CallableCustomInfo {
             object_id,
             callable_userdata: Box::into_raw(Box::new(userdata)) as *mut std::ffi::c_void,
-            call_func: Some(rust_callable_call_fn::<F>),
+            call_func: Some(rust_callable_call_fn::<F, R>),
             free_func: Some(rust_callable_destroy::<FnWrapper<F>>),
             to_string_func: Some(rust_callable_to_string_named::<F>),
             is_valid_func: Some(rust_callable_is_valid),
@@ -593,7 +613,7 @@ mod custom_callable {
         /// Return `Ok(...)` if the call succeeded, and `Err(())` otherwise.
         /// Error handling is mostly needed in case argument number or types mismatch.
         #[allow(clippy::result_unit_err)] // TODO remove once there's a clear error type here.
-        fn invoke(&mut self, args: &[&Variant]) -> Result<Variant, ()>;
+        fn invoke(&mut self, args: &[&Variant]) -> Variant;
 
         // TODO(v0.3): add object_id().
 
@@ -626,19 +646,20 @@ mod custom_callable {
             // Get the RustCallable again inside closure so it doesn't have to be UnwindSafe.
             let c: &mut C = CallableUserdata::inner_from_raw(callable_userdata);
             let result = c.invoke(arg_refs);
-            meta::varcall_return_checked(result, r_return, r_error);
+            meta::varcall_return_checked(Ok(result), r_return, r_error);
             Ok(())
         });
     }
 
-    pub unsafe extern "C" fn rust_callable_call_fn<F>(
+    pub unsafe extern "C" fn rust_callable_call_fn<F, R>(
         callable_userdata: *mut std::ffi::c_void,
         p_args: *const sys::GDExtensionConstVariantPtr,
         p_argument_count: sys::GDExtensionInt,
         r_return: sys::GDExtensionVariantPtr,
         r_error: *mut sys::GDExtensionCallError,
     ) where
-        F: FnMut(&[&Variant]) -> Result<Variant, ()>,
+        F: FnMut(&[&Variant]) -> R,
+        R: ToGodot,
     {
         let arg_refs: &[&Variant] = Variant::borrow_ref_slice(p_args, p_argument_count as usize);
 
@@ -664,8 +685,8 @@ mod custom_callable {
                 );
             }
 
-            let result = (w.rust_function)(arg_refs);
-            meta::varcall_return_checked(result, r_return, r_error);
+            let result = (w.rust_function)(arg_refs).to_variant();
+            meta::varcall_return_checked(Ok(result), r_return, r_error);
             Ok(())
         });
     }

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

@@ -710,7 +710,7 @@ impl<T: ArrayElement> Array<T> {
             let value = T::from_variant(args[0]);
             let is_less = matches!(func(&value), cmp::Ordering::Less);
 
-            Ok(is_less.to_variant())
+            is_less.to_variant()
         };
 
         let debug_name = std::any::type_name::<F>();
@@ -787,7 +787,7 @@ impl<T: ArrayElement> Array<T> {
             let rhs = T::from_variant(args[1]);
             let is_less = matches!(func(&lhs, &rhs), cmp::Ordering::Less);
 
-            Ok(is_less.to_variant())
+            is_less.to_variant()
         };
 
         let debug_name = std::any::type_name::<F>();

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);
-            Ok(Variant::nil())
         });
 
         // Use Callable::call_deferred() instead of Gd::apply_deferred(). The latter implicitly borrows &mut self,

godot-core/src/obj/gd.rs

@@ -586,7 +586,7 @@ impl<T: GodotClass> Gd<T> {
     /// If you need a Callable which can live indefinitely use [`Callable::from_local_fn()`].
     pub fn linked_callable<F>(&self, method_name: impl AsArg<GString>, rust_function: F) -> Callable
     where
-        F: 'static + FnMut(&[&Variant]) -> Result<Variant, ()>,
+        F: 'static + FnMut(&[&Variant]) -> Variant,
     {
         Callable::from_linked_fn(method_name, self, rust_function)
     }
@@ -711,7 +711,6 @@ impl<T: GodotClass> Gd<T> {
 
         let callable = Callable::from_once_fn("run_deferred", move |_| {
             gd_function(obj);
-            Ok(Variant::nil())
         });
         callable.call_deferred(&[]);
     }

godot-core/src/registry/signal/connect_builder.rs

@@ -126,7 +126,7 @@ where
     /// type state builder for simple + common connections, thus hopefully being a tiny bit lighter on compile times.
     fn inner_connect_godot_fn<F>(
         self,
-        godot_fn: impl FnMut(&[&Variant]) -> Result<Variant, ()> + 'static,
+        godot_fn: impl FnMut(&[&Variant]) -> Variant + 'static,
         bound: &Gd<impl GodotClass>,
     ) -> ConnectHandle {
         let callable_name = match &self.data.callable_name {

godot-core/src/registry/signal/mod.rs

@@ -39,16 +39,15 @@ pub mod priv_re_export {
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 
 // Used by both `TypedSignal` and `ConnectBuilder`.
-fn make_godot_fn<Ps, F>(mut input: F) -> impl FnMut(&[&Variant]) -> Result<Variant, ()>
+fn make_godot_fn<Ps, F>(mut input: F) -> impl FnMut(&[&Variant]) -> Variant
 where
     F: FnMut(Ps),
     Ps: meta::InParamTuple,
 {
-    move |variant_args: &[&Variant]| -> Result<Variant, ()> {
+    move |variant_args: &[&Variant]| {
         let args = Ps::from_variant_array(variant_args);
         input(args);
-
-        Ok(Variant::nil())
+        Variant::nil()
     }
 }
 

godot-core/src/registry/signal/typed_signal.rs

@@ -149,7 +149,7 @@ impl<'c, C: WithSignals, Ps: meta::ParamTuple> TypedSignal<'c, C, Ps> {
     /// type state builder for simple + common connections, thus hopefully being a tiny bit lighter on compile times.
     fn inner_connect_godot_fn<F>(
         &self,
-        godot_fn: impl FnMut(&[&Variant]) -> Result<Variant, ()> + 'static,
+        godot_fn: impl FnMut(&[&Variant]) -> Variant + 'static,
         bound: &Gd<impl GodotClass>,
     ) -> ConnectHandle {
         let callable_name = make_callable_name::<F>();

godot-core/src/task/async_runtime.rs

@@ -500,7 +500,7 @@ impl Wake for GodotWaker {
         /// This appears to be a common issue: https://github.com/rust-lang/rust/issues/89976
         fn callback_type_hint<F>(f: F) -> F
         where
-            F: for<'a> FnMut(&'a [&Variant]) -> Result<Variant, ()>,
+            F: for<'a> FnMut(&'a [&Variant]) -> Variant,
         {
             f
         }
@@ -515,7 +515,7 @@ impl Wake for GodotWaker {
             "GodotWaker::wake",
             callback_type_hint(move |_args| {
                 poll_future(waker.take().expect("Callable will never be called again"));
-                Ok(Variant::nil())
+                Variant::nil()
             }),
         );
 

godot-core/src/task/futures.rs

@@ -115,7 +115,7 @@ impl<R: IntoDynamicSend> PartialEq for SignalFutureResolver<R> {
 }
 
 impl<R: InParamTuple + IntoDynamicSend> RustCallable for SignalFutureResolver<R> {
-    fn invoke(&mut self, args: &[&Variant]) -> Result<Variant, ()> {
+    fn invoke(&mut self, args: &[&Variant]) -> Variant {
         let waker = {
             let mut data = self.data.lock().unwrap();
             data.state = SignalFutureState::Ready(R::from_variant_array(args).into_dynamic_send());
@@ -128,7 +128,7 @@ impl<R: InParamTuple + IntoDynamicSend> RustCallable for SignalFutureResolver<R>
             waker.wake();
         }
 
-        Ok(Variant::nil())
+        Variant::nil()
     }
 }
 

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>();
-        Ok(res.to_variant())
+    // No &[&Variant] explicit type in arguments.
+    Callable::from_local_fn("sort backwards", |args| {
+        args[0].to::<i32>() > args[1].to::<i32>()
     })
 }