# Objective
Fixes#11964.
## Solution
Adds the `serde` feature to `bitflags` for `bevy_render`. This makes
`bevy_render` compile correctly when used alone.
---
## Changelog
- Fixed an issue where depending on `bevy_render` alone would fail to
compile.
# Objective
Improve code quality and performance
## Solution
Instead of using `plugin.downcast_ref::<T>().is_some()` in
`App::is_plugin_added`, use `plugin.is::<T>()`. Which is more performant
and cleaner.
# Objective
- Fixes#11960
- The compilation of `bevy_core_pipeline` failed with the `dds` feature
enabled
## Solution
- Enable the `dds` feature of `bevy_render` when enabling it for
`bevy_core_pipeline`
# Objective
`update_archetype_component_access` was removed from queries in #9774,
but some documentation still refers to it.
## Solution
Update the documentation. Since a bunch of these were in SAFETY comments
it would be nice if someone who knows the details better could check
that the rest of those comments are still valid.
# Objective
Right now, if you call `embedded_asset` with 2 arguments as a qualified
path it doesn't work (`bevy::asset::embedded_asset!(app, "foo.wgsl")` ->
"cannot find macro `embedded_asset` in this scope")
## Solution
Use `$crate::` in expansion for 2-arg case.
# Objective
- `#[reflect(FromWorld)]` was added to `ComponentB` somewhere between
`v0.12.1` and `v0.13.0`, but it is unnecessary
## Solution
- remove the unnecessary `FromWorld`
# Objective
- I hated having to do `Cuboid::new(1.0, 1.0, 1.0)` or
`Cuboid::from_size(Vec3::splat(1.0))` when there should be a much easier
way to do this.
## Solution
- Implemented a `from_length()` method that only takes in a single
float, and constructs a primitive of equal size in all directions.
- Ex:
```rs
// These:
Cuboid::new(1.0, 1.0, 1.0);
Cuboid::from_size(Vec3::splat(1.0));
// Are equivalent to this:
Cuboid::from_length(1.0);
```
- For the rest of the changed primitives:
```rs
Rectangle::from_length(1.0);
Plane3d::default().mesh().from_length(1.0);
```
# Objective
- revert a single-line change made to `examples/asset/asset_loading`
example between `v0.12.1` release and `v0.13.0` release which resulted
in a too-bright, washed-out rendering
## Solution
- reverted the changes made to this example between `v0.12.1` and
`v0.13.0`
# Objective
Another PR failed CI due to duplicate deps, and I noticed this one in
particular while scanning through the error messages.
I think this was missed in #11082.
## Solution
Bump `encase_derive_impl` dep in `bevy_encase_derive` to same version as
`encase` dep for `bevy_render`.
I spot-checked a few examples, and glanced at the
[changelog](<https://github.com/teoxoy/encase/blob/main/CHANGELOG.md#v070-2024-01-02>)
and I don't think there's anything to be concerned about, but I barely
know what this thing does.
# Objective
Fixes#11908
## Solution
- Remove the `naga_oil` dependency from `bevy_pbr`.
- We were doing a little dance to disable `glsl` support on not-wasm, so
incorporate that dance into `bevy_render`'s `Cargo.toml`.
They cause the number of texture bindings to overflow on those
platforms. Ultimately, we shouldn't unconditionally disable them, but
this fixes a crash blocking 0.13.
Closes#11885.
# Objective
Fixes two small quality issues:
1. With the new default ev100 exposure value, the irradiance intensity
was too low
2. The camera was rotating at a fixed speed (instead of a speed
multiplied by delta time), resulting in frame-rate dependent rotation
speed.
I did this during the prepass, but I neglected to do it during the
shadow map pass, causing a panic when directional lights with shadows
were enabled with lightmapped meshes present. This patch fixes the
issue.
Closes#11898.
# Objective
- Being able to build for WebGPU
```
error[E0061]: this function takes 1 argument but 3 arguments were supplied
--> .cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-0.19.1/src/backend/webgpu.rs:375:22
|
375 | let mut mapped = web_sys::GpuDepthStencilState::new(
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
376 | map_compare_function(desc.depth_compare),
| ---------------------------------------- unexpected argument of type `GpuCompareFunction`
377 | desc.depth_write_enabled,
| ------------------------ unexpected argument of type `bool`
|
note: associated function defined here
--> .cargo/registry/src/index.crates.io-6f17d22bba15001f/web-sys-0.3.68/src/features/gen_GpuDepthStencilState.rs:27:12
|
27 | pub fn new(format: GpuTextureFormat) -> Self {
| ^^^
help: remove the extra arguments
|
376 - map_compare_function(desc.depth_compare),
376 + map_texture_format(desc.format),
|
error[E0061]: this function takes 1 argument but 2 arguments were supplied
--> .cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-0.19.1/src/backend/webgpu.rs:1693:13
|
1693 | web_sys::GpuVertexState::new(desc.vertex.entry_point, &module.0);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -------------------------
| |
| unexpected argument of type `&str`
| help: remove the extra argument
|
note: associated function defined here
--> .cargo/registry/src/index.crates.io-6f17d22bba15001f/web-sys-0.3.68/src/features/gen_GpuVertexState.rs:27:12
|
27 | pub fn new(module: &GpuShaderModule) -> Self {
| ^^^
error[E0061]: this function takes 2 arguments but 3 arguments were supplied
--> .cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-0.19.1/src/backend/webgpu.rs:1768:17
|
1768 | web_sys::GpuFragmentState::new(frag.entry_point, &module.0, &targets);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ---------------- -------- unexpected argument of type `&js_sys::Array`
| |
| expected `&GpuShaderModule`, found `&str`
|
= note: expected reference `&GpuShaderModule`
found reference `&str`
note: associated function defined here
--> .cargo/registry/src/index.crates.io-6f17d22bba15001f/web-sys-0.3.68/src/features/gen_GpuFragmentState.rs:27:12
|
27 | pub fn new(module: &GpuShaderModule, targets: &::wasm_bindgen::JsValue) -> Self {
| ^^^
help: remove the extra argument
|
1768 - web_sys::GpuFragmentState::new(frag.entry_point, &module.0, &targets);
1768 + web_sys::GpuFragmentState::new(/* &GpuShaderModule */, &module.0);
|
error[E0061]: this function takes 1 argument but 2 arguments were supplied
--> .cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-0.19.1/src/backend/webgpu.rs:1793:13
|
1793 | web_sys::GpuProgrammableStage::new(desc.entry_point, &shader_module.0);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ------------------
| |
| unexpected argument of type `&str`
| help: remove the extra argument
|
note: associated function defined here
--> .cargo/registry/src/index.crates.io-6f17d22bba15001f/web-sys-0.3.68/src/features/gen_GpuProgrammableStage.rs:27:12
|
27 | pub fn new(module: &GpuShaderModule) -> Self {
| ^^^
error[E0599]: no method named `write_timestamp` found for struct `GpuCommandEncoder` in the current scope
--> .cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-0.19.1/src/backend/webgpu.rs:2505:14
|
2503 | / encoder_data
2504 | | .0
2505 | | .write_timestamp(&query_set_data.0, query_index);
| | -^^^^^^^^^^^^^^^ method not found in `GpuCommandEncoder`
| |_____________|
|
Some errors have detailed explanations: E0061, E0599.
For more information about an error, try `rustc --explain E0061`.
```
## Solution
- `web-sys` doesn't follow semver for the WebGPU APIs as they are
unstable. Force using a compatible version
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
When registering and preregistering asset loaders, there would be a
`warn!` if multiple asset loaders use a given extension, and an `info!`
if multiple asset loaders load the same asset type. Since both of these
situations are individually fine, it was decided that these messages
should be removed.
## Solution
Replace both of these messages with a new `warn!` that notes that if
multiple asset loaders share the same asset type _and_ share extensions,
that the loader must be specified in the `.meta` file for those assets
in order to solve the ambiguity. This is a more useful message, since it
notes when a user must take special action / consideration.
# Objective
Fixes#11846
## Solution
Add a `synchronous_pipeline_compilation ` field to `RenderPlugin`,
defaulting to `false`.
Most of the diff is whitespace.
## Changelog
Added `synchronous_pipeline_compilation ` to `RenderPlugin` for
disabling async pipeline creation.
## Migration Guide
TODO: consider combining this with the guide for #11846
`RenderPlugin` has a new `synchronous_pipeline_compilation ` property.
The default value is `false`. Set this to `true` if you want to retain
the previous synchronous behavior.
---------
Co-authored-by: JMS55 <47158642+JMS55@users.noreply.github.com>
Co-authored-by: François <mockersf@gmail.com>
This represents when the user has configured `ClearColorConfig::None` in
their application. If the clear color is `None`, we will always `Load`
instead of attempting to clear the attachment on the first call.
Fixes#11883.
# Objective
The new render graph labels do not (and cannot) implement normal
Reflect, which breaks spawning scenes with cameras (including GLTF
scenes). Likewise, the new `CameraMainTextureUsages` also does not (and
cannot) implement normal Reflect because it uses `wgpu::TextureUsages`
under the hood.
Fixes#11852
## Solution
This implements minimal "reflect value" for `CameraRenderGraph` and
`CameraMainTextureUsages` and registers the types, which satisfies our
spawn logic.
Note that this _does not_ fix scene serialization for these types, which
will require more significant changes. We will especially need to think
about how (and if) "interned labels" will fit into the scene system. For
the purposes of 0.13, I think this is the best we can do. Given that
this serialization issue is prevalent throughout Bevy atm, I'm ok with
adding a couple more to the pile. When we roll out the new scene system,
we will be forced to solve these on a case-by-case basis.
---
## Changelog
- Implement Reflect (value) for `CameraMainTextureUsages` and
`CameraRenderGraph`, and register those types.
# Objective
#10644 introduced nice "statically typed" labels that replace the old
strings. I would like to propose some changes to the names introduced:
* `SubGraph2d` -> `Core2d` and `SubGraph3d` -> `Core3d`. The names of
these graphs have been / should continue to be the "core 2d" graph not
the "sub graph 2d" graph. The crate is called `bevy_core_pipeline`, the
modules are still `core_2d` and `core_3d`, etc.
* `Labels2d` and `Labels3d`, at the very least, should not be plural to
follow naming conventions. A Label enum is not a "collection of labels",
it is a _specific_ Label. However I think `Label2d` and `Label3d` is
significantly less clear than `Node2d` and `Node3d`, so I propose those
changes here. I've done the same for `LabelsPbr` -> `NodePbr` and
`LabelsUi` -> `NodeUi`
Additionally, #10644 accidentally made one of the Camera2dBundle
constructors use the 3D graph instead of the 2D graph. I've fixed that
here.
---
## Changelog
* Renamed `SubGraph2d` -> `Core2d`, `SubGraph3d` -> `Core3d`, `Labels2d`
-> `Node2d`, `Labels3d` -> `Node3d`, `LabelsUi` -> `NodeUi`, `LabelsPbr`
-> `NodePbr`
# Objective
Provide a public replacement for `Into<MeshUniform>` trait impl which
was removed by #10231.
I made use of this in the `bevy_mod_outline` crate and will have to
duplicate this function if it's not accessible.
## Solution
Change the MeshUniform::new() method to be public.
# Objective
After adding configurable exposure, we set the default ev100 value to
`7` (indoor). This brought us out of sync with Blender's configuration
and defaults. This PR changes the default to `9.7` (bright indoor or
very overcast outdoors), as I calibrated in #11577. This feels like a
very reasonable default.
The other changes generally center around tweaking Bevy's lighting
defaults and examples to play nicely with this number, alongside a few
other tweaks and improvements.
Note that for artistic reasons I have reverted some examples, which
changed to directional lights in #11581, back to point lights.
Fixes#11577
---
## Changelog
- Changed `Exposure::ev100` from `7` to `9.7` to better match Blender
- Renamed `ExposureSettings` to `Exposure`
- `Camera3dBundle` now includes `Exposure` for discoverability
- Bumped `FULL_DAYLIGHT ` and `DIRECT_SUNLIGHT` to represent the
middle-to-top of those ranges instead of near the bottom
- Added new `AMBIENT_DAYLIGHT` constant and set that as the new
`DirectionalLight` default illuminance.
- `PointLight` and `SpotLight` now have a default `intensity` of
1,000,000 lumens. This makes them actually useful in the context of the
new "semi-outdoor" exposure and puts them in the "cinema lighting"
category instead of the "common household light" category. They are also
reasonably close to the Blender default.
- `AmbientLight` default has been bumped from `20` to `80`.
## Migration Guide
- The increased `Exposure::ev100` means that all existing 3D lighting
will need to be adjusted to match (DirectionalLights, PointLights,
SpotLights, EnvironmentMapLights, etc). Or alternatively, you can adjust
the `Exposure::ev100` on your cameras to work nicely with your current
lighting values. If you are currently relying on default intensity
values, you might need to change the intensity to achieve the same
effect. Note that in Bevy 0.12, point/spot lights had a different hard
coded ev100 value than directional lights. In Bevy 0.13, they use the
same ev100, so if you have both in your scene, the _scale_ between these
light types has changed and you will likely need to adjust one or both
of them.
# Objective
We recently got some neat new 2d shapes and the shapes are no longer
centered on the screen.
The hardcoded positions and colors are a pain to deal with when a new
shape is added.
## Solution
Delete a bunch of code and position shapes evenly. Assign colors evenly
too.
## Before
<img width="1280" alt="Screenshot 2024-02-14 at 3 17 40 PM"
src="https://github.com/bevyengine/bevy/assets/200550/cc9fd9a8-4019-4907-a50e-621cb656c20a">
## After
<img width="1280" alt="Screenshot 2024-02-14 at 3 17 24 PM"
src="https://github.com/bevyengine/bevy/assets/200550/033a3f91-d3bc-4ec8-af59-42a221f8b8e7">
---------
Co-authored-by: BD103 <59022059+BD103@users.noreply.github.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Joona Aalto <jondolf.dev@gmail.com>
# Objective
Fix https://github.com/bevyengine/bevy/issues/11577.
## Solution
Fix the examples, add a few constants to make setting light values
easier, and change the default lighting settings to be more realistic.
(Now designed for an overcast day instead of an indoor environment)
---
I did not include any example-related changes in here.
## Changelogs (not including breaking changes)
### bevy_pbr
- Added `light_consts` module (included in prelude), which contains
common lux and lumen values for lights.
- Added `AmbientLight::NONE` constant, which is an ambient light with a
brightness of 0.
- Added non-EV100 variants for `ExposureSettings`'s EV100 constants,
which allow easier construction of an `ExposureSettings` from a EV100
constant.
## Breaking changes
### bevy_pbr
The several default lighting values were changed:
- `PointLight`'s default `intensity` is now `2000.0`
- `SpotLight`'s default `intensity` is now `2000.0`
- `DirectionalLight`'s default `illuminance` is now
`light_consts::lux::OVERCAST_DAY` (`1000.`)
- `AmbientLight`'s default `brightness` is now `20.0`
# Objective
- The current implementations for `&Visibility == Visibility` and
`Visibility == &Visibility` are ambiguous, so they raise a warning for
being unconditionally recursive.
- `TaskPool`'s `LOCAL_EXECUTOR` thread local calls a `const` constructor
in a non-`const` context.
## Solution
- Make `&Visibility == Visibility` and `Visibility == &Visibility`
implementations use `Visibility == Visibility`.
- Wrap `LocalExecutor::new` in a special `const` block supported by
[`thread_local`](https://doc.rust-lang.org/stable/std/macro.thread_local.html).
---
This lints were found by running:
```shell
$ cargo clippy --workspace
```
There are a few other warnings that were more complicated, so I chose
not to include them in this PR.
<details>
<summary>Here they are...</summary>
```shell
warning: function cannot return without recursing
--> crates/bevy_utils/src/cow_arc.rs:92:5
|
92 | / fn eq(&self, other: &Self) -> bool {
93 | | self.deref().eq(other.deref())
94 | | }
| |_____^
|
note: recursive call site
--> crates/bevy_utils/src/cow_arc.rs:93:9
|
93 | self.deref().eq(other.deref())
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
warning: method `get_path` is never used
--> crates/bevy_reflect/src/serde/de.rs:26:8
|
25 | trait StructLikeInfo {
| -------------- method in this trait
26 | fn get_path(&self) -> &str;
| ^^^^^^^^
|
= note: `#[warn(dead_code)]` on by default
warning: methods `get_path` and `get_field` are never used
--> crates/bevy_reflect/src/serde/de.rs:34:8
|
33 | trait TupleLikeInfo {
| ------------- methods in this trait
34 | fn get_path(&self) -> &str;
| ^^^^^^^^
35 | fn get_field(&self, index: usize) -> Option<&UnnamedField>;
| ^^^^^^^^^
```
The other warnings are fixed by #11865.
</details>
# Objective
- There are multiple instances of `let Some(x) = ... else { None };`
throughout the project.
- Because `Option<T>` implements
[`Try`](https://doc.rust-lang.org/stable/std/ops/trait.Try.html), it can
use the question mark `?` operator.
## Solution
- Use question mark operator instead of `let Some(x) = ... else { None
}`.
---
There was another PR that did a similar thing a few weeks ago, but I
couldn't find it.
I just implemented this to record a video for the new blog post, but I
figured it would also make a good dedicated example. This also allows us
to remove a lot of code from the 2d/3d gizmo examples since it
supersedes this portion of code.
Depends on: https://github.com/bevyengine/bevy/pull/11699
# Objective
I tried using `insert_gizmo_group` to configure my physics gizmos in a
bevy_xpbd example, but was surprised to see that nothing happened. I
found out that the method does *not* overwrite gizmo groups that have
already been initialized (with `init_gizmo_group`). This is unexpected,
since methods like `insert_resource` *do* overwrite.
## Solution
Insert the configuration even if it has already been initialized.
# Objective
`RenderMeshInstance::material_bind_group_id` is only set from
`queue_material_meshes::<M>`. this field is used (only) for determining
batch groups, so some items may be batched incorrectly if they have
never been in the camera's view or if they don't use the Material
abstraction.
in particular, shadow views render more meshes than the main camera, and
currently batch some meshes where the object has never entered the
camera view together. this is quite hard to trigger, but should occur in
a scene with out-of-view alpha-mask materials (so that the material
instance actually affects the shadow) in the path of a light.
this is also a footgun for custom pipelines: failing to set the
material_bind_group_id will result in all meshes being batched together
and all using the closest/furthest material to the camera (depending on
sort order).
## Solution
- queue_shadows now sets the material_bind_group_id correctly
- `MeshPipeline` doesn't attempt to batch meshes if the
material_bind_group_id has not been set. custom pipelines still need to
set this field to take advantage of batching, but will at least render
correctly if it is not set
# Objective
sysinfo was updated to 0.30 in #11071. Ever since then the `cpu` field
of the `SystemInfo` struct that gets printed every time one starts an
bevy app has been empty. This is because the following part of the
sysinfo migration guide was overlooked:
---
### `Cpu` changes
Information like `Cpu::brand`, `Cpu::vendor_id` or `Cpu::frequency` are
not set on the "global" CPU.
---
## Solution
- Get the CPU brand information from a specific CPU instead. In this
case, just choose the first one. It's theoretically possible for
different CPUs to have different names, but in practice this doesn't
really happen I think. Even Intel's newer hybrid processors use a
uniform name for all CPUs in my experience.
- We can use this opportunity to also update our `sysinfo::System`
initialization here to only fetch the information we're interested in.
Make the renamings/changes regarding texture atlases a bit less
confusing by calling `TextureAtlasLayout` a layout, not a texture atlas.
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
It can sometimes be useful to combine several meshes into one. This
allows constructing more complex meshes out of simple primitives without
needing to use a 3D modeling program or entity hierarchies.
This could also be used internally to increase code reuse by using
existing mesh generation logic for e.g. circles and using that in
cylinder mesh generation logic to add the top and bottom of the
cylinder.
**Note**: This is *not* implementing CSGs (Constructive Solid Geometry)
or any boolean operations, as that is much more complex. This is simply
adding the mesh data of another mesh to a mesh.
## Solution
Add a `merge` method to `Mesh`. It appends the vertex attributes and
indices of `other` to `self`, resulting in a `Mesh` that is the
combination of the two.
For example, you could do this:
```rust
let mut cuboid = Mesh::from(shape::Box::default());
let mut cylinder = Mesh::from(shape::Cylinder::default());
let mut torus = Mesh::from(shape::Torus::default());
cuboid.merge(cylinder);
cuboid.merge(torus);
```
This would result in `cuboid` being a `Mesh` that also has the cylinder
mesh and torus mesh. In this case, they would just be placed on top of
each other, but by utilizing #11454 we can transform the cylinder and
torus to get a result like this:
https://github.com/bevyengine/bevy/assets/57632562/557402c6-b896-4aba-bd95-312e7d1b5238
This is just a single entity and a single mesh.
# Objective
- Fixes#11782.
## Solution
- Remove the run condition for `apply_global_wireframe_material`, since
it prevent detecting when meshes are added or the `NoWireframe` marker
component is removed from an entity. Alternatively this could be done by
using a run condition like "added `Handle<Mesh>` or removed
`NoWireframe` or `WireframeConfig` changed" but this seems less clear to
me than directly letting the queries on
`apply_global_wireframe_material` do the filtering.
# Objective
-
[`crossbeam::scope`](https://docs.rs/crossbeam/latest/crossbeam/fn.scope.html)
is soft-deprecated in favor of the standard library's implementation.
## Solution
- Replace reference in `TaskPool`'s docs to mention `std:🧵:scope`
instead.
# Objective
- Fixes#11695
## Solution
- Added `delta: Option<Vec2>` to `bevy_window::CursorMoved`. `delta` is
an `Option` because the `CursorMoved` event does get fired when the
cursor was outside the window area in the last frame. In that case there
is no cursor position from the last frame to compare with the current
cursor position.
---
## Changelog
- Added `delta: Option<Vec2>` to `bevy_window::CursorMoved`.
## Migration Guide
- You need to add `delta` to any manually created `CursorMoved` struct.
---------
Co-authored-by: Kanabenki <lucien.menassol@gmail.com>
Co-authored-by: James Liu <contact@jamessliu.com>
# Objective
- Fixes#11638
- See
[here](https://github.com/bevyengine/bevy/issues/11638#issuecomment-1920508465)
for details on the cause of this issue.
## Solution
- Modified `AssetLoaders` to capture possibility of multiple
`AssetLoader` registrations operating on the same `Asset` type, but
different extensions.
- Added an algorithm which will attempt to resolve via `AssetLoader`
name, then `Asset` type, then by extension. If at any point multiple
loaders fit a particular criteria, the next criteria is used as a tie
breaker.
# Objective
At the start of every schedule run, there's currently a guaranteed piece
of overhead as the async executor spawns the MultithreadeExecutor task
onto one of the ComputeTaskPool threads.
## Solution
Poll the executor once to immediately schedule systems without waiting
for the async executor, then spawn the task if and only if the executor
does not immediately terminate.
On a similar note, having the executor task immediately start executing
a system in the same async task might yield similar results over a
broader set of cases. However, this might be more involved, and may need
a solution like #8304.
# Objective
When applying a command, we currently use double indirection for the
world reference `&mut Option<&mut World>`. Since this is used across a
`fn` pointer boundary, this can't get optimized away.
## Solution
Reborrow the world reference and pass `Option<&mut World>` instead.
# Objective
Scheduling low cost systems has significant overhead due to task pool
contention and the extra machinery to schedule and run them. Following
the example of #7728, `asset_events` is good example of this kind of
system, where there is no work to be done when there are no queued asset
events.
## Solution
Put a run condition on it that checks if there are any queued events.
## Performance
Tested against `many_foxes`, we can see a slight improvement in the
total time spent in `UpdateAssets`. Also noted much less volatility due
to not being at the whim of the OS thread scheduler.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
The code in `bevy_reflect_derive` could use some cleanup.
## Solution
Took some of the changes in #11659 to create a dedicated PR for cleaning
up the field and container attribute logic.
#### Updated Naming
I renamed `ReflectTraits` and `ReflectFieldAttr` to
`ContainerAttributes` and `FieldAttributes`, respectively. I think these
are clearer.
#### Updated Parsing
##### Readability
The parsing logic wasn't too bad before, but it was getting difficult to
read. There was some duplicated logic between `Meta::List` and
`Meta::Path` attributes. Additionally, all the logic was kept inside a
large method.
To simply things, I replaced the nested meta parsing with `ParseStream`
parsing. In my opinion, this is easier to follow since it breaks up the
large match statement into a small set of single-line if statements,
where each if-block contains a single call to the appropriate attribute
parsing method.
##### Flexibility
On top of the added simplicity, this also makes our attribute parsing
much more flexible. It allows us to more elegantly handle custom where
clauses (i.e. `#[reflect(where T: Foo)]`) and it opens the door for more
non-standard attribute syntax (e.g. #11659).
##### Errors
This also allows us to automatically provide certain errors when
parsing. For example, since we can use `stream.lookahead1()`, we get
errors like the following for free:
```
error: expected one of: `ignore`, `skip_serializing`, `default`
--> crates/bevy_reflect/src/lib.rs:1988:23
|
1988 | #[reflect(foo)]
| ^^^
```
---
## Changelog
> [!note]
> All changes are internal to `bevy_reflect_derive` and should not
affect the public API[^1].
- Renamed `ReflectTraits` to `ContainerAttributes`
- Renamed `ReflectMeta::traits` to `ReflectMeta::attrs`
- Renamed `ReflectFieldAttr` to `FieldAttributes`
- Updated parsing logic for field/container attribute parsing
- Now uses a `ParseStream` directly instead of nested meta parsing
- General code cleanup of the field/container attribute modules for
`bevy_reflect_derive`
[^1]: Does not include errors, which may look slightly different.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
