## Objective
Add documentation useful to users of `bevy_ecs` not also using `App`.
Fixes#19270.
## Solution
* Add explanation of labels to `Schedule` documentation.
* Add example of `derive(ScheduleLabel)` to `trait ScheduleLabel`.
* Add a third example to `Schedule` which demonstrates using a schedule
via label instead of owning it directly.
* Add further explanation and links to `World::add_schedule()`, and
`World::run_schedule()`.
## Testing
Reviewed generated documentation.
Please review this documentation carefully for correctness, as I have
little experience with `bevy_ecs` and I am adding this information
because it would have helped my own past confusion, but I may still be
wrong about how things should be done.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: theotherphil <phil.j.ellison@gmail.com>
# Objective
Fixes#19120
## Solution
Use the find and replace token feature in VSCode to replace all the
`Condition`s with `SystemCondition`s. Then look through all the
documentation with find and replace to replace all the `Condition`s
there.
## Testing
- Did you test these changes? If so, how?
Yes, used cargo clippy, cargo build and cargo test.
- Are there any parts that need more testing?
Nope
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
By compiling and running bevy
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
Shouldn't be, but Fedora Linux with KDE Wayland
# Objective
[see original
comment](https://github.com/bevyengine/bevy/pull/18801#issuecomment-2796981745)
> Alternately, could we store it on the World instead of a global? I
think we have a World nearby whenever we call default_error_handler().
That would avoid the need for atomics or locks, since we could do
ordinary reads and writes to the World.
Global error handlers don't actually need to be global – per world is
enough. This allows using different handlers for different worlds and
also removes the restrictions on changing the handler only once.
## Solution
Each `World` can now store its own error handler in a resource.
For convenience, you can also set the default error handler for an
`App`, which applies it to the worlds of all `SubApp`s. The old behavior
of only being able to set the error handler once is kept for apps.
We also don't need the `configurable_error_handler` feature anymore now.
## Testing
New/adjusted tests for failing schedule systems & observers.
---
## Showcase
```rust
App::new()
.set_error_handler(info)
…
```
`bevy_ecs` was meant to have the `States` and `SubStates`
`proc_macro_derive`s removed when the separate `bevy_state` [was
created](https://github.com/bevyengine/bevy/issues/13216) but they were
missed.
# Objective
Fixes#19130
## Solution
Fully quality `Result::Ok` so as to not accidentally invoke the anyhow
function of the same name
## Testing
Tested on this minimal repro with and without change.
main.rs
```rs
use anyhow::Ok;
use bevy::ecs::system::SystemParam;
#[derive(SystemParam)]
pub struct SomeParams;
fn main() {
}
```
Cargo.toml
```toml
[package]
name = "bevy-playground"
version = "0.1.0"
edition = "2024"
[dependencies]
anyhow = "1.0.98"
bevy = { path = "../bevy" }
```
# Objective
resolves#19092
## Solution
- remove the `.saturating_sub` from the index transformation
- add `.saturating_add` to the internal offset calculation
## Testing
- added regression test, confirming 0 index order + testing max bound
# Objective
In my own project I was encountering the issue to find out which
entities were spawned after applying commands. I began maintaining a
vector of all entities with generational information before and after
applying the command and diffing it. This was awfully complicated though
and has no constant complexity but grows with the number of entities.
## Solution
Looking at `EntyMeta` it seemed obvious to me that struct can track the
tick just as it does with `MaybeLocation`, updated from the same call.
After that it became almost a given to also introduce query data
`SpawnDetails` which offers methods to get the spawn tick and location,
and query filter `Spawned` that filters entities out that were not
spawned since the last run.
## Testing
I expanded a few tests and added new ones, though maybe I forgot a group
of tests that should be extended too. I basically searched `bevy_ecs`
for mentions of `Changed` and `Added` to see where the tests and docs
are.
Benchmarks of spawn/despawn can be found
[here](https://github.com/bevyengine/bevy/pull/19047#issuecomment-2852181374).
---
## Showcase
From the added docs, systems with equal complexity since the filter is
not archetypal:
```rs
fn system1(q: Query<Entity, Spawned>) {
for entity in &q { /* entity spawned */ }
}
fn system2(query: Query<(Entity, SpawnDetails)>) {
for (entity, spawned) in &query {
if spawned.is_spawned() { /* entity spawned */ }
}
}
```
`SpawnedDetails` has a few more methods:
```rs
fn print_spawn_details(query: Query<(Entity, SpawnDetails)>) {
for (entity, spawn_details) in &query {
if spawn_details.is_spawned() {
print!("new ");
}
println!(
"entity {:?} spawned at {:?} by {:?}",
entity,
spawn_details.spawned_at(),
spawn_details.spawned_by()
);
}
}
```
## Changes
No public api was changed, I only added to it. That is why I added no
migration guide.
- query data `SpawnDetails`
- query filter `Spawned`
- method `Entities::entity_get_spawned_or_despawned_at`
- method `EntityRef::spawned_at`
- method `EntityMut::spawned_at`
- method `EntityWorldMut::spawned_at`
- method `UnsafeEntityCell::spawned_at`
- method `FilteredEntityRef::spawned_at`
- method `FilteredEntityMut::spawned_at`
- method `EntityRefExcept::spawned_at`
- method `EntityMutExcept::spawned_at`
---------
Co-authored-by: Eagster <79881080+ElliottjPierce@users.noreply.github.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
This is a followup to #18704 . There's lots more followup work, but this
is the minimum to unblock #18670, etc.
This direction has been given the green light by Alice
[here](https://github.com/bevyengine/bevy/pull/18704#issuecomment-2853368129).
## Solution
I could have split this over multiple PRs, but I figured skipping
straight here would be easiest for everyone and would unblock things the
quickest.
This removes the now no longer needed `identifier` module and makes
`Entity::generation` go from `NonZeroU32` to `struct
EntityGeneration(u32)`.
## Testing
CI
---------
Co-authored-by: Mark Nokalt <marknokalt@live.com>
# Objective
There are two problems this aims to solve.
First, `Entity::index` is currently a `u32`. That means there are
`u32::MAX + 1` possible entities. Not only is that awkward, but it also
make `Entity` allocation more difficult. I discovered this while working
on remote entity reservation, but even on main, `Entities` doesn't
handle the `u32::MAX + 1` entity very well. It can not be batch reserved
because that iterator uses exclusive ranges, which has a maximum upper
bound of `u32::MAX - 1`. In other words, having `u32::MAX` as a valid
index can be thought of as a bug right now. We either need to make that
invalid (this PR), which makes Entity allocation cleaner and makes
remote reservation easier (because the length only needs to be u32
instead of u64, which, in atomics is a big deal), or we need to take
another pass at `Entities` to make it handle the `u32::MAX` index
properly.
Second, `TableRow`, `ArchetypeRow` and `EntityIndex` (a type alias for
u32) all have `u32` as the underlying type. That means using these as
the index type in a `SparseSet` uses 64 bits for the sparse list because
it stores `Option<IndexType>`. By using `NonMaxU32` here, we cut the
memory of that list in half. To my knowledge, `EntityIndex` is the only
thing that would really benefit from this niche. `TableRow` and
`ArchetypeRow` I think are not stored in an `Option` in bulk. But if
they ever are, this would help. Additionally this ensures
`TableRow::INVALID` and `ArchetypeRow::INVALID` never conflict with an
actual row, which in a nice bonus.
As a related note, if we do components as entities where `ComponentId`
becomes `Entity`, the the `SparseSet<ComponentId>` will see a similar
memory improvement too.
## Solution
Create a new type `EntityRow` that wraps `NonMaxU32`, similar to
`TableRow` and `ArchetypeRow`.
Change `Entity::index` to this type.
## Downsides
`NonMax` is implemented as a `NonZero` with a binary inversion. That
means accessing and storing the value takes one more instruction. I
don't think that's a big deal, but it's worth mentioning.
As a consequence, `to_bits` uses `transmute` to skip the inversion which
keeps it a nop. But that also means that ordering has now flipped. In
other words, higher indices are considered less than lower indices. I
don't think that's a problem, but it's also worth mentioning.
## Alternatives
We could keep the index as a u32 type and just document that `u32::MAX`
is invalid, modifying `Entities` to ensure it never gets handed out.
(But that's not enforced by the type system.) We could still take
advantage of the niche here in `ComponentSparseSet`. We'd just need some
unsafe manual conversions, which is probably fine, but opens up the
possibility for correctness problems later.
We could change `Entities` to fully support the `u32::MAX` index. (But
that makes `Entities` more complex and potentially slightly slower.)
## Testing
- CI
- A few tests were changed because they depend on different ordering and
`to_bits` values.
## Future Work
- It might be worth removing the niche on `Entity::generation` since
there is now a different niche.
- We could move `Entity::generation` into it's own type too for clarity.
- We should change `ComponentSparseSet` to take advantage of the new
niche. (This PR doesn't change that yet.)
- Consider removing or updating `Identifier`. This is only used for
`Entity`, so it might be worth combining since `Entity` is now more
unique.
---------
Co-authored-by: atlv <email@atlasdostal.com>
Co-authored-by: Zachary Harrold <zac@harrold.com.au>
# Objective
Provide a generic `impl SystemParam for Option<P>` that uses system
parameter validation. This immediately gives useful impls for params
like `EventReader` and `GizmosState` that are defined in terms of `Res`.
It also allows third-party system parameters to be usable with `Option`,
which was previously impossible due to orphan rules.
Note that this is a behavior change for `Option<Single>`. It currently
fails validation if there are multiple matching entities, but with this
change it will pass validation and produce `None`.
Also provide an impl for `Result<P, SystemParamValidationError>`. This
allows systems to inspect the error if necessary, either for bubbling it
up or for checking the `skipped` flag.
Fixes#12634Fixes#14949
Related to #18516
## Solution
Add generic `SystemParam` impls for `Option` and `Result`, and remove
the impls for specific types.
Update documentation and `fallible_params` example with the new
semantics for `Option<Single>`.
# Objective
- Fixes a subset of https://github.com/bevyengine/bevy/issues/13735 by
making `EntityRef`, `EntityMut` + similar WorldQueries use the system's
change ticks when being created from within a system.
In particular, this means that `entity_ref.get_ref::<T>()` will use the
correct change ticks (the ones from the system), which matches the
behaviour of querying for `Ref<T>` directly in the system parameters.
## Solution
- Implements the solution described by
https://github.com/bevyengine/bevy/issues/13735#issuecomment-2652482918
which is to add change ticks to the `UnsafeEntityCell`
## Testing
- Added a unit test that is close to what users would encounter: before
this PR the `Added`/`Changed` filters on `Ref`s created from `EntityRef`
are incorrect.
# 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
In #18301, `NonSendMarker` was defined in such a way that it actually
implements `Send`. This isn't strictly a soundness issue, as its goal is
to be used as a `SystemParam`, and it _does_ appropriately mark system
access as `!Send`. It just seems odd that `NonSendMarker: Send`.
## Solution
- Made `NonSendMarker` wrap `PhantomData<*mut ()>`, which forces it to
be `!Send`.
## Testing
- CI
---
## Notes
This does mean constructing a `NonSendMarker` _value_ will require using
the `SystemParam` trait, but I think that's acceptable as the marker as
a value should be rarely required if at all.
# Objective
Fixes a part of #14274.
Bevy has an incredibly inconsistent naming convention for its system
sets, both internally and across the ecosystem.
<img alt="System sets in Bevy"
src="https://github.com/user-attachments/assets/d16e2027-793f-4ba4-9cc9-e780b14a5a1b"
width="450" />
*Names of public system set types in Bevy*
Most Bevy types use a naming of `FooSystem` or just `Foo`, but there are
also a few `FooSystems` and `FooSet` types. In ecosystem crates on the
other hand, `FooSet` is perhaps the most commonly used name in general.
Conventions being so wildly inconsistent can make it harder for users to
pick names for their own types, to search for system sets on docs.rs, or
to even discern which types *are* system sets.
To reign in the inconsistency a bit and help unify the ecosystem, it
would be good to establish a common recommended naming convention for
system sets in Bevy itself, similar to how plugins are commonly suffixed
with `Plugin` (ex: `TimePlugin`). By adopting a consistent naming
convention in first-party Bevy, we can softly nudge ecosystem crates to
follow suit (for types where it makes sense to do so).
Choosing a naming convention is also relevant now, as the [`bevy_cli`
recently adopted
lints](https://github.com/TheBevyFlock/bevy_cli/pull/345) to enforce
naming for plugins and system sets, and the recommended naming used for
system sets is still a bit open.
## Which Name To Use?
Now the contentious part: what naming convention should we actually
adopt?
This was discussed on the Bevy Discord at the end of last year, starting
[here](<https://discord.com/channels/691052431525675048/692572690833473578/1310659954683936789>).
`FooSet` and `FooSystems` were the clear favorites, with `FooSet` very
narrowly winning an unofficial poll. However, it seems to me like the
consensus was broadly moving towards `FooSystems` at the end and after
the poll, with Cart
([source](https://discord.com/channels/691052431525675048/692572690833473578/1311140204974706708))
and later Alice
([source](https://discord.com/channels/691052431525675048/692572690833473578/1311092530732859533))
and also me being in favor of it.
Let's do a quick pros and cons list! Of course these are just what I
thought of, so take it with a grain of salt.
`FooSet`:
- Pro: Nice and short!
- Pro: Used by many ecosystem crates.
- Pro: The `Set` suffix comes directly from the trait name `SystemSet`.
- Pro: Pairs nicely with existing APIs like `in_set` and
`configure_sets`.
- Con: `Set` by itself doesn't actually indicate that it's related to
systems *at all*, apart from the implemented trait. A set of what?
- Con: Is `FooSet` a set of `Foo`s or a system set related to `Foo`? Ex:
`ContactSet`, `MeshSet`, `EnemySet`...
`FooSystems`:
- Pro: Very clearly indicates that the type represents a collection of
systems. The actual core concept, system(s), is in the name.
- Pro: Parallels nicely with `FooPlugins` for plugin groups.
- Pro: Low risk of conflicts with other names or misunderstandings about
what the type is.
- Pro: In most cases, reads *very* nicely and clearly. Ex:
`PhysicsSystems` and `AnimationSystems` as opposed to `PhysicsSet` and
`AnimationSet`.
- Pro: Easy to search for on docs.rs.
- Con: Usually results in longer names.
- Con: Not yet as widely used.
Really the big problem with `FooSet` is that it doesn't actually
describe what it is. It describes what *kind of thing* it is (a set of
something), but not *what it is a set of*, unless you know the type or
check its docs or implemented traits. `FooSystems` on the other hand is
much more self-descriptive in this regard, at the cost of being a bit
longer to type.
Ultimately, in some ways it comes down to preference and how you think
of system sets. Personally, I was originally in favor of `FooSet`, but
have been increasingly on the side of `FooSystems`, especially after
seeing what the new names would actually look like in Avian and now
Bevy. I prefer it because it usually reads better, is much more clearly
related to groups of systems than `FooSet`, and overall *feels* more
correct and natural to me in the long term.
For these reasons, and because Alice and Cart also seemed to share a
preference for it when it was previously being discussed, I propose that
we adopt a `FooSystems` naming convention where applicable.
## Solution
Rename Bevy's system set types to use a consistent `FooSet` naming where
applicable.
- `AccessibilitySystem` → `AccessibilitySystems`
- `GizmoRenderSystem` → `GizmoRenderSystems`
- `PickSet` → `PickingSystems`
- `RunFixedMainLoopSystem` → `RunFixedMainLoopSystems`
- `TransformSystem` → `TransformSystems`
- `RemoteSet` → `RemoteSystems`
- `RenderSet` → `RenderSystems`
- `SpriteSystem` → `SpriteSystems`
- `StateTransitionSteps` → `StateTransitionSystems`
- `RenderUiSystem` → `RenderUiSystems`
- `UiSystem` → `UiSystems`
- `Animation` → `AnimationSystems`
- `AssetEvents` → `AssetEventSystems`
- `TrackAssets` → `AssetTrackingSystems`
- `UpdateGizmoMeshes` → `GizmoMeshSystems`
- `InputSystem` → `InputSystems`
- `InputFocusSet` → `InputFocusSystems`
- `ExtractMaterialsSet` → `MaterialExtractionSystems`
- `ExtractMeshesSet` → `MeshExtractionSystems`
- `RumbleSystem` → `RumbleSystems`
- `CameraUpdateSystem` → `CameraUpdateSystems`
- `ExtractAssetsSet` → `AssetExtractionSystems`
- `Update2dText` → `Text2dUpdateSystems`
- `TimeSystem` → `TimeSystems`
- `AudioPlaySet` → `AudioPlaybackSystems`
- `SendEvents` → `EventSenderSystems`
- `EventUpdates` → `EventUpdateSystems`
A lot of the names got slightly longer, but they are also a lot more
consistent, and in my opinion the majority of them read much better. For
a few of the names I took the liberty of rewording things a bit;
definitely open to any further naming improvements.
There are still also cases where the `FooSystems` naming doesn't really
make sense, and those I left alone. This primarily includes system sets
like `Interned<dyn SystemSet>`, `EnterSchedules<S>`, `ExitSchedules<S>`,
or `TransitionSchedules<S>`, where the type has some special purpose and
semantics.
## Todo
- [x] Should I keep all the old names as deprecated type aliases? I can
do this, but to avoid wasting work I'd prefer to first reach consensus
on whether these renames are even desired.
- [x] Migration guide
- [x] Release notes
# Objective
Originally [provided as a solution to a user's problem in
Discord](https://discord.com/channels/691052431525675048/1247654592838111302/1344431131277394042),
library authors might find the need to present user-registered systems
with system-specific data. Typically `Local<T>` is used for this type of
thing, but its not generally feasible or possible to configure/set the
underlying `T` data for locals. Alternatively, we can use `SystemInput`
to pass the data.
## Solution
- Added `IntoSystem::with_input`: Allows system-specific data to be
passed in explicitly.
- Added `IntoSystem::with_input_from`: Allows system-specific data to be
created at initialization time via `FromWorld`.
## Testing
Added two new tests, testing each of `with_input` and `with_input_from`.
# Objective
With the current `MapEntities` `impl`s, it is not possible to derive
things like this:
```rust
#[derive(Component)]
pub struct Inventory {
#[entities]
slots: Vec<Option<Entity>>,
}
```
This is because `MapEntities` is only implemented for `Vec<Entity>` &
`Option<Entity>`, and not arbitrary combinations of those.
It would be nice to also support those types.
## Solution
I replaced the `impl`s of the following types
- `Option<Entity>`: replaced with `Option<T>`
- `Vec<Entity>`: replaced with `Vec<T>`
- `HashSet<Entity, S>`: replaced with `HashSet<T, S>`
- `T` also had to be `Eq + core:#️⃣:Hash` here. **Not sure if this is
too restrictive?**
- `IndexSet<Entity, S>`: replaced with `IndexSet <T, S>`
- `T` also had to be `Eq + core:#️⃣:Hash` here. **Not sure if this is
too restrictive?**
- `BTreeSet<Entity>`: replaced with `BTreeSet<T>`
- `VecDeque<Entity>`: replaced with `VecDeque<T>`
- `SmallVec<A: smallvec::Array<Item = Entity>>`: replaced with
`SmallVec<A: smallvec::Array<Item = T>>`
(in all of the above, `T` is a generic type that implements
`MapEntities` (`Entity` being one of them).)
## Testing
I did not test any of this, but extended the `Component::map_entities`
doctest with an example usage of the newly supported types.
---
## Showcase
With these changes, this is now possible:
```rust
#[derive(Component)]
pub struct Inventory {
#[entities]
slots: Vec<Option<Entity>>,
}
```
# Objective
`RelatedSpawnerCommands` offers methods to get the underlying
`Commands`.
`RelatedSpawner` does not expose the inner `World` reference so far.
I currently want to write extension traits for both of them but I need
to duplicate the whole API for the latter because I cannot get it's
`&mut World`.
## Solution
Add methods for immutable and mutable `World` access
# Objective
Currently, `bevy_ecs`'s `children!` macro only supports spawning up to
twelve children at once. Ideally there would be no limit.
## Solution
`children!` is limited because `SpawnableList`, [the primary trait bound
here](https://docs.rs/bevy/0.16.0-rc.5/bevy/ecs/hierarchy/struct.Children.html#method.spawn),
uses the fake variadics pattern on tuples of up to twelve elements.
However, since a tuple itself implements `SpawnableList`, we can simply
nest tuples of entities when we run out of room.
This PR achieves this using `macro_rules` macros with a bit of brute
force, following [some discussion on
Discord](https://discord.com/channels/691052431525675048/692572690833473578/1362174415458013314).
If we create patterns for lists of up to eleven bundles, then use a
repetition pattern to handle the rest, we can "special-case" the
recursion into a nested tuple.
In principle, this would permit an arbitrary number of children, but
Rust's recursion limits will cut things short at around 1400 elements by
default. Of course, it's generally not a good idea to stick that many
bundles in a single invocation, but it might be worth mentioning in the
docs.
## Implementation notes
### Why are cases 0-11 expanded by hand?
We could make use of a tertiary macro:
```rs
macro_rules! recursive_spawn {
// so that this...
($a:expr, $b:expr) => {
(
$crate::spawn::Spawn($a),
$crate::spawn::Spawn($b),
)
};
// becomes this...
($a:expr, $b:expr) => {
$crate::spawn_tuple!($a, $b)
};
}
```
But I already feel a little bad exporting `recursive_spawn`. I'd really
like to avoid exposing more internals, even if they are annotated with
`#[doc(hidden)]`. If I had to guess, I'd say it'll also make the
expansion a tiny bit slower.
### Do we really need to handle up to twelve elements in the macro?
The macro is a little long, but doing it this way maximizes the
"flatness" of the types to be spawned. This should improve the codegen a
bit and makes the macro output a little bit easier to look at.
## Future work
The `related!` macro is essentially the same as `children!`, so if this
direction is accepted, `related!` should receive the same treatment. I
imagine we'd want to extract out the `recursive_spawn` macro into its
own file since it can be used for both. If this should be tackled in
this PR, let me know!
## Testing
This change is fairly trivial, but I added a single test to verify that
it compiles and nothing goes wrong once recursion starts happening. It's
pretty easy to verify that the change works in practice -- just spawn
over twelve entities as children at once!
# Objective
Contributes to #18741 and #18453.
## Solution
Deprecate `SimpleExecutor`. If users run into migration issues, we can
backtrack. Otherwise, we follow this up with #18741
We can't easily deprecate the module too because of
[this](https://github.com/rust-lang/rust/issues/47238).
## Testing
CI
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Cyrill Schenkel <cyrill.schenkel@gmail.com>
# Objective
- bevy removed `Observe` type parameters in #15151 ,it enables merging
`Observer` and `ObserverState ` into a single component. with this
consolidation ,we can improve efficiency while reducing boilerplate.
## Solution
- remove `ObserverState `and merge it into `Observer`
## Testing
40%~60% performance win due to removal of redundant look up.
This also improves ergonomics when using dynamic observer
```rust
// previously
world.spawn(ObserverState {
// SAFETY: we registered `event_a` above and it matches the type of EventA
descriptor: unsafe { ObserverDescriptor::default().with_events(vec![event_a]) },
runner: |mut world, _trigger, _ptr, _propagate| {
world.resource_mut::<Order>().observed("event_a");
},
..Default::default()
});
// now
let observe = unsafe {
Observer::with_dynamic_runner(|mut world, _trigger, _ptr, _propagate| {
world.resource_mut::<Order>().observed("event_a");
})
.with_event(event_a)
};
world.spawn(observe);
```
# Objective
Simplify code in the `SingleThreadedExecutor` by removing a special case
for exclusive systems.
The `SingleThreadedExecutor` runs systems without immediately applying
deferred buffers. That required calling `run_unsafe()` instead of
`run()`, but that would `panic` for exclusive systems, so the code also
needed a special case for those. Following #18076 and #18406, we have a
`run_without_applying_deferred` method that has the exact behavior we
want and works on exclusive systems.
## Solution
Replace the code in `SingleThreadedExecutor` that runs systems with a
single call to `run_without_applying_deferred()`. Also add this as a
wrapper in the `__rust_begin_short_backtrace` module to preserve the
special behavior for backtraces.
# Objective
It has long been a todo item in the ecs to create a `BundleRemover`
alongside the inserter, spawner, etc.
This is an uncontroversial first step of #18514.
## Solution
Move existing code from complex helper functions to one generalized
`BundleRemover`.
## Testing
Existing tests.
# Objective
Let `FilteredEntityRef` and `FilteredEntityMut` receive access when
nested inside tuples or `#[derive(QueryData)]` types. Make sure to
exclude any access that would conflict with other subqueries!
Fixes#14349
## Solution
Replace `WorldQuery::set_access(state, access)` with a new method,
`QueryData::provide_extra_access(state, access, available_access)`, that
passes both the total available access and the currently used access.
This is called after `WorldQuery::update_component_access()`, so any
access used by ordinary subqueries will be known. `FilteredEntityRef`
and `FilteredEntityMut` can use the combination to determine how much
access they can safely take, while tuples can safely pass those
parameters directly to their subqueries.
This requires a new `Access::remove_conflicting_access()` method that
can be used to remove any access that would conflict with existing
access. Implementing this method was easier by first factoring some
common set manipulation code out of `Access::extend`. I can extract that
refactoring to a separate PR if desired.
Have `FilteredEntity(Ref|Mut)` store `Access` instead of
`FilteredAccess` because they do not need to keep track of the filter.
This was necessary in an early draft but no longer is. I left it in
because it's small and I'm touching that code anyway, but I can extract
it to a separate PR if desired.
# Objective
Fixes#17803
## Solution
- Add an `Allows<T>` `QueryFilter` that adds archetypal access for `T`
- Fix access merging to include archetypal from both sides
## Testing
- Added a case to the unit test for the application of
`DefaultQueryFilters`
# Objective
Based on and closes#18054, this PR builds on #18035 and #18147 to
remove:
- `Commands::insert_or_spawn_batch`
- `Entities::alloc_at_without_replacement`
- `Entities::alloc_at`
- `entity::AllocAtWithoutReplacement`
- `World::insert_or_spawn_batch`
- `World::insert_or_spawn_batch_with_caller`
## Testing
Just removing unused, deprecated code, so no new tests. Note that as of
writing, #18035 is still under testing and review.
## Future Work
Per
[this](https://github.com/bevyengine/bevy/issues/18054#issuecomment-2689088899)
comment on #18054, there may be additional performance improvements
possible to the entity allocator now that `alloc_at` no longer is
supported. At a glance, I don't see anything obvious to improve, but it
may be worth further investigation in the future.
---------
Co-authored-by: JaySpruce <jsprucebruce@gmail.com>
# Objective
Stop using `ArchetypeComponentId` in the executor. These IDs will grow
even more quickly with relations, and the size may start to degrade
performance.
## Solution
Have systems expose their `FilteredAccessSet<ComponentId>`, and have the
executor use that to determine which systems conflict. This can be
determined statically, so determine all conflicts during initialization
and only perform bit tests when running.
## Testing
I ran many_foxes and didn't see any performance changes. It's probably
worth testing this with a wider range of realistic schedules to see
whether the reduced concurrency has a cost in practice, but I don't know
what sort of test cases to use.
## Migration Guide
The schedule will now prevent systems from running in parallel if there
*could* be an archetype that they conflict on, even if there aren't
actually any. For example, these systems will now conflict even if no
entity has both `Player` and `Enemy` components:
```rust
fn player_system(query: Query<(&mut Transform, &Player)>) {}
fn enemy_system(query: Query<(&mut Transform, &Enemy)>) {}
```
To allow them to run in parallel, use `Without` filters, just as you
would to allow both queries in a single system:
```rust
// Either one of these changes alone would be enough
fn player_system(query: Query<(&mut Transform, &Player), Without<Enemy>>) {}
fn enemy_system(query: Query<(&mut Transform, &Enemy), Without<Player>>) {}
```
# Objective
Prevent using exclusive systems as observers. Allowing them is unsound,
because observers are only expected to have `DeferredWorld` access, and
the observer infrastructure will keep pointers that are invalidated by
the creation of `&mut World`.
See
https://github.com/bevyengine/bevy/actions/runs/14778342801/job/41491517847?pr=19011
for a MIRI failure in a recent PR caused by an exclusive system being
used as an observer in a test.
## Solution
Have `Observer::new` panic if `System::is_exclusive()` is true. Document
that method, and methods that call it, as panicking.
(It should be possible to express this in the type system so that the
calls won't even compile, but I did not want to attempt that.)
## Testing
Added a unit test that calls `World::add_observer` with an exclusive
system.
# Objective
I've been tinkering with ECS insertion/removal lately, and noticed that
sparse sets just... don't interact with `InsertMode` at all. Sure
enough, using `insert_if_new` with a sparse component does the same
thing as `insert`.
# Solution
- Add a check in `BundleInfo::write_components` to drop the new value if
the entity already has the component and `InsertMode` is `Keep`.
- Add necessary methods to sparse set internals to fetch the drop
function.
# Testing
Minimal reproduction:
<details>
<summary>Code</summary>
```
use bevy::prelude::*;
fn main() {
App::new()
.add_plugins(DefaultPlugins)
.add_systems(Startup, setup)
.add_systems(PostStartup, component_print)
.run();
}
#[derive(Component)]
#[component(storage = "SparseSet")]
struct SparseComponent(u32);
fn setup(mut commands: Commands) {
let mut entity = commands.spawn_empty();
entity.insert(SparseComponent(1));
entity.insert(SparseComponent(2));
let mut entity = commands.spawn_empty();
entity.insert(SparseComponent(3));
entity.insert_if_new(SparseComponent(4));
}
fn component_print(query: Query<&SparseComponent>) {
for component in &query {
info!("{}", component.0);
}
}
```
</details>
Here it is on Bevy Playground (0.15.3):
https://learnbevy.com/playground?share=2a96a68a81e804d3fdd644a833c1d51f7fa8dd33fc6192fbfd077b082a6b1a41
Output on `main`:
```
2025-05-04T17:50:50.401328Z INFO system{name="fork::component_print"}: fork: 2
2025-05-04T17:50:50.401583Z INFO system{name="fork::component_print"}: fork: 4
```
Output with this PR :
```
2025-05-04T17:51:33.461835Z INFO system{name="fork::component_print"}: fork: 2
2025-05-04T17:51:33.462091Z INFO system{name="fork::component_print"}: fork: 3
```
# Objective
`BTreeSet` doesn't implement `RelationshipSourceCollection`.
## Solution
Implement it.
## Testing
`cargo clippy`
---
## Showcase
You can now use `BTreeSet` in a `RelationshipTarget`
---------
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
Co-authored-by: François Mockers <mockersf@gmail.com>
# Objective
Create a `When` system param wrapper for skipping systems that fail
validation.
Currently, the `Single` and `Populated` parameters cause systems to skip
when they fail validation, while the `Res` family causes systems to
error. Generalize this so that any fallible parameter can be used either
to skip a system or to raise an error. A parameter used directly will
always raise an error, and a parameter wrapped in `When<P>` will always
cause the system to be silently skipped.
~~Note that this changes the behavior for `Single` and `Populated`. The
current behavior will be available using `When<Single>` and
`When<Populated>`.~~
Fixes#18516
## Solution
Create a `When` system param wrapper that wraps an inner parameter and
converts all validation errors to `skipped`.
~~Change the behavior of `Single` and `Populated` to fail by default.~~
~~Replace in-engine use of `Single` with `When<Single>`. I updated the
`fallible_systems` example, but not all of the others. The other
examples I looked at appeared to always have one matching entity, and it
seemed more clear to use the simpler type in those cases.~~
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Zachary Harrold <zac@harrold.com.au>
Co-authored-by: François Mockers <mockersf@gmail.com>
# Objective
We have methods to:
- Add related entities
- Replace related entities
- Remove specific related entities
We don't have a method the remove all related entities so.
## Solution
Add a method to remove all related entities.
## Testing
A new test case.
# Objective
When implementing `SystemParam` for an object which contains a mutable
reference to World, which cannot be derived due to a required lifetime
parameter, it's necessary to check that there aren't any conflicts.
As far as I know, the is_empty method is the only way provided to check
for no conflicts at all
# Objective
Fixes#18857.
## Solution
Add the requested method, and a `try_` variant as well.
## Testing
It compiles, doctests succeed, and is trivial enough that I don't think
it needs a unit test (correct me if I'm wrong though).
# Objective
A small typo was found on `bevy_ecs/examples/event.rs`.
I know it's very minor but I'd think fixing it would still help others
in the long run.
## Solution
Fix the typo.
## Testing
I don't think this is necessary.
# Objective
One to one relationships (added in
https://github.com/bevyengine/bevy/pull/18087) can currently easily be
invalidated by having two entities relate to the same target.
Alternative to #18817 (removing one-to-one relationships)
## Solution
Panic if a RelationshipTarget is already targeted. Thanks @urben1680 for
the idea!
---------
Co-authored-by: François Mockers <mockersf@gmail.com>
Fixes#18834.
`EntityWorldMut::remove_children` and `EntityCommands::remove_children`
were removed in the relationships overhaul (#17398) and never got
replaced.
I don't *think* this was intentional (the methods were never mentioned
in the PR or its comments), but I could've missed something.
Fixes a small mix-up from #18058, which added bulk relationship
replacement methods.
`EntityCommands::replace_related_with_difference` calls
`EntityWorldMut::replace_children_with_difference` instead of
`EntityWorldMut::replace_related_with_difference`, which means it always
operates on the `ChildOf` relationship instead of the `R: Relationship`
generic it's provided.
`EntityCommands::replace_children_with_difference` takes an `R:
Relationship` generic that it shouldn't, but it accidentally works
correctly on `main` because it calls the above method.
# Objective
After #17967, closures which always panic no longer satisfy various Bevy
traits. Principally, this affects observers, systems and commands.
While this may seem pointless (systems which always panic are kind of
useless), it is distinctly annoying when using the `todo!` macro, or
when writing tests that should panic.
Fixes#18778.
## Solution
- Add failing tests to demonstrate the problem
- Add the trick from
[`never_say_never`](https://docs.rs/never-say-never/latest/never_say_never/)
to name the `!` type on stable Rust
- Write looots of docs explaining what the heck is going on and why
we've done this terrible thing
## To do
Unfortunately I couldn't figure out how to avoid conflicting impls, and
I am out of time for today, the week and uh the week after that.
Vacation! If you feel like finishing this for me, please submit PRs to
my branch and I can review and press the button for it while I'm off.
Unless you're Cart, in which case you have write permissions to my
branch!
- [ ] fix for commands
- [ ] fix for systems
- [ ] fix for observers
- [ ] revert https://github.com/bevyengine/bevy-website/pull/2092/
## Testing
I've added a compile test for these failure cases and a few adjacent
non-failing cases (with explicit return types).
---------
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
The parameter `In` of `call_inner` is completely unconstrained by its
arguments and return type. We are only able to infer it by assuming that
the only associated type equal to `In::Param<'_>` is `In::Param<'_>`
itself. It could just as well be some other associated type which only
normalizes to `In::Param<'_>`. This will change with the next-generation
trait solver and was encountered by a crater run
https://github.com/rust-lang/rust/pull/133502-
cc
https://github.com/rust-lang/trait-system-refactor-initiative/issues/168
I couldn't think of a cleaner alternative here. I first tried to just
provide `In` as an explicit type parameter. This is also kinda ugly as I
need to provide a variable number of them and `${ignore(..)}` is
currently still unstable https://github.com/rust-lang/rust/issues/83527.
Sorry for the inconvenience. Also fun that this function exists to avoid
a separate solver bug in the first place 😅
# 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
- Piped systems are an edge case that we missed when reworking system
parameter validation.
- Fixes#18755.
## Solution
- Validate the parameters for both systems, ~~combining the errors if
both failed validation~~ by simply using an early out.
- ~~Also fix the same bug for combinator systems while we're here.~~
## Testing
I've added a large number of tests checking the behavior under various
permutations. These are separate tests, rather than one mega test
because a) it's easier to track down bugs that way and b) many of these
are `should_panic` tests, which will halt the evaluation of the rest of
the test!
I've also added a test for exclusive systems being pipeable because we
don't have one and I was very surprised that that works!
---------
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
# Objective
Fixes#18678
## Solution
Moved the current `with_related` method to `with_relationships` and
added a new `with_related` that uses a bundle.
I'm not entirely sold on the name just yet, if anyone has any ideas let
me know.
## Testing
I wasn't able to test these changes because it crashed my computer every
time I tried (fun). But there don't seem to be any tests that use the
old `with_related` method so it should be fine, hopefully
## Showcase
```rust
commands.spawn_empty()
.with_related::<Relationship>(Name::new("Related thingy"))
.with_relationships(|rel| {
rel.spawn(Name::new("Second related thingy"));
});
```
---------
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
# Objective
- Allow viewing and setting the added tick for change detection aware
data, to allow operations like checking if the value has been modified
since first being added, and spoofing that state (i.e. returning the
value to default in place without a remove/insert dance)
## Solution
- Added corresponding functions matching the existing `changed` API:
- `fn added(&self) -> Tick`
- `fn set_added(&mut self)`
- `fn set_last_added(&mut self, last_added: Tick)`
Discussed on discord @
https://canary.discord.com/channels/691052431525675048/749335865876021248/1358718892465193060
## Testing
- Running the bevy test suite by.. making a PR, heck.
- No new tests were introduced due to triviality (i.e. I don't know what
to test about this API, and the corresponding API for `changed` is
similarly lacking tests.)
---------
Co-authored-by: moonheart08 <moonheart08@users.noreply.github.com>
Co-authored-by: SpecificProtagonist <vincentjunge@posteo.net>
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
# Objective
Clarify information in the docs about the bundle removal commands.
## Solution
Added information about how the intersection of components are removed.
# Objective
- The `#[deprecated]` attributes supports a `since` field, which
documents in which version an item was deprecated. This field is visible
in `rustdoc`.
- We inconsistently use `since` throughout the project.
For an example of what `since` renders as, take a look at
`ChildOf::get()`:
```rust
/// The parent entity of this child entity.
#[deprecated(since = "0.16.0", note = "Use child_of.parent() instead")]
#[inline]
pub fn get(&self) -> Entity {
self.0
}
```
## Solution
- Add `since = "0.16.0"` to all `#[deprecated]` attributes that do not
already use it.
- Add an example of deprecating a struct with the `since` field in the
migration guide document.
I would appreciate if this could be included in 0.16's release, as its a
low-risk documentation improvement that is valuable for the release, but
I'd understand if this was cut.
## Testing
You can use `cargo doc` to inspect the rendered form of
`#[deprecated(since = "0.16.0", ...)]`.
# Objective
Newest installment of the #16547 series.
In #18319 we introduced `Entity` defaults to accomodate the most common
use case for these types, however that resulted in the switch of the `T`
and `N` generics of `UniqueEntityArray`.
Swapping generics might be somewhat acceptable for `UniqueEntityArray`,
it is not at all acceptable for map and set types, which we would make
generic over `T: EntityEquivalent` in #18408.
Leaving these defaults in place would result in a glaring inconsistency
between these set collections and the others.
Additionally, the current standard in the engine is for "entity" to mean
`Entity`. APIs could be changed to accept `EntityEquivalent`, however
that is a separate and contentious discussion.
## Solution
Name these set collections `UniqueEntityEquivalent*`, and retain the
`UniqueEntity*` name for an alias of the `Entity` case.
While more verbose, this allows for all generics to be in proper order,
full consistency between all set types*, and the "entity" name to be
restricted to `Entity`.
On top of that, `UniqueEntity*` now always have 1 generic less, when
previously this was not enforced for the default case.
*`UniqueEntityIter<I: Iterator<T: EntityEquivalent>>` is the sole
exception to this. Aliases are unable to enforce bounds
(`lazy_type_alias` is needed for this), so for this type, doing this
split would be a mere suggestion, and in no way enforced.
Iterator types are rarely ever named, and this specific one is intended
to be aliased when it sees more use, like we do for the corresponding
set collection iterators.
Furthermore, the `EntityEquivalent` precursor `Borrow<Entity>` was used
exactly because of such iterator bounds!
Because of that, we leave it as is.
While no migration guide for 0.15 users, for those that upgrade from
main:
`UniqueEntityVec<T>` -> `UniqueEntityEquivalentVec<T>`
`UniqueEntitySlice<T>` -> `UniqueEntityEquivalentSlice<T>`
`UniqueEntityArray<N, T>` -> `UniqueEntityEquivalentArray<T, N>`
#18555 improved syntax for required components.
However some code was a bit redundant after the new parsing and struct
initializing would not give proper errors.
This PR fixes that.
---------
Co-authored-by: Tim Overbeek <oorbecktim@Tims-MacBook-Pro.local>
# Objective
Improve the parameter validation error message for
`Event(Reader|Writer|Mutator)`.
System parameters defined using `#[derive(SystemParam)]`, including the
parameters for events, currently propagate the validation errors from
their subparameters. The error includes the type of the failing
parameter, so the resulting error includes the type of the failing
subparameter instead of the derived parameter.
In particular, `EventReader<T>` will report an error from a
`Res<Events<T>>`, even though the user has no parameter of that type!
This is a follow-up to #18593.
## Solution
Have `#[derive]`d system parameters map errors during propagation so
that they report the outer parameter type.
To continue to provide context, add a field to
`SystemParamValidationError` that identifies the subparameter by name,
and is empty for non-`#[derive]`d parameters.
Allow them to override the failure message for individual parameters.
Use this to convert "Resource does not exist" to "Event not initialized"
for `Event(Reader|Writer|Mutator)`.
## Showcase
The validation error for a `EventReader<SomeEvent>` parameter when
`add_event` has not been called changes from:
Before:
```
Parameter `Res<Events<SomeEvent>>` failed validation: Resource does not exist
```
After
```
Parameter `EventReader<SomeEvent>::events` failed validation: Event not initialized
```
Extension of #18409.
I was updating a migration guide for hierarchy commands and realized
`insert_children` wasn't added to `EntityCommands`, only
`EntityWorldMut`.
This adds that and `insert_related` (basically just some
copy-and-pasting).
# Objective
In #17905 we swapped to a named field on `ChildOf` to help resolve
variable naming ambiguity of child vs parent (ex: `child_of.parent`
clearly reads as "I am accessing the parent of the child_of
relationship", whereas `child_of.0` is less clear).
Unfortunately this has the side effect of making initialization less
ideal. `ChildOf { parent }` reads just as well as `ChildOf(parent)`, but
`ChildOf { parent: root }` doesn't read nearly as well as
`ChildOf(root)`.
## Solution
Move back to `ChildOf(pub Entity)` but add a `child_of.parent()`
function and use it for all accesses. The downside here is that users
are no longer "forced" to access the parent field with `parent`
nomenclature, but I think this strikes the right balance.
Take a look at the diff. I think the results provide strong evidence for
this change. Initialization has the benefit of reading much better _and_
of taking up significantly less space, as many lines go from 3 to 1, and
we're cutting out a bunch of syntax in some cases.
Sadly I do think this should land in 0.16 as the cost of doing this
_after_ the relationships migration is high.
# Objective
Provide more useful errors when `World::run_system` and related methods
fail parameter validation.
Let callers determine whether the validation failure would have skipped
or failed the system.
Follow-up to #18541.
## Solution
Add a `SystemParamValidationError` value to the
`RunSystemError::InvalidParams` and
`RegisteredSystemError::InvalidParams` variants. That includes the
complete context of the parameter validation error, including the
`skipped` flag.
fixes#17478
# Objective
- Complete #17558.
- the `insert_children` method was previously removed, and as #17478
points out, needs to be added back.
## Solution
- Add a `OrderedRelationshipSourceCollection`, which allows sorting,
ordering, rearranging, etc of a `RelationshipSourceCollection`.
- Implement `insert_related`
- Implement `insert_children`
- Tidy up some docs while I'm here.
## Testing
@bjoernp116 set up a unit test, and I added a doc test to
`OrderedRelationshipSourceCollection`.
---------
Co-authored-by: bjoernp116 <bjoernpollen@gmail.com>
Co-authored-by: Dmytro Banin <banind@cs.washington.edu>
Co-authored-by: Talin <viridia@gmail.com>
# Objective
#18173 allows components to be queued without being fully registered.
But much of bevy's debug logging contained
`components.get_name(id).unwrap()`. However, this panics when the id is
queued. This PR fixes this, allowing names to be retrieved for debugging
purposes, etc, even while they're still queued.
## Solution
We change `ComponentInfo::descriptor` to be `Arc<ComponentDescriptor>`
instead of not arc'd. This lets us pass the descriptor around (as a name
or otherwise) as needed. The alternative would require some form of
`MappedRwLockReadGuard`, which is unstable, and would be terribly
blocking. Putting it in an arc also signifies that it doesn't change,
which is a nice signal to users. This does mean there's an extra pointer
dereference, but I don't think that's an issue here, as almost all paths
that use this are for debugging purposes or one-time set ups.
## Testing
Existing tests.
## Migration Guide
`Components::get_name` now returns `Option<Cow<'_, str>` instead of
`Option<&str>`. This is because it now returns results for queued
components. If that behavior is not desired, or you know the component
is not queued, you can use
`components.get_info().map(ComponentInfo::name)` instead.
Similarly, `ScheduleGraph::conflicts_to_string` now returns `impl
Iterator<Item = (String, String, Vec<Cow<str>>)>` instead of `impl
Iterator<Item = (String, String, Vec<&str>)>`. Because `Cow<str>` derefs
to `&str`, most use cases can remain unchanged.
---------
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
- Lots of nits, formatting, and rephrasing, with the goal of making
things more consistent.
- Fix outdated error handler explanation in `Commands` and
`EntityCommands` docs.
- Expand docs for system-related commands.
- Remove panic notes if the command only panics with the default error
handler.
- Update error handling notes for `try_` variants.
- Hide `prelude` import in most doctest examples, unless the example
uses something that people might not realize is in the prelude (like
`Name`).
- Remove a couple doctest examples that (in my opinion) didn't make
sense.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
# Objective
Improve error messages for missing resources.
The default error handler currently prints the `Debug` representation of
the error type instead of `Display`. Most error types use
`#[derive(Debug)]`, resulting in a dump of the structure, but will have
a user-friendly message for `Display`.
Follow-up to #18593
## Solution
Change the default error handler to use `Display` instead of `Debug`.
Change `BevyError` to include the backtrace in the `Display` format in
addition to `Debug` so that it is still included.
## Showcase
Before:
```
Encountered an error in system `system_name`: SystemParamValidationError { skipped: false, message: "Resource does not exist", param: "bevy_ecs::change_detection::Res<app_name::ResourceType>" }
Encountered an error in system `other_system_name`: "String message with\nmultiple lines."
```
After
```
Encountered an error in system `system_name`: Parameter `Res<ResourceType>` failed validation: Resource does not exist
Encountered an error in system `other_system_name`: String message with
multiple lines.
```
# Objective
- There's been several changes to `Query` for this release cycle, and
`Query`'s top-level documentation has gotten slightly out-of-date.
- Alternative to #18615.
## Solution
- Edit `Query`'s docs for consistency, clarity, and correctness.
- Make sure to group `get()` and `get_many()` together instead of
`single()` and `get_many()`, to enforce the distinction from
https://github.com/bevyengine/bevy/pull/18615#issuecomment-2764355672.
- Reformat doc tests so they would be readable if extracted into their
own file. (Which mainly involves adding more spacing.)
- Move link definitions to be nearer where they are used.
- Fix the tables so they are up-to-date and correctly escape square
brackets `\[ \]`.
## Testing
I ran `cargo doc -p bevy_ecs --no-deps` to view the docs and `cargo test
-p bevy_ecs --doc` to test the doc comments.
## Reviewing
The diff is difficult to read, so I don't recommend _just_ looking at
that. Instead, run `cargo doc -p bevy_ecs --no-deps` locally and read
through the new version. It should theoretically read smoother with less
super-technical jargon. :)
## Follow-up
I want to go through some of `Query`'s methods, such as `single()`,
`get()`, and `get_many()`, but I'll leave that for another PR.
---------
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
# Objective
Fixes#9367.
Yet another follow-up to #16547.
These traits were initially based on `Borrow<Entity>` because that trait
was what they were replacing, and it felt close enough in meaning.
However, they ultimately don't quite match: `borrow` always returns
references, whereas `EntityBorrow` always returns a plain `Entity`.
Additionally, `EntityBorrow` can imply that we are borrowing an `Entity`
from the ECS, which is not what it does.
Due to its safety contract, `TrustedEntityBorrow` is important an
important and widely used trait for `EntitySet` functionality.
In contrast, the safe `EntityBorrow` does not see much use, because even
outside of `EntitySet`-related functionality, it is a better idea to
accept `TrustedEntityBorrow` over `EntityBorrow`.
Furthermore, as #9367 points out, abstracting over returning `Entity`
from pointers/structs that contain it can skip some ergonomic friction.
On top of that, there are aspects of #18319 and #18408 that are relevant
to naming:
We've run into the issue that relying on a type default can switch
generic order. This is livable in some contexts, but unacceptable in
others.
To remedy that, we'd need to switch to a type alias approach:
The "defaulted" `Entity` case becomes a
`UniqueEntity*`/`Entity*Map`/`Entity*Set` alias, and the base type
receives a more general name. `TrustedEntityBorrow` does not mesh
clearly with sensible base type names.
## Solution
Replace any `EntityBorrow` bounds with `TrustedEntityBorrow`.
+
Rename them as such:
`EntityBorrow` -> `ContainsEntity`
`TrustedEntityBorrow` -> `EntityEquivalent`
For `EntityBorrow` we produce a change in meaning; We designate it for
types that aren't necessarily strict wrappers around `Entity` or some
pointer to `Entity`, but rather any of the myriad of types that contain
a single associated `Entity`.
This pattern can already be seen in the common `entity`/`id` methods
across the engine.
We do not mean for `ContainsEntity` to be a trait that abstracts input
API (like how `AsRef<T>` is often used, f.e.), because eliding
`entity()` would be too implicit in the general case.
We prefix "Contains" to match the intuition of a struct with an `Entity`
field, like some contain a `length` or `capacity`.
It gives the impression of structure, which avoids the implication of a
relationship to the `ECS`.
`HasEntity` f.e. could be interpreted as "a currently live entity",
As an input trait for APIs like #9367 envisioned, `TrustedEntityBorrow`
is a better fit, because it *does* restrict itself to strict wrappers
and pointers. Which is why we replace any
`EntityBorrow`/`ContainsEntity` bounds with
`TrustedEntityBorrow`/`EntityEquivalent`.
Here, the name `EntityEquivalent` is a lot closer to its actual meaning,
which is "A type that is both equivalent to an `Entity`, and forms the
same total order when compared".
Prior art for this is the
[`Equivalent`](https://docs.rs/hashbrown/latest/hashbrown/trait.Equivalent.html)
trait in `hashbrown`, which utilizes both `Borrow` and `Eq` for its one
blanket impl!
Given that we lose the `Borrow` moniker, and `Equivalent` can carry
various meanings, we expand on the safety comment of `EntityEquivalent`
somewhat. That should help prevent the confusion we saw in
[#18408](https://github.com/bevyengine/bevy/pull/18408#issuecomment-2742094176).
The new name meshes a lot better with the type aliasing approach in
#18408, by aligning with the base name `EntityEquivalentHashMap`.
For a consistent scheme among all set types, we can use this scheme for
the `UniqueEntity*` wrapper types as well!
This allows us to undo the switched generic order that was introduced to
`UniqueEntityArray` by its `Entity` default.
Even without the type aliases, I think these renames are worth doing!
## Migration Guide
Any use of `EntityBorrow` becomes `ContainsEntity`.
Any use of `TrustedEntityBorrow` becomes `EntityEquivalent`.
# Objective
Unlike for their helper typers, the import paths for
`unique_array::UniqueEntityArray`, `unique_slice::UniqueEntitySlice`,
`unique_vec::UniqueEntityVec`, `hash_set::EntityHashSet`,
`hash_map::EntityHashMap`, `index_set::EntityIndexSet`,
`index_map::EntityIndexMap` are quite redundant.
When looking at the structure of `hashbrown`, we can also see that while
both `HashSet` and `HashMap` have their own modules, the main types
themselves are re-exported to the crate level.
## Solution
Re-export the types in their shared `entity` parent module, and simplify
the imports where they're used.
# Objective
Fixes#18515
After the recent changes to system param validation, the panic message
for a missing resource is currently:
```
Encountered an error in system `missing_resource_error::res_system`: SystemParamValidationError { skipped: false }
```
Add the parameter type name and a descriptive message, improving the
panic message to:
```
Encountered an error in system `missing_resource_error::res_system`: SystemParamValidationError { skipped: false, message: "Resource does not exist", param: "bevy_ecs::change_detection::Res<missing_resource_error::MissingResource>" }
```
## Solution
Add fields to `SystemParamValidationError` for error context. Include
the `type_name` of the param and a message.
Store them as `Cow<'static, str>` and only format them into a friendly
string in the `Display` impl. This lets us create errors using a
`&'static str` with no allocation or formatting, while still supporting
runtime `String` values if necessary.
Add a unit test that verifies the panic message.
## Future Work
If we change the default error handling to use `Display` instead of
`Debug`, and to use `ShortName` for the system name, the panic message
could be further improved to:
```
Encountered an error in system `res_system`: Parameter `Res<MissingResource>` failed validation: Resource does not exist
```
However, `BevyError` currently includes the backtrace in `Debug` but not
`Display`, and I didn't want to try to change that in this PR.
# Objective
#18555 added improved require syntax, but inline structs didn't support
`..Default::default()` syntax (for technical reasons we can't parse the
struct directly, so there is manual logic that missed this case).
## Solution
When a `{}` or `()` section is encountered for a required component,
rather than trying to parse the fields directly, just pass _all_ of the
tokens through. This ensures no tokens are dropped, protects us against
any future syntax changes, and optimizes our parsing logic (as we're
dropping the field parsing logic entirely).
# Objective
Requires are currently more verbose than they need to be. People would
like to define inline component values. Additionally, the current
`#[require(Foo(custom_constructor))]` and `#[require(Foo(|| Foo(10))]`
syntax doesn't really make sense within the context of the Rust type
system. #18309 was an attempt to improve ergonomics for some cases, but
it came at the cost of even more weirdness / unintuitive behavior. Our
approach as a whole needs a rethink.
## Solution
Rework the `#[require()]` syntax to make more sense. This is a breaking
change, but I think it will make the system easier to learn, while also
improving ergonomics substantially:
```rust
#[derive(Component)]
#[require(
A, // this will use A::default()
B(1), // inline tuple-struct value
C { value: 1 }, // inline named-struct value
D::Variant, // inline enum variant
E::SOME_CONST, // inline associated const
F::new(1), // inline constructor
G = returns_g(), // an expression that returns G
H = SomethingElse::new(), // expression returns SomethingElse, where SomethingElse: Into<H>
)]
struct Foo;
```
## Migration Guide
Custom-constructor requires should use the new expression-style syntax:
```rust
// before
#[derive(Component)]
#[require(A(returns_a))]
struct Foo;
// after
#[derive(Component)]
#[require(A = returns_a())]
struct Foo;
```
Inline-closure-constructor requires should use the inline value syntax
where possible:
```rust
// before
#[derive(Component)]
#[require(A(|| A(10))]
struct Foo;
// after
#[derive(Component)]
#[require(A(10)]
struct Foo;
```
In cases where that is not possible, use the expression-style syntax:
```rust
// before
#[derive(Component)]
#[require(A(|| A(10))]
struct Foo;
// after
#[derive(Component)]
#[require(A = A(10)]
struct Foo;
```
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: François Mockers <mockersf@gmail.com>
# Objective
Fix panic in `run_system` when running an exclusive system wrapped in a
`PipeSystem` or `AdapterSystem`.
#18076 introduced a `System::run_without_applying_deferred` method. It
normally calls `System::run_unsafe`, but
`ExclusiveFunctionSystem::run_unsafe` panics, so it was overridden for
that type. Unfortunately, `PipeSystem::run_without_applying_deferred`
still calls `PipeSystem::run_unsafe`, which can then call
`ExclusiveFunctionSystem::run_unsafe` and panic.
## Solution
Make `ExclusiveFunctionSystem::run_unsafe` work instead of panicking.
Clarify the safety requirements that make this sound.
The alternative is to override `run_without_applying_deferred` in
`PipeSystem`, `CombinatorSystem`, `AdapterSystem`,
`InfallibleSystemWrapper`, and `InfallibleObserverWrapper`. That seems
like a lot of extra code just to preserve a confusing special case!
Remove some implementations of `System::run` that are no longer
necessary with this change. This slightly changes the behavior of
`PipeSystem` and `CombinatorSystem`: Currently `run` will call
`apply_deferred` on the first system before running the second, but
after this change it will only call it after *both* systems have run.
The new behavior is consistent with `run_unsafe` and
`run_without_applying_deferred`, and restores the behavior prior to
#11823.
The panic was originally necessary because [`run_unsafe` took
`&World`](https://github.com/bevyengine/bevy/pull/6083/files#diff-708dfc60ec5eef432b20a6f471357a7ea9bfb254dc2f918d5ed4a66deb0e85baR90).
Now that it takes `UnsafeWorldCell`, it is possible to make it work. See
also Cart's concerns at
https://github.com/bevyengine/bevy/pull/4166#discussion_r979140356,
although those also predate `UnsafeWorldCell`.
And see #6698 for a previous bug caused by this panic.
# Objective
Make it easier to short-circuit system parameter validation.
Simplify the API surface by combining `ValidationOutcome` with
`SystemParamValidationError`.
## Solution
Replace `ValidationOutcome` with `Result<(),
SystemParamValidationError>`. Move the docs from `ValidationOutcome` to
`SystemParamValidationError`.
Add a `skipped` field to `SystemParamValidationError` to distinguish the
`Skipped` and `Invalid` variants.
Use the `?` operator to short-circuit validation in tuples of system
params.
# Objective
- Fixes#16861
## Solution
- Added:
- `UnsafeEntityCell::get_mut_assume_mutable_by_id`
- `EntityMut::get_mut_assume_mutable_by_id`
- `EntityMut::get_mut_assume_mutable_by_id_unchecked`
- `EntityWorldMut::into_mut_assume_mutable_by_id`
- `EntityWorldMut::into_mut_assume_mutable`
- `EntityWorldMut::get_mut_assume_mutable_by_id`
- `EntityWorldMut::into_mut_assume_mutable_by_id`
- `EntityWorldMut::modify_component_by_id`
- `World::modify_component_by_id`
- `DeferredWorld::modify_component_by_id`
- Added `fetch_mut_assume_mutable` to `DynamicComponentFetch` trait
(this is a breaking change)
## Testing
- CI
---
## Migration Guide
If you had previously implemented `DynamicComponentFetch` you must now
include a definition for `fetch_mut_assume_mutable`. In general this
will be identical to `fetch_mut` using the relevant alternatives for
actually getting a component.
---
## Notes
All of the added methods are minor variations on existing functions and
should therefore be of low risk for inclusion during the RC process.
# Objective
fixes#18452.
## Solution
Spawning used to flush commands only, but those commands can reserve
entities. Now, spawning flushes everything, including reserved entities.
I checked, and this was the only place where `flush_commands` is used
instead of `flush` by mistake.
## Testing
I simplified the MRE from #18452 into its own test, which fails on main,
but passes on this branch.
# Objective
When introduced, `Single` was intended to simply be silently skipped,
allowing for graceful and efficient handling of systems during invalid
game states (such as when the player is dead).
However, this also caused missing resources to *also* be silently
skipped, leading to confusing and very hard to debug failures. In
0.15.1, this behavior was reverted to a panic, making missing resources
easier to debug, but largely making `Single` (and `Populated`)
worthless, as they would panic during expected game states.
Ultimately, the consensus is that this behavior should differ on a
per-system-param basis. However, there was no sensible way to *do* that
before this PR.
## Solution
Swap `SystemParam::validate_param` from a `bool` to:
```rust
/// The outcome of system / system param validation,
/// used by system executors to determine what to do with a system.
pub enum ValidationOutcome {
/// All system parameters were validated successfully and the system can be run.
Valid,
/// At least one system parameter failed validation, and an error must be handled.
/// By default, this will result in1 a panic. See [crate::error] for more information.
///
/// This is the default behavior, and is suitable for system params that should *always* be valid,
/// either because sensible fallback behavior exists (like [`Query`] or because
/// failures in validation should be considered a bug in the user's logic that must be immediately addressed (like [`Res`]).
Invalid,
/// At least one system parameter failed validation, but the system should be skipped due to [`ValidationBehavior::Skip`].
/// This is suitable for system params that are intended to only operate in certain application states, such as [`Single`].
Skipped,
}
```
Then, inside of each `SystemParam` implementation, return either Valid,
Invalid or Skipped.
Currently, only `Single`, `Option<Single>` and `Populated` use the
`Skipped` behavior. Other params (like resources) retain their current
failing
## Testing
Messed around with the fallible_params example. Added a pair of tests:
one for panicking when resources are missing, and another for properly
skipping `Single` and `Populated` system params.
## To do
- [x] get https://github.com/bevyengine/bevy/pull/18454 merged
- [x] fix the todo!() in the macro-powered tuple implementation (please
help 🥺)
- [x] test
- [x] write a migration guide
- [x] update the example comments
## Migration Guide
Various system and system parameter validation methods
(`SystemParam::validate_param`, `System::validate_param` and
`System::validate_param_unsafe`) now return and accept a
`ValidationOutcome` enum, rather than a `bool`. The previous `true`
values map to `ValidationOutcome::Valid`, while `false` maps to
`ValidationOutcome::Invalid`.
However, if you wrote a custom schedule executor, you should now respect
the new `ValidationOutcome::Skipped` parameter, skipping any systems
whose validation was skipped. By contrast, `ValidationOutcome::Invalid`
systems should also be skipped, but you should call the
`default_error_handler` on them first, which by default will result in a
panic.
If you are implementing a custom `SystemParam`, you should consider
whether failing system param validation is an error or an expected
state, and choose between `Invalid` and `Skipped` accordingly. In Bevy
itself, `Single` and `Populated` now once again skip the system when
their conditions are not met. This is the 0.15.0 behavior, but stands in
contrast to the 0.15.1 behavior, where they would panic.
---------
Co-authored-by: MiniaczQ <xnetroidpl@gmail.com>
Co-authored-by: Dmytro Banin <banind@cs.washington.edu>
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
# Objective
There are two related problems here:
1. Users should be able to change the fallback behavior of *all*
ECS-based errors in their application by setting the
`GLOBAL_ERROR_HANDLER`. See #18351 for earlier work in this vein.
2. The existing solution (#15500) for customizing this behavior is high
on boilerplate, not global and adds a great deal of complexity.
The consensus is that the default behavior when a parameter fails
validation should be set based on the kind of system parameter in
question: `Single` / `Populated` should silently skip the system, but
`Res` should panic. Setting this behavior at the system level is a
bandaid that makes getting to that ideal behavior more painful, and can
mask real failures (if a resource is missing but you've ignored a system
to make the Single stop panicking you're going to have a bad day).
## Solution
I've removed the existing `ParamWarnPolicy`-based configuration, and
wired up the `GLOBAL_ERROR_HANDLER`/`default_error_handler` to the
various schedule executors to properly plumb through errors .
Additionally, I've done a small cleanup pass on the corresponding
example.
## Testing
I've run the `fallible_params` example, with both the default and a
custom global error handler. The former panics (as expected), and the
latter spams the error console with warnings 🥲
## Questions for reviewers
1. Currently, failed system param validation will result in endless
console spam. Do you want me to implement a solution for warn_once-style
debouncing somehow?
2. Currently, the error reporting for failed system param validation is
very limited: all we get is that a system param failed validation and
the name of the system. Do you want me to implement improved error
reporting by bubbling up errors in this PR?
3. There is broad consensus that the default behavior for failed system
param validation should be set on a per-system param basis. Would you
like me to implement that in this PR?
My gut instinct is that we absolutely want to solve 2 and 3, but it will
be much easier to do that work (and review it) if we split the PRs
apart.
## Migration Guide
`ParamWarnPolicy` and the `WithParamWarnPolicy` have been removed
completely. Failures during system param validation are now handled via
the `GLOBAL_ERROR_HANDLER`: please see the `bevy_ecs::error` module docs
for more information.
---------
Co-authored-by: MiniaczQ <xnetroidpl@gmail.com>
# Objective
Bevy 0.15 used to have methods on `Children` for sorting and reordering
them. This is very important, because in certain situations, the order
of children matters. For example, in the context of UI nodes.
These methods are missing/omitted/forgotten in the current version,
after the Relationships rework.
Without them, it is impossible for me to upgrade `iyes_perf_ui` to Bevy
0.16.
## Solution
Reintroduce the methods. This PR simply copy-pastes them from Bevy 0.15.
# Objective
Create new `NonSendMarker` that does not depend on `NonSend`.
Required, in order to accomplish #17682. In that issue, we are trying to
replace `!Send` resources with `thread_local!` in order to unblock the
resources-as-components effort. However, when we remove all the `!Send`
resources from a system, that allows the system to run on a thread other
than the main thread, which is against the design of the system. So this
marker gives us the control to require a system to run on the main
thread without depending on `!Send` resources.
## Solution
Create a new `NonSendMarker` to replace the existing one that does not
depend on `NonSend`.
## Testing
Other than running tests, I ran a few examples:
- `window_resizing`
- `wireframe`
- `volumetric_fog` (looks so cool)
- `rotation`
- `button`
There is a Mac/iOS-specific change and I do not have a Mac or iOS device
to test it. I am doubtful that it would cause any problems for 2
reasons:
1. The change is the same as the non-wasm change which I did test
2. The Pixel Eagle tests run Mac tests
But it wouldn't hurt if someone wanted to spin up an example that
utilizes the `bevy_render` crate, which is where the Mac/iSO change was.
## Migration Guide
If `NonSendMarker` is being used from `bevy_app::prelude::*`, replace it
with `bevy_ecs::system::NonSendMarker` or use it from
`bevy_ecs::prelude::*`. In addition to that, `NonSendMarker` does not
need to be wrapped like so:
```rust
fn my_system(_non_send_marker: Option<NonSend<NonSendMarker>>) {
...
}
```
Instead, it can be used without any wrappers:
```rust
fn my_system(_non_send_marker: NonSendMarker) {
...
}
```
---------
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
# Objective
- `bevy_ecs` has lint errors without some features
## Solution
- Fix `clippy::allow-attributes-without-reason` when `bevy_reflect` is
disabled by adding a reason
- Fix `clippy::needless_return` when `std` is disabled by adding a gated
`expect` attribute and a comment to remove it when the `no_std` stuff is
addressed
## Testing
- `cargo clippy -p bevy_ecs --no-default-features --no-deps -- --D
warnings`
- CI
# Objective
There are currently too many disparate ways to handle entity mapping,
especially after #17687. We now have MapEntities, VisitEntities,
VisitEntitiesMut, Component::visit_entities,
Component::visit_entities_mut.
Our only known use case at the moment for these is entity mapping. This
means we have significant consolidation potential.
Additionally, VisitEntitiesMut cannot be implemented for map-style
collections like HashSets, as you cant "just" mutate a `&mut Entity`.
Our current approach to Component mapping requires VisitEntitiesMut,
meaning this category of entity collection isn't mappable. `MapEntities`
is more generally applicable. Additionally, the _existence_ of the
blanket From impl on VisitEntitiesMut blocks us from implementing
MapEntities for HashSets (or any types we don't own), because the owner
could always add a conflicting impl in the future.
## Solution
Use `MapEntities` everywhere and remove all "visit entities" usages.
* Add `Component::map_entities`
* Remove `Component::visit_entities`, `Component::visit_entities_mut`,
`VisitEntities`, and `VisitEntitiesMut`
* Support deriving `Component::map_entities` in `#[derive(Coomponent)]`
* Add `#[derive(MapEntities)]`, and share logic with the
`Component::map_entities` derive.
* Add `ComponentCloneCtx::queue_deferred`, which is command-like logic
that runs immediately after normal clones. Reframe `FromWorld` fallback
logic in the "reflect clone" impl to use it. This cuts out a lot of
unnecessary work and I think justifies the existence of a pseudo-command
interface (given how niche, yet performance sensitive this is).
Note that we no longer auto-impl entity mapping for ` IntoIterator<Item
= &'a Entity>` types, as this would block our ability to implement cases
like `HashMap`. This means the onus is on us (or type authors) to add
explicit support for types that should be mappable.
Also note that the Component-related changes do not require a migration
guide as there hasn't been a release with them yet.
## Migration Guide
If you were previously implementing `VisitEntities` or
`VisitEntitiesMut` (likely via a derive), instead use `MapEntities`.
Those were almost certainly used in the context of Bevy Scenes or
reflection via `ReflectMapEntities`. If you have a case that uses
`VisitEntities` or `VisitEntitiesMut` directly, where `MapEntities` is
not a viable replacement, please let us know!
```rust
// before
#[derive(VisitEntities, VisitEntitiesMut)]
struct Inventory {
items: Vec<Entity>,
#[visit_entities(ignore)]
label: String,
}
// after
#[derive(MapEntities)]
struct Inventory {
#[entities]
items: Vec<Entity>,
label: String,
}
```
# Objective
Const values should be more ergonomic to insert, since this is too
verbose
``` rust
#[derive(Component)]
#[require(
LockedAxes(||LockedAxes::ROTATION_LOCKED),
)]
pub struct CharacterController;
```
instead, users can now abbreviate that nonsense like this
``` rust
#[derive(Component)]
#[require(
LockedAxes = ROTATION_LOCKED),
)]
pub struct CharacterController;
```
it also works for enum labels.
I chose to omit the type, since were trying to reduce typing here. The
alternative would have been this:
```rust
#[require(
LockedAxes = LockedAxes::ROTATION_LOCKED),
)]
```
This of course has its disadvantages, since the const has to be
associated, but the old closure method is still possible, so I dont
think its a problem.
- Fixes#16720
## Testing
I added one new test in the docs, which also explain the new change. I
also saw that the docs for the required components on line 165 was
missing an assertion, so I added it back in
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
While working on #18058 I realized I could use
`RelationshipTargetCollection::new`, so I added it.
## Solution
- Add `RelationshipTargetCollection::new`
- Add `RelationshipTargetCollection::reserve`. Could generally be useful
when doing micro-optimizations.
- Add `RelationshipTargetCollection::shrink_to_fit`. Rust collections
generally don't shrink when removing elements. Might be a good idea to
call this once in a while.
## Testing
`cargo clippy`
---
## Showcase
`RelationshipSourceCollection` now implements `new`, `reserve` and
`shrink_to_fit` to give greater control over how much memory it
consumes.
## Migration Guide
Any type implementing `RelationshipSourceCollection` now needs to also
implement `new`, `reserve` and `shrink_to_fit`. `reserve` and
`shrink_to_fit` can be made no-ops if they conceptually mean nothing to
a collection.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
# Objective
Add a way to efficiently replace a set of specifically related entities
with a new set.
Closes#18041
## Solution
Add new `replace_(related/children)` to `EntityWorldMut` and friends.
## Testing
Added a new test to `hierarchy.rs` that specifically check if
`replace_children` actually correctly replaces the children on a entity
while keeping the original one.
---
## Showcase
`EntityWorldMut` and `EntityCommands` can now be used to efficiently
replace the entities a entity is related to.
```rust
/// `parent` has 2 children. `entity_a` and `entity_b`.
assert_eq!([entity_a, entity_b], world.entity(parent).get::<Children>());
/// Replace `parent`s children with `entity_a` and `entity_c`
world.entity_mut(parent).replace_related(&[entity_a, entity_c]);
/// `parent` now has 2 children. `entity_a` and `entity_c`.
///
/// `replace_children` has saved time by not removing and reading
/// the relationship between `entity_a` and `parent`
assert_eq!([entity_a, entity_c], world.entity(parent).get::<Children>());
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
## Objective
Fix `bevy_ecs` doc tests failing when used with `--all-features`.
```
---- crates\bevy_ecs\src\error\handler.rs - error::handler::GLOBAL_ERROR_HANDLER (line 87) stdout ----
error[E0425]: cannot find function `default_error_handler` in this scope
--> crates\bevy_ecs\src\error\handler.rs:92:24
|
8 | let error_handler = default_error_handler();
| ^^^^^^^^^^^^^^^^^^^^^ not found in this scope
```
I happened to come across this while testing #12207. I'm not sure it
actually needs fixing but seemed worth a go
## Testing
```
cargo test --doc -p bevy_ecs --all-features
```
## Side Notes
The CI misses this error as it doesn't use `--all-features`. Perhaps it
should?
I tried adding `--all-features` to `ci/src/commands/doc_tests.rs` but
this triggered a linker error:
```
Compiling bevy_dylib v0.16.0-dev (C:\Projects\bevy\crates\bevy_dylib)
error: linking with `link.exe` failed: exit code: 1189
= note: LINK : fatal error LNK1189: library limit of 65535 objects exceeded␍
```
# Objective
I was looking over a PR yesterday, and got confused by the docs on
deferred world. I thought I would add a little more detail to the struct
in order to clarify it a little.
## Solution
Document some more about deferred worlds.
# Objective
- ECS error handling is a lovely flagship feature for Bevy 0.16, all in
the name of reducing panics and encouraging better error handling
(#14275).
- Currently though, command and system error handling are completely
disjoint and use different mechanisms.
- Additionally, there's a number of distinct ways to set the
default/fallback/global error handler that have limited value. As far as
I can tell, this will be cfg flagged to toggle between dev and
production builds in 99.9% of cases, with no real value in more granular
settings or helpers.
- Fixes#17272
## Solution
- Standardize error handling on the OnceLock global error mechanisms
ironed out in https://github.com/bevyengine/bevy/pull/17215
- As discussed there, there are serious performance concerns there,
especially for commands
- I also think this is a better fit for the use cases, as it's truly
global
- Move from `SystemErrorContext` to a more general purpose
`ErrorContext`, which can handle observers and commands more clearly
- Cut the superfluous setter methods on `App` and `SubApp`
- Rename the limited (and unhelpful) `fallible_systems` example to
`error_handling`, and add an example of command error handling
## Testing
Ran the `error_handling` example.
## Notes for reviewers
- Do you see a clear way to allow commands to retain &mut World access
in the per-command custom error handlers? IMO that's a key feature here
(allowing the ad-hoc creation of custom commands), but I'm not sure how
to get there without exploding complexity.
- I've removed the feature gate on the default_error_handler: contrary
to @cart's opinion in #17215 I think that virtually all apps will want
to use this. Can you think of a category of app that a) is extremely
performance sensitive b) is fine with shipping to production with the
panic error handler? If so, I can try to gather performance numbers
and/or reintroduce the feature flag. UPDATE: see benches at the end of
this message.
- ~~`OnceLock` is in `std`: @bushrat011899 what should we do here?~~
- Do you have ideas for more automated tests for this collection of
features?
## Benchmarks
I checked the impact of the feature flag introduced: benchmarks might
show regressions. This bears more investigation. I'm still skeptical
that there are users who are well-served by a fast always panicking
approach, but I'm going to re-add the feature flag here to avoid
stalling this out.
---------
Co-authored-by: Zachary Harrold <zac@harrold.com.au>
# Objective
Fixes#18357
## Solution
Generalize `RelationshipInsertHookMode` to `RelationshipHookMode`, wire
it up to on_replace execution, and use it in the
`Relationship::on_replace` hook.
# Objective
FilteredResource::get should return a Result instead of Option
Fixes#17480
---
## Migration Guide
Users will need to handle the different return type on
FilteredResource::get, FilteredResource::get_id,
FilteredResource::get_mut as it is now a Result not an Option.
# Objective
Continuation of #17449.
#17449 implemented the wrapper types around `IndexMap`/`Set` and co.,
however punted on the slice types.
They are needed to support creating `EntitySetIterator`s from their
slices, not just the base maps and sets.
## Solution
Add the wrappers, in the same vein as #17449 and #17589 before.
The `Index`/`IndexMut` implementations take up a lot of space, however
they cannot be merged because we'd then get overlaps.
They are simply named `Slice` to match the `indexmap` naming scheme, but
this means they cannot be differentiated properly until their modules
are made public, which is already a follow-up mentioned in #17954.
# 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
```
# Objective
Currently experimenting with manually implementing
`Relationship`/`RelationshipTarget` to support associated edge data,
which means I need to replace the default hook implementations provided
by those traits. However, copying them over for editing revealed that
`UnsafeWorldCell::get_raw_command_queue` is `pub(crate)`, and I would
like to not have to clone the source collection, like the default impl.
So instead, I've taken to providing a safe abstraction for being able to
access entities and queue commands simultaneously.
## Solution
Added `World::entities_and_commands` and
`DeferredWorld::entities_and_commands`, which can be used like so:
```rust
let eid: Entity = /* ... */;
let (mut fetcher, mut commands) = world.entities_and_commands();
let emut = fetcher.get_mut(eid).unwrap();
commands.entity(eid).despawn();
```
## Testing
- Added a new test for each of the added functions.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
Continuation to #16547 and #17954.
The `get_many` family are the last methods on `Query`/`QueryState` for
which we're still missing a `unique` version.
## Solution
Offer `get_many_unique`/`get_many_unique_mut` and
`get_many_unique_inner`!
Their implementation is the same as `get_many`, the difference lies in
their guaranteed-to-be unique inputs, meaning we never do any aliasing
checks.
To reduce confusion, we also rename `get_many_readonly` into
`get_many_inner` and the current `get_many_inner` into
`get_many_mut_inner` to clarify their purposes.
## Testing
Doc examples.
## Migration Guide
`get_many_inner` is now called `get_many_mut_inner`.
`get_many_readonly` is now called `get_many_inner`.
Registering dynamic bundles was not possible for the user yet.
It is alone not very useful though as there are no methods to clone,
move or remove components via a `BundleId`. This could be a follow-up
work if this PR is approved and such a third (besides `T: Bundle` and
`ComponentId`(s)) API for structural operation is desired. I certainly
would use a hypothetical `EntityClonerBuilder::allow_by_bundle_id`.
I personally still would like this register method because I have a
`Bundles`-like custom data structure and I would like to not reinvent
the wheel. Then instead of having boxed `ComponentId` slices in my
collection I could look up explicit and required components there.
For reference scroll up to the typed version right above the new one.
# Objective
Installment of the #16547 series.
The vast majority of uses for these types will be the `Entity` case, so
it makes sense for it to be the default.
## Solution
`UniqueEntityVec`, `UniqueEntitySlice`, `UniqueEntityArray` and their
helper iterator aliases now have `Entity` as a default.
Unfortunately, this means the the `T` parameter for `UniqueEntityArray`
now has to be ordered after the `N` constant, which breaks the
consistency to `[T; N]`.
Same with about a dozen iterator aliases that take some `P`/`F`
predicate/function parameter.
This should however be an ergonomic improvement in most cases, so we'll
just have to live with this inconsistency.
## Migration Guide
Switch type parameter order for the relevant wrapper types/aliases.
# 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
While poking at https://github.com/bevyengine/bevy/issues/17272, I
noticed a few small things to clean up.
## Solution
- Improve the docs
- ~~move `SystemErrorContext` out of the `handler.rs` module: it's not
an error handler~~
