bevy_reflect: Store functions as DynamicClosure<'static> in FunctionRegistry

[MrGVSV]

Objective

#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:

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 #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:

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

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

[mweatherley]

In my mind, the changes here to DynamicClosure make DynamicFunction have basically no purpose, since DynamicFunction is now, as far as I can tell, literally isomorphic to DynamicClosure<'static>. I kind of always regarded the function/closure distinction as one without a substantive difference underlying it, so I'm basically in favor of unifying the two in the long run even if we don't do it here.

[MrGVSV]

In my mind, the changes here to DynamicClosure make DynamicFunction have basically no purpose, since DynamicFunction is now, as far as I can tell, literally isomorphic to DynamicClosure<'static>. I kind of always regarded the function/closure distinction as one without a substantive difference underlying it, so I'm basically in favor of unifying the two in the long run even if we don't do it here.

Yeah I think it's really unfortunate we can't get any additional benefits out of DynamicFunction. Theoretically it should be trivially Copy + Send + Sync + ..., but the issue is that IntoFunction needs to move self into the wrapped function.

In fact, I think that's the only major issue wrt improving DynamicFunction. We can make the manual constructor take fn instead of Fn, but we can't utilize that for IntoFunction (or for wrapping fn pointers).

But I think we should save the possible removal/refactor of DynamicFunction for a future PR.

[alice-i-cecile]

CI is failing on merge: https://github.com/bevyengine/bevy/actions/runs/10405781995/job/28817325133

[MrGVSV]

As an update, once this is merged, I'm going to refactor function reflection to get rid of the DynamicFunction/DynamicClosure issues (relevant Discord thread).