bevy_reflect: Store functions as DynamicClosure<'static> in `Functi…

…onRegistry` (bevyengine#14704)

# Objective

bevyengine#14098 added the `FunctionRegistry` for registering functions such that
they can be retrieved by name and used dynamically. One thing we chose
to leave out in that initial PR is support for closures.

Why support closures? Mainly, we don't want to prohibit users from
injecting environmental data into their registered functions. This
allows these functions to not leak their internals to the public API.

For example, let's say we're writing a library crate that allows users
to register callbacks for certain actions. We want to perform some
actions before invoking the user's callback so we can't just call it
directly. We need a closure for this:

```rust
registry.register("my_lib::onclick", move |event: ClickEvent| {
    // ...other work...

    user_onclick.call(event); // <-- Captured variable
});
```

We could have made our callback take a reference to the user's callback.
This would remove the need for the closure, but it would change our
desired API to place the burden of fetching the correct callback on the
caller.

## Solution

Modify the `FunctionRegistry` to store registered functions as
`DynamicClosure<'static>` instead of `DynamicFunction` (now using
`IntoClosure` instead of `IntoFunction`).

Due to limitations in Rust and how function reflection works,
`DynamicClosure<'static>` is functionally equivalent to
`DynamicFunction`. And a normal function is considered a subset of
closures (it's a closure that doesn't capture anything), so there
shouldn't be any difference in usage: all functions that satisfy
`IntoFunction` should satisfy `IntoClosure`.

This means that the registration API introduced in bevyengine#14098 should require
little-to-no changes on anyone following `main`.

### Closures vs Functions

One consideration here is whether we should keep closures and functions
separate.

This PR unifies them into `DynamicClosure<'static>`, but we can consider
splitting them up. The reasons we might want to do so are:

- Simplifies mental model and terminology (users don't have to
understand that functions turn into closures)
- If Rust ever improves its function model, we may be able to add
additional guarantees to `DynamicFunction` that make it useful to
separate the two
- Adding support for generic functions may be less confusing for users
since closures in Rust technically can't be generic

The reasons behind this PR's unification approach are:

- Reduces the number of methods needed on `FunctionRegistry`
- Reduces the number of lookups a user may have to perform (i.e.
"`get_function` or else `get_closure`")
- Establishes `DynamicClosure<'static>` as the de facto dynamic callable
(similar to how most APIs in Rust code tend to prefer `impl Fn() ->
String` over `fn() -> String`)

I'd love to hear feedback on this matter, and whether we should continue
with this PR's approach or switch to a split model.

## Testing

You can test locally by running:

```
cargo test --package bevy_reflect
```

---

## Showcase

Closures can now be registered into the `FunctionRegistry`:

```rust
let punct = String::from("!!!");

registry.register_with_name("my_crate::punctuate", move |text: String| {
  format!("{}{}", text, punct)
});
```

crates/bevy_app/src/app.rs

@@ -605,16 +605,16 @@ impl App {
 
     /// Registers the given function into the [`AppFunctionRegistry`] resource.
     ///
-    /// The given function will internally be stored as a [`DynamicFunction`]
+    /// The given function will internally be stored as a [`DynamicClosure`]
     /// and mapped according to its [name].
     ///
     /// Because the function must have a name,
-    /// anonymous functions (e.g. `|a: i32, b: i32| { a + b }`) must instead
-    /// be registered using [`register_function_with_name`] or converted to a [`DynamicFunction`]
-    /// and named using [`DynamicFunction::with_name`].
-    /// Failure to do so will result in an error being returned.
+    /// anonymous functions (e.g. `|a: i32, b: i32| { a + b }`) and closures must instead
+    /// be registered using [`register_function_with_name`] or converted to a [`DynamicClosure`]
+    /// and named using [`DynamicClosure::with_name`].
+    /// Failure to do so will result in a panic.
     ///
-    /// Only functions that implement [`IntoFunction`] may be registered via this method.
+    /// Only types that implement [`IntoClosure`] may be registered via this method.
     ///
     /// See [`FunctionRegistry::register`] for more information.
     ///
@@ -650,7 +650,7 @@ impl App {
     ///     .register_function(add);
     /// ```
     ///
-    /// Anonymous functions should be registered using [`register_function_with_name`] or given a name using [`DynamicFunction::with_name`].
+    /// Anonymous functions and closures should be registered using [`register_function_with_name`] or given a name using [`DynamicClosure::with_name`].
     ///
     /// ```should_panic
     /// use bevy_app::App;
@@ -660,21 +660,21 @@ impl App {
     /// ```
     ///
     /// [`register_function_with_name`]: Self::register_function_with_name
-    /// [`DynamicFunction`]: bevy_reflect::func::DynamicFunction
+    /// [`DynamicClosure`]: bevy_reflect::func::DynamicClosure
     /// [name]: bevy_reflect::func::FunctionInfo::name
-    /// [`DynamicFunction::with_name`]: bevy_reflect::func::DynamicFunction::with_name
-    /// [`IntoFunction`]: bevy_reflect::func::IntoFunction
+    /// [`DynamicClosure::with_name`]: bevy_reflect::func::DynamicClosure::with_name
+    /// [`IntoClosure`]: bevy_reflect::func::IntoClosure
     /// [`FunctionRegistry::register`]: bevy_reflect::func::FunctionRegistry::register
     #[cfg(feature = "reflect_functions")]
     pub fn register_function<F, Marker>(&mut self, function: F) -> &mut Self
     where
-        F: bevy_reflect::func::IntoFunction<Marker> + 'static,
+        F: bevy_reflect::func::IntoClosure<'static, Marker> + 'static,
     {
         self.main_mut().register_function(function);
         self
     }
 
-    /// Registers the given function into the [`AppFunctionRegistry`] resource using the given name.
+    /// Registers the given function or closure into the [`AppFunctionRegistry`] resource using the given name.
     ///
     /// To avoid conflicts, it's recommended to use a unique name for the function.
     /// This can be achieved by "namespacing" the function with a unique identifier,
@@ -689,7 +689,7 @@ impl App {
     /// For named functions (e.g. `fn add(a: i32, b: i32) -> i32 { a + b }`) where a custom name is not needed,
     /// it's recommended to use [`register_function`] instead as the generated name is guaranteed to be unique.
     ///
-    /// Only functions that implement [`IntoFunction`] may be registered via this method.
+    /// Only types that implement [`IntoClosure`] may be registered via this method.
     ///
     /// See [`FunctionRegistry::register_with_name`] for more information.
     ///
@@ -718,7 +718,7 @@ impl App {
     ///     // Registering an existing function with a custom name
     ///     .register_function_with_name("my_crate::mul", mul)
     ///     // Be careful not to register anonymous functions with their type name.
-    ///     // This code works but registers the function with the non-unique name of `fn(i32, i32) -> i32`
+    ///     // This code works but registers the function with a non-unique name like `foo::bar::{{closure}}`
     ///     .register_function_with_name(std::any::type_name_of_val(&div), div);
     /// ```
     ///
@@ -738,7 +738,7 @@ impl App {
     ///
     /// [type name]: std::any::type_name
     /// [`register_function`]: Self::register_function
-    /// [`IntoFunction`]: bevy_reflect::func::IntoFunction
+    /// [`IntoClosure`]: bevy_reflect::func::IntoClosure
     /// [`FunctionRegistry::register_with_name`]: bevy_reflect::func::FunctionRegistry::register_with_name
     #[cfg(feature = "reflect_functions")]
     pub fn register_function_with_name<F, Marker>(
@@ -747,7 +747,7 @@ impl App {
         function: F,
     ) -> &mut Self
     where
-        F: bevy_reflect::func::IntoFunction<Marker> + 'static,
+        F: bevy_reflect::func::IntoClosure<'static, Marker> + 'static,
     {
         self.main_mut().register_function_with_name(name, function);
         self

crates/bevy_app/src/sub_app.rs

@@ -413,7 +413,7 @@ impl SubApp {
     #[cfg(feature = "reflect_functions")]
     pub fn register_function<F, Marker>(&mut self, function: F) -> &mut Self
     where
-        F: bevy_reflect::func::IntoFunction<Marker> + 'static,
+        F: bevy_reflect::func::IntoClosure<'static, Marker> + 'static,
     {
         let registry = self.world.resource_mut::<AppFunctionRegistry>();
         registry.write().register(function).unwrap();
@@ -428,7 +428,7 @@ impl SubApp {
         function: F,
     ) -> &mut Self
     where
-        F: bevy_reflect::func::IntoFunction<Marker> + 'static,
+        F: bevy_reflect::func::IntoClosure<'static, Marker> + 'static,
     {
         let registry = self.world.resource_mut::<AppFunctionRegistry>();
         registry.write().register_with_name(name, function).unwrap();

crates/bevy_reflect/src/func/closures/dynamic_closure.rs

@@ -1,9 +1,11 @@
-use alloc::borrow::Cow;
-use core::fmt::{Debug, Formatter};
-
 use crate::func::args::{ArgInfo, ArgList};
 use crate::func::info::FunctionInfo;
-use crate::func::{FunctionResult, IntoClosure, ReturnInfo};
+use crate::func::{
+    DynamicClosureMut, DynamicFunction, FunctionResult, IntoClosure, IntoClosureMut, ReturnInfo,
+};
+use alloc::borrow::Cow;
+use core::fmt::{Debug, Formatter};
+use std::sync::Arc;
 
 /// A dynamic representation of a Rust closure.
 ///
@@ -42,12 +44,9 @@ use crate::func::{FunctionResult, IntoClosure, ReturnInfo};
 /// // Check the result:
 /// assert_eq!(value.try_take::<String>().unwrap(), "Hello, world!!!");
 /// ```
-///
-/// [`DynamicClosureMut`]: crate::func::closures::DynamicClosureMut
-/// [`DynamicFunction`]: crate::func::DynamicFunction
 pub struct DynamicClosure<'env> {
-    info: FunctionInfo,
-    func: Box<dyn for<'a> Fn(ArgList<'a>) -> FunctionResult<'a> + 'env>,
+    pub(super) info: FunctionInfo,
+    pub(super) func: Arc<dyn for<'a> Fn(ArgList<'a>) -> FunctionResult<'a> + Send + Sync + 'env>,
 }
 
 impl<'env> DynamicClosure<'env> {
@@ -57,13 +56,13 @@ impl<'env> DynamicClosure<'env> {
     ///
     /// It's important that the closure signature matches the provided [`FunctionInfo`].
     /// This info may be used by consumers of the function for validation and debugging.
-    pub fn new<F: for<'a> Fn(ArgList<'a>) -> FunctionResult<'a> + 'env>(
+    pub fn new<F: for<'a> Fn(ArgList<'a>) -> FunctionResult<'a> + Send + Sync + 'env>(
         func: F,
         info: FunctionInfo,
     ) -> Self {
         Self {
             info,
-            func: Box::new(func),
+            func: Arc::new(func),
         }
     }
 
@@ -160,13 +159,39 @@ impl<'env> Debug for DynamicClosure<'env> {
     }
 }
 
+impl<'env> Clone for DynamicClosure<'env> {
+    fn clone(&self) -> Self {
+        Self {
+            info: self.info.clone(),
+            func: Arc::clone(&self.func),
+        }
+    }
+}
+
+impl From<DynamicFunction> for DynamicClosure<'static> {
+    #[inline]
+    fn from(func: DynamicFunction) -> Self {
+        Self {
+            info: func.info,
+            func: func.func,
+        }
+    }
+}
+
 impl<'env> IntoClosure<'env, ()> for DynamicClosure<'env> {
     #[inline]
     fn into_closure(self) -> DynamicClosure<'env> {
         self
     }
 }
 
+impl<'env> IntoClosureMut<'env, ()> for DynamicClosure<'env> {
+    #[inline]
+    fn into_closure_mut(self) -> DynamicClosureMut<'env> {
+        DynamicClosureMut::from(self)
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -190,4 +215,26 @@ mod tests {
         let closure: DynamicClosure = make_closure(|a: i32, b: i32| a + b + c);
         let _: DynamicClosure = make_closure(closure);
     }
+
+    #[test]
+    fn should_clone_dynamic_closure() {
+        let hello = String::from("Hello");
+
+        let greet = |name: &String| -> String { format!("{}, {}!", hello, name) };
+
+        let greet = greet.into_closure().with_name("greet");
+        let clone = greet.clone();
+
+        assert_eq!(greet.name().unwrap(), "greet");
+        assert_eq!(clone.name().unwrap(), "greet");
+
+        let clone_value = clone
+            .call(ArgList::default().push_ref(&String::from("world")))
+            .unwrap()
+            .unwrap_owned()
+            .try_take::<String>()
+            .unwrap();
+
+        assert_eq!(clone_value, "Hello, world!");
+    }
 }

crates/bevy_reflect/src/func/closures/dynamic_closure_mut.rs

@@ -3,7 +3,7 @@ use core::fmt::{Debug, Formatter};
 
 use crate::func::args::{ArgInfo, ArgList};
 use crate::func::info::FunctionInfo;
-use crate::func::{FunctionResult, IntoClosureMut, ReturnInfo};
+use crate::func::{DynamicClosure, DynamicFunction, FunctionResult, IntoClosureMut, ReturnInfo};
 
 /// A dynamic representation of a Rust closure.
 ///
@@ -51,9 +51,6 @@ use crate::func::{FunctionResult, IntoClosureMut, ReturnInfo};
 /// drop(func);
 /// assert_eq!(list, vec![1, -2, 3]);
 /// ```
-///
-/// [`DynamicClosure`]: crate::func::closures::DynamicClosure
-/// [`DynamicFunction`]: crate::func::DynamicFunction
 pub struct DynamicClosureMut<'env> {
     info: FunctionInfo,
     func: Box<dyn for<'a> FnMut(ArgList<'a>) -> FunctionResult<'a> + 'env>,
@@ -202,6 +199,26 @@ impl<'env> Debug for DynamicClosureMut<'env> {
     }
 }
 
+impl From<DynamicFunction> for DynamicClosureMut<'static> {
+    #[inline]
+    fn from(func: DynamicFunction) -> Self {
+        Self {
+            info: func.info,
+            func: Box::new(move |args| (func.func)(args)),
+        }
+    }
+}
+
+impl<'env> From<DynamicClosure<'env>> for DynamicClosureMut<'env> {
+    #[inline]
+    fn from(closure: DynamicClosure<'env>) -> Self {
+        Self {
+            info: closure.info,
+            func: Box::new(move |args| (closure.func)(args)),
+        }
+    }
+}
+
 impl<'env> IntoClosureMut<'env, ()> for DynamicClosureMut<'env> {
     #[inline]
     fn into_closure_mut(self) -> DynamicClosureMut<'env> {

crates/bevy_reflect/src/func/closures/into_closure.rs

@@ -18,7 +18,7 @@ pub trait IntoClosure<'env, Marker> {
 
 impl<'env, F, Marker1, Marker2> IntoClosure<'env, (Marker1, Marker2)> for F
 where
-    F: ReflectFn<'env, Marker1> + TypedFunction<Marker2> + 'env,
+    F: ReflectFn<'env, Marker1> + TypedFunction<Marker2> + Send + Sync + 'env,
 {
     fn into_closure(self) -> DynamicClosure<'env> {
         DynamicClosure::new(move |args| self.reflect_call(args), Self::function_info())

crates/bevy_reflect/src/func/function.rs

@@ -4,7 +4,10 @@ use std::sync::Arc;
 
 use crate::func::args::{ArgInfo, ArgList};
 use crate::func::info::FunctionInfo;
-use crate::func::{FunctionResult, IntoFunction, ReturnInfo};
+use crate::func::{
+    DynamicClosure, DynamicClosureMut, FunctionResult, IntoClosure, IntoClosureMut, IntoFunction,
+    ReturnInfo,
+};
 
 /// A dynamic representation of a Rust function.
 ///
@@ -88,11 +91,10 @@ use crate::func::{FunctionResult, IntoFunction, ReturnInfo};
 /// assert_eq!(list, vec!["Hello, World!!!"]);
 /// ```
 ///
-/// [`DynamicClosure`]: crate::func::DynamicClosure
 /// [module-level documentation]: crate::func
 pub struct DynamicFunction {
-    info: FunctionInfo,
-    func: Arc<dyn for<'a> Fn(ArgList<'a>) -> FunctionResult<'a> + Send + Sync + 'static>,
+    pub(super) info: FunctionInfo,
+    pub(super) func: Arc<dyn for<'a> Fn(ArgList<'a>) -> FunctionResult<'a> + Send + Sync + 'static>,
 }
 
 impl DynamicFunction {
@@ -221,6 +223,20 @@ impl IntoFunction<()> for DynamicFunction {
     }
 }
 
+impl IntoClosure<'_, ()> for DynamicFunction {
+    #[inline]
+    fn into_closure(self) -> DynamicClosure<'static> {
+        DynamicClosure::from(self)
+    }
+}
+
+impl IntoClosureMut<'_, ()> for DynamicFunction {
+    #[inline]
+    fn into_closure_mut(self) -> DynamicClosureMut<'static> {
+        DynamicClosureMut::from(self)
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;