Remove Result from custom callables

Author: Bromeon

godot-core/src/builtin/callable.rs

@@ -111,7 +111,7 @@ impl Callable {
                 &function_name,
                 args.as_slice(),
             );
-            Ok(result)
+            result
         })
     }
 
@@ -139,14 +139,16 @@ 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
+    #[cfg(since_api = "4.2")]
+    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 +167,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()),
@@ -186,7 +188,7 @@ impl Callable {
     /// 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
     where
-        F: 'static + FnOnce(&[&Variant]) -> Result<Variant, ()>,
+        F: 'static + FnOnce(&[&Variant]) -> Variant,
         S: meta::AsArg<GString>,
     {
         meta::arg_into_owned!(name);
@@ -204,7 +206,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 +215,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()),
@@ -250,12 +252,12 @@ impl Callable {
     #[cfg(feature = "experimental-threads")]
     pub fn from_sync_fn<F, S>(name: S, rust_function: F) -> Self
     where
-        F: 'static + Send + Sync + FnMut(&[&Variant]) -> Result<Variant, ()>,
+        F: 'static + Send + Sync + 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: None,
@@ -287,9 +289,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 +301,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 +596,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 +629,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 +668,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

@@ -226,7 +226,7 @@ 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())
+            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,7 @@ impl<T: GodotClass> Gd<T> {
 
         let callable = Callable::from_once_fn("run_deferred", move |_| {
             gd_function(obj);
-            Ok(Variant::nil())
+            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

@@ -564,7 +564,7 @@ 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())
+        res.to_variant()
     })
 }