Commit Graph

14 Commits

Author SHA1 Message Date
poopy
15f00278e7
Rename ArgList::push methods to with and add new push methods which take &mut self (#16567)
# Objective

The `ArgList::push` family of methods consume `self` and return a new
`ArgList` which means they can't be used with `&mut ArgList` references.

```rust
fn foo(args: &mut ArgList) {
    args.push_owned(47_i32); // doesn't work :(
}
```

It's typical for `push` methods on other existing types to take `&mut
self`.

## Solution

Renamed the existing push methods to `with_arg`, `with_ref` etc and
added new `push` methods which take `&mut self`.

## Migration Guide

Uses of the `ArgList::push` methods should be replaced with the `with`
counterpart.

<details>

| old | new |
| --- | --- |
| push_arg | with_arg |
| push_ref | with_ref |
| push_mut | with_mut |
| push_owned | with_owned | 
| push_boxed | with_boxed |

</details>
2025-01-28 05:06:50 +00:00
Zachary Harrold
9bc0ae33c3
Move hashbrown and foldhash out of bevy_utils (#17460)
# Objective

- Contributes to #16877

## Solution

- Moved `hashbrown`, `foldhash`, and related types out of `bevy_utils`
and into `bevy_platform_support`
- Refactored the above to match the layout of these types in `std`.
- Updated crates as required.

## Testing

- CI

---

## Migration Guide

- The following items were moved out of `bevy_utils` and into
`bevy_platform_support::hash`:
  - `FixedState`
  - `DefaultHasher`
  - `RandomState`
  - `FixedHasher`
  - `Hashed`
  - `PassHash`
  - `PassHasher`
  - `NoOpHash`
- The following items were moved out of `bevy_utils` and into
`bevy_platform_support::collections`:
  - `HashMap`
  - `HashSet`
- `bevy_utils::hashbrown` has been removed. Instead, import from
`bevy_platform_support::collections` _or_ take a dependency on
`hashbrown` directly.
- `bevy_utils::Entry` has been removed. Instead, import from
`bevy_platform_support::collections::hash_map` or
`bevy_platform_support::collections::hash_set` as appropriate.
- All of the above equally apply to `bevy::utils` and
`bevy::platform_support`.

## Notes

- I left `PreHashMap`, `PreHashMapExt`, and `TypeIdMap` in `bevy_utils`
as they might be candidates for micro-crating. They can always be moved
into `bevy_platform_support` at a later date if desired.
2025-01-23 16:46:08 +00:00
Zachary Harrold
a64446b77e
Create bevy_platform_support Crate (#17250)
# Objective

- Contributes to #16877

## Solution

- Initial creation of `bevy_platform_support` crate.
- Moved `bevy_utils::Instant` into new `bevy_platform_support` crate.
- Moved `portable-atomic`, `portable-atomic-util`, and
`critical-section` into new `bevy_platform_support` crate.

## Testing

- CI

---

## Showcase

Instead of needing code like this to import an `Arc`:

```rust
#[cfg(feature = "portable-atomic")]
use portable_atomic_util::Arc;

#[cfg(not(feature = "portable-atomic"))]
use alloc::sync::Arc;
```

We can now use:

```rust
use bevy_platform_support::sync::Arc;
```

This applies to many other types, but the goal is overall the same:
allowing crates to use `std`-like types without the boilerplate of
conditional compilation and platform-dependencies.

## Migration Guide

- Replace imports of `bevy_utils::Instant` with
`bevy_platform_support::time::Instant`
- Replace imports of `bevy::utils::Instant` with
`bevy::platform_support::time::Instant`

## Notes

- `bevy_platform_support` hasn't been reserved on `crates.io`
- ~~`bevy_platform_support` is not re-exported from `bevy` at this time.
It may be worthwhile exporting this crate, but I am unsure of a
reasonable name to export it under (`platform_support` may be a bit
wordy for user-facing).~~
- I've included an implementation of `Instant` which is suitable for
`no_std` platforms that are not Wasm for the sake of eliminating feature
gates around its use. It may be a controversial inclusion, so I'm happy
to remove it if required.
- There are many other items (`spin`, `bevy_utils::Sync(Unsafe)Cell`,
etc.) which should be added to this crate. I have kept the initial scope
small to demonstrate utility without making this too unwieldy.

---------

Co-authored-by: TimJentzsch <TimJentzsch@users.noreply.github.com>
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
Co-authored-by: François Mockers <francois.mockers@vleue.com>
2025-01-20 20:45:30 +00:00
Zachary Harrold
0403948aa2
Remove Implicit std Prelude from no_std Crates (#17086)
# Background

In `no_std` compatible crates, there is often an `std` feature which
will allow access to the standard library. Currently, with the `std`
feature _enabled_, the
[`std::prelude`](https://doc.rust-lang.org/std/prelude/index.html) is
implicitly imported in all modules. With the feature _disabled_, instead
the [`core::prelude`](https://doc.rust-lang.org/core/prelude/index.html)
is implicitly imported. This creates a subtle and pervasive issue where
`alloc` items _may_ be implicitly included (if `std` is enabled), or
must be explicitly included (if `std` is not enabled).

# Objective

- Make the implicit imports for `no_std` crates consistent regardless of
what features are/not enabled.

## Solution

- Replace the `cfg_attr` "double negative" `no_std` attribute with
conditional compilation to _include_ `std` as an external crate.
```rust
// Before
#![cfg_attr(not(feature = "std"), no_std)]

// After
#![no_std]

#[cfg(feature = "std")]
extern crate std;
```
- Fix imports that are currently broken but are only now visible with
the above fix.

## Testing

- CI

## Notes

I had previously used the "double negative" version of `no_std` based on
general consensus that it was "cleaner" within the Rust embedded
community. However, this implicit prelude issue likely was considered
when forming this consensus. I believe the reason why is the items most
affected by this issue are provided by the `alloc` crate, which is
rarely used within embedded but extensively used within Bevy.
2025-01-03 01:58:43 +00:00
Benjamin Brienen
64efd08e13
Prefer Display over Debug (#16112)
# Objective

Fixes #16104

## Solution

I removed all instances of `:?` and put them back one by one where it
caused an error.

I removed some bevy_utils helper functions that were only used in 2
places and don't add value. See: #11478

## Testing

CI should catch the mistakes

## Migration Guide

`bevy::utils::{dbg,info,warn,error}` were removed. Use
`bevy::utils::tracing::{debug,info,warn,error}` instead.

---------

Co-authored-by: SpecificProtagonist <vincentjunge@posteo.net>
2024-12-27 00:40:06 +00:00
Zachary Harrold
bf765e61b5
Add no_std support to bevy_reflect (#16256)
# Objective

- Contributes to #15460

## Solution

- Added `std` feature (enabled by default)

## Testing

- CI
- `cargo check -p bevy_reflect --no-default-features --target
"x86_64-unknown-none"`
- UEFI demo application runs with this branch of `bevy_reflect`,
allowing `derive(Reflect)`

## Notes

- The [`spin`](https://crates.io/crates/spin) crate has been included to
provide `RwLock` and `Once` (as an alternative to `OnceLock`) when the
`std` feature is not enabled. Another alternative may be more desirable,
please provide feedback if you have a strong opinion here!
- Certain items (`Box`, `String`, `ToString`) provided by `alloc` have
been added to `__macro_exports` as a way to avoid `alloc` vs `std`
namespacing. I'm personally quite annoyed that we can't rely on `alloc`
as a crate name in `std` environments within macros. I'd love an
alternative to my approach here, but I suspect it's the least-bad
option.
- I would've liked to have an `alloc` feature (for allocation-free
`bevy_reflect`), unfortunately, `erased_serde` unconditionally requires
access to `Box`. Maybe one day we could design around this, but for now
it just means `bevy_reflect` requires `alloc`.

---------

Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
2024-12-05 21:15:21 +00:00
Zachary Harrold
d70595b667
Add core and alloc over std Lints (#15281)
# Objective

- Fixes #6370
- Closes #6581

## Solution

- Added the following lints to the workspace:
  - `std_instead_of_core`
  - `std_instead_of_alloc`
  - `alloc_instead_of_core`
- Used `cargo +nightly fmt` with [item level use
formatting](https://rust-lang.github.io/rustfmt/?version=v1.6.0&search=#Item%5C%3A)
to split all `use` statements into single items.
- Used `cargo clippy --workspace --all-targets --all-features --fix
--allow-dirty` to _attempt_ to resolve the new linting issues, and
intervened where the lint was unable to resolve the issue automatically
(usually due to needing an `extern crate alloc;` statement in a crate
root).
- Manually removed certain uses of `std` where negative feature gating
prevented `--all-features` from finding the offending uses.
- Used `cargo +nightly fmt` with [crate level use
formatting](https://rust-lang.github.io/rustfmt/?version=v1.6.0&search=#Crate%5C%3A)
to re-merge all `use` statements matching Bevy's previous styling.
- Manually fixed cases where the `fmt` tool could not re-merge `use`
statements due to conditional compilation attributes.

## Testing

- Ran CI locally

## Migration Guide

The MSRV is now 1.81. Please update to this version or higher.

## Notes

- This is a _massive_ change to try and push through, which is why I've
outlined the semi-automatic steps I used to create this PR, in case this
fails and someone else tries again in the future.
- Making this change has no impact on user code, but does mean Bevy
contributors will be warned to use `core` and `alloc` instead of `std`
where possible.
- This lint is a critical first step towards investigating `no_std`
options for Bevy.

---------

Co-authored-by: François Mockers <francois.mockers@vleue.com>
2024-09-27 00:59:59 +00:00
Clar Fon
efda7f3f9c
Simpler lint fixes: makes ci lints work but disables a lint for now (#15376)
Takes the first two commits from #15375 and adds suggestions from this
comment:
https://github.com/bevyengine/bevy/pull/15375#issuecomment-2366968300

See #15375 for more reasoning/motivation.

## Rebasing (rerunning)

```rust
git switch simpler-lint-fixes
git reset --hard main
cargo fmt --all -- --unstable-features --config normalize_comments=true,imports_granularity=Crate
cargo fmt --all
git add --update
git commit --message "rustfmt"
cargo clippy --workspace --all-targets --all-features --fix
cargo fmt --all -- --unstable-features --config normalize_comments=true,imports_granularity=Crate
cargo fmt --all
git add --update
git commit --message "clippy"
git cherry-pick e6c0b94f6795222310fb812fa5c4512661fc7887
```
2024-09-24 11:42:59 +00:00
Gino Valente
ebb57c5511
bevy_reflect: Add FunctionRegistry::call (#15148)
# Objective

There may be times where a function in the `FunctionRegistry` doesn't
need to be fully retrieved. A user may just need to call it with a set
of arguments.

We should provide a shortcut for doing this.

## Solution

Add the `FunctionRegistry::call` method to directly call a function in
the registry with the given name and arguments.

## Testing

You can test locally by running:

```
cargo test --package bevy_reflect --all-features
```
2024-09-20 19:14:57 +00:00
Gino Valente
2b4180ca8f
bevy_reflect: Function reflection terminology refactor (#14813)
# Objective

One of the changes in #14704 made `DynamicFunction` effectively the same
as `DynamicClosure<'static>`. This change meant that the de facto
function type would likely be `DynamicClosure<'static>` instead of the
intended `DynamicFunction`, since the former is much more flexible.

We _could_ explore ways of making `DynamicFunction` implement `Copy`
using some unsafe code, but it likely wouldn't be worth it. And users
would likely still reach for the convenience of
`DynamicClosure<'static>` over the copy-ability of `DynamicFunction`.

The goal of this PR is to fix this confusion between the two types.

## Solution

Firstly, the `DynamicFunction` type was removed. Again, it was no
different than `DynamicClosure<'static>` so it wasn't a huge deal to
remove.

Secondly, `DynamicClosure<'env>` and `DynamicClosureMut<'env>` were
renamed to `DynamicFunction<'env>` and `DynamicFunctionMut<'env>`,
respectively.

Yes, we still ultimately kept the naming of `DynamicFunction`, but
changed its behavior to that of `DynamicClosure<'env>`. We need a term
to refer to both functions and closures, and "function" was the best
option.


[Originally](https://discord.com/channels/691052431525675048/1002362493634629796/1274091992162242710),
I was going to go with "callable" as the replacement term to encompass
both functions and closures (e.g. `DynamciCallable<'env>`). However, it
was
[suggested](https://discord.com/channels/691052431525675048/1002362493634629796/1274653581777047625)
by @SkiFire13 that the simpler "function" term could be used instead.

While "callable" is perhaps the better umbrella term—being truly
ambiguous over functions and closures— "function" is more familiar, used
more often, easier to discover, and is subjectively just
"better-sounding".

## Testing

Most changes are purely swapping type names or updating documentation,
but you can verify everything still works by running the following
command:

```
cargo test --package bevy_reflect
```
2024-08-19 21:52:36 +00:00
Gino Valente
423285cf1c
bevy_reflect: Store functions as DynamicClosure<'static> in FunctionRegistry (#14704)
# 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:

```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 #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)
});
```
2024-08-17 00:20:47 +00:00
radiish
6ab8767d3b
reflect: implement the unique reflect rfc (#7207)
# Objective

- Implements the [Unique Reflect
RFC](https://github.com/nicopap/rfcs/blob/bevy-reflect-api/rfcs/56-better-reflect.md).

## Solution

- Implements the RFC.
- This implementation differs in some ways from the RFC:
- In the RFC, it was suggested `Reflect: Any` but `PartialReflect:
?Any`. During initial implementation I tried this, but we assume the
`PartialReflect: 'static` in a lot of places and the changes required
crept out of the scope of this PR.
- `PartialReflect::try_into_reflect` originally returned `Option<Box<dyn
Reflect>>` but i changed this to `Result<Box<dyn Reflect>, Box<dyn
PartialReflect>>` since the method takes by value and otherwise there
would be no way to recover the type. `as_full` and `as_full_mut` both
still return `Option<&(mut) dyn Reflect>`.

---

## Changelog

- Added `PartialReflect`.
- `Reflect` is now a subtrait of `PartialReflect`.
- Moved most methods on `Reflect` to the new `PartialReflect`.
- Added `PartialReflect::{as_partial_reflect, as_partial_reflect_mut,
into_partial_reflect}`.
- Added `PartialReflect::{try_as_reflect, try_as_reflect_mut,
try_into_reflect}`.
- Added `<dyn PartialReflect>::{try_downcast_ref, try_downcast_mut,
try_downcast, try_take}` supplementing the methods on `dyn Reflect`.

## Migration Guide

- Most instances of `dyn Reflect` should be changed to `dyn
PartialReflect` which is less restrictive, however trait bounds should
generally stay as `T: Reflect`.
- The new `PartialReflect::{as_partial_reflect, as_partial_reflect_mut,
into_partial_reflect, try_as_reflect, try_as_reflect_mut,
try_into_reflect}` methods as well as `Reflect::{as_reflect,
as_reflect_mut, into_reflect}` will need to be implemented for manual
implementors of `Reflect`.

## Future Work

- This PR is designed to be followed up by another "Unique Reflect Phase
2" that addresses the following points:
- Investigate making serialization revolve around `Reflect` instead of
`PartialReflect`.
- [Remove the `try_*` methods on `dyn PartialReflect` since they are
stop
gaps](https://github.com/bevyengine/bevy/pull/7207#discussion_r1083476050).
- Investigate usages like `ReflectComponent`. In the places they
currently use `PartialReflect`, should they be changed to use `Reflect`?
- Merging this opens the door to lots of reflection features we haven't
been able to implement.
- We could re-add [the `Reflectable`
trait](8e3488c880/crates/bevy_reflect/src/reflect.rs (L337-L342))
and make `FromReflect` a requirement to improve [`FromReflect`
ergonomics](https://github.com/bevyengine/rfcs/pull/59). This is
currently not possible because dynamic types cannot sensibly be
`FromReflect`.
  - Since this is an alternative to #5772, #5781 would be made cleaner.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
2024-08-12 17:01:41 +00:00
Gino Valente
a0cc636ea3
bevy_reflect: Anonymous function parsing (#14641)
# Objective

### TL;DR

#14098 added the `FunctionRegistry` but had some last minute
complications due to anonymous functions. It ended up going with a
"required name" approach to ensure anonymous functions would always have
a name.

However, this approach isn't ideal for named functions since, by
definition, they will always have a name.

Therefore, this PR aims to modify function reflection such that we can
make function registration easier for named functions, while still
allowing anonymous functions to be registered as well.

### Context

Function registration (#14098) ran into a little problem: anonymous
functions.

Anonymous functions, including function pointers, have very non-unique
type names. For example, the anonymous function `|a: i32, b: i32| a + b`
has the type name of `fn(i32, i32) -> i32`. This obviously means we'd
conflict with another function like `|a: i32, b: i32| a - b`.

The solution that #14098 landed on was to always require a name during
function registration.

The downside with this is that named functions (e.g. `fn add(a: i32, b:
i32) -> i32 { a + b }`) had to redundantly provide a name. Additionally,
manually constructed `DynamicFunction`s also ran into this ergonomics
issue.

I don't entirely know how the function registry will be used, but I have
a strong suspicion that most of its registrations will either be named
functions or manually constructed `DynamicFunction`s, with anonymous
functions only being used here and there for quick prototyping or adding
small functionality.

Why then should the API prioritize the anonymous function use case by
always requiring a name during registration?

#### Telling Functions Apart

Rust doesn't provide a lot of out-of-the-box tools for reflecting
functions. One of the biggest hurdles in attempting to solve the problem
outlined above would be to somehow tell the different kinds of functions
apart.

Let's briefly recap on the categories of functions in Rust:

| Category           | Example                                   |
| ------------------ | ----------------------------------------- |
| Named function     | `fn add(a: i32, b: i32) -> i32 { a + b }` |
| Closure            | `\|a: i32\| a + captured_variable`          |
| Anonymous function | `\|a: i32, b: i32\| a + b`                  |
| Function pointer   | `fn(i32, i32) -> i32`                     |

My first thought was to try and differentiate these categories based on
their size. However, we can see that this doesn't quite work:

| Category           | `size_of` |
| ------------------ | --------- |
| Named function     | 0         |
| Closure            | 0+        |
| Anonymous function | 0         |
| Function pointer   | 8         |

Not only does this not tell anonymous functions from named ones, but it
struggles with pretty much all of them.

My second then was to differentiate based on type name:

| Category           | `type_name`             |
| ------------------ | ----------------------- |
| Named function     | `foo::bar::baz`         |
| Closure            | `foo::bar::{{closure}}` |
| Anonymous function | `fn() -> String`        |
| Function pointer   | `fn() -> String`        |

This is much better. While it can't distinguish between function
pointers and anonymous functions, this doesn't matter too much since we
only care about whether we can _name_ the function.

So why didn't we implement this in #14098?

#### Relying on `type_name`

While this solution was known about while working on #14098, it was left
out from that PR due to it being potentially controversial.

The [docs](https://doc.rust-lang.org/stable/std/any/fn.type_name.html)
for `std::any::type_name` state:

> 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. Similarly,
there is no guarantee that all parts of a type will appear in the
returned string: for example, lifetime specifiers are currently not
included. In addition, the output may change between versions of the
compiler.

So that's it then? We can't use `type_name`?

Well, this statement isn't so much a rule as it is a guideline. And Bevy
is no stranger to bending the rules to make things work or to improve
ergonomics. Remember that before `TypePath`, Bevy's scene system was
entirely dependent on `type_name`. Not to mention that `type_name` is
being used as a key into both the `TypeRegistry` and the
`FunctionRegistry`.

Bevy's practices aside, can we reliably use `type_name` for this?

My answer would be "yes".

Anonymous functions are anonymous. They have no name. There's nothing
Rust could do to give them a name apart from generating a random string
of characters. But remember that this is a diagnostic tool, it doesn't
make sense to obfuscate the type by randomizing the output. So changing
it to be anything other than what it is now is very unlikely.

The only changes that I could potentially see happening are:

1. Closures replace `{{closure}}` with the name of their variable
2. Lifetimes are included in the output

I don't think the first is likely to happen, but if it does then it
actually works out in our favor: closures are now named!

The second point is probably the likeliest. However, adding lifetimes
doesn't mean we can't still rely on `type_name` to determine whether or
not a function is named. So we should be okay in this case as well.

## Solution

Parse the `type_name` of the function in the `TypedFunction` impl to
determine if the function is named or anonymous.

This once again makes `FunctionInfo::name` optional. For manual
constructions of `DynamicFunction`, `FunctionInfo::named` or
``FunctionInfo::anonymous` can be used.

The `FunctionRegistry` API has also been reworked to account for this
change.

`FunctionRegistry::register` no longer takes a name and instead takes it
from the supplied function, returning a
`FunctionRegistrationError::MissingName` error if the name is `None`.
This also doubles as a replacement for the old
`FunctionRegistry::register_dynamic` method, which has been removed.

To handle anonymous functions, a `FunctionRegistry::register_with_name`
method has been added. This works in the same way
`FunctionRegistry::register` used to work before this PR.

The overwriting methods have been updated in a similar manner, with
modifications to `FunctionRegistry::overwrite_registration`, the removal
of `FunctionRegistry::overwrite_registration_dynamic`, and the addition
of `FunctionRegistry::overwrite_registration_with_name`.

This PR also updates the methods on `App` in a similar way:
`App::register_function` no longer requires a name argument and
`App::register_function_with_name` has been added to handle anonymous
functions (and eventually closures).

## Testing

You can run the tests locally by running:

```
cargo test --package bevy_reflect --features functions
```

---

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

> [!note]
> This list is not exhaustive. It only contains some of the most
important changes.

`FunctionRegistry::register` no longer requires a name string for named
functions. Anonymous functions, however, need to be registered using
`FunctionRegistry::register_with_name`.

```rust
// BEFORE
registry
  .register(std::any::type_name_of_val(&foo), foo)?
  .register("bar", || println!("Hello world!"));

// AFTER
registry
  .register(foo)?
  .register_with_name("bar", || println!("Hello world!"));
```

`FunctionInfo::name` is now optional. Anonymous functions and closures
will now have their name set to `None` by default. Additionally,
`FunctionInfo::new` has been renamed to `FunctionInfo::named`.
2024-08-07 03:11:08 +00:00
Gino Valente
df61117850
bevy_reflect: Function registry (#14098)
# 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.

```rust
fn foo() -> i32 {
    123
}

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

let function = registry.get_mut("my_function").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.

#### Closures

Unfortunately, closures can't be registered yet. This will likely be
explored and added 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`
- Added `FunctionRegistrationError`
- Added `reflect_functions` feature to `bevy_ecs` and `bevy_app`
- `FunctionInfo` is no longer `Default`
- `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.
2024-08-06 01:09:48 +00:00