# Objective
Three impls are generated for each of these traits when the
`reflect_functions` feature is enabled.
Helps with #19873.
## Solution
Two of the three (the `&T` and `&mut T` ones) can be avoided by instead
providing blanket impls. The impl for `T` remains.
## Testing
I checked the output via `cargo expand`.
According to `-Zmacro-stats`, the size of the `Reflect` code generate
for `bevy_ui` drops by 10.4%.
# Objective
There is a pattern that appears in multiple places, involving
`reflect_clone`, followed by `take`, followed by `map_err` that produces
a `FailedDowncast` in a particular form.
## Solution
Introduces `reflect_clone_and_take`, which factors out the repeated
code.
## Testing
`cargo run -p ci`
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
All the derived reflection methods currently have multiple trait bounds
on non-generic field types, which serve no purpose. The are emitted
because "emit bounds on all fields" is easier than "emit bounds on
fields that need them". But improving things isn't too hard.
Similarly, lots of useless `Any + Send + Sync` bounds exist on
non-generic types.
Helps a lot with #19873.
## Solution
Remove the unnecessary bounds by only emitting them if the relevant type
is generic.
## Testing
I used `cargo expand` to confirm the unnecessary bounds are no longer
produced.
`-Zmacro-stats` output tells me this reduces the size of the `Reflect`
code produced for `bevy_ui` by 21.2%.
# Objective
- Fixes https://github.com/bevyengine/bevy/issues/14328
- `DynamicMap::drain` was broken (indices weren't cleared, causing a
panic when reading later)
- `PartialReflect::apply` was broken for maps and sets, because they
don't remove entries from the `self` map that aren't in the applied map.
- I discovered this bug when implementing MapEntities on a Component
containing a `HashMap<Entity, _>`. Because `apply` is used to reapply
the changes to the reflected map, the map ended up littered with a ton
of outdated entries.
## Solution
- Remove the separate `Vec` in `DynamicMap` and use the `HashTable`
directly, like it is in `DynamicSet`.
- Replace `MapIter` by `Box<dyn Iterator>` (like for `DynamicSet`), and
`Map::get_at` and `Map::get_at_mut` which are now unused.
- Now assume `DynamicMap` types are unordered and adjust documentation
accordingly.
- Fix documentation of `DynamicSet` (ordered -> unordered)
- Added `Map::retain` and `Set::retain`, and use them to remove excess
entries in `PartialReflect::apply` implementations.
## Testing
- Added `map::tests::apply` and `set::tests::apply` to validate
`<DynamicMap as PartialReflect>::apply` and `<DynamicSet as
PartialReflect>::apply`
# Objective
Fix https://github.com/bevyengine/bevy/issues/19617
# Solution
Add newlines before all impl blocks.
I suspect that at least some of these will be objectionable! If there's
a desired Bevy style for this then I'll update the PR. If not then we
can just close it - it's the work of a single find and replace.
# Objective
- When trying to serialize an structure that contains `&'static str`
using only Reflection, I get the following error:
```
"type `&str` did not register the `ReflectSerialize` or `ReflectSerializeWithRegistry` type data.
For certain types, this may need to be registered manually using `register_type_data` (stack: ... -> `core::option::Option<&str>` -> `&str`)")
```
## Solution
- Register `ReflectSerialize` for `&str`
## Testing
- `cargo run -p ci`: OK
> [!important]
> To **maintainers**: we should wait to merge this one in until after
#18944 lands so that cherry-picking the latter for 0.16.1 is simpler.
# Objective
The `std` module is where we implement the reflection traits for types
that are exported from `std` (including `core` and `alloc`). Over time,
this file has grown increasingly large, making it difficult to navigate
and a pain point for merge conflicts.
The goal of this PR is to break up the module into smaller chunks.
## Solution
The `std` module has been split into many submodules:
- `alloc`
- `bevy_platform`
- `core`
- `std`
Each of these new modules is comprised of submodules that closely
resemble the actual module. For example, the impls for
`::alloc::vec::Vec` have been moved to
`bevy_reflect::impls::alloc::vec::Vec`.
Some liberties were taken. For example, `Cow<'static, Path>` was kept in
`bevy_reflect::impls::std::path` rather than
`bevy_reflect::impls::alloc::borrow`.
You may ask: _Isn't this a little overkill? Why does the one-line impl
for `TypeId` need its own file?_
And yes, it is partly overkill. But the benefit with this approach is
that where an `std`-related type should live is mostly unambiguous. If
we wanted to reflect `::core::net::Ipv4Addr`, it's very clear that it
should be done in `bevy_reflect::impls::core::net`.
We can discuss better ways of breaking this up if people have other
ideas or opinions, but I think this is a pretty straightforward way of
doing it.
### Note to Reviewers
The code is pretty much copy-paste from the mega module to the new
submodules. It's probably best to focus efforts on reviewing the general
module structure, as well as maybe which impls are included where.
You _can_ review the code contained within each impl, but I promise you
the only thing I touched were the paths in the macros so they could be
more hygienic :)
## Testing
You can just check that everything compiles still:
```
cargo check -p bevy_reflect --tests
```
# Objective
A fair few items were deprecated in 0.16. Let's delete them now that
we're in the 0.17 development cycle!
## Solution
- Deleted items marked deprecated in 0.16.
## Testing
- CI
---
## Notes
I'm making the assumption that _everything_ deprecated in 0.16 should be
removed in 0.17. That may be a false assumption in certain cases. Please
check the items to be removed to see if there are any exceptions we
should keep around for another cycle!
# Objective
- If using a `NonNilUuid` in Bevy, it's difficult to reflect it.
## Solution
- Adds `NonNilUuid` using `impl_reflect_opaque!`.
## Testing
- Built with no issues found locally.
- Essentially the same as the `Uuid` support except without `Default`.
Co-authored-by: TM Storey <mail@tmstorey.id.au>
# Objective
Fixes#18943
## Solution
Reintroduces support for `hashbrown`'s `HashMap` and `HashSet` types.
These were inadvertently removed when `bevy_platform` newtyped the
`hashbrown` types.
Since we removed our `hashbrown` dependency, I gated these impls behind
a `hashbrown` feature. Not entirely sure if this is necessary since we
enabled it for `bevy_reflect` through `bevy_platform` anyways. (Complex
features still confuse me a bit so let me know if I can just remove it!)
I also went ahead and preemptively implemented `TypePath` for `PassHash`
while I was here.
## Testing
You can test that it works by adding the following to a Bevy example
based on this PR (you'll also need to include `hashbrown` of course):
```rust
#[derive(Reflect)]
struct Foo(hashbrown::HashMap<String, String>);
```
Then check it compiles with:
```
cargo check --example hello_world --no-default-features --features=bevy_reflect/hashbrown
```
# Objective
The goal of `bevy_platform_support` is to provide a set of platform
agnostic APIs, alongside platform-specific functionality. This is a high
traffic crate (providing things like HashMap and Instant). Especially in
light of https://github.com/bevyengine/bevy/discussions/18799, it
deserves a friendlier / shorter name.
Given that it hasn't had a full release yet, getting this change in
before Bevy 0.16 makes sense.
## Solution
- Rename `bevy_platform_support` to `bevy_platform`.
# Objective
Fixes#18701
## Solution
Add reflection of `PartialEq` and `Hash` to `AnimationNodeIndex`
## Testing
Added a new `#[test]` with the minimal reproduction posted on #18701.
# Objective
#13432 added proper reflection-based cloning. This is a better method
than cloning via `clone_value` for reasons detailed in the description
of that PR. However, it may not be immediately apparent to users why one
should be used over the other, and what the gotchas of `clone_value`
are.
## Solution
This PR marks `PartialReflect::clone_value` as deprecated, with the
deprecation notice pointing users to `PartialReflect::reflect_clone`.
However, it also suggests using a new method introduced in this PR:
`PartialReflect::to_dynamic`.
`PartialReflect::to_dynamic` is essentially a renaming of
`PartialReflect::clone_value`. By naming it `to_dynamic`, we make it
very obvious that what's returned is a dynamic type. The one caveat to
this is that opaque types still use `reflect_clone` as they have no
corresponding dynamic type.
Along with changing the name, the method is now optional, and comes with
a default implementation that calls out to the respective reflection
subtrait method. This was done because there was really no reason to
require manual implementors provide a method that almost always calls
out to a known set of methods.
Lastly, to make this default implementation work, this PR also did a
similar thing with the `clone_dynamic ` methods on the reflection
subtraits. For example, `Struct::clone_dynamic` has been marked
deprecated and is superseded by `Struct::to_dynamic_struct`. This was
necessary to avoid the "multiple names in scope" issue.
### Open Questions
This PR maintains the original signature of `clone_value` on
`to_dynamic`. That is, it takes `&self` and returns `Box<dyn
PartialReflect>`.
However, in order for this to work, it introduces a panic if the value
is opaque and doesn't override the default `reflect_clone`
implementation.
One thing we could do to avoid the panic would be to make the conversion
fallible, either returning `Option<Box<dyn PartialReflect>>` or
`Result<Box<dyn PartialReflect>, ReflectCloneError>`.
This makes using the method a little more involved (i.e. users have to
either unwrap or handle the rare possibility of an error), but it would
set us up for a world where opaque types don't strictly need to be
`Clone`. Right now this bound is sort of implied by the fact that
`clone_value` is a required trait method, and the default behavior of
the macro is to use `Clone` for opaque types.
Alternatively, we could keep the signature but make the method required.
This maintains that implied bound where manual implementors must provide
some way of cloning the value (or YOLO it and just panic), but also
makes the API simpler to use.
Finally, we could just leave it with the panic. It's unlikely this would
occur in practice since our macro still requires `Clone` for opaque
types, and thus this would only ever be an issue if someone were to
manually implement `PartialReflect` without a valid `to_dynamic` or
`reflect_clone` method.
## Testing
You can test locally using the following command:
```
cargo test --package bevy_reflect --all-features
```
---
## Migration Guide
`PartialReflect::clone_value` is being deprecated. Instead, use
`PartialReflect::to_dynamic` if wanting to create a new dynamic instance
of the reflected value. Alternatively, use
`PartialReflect::reflect_clone` to attempt to create a true clone of the
underlying value.
Similarly, the following methods have been deprecated and should be
replaced with these alternatives:
- `Array::clone_dynamic` → `Array::to_dynamic_array`
- `Enum::clone_dynamic` → `Enum::to_dynamic_enum`
- `List::clone_dynamic` → `List::to_dynamic_list`
- `Map::clone_dynamic` → `Map::to_dynamic_map`
- `Set::clone_dynamic` → `Set::to_dynamic_set`
- `Struct::clone_dynamic` → `Struct::to_dynamic_struct`
- `Tuple::clone_dynamic` → `Tuple::to_dynamic_tuple`
- `TupleStruct::clone_dynamic` → `TupleStruct::to_dynamic_tuple_struct`
# Objective
Using `Reflect::clone_value` can be somewhat confusing to those
unfamiliar with how Bevy's reflection crate works. For example take the
following code:
```rust
let value: usize = 123;
let clone: Box<dyn Reflect> = value.clone_value();
```
What can we expect to be the underlying type of `clone`? If you guessed
`usize`, then you're correct! Let's try another:
```rust
#[derive(Reflect, Clone)]
struct Foo(usize);
let value: Foo = Foo(123);
let clone: Box<dyn Reflect> = value.clone_value();
```
What about this code? What is the underlying type of `clone`? If you
guessed `Foo`, unfortunately you'd be wrong. It's actually
`DynamicStruct`.
It's not obvious that the generated `Reflect` impl actually calls
`Struct::clone_dynamic` under the hood, which always returns
`DynamicStruct`.
There are already some efforts to make this a bit more apparent to the
end-user: #7207 changes the signature of `Reflect::clone_value` to
instead return `Box<dyn PartialReflect>`, signaling that we're
potentially returning a dynamic type.
But why _can't_ we return `Foo`?
`Foo` can obviously be cloned— in fact, we already derived `Clone` on
it. But even without the derive, this seems like something `Reflect`
should be able to handle. Almost all types that implement `Reflect`
either contain no data (trivially clonable), they contain a
`#[reflect_value]` type (which, by definition, must implement `Clone`),
or they contain another `Reflect` type (which recursively fall into one
of these three categories).
This PR aims to enable true reflection-based cloning where you get back
exactly the type that you think you do.
## Solution
Add a `Reflect::reflect_clone` method which returns `Result<Box<dyn
Reflect>, ReflectCloneError>`, where the `Box<dyn Reflect>` is
guaranteed to be the same type as `Self`.
```rust
#[derive(Reflect)]
struct Foo(usize);
let value: Foo = Foo(123);
let clone: Box<dyn Reflect> = value.reflect_clone().unwrap();
assert!(clone.is::<Foo>());
```
Notice that we didn't even need to derive `Clone` for this to work: it's
entirely powered via reflection!
Under the hood, the macro generates something like this:
```rust
fn reflect_clone(&self) -> Result<Box<dyn Reflect>, ReflectCloneError> {
Ok(Box::new(Self {
// The `reflect_clone` impl for `usize` just makes use of its `Clone` impl
0: Reflect::reflect_clone(&self.0)?.take().map_err(/* ... */)?,
}))
}
```
If we did derive `Clone`, we can tell `Reflect` to rely on that instead:
```rust
#[derive(Reflect, Clone)]
#[reflect(Clone)]
struct Foo(usize);
```
<details>
<summary>Generated Code</summary>
```rust
fn reflect_clone(&self) -> Result<Box<dyn Reflect>, ReflectCloneError> {
Ok(Box::new(Clone::clone(self)))
}
```
</details>
Or, we can specify our own cloning function:
```rust
#[derive(Reflect)]
#[reflect(Clone(incremental_clone))]
struct Foo(usize);
fn incremental_clone(value: &usize) -> usize {
*value + 1
}
```
<details>
<summary>Generated Code</summary>
```rust
fn reflect_clone(&self) -> Result<Box<dyn Reflect>, ReflectCloneError> {
Ok(Box::new(incremental_clone(self)))
}
```
</details>
Similarly, we can specify how fields should be cloned. This is important
for fields that are `#[reflect(ignore)]`'d as we otherwise have no way
to know how they should be cloned.
```rust
#[derive(Reflect)]
struct Foo {
#[reflect(ignore, clone)]
bar: usize,
#[reflect(ignore, clone = "incremental_clone")]
baz: usize,
}
fn incremental_clone(value: &usize) -> usize {
*value + 1
}
```
<details>
<summary>Generated Code</summary>
```rust
fn reflect_clone(&self) -> Result<Box<dyn Reflect>, ReflectCloneError> {
Ok(Box::new(Self {
bar: Clone::clone(&self.bar),
baz: incremental_clone(&self.baz),
}))
}
```
</details>
If we don't supply a `clone` attribute for an ignored field, then the
method will automatically return
`Err(ReflectCloneError::FieldNotClonable {/* ... */})`.
`Err` values "bubble up" to the caller. So if `Foo` contains `Bar` and
the `reflect_clone` method for `Bar` returns `Err`, then the
`reflect_clone` method for `Foo` also returns `Err`.
### Attribute Syntax
You might have noticed the differing syntax between the container
attribute and the field attribute.
This was purely done for consistency with the current attributes. There
are PRs aimed at improving this. #7317 aims at making the
"special-cased" attributes more in line with the field attributes
syntactically. And #9323 aims at moving away from the stringified paths
in favor of just raw function paths.
### Compatibility with Unique Reflect
This PR was designed with Unique Reflect (#7207) in mind. This method
actually wouldn't change that much (if at all) under Unique Reflect. It
would still exist on `Reflect` and it would still `Option<Box<dyn
Reflect>>`. In fact, Unique Reflect would only _improve_ the user's
understanding of what this method returns.
We may consider moving what's currently `Reflect::clone_value` to
`PartialReflect` and possibly renaming it to `partial_reflect_clone` or
`clone_dynamic` to better indicate how it differs from `reflect_clone`.
## Testing
You can test locally by running the following command:
```
cargo test --package bevy_reflect
```
---
## Changelog
- Added `Reflect::reflect_clone` method
- Added `ReflectCloneError` error enum
- Added `#[reflect(Clone)]` container attribute
- Added `#[reflect(clone)]` field attribute
# Objective
- Contributes to #15460
- Reduce quantity and complexity of feature gates across Bevy
## Solution
- Used `target_has_atomic` configuration variable to automatically
detect impartial atomic support and automatically switch to
`portable-atomic` over the standard library on an as-required basis.
## Testing
- CI
## Notes
To explain the technique employed here, consider getting `Arc` either
from `alloc::sync` _or_ `portable-atomic-util`. First, we can inspect
the `alloc` crate to see that you only have access to `Arc` _if_
`target_has_atomic = "ptr"`. We add a target dependency for this
particular configuration _inverted_:
```toml
[target.'cfg(not(target_has_atomic = "ptr"))'.dependencies]
portable-atomic-util = { version = "0.2.4", default-features = false }
```
This ensures we only have the dependency when it is needed, and it is
entirely excluded from the dependency graph when it is not. Next, we
adjust our configuration flags to instead of checking for `feature =
"portable-atomic"` to instead check for `target_has_atomic = "ptr"`:
```rust
// `alloc` feature flag hidden for brevity
#[cfg(not(target_has_atomic = "ptr"))]
use portable_atomic_util as arc;
#[cfg(target_has_atomic = "ptr")]
use alloc::sync as arc;
pub use arc::{Arc, Weak};
```
The benefits of this technique are three-fold:
1. For platforms without full atomic support, the functionality is
enabled automatically.
2. For platforms with atomic support, the dependency is never included,
even if a feature was enabled using `--all-features` (for example)
3. The `portable-atomic` feature no longer needs to virally spread to
all user-facing crates, it's instead something handled within
`bevy_platform_support` (with some extras where other dependencies also
need their features enabled).
This pr uses the `extern crate self as` trick to make proc macros behave
the same way inside and outside bevy.
# Objective
- Removes noise introduced by `crate as` in the whole bevy repo.
- Fixes#17004.
- Hardens proc macro path resolution.
## TODO
- [x] `BevyManifest` needs cleanup.
- [x] Cleanup remaining `crate as`.
- [x] Add proper integration tests to the ci.
## Notes
- `cargo-manifest-proc-macros` is written by me and based/inspired by
the old `BevyManifest` implementation and
[`bkchr/proc-macro-crate`](https://github.com/bkchr/proc-macro-crate).
- What do you think about the new integration test machinery I added to
the `ci`?
More and better integration tests can be added at a later stage.
The goal of these integration tests is to simulate an actual separate
crate that uses bevy. Ideally they would lightly touch all bevy crates.
## Testing
- Needs RA test
- Needs testing from other users
- Others need to run at least `cargo run -p ci integration-test` and
verify that they work.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# 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.
# Objective
- Contributes to #15460
## Solution
- Switched `tracing` for `log` for the atomically challenged platforms
- Setup feature flags as required
- Added to `compile-check-no-std` CI task
- Made `crossbeam-channel` optional depending on `std`.
## Testing
- CI
---
## Notes
- `crossbeam-channel` provides a MPMC channel type which isn't readily
replicable in `no_std`, and is only used for a `bevy_render`
integration. As such, I've feature-gated the `TimeReceiver` and
`TimeSender` types.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
Add reflection support to more `glam` `Vec` types, specifically
* I8Vec2
* I8Vec3
* I8Vec4
* U8Vec2
* U8Vec3
* U8Vec4
* I16Vec2
* I16Vec3
* I16Vec4
* U16Vec2
* U16Vec3
* U16Vec4
I needed to do this because I'm using various of these in my Bevy types,
and due to the orphan rules, I can't make these impls locally.
## Solution
Used `impl_reflect!` like for the existing types.
## Testing
This should not require additional testing, though I have verified that
reflection now works for these types in my own project.
# Objective
Fixes#17416
## Solution
I just included ReflectFromReflect in all macros and implementations. I
think this should be ok, at least it compiles properly and does fix the
errors in my test code.
## Testing
I generated a DynamicMap and tried to convert it into a concrete
`HashMap` as a `Box<dyn Reflect>`. Without my fix, it doesn't work,
because this line panics:
```rust
let rfr = ty.data::<ReflectFromReflect>().unwrap();
```
where `ty` is the `TypeRegistration` for the (matching) `HashMap`.
I don't know why `ReflectFromReflect` wasn't included everywhere, I
assume that it was an oversight and not an architecture decision I'm not
aware of.
# Migration Guide
The hasher in reflected `HashMap`s and `HashSet`s now have to implement
`Default`. This is the case for the ones provided by Bevy already, and
is generally a sensible thing to do.
# 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>
# Objective
- I'm compiling (parts of) bevy for an embedded platform with no 64bit
atomic and ctrlc handler support. Some compilation errors came up. This
PR contains the fixes for those.
- Fix depth_bias casting in PBR material (Fixes#14169)
- Negative depth_bias values were casted to 0 before this PR
- f32::INFINITY depth_bias value was casted to -1 before this PR
## Solutions
- Restrict 64bit atomic reflection to supported platforms
- Restrict ctrlc handler to supported platforms (linux, windows or macos
instead of "not wasm")
- The depth bias value (f32) is first casted to i32 then u64 in order to
preserve negative values
## Testing
- This version compiles on a platform with no 64bit atomic support, and
no ctrlc support
- CtrlC handler still works on Linux and Windows (I can't test on Macos)
- depth_bias:
```rust
println!("{}",f32::INFINITY as u64 as i32); // Prints: -1 (old implementation)
println!("{}",f32::INFINITY as i32 as u64 as i32); // Prints: 2147483647 (expected, new implementation)
```
Also ran a modified version of 3d_scene example with the following
results:
RED cube depth_bias: -1000.0
BLUE cube depth_bias: 0.0
RED cube depth_bias: -INF
BLUE cube depth_bias: 0.0
RED cube depth_bias: INF (case reported in #14169)
BLUE cube depth_bias: 0.0
(Im not completely sure whats going on with the shadows here, it seems
like depth_bias has some affect to those aswell, if this is
unintentional this issue was not introduced by this PR)
# Objective
- Contributes to #11478
- Contributes to #16877
## Solution
- Removed everything except `Instant` from `bevy_utils::time`
## Testing
- CI
---
## Migration Guide
If you relied on any of the following from `bevy_utils::time`:
- `Duration`
- `TryFromFloatSecsError`
Import these directly from `core::time` regardless of platform target
(WASM, mobile, etc.)
If you relied on any of the following from `bevy_utils::time`:
- `SystemTime`
- `SystemTimeError`
Instead import these directly from either `std::time` or `web_time` as
appropriate for your target platform.
## Notes
`Duration` and `TryFromFloatSecsError` are both re-exports from
`core::time` regardless of whether they are used from `web_time` or
`std::time`, so there is no value gained from re-exporting them from
`bevy_utils::time` as well. As for `SystemTime` and `SystemTimeError`,
no Bevy internal crates or examples rely on these types. Since Bevy
doesn't have a `Time<Wall>` resource for interacting with wall-time (and
likely shouldn't need one), I think removing these from `bevy_utils`
entirely and waiting for a use-case to justify inclusion is a reasonable
path forward.
# Objective
We want to deny the following lints:
* `clippy::allow_attributes` - Because there's no reason to
`#[allow(...)]` an attribute if it wouldn't lint against anything; you
should always use `#[expect(...)]`
* `clippy::allow_attributes_without_reason` - Because documenting the
reason for allowing/expecting a lint is always good
## Solution
Set the `clippy::allow_attributes` and
`clippy::allow_attributes_without_reason` lints to `deny`, and bring
`bevy_reflect` in line with the new restrictions.
No code changes have been made - except if a lint that was previously
`allow(...)`'d could be removed via small code changes. For example,
`unused_variables` can be handled by adding a `_` to the beginning of a
field's name.
## Testing
I ran `cargo clippy`, and received no errors.
# 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.
# Objective
- Resolve several warnings encountered when compiling for `no_std`
around `dead_code`
- Fix compatibility with `wasm32-unknown-unknown` when using `no_std`
(identified by Sachymetsu on
[Discord](https://discord.com/channels/691052431525675048/692572690833473578/1323365426901549097))
## Solution
- Removed some unused imports
- Added `allow(dead_code)` for certain private items when compiling on
`no_std`
- Fixed `bevy_app` and `bevy_tasks` compatibility with WASM when
compiling without `std` by appropriately importing `Box` and feature
gating panic unwinding
## Testing
- CI
# Objective
- Contributes to #15460
## Solution
- Added the following features:
- `std` (default)
## Testing
- CI
## Notes
- There was a minor issue with `bevy_reflect`'s `smallvec` feature
noticed in this PR which I have also resolved here. I can split this out
if desired, but I've left it here for now as it's a very small change
and I don't consider this PR itself to be very controversial.
# Objective
Fixes#16776
## Solution
- reflect `&'static Location` as an opaque type
- I've added this to `impls/std.rs` because other core types are there
too. Maybe they should be split out into a `core.rs` in another PR.
- add source location to `EventId` (behind the
`tracking_change_detection` feature flag)
## Testing
---
## Showcase
```rust
fn apply_damage_to_health(
mut dmg_events: EventReader<DealDamage>,
) {
for (event, event_id) in dmg_events.read_with_id() {
info!(
"Applying {} damage, triggered by {}",
event.amount, event_id.caller
);
…
```
```
2024-12-12T01:21:50.126827Z INFO event: Applying 9 damage, triggered by examples/ecs/event.rs:47:16
```
## Migration Guide
- If you manually construct a `SendEvent`, use `SendEvent::new()`
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Updating dependencies; adopted version of #15696. (Supercedes #15696.)
Long answer: hashbrown is no longer using ahash by default, meaning that
we can't use the default-hasher methods with ahasher. So, we have to use
the longer-winded versions instead. This takes the opportunity to also
switch our default hasher as well, but without actually enabling the
default-hasher feature for hashbrown, meaning that we'll be able to
change our hasher more easily at the cost of all of these method calls
being obnoxious forever.
One large change from 0.15 is that `insert_unique_unchecked` is now
`unsafe`, and for cases where unsafe code was denied at the crate level,
I replaced it with `insert`.
## Migration Guide
`bevy_utils` has updated its version of `hashbrown` to 0.15 and now
defaults to `foldhash` instead of `ahash`. This means that if you've
hard-coded your hasher to `bevy_utils::AHasher` or separately used the
`ahash` crate in your code, you may need to switch to `foldhash` to
ensure that everything works like it does in Bevy.
# 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>
# Objective
```
cargo check -p bevy_reflect
```
outputs a lot of warnings like:
```
warning: non-local `impl` definition, `impl` blocks should be written at the same level as their item
--> crates/bevy_reflect/src/impls/std.rs:223:13
|
223 | impl_type_path!($ty);
| ^-------------------
| |
| `TypePath` is not local
| move the `impl` block outside of this constant `_` and up 2 bodies
...
346 | / impl_reflect_for_atomic!(
347 | | ::core::sync::atomic::AtomicIsize,
| | --------------------------------- `AtomicIsize` is not local
348 | | ::core::sync::atomic::Ordering::SeqCst
349 | | );
| |_- in this macro invocation
|
= note: the macro `impl_type_path` defines the non-local `impl`, and may need to be changed
= note: the macro `impl_type_path` may come from an old version of the `bevy_reflect_derive` crate, try updating your dependency with `cargo update -p bevy_reflect_derive`
= note: an `impl` is never scoped, even when it is nested inside an item, as it may impact type checking outside of that item, which can be the case if neither the trait or the self type are at the same nesting level as the `impl`
= note: items in an anonymous const item (`const _: () = { ... }`) are treated as in the same scope as the anonymous const's declaration for the purpose of this lint
= note: `#[warn(non_local_definitions)]` on by default
= note: this warning originates in the macro `impl_type_path` which comes from the expansion of the macro `impl_reflect_for_atomic` (in Nightly builds, run with -Z macro-backtrace for more info)
```
## Solution
Move `impl_type_path!` into global scope. Warnings no longer pop up.
## Testing
CI
# Objective
### The Problem
Currently, the reflection deserializers give little control to users for
how a type is deserialized. The most control a user can have is to
register `ReflectDeserialize`, which will use a type's `Deserialize`
implementation.
However, there are times when a type may require slightly more control.
For example, let's say we want to make Bevy's `Mesh` easier to
deserialize via reflection (assume `Mesh` actually implemented
`Reflect`). Since we want this to be extensible, we'll make it so users
can use their own types so long as they satisfy `Into<Mesh>`. The end
result should allow users to define a RON file like:
```rust
{
"my_game::meshes::Sphere": (
radius: 2.5
)
}
```
### The Current Solution
Since we don't know the types ahead of time, we'll need to use
reflection. Luckily, we can access type information dynamically via the
type registry. Let's make a custom type data struct that users can
register on their types:
```rust
pub struct ReflectIntoMesh {
// ...
}
impl<T: FromReflect + Into<Mesh>> FromType<T> for ReflectIntoMesh {
fn from_type() -> Self {
// ...
}
}
```
Now we'll need a way to use this type data during deserialization.
Unfortunately, we can't use `Deserialize` since we need access to the
registry. This is where `DeserializeSeed` comes in handy:
```rust
pub struct MeshDeserializer<'a> {
pub registry: &'a TypeRegistry
}
impl<'a, 'de> DeserializeSeed<'de> for MeshDeserializer<'a> {
type Value = Mesh;
fn deserialize<D>(self, deserializer: D) -> Result<Self::Value, D::Error>
where
D: serde::Deserializer<'de>,
{
struct MeshVisitor<'a> {
registry: &'a TypeRegistry
}
impl<'a, 'de> Visitor<'de> for MeshVisitor<'a> {
fn expecting(&self, formatter: &mut Formatter) -> std::fmt::Result {
write!(formatter, "map containing mesh information")
}
fn visit_map<A>(self, mut map: A) -> Result<Self::Value, serde:🇩🇪:Error> where A: MapAccess<'de> {
// Parse the type name
let type_name = map.next_key::<String>()?.unwrap();
// Deserialize the value based on the type name
let registration = self.registry
.get_with_name(&type_name)
.expect("should be registered");
let value = map.next_value_seed(TypedReflectDeserializer {
registration,
registry: self.registry,
})?;
// Convert the deserialized value into a `Mesh`
let into_mesh = registration.data::<ReflectIntoMesh>().unwrap();
Ok(into_mesh.into(value))
}
}
}
}
```
### The Problem with the Current Solution
The solution above works great when all we need to do is deserialize
`Mesh` directly. But now, we want to be able to deserialize a struct
like this:
```rust
struct Fireball {
damage: f32,
mesh: Mesh,
}
```
This might look simple enough and should theoretically be no problem for
the reflection deserializer to handle, but this is where our
`MeshDeserializer` solution starts to break down.
In order to use `MeshDeserializer`, we need to have access to the
registry. The reflection deserializers have access to that, but we have
no way of borrowing it for our own deserialization since they have no
way of knowing about `MeshDeserializer`.
This means we need to implement _another_ `DeserializeSeed`— this time
for `Fireball`!
And if we decided to put `Fireball` inside another type, well now we
need one for that type as well.
As you can see, this solution does not scale well and results in a lot
of unnecessary boilerplate for the user.
## Solution
> [!note]
> This PR originally only included the addition of
`DeserializeWithRegistry`. Since then, a corresponding
`SerializeWithRegistry` trait has also been added. The reasoning and
usage is pretty much the same as the former so I didn't bother to update
the full PR description.
Created the `DeserializeWithRegistry` trait and
`ReflectDeserializeWithRegistry` type data.
The `DeserializeWithRegistry` trait works like a standard `Deserialize`
but provides access to the registry. And by registering the
`ReflectDeserializeWithRegistry` type data, the reflection deserializers
will automatically use the `DeserializeWithRegistry` implementation,
just like it does for `Deserialize`.
All we need to do is make the following changes:
```diff
#[derive(Reflect)]
+ #[reflect(DeserializeWithRegistry)]
struct Mesh {
// ...
}
- impl<'a, 'de> DeserializeSeed<'de> for MeshDeserializer<'a> {
- type Value = Mesh;
- fn deserialize<D>(self, deserializer: D) -> Result<Self::Value, D::Error>
+ impl<'de> DeserializeWithRegistry<'de> for Mesh {
+ fn deserialize<D>(deserializer: D, registry: &TypeRegistry) -> Result<Self, D::Error>
where
D: serde::Deserializer<'de>,
{
// ...
}
}
```
Now, any time the reflection deserializer comes across `Mesh`, it will
opt to use its `DeserializeWithRegistry` implementation. And this means
we no longer need to create a whole slew of `DeserializeSeed` types just
to deserialize `Mesh`.
### Why not a trait like `DeserializeSeed`?
While this would allow for anyone to define a deserializer for `Mesh`,
the problem is that it means __anyone can define a deserializer for
`Mesh`.__ This has the unfortunate consequence that users can never be
certain that their registration of `ReflectDeserializeSeed` is the one
that will actually be used.
We could consider adding something like that in the future, but I think
this PR's solution is much safer and follows the example set by
`ReflectDeserialize`.
### What if we made the `TypeRegistry` globally available?
This is one potential solution and has been discussed before (#6101).
However, that change is much more controversial and comes with its own
set of disadvantages (can't have multiple registries such as with
multiple worlds, likely some added performance cost with each access,
etc.).
### Followup Work
Once this PR is merged, we should consider merging `ReflectDeserialize`
into `DeserializeWithRegistry`. ~~There is already a blanket
implementation to make this transition generally pretty
straightforward.~~ The blanket implementations were removed for the sake
of this PR and will need to be re-added in the followup. I would propose
that we first mark `ReflectDeserialize` as deprecated, though, before we
outright remove it in a future release.
---
## Changelog
- Added the `DeserializeReflect` trait and `ReflectDeserializeReflect`
type data
- Added the `SerializeReflect` trait and `ReflectSerializeReflect` type
data
- Added `TypedReflectDeserializer::of` convenience constructor
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: aecsocket <43144841+aecsocket@users.noreply.github.com>
# 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);
```
# 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.
# 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>
# Objective
#15349 added an `impl_reflect!` for `glam::EulerRot`. This was done by
copying and pasting the enum definition from `glam` into `bevy_reflect`
so that the macro could interpret the variants.
However, as mentioned in the description for that PR, this would need to
be updated for `glam` 0.29, as it had not been updated yet.
#15249 came and updated `glam` to 0.29, but did not change these impls.
This is understandable as failing to do so doesn't cause any compile
errors.
This PR updates the definition and aims to make this silent breakage a
little less silent.
## Solution
Firstly, I updated the definition for `EulerRot` to match the one from
`glam`.
Secondly, I added the `assert_type_match` crate, which I created
specifically to solve this problem. By using this crate, we'll get a
compile time error if `glam` ever decides to change `EulerRot` again.
In the future we can consider using it for other types with this
problem, including in other crates (I'm pretty sure `bevy_window` and/or
`bevy_winit` also copy+paste some types). I made sure to use as few
dependencies as possible so everything should already be in-tree (it's
just `quote`, `proc-macro2`, and `syn` with default features).
## Testing
No tests added. CI should pass.
---
## Migration Guide
The reflection implementation for `EulerRot` has been updated to align
with `glam` 0.29. Please update any reflection-based usages accordingly.
# Objective
Currently, the term "value" in the context of reflection is a bit
overloaded.
For one, it can be used synonymously with "data" or "variable". An
example sentence would be "this function takes a reflected value".
However, it is also used to refer to reflected types which are
`ReflectKind::Value`. These types are usually either primitives, opaque
types, or types that don't fall into any other `ReflectKind` (or perhaps
could, but don't due to some limitation/difficulty). An example sentence
would be "this function takes a reflected value type".
This makes it difficult to write good documentation or other learning
material without causing some amount of confusion to readers. Ideally,
we'd be able to move away from the `ReflectKind::Value` usage and come
up with a better term.
## Solution
This PR replaces the terminology of "value" with "opaque" across
`bevy_reflect`. This includes in documentation, type names, variant
names, and macros.
The term "opaque" was chosen because that's essentially how the type is
treated within the reflection API. In other words, its internal
structure is hidden. All we can do is work with the type itself.
### Primitives
While primitives are not technically opaque types, I think it's still
clearer to refer to them as "opaque" rather than keep the confusing
"value" terminology.
We could consider adding another concept for primitives (e.g.
`ReflectKind::Primitive`), but I'm not sure that provides a lot of
benefit right now. In most circumstances, they'll be treated just like
an opaque type. They would also likely use the same macro (or two copies
of the same macro but with different names).
## Testing
You can test locally by running:
```
cargo test --package bevy_reflect --all-features
```
---
## Migration Guide
The reflection concept of "value type" has been replaced with a clearer
"opaque type". The following renames have been made to account for this:
- `ReflectKind::Value` → `ReflectKind::Opaque`
- `ReflectRef::Value` → `ReflectRef::Opaque`
- `ReflectMut::Value` → `ReflectMut::Opaque`
- `ReflectOwned::Value` → `ReflectOwned::Opaque`
- `TypeInfo::Value` → `TypeInfo::Opaque`
- `ValueInfo` → `OpaqueInfo`
- `impl_reflect_value!` → `impl_reflect_opaque!`
- `impl_from_reflect_value!` → `impl_from_reflect_opaque!`
Additionally, declaring your own opaque types no longer uses
`#[reflect_value]`. This attribute has been replaced by
`#[reflect(opaque)]`:
```rust
// BEFORE
#[derive(Reflect)]
#[reflect_value(Default)]
struct MyOpaqueType(u32);
// AFTER
#[derive(Reflect)]
#[reflect(opaque)]
#[reflect(Default)]
struct MyOpaqueType(u32);
```
Note that the order in which `#[reflect(opaque)]` appears does not
matter.
# Objective
In order to derive `Reflect`, all of a struct's fields must implement
`FromReflect`. [As part of looking into some of the work mentioned
here](https://github.com/bevyengine/bevy/issues/13713#issuecomment-2364786694),
I noticed that `TextureFormat` doesn't implement `Reflect`, and decided
to split that into a separate PR.
## Solution
I decided that `TextureFormat` should be a `reflect_value` since,
although one variant has fields, most users will treat this as an opaque
value set explicitly. It also substantially reduces the complexity of
the implementation.
For now, this implementation isn't actually used by any crates, so, I
decided to not preemptively enable the feature on anything. But it's
technically an option, now, and more `wgpu` types can be added in the
future.
## Testing
Everything compiles okay, and I can't really see how this could be done
incorrectly given the above constraints.
# Objective
Currently, Bevy implements reflection for `glam::EulerRot` using:
```rs
impl_reflect_value!(::glam::EulerRot(Debug, Default, Deserialize, Serialize));
```
Treating it as an opaque type. However, it's useful to expose the
EulerRot enum variants directly, which I make use of from a drop down
selection box in `bevy_egui`. This PR changes this to use
`impl_reflect!`.
**Importantly**, Bevy currently uses glam 0.28.0, in which `EulerRot`
has just 6 variants. In glam 0.29.0, this is exanded to 24 variants, see
bb2ab05613.
When Bevy updates to 0.29.0, this reflect impl must also be updated to
include the new variants.
## Solution
Replaces the `impl_reflect_value!` with `impl_reflect!` and a
handwritten version of `EulerRot` with the same variants.
## Testing
Added a `tests` module to `glam.rs` to ensure that de/serialization
works. However, my main concern is making sure that the number of enum
variants matches glam's, which I'm not sure how to do using `Enum`.
Currently, Bevy restricts animation clips to animating
`Transform::translation`, `Transform::rotation`, `Transform::scale`, or
`MorphWeights`, which correspond to the properties that glTF can
animate. This is insufficient for many use cases such as animating UI,
as the UI layout systems expect to have exclusive control over UI
elements' `Transform`s and therefore the `Style` properties must be
animated instead.
This commit fixes this, allowing for `AnimationClip`s to animate
arbitrary properties. The `Keyframes` structure has been turned into a
low-level trait that can be implemented to achieve arbitrary animation
behavior. Along with `Keyframes`, this patch adds a higher-level trait,
`AnimatableProperty`, that simplifies the task of animating single
interpolable properties. Built-in `Keyframes` implementations exist for
translation, rotation, scale, and morph weights. For the most part, you
can migrate by simply changing your code from
`Keyframes::Translation(...)` to `TranslationKeyframes(...)`, and
likewise for rotation, scale, and morph weights.
An example `AnimatableProperty` implementation for the font size of a
text section follows:
#[derive(Reflect)]
struct FontSizeProperty;
impl AnimatableProperty for FontSizeProperty {
type Component = Text;
type Property = f32;
fn get_mut(component: &mut Self::Component) -> Option<&mut
Self::Property> {
Some(&mut component.sections.get_mut(0)?.style.font_size)
}
}
In order to keep this patch relatively small, this patch doesn't include
an implementation of `AnimatableProperty` on top of the reflection
system. That can be a follow-up.
This patch builds on top of the new `EntityMutExcept<>` type in order to
widen the `AnimationTarget` query to include write access to all
components. Because `EntityMutExcept<>` has some performance overhead
over an explicit query, we continue to explicitly query `Transform` in
order to avoid regressing the performance of skeletal animation, such as
the `many_foxes` benchmark. I've measured the performance of that
benchmark and have found no significant regressions.
A new example, `animated_ui`, has been added. This example shows how to
use Bevy's built-in animation infrastructure to animate font size and
color, which wasn't possible before this patch.
## Showcase
https://github.com/user-attachments/assets/1fa73492-a9ce-405a-a8f2-4aacd7f6dc97
## Migration Guide
* Animation keyframes are now an extensible trait, not an enum. Replace
`Keyframes::Translation(...)`, `Keyframes::Scale(...)`,
`Keyframes::Rotation(...)`, and `Keyframes::Weights(...)` with
`Box::new(TranslationKeyframes(...))`, `Box::new(ScaleKeyframes(...))`,
`Box::new(RotationKeyframes(...))`, and
`Box::new(MorphWeightsKeyframes(...))` respectively.
# 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>
# Objective
Fix#10284.
## Solution
When `DynamicSceneBuilder` extracts entities, they are cloned via
`PartialReflect::clone_value`, making them into dynamic versions of the
original components. This loses any custom `ReflectSerialize` type data.
Dynamic scenes are deserialized with the original types, not the dynamic
versions, and so any component with a custom serialize may fail. In this
case `Rect` and `Vec2`. The dynamic version includes the field names 'x'
and 'y' but the `Serialize` impl doesn't, hence the "expect float"
error.
The solution here: Instead of using `clone_value` to clone the
components, `FromReflect` clones and retains the original information
needed to serialize with any custom `Serialize` impls. I think using
something like `reflect_clone` from
(https://github.com/bevyengine/bevy/pull/13432) might make this more
efficient.
I also did the same when deserializing dynamic scenes to appease some of
the round-trip tests which use `ReflectPartialEq`, which requires the
types be the same and not a unique/proxy pair. I'm not sure it's
otherwise necessary. Maybe this would also be more efficient when
spawning dynamic scenes with `reflect_clone` instead of `FromReflect`
again?
An alternative solution would be to fall back to the dynamic version
when deserializing `DynamicScene`s if the custom version fails. I think
that's possible. Or maybe simply always deserializing via the dynamic
route for dynamic scenes?
## Testing
This example is similar to the original test case in #10284:
``` rust
#![allow(missing_docs)]
use bevy::{prelude::*, scene::SceneInstanceReady};
fn main() {
App::new()
.add_plugins(DefaultPlugins)
.add_systems(Startup, (save, load).chain())
.observe(check)
.run();
}
static SAVEGAME_SAVE_PATH: &str = "savegame.scn.ron";
fn save(world: &mut World) {
let entity = world.spawn(OrthographicProjection::default()).id();
let scene = DynamicSceneBuilder::from_world(world)
.extract_entity(entity)
.build();
if let Some(registry) = world.get_resource::<AppTypeRegistry>() {
let registry = registry.read();
let serialized_scene = scene.serialize(®istry).unwrap();
// println!("{}", serialized_scene);
std::fs::write(format!("assets/{SAVEGAME_SAVE_PATH}"), serialized_scene).unwrap();
}
world.entity_mut(entity).despawn_recursive();
}
fn load(mut commands: Commands, asset_server: Res<AssetServer>) {
commands.spawn(DynamicSceneBundle {
scene: asset_server.load(SAVEGAME_SAVE_PATH),
..default()
});
}
fn check(_trigger: Trigger<SceneInstanceReady>, query: Query<&OrthographicProjection>) {
dbg!(query.single());
}
```
## Migration Guide
The `DynamicScene` format is changed to use custom serialize impls so
old scene files will need updating:
Old:
```ron
(
resources: {},
entities: {
4294967299: (
components: {
"bevy_render:📷:projection::OrthographicProjection": (
near: 0.0,
far: 1000.0,
viewport_origin: (
x: 0.5,
y: 0.5,
),
scaling_mode: WindowSize(1.0),
scale: 1.0,
area: (
min: (
x: -1.0,
y: -1.0,
),
max: (
x: 1.0,
y: 1.0,
),
),
),
},
),
},
)
```
New:
```ron
(
resources: {},
entities: {
4294967299: (
components: {
"bevy_render:📷:projection::OrthographicProjection": (
near: 0.0,
far: 1000.0,
viewport_origin: (0.5, 0.5),
scaling_mode: WindowSize(1.0),
scale: 1.0,
area: (
min: (-1.0, -1.0),
max: (1.0, 1.0),
),
),
},
),
},
)
```
---------
Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
# Objective
`EntityHash` and related types were moved from `bevy_utils` to
`bevy_ecs` in #11498, but seemed to have been accidentally reintroduced
a week later in #11707.
## Solution
Remove the old leftover code.
---
## Migration Guide
- Uses of `bevy::utils::{EntityHash, EntityHasher, EntityHashMap,
EntityHashSet}` now have to be imported from `bevy::ecs::entity`.
# Objective
- Fixes#14969
## Solution
- Added `Deserialize` to the list of reflected traits for `SmolStr`
## Testing
- CI passed locally.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
- Fixes#14974
## Solution
- Replace all* instances of `NonZero*` with `NonZero<*>`
## Testing
- CI passed locally.
---
## Notes
Within the `bevy_reflect` implementations for `std` types,
`impl_reflect_value!()` will continue to use the type aliases instead,
as it inappropriately parses the concrete type parameter as a generic
argument. If the `ZeroablePrimitive` trait was stable, or the macro
could be modified to accept a finite list of types, then we could fully
migrate.
# Objective
- Fixes#14844
## Solution
- implement reflect using the `impl_reflect_value` macro
## Testing
- I wrote a test locally to understand and learn how reflection worked
on a basic level and to confirm that yes indeed the bound struct could
use the reflection traits that have been implemented for it.
note: I did remove a line that asked for bound to not have reflect
implemented in a test, since that's the point of this PR and the test
worked without the line so I am not sure what that was about, not sure
if that uncovers a deeper issue or not.
# Objective
Fixes#14782
## Solution
Enable the lint and fix all upcoming hints (`--fix`). Also tried to
figure out the false-positive (see review comment). Maybe split this PR
up into multiple parts where only the last one enables the lint, so some
can already be merged resulting in less many files touched / less
potential for merge conflicts?
Currently, there are some cases where it might be easier to read the
code with the qualifier, so perhaps remove the import of it and adapt
its cases? In the current stage it's just a plain adoption of the
suggestions in order to have a base to discuss.
## Testing
`cargo clippy` and `cargo run -p ci` are happy.
