# Objective
Now that #13432 has been merged, it's important we update our reflected
types to properly opt into this feature. If we do not, then this could
cause issues for users downstream who want to make use of
reflection-based cloning.
## Solution
This PR is broken into 4 commits:
1. Add `#[reflect(Clone)]` on all types marked `#[reflect(opaque)]` that
are also `Clone`. This is mandatory as these types would otherwise cause
the cloning operation to fail for any type that contains it at any
depth.
2. Update the reflection example to suggest adding `#[reflect(Clone)]`
on opaque types.
3. Add `#[reflect(clone)]` attributes on all fields marked
`#[reflect(ignore)]` that are also `Clone`. This prevents the ignored
field from causing the cloning operation to fail.
Note that some of the types that contain these fields are also `Clone`,
and thus can be marked `#[reflect(Clone)]`. This makes the
`#[reflect(clone)]` attribute redundant. However, I think it's safer to
keep it marked in the case that the `Clone` impl/derive is ever removed.
I'm open to removing them, though, if people disagree.
4. Finally, I added `#[reflect(Clone)]` on all types that are also
`Clone`. While not strictly necessary, it enables us to reduce the
generated output since we can just call `Clone::clone` directly instead
of calling `PartialReflect::reflect_clone` on each variant/field. It
also means we benefit from any optimizations or customizations made in
the `Clone` impl, including directly dereferencing `Copy` values and
increasing reference counters.
Along with that change I also took the liberty of adding any missing
registrations that I saw could be applied to the type as well, such as
`Default`, `PartialEq`, and `Hash`. There were hundreds of these to
edit, though, so it's possible I missed quite a few.
That last commit is **_massive_**. There were nearly 700 types to
update. So it's recommended to review the first three before moving onto
that last one.
Additionally, I can break the last commit off into its own PR or into
smaller PRs, but I figured this would be the easiest way of doing it
(and in a timely manner since I unfortunately don't have as much time as
I used to for code contributions).
## Testing
You can test locally with a `cargo check`:
```
cargo check --workspace --all-features
```
Fixes#17170
# Objective
Tangents are not currently transformed as described in #17170. I came
across this while working on #17989 and it seemed like low hanging
fruit.
# Objective
- Fixes#17960
## Solution
- Followed the [edition upgrade
guide](https://doc.rust-lang.org/edition-guide/editions/transitioning-an-existing-project-to-a-new-edition.html)
## Testing
- CI
---
## Summary of Changes
### Documentation Indentation
When using lists in documentation, proper indentation is now linted for.
This means subsequent lines within the same list item must start at the
same indentation level as the item.
```rust
/* Valid */
/// - Item 1
/// Run-on sentence.
/// - Item 2
struct Foo;
/* Invalid */
/// - Item 1
/// Run-on sentence.
/// - Item 2
struct Foo;
```
### Implicit `!` to `()` Conversion
`!` (the never return type, returned by `panic!`, etc.) no longer
implicitly converts to `()`. This is particularly painful for systems
with `todo!` or `panic!` statements, as they will no longer be functions
returning `()` (or `Result<()>`), making them invalid systems for
functions like `add_systems`. The ideal fix would be to accept functions
returning `!` (or rather, _not_ returning), but this is blocked on the
[stabilisation of the `!` type
itself](https://doc.rust-lang.org/std/primitive.never.html), which is
not done.
The "simple" fix would be to add an explicit `-> ()` to system
signatures (e.g., `|| { todo!() }` becomes `|| -> () { todo!() }`).
However, this is _also_ banned, as there is an existing lint which (IMO,
incorrectly) marks this as an unnecessary annotation.
So, the "fix" (read: workaround) is to put these kinds of `|| -> ! { ...
}` closuers into variables and give the variable an explicit type (e.g.,
`fn()`).
```rust
// Valid
let system: fn() = || todo!("Not implemented yet!");
app.add_systems(..., system);
// Invalid
app.add_systems(..., || todo!("Not implemented yet!"));
```
### Temporary Variable Lifetimes
The order in which temporary variables are dropped has changed. The
simple fix here is _usually_ to just assign temporaries to a named
variable before use.
### `gen` is a keyword
We can no longer use the name `gen` as it is reserved for a future
generator syntax. This involved replacing uses of the name `gen` with
`r#gen` (the raw-identifier syntax).
### Formatting has changed
Use statements have had the order of imports changed, causing a
substantial +/-3,000 diff when applied. For now, I have opted-out of
this change by amending `rustfmt.toml`
```toml
style_edition = "2021"
```
This preserves the original formatting for now, reducing the size of
this PR. It would be a simple followup to update this to 2024 and run
`cargo fmt`.
### New `use<>` Opt-Out Syntax
Lifetimes are now implicitly included in RPIT types. There was a handful
of instances where it needed to be added to satisfy the borrow checker,
but there may be more cases where it _should_ be added to avoid
breakages in user code.
### `MyUnitStruct { .. }` is an invalid pattern
Previously, you could match against unit structs (and unit enum
variants) with a `{ .. }` destructuring. This is no longer valid.
### Pretty much every use of `ref` and `mut` are gone
Pattern binding has changed to the point where these terms are largely
unused now. They still serve a purpose, but it is far more niche now.
### `iter::repeat(...).take(...)` is bad
New lint recommends using the more explicit `iter::repeat_n(..., ...)`
instead.
## Migration Guide
The lifetimes of functions using return-position impl-trait (RPIT) are
likely _more_ conservative than they had been previously. If you
encounter lifetime issues with such a function, please create an issue
to investigate the addition of `+ use<...>`.
## Notes
- Check the individual commits for a clearer breakdown for what
_actually_ changed.
---------
Co-authored-by: François Mockers <francois.mockers@vleue.com>
Didn't remove WgpuWrapper. Not sure if it's needed or not still.
## Testing
- Did you test these changes? If so, how? Example runner
- Are there any parts that need more testing? Web (portable atomics
thingy?), DXC.
## Migration Guide
- Bevy has upgraded to [wgpu
v24](https://github.com/gfx-rs/wgpu/blob/trunk/CHANGELOG.md#v2400-2025-01-15).
- When using the DirectX 12 rendering backend, the new priority system
for choosing a shader compiler is as follows:
- If the `WGPU_DX12_COMPILER` environment variable is set at runtime, it
is used
- Else if the new `statically-linked-dxc` feature is enabled, a custom
version of DXC will be statically linked into your app at compile time.
- Else Bevy will look in the app's working directory for
`dxcompiler.dll` and `dxil.dll` at runtime.
- Else if they are missing, Bevy will fall back to FXC (not recommended)
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: IceSentry <c.giguere42@gmail.com>
Co-authored-by: François Mockers <francois.mockers@vleue.com>
# Objective
Make `Mesh::merge` more resilient to use.
Currently, it's difficult to make sure `Mesh::merge` will not panic
(we'd have to check if all attributes are compatible).
- I'd appreciate it for utility function to convert different mesh
representations such as:
https://github.com/dimforge/bevy_rapier/pull/628.
## Solution
- Make `Mesh::merge` return a `Result`.
## Testing
- It builds
## Migration Guide
- `Mesh::merge` now returns a `Result<(), MeshMergeError>`.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Greeble <166992735+greeble-dev@users.noreply.github.com>
Co-authored-by: Benjamin Brienen <benjamin.brienen@outlook.com>
# Objective
- https://github.com/bevyengine/bevy/issues/17111
## Solution
Set the `clippy::allow_attributes` and
`clippy::allow_attributes_without_reason` lints to `deny`, and bring
`bevy_mesh` in line with the new restrictions.
## Testing
`cargo clippy --tests` and `cargo test --package bevy_mesh` were run,
and no errors were encountered.
# Objective
- Contributes to #11478
## Solution
- Made `bevy_utils::tracing` `doc(hidden)`
- Re-exported `tracing` from `bevy_log` for end-users
- Added `tracing` directly to crates that need it.
## Testing
- CI
---
## Migration Guide
If you were importing `tracing` via `bevy::utils::tracing`, instead use
`bevy::log::tracing`. Note that many items within `tracing` are also
directly re-exported from `bevy::log` as well, so you may only need
`bevy::log` for the most common items (e.g., `warn!`, `trace!`, etc.).
This also applies to the `log_once!` family of macros.
## Notes
- While this doesn't reduce the line-count in `bevy_utils`, it further
decouples the internal crates from `bevy_utils`, making its eventual
removal more feasible in the future.
- I have just imported `tracing` as we do for all dependencies. However,
a workspace dependency may be more appropriate for version management.
# Objective
- dont depend on wgpu if we dont have to
## Solution
- works towards this, but doesnt fully accomplish it. bevy_mesh depends
on bevy_image
## Testing
- 3d_scene runs
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: BD103 <59022059+BD103@users.noreply.github.com>
# Objective
Avoid a premature normalize operation and get better smooth normals for
it.
## Inspiration
@IceSentry suggested `face_normal()` could have its normalize removed
based on [this article](https://iquilezles.org/articles/normals/) in PR
#16039.
## Solution
I did not want to change `face_normal()` to return a vector that's not
normalized. The name "normal" implies it'll be normalized. Instead I
added the `face_area_normal()` function, whose result is not normalized.
Its magnitude is equal two times the triangle's area. I've noted why
this is the case in its doc comment.
I changed `compute_smooth_normals()` from computing normals from
adjacent faces with equal weight to use the area of the faces as a
weight. This has the benefit of being cheaper computationally and
hopefully produces better normals.
The `compute_flat_normals()` is unchanged and still uses
`face_normal()`.
## Testing
One test was added which shows the bigger triangle having an effect on
the normal, but the previous test that uses the same size triangles is
unchanged.
**WARNING:** No visual test has been done yet. No example exists that
demonstrates the compute_smooth_normals(). Perhaps there's a good model
to demonstrate what the differences are. I would love to have some input
on this.
I'd suggest @IceSentry and @stepancheg to review this PR.
## Further Considerations
It's possible weighting normals by their area is not definitely better
than unweighted. It's possible there may be aesthetic reasons to prefer
one over the other. In such a case, we could offer two variants:
weighted or unweighted. Or we could offer another function perhaps like
this: `compute_smooth_normals_with_weights(|normal, area| 1.0)` which
would restore the original unweighted sum of normals.
---
## Showcase
Smooth normal calculation now weights adjacent face normals by their
area.
## Migration Guide
# Objective
When merging two meshes, we need to find the offset of indices for the
second mesh. Currently it is done by inserting empty positions if
positions is not set.
Although practically it is not an issue, this does not feel right:
- We did not have positions before, then why we have positions after
merge?
- Moreover, if positions are not set, but uvs are not empty, computed
offset will be zero, while it should be equal to the number of uvs.
## Solution
Use `Mesh::count_vertices` to find the number of vertices.
## Testing
Looking hard.
# Objective
There's integer overflow in `Mesh::merge` in branches like this:
405fa3e8ea/crates/bevy_mesh/src/mesh.rs (L857-L859)
we truncate `u32` to `u16` and ignore integer overflow on `u16`. This
may lead to unexpected results when the number of vertices exceeds
`u16::MAX`.
## Solution
Convert indices storage to `u32` when necessary.
## Testing
- Unit test added for `extend` function
- For changes in `Mesh`, I presume it is already tested elsewhere
# 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
- bevy_render is gargantuan
## Solution
- Split out bevy_mesh
## Testing
- Ran some examples, everything looks fine
## Migration Guide
`bevy_render::mesh::morph::inherit_weights` is now
`bevy_render::mesh::inherit_weights`
if you were using `Mesh::compute_aabb`, you will need to `use
bevy_render::mesh::MeshAabb;` now
---------
Co-authored-by: Joona Aalto <jondolf.dev@gmail.com>
