Commit graph

57 commits

Author SHA1 Message Date
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
Torstein Grindvik
9f5f5d3d41
bevy_reflect: get_represented_kind_info APIs for reflected kinds (#14380)
# Objective

Fixes #14378

---------

Signed-off-by: Torstein Grindvik <torstein.grindvik@muybridge.com>
Co-authored-by: Torstein Grindvik <torstein.grindvik@muybridge.com>
Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
2024-10-15 02:08:31 +00:00
Gino Valente
397f20e835
bevy_reflect: Generic parameter info (#15475)
# Objective

Currently, reflecting a generic type provides no information about the
generic parameters. This means that you can't get access to the type of
`T` in `Foo<T>` without creating custom type data (we do this for
[`ReflectHandle`](https://docs.rs/bevy/0.14.2/bevy/asset/struct.ReflectHandle.html#method.asset_type_id)).

## Solution

This PR makes it so that generic type parameters and generic const
parameters are tracked in a `Generics` struct stored on the `TypeInfo`
for a type.

For example, `struct Foo<T, const N: usize>` will store `T` and `N` as a
`TypeParamInfo` and `ConstParamInfo`, respectively.

The stored information includes:

- The name of the generic parameter (i.e. `T`, `N`, etc.)
- The type of the generic parameter (remember that we're dealing with
monomorphized types, so this will actually be a concrete type)
- The default type/value, if any (e.g. `f32` in `T = f32` or `10` in
`const N: usize = 10`)

### Caveats

The only requirement for this to work is that the user does not opt-out
of the automatic `TypePath` derive with `#[reflect(type_path = false)]`.

Doing so prevents the macro code from 100% knowing that the generic type
implements `TypePath`. This in turn means the generated `Typed` impl
can't add generics to the type.

There are two solutions for this—both of which I think we should explore
in a future PR:

1. We could just not use `TypePath`. This would mean that we can't store
the `Type` of the generic, but we can at least store the `TypeId`.
2. We could provide a way to opt out of the automatic `Typed` derive
with a `#[reflect(typed = false)]` attribute. This would allow users to
manually implement `Typed` to add whatever generic information they need
(e.g. skipping a parameter that can't implement `TypePath` while the
rest can).

I originally thought about making `Generics` an enum with `Generic`,
`NonGeneric`, and `Unavailable` variants to signify whether there are
generics, no generics, or generics that cannot be added due to opting
out of `TypePath`. I ultimately decided against this as I think it adds
a bit too much complexity for such an uncommon problem.

Additionally, user's don't necessarily _have_ to know the generics of a
type, so just skipping them should generally be fine for now.

## Testing

You can test locally by running:

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

---

## Showcase

You can now access generic parameters via `TypeInfo`!

```rust
#[derive(Reflect)]
struct MyStruct<T, const N: usize>([T; N]);

let generics = MyStruct::<f32, 10>::type_info().generics();

// Get by index:
let t = generics.get(0).unwrap();
assert_eq!(t.name(), "T");
assert!(t.ty().is::<f32>());
assert!(!t.is_const());

// Or by name:
let n = generics.get_named("N").unwrap();
assert_eq!(n.name(), "N");
assert!(n.ty().is::<usize>());
assert!(n.is_const());
```

You can even access parameter defaults:

```rust
#[derive(Reflect)]
struct MyStruct<T = String, const N: usize = 10>([T; N]);

let generics = MyStruct::<f32, 5>::type_info().generics();

let GenericInfo::Type(info) = generics.get_named("T").unwrap() else {
    panic!("expected a type parameter");
};

let default = info.default().unwrap();

assert!(default.is::<String>());

let GenericInfo::Const(info) = generics.get_named("N").unwrap() else {
    panic!("expected a const parameter");
};

let default = info.default().unwrap();

assert_eq!(default.downcast_ref::<usize>().unwrap(), &10);
```
2024-09-30 17:58:37 +00:00
andriyDev
04d5685889
Make drain take a mutable borrow instead of Box<Self> for reflected Map, List, and Set. (#15406)
# Objective

Fixes #15185.

# Solution

Change `drain` to take a `&mut self` for most reflected types.

Some notable exceptions to this change are `Array` and `Tuple`. These
types don't make sense with `drain` taking a mutable borrow since they
can't get "smaller". Also `BTreeMap` doesn't have a `drain` function, so
we have to pop elements off one at a time.

## Testing

- The existing tests are sufficient.

---

## Migration Guide

- `reflect::Map`, `reflect::List`, and `reflect::Set` all now take a
`&mut self` instead of a `Box<Self>`. Callers of these traits should add
`&mut` before their boxes, and implementers of these traits should
update to match.
2024-09-30 17:19:13 +00:00
Giacomo Stevanato
0d751e8809
Use HashTable in DynamicMap and fix bug in remove (#15158)
# Objective

- `DynamicMap` currently uses an `HashMap` from a `u64` hash to the
entry index in a `Vec`. This is incorrect in the presence of hash
collisions, so let's fix it;
- `DynamicMap::remove` was also buggy, as it didn't fix up the indexes
of the other elements after removal. Fix that up as well and add a
regression test.

## Solution

- Use `HashTable` in `DynamicMap` to distinguish entries that have the
same hash by using `reflect_partial_eq`, bringing it more in line with
what `DynamicSet` does;
- Reimplement `DynamicMap::remove` to properly fix up the index of moved
elements after the removal.

## Testing

- A regression test was added for the `DynamicMap::remove` issue.

---

Some kinda related considerations: the use of a separate `Vec` for
storing the entries adds some complications that I'm not sure are worth.
This is mainly used to implement an efficient `get_at`, which is relied
upon by `MapIter`. However both `HashMap` and `BTreeMap` implement
`get_at` inefficiently (and cannot do so efficiently), leading to a
`O(N^2)` complexity for iterating them. This could be removed in favor
of a `Box<dyn Iterator>` like it's done in `DynamicSet`.
2024-09-30 17:02:10 +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
4d0961cc8a
bevy_reflect: Add ReflectRef/ReflectMut/ReflectOwned convenience casting methods (#15235)
# Objective

#13320 added convenience methods for casting a `TypeInfo` into its
respective variant:

```rust
let info: &TypeInfo = <Vec<i32> as Typed>::type_info();

// We know `info` contains a `ListInfo`, so we can simply cast it:
let list_info: &ListInfo = info.as_list().unwrap();
```

This is especially helpful when you have already verified a type is a
certain kind via `ReflectRef`, `ReflectMut`, `ReflectOwned`, or
`ReflectKind`.

As mentioned in that PR, though, it would be useful to add similar
convenience methods to those types as well.

## Solution

Added convenience casting methods to `ReflectRef`, `ReflectMut`, and
`ReflectOwned`.

With these methods, I was able to reduce our nesting in certain places
throughout the crate.

Additionally, I took this opportunity to move these types (and
`ReflectKind`) to their own module to help clean up the `reflect`
module.

## Testing

You can test locally by running:

```
cargo test --package bevy_reflect --all-features
```

---

## Showcase

Convenience methods for casting `ReflectRef`, `ReflectMut`, and
`ReflectOwned` into their respective variants has been added! This
allows you to write cleaner code if you already know the kind of your
reflected data:

```rust
// BEFORE
let ReflectRef::List(list) = list.reflect_ref() else {
    panic!("expected list");
};

// AFTER
let list = list.reflect_ref().as_list().unwrap();
```

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Pablo Reinhardt <126117294+pablo-lua@users.noreply.github.com>
2024-09-23 16:50:46 +00:00
VitalyR
661ab1ab41
Fix warnings triggered by elided_named_lifetimes lint (#15328)
# Objective

Eliminate some warnings introduced by the new rust lint
[elided_named_lifetimes](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/builtin/static.ELIDED_NAMED_LIFETIMES.html),
fix #15326.

## Solution

- Add or remove lifetime markers to not trigger the lint.

## Testing

- When the lint comes to stable, the CI will fail and this PR could fix
that.
2024-09-20 19:17:33 +00:00
Gino Valente
3892adcb47
bevy_reflect: Add Type type (#14838)
# Objective

Closes #7622.

I was working on adding support for reflecting generic functions and
found that I wanted to use an argument's `TypeId` for hashing and
comparison, but its `TypePath` for debugging and error messaging.

While I could just keep them separate, place them in a tuple or a local
struct or something, I think I see an opportunity to make a dedicate
type for this.

Additionally, we can use this type to clean up some duplication amongst
the type info structs in a manner similar to #7622.

## Solution

Added the `Type` type. This should be seen as the most basic
representation of a type apart from `TypeId`. It stores both the
`TypeId` of the type as well as its `TypePathTable`.

The `Hash` and `PartialEq` implementations rely on the `TypeId`, while
the `Debug` implementation relies on the `TypePath`.

This makes it especially useful as a key in a `HashMap` since we get the
speed of the `TypeId` hashing/comparisons with the readability of
`TypePath`.

With this type, we're able to reduce the duplication across the type
info structs by removing individual fields for `TypeId` and
`TypePathTable`, replacing them with a single `Type` field. Similarly,
we can remove many duplicate methods and replace it with a macro that
delegates to the stored `Type`.

### Caveats

It should be noted that this type is currently 3x larger than `TypeId`.
On my machine, it's 48 bytes compared to `TypeId`'s 16. While this
doesn't matter for `TypeInfo` since it would contain that data
regardless, it is something to keep in mind when using elsewhere.

## Testing

All tests should pass as normal:

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

---

## Showcase

`bevy_reflect` now exports a `Type` struct. This type contains both the
`TypeId` and the `TypePathTable` of the given type, allowing it to be
used like `TypeId` but have the debuggability of `TypePath`.

```rust
// We can create this for any type implementing `TypePath`:
let ty = Type::of::<String>();

// It has `Hash` and `Eq` impls powered by `TypeId`, making it useful for maps:
let mut map = HashMap::<Type, i32>::new();
map.insert(ty, 25);

// And it has a human-readable `Debug` representation:
let debug = format!("{:?}", map);
assert_eq!(debug, "{alloc::string::String: 25}");
```

## Migration Guide

Certain type info structs now only return their item types as `Type`
instead of exposing direct methods on them.

The following methods have been removed:

- `ArrayInfo::item_type_path_table`
- `ArrayInfo::item_type_id`
- `ArrayInfo::item_is`
- `ListInfo::item_type_path_table`
- `ListInfo::item_type_id`
- `ListInfo::item_is`
- `SetInfo::value_type_path_table`
- `SetInfo::value_type_id`
- `SetInfo::value_is`
- `MapInfo::key_type_path_table`
- `MapInfo::key_type_id`
- `MapInfo::key_is`
- `MapInfo::value_type_path_table`
- `MapInfo::value_type_id`
- `MapInfo::value_is`

Instead, access the `Type` directly using one of the new methods:

- `ArrayInfo::item_ty`
- `ListInfo::item_ty`
- `SetInfo::value_ty`
- `MapInfo::key_ty`
- `MapInfo::value_ty`

For example:

```rust
// BEFORE
let type_id = array_info.item_type_id();

// AFTER
let type_id = array_info.item_ty().id();
```
2024-08-25 17:57:07 +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
SpecificProtagonist
ab255aefc6
Implement FromIterator/IntoIterator for dynamic types (#14250)
# Objective

Implement FromIterator/IntoIterator for dynamic types where missing

Note:
- can't impl `IntoIterator` for `&Array` & co because of orphan rules
- `into_iter().collect()` is a no-op for `Vec`s because of
specialization

---

## Migration Guide

- Change `DynamicArray::from_vec` to `DynamicArray::from_iter`
2024-07-15 15:38:11 +00:00
Gino Valente
aa241672e1
bevy_reflect: Nested TypeInfo getters (#13321)
# Objective

Right now, `TypeInfo` can be accessed directly from a type using either
`Typed::type_info` or `Reflect::get_represented_type_info`.

However, once that `TypeInfo` is accessed, any nested types must be
accessed via the `TypeRegistry`.

```rust
#[derive(Reflect)]
struct Foo {
  bar: usize
}

let registry = TypeRegistry::default();

let TypeInfo::Struct(type_info) = Foo::type_info() else {
  panic!("expected struct info");
};

let field = type_info.field("bar").unwrap();

let field_info = registry.get_type_info(field.type_id()).unwrap();
assert!(field_info.is::<usize>());;
```

## Solution

Enable nested types within a `TypeInfo` to be retrieved directly.

```rust
#[derive(Reflect)]
struct Foo {
  bar: usize
}

let TypeInfo::Struct(type_info) = Foo::type_info() else {
  panic!("expected struct info");
};

let field = type_info.field("bar").unwrap();

let field_info = field.type_info().unwrap();
assert!(field_info.is::<usize>());;
```

The particular implementation was chosen for two reasons.

Firstly, we can't just store `TypeInfo` inside another `TypeInfo`
directly. This is because some types are recursive and would result in a
deadlock when trying to create the `TypeInfo` (i.e. it has to create the
`TypeInfo` before it can use it, but it also needs the `TypeInfo` before
it can create it). Therefore, we must instead store the function so it
can be retrieved lazily.

I had considered also using a `OnceLock` or something to lazily cache
the info, but I figured we can look into optimizations later. The API
should remain the same with or without the `OnceLock`.

Secondly, a new wrapper trait had to be introduced: `MaybeTyped`. Like
`RegisterForReflection`, this trait is `#[doc(hidden)]` and only exists
so that we can properly handle dynamic type fields without requiring
them to implement `Typed`. We don't want dynamic types to implement
`Typed` due to the fact that it would make the return type
`Option<&'static TypeInfo>` for all types even though only the dynamic
types ever need to return `None` (see #6971 for details).

Users should never have to interact with this trait as it has a blanket
impl for all `Typed` types. And `Typed` is automatically implemented
when deriving `Reflect` (as it is required).

The one downside is we do need to return `Option<&'static TypeInfo>`
from all these new methods so that we can handle the dynamic cases. If
we didn't have to, we'd be able to get rid of the `Option` entirely. But
I think that's an okay tradeoff for this one part of the API, and keeps
the other APIs intact.

## Testing

This PR contains tests to verify everything works as expected. You can
test locally by running:

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

---

## Changelog

### Public Changes

- Added `ArrayInfo::item_info` method
- Added `NamedField::type_info` method
- Added `UnnamedField::type_info` method
- Added `ListInfo::item_info` method
- Added `MapInfo::key_info` method
- Added `MapInfo::value_info` method
- All active fields now have a `Typed` bound (remember that this is
automatically satisfied for all types that derive `Reflect`)

### Internal Changes

- Added `MaybeTyped` trait

## Migration Guide

All active fields for reflected types (including lists, maps, tuples,
etc.), must implement `Typed`. For the majority of users this won't have
any visible impact.

However, users implementing `Reflect` manually may need to update their
types to implement `Typed` if they weren't already.

Additionally, custom dynamic types will need to implement the new hidden
`MaybeTyped` trait.
2024-07-15 00:40:07 +00:00
Gino Valente
99c9218b56
bevy_reflect: Feature-gate function reflection (#14174)
# Objective

Function reflection requires a lot of macro code generation in the form
of several `all_tuples!` invocations, as well as impls generated in the
`Reflect` derive macro.

Seeing as function reflection is currently a bit more niche, it makes
sense to gate it all behind a feature.

## Solution

Add a `functions` feature to `bevy_reflect`, which can be enabled in
Bevy using the `reflect_functions` feature.

## Testing

You can test locally by running:

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

That should ensure that everything still works with the feature
disabled.

To test with the feature on, you can run:

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

---

## Changelog

- Moved function reflection behind a Cargo feature
(`bevy/reflect_functions` and `bevy_reflect/functions`)
- Add `IntoFunction` export in `bevy_reflect::prelude`

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

Function reflection is now gated behind a feature. To use function
reflection, enable the feature:
- If using `bevy_reflect` directly, enable the `functions` feature
- If using `bevy`, enable the `reflect_functions` feature
2024-07-14 15:55:31 +00:00
Gino Valente
276dd04001
bevy_reflect: Function reflection (#13152)
# Objective

We're able to reflect types sooooooo... why not functions?

The goal of this PR is to make functions callable within a dynamic
context, where type information is not readily available at compile
time.

For example, if we have a function:

```rust
fn add(left: i32, right: i32) -> i32 {
  left + right
}
```

And two `Reflect` values we've already validated are `i32` types:

```rust
let left: Box<dyn Reflect> = Box::new(2_i32);
let right: Box<dyn Reflect> = Box::new(2_i32);
```

We should be able to call `add` with these values:

```rust
// ?????
let result: Box<dyn Reflect> = add.call_dynamic(left, right);
```

And ideally this wouldn't just work for functions, but methods and
closures too!

Right now, users have two options:

1. Manually parse the reflected data and call the function themselves
2. Rely on registered type data to handle the conversions for them

For a small function like `add`, this isn't too bad. But what about for
more complex functions? What about for many functions?

At worst, this process is error-prone. At best, it's simply tedious.

And this is assuming we know the function at compile time. What if we
want to accept a function dynamically and call it with our own
arguments?

It would be much nicer if `bevy_reflect` could alleviate some of the
problems here.

## Solution

Added function reflection!

This adds a `DynamicFunction` type to wrap a function dynamically. This
can be called with an `ArgList`, which is a dynamic list of
`Reflect`-containing `Arg` arguments. It returns a `FunctionResult`
which indicates whether or not the function call succeeded, returning a
`Reflect`-containing `Return` type if it did succeed.

Many functions can be converted into this `DynamicFunction` type thanks
to the `IntoFunction` trait.

Taking our previous `add` example, this might look something like
(explicit types added for readability):

```rust
fn add(left: i32, right: i32) -> i32 {
  left + right
}

let mut function: DynamicFunction = add.into_function();
let args: ArgList = ArgList::new().push_owned(2_i32).push_owned(2_i32);
let result: Return = function.call(args).unwrap();
let value: Box<dyn Reflect> = result.unwrap_owned();
assert_eq!(value.take::<i32>().unwrap(), 4);
```

And it also works on closures:

```rust
let add = |left: i32, right: i32| left + right;

let mut function: DynamicFunction = add.into_function();
let args: ArgList = ArgList::new().push_owned(2_i32).push_owned(2_i32);
let result: Return = function.call(args).unwrap();
let value: Box<dyn Reflect> = result.unwrap_owned();
assert_eq!(value.take::<i32>().unwrap(), 4);
```

As well as methods:

```rust
#[derive(Reflect)]
struct Foo(i32);

impl Foo {
  fn add(&mut self, value: i32) {
    self.0 += value;
  }
}

let mut foo = Foo(2);

let mut function: DynamicFunction = Foo::add.into_function();
let args: ArgList = ArgList::new().push_mut(&mut foo).push_owned(2_i32);
function.call(args).unwrap();
assert_eq!(foo.0, 4);
```

### Limitations

While this does cover many functions, it is far from a perfect system
and has quite a few limitations. Here are a few of the limitations when
using `IntoFunction`:

1. The lifetime of the return value is only tied to the lifetime of the
first argument (useful for methods). This means you can't have a
function like `(a: i32, b: &i32) -> &i32` without creating the
`DynamicFunction` manually.
2. Only 15 arguments are currently supported. If the first argument is a
(mutable) reference, this number increases to 16.
3. Manual implementations of `Reflect` will need to implement the new
`FromArg`, `GetOwnership`, and `IntoReturn` traits in order to be used
as arguments/return types.

And some limitations of `DynamicFunction` itself:

1. All arguments share the same lifetime, or rather, they will shrink to
the shortest lifetime.
2. Closures that capture their environment may need to have their
`DynamicFunction` dropped before accessing those variables again (there
is a `DynamicFunction::call_once` to make this a bit easier)
3. All arguments and return types must implement `Reflect`. While not a
big surprise coming from `bevy_reflect`, this implementation could
actually still work by swapping `Reflect` out with `Any`. Of course,
that makes working with the arguments and return values a bit harder.
4. Generic functions are not supported (unless they have been manually
monomorphized)

And general, reflection gotchas:

1. `&str` does not implement `Reflect`. Rather, `&'static str`
implements `Reflect` (the same is true for `&Path` and similar types).
This means that `&'static str` is considered an "owned" value for the
sake of generating arguments. Additionally, arguments and return types
containing `&str` will assume it's `&'static str`, which is almost never
the desired behavior. In these cases, the only solution (I believe) is
to use `&String` instead.

### Followup Work

This PR is the first of two PRs I intend to work on. The second PR will
aim to integrate this new function reflection system into the existing
reflection traits and `TypeInfo`. The goal would be to register and call
a reflected type's methods dynamically.

I chose not to do that in this PR since the diff is already quite large.
I also want the discussion for both PRs to be focused on their own
implementation.

Another followup I'd like to do is investigate allowing common container
types as a return type, such as `Option<&[mut] T>` and `Result<&[mut] T,
E>`. This would allow even more functions to opt into this system. I
chose to not include it in this one, though, for the same reasoning as
previously mentioned.

### Alternatives

One alternative I had considered was adding a macro to convert any
function into a reflection-based counterpart. The idea would be that a
struct that wraps the function would be created and users could specify
which arguments and return values should be `Reflect`. It could then be
called via a new `Function` trait.

I think that could still work, but it will be a fair bit more involved,
requiring some slightly more complex parsing. And it of course is a bit
more work for the user, since they need to create the type via macro
invocation.

It also makes registering these functions onto a type a bit more
complicated (depending on how it's implemented).

For now, I think this is a fairly simple, yet powerful solution that
provides the least amount of friction for users.

---

## Showcase

Bevy now adds support for storing and calling functions dynamically
using reflection!

```rust
// 1. Take a standard Rust function
fn add(left: i32, right: i32) -> i32 {
  left + right
}

// 2. Convert it into a type-erased `DynamicFunction` using the `IntoFunction` trait
let mut function: DynamicFunction = add.into_function();
// 3. Define your arguments from reflected values
let args: ArgList = ArgList::new().push_owned(2_i32).push_owned(2_i32);
// 4. Call the function with your arguments
let result: Return = function.call(args).unwrap();
// 5. Extract the return value
let value: Box<dyn Reflect> = result.unwrap_owned();
assert_eq!(value.take::<i32>().unwrap(), 4);
```

## Changelog

#### TL;DR

- Added support for function reflection
- Added a new `Function Reflection` example:
ba727898f2/examples/reflection/function_reflection.rs (L1-L157)

#### Details

Added the following items:

- `ArgError` enum
- `ArgId` enum
- `ArgInfo` struct
- `ArgList` struct
- `Arg` enum
- `DynamicFunction` struct
- `FromArg` trait (derived with `derive(Reflect)`)
- `FunctionError` enum
- `FunctionInfo` struct
- `FunctionResult` alias
- `GetOwnership` trait (derived with `derive(Reflect)`)
- `IntoFunction` trait (with blanket implementation)
- `IntoReturn` trait (derived with `derive(Reflect)`)
- `Ownership` enum
- `ReturnInfo` struct
- `Return` enum

---------

Co-authored-by: Periwink <charlesbour@gmail.com>
2024-07-01 13:49:08 +00:00
Wuketuke
d45bcfd043
improved the error message by insert_boxed (issue #13646) (again) (#13706)
previously I worked on fixing issue #13646, back when the error message
did not include the type at all.
But that error message had room for improvement, so I included the
feedback of @alice-i-cecile and @MrGVSV.
The error message will now read `the given key (of type
bevy_reflect::tests::Foo) does not support hashing` or 'the given key
(of type bevy_reflect::DynamicStruct) does not support hashing' in case
of a dynamic struct that represents a hashable struct

i also added a new unit test for this new behaviour
(`reflect_map_no_hash_dynamic`).
Fixes #13646 (again)

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
2024-06-07 20:56:16 +00:00
Wuketuke
2eb9d5cc38
hashing error in bevy_reflect now includes the type (bevyengine#13646) (#13691)
# Objective
If you try to add an object to the hashmap that is not capable of
hashing, the program panics. For easier debugging, the type for that
object should be included in the error message.

Fixes #13646.

## Solution
initially i tried calling std::any::type_name_of_val. this had the
problem that it would print something like dyn Box<dyn Reflect>, not
helpful. But since these objects all implement Reflect, i used
Reflect::type_path() instead. Previously, the error message was part of
a constant called HASH_ERROR. i changed that to a macro called
hash_error to print the type of that object more easily

## Testing
i adapted the unit test reflect_map_no_hash to expect the type in that
panic aswell

since this is my first contribution, please let me know if i have done
everything properly
2024-06-05 19:41:23 +00:00
rmsthebest
278380394f
Avoid bevy_reflect::List::iter wrapping in release mode (#13271)
# Objective
Fixes  #13230

## Solution
Uses solution described in  #13230
They mention a worry about adding a branch, but I'm not sure there is
one.

This code
```Rust
#[no_mangle]
pub fn next_if_some(num: i32, b: Option<bool>) -> i32 {
    num + b.is_some() as i32
}
```
produces this assembly with opt-level 3
```asm
next_if_some:
        xor     eax, eax
        cmp     sil, 2
        setne   al
        add     eax, edi
        ret
```

## Testing
Added test from #13230, tagged it as ignore as it is only useful in
release mode and very slow if you accidentally invoke it in debug mode.

---

## Changelog
Iterationg of ListIter will no longer overflow and wrap around

## Migration Guide
2024-05-12 15:01:05 +00:00
Brezak
9c4ac7c297
Finish the work on try_apply (#12646)
# Objective

Finish the `try_apply` implementation started in #6770 by @feyokorenhof.
Supersedes and closes #6770. Closes #6182

## Solution

Add `try_apply` to `Reflect` and implement it in all the places that
implement `Reflect`.

---

## Changelog

Added `try_apply` to `Reflect`.

---------

Co-authored-by: Feyo Korenhof <feyokorenhof@gmail.com>
Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
2024-05-08 14:26:01 +00:00
BD103
7b8d502083
Fix beta lints (#12980)
# Objective

- Fixes #12976

## Solution

This one is a doozy.

- Run `cargo +beta clippy --workspace --all-targets --all-features` and
fix all issues
- This includes:
- Moving inner attributes to be outer attributes, when the item in
question has both inner and outer attributes
  - Use `ptr::from_ref` in more scenarios
- Extend the valid idents list used by `clippy:doc_markdown` with more
names
  - Use `Clone::clone_from` when possible
  - Remove redundant `ron` import
  - Add backticks to **so many** identifiers and items
    - I'm sorry whoever has to review this

---

## Changelog

- Added links to more identifiers in documentation.
2024-04-16 02:46:46 +00:00
Mincong Lu
f45450e26b
Added reflect support for std::HashSet, BTreeSet and BTreeMap. (#12124)
# Objective

Added reflect support for `std::HashSet`, `BTreeSet` and `BTreeMap`.

The set support is limited to `reflect_value` since that's the level of
support prior art `bevy_util::HashSet` got.

## Changelog

Dropped `Hash` Requirement on `MapInfo` since it's not needed on
`BTreeMap`s.
2024-02-26 16:36:04 +00:00
Doonv
054134fba2
Add ReflectKind (#11664)
# Objective

Fix https://github.com/bevyengine/bevy/issues/11657

## Solution

Add a `ReflectKind` enum, add `Reflect::reflect_kind` which returns a
`ReflectKind`, and add `kind` method implementions to `ReflectRef`,
`ReflectMut`, and `ReflectOwned`, which returns a `ReflectKind`.

I also changed `AccessError` to use this new struct instead of it's own
`TypeKind` struct.

---

## Changelog

- Added `ReflectKind`, an enumeration over the kinds of a reflected type
without its data.
- Added `Reflect::reflect_kind` (with default implementation)
- Added implementation for the `kind` method on `ReflectRef`,
`ReflectMut`, and `ReflectOwned` which gives their kind without any
information, as a `ReflectKind`
2024-02-07 00:36:23 +00:00
Tygyh
1568d4a415
Reorder impl to be the same as the trait (#11076)
# Objective

- Make the implementation order consistent between all sources to fit
the order in the trait.

## Solution

- Change the implementation order.
2023-12-24 17:43:55 +00:00
tygyh
fd308571c4
Remove unnecessary path prefixes (#10749)
# Objective

- Shorten paths by removing unnecessary prefixes

## Solution

- Remove the prefixes from many paths which do not need them. Finding
the paths was done automatically using built-in refactoring tools in
Jetbrains RustRover.
2023-11-28 23:43:40 +00:00
radiish
262846e702
reflect: TypePath part 2 (#8768)
# Objective

- Followup to #7184.
- ~Deprecate `TypeUuid` and remove its internal references.~ No longer
part of this PR.
- Use `TypePath` for the type registry, and (de)serialisation instead of
`std::any::type_name`.
- Allow accessing type path information behind proxies.

## Solution
- Introduce methods on `TypeInfo` and friends for dynamically querying
type path. These methods supersede the old `type_name` methods.
- Remove `Reflect::type_name` in favor of `DynamicTypePath::type_path`
and `TypeInfo::type_path_table`.
- Switch all uses of `std::any::type_name` in reflection, non-debugging
contexts to use `TypePath`.

---

## Changelog

- Added `TypePathTable` for dynamically accessing methods on `TypePath`
through `TypeInfo` and the type registry.
- Removed `type_name` from all `TypeInfo`-like structs.
- Added `type_path` and `type_path_table` methods to all `TypeInfo`-like
structs.
- Removed `Reflect::type_name` in favor of
`DynamicTypePath::reflect_type_path` and `TypeInfo::type_path`.
- Changed the signature of all `DynamicTypePath` methods to return
strings with a static lifetime.

## Migration Guide

- Rely on `TypePath` instead of `std::any::type_name` for all stability
guarantees and for use in all reflection contexts, this is used through
with one of the following APIs:
  - `TypePath::type_path` if you have a concrete type and not a value.
- `DynamicTypePath::reflect_type_path` if you have an `dyn Reflect`
value without a concrete type.
- `TypeInfo::type_path` for use through the registry or if you want to
work with the represented type of a `DynamicFoo`.
  
- Remove `type_name` from manual `Reflect` implementations.
- Use `type_path` and `type_path_table` in place of `type_name` on
`TypeInfo`-like structs.
- Use `get_with_type_path(_mut)` over `get_with_type_name(_mut)`.

## Note to reviewers

I think if anything we were a little overzealous in merging #7184 and we
should take that extra care here.

In my mind, this is the "point of no return" for `TypePath` and while I
think we all agree on the design, we should carefully consider if the
finer details and current implementations are actually how we want them
moving forward.

For example [this incorrect `TypePath` implementation for
`String`](3fea3c6c0b/crates/bevy_reflect/src/impls/std.rs (L90))
(note that `String` is in the default Rust prelude) snuck in completely
under the radar.
2023-10-09 19:33:03 +00:00
Rob Parrett
a788e31ad5
Fix CI for Rust 1.72 (#9562)
# Objective

[Rust 1.72.0](https://blog.rust-lang.org/2023/08/24/Rust-1.72.0.html) is
now stable.

# Notes

- `let-else` formatting has arrived!
- I chose to allow `explicit_iter_loop` due to
https://github.com/rust-lang/rust-clippy/issues/11074.
  
We didn't hit any of the false positives that prevent compilation, but
fixing this did produce a lot of the "symbol soup" mentioned, e.g. `for
image in &mut *image_events {`.
  
  Happy to undo this if there's consensus the other way.

---------

Co-authored-by: François <mockersf@gmail.com>
2023-08-25 12:34:24 +00:00
Carter Anderson
8b9d88f4d0
Reflect now requires DynamicTypePath. Remove Reflect::get_type_path() (#8764)
Followup to #7184

This makes `Reflect: DynamicTypePath` which allows us to remove
`Reflect::get_type_path`, reducing unnecessary codegen and simplifying
`Reflect` implementations.
2023-06-06 17:23:58 +00:00
radiish
1efc762924
reflect: stable type path v2 (#7184)
# Objective

- Introduce a stable alternative to
[`std::any::type_name`](https://doc.rust-lang.org/std/any/fn.type_name.html).
- Rewrite of #5805 with heavy inspiration in design.
- On the path to #5830.
- Part of solving #3327.


## Solution

- Add a `TypePath` trait for static stable type path/name information.
- Add a `TypePath` derive macro.
- Add a `impl_type_path` macro for implementing internal and foreign
types in `bevy_reflect`.

---

## Changelog

- Added `TypePath` trait.
- Added `DynamicTypePath` trait and `get_type_path` method to `Reflect`.
- Added a `TypePath` derive macro.
- Added a `bevy_reflect::impl_type_path` for implementing `TypePath` on
internal and foreign types in `bevy_reflect`.
- Changed `bevy_reflect::utility::(Non)GenericTypeInfoCell` to
`(Non)GenericTypedCell<T>` which allows us to be generic over both
`TypeInfo` and `TypePath`.
- `TypePath` is now a supertrait of `Asset`, `Material` and
`Material2d`.
- `impl_reflect_struct` needs a `#[type_path = "..."]` attribute to be
specified.
- `impl_reflect_value` needs to either specify path starting with a
double colon (`::core::option::Option`) or an `in my_crate::foo`
declaration.
- Added `bevy_reflect_derive::ReflectTypePath`.
- Most uses of `Ident` in `bevy_reflect_derive` changed to use
`ReflectTypePath`.

## Migration Guide

- Implementors of `Asset`, `Material` and `Material2d` now also need to
derive `TypePath`.
- Manual implementors of `Reflect` will need to implement the new
`get_type_path` method.

## Open Questions
- [x] ~This PR currently does not migrate any usages of
`std::any::type_name` to use `bevy_reflect::TypePath` to ease the review
process. Should it?~ Migration will be left to a follow-up PR.
- [ ] This PR adds a lot of `#[derive(TypePath)]` and `T: TypePath` to
satisfy new bounds, mostly when deriving `TypeUuid`. Should we make
`TypePath` a supertrait of `TypeUuid`? [Should we remove `TypeUuid` in
favour of
`TypePath`?](2afbd85532 (r961067892))
2023-06-05 20:31:20 +00:00
Егор Куклин
6b4c7d5d88
Add get_at_mut to bevy_reflect::Map trait (#8691)
# Objective

Fixes #8596 

## Solution

Change interface of the trait Map. Adjust implementations of this trait

---

## Changelog

### Changed
- Interface of Map trait

### Added
- `Map::get_at_mut`

## Migration Guide

Every implementor of Map trait would need to implement `get_at_mut`.
Which, judging by changes in this PR, should be fairly trivial.
2023-06-02 12:24:40 +00:00
Gauthier Acquitter
acf1362b9a
bevy_reflect: Allow construction of MapIter outside of the bevy_reflect crate. (#8723)
# Objective

Right now it's impossible to construct a MapIter outside of the
bevy_reflect crate, making it impossible to implement the Map trait for
custom map types.

## Solution

Addition of a pub constructor to MapIter.
2023-06-01 10:12:57 +00:00
Gino Valente
75130bd5ec
bevy_reflect: Better proxies (#6971)
# Objective

> This PR is based on discussion from #6601

The Dynamic types (e.g. `DynamicStruct`, `DynamicList`, etc.) act as
both:
1. Dynamic containers which may hold any arbitrary data
2. Proxy types which may represent any other type

Currently, the only way we can represent the proxy-ness of a Dynamic is
by giving it a name.

```rust
// This is just a dynamic container
let mut data = DynamicStruct::default();

// This is a "proxy"
data.set_name(std::any::type_name::<Foo>());
```

This type name is the only way we check that the given Dynamic is a
proxy of some other type. When we need to "assert the type" of a `dyn
Reflect`, we call `Reflect::type_name` on it. However, because we're
only using a string to denote the type, we run into a few gotchas and
limitations.

For example, hashing a Dynamic proxy may work differently than the type
it proxies:

```rust
#[derive(Reflect, Hash)]
#[reflect(Hash)]
struct Foo(i32);

let concrete = Foo(123);
let dynamic = concrete.clone_dynamic();

let concrete_hash = concrete.reflect_hash();
let dynamic_hash = dynamic.reflect_hash();

// The hashes are not equal because `concrete` uses its own `Hash` impl
// while `dynamic` uses a reflection-based hashing algorithm
assert_ne!(concrete_hash, dynamic_hash);
```

Because the Dynamic proxy only knows about the name of the type, it's
unaware of any other information about it. This means it also differs on
`Reflect::reflect_partial_eq`, and may include ignored or skipped fields
in places the concrete type wouldn't.

## Solution

Rather than having Dynamics pass along just the type name of proxied
types, we can instead have them pass around the `TypeInfo`.

Now all Dynamic types contain an `Option<&'static TypeInfo>` rather than
a `String`:

```diff
pub struct DynamicTupleStruct {
-    type_name: String,
+    represented_type: Option<&'static TypeInfo>,
    fields: Vec<Box<dyn Reflect>>,
}
```

By changing `Reflect::get_type_info` to
`Reflect::represented_type_info`, hopefully we make this behavior a
little clearer. And to account for `None` values on these dynamic types,
`Reflect::represented_type_info` now returns `Option<&'static
TypeInfo>`.

```rust
let mut data = DynamicTupleStruct::default();

// Not proxying any specific type
assert!(dyn_tuple_struct.represented_type_info().is_none());

let type_info = <Foo as Typed>::type_info();
dyn_tuple_struct.set_represented_type(Some(type_info));
// Alternatively:
// let dyn_tuple_struct = foo.clone_dynamic();

// Now we're proxying `Foo`
assert!(dyn_tuple_struct.represented_type_info().is_some());
```

This means that we can have full access to all the static type
information for the proxied type. Future work would include
transitioning more static type information (trait impls, attributes,
etc.) over to the `TypeInfo` so it can actually be utilized by Dynamic
proxies.

### Alternatives & Rationale

> **Note** 
> These alternatives were written when this PR was first made using a
`Proxy` trait. This trait has since been removed.

<details>
<summary>View</summary>

#### Alternative: The `Proxy<T>` Approach

I had considered adding something like a `Proxy<T>` type where `T` would
be the Dynamic and would contain the proxied type information.

This was nice in that it allows us to explicitly determine whether
something is a proxy or not at a type level. `Proxy<DynamicStruct>`
proxies a struct. Makes sense.

The reason I didn't go with this approach is because (1) tuples, (2)
complexity, and (3) `PartialReflect`.

The `DynamicTuple` struct allows us to represent tuples at runtime. It
also allows us to do something you normally can't with tuples: add new
fields. Because of this, adding a field immediately invalidates the
proxy (e.g. our info for `(i32, i32)` doesn't apply to `(i32, i32,
NewField)`). By going with this PR's approach, we can just remove the
type info on `DynamicTuple` when that happens. However, with the
`Proxy<T>` approach, it becomes difficult to represent this behavior—
we'd have to completely control how we access data for `T` for each `T`.

Secondly, it introduces some added complexities (aside from the manual
impls for each `T`). Does `Proxy<T>` impl `Reflect`? Likely yes, if we
want to represent it as `dyn Reflect`. What `TypeInfo` do we give it?
How would we forward reflection methods to the inner type (remember, we
don't have specialization)? How do we separate this from Dynamic types?
And finally, how do all this in a way that's both logical and intuitive
for users?

Lastly, introducing a `Proxy` trait rather than a `Proxy<T>` struct is
actually more inline with the [Unique Reflect
RFC](https://github.com/bevyengine/rfcs/pull/56). In a way, the `Proxy`
trait is really one part of the `PartialReflect` trait introduced in
that RFC (it's technically not in that RFC but it fits well with it),
where the `PartialReflect` serves as a way for proxies to work _like_
concrete types without having full access to everything a concrete
`Reflect` type can do. This would help bridge the gap between the
current state of the crate and the implementation of that RFC.

All that said, this is still a viable solution. If the community
believes this is the better path forward, then we can do that instead.
These were just my reasons for not initially going with it in this PR.

#### Alternative: The Type Registry Approach

The `Proxy` trait is great and all, but how does it solve the original
problem? Well, it doesn't— yet!

The goal would be to start moving information from the derive macro and
its attributes to the generated `TypeInfo` since these are known
statically and shouldn't change. For example, adding `ignored: bool` to
`[Un]NamedField` or a list of impls.

However, there is another way of storing this information. This is, of
course, one of the uses of the `TypeRegistry`. If we're worried about
Dynamic proxies not aligning with their concrete counterparts, we could
move more type information to the registry and require its usage.

For example, we could replace `Reflect::reflect_hash(&self)` with
`Reflect::reflect_hash(&self, registry: &TypeRegistry)`.

That's not the _worst_ thing in the world, but it is an ergonomics loss.

Additionally, other attributes may have their own requirements, further
restricting what's possible without the registry. The `Reflect::apply`
method will require the registry as well now. Why? Well because the
`map_apply` function used for the `Reflect::apply` impls on `Map` types
depends on `Map::insert_boxed`, which (at least for `DynamicMap`)
requires `Reflect::reflect_hash`. The same would apply when adding
support for reflection-based diffing, which will require
`Reflect::reflect_partial_eq`.

Again, this is a totally viable alternative. I just chose not to go with
it for the reasons above. If we want to go with it, then we can close
this PR and we can pursue this alternative instead.

#### Downsides

Just to highlight a quick potential downside (likely needs more
investigation): retrieving the `TypeInfo` requires acquiring a lock on
the `GenericTypeInfoCell` used by the `Typed` impls for generic types
(non-generic types use a `OnceBox which should be faster). I am not sure
how much of a performance hit that is and will need to run some
benchmarks to compare against.

</details>

### Open Questions

1. Should we use `Cow<'static, TypeInfo>` instead? I think that might be
easier for modding? Perhaps, in that case, we need to update
`Typed::type_info` and friends as well?
2. Are the alternatives better than the approach this PR takes? Are
there other alternatives?

---

## Changelog

### Changed

- `Reflect::get_type_info` has been renamed to
`Reflect::represented_type_info`
- This method now returns `Option<&'static TypeInfo>` rather than just
`&'static TypeInfo`

### Added

- Added `Reflect::is_dynamic` method to indicate when a type is dynamic
- Added a `set_represented_type` method on all dynamic types

### Removed

- Removed `TypeInfo::Dynamic` (use `Reflect::is_dynamic` instead)
- Removed `Typed` impls for all dynamic types

## Migration Guide

- The Dynamic types no longer take a string type name. Instead, they
require a static reference to `TypeInfo`:

    ```rust
    #[derive(Reflect)]
    struct MyTupleStruct(f32, f32);
    
    let mut dyn_tuple_struct = DynamicTupleStruct::default();
    dyn_tuple_struct.insert(1.23_f32);
    dyn_tuple_struct.insert(3.21_f32);
    
    // BEFORE:
    let type_name = std::any::type_name::<MyTupleStruct>();
    dyn_tuple_struct.set_name(type_name);
    
    // AFTER:
    let type_info = <MyTupleStruct as Typed>::type_info();
    dyn_tuple_struct.set_represented_type(Some(type_info));
    ```

- `Reflect::get_type_info` has been renamed to
`Reflect::represented_type_info` and now also returns an
`Option<&'static TypeInfo>` (instead of just `&'static TypeInfo`):

    ```rust
    // BEFORE:
    let info: &'static TypeInfo = value.get_type_info();
    // AFTER:
let info: &'static TypeInfo = value.represented_type_info().unwrap();
    ```

- `TypeInfo::Dynamic` and `DynamicInfo` has been removed. Use
`Reflect::is_dynamic` instead:
   
    ```rust
    // BEFORE:
    if matches!(value.get_type_info(), TypeInfo::Dynamic) {
      // ...
    }
    // AFTER:
    if value.is_dynamic() {
      // ...
    }
    ```

---------

Co-authored-by: radiish <cb.setho@gmail.com>
2023-04-26 12:17:46 +00:00
Gino Valente
cd1737ecca bevy_reflect: Improved documentation (#7148)
# Objective

`bevy_reflect` can be a moderately complex crate to try and understand. It has many moving parts, a handful of gotchas, and a few subtle contracts that aren't immediately obvious to users and even other contributors.

The current README does an okay job demonstrating how the crate can be used. However, the crate's actual documentation should give a better overview of the crate, its inner-workings, and show some of its own examples.

## Solution

Added crate-level documentation that attempts to summarize the main parts of `bevy_reflect` into small sections.

This PR also updates the documentation for:
- `Reflect`
- `FromReflect`
- The reflection subtraits
- Other important types and traits
- The reflection macros (including the derive macros)
- Crate features

### Open Questions

1. ~~Should I update the docs for the Dynamic types? I was originally going to, but I'm getting a little concerned about the size of this PR 😅~~ Decided to not do this in this PR. It'll be better served from its own PR.
2. Should derive macro documentation be moved to the trait itself? This could improve visibility and allow for better doc links, but could also clutter up the trait's documentation (as well as not being on the actual derive macro's documentation).

### TODO

- [ ] ~~Document Dynamic types (?)~~ I think this should be done in a separate PR.
- [x] Document crate features
- [x] Update docs for `GetTypeRegistration`
- [x] Update docs for `TypeRegistration`
- [x] Update docs for `derive_from_reflect`
- [x] Document `reflect_trait`
- [x] Document `impl_reflect_value`
- [x] Document `impl_from_reflect_value`

---

## Changelog

- Updated documentation across the `bevy_reflect` crate
- Removed `#[module]` helper attribute for `Reflect` derives (this is not currently used)

## Migration Guide

- Removed `#[module]` helper attribute for `Reflect` derives. If your code is relying on this attribute, please replace it with either `#[reflect]` or `#[reflect_value]` (dependent on use-case).


Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
2023-02-18 20:42:01 +00:00
radiish
9498bfffcb Add remove method to Map reflection trait. (#6564)
# Objective

- Implements removal of entries from a `dyn Map`
- Fixes #6563

## Solution

- Adds a `remove` method to the `Map` trait which takes in a `&dyn Reflect` key and returns the value removed if it was present.

---

## Changelog

- Added `Map::remove`

## Migration Guide

- Implementors of `Map` will need to implement the `remove` method.


Co-authored-by: radiish <thesethskigamer@gmail.com>
2022-11-14 21:03:39 +00:00
Gino Valente
02fbf16c80 bevy_reflect: Add Reflect::into_reflect (#6502)
# Objective

Using `Reflect` we can easily switch between a specific reflection trait object, such as a `dyn Struct`, to a `dyn Reflect` object via `Reflect::as_reflect` or `Reflect::as_reflect_mut`.

```rust
fn do_something(value: &dyn Reflect) {/* ... */}

let foo: Box<dyn Struct> = Box::new(Foo::default());
do_something(foo.as_reflect());
```

However, there is no way to convert a _boxed_ reflection trait object to a `Box<dyn Reflect>`.

## Solution

Add a `Reflect::into_reflect` method which allows converting a boxed reflection trait object back into a boxed `Reflect` trait object.

```rust
fn do_something(value: Box<dyn Reflect>) {/* ... */}

let foo: Box<dyn Struct> = Box::new(Foo::default());
do_something(foo.into_reflect());
```

---

## Changelog

- Added `Reflect::into_reflect`
2022-11-07 02:11:16 +00:00
Hennadii Chernyshchyk
feebbc5ea9 Add reflect_owned (#6494)
# Objective

There is no way to gen an owned value of `Reflect`.

## Solution

Add it! This was originally a part of #6421, but @MrGVSV asked me to create a separate for it to implement reflect diffing.

---

## Changelog

### Added

- `Reflect::reflect_owned` to get an owned version of `Reflect`.
2022-11-06 16:58:38 +00:00
Carter Anderson
e5905379de Use new let-else syntax where possible (#6463)
# Objective

Let-else syntax is now stable!

## Solution

Use it where possible!
2022-11-04 21:32:09 +00:00
Gino Valente
a658bfef19 bevy_reflect: Reflect doc comments (#6234)
# Objective

Resolves #6197

Make it so that doc comments can be retrieved via reflection.

## Solution

Adds the new `documentation` feature to `bevy_reflect` (disabled by default).

When enabled, documentation can be found using `TypeInfo::doc` for reflected types:

```rust
/// Some struct.
///
/// # Example
///
/// ```ignore
/// let some_struct = SomeStruct;
/// ```
#[derive(Reflect)]
struct SomeStruct;

let info = <SomeStruct as Typed>::type_info();
assert_eq!(
    Some(" Some struct.\n\n # Example\n\n ```ignore\n let some_struct = SomeStruct;\n ```"),
    info.docs()
);
```

### Notes for Reviewers

The bulk of the files simply added the same 16 lines of code (with slightly different documentation). Most of the real changes occur in the `bevy_reflect_derive` files as well as in the added tests.

---

## Changelog

* Added `documentation` feature to `bevy_reflect`
* Added `TypeInfo::docs` method (and similar methods for all info types)
2022-10-18 13:49:57 +00:00
Gino Valente
ecc584ff23 bevy_reflect: Get owned fields (#5728)
# Objective

Sometimes it's useful to be able to retrieve all the fields of a container type so that they may be processed separately. With reflection, however, we typically only have access to references.

The only alternative is to "clone" the value using `Reflect::clone_value`. This, however, returns a Dynamic type in most cases. The solution there would be to use `FromReflect` instead, but this also has a problem in that it means we need to add `FromReflect` as an additional bound.

## Solution

Add a `drain` method to all container traits. This returns a `Vec<Box<dyn Reflect>>` (except for `Map` which returns `Vec<(Box<dyn Reflect>, Box<dyn Reflect>)>`).

This allows us to do things a lot simpler. For example, if we finished processing a struct and just need a particular value:

```rust
// === OLD === //
/// May or may not return a Dynamic*** value (even if `container` wasn't a `DynamicStruct`)
fn get_output(container: Box<dyn Struct>, output_index: usize) -> Box<dyn Reflect> {
  container.field_at(output_index).unwrap().clone_value()
}

// === NEW === //
/// Returns _exactly_ whatever was in the given struct
fn get_output(container: Box<dyn Struct>, output_index: usize) -> Box<dyn Reflect> {
  container.drain().remove(output_index).unwrap()
}
```

### Discussion

* Is `drain` the best method name? It makes sense that it "drains" all the fields and that it consumes the container in the process, but I'm open to alternatives.

---

## Changelog

* Added a `drain` method to the following traits:
  * `Struct`
  * `TupleStruct`
  * `Tuple`
  * `Array`
  * `List`
  * `Map`
  * `Enum`
2022-08-30 21:20:58 +00:00
Afonso Lage
40982cd0a2 Make reflect_partial_eq return more accurate results (#5210)
# Objective

Closes #5204

## Solution

- Followed @nicopap suggestion on https://github.com/bevyengine/bevy/pull/4761#discussion_r903982224

## Changelog

- [x] Updated [struct_trait](dfe9690052/crates/bevy_reflect/src/struct_trait.rs (L455-L457)), [tuple_struct](dfe9690052/crates/bevy_reflect/src/tuple_struct.rs (L366-L368)), [tuple](dfe9690052/crates/bevy_reflect/src/tuple.rs (L386)), [array](dfe9690052/crates/bevy_reflect/src/array.rs (L335-L337)), [list](dfe9690052/crates/bevy_reflect/src/list.rs (L309-L311)) and [map](dfe9690052/crates/bevy_reflect/src/map.rs (L361-L363)) to return `None` when comparison couldn't be performed.
- [x] Updated docs comments to reflect above changes.
2022-07-05 17:41:54 +00:00
James Liu
5498ef81fb bevy_reflect: support map insertion (#5173)
# Objective

This is a rebase of #3701 which is currently scheduled for 0.8 but is marked for adoption.

> Fixes https://github.com/bevyengine/bevy/discussions/3609

## Solution
> - add an `insert_boxed()` method on the `Map` trait
> - implement it for `HashMap` using a new `FromReflect` generic bound
> - add a `map_apply()` helper method to implement `Map::apply()`, that inserts new values instead of ignoring them


---

## Changelog
TODO

Co-authored-by: james7132 <contact@jamessliu.com>
2022-07-04 13:04:19 +00:00
PROMETHIA-27
c27a3cff6d Make Reflect safe to implement (#5010)
# Objective

Currently, `Reflect` is unsafe to implement because of a contract in which `any` and `any_mut` must return `self`, or `downcast` will cause UB. This PR makes `Reflect` safe, makes `downcast` not use unsafe, and eliminates this contract. 

## Solution

This PR adds a method to `Reflect`, `any`. It also renames the old `any` to `as_any`.
`any` now takes a `Box<Self>` and returns a `Box<dyn Any>`. 

---

## Changelog

### Added:
- `any()` method
- `represents()` method

### Changed:
- `Reflect` is now a safe trait
- `downcast()` is now safe
- The old `any` is now called `as_any`, and `any_mut` is now `as_mut_any`

## Migration Guide

- Reflect derives should not have to change anything
- Manual reflect impls will need to remove the `unsafe` keyword, add `any()` implementations, and rename the old `any` and `any_mut` to `as_any` and `as_mut_any`.
- Calls to `any`/`any_mut` must be changed to `as_any`/`as_mut_any`

## Points of discussion:

- Should renaming `any` be avoided and instead name the new method `any_box`?
- ~~Could there be a performance regression from avoiding the unsafe? I doubt it, but this change does seem to introduce redundant checks.~~
- ~~Could/should `is` and `type_id()` be implemented differently? For example, moving `is` onto `Reflect` as an `fn(&self, TypeId) -> bool`~~


Co-authored-by: PROMETHIA-27 <42193387+PROMETHIA-27@users.noreply.github.com>
2022-06-27 16:52:25 +00:00
Gino Valente
e6f34ba47f bevy_reflect: Add statically available type info for reflected types (#4042)
# Objective

> Resolves #4504

It can be helpful to have access to type information without requiring an instance of that type. Especially for `Reflect`, a lot of the gathered type information is known at compile-time and should not necessarily require an instance.

## Solution

Created a dedicated `TypeInfo` enum to store static type information. All types that derive `Reflect` now also implement the newly created `Typed` trait:

```rust
pub trait Typed: Reflect {
  fn type_info() -> &'static TypeInfo;
}
```

> Note: This trait was made separate from `Reflect` due to `Sized` restrictions.

If you only have access to a `dyn Reflect`, just call `.get_type_info()` on it. This new trait method on `Reflect` should return the same value as if you had called it statically. 

If all you have is a `TypeId` or type name, you can get the `TypeInfo` directly from the registry using the `TypeRegistry::get_type_info` method (assuming it was registered).

### Usage

Below is an example of working with `TypeInfo`. As you can see, we don't have to generate an instance of `MyTupleStruct` in order to get this information.

```rust
#[derive(Reflect)]
struct MyTupleStruct(usize, i32, MyStruct);

let info = MyTupleStruct::type_info();
if let TypeInfo::TupleStruct(info) = info {
  assert!(info.is::<MyTupleStruct>());
  assert_eq!(std::any::type_name::<MyTupleStruct>(), info.type_name());
  assert!(info.field_at(1).unwrap().is::<i32>());
} else {
  panic!("Expected `TypeInfo::TupleStruct`");
}
```

### Manual Implementations

It's not recommended to manually implement `Typed` yourself, but if you must, you can use the `TypeInfoCell` to automatically create and manage the static `TypeInfo`s for you (which is very helpful for blanket/generic impls):

```rust
use bevy_reflect::{Reflect, TupleStructInfo, TypeInfo, UnnamedField};
use bevy_reflect::utility::TypeInfoCell;

struct Foo<T: Reflect>(T);

impl<T: Reflect> Typed for Foo<T> {
  fn type_info() -> &'static TypeInfo {
    static CELL: TypeInfoCell = TypeInfoCell::generic();
    CELL.get_or_insert::<Self, _>(|| {
      let fields = [UnnamedField:🆕:<T>()];
      let info = TupleStructInfo:🆕:<Self>(&fields);
      TypeInfo::TupleStruct(info)
    })
  }
}
```

## Benefits

One major benefit is that this opens the door to other serialization methods. Since we can get all the type info at compile time, we can know how to properly deserialize something like:

```rust
#[derive(Reflect)]
struct MyType {
  foo: usize,
  bar: Vec<String>
}

// RON to be deserialized:
(
  type: "my_crate::MyType", // <- We now know how to deserialize the rest of this object
  value: {
    // "foo" is a value type matching "usize"
    "foo": 123,
    // "bar" is a list type matching "Vec<String>" with item type "String"
    "bar": ["a", "b", "c"]
  }
)
```

Not only is this more compact, but it has better compatibility (we can change the type of `"foo"` to `i32` without having to update our serialized data).

Of course, serialization/deserialization strategies like this may need to be discussed and fully considered before possibly making a change. However, we will be better equipped to do that now that we can access type information right from the registry.

## Discussion

Some items to discuss:

1. Duplication. There's a bit of overlap with the existing traits/structs since they require an instance of the type while the type info structs do not (for example, `Struct::field_at(&self, index: usize)` and `StructInfo::field_at(&self, index: usize)`, though only `StructInfo` is accessible without an instance object). Is this okay, or do we want to handle it in another way?
2. Should `TypeInfo::Dynamic` be removed? Since the dynamic types don't have type information available at runtime, we could consider them `TypeInfo::Value`s (or just even just `TypeInfo::Struct`). The intention with `TypeInfo::Dynamic` was to keep the distinction from these dynamic types and actual structs/values since users might incorrectly believe the methods of the dynamic type's info struct would map to some contained data (which isn't possible statically).
4. General usefulness of this change, including missing/unnecessary parts.
5. Possible changes to the scene format? (One possible issue with changing it like in the example above might be that we'd have to be careful when handling generic or trait object types.)

## Compile Tests

I ran a few tests to compare compile times (as suggested [here](https://github.com/bevyengine/bevy/pull/4042#discussion_r876408143)). I toggled `Reflect` and `FromReflect` derive macros using `cfg_attr` for both this PR (aa5178e773) and main (c309acd432).

<details>
<summary>See More</summary>

The test project included 250 of the following structs (as well as a few other structs):

```rust
#[derive(Default)]
#[cfg_attr(feature = "reflect", derive(Reflect))]
#[cfg_attr(feature = "from_reflect", derive(FromReflect))]
pub struct Big001 {
    inventory: Inventory,
    foo: usize,
    bar: String,
    baz: ItemDescriptor,
    items: [Item; 20],
    hello: Option<String>,
    world: HashMap<i32, String>,
    okay: (isize, usize, /* wesize */),
    nope: ((String, String), (f32, f32)),
    blah: Cow<'static, str>,
}
```

> I don't know if the compiler can optimize all these duplicate structs away, but I think it's fine either way. We're comparing times, not finding the absolute worst-case time.

I only ran each build 3 times using `cargo build --timings` (thank you @devil-ira), each of which were preceeded by a `cargo clean --package bevy_reflect_compile_test`. 

Here are the times I got:

| Test                             | Test 1 | Test 2 | Test 3 | Average |
| -------------------------------- | ------ | ------ | ------ | ------- |
| Main                             | 1.7s   | 3.1s   | 1.9s   | 2.33s   |
| Main + `Reflect`                 | 8.3s   | 8.6s   | 8.1s   | 8.33s   |
| Main + `Reflect` + `FromReflect` | 11.6s  | 11.8s  | 13.8s  | 12.4s   |
| PR                               | 3.5s   | 1.8s   | 1.9s   | 2.4s    |
| PR + `Reflect`                   | 9.2s   | 8.8s   | 9.3s   | 9.1s    |
| PR + `Reflect` + `FromReflect`   | 12.9s  | 12.3s  | 12.5s  | 12.56s  |

</details>

---

## Future Work

Even though everything could probably be made `const`, we unfortunately can't. This is because `TypeId::of::<T>()` is not yet `const` (see https://github.com/rust-lang/rust/issues/77125). When it does get stabilized, it would probably be worth coming back and making things `const`. 

Co-authored-by: MrGVSV <49806985+MrGVSV@users.noreply.github.com>
2022-06-09 21:18:15 +00:00
Gino Valente
2f5591ff8c bevy_reflect: Improve debug formatting for reflected types (#4218)
# Objective

Debugging reflected types can be somewhat frustrating since all `dyn Reflect` trait objects return something like `Reflect(core::option::Option<alloc::string::String>)`.

It would be much nicer to be able to see the actual value— or even use a custom `Debug` implementation.

## Solution

Added `Reflect::debug` which allows users to customize the debug output. It sets defaults for all `ReflectRef` subtraits and falls back to `Reflect(type_name)` if no `Debug` implementation was registered.

To register a custom `Debug` impl, users can add `#[reflect(Debug)]` like they can with other traits.

### Example

Using the following structs:

```rust
#[derive(Reflect)]
pub struct Foo {
    a: usize,
    nested: Bar,
    #[reflect(ignore)]
    _ignored: NonReflectedValue,
}

#[derive(Reflect)]
pub struct Bar {
    value: Vec2,
    tuple_value: (i32, String),
    list_value: Vec<usize>,
    // We can't determine debug formatting for Option<T> yet
    unknown_value: Option<String>,
    custom_debug: CustomDebug
}

#[derive(Reflect)]
#[reflect(Debug)]
struct CustomDebug;

impl Debug for CustomDebug {
    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
        write!(f, "This is a custom debug!")
    }
}

pub struct NonReflectedValue {
    _a: usize,
}
```

We can do:

```rust
let value = Foo {
  a: 1,
  _ignored: NonReflectedValue { _a: 10 },
  nested: Bar {
    value: Vec2::new(1.23, 3.21),
    tuple_value: (123, String::from("Hello")),
    list_value: vec![1, 2, 3],
    unknown_value: Some(String::from("World")),
    custom_debug: CustomDebug
  },
};
let reflected_value: &dyn Reflect = &value;
println!("{:#?}", reflected_value)
```

Which results in:

```rust
Foo {
  a: 2,
  nested: Bar {
    value: Vec2(
      1.23,
      3.21,
    ),
    tuple_value: (
      123,
      "Hello",
    ),
    list_value: [
      1,
      2,
      3,
    ],
    unknown_value: Reflect(core::option::Option<alloc::string::String>),
    custom_debug: This is a custom debug!,
  },
}
```

Notice that neither `Foo` nor `Bar` implement `Debug`, yet we can still deduce it. This might be a concern if we're worried about leaking internal values. If it is, we might want to consider a way to exclude fields (possibly with a `#[reflect(hide)]` macro) or make it purely opt in (as opposed to the default implementation automatically handled by ReflectRef subtraits).

Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
2022-05-30 16:41:31 +00:00
MrGVSV
15acd6f45d bevy_reflect: Small refactor and default Reflect methods (#4739)
# Objective

Quick followup to #4712.

While updating some [other PRs](https://github.com/bevyengine/bevy/pull/4218), I realized the `ReflectTraits` struct could be improved. The issue with the current implementation is that `ReflectTraits::get_xxx_impl(...)` returns just the _logic_ to the corresponding `Reflect` trait method, rather than the entire function.

This makes it slightly more annoying to manage since the variable names need to be consistent across files. For example, `get_partial_eq_impl` uses a `value` variable. But the name "value" isn't defined in the `get_partial_eq_impl` method, it's defined in three other methods in a completely separate file.

It's not likely to cause any bugs if we keep it as it is since differing variable names will probably just result in a compile error (except in very particular cases). But it would be useful to someone who wanted to edit/add/remove a method.

## Solution

Made `get_hash_impl`, `get_partial_eq_impl` and `get_serialize_impl` return the entire method implementation for `reflect_hash`, `reflect_partial_eq`, and `serializable`, respectively.

As a result of this, those three `Reflect` methods were also given default implementations. This was fairly simple to do since all three could just be made to return `None`.

---

## Changelog

* Small cleanup/refactor to `ReflectTraits` in `bevy_reflect_derive`
* Gave `Reflect::reflect_hash`, `Reflect::reflect_partial_eq`, and `Reflect::serializable` default implementations
2022-05-18 12:26:11 +00:00
MrGVSV
00f83941b1 bevy_reflect: IntoIter for DynamicList and DynamicMap (#4108)
# Objective

In some cases, you may want to take ownership of the values in `DynamicList` or `DynamicMap`. 

I came across this need while trying to implement a custom deserializer, but couldn't get ownership of the values in the list.

## Solution

Implemented `IntoIter` for both `DynamicList` and `DynamicMap`.
2022-04-26 00:17:38 +00:00
MrGVSV
5047e1f08e bevy_reflect: Add as_reflect and as_reflect_mut (#4350)
# Objective

Trait objects that have `Reflect` as a supertrait cannot be upcast to a `dyn Reflect`.

Attempting something like:

```rust
trait MyTrait: Reflect {
  // ...
}

fn foo(value: &dyn MyTrait) {
  let reflected = value as &dyn Reflect; // Error!
  // ...
}
```

Results in `error[E0658]: trait upcasting coercion is experimental`.

The reason this is important is that a lot of `bevy_reflect` methods require a `&dyn Reflect`. This is trivial with concrete types, but if we don't know the concrete type (we only have the trait object), we can't use these methods. For example, we couldn't create a `ReflectSerializer` for the type since it expects a `&dyn Reflect` value— even though we should be able to.

## Solution

Add `as_reflect` and `as_reflect_mut` to `Reflect` to allow upcasting to a `dyn Reflect`:

```rust
trait MyTrait: Reflect {
  // ...
}

fn foo(value: &dyn MyTrait) {
  let reflected = value.as_reflect();
  // ...
}
```

## Alternatives

We could defer this type of logic to the crate/user. They can add these methods to their trait in the same exact way we do here. The main benefit of doing it ourselves is it makes things convenient for them (especially when using the derive macro).

We could also create an `AsReflect` trait with a blanket impl over all reflected types, however, I could not get that to work for trait objects since they aren't sized.

---

## Changelog

- Added trait method `Reflect::as_reflect(&self)`
- Added trait method `Reflect::as_reflect_mut(&mut self)`

## Migration Guide

- Manual implementors of `Reflect` will need to add implementations for the methods above (this should be pretty easy as most cases just need to return `self`)
2022-04-25 13:54:48 +00:00
Carter Anderson
b3a1db60f2 Proper prehashing (#3963)
For some keys, it is too expensive to hash them on every lookup. Historically in Bevy, we have regrettably done the "wrong" thing in these cases (pre-computing hashes, then re-hashing them) because Rust's built in hashed collections don't give us the tools we need to do otherwise. Doing this is "wrong" because two different values can result in the same hash. Hashed collections generally get around this by falling back to equality checks on hash collisions. You can't do that if the key _is_ the hash. Additionally, re-hashing a hash increase the odds of collision!
 
#3959 needs pre-hashing to be viable, so I decided to finally properly solve the problem. The solution involves two different changes:

1. A new generalized "pre-hashing" solution in bevy_utils: `Hashed<T>` types, which store a value alongside a pre-computed hash. And `PreHashMap<K, V>` (which uses `Hashed<T>` internally) . `PreHashMap` is just an alias for a normal HashMap that uses `Hashed<T>` as the key and a new `PassHash` implementation as the Hasher. 
2. Replacing the `std::collections` re-exports in `bevy_utils` with equivalent `hashbrown` impls. Avoiding re-hashes requires the `raw_entry_mut` api, which isn't stabilized yet (and may never be ... `entry_ref` has favor now, but also isn't available yet). If std's HashMap ever provides the tools we need, we can move back to that. The latest version of `hashbrown` adds support for the `entity_ref` api, so we can move to that in preparation for an std migration, if thats the direction they seem to be going in. Note that adding hashbrown doesn't increase our dependency count because it was already in our tree.

In addition to providing these core tools, I also ported the "table identity hashing" in `bevy_ecs` to `raw_entry_mut`, which was a particularly egregious case.

The biggest outstanding case is `AssetPathId`, which stores a pre-hash. We need AssetPathId to be cheaply clone-able (and ideally Copy), but `Hashed<AssetPath>` requires ownership of the AssetPath, which makes cloning ids way more expensive. We could consider doing `Hashed<Arc<AssetPath>>`, but cloning an arc is still a non-trivial expensive that needs to be considered. I would like to handle this in a separate PR. And given that we will be re-evaluating the Bevy Assets implementation in the very near future, I'd prefer to hold off until after that conversation is concluded.
2022-02-18 03:26:01 +00:00
danieleades
d8974e7c3d small and mostly pointless refactoring (#2934)
What is says on the tin.

This has got more to do with making `clippy` slightly more *quiet* than it does with changing anything that might greatly impact readability or performance.

that said, deriving `Default` for a couple of structs is a nice easy win
2022-02-13 22:33:55 +00:00
dataphract
f073b2d7f3 document more of bevy_reflect (#3655)
This adds documentation for:

- The trait methods of `Reflect` and its subtraits
- The `partial_eq` and `apply` functions for `Map` et al.
- `DynamicList` and `DynamicMap`
- `TypeRegistry` and related types & traits
- `GetPath`, including an explanation of path string syntax

among other things.

Still to be documented are the various macros and `bevy_reflect::serde`.
2022-01-14 19:09:44 +00:00
Michael Dorst
b1e4984c55 Fix doc_markdown lints in bevy_reflect (#3478)
#3457 adds the `doc_markdown` clippy lint, which checks doc comments to make sure code identifiers are escaped with backticks. This causes a lot of lint errors, so this is one of a number of PR's that will fix those lint errors one crate at a time.

This PR fixes lints in the `bevy_reflect` crate.
2021-12-29 17:38:12 +00:00