bevy_reflect: Function registry

[MrGVSV]

[!note] Blocked on #14141

Objective

#13152 added support for reflecting functions. Now, we need a way to register those functions such that they may be accessed anywhere within the ECS.

Solution

Added a FunctionRegistry type similar to TypeRegistry.

This allows a function to be registered and retrieved by name.

fn foo() -> i32 {
    123
}

let mut registry = FunctionRegistry::default();
registry.register(foo);

let function = registry.get_mut(std::any::type_name_of_val(&foo)).unwrap();
let value = function.call(ArgList::new()).unwrap().unwrap_owned();
assert_eq!(value.downcast_ref::<i32>(), Some(&123));

Additionally, I added an AppFunctionRegistry resource which wraps a FunctionRegistryArc. Functions can be registered into this resource using App::register_function or by getting a mutable reference to the resource itself.

Limitations

Send + Sync

In order to get this registry to work across threads, it needs to be Send + Sync. This means that DynamicFunction needs to be Send + Sync, which means that its internal function also needs to be Send + Sync.

In most cases, this won't be an issue because standard Rust functions (the type most likely to be registered) are always Send + Sync. Additionally, closures tend to be Send + Sync as well, granted they don't capture any !Send or !Sync variables.

This PR adds this Send + Sync requirement, but as mentioned above, it hopefully shouldn't be too big of an issue.

FnMut

Right now, DynamicFunction wraps a Box<dyn FnMut>. This closure is used to call parse the given ArgList and call the intended function. However, because it is an FnMut closure, it requires a mutable reference to self in order to be called.

This mutability requirement makes calling these functions a bit harder. For example, in order to call any registered function, we actually need a mutable reference to the registry. This is obviously not very ideal.

Thankfully we can likely improve this in a subsequent PR. dyn FnMut was originally used so that manual constructions of DynamicFunction could do more complex logic in these closures. But we can actually just use dyn Fn instead for most cases. Closures that need to mutate their environment will probably need a separate DynamicClosure type (again, to be explored in a followup PR).

Future Work

Besides addressing the limitations listed above, another thing we could look into is improving the lookup of registered functions. One aspect is in the performance of hashing strings. The other is in the developer experience of having to call std::any::type_name_of_val to get the name of their function (assuming they didn't give it a custom name).

Testing

You can run the tests locally with:

cargo test --package bevy_reflect

---

Changelog

• Added FunctionRegistry
• Added AppFunctionRegistry (a Resource available from bevy_ecs)
• Added FunctionRegistryArc
• DynamicFunction now requires its wrapped function be Send + Sync

Internal Migration Guide

[!important] Function reflection was introduced as part of the 0.15 dev cycle. This migration guide was written for developers relying on main during this cycle, and is not a breaking change coming from 0.14.

DynamicFunction (both those created manually and those created with IntoFunction), now require Send + Sync. All standard Rust functions should meet that requirement. Closures, on the other hand, may not if they capture any !Send or !Sync variables from its environment.

[BD103]

Is there any better alternative to use beyond type_name()? (Maybe TypeId?) The type name is not guaranteed to be unique, so there's a chance multiple functions could collide.

"The returned string must not be considered to be a unique identifier of a type as multiple types may map to the same type name." - type_name() docs

[MrGVSV]

Is there any better alternative to use beyond type_name()? (Maybe TypeId?) The type name is not guaranteed to be unique, so there's a chance multiple functions could collide.

"The returned string must not be considered to be a unique identifier of a type as multiple types may map to the same type name."

• type_name() docs

Great question! I probably should have mentioned the reasoning for using type_name here.

Ideally, yes, we'd use a TypeId. Unfortunately, there's a few issues with this:

~~1. DynamicFunction can be created manually and do not need to be tied to any one function. This is because it essentially holds a wrapper function ((ArgList, FunctionInfo) -> FunctionResult) that reifies all the reflected arguments. A user can choose to do their logic their, not needing a dedicated function, or they can choose to call multiple functions. This means that we can't guarantee there's always exactly one TypeId to represent the function. 2. We can't pull the TypeId off the closure that calls to the actual function either, since each instance of that closure would be unique. This is an issue because it means that foo.into_function() won't have the same TypeId as another call to foo.into_function() 3. We don't want to have the registry itself extract the TypeId from the given function since it accepts DynamicFunction, which results in the same problems as above (note that we really should keep the ability to register a DynamicFunction directly since we're very limited by Rust in how many functions we can realistically have implement IntoFunction).~~

Edit: While I was typing this I ended up doing a couple quick tests regarding that second point and I'm completely wrong lol. It turns out it does result in the same TypeId every time!

So it is possible to instead store these by their TypeId. The only issue right now is that it has to be done in the constructor of DynamicFunction since we have to have access to the actual type of the closure, F, and it only works if F has a 'static lifetime.

So this should theoretically be possible with #14141 since it adds the requirement that DynamicFunction is 'static. Maybe we wait until that one is merged?

[BD103]

So this should theoretically be possible with #14141 since it adds the requirement that DynamicFunction is 'static. Maybe we wait until that one is merged?

I would prefer that, since TypeId is guaranteed unique and not intended specifically for debugging.

[NthTensor]

I think it makes sense to hold off on this until after the DynamicClosure split.

[MrGVSV]

Okay so since function reflection is now behind a feature, I had to add corresponding features to bevy_app and bevy_ecs. Hopefully, that's okay!

[MrGVSV]

I think we should probably document somewhere that the blessed way of registering a function with a chosen name into the registry is manually calling into_function and relying on the no-op IntoFunction implementation of DynamicFunction.

I think I'll hold off on this until we figure out (in followup PRs):

1. Closure registration
2. Short names

Until then, I don't think it really matters whether or not you call into_function first since you'd really only do that to change the name of the function. But regular functions should be fine just using their type_name.

[MrGVSV]

Update: I brought this up on Discord but there's actually a potential problem here: anonymous functions.

While we can sorta[^1] prevent closures and their foo::bar::{{closure}} type names from being registered, we can't really do that for anonymous functions— at least, not at compile-time.

Unlike regular functions, anonymous functions don't have a standard type name. For example, the anonymous function |a: i32, b: i32| a + b would have a type name of fn(i32, i32) -> i32. This makes conflicts super easy to run into (think of all the plain fn() functions that might be registered!).

We have a few options:

1. Just document this potential footgun. Users can then resolve the problem by converting their anonymous function to a DynamicFunction and giving it a proper name.

   registry.register(
     (|a: i32, b: i32| a + b)
       .into_function()
       .with_name("my_crate::add")
   );
   

2. Make FunctionInfo::name optional again and have the registration return a FunctionRegistrationError::MissingName error. This would also require users to update their code like the snippet above.
3. Force users to register all functions with a custom name.

   registry.register("my_crate::add", |a: i32, b: i32| a + b);
   

4. ???. There could be other possibilities I haven't considered. Feel free to share ideas for alternatives!

Personally, Option 3 sounds like the simplest, even though it does introduce some mild duplication. It also has the benefit of aligning with how closures will be handled in a followup PR (we could maybe even just use the one register method instead of having to introduce a separate register_closure method).

Additionally, with anonymous functions giving unhelpful type names, should we make FunctionInfo::name optional again anyways? We could include a runtime check in the IntoFunction impl that will give None if the type name starts with fn or something.

Thoughts?

[^1]: Due to a limitation in Rust, IntoFunction will work for closures assuming they meet the necessary trait bounds, including Copy and 'static. We can't really work around this at compile-time unfortunately.

[MrGVSV]

I suppose another option could be to mix 2 and 3 such that we return an error when functions are missing a name (i.e. anonymous functions), but provide a separate method for registering them with a name.

So if the user tried to write:

registry.register(|a: i32, b: i32| a + b); 

They'd get back a FunctionRegistrationError::MissingName error and would have to write:

registry.register_named("my_crate::add", |a: i32, b: i32| a + b);

This differs from Option 2 in that they wouldn't have to manually do the IntoFunction conversion.

The idea here is that we don't affect the ergonomics of the common case of just passing a function directly, but we still allow for a nicer way of registering anonymous functions.

Closures would also have to be registered with register_named.

The downside with this approach (and really any of the approaches listed) is that it increases the size of the registration API. Unlike with the TypeRegistry which doesn't really have this naming problem, we might want to expose both register and register_named directly on App (as register_function and register_named_function).

[MrGVSV]

Okay so I've updated the PR to use Option 3: require names during registration.

I mentioned it on Discord, but the only way to get the 2/3 mix I mentioned here is to parse the result of std::any::type_name to determine if the given function is named or not.

While that should work well and is unlikely to suddenly break, it's not guaranteed it will never break. std::any::type_name mentions in its docs that it shouldn't really be used for stuff like this. Now, I know Bevy is one to bend the rules a bit so I will try to put together a followup PR that makes use of that approach. But for now, we'll try to make this current PR as non-controversial as possible.