Merge pull request #1346 from godot-rust/qol/deprecated-from-local-fn

`Callable`: rename `from_local_fn` -> `from_fn`, keep deprecated `from_local_fn` with old signature

Author: Bromeon

godot-core/src/builtin/callable.rs

@@ -116,7 +116,7 @@ impl Callable {
         let callable = Self::from_sync_fn(&callable_name, function);
 
         #[cfg(not(feature = "experimental-threads"))]
-        let callable = Self::from_local_fn(&callable_name, function);
+        let callable = Self::from_fn(&callable_name, function);
 
         callable
     }
@@ -154,7 +154,9 @@ 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<R, F, S>(name: S, rust_function: F) -> Self
+    ///
+    /// You can also couple the callable to the lifetime of an object, see [`from_linked_fn()`][Self::from_linked_fn].
+    pub fn from_fn<R, F, S>(name: S, rust_function: F) -> Self
     where
         R: ToGodot,
         F: 'static + FnMut(&[&Variant]) -> R,
@@ -177,7 +179,7 @@ impl Callable {
     /// Such a callable will be automatically invalidated by Godot when a linked object is freed.
     /// Prefer using [`Gd::linked_callable()`] instead.
     ///
-    /// If you need a callable which can live indefinitely use [`Callable::from_local_fn()`].
+    /// If you need a callable which can live indefinitely, use [`Callable::from_fn()`].
     pub fn from_linked_fn<R, F, T, S>(name: S, linked_object: &Gd<T>, rust_function: F) -> Self
     where
         R: ToGodot,
@@ -195,12 +197,29 @@ impl Callable {
         })
     }
 
+    /// This constructor is being phased out in favor of [`from_fn()`][Self::from_fn], but kept through v0.4 for smoother migration.
+    ///
+    /// `from_fn()` accepts any `R: ToGodot` return type directly instead of requiring `Result<Variant, ()>`.
+    #[deprecated = "Migrate to `from_fn()`, which returns `R: ToGodot` directly."]
+    pub fn from_local_fn<F, S>(name: S, mut rust_function: F) -> Self
+    where
+        F: 'static + FnMut(&[&Variant]) -> Result<Variant, ()>,
+        S: meta::AsArg<GString>,
+    {
+        meta::arg_into_owned!(name);
+
+        Self::from_fn(&name, move |args| {
+            // Ignore errors.
+            rust_function(args).unwrap_or_else(|()| Variant::nil())
+        })
+    }
+
     /// Create callable from **single-threaded** Rust function or closure that can only be called once.
     ///
     /// `name` is used for the string representation of the closure, which helps debugging.
     ///
     /// 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].
+    /// useful for deferred operations that should only execute once. For repeated execution, use [`from_fn()][Self::from_fn].
     pub(crate) fn from_once_fn<R, F, S>(name: S, rust_function: F) -> Self
     where
         R: ToGodot,
@@ -210,7 +229,7 @@ impl Callable {
         meta::arg_into_owned!(name);
 
         let mut rust_fn_once = Some(rust_function);
-        Self::from_local_fn(&name, move |args| {
+        Self::from_fn(&name, move |args| {
             let rust_fn_once = rust_fn_once
                 .take()
                 .expect("callable created with from_once_fn() has already been consumed");
@@ -252,9 +271,9 @@ impl Callable {
     /// `name` is used for the string representation of the closure, which helps debugging.
     ///
     /// This constructor requires `Send` + `Sync` bound and allows the callable to be invoked from any thread. If you guarantee that you invoke
-    /// it from the same thread as creating it, use [`from_local_fn`][Self::from_local_fn] instead.
+    /// it from the same thread as creating it, use [`from_fn`][Self::from_fn] instead.
     ///
-    /// Callables created through multiple `from_local_fn` or `from_sync_fn()` calls are never equal, even if they refer to the same function.
+    /// Callables created through multiple `from_fn` or `from_sync_fn()` calls are never equal, even if they refer to the same function.
     /// If you want to use equality, either clone an existing `Callable` instance, or define your own `PartialEq` impl with
     /// [`Callable::from_custom`].
     ///
@@ -680,7 +699,7 @@ mod custom_callable {
                 // NOTE: this panic is currently not propagated to the caller, but results in an error message and Nil return.
                 // See comments in itest callable_call() for details.
                 panic!(
-                    "Callable '{}' created with from_local_fn() must be called from the same thread it was created in.\n\
+                    "Callable '{}' created with from_fn() must be called from the same thread it was created in.\n\
                     If you need to call it from any thread, use from_sync_fn() instead (requires `experimental-threads` feature).",
                     w.name
                 );

godot-core/src/obj/gd.rs

@@ -583,7 +583,7 @@ impl<T: GodotClass> Gd<T> {
     /// `name` is used for the string representation of the closure, which helps with debugging.
     ///
     /// Such a callable will be automatically invalidated by Godot when a linked Object is freed.
-    /// If you need a Callable which can live indefinitely use [`Callable::from_local_fn()`].
+    /// If you need a Callable which can live indefinitely, use [`Callable::from_fn()`].
     pub fn linked_callable<R, F>(
         &self,
         method_name: impl AsArg<GString>,

godot-core/src/task/async_runtime.rs

@@ -506,7 +506,7 @@ impl Wake for GodotWaker {
         }
 
         #[cfg(not(feature = "experimental-threads"))]
-        let create_callable = Callable::from_local_fn;
+        let create_callable = Callable::from_fn;
 
         #[cfg(feature = "experimental-threads")]
         let create_callable = Callable::from_sync_fn;

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

@@ -563,7 +563,7 @@ fn array_bsearch_custom() {
 
 fn backwards_sort_callable() -> Callable {
     // No &[&Variant] explicit type in arguments.
-    Callable::from_local_fn("sort backwards", |args| {
+    Callable::from_fn("sort backwards", |args| {
         args[0].to::<i32>() > args[1].to::<i32>()
     })
 }

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

@@ -382,8 +382,8 @@ pub mod custom_callable {
     use crate::framework::{assert_eq_self, quick_thread, suppress_panic_log, ThreadCrosser};
 
     #[itest]
-    fn callable_from_local_fn() {
-        let callable = Callable::from_local_fn("sum", sum);
+    fn callable_from_fn() {
+        let callable = Callable::from_fn("sum", sum);
 
         assert!(callable.is_valid());
         assert!(!callable.is_null());
@@ -398,14 +398,14 @@ pub mod custom_callable {
         assert_eq!(sum2, 0.to_variant());
     }
 
-    // Without this feature, any access to the global binding from another thread fails; so the from_local_fn() cannot be tested in isolation.
+    // Without this feature, any access to the global binding from another thread fails; so the from_fn() cannot be tested in isolation.
     #[itest]
-    fn callable_from_local_fn_crossthread() {
+    fn callable_from_fn_crossthread() {
         // This static is a workaround for not being able to propagate failed `Callable` invocations as panics.
         // See note in itest callable_call() for further info.
         static GLOBAL: sys::Global<i32> = sys::Global::default();
 
-        let callable = Callable::from_local_fn("change_global", |_args| {
+        let callable = Callable::from_fn("change_global", |_args| {
             *GLOBAL.lock() = 777;
         });
 
@@ -423,7 +423,7 @@ pub mod custom_callable {
         if !cfg!(feature = "experimental-threads") && cfg!(debug_assertions) {
             // Single-threaded and Debug.
             crate::framework::expect_panic(
-                "Callable created with from_local_fn() must panic when invoked on other thread",
+                "Callable created with from_fn() must panic when invoked on other thread",
                 || {
                     quick_thread(|| {
                         let callable = unsafe { crosser.extract() };
@@ -442,7 +442,7 @@ pub mod custom_callable {
         assert_eq!(
             *GLOBAL.lock(),
             0,
-            "Callable created with from_local_fn() must not run when invoked on other thread"
+            "Callable created with from_fn() must not run when invoked on other thread"
         );
     }
 
@@ -469,16 +469,16 @@ pub mod custom_callable {
 
     #[itest]
     fn callable_from_fn_nil() {
-        let callable_with_err = Callable::from_local_fn("returns_nil", |_args: &[&Variant]| {});
+        let callable_with_err = Callable::from_fn("returns_nil", |_args: &[&Variant]| {});
 
         assert_eq!(callable_with_err.callv(&varray![]), Variant::nil());
     }
 
     #[itest]
     fn callable_from_fn_eq() {
-        let a = Callable::from_local_fn("sum", sum);
+        let a = Callable::from_fn("sum", sum);
         let b = a.clone();
-        let c = Callable::from_local_fn("sum", sum);
+        let c = Callable::from_fn("sum", sum);
 
         assert_eq!(a, b, "same function, same instance -> equal");
         assert_ne!(a, c, "same function, different instance -> not equal");
@@ -585,7 +585,7 @@ 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| {
+        let callable = Callable::from_fn("test", move |_args| {
             suppress_panic_log(|| {
                 panic!("TEST: {}", received_callable.fetch_add(1, Ordering::SeqCst))
             });

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

@@ -821,7 +821,7 @@ 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 {
+        Callable::from_fn("test", move |_args| -> Variant {
             panic!("TEST: {}", received.fetch_add(1, Ordering::SeqCst))
         })
     }

itest/rust/src/engine_tests/async_test.rs

@@ -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", |_| {});
+    let unrelated_callable = Callable::from_fn("unrelated", |_| {});
 
     assert_eq!(callable, cloned_callable);
     assert_ne!(callable, unrelated_callable);

itest/rust/src/framework/runner.rs

@@ -473,7 +473,7 @@ fn check_async_test_task(
     let mut callback = Some(on_test_finished);
     let mut probably_task_handle = Some(task_handle);
 
-    let deferred = Callable::from_local_fn("run_async_rust_test", move |_| {
+    let deferred = Callable::from_fn("run_async_rust_test", move |_| {
         check_async_test_task(
             probably_task_handle
                 .take()