# Objective
Fixes typos in bevy project, following suggestion in
https://github.com/bevyengine/bevy-website/pull/1912#pullrequestreview-2483499337
## Solution
I used https://github.com/crate-ci/typos to find them.
I included only the ones that feel undebatable too me, but I am not in
game engine so maybe some terms are expected.
I left out the following typos:
- `reparametrize` => `reparameterize`: There are a lot of occurences, I
believe this was expected
- `semicircles` => `hemicircles`: 2 occurences, may mean something
specific in geometry
- `invertation` => `inversion`: may mean something specific
- `unparented` => `parentless`: may mean something specific
- `metalness` => `metallicity`: may mean something specific
## Testing
- Did you test these changes? If so, how? I did not test the changes,
most changes are related to raw text. I expect the others to be tested
by the CI.
- Are there any parts that need more testing? I do not think
- How can other people (reviewers) test your changes? Is there anything
specific they need to know? To me there is nothing to test
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
---
## Migration Guide
> This section is optional. If there are no breaking changes, you can
delete this section.
(kept in case I include the `reparameterize` change here)
- If this PR is a breaking change (relative to the last release of
Bevy), describe how a user might need to migrate their code to support
these changes
- Simply adding new functionality is not a breaking change.
- Fixing behavior that was definitely a bug, rather than a questionable
design choice is not a breaking change.
## Questions
- [x] Should I include the above typos? No
(https://github.com/bevyengine/bevy/pull/16702#issuecomment-2525271152)
- [ ] Should I add `typos` to the CI? (I will check how to configure it
properly)
This project looks awesome, I really enjoy reading the progress made,
thanks to everyone involved.
# Objective
- Remove `derive_more`'s error derivation and replace it with
`thiserror`
## Solution
- Added `derive_more`'s `error` feature to `deny.toml` to prevent it
sneaking back in.
- Reverted to `thiserror` error derivation
## Notes
Merge conflicts were too numerous to revert the individual changes, so
this reversion was done manually. Please scrutinise carefully during
review.
# 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
Fixes#15941
## Solution
Created https://crates.io/crates/variadics_please and moved the code
there; updating references
`bevy_utils/macros` is deleted.
## Testing
cargo check
## Migration Guide
Use `variadics_please::{all_tuples, all_tuples_with_size}` instead of
`bevy::utils::{all_tuples, all_tuples_with_size}`.
**NOTE: This is based on, and should be merged alongside,
https://github.com/bevyengine/bevy/pull/15482.** I'll leave this in
draft until that PR is merged.
# Objective
Equivalent of https://github.com/bevyengine/bevy/pull/15482 but for
serialization. See that issue for the motivation.
Also part of this tracking issue:
https://github.com/bevyengine/bevy/issues/15518
This PR is non-breaking, just like the deserializer PR (because the new
type parameter `P` has a default `P = ()`).
## Solution
Identical solution to the deserializer PR.
## Testing
Added unit tests and a very comprehensive doc test outlining a clear
example and use case.
# Objective
MSRV in the standalone crates should be accurate
## Solution
Determine the msrv of each crate and set it
## Testing
Adding better msrv checks to the CI is a next-step.
**NOTE: Also see https://github.com/bevyengine/bevy/pull/15548 for the
serializer equivalent**
# Objective
The current `ReflectDeserializer` and `TypedReflectDeserializer` use the
`TypeRegistration` and/or `ReflectDeserialize` of a given type in order
to determine how to deserialize a value of that type. However, there is
currently no way to statefully override deserialization of a given type
when using these two deserializers - that is, to have some local data in
the same scope as the `ReflectDeserializer`, and make use of that data
when deserializing.
The motivating use case for this came up when working on
[`bevy_animation_graph`](https://github.com/aecsocket/bevy_animation_graph/tree/feat/dynamic-nodes),
when loading an animation graph asset. The `AnimationGraph` stores
`Vec<Box<dyn NodeLike>>`s which we have to load in. Those `Box<dyn
NodeLike>`s may store `Handle`s to e.g. `Handle<AnimationClip>`. I want
to trigger a `load_context.load()` for that handle when it's loaded.
```rs
#[derive(Reflect)]
struct Animation {
clips: Vec<Handle<AnimationClip>>,
}
```
```rs
(
clips: [
"animation_clips/walk.animclip.ron",
"animation_clips/run.animclip.ron",
"animation_clips/jump.animclip.ron",
],
)
````
Currently, if this were deserialized from an asset loader, this would be
deserialized as a vec of `Handle::default()`s, which isn't useful since
we also need to `load_context.load()` those handles for them to be used.
With this processor field, a processor can detect when `Handle<T>`s are
being loaded, then actually load them in.
## Solution
```rs
trait ReflectDeserializerProcessor {
fn try_deserialize<'de, D>(
&mut self,
registration: &TypeRegistration,
deserializer: D,
) -> Result<Result<Box<dyn PartialReflect>, D>, D::Error>
where
D: serde::Deserializer<'de>;
}
```
```diff
- pub struct ReflectDeserializer<'a> {
+ pub struct ReflectDeserializer<'a, P = ()> { // also for ReflectTypedDeserializer
registry: &'a TypeRegistry,
+ processor: Option<&'a mut P>,
}
```
```rs
impl<'a, P: ReflectDeserializerProcessor> ReflectDeserializer<'a, P> { // also for ReflectTypedDeserializer
pub fn with_processor(registry: &'a TypeRegistry, processor: &'a mut P) -> Self {
Self {
registry,
processor: Some(processor),
}
}
}
```
This does not touch the existing `fn new`s.
This `processor` field is also added to all internal visitor structs.
When `TypedReflectDeserializer` runs, it will first try to deserialize a
value of this type by passing the `TypeRegistration` and deserializer to
the processor, and fallback to the default logic. This processor runs
the earliest, and takes priority over all other deserialization logic.
## Testing
Added unit tests to `bevy_reflect::serde::de`. Also using almost exactly
the same implementation in [my fork of
`bevy_animation_graph`](https://github.com/aecsocket/bevy_animation_graph/tree/feat/dynamic-nodes).
## Migration Guide
(Since I added `P = ()`, I don't think this is actually a breaking
change anymore, but I'll leave this in)
`bevy_reflect`'s `ReflectDeserializer` and `TypedReflectDeserializer`
now take a `ReflectDeserializerProcessor` as the type parameter `P`,
which allows you to customize deserialization for specific types when
they are found. However, the rest of the API surface (`new`) remains the
same.
<details>
<summary>Original implementation</summary>
Add `ReflectDeserializerProcessor`:
```rs
struct ReflectDeserializerProcessor {
pub can_deserialize: Box<dyn FnMut(&TypeRegistration) -> bool + 'p>,
pub deserialize: Box<
dyn FnMut(
&TypeRegistration,
&mut dyn erased_serde::Deserializer,
) -> Result<Box<dyn PartialReflect>, erased_serde::Error>
+ 'p,
}
```
Along with `ReflectDeserializer::new_with_processor` and
`TypedReflectDeserializer::new_with_processor`. This does not touch the
public API of the existing `new` fns.
This is stored as an `Option<&mut ReflectDeserializerProcessor>` on the
deserializer and any of the private `-Visitor` structs, and when we
attempt to deserialize a value, we first pass it through this processor.
Also added a very comprehensive doc test to
`ReflectDeserializerProcessor`, which is actually a scaled down version
of the code for the `bevy_animation_graph` loader. This should give
users a good motivating example for when and why to use this feature.
### Why `Box<dyn ..>`?
When I originally implemented this, I added a type parameter to
`ReflectDeserializer` to determine the processor used, with `()` being
"no processor". However when using this, I kept running into rustc
errors where it failed to validate certain type bounds and led to
overflows. I then switched to a dynamic dispatch approach.
The dynamic dispatch should not be that expensive, nor should it be a
performance regression, since it's only used if there is `Some`
processor. (Note: I have not benchmarked this, I am just speculating.)
Also, it means that we don't infect the rest of the code with an extra
type parameter, which is nicer to maintain.
### Why the `'p` on `ReflectDeserializerProcessor<'p>`?
Without a lifetime here, the `Box`es would automatically become `Box<dyn
FnMut(..) + 'static>`. This makes them practically useless, since any
local data you would want to pass in must then be `'static`. In the
motivating example, you couldn't pass in that `&mut LoadContext` to the
function.
This means that the `'p` infects the rest of the Visitor types, but this
is acceptable IMO. This PR also elides the lifetimes in the `impl<'de>
Visitor<'de> for -Visitor` blocks where possible.
### Future possibilities
I think it's technically possible to turn the processor into a trait,
and make the deserializers generic over that trait. This would also open
the door to an API like:
```rs
type Seed;
fn seed_deserialize(&mut self, r: &TypeRegistration) -> Option<Self::Seed>;
fn deserialize(&mut self, r: &TypeRegistration, d: &mut dyn erased_serde::Deserializer, s: Self::Seed) -> ...;
```
A similar processor system should also be added to the serialization
side, but that's for another PR. Ideally, both PRs will be in the same
release, since one isn't very useful without the other.
## Testing
Added unit tests to `bevy_reflect::serde::de`. Also using almost exactly
the same implementation in [my fork of
`bevy_animation_graph`](https://github.com/aecsocket/bevy_animation_graph/tree/feat/dynamic-nodes).
## Migration Guide
`bevy_reflect`'s `ReflectDeserializer` and `TypedReflectDeserializer`
now take a second lifetime parameter `'p` for storing the
`ReflectDeserializerProcessor` field lifetimes. However, the rest of the
API surface (`new`) remains the same, so if you are not storing these
deserializers or referring to them with lifetimes, you should not have
to make any changes.
</details>
# Objective
Bevy seems to want to standardize on "American English" spellings. Not
sure if this is laid out anywhere in writing, but see also #15947.
While perusing the docs for `typos`, I noticed that it has a `locale`
config option and tried it out.
## Solution
Switch to `en-us` locale in the `typos` config and run `typos -w`
## Migration Guide
The following methods or fields have been renamed from `*dependants*` to
`*dependents*`.
- `ProcessorAssetInfo::dependants`
- `ProcessorAssetInfos::add_dependant`
- `ProcessorAssetInfos::non_existent_dependants`
- `AssetInfo::dependants_waiting_on_load`
- `AssetInfo::dependants_waiting_on_recursive_dep_load`
- `AssetInfos::loader_dependants`
- `AssetInfos::remove_dependants_and_labels`
# 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
Another clippy-lint fix: the goal is so that `ci lints` actually
displays the problems that a contributor caused, and not a bunch of
existing stuff in the repo. (when run on nightly)
## Solution
This fixes all but the `clippy::needless_lifetimes` lint, which will
result in substantially more fixes and be in other PR(s). I also
explicitly allow `non_local_definitions` since it is [not working
correctly, but will be
fixed](https://github.com/rust-lang/rust/issues/131643).
A few things were manually fixed: for example, some places had an
explicitly defined `div_ceil` function that was used, which is no longer
needed since this function is stable on unsigned integers. Also, empty
lines in doc comments were handled individually.
## Testing
I ran `cargo clippy --workspace --all-targets --all-features --fix
--allow-staged` with the `clippy::needless_lifetimes` lint marked as
`allow` in `Cargo.toml` to avoid fixing that too. It now passes with all
but the listed lint.
# Objective
- fix https://github.com/bevyengine/bevy/issues/15623
## Solution
- Checking field length of tuple struct before ser/der
## Testing
- CI should pass
## Migration Guide
- Reflection now will serialize and deserialize tuple struct with single
field as newtype struct. Consider this code.
```rs
#[derive(Reflect, Serialize)]
struct Test(usize);
let reflect = Test(3);
let serializer = TypedReflectSerializer::new(reflect.as_partial_reflect(), ®istry);
return serde_json::to_string(&serializer)
```
Old behavior will return `["3"]`. New behavior will return `"3"`. If you
were relying on old behavior you need to update your logic. Especially
with `serde_json`. `ron` doesn't affect from this.
# Objective
Relevant: #15208
## Solution
I went ahead and added the variadics documentation in all applicable
locations.
## Testing
- I built the documentation and inspected it to see whether the feature
is there.
# 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
- `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`.
# Objective
- Fixes#15447
## Solution
- Remove the `Return::Unit` variant and use a `Return::Owned` variant
holding a unit `()` type.
## Migration Guide
- Removed the `Return::Unit` variant; use `Return::unit()` instead.
---------
Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
# Objective
The "zero-sized" description was outdated and misleading.
## Solution
Changed the description to just say that it's an enumeration (an enum)
# 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
Updating ``glam`` to 0.29, ``encase`` to 0.10.
## Solution
Update the necessary ``Cargo.toml`` files.
## Testing
Ran ``cargo run -p ci`` on Windows; no issues came up.
---------
Co-authored-by: aecsocket <aecsocket@tutanota.com>
# 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
Fixes#15351
## Solution
- Created new external crate and ported over the code
## Testing
- CI
## Migration guide
Replace references to `bevy_utils::ShortName` with
`disqualified::ShortName`.
# 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`.
# Objective
There's currently no way to iterate through all the type data in a
`TypeRegistration`. While these are all type-erased, it can still be
useful to see what types (by `TypeId`) are registered for a given type.
Additionally, it might be good to have ways of dynamically working with
`TypeRegistration`.
## Solution
Added a way to iterate through all type data on a given
`TypeRegistration`. This PR also adds methods for working with type data
dynamically as well as methods for conveniently checking if a given type
data exists on the registration.
I also took this opportunity to reorganize the methods on
`TypeRegistration` as it has always bothered me haha (i.e. the
constructor not being at the top, etc.).
## Testing
You can test locally by running:
```
cargo test --package bevy_reflect
```
---
## Showcase
The type-erased type data on a `TypeRegistration` can now be iterated!
```rust
#[derive(Reflect)]
struct Foo;
#[derive(Clone)]
struct DataA(i32);
#[derive(Clone)]
struct DataB(i32);
let mut registration = TypeRegistration::of::<Foo>();
registration.insert(DataA(123));
registration.insert(DataB(456));
let mut iter = registration.iter();
let (id, data) = iter.next().unwrap();
assert_eq!(id, TypeId::of::<DataA>());
assert_eq!(data.downcast_ref::<DataA>().unwrap().0, 123);
let (id, data) = iter.next().unwrap();
assert_eq!(id, TypeId::of::<DataB>());
assert_eq!(data.downcast_ref::<DataB>().unwrap().0, 456);
assert!(iter.next().is_none());
```
