Follow-up of #19274.
Make the `check_change_tick` methods, of which some are now public, take
`CheckChangeTicks` to make it obvious where this tick comes from, see
other PR.
This also affects the `System` trait, hence the many changed files.
---------
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
# Objective
Reduce memory usage by storing fewer copies of
`FilteredAccessSet<ComponentId>`.
Currently, the `System` trait exposes the `component_access_set` for the
system, which is used by the multi-threaded executor to determine which
systems can run concurrently. But because it is available on the trait,
it needs to be stored for *every* system, even ones that are not run by
the executor! In particular, it is never needed for observers, or for
the inner systems in a `PipeSystem` or `CombinatorSystem`.
## Solution
Instead of exposing the access from a method on `System`, return it from
`System::initialize`. Since it is still needed during scheduling, store
the access alongside the boxed system in the schedule.
That's not quite enough for systems built using `SystemParamBuilder`s,
though. Those calculate the access in `SystemParamBuilder::build`, which
happens earlier than `System::initialize`. To handle those, we separate
`SystemParam::init_state` into `init_state`, which creates the state
value, and `init_access`, which calculates the access. This lets
`System::initialize` call `init_access` on a state that was provided by
the builder.
An additional benefit of that separation is that it removes the need to
duplicate access checks between `SystemParamBuilder::build` and
`SystemParam::init_state`.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
- Add example to `Single` docs, highlighting that you can use methods
and properties directly.
- Fixes#19461
## Solution
- Added example to inline docs of `Single`
## Testing
- `cargo test --doc`
- `cargo doc --open`
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
As raised by @Jondolf, this type is `pub`, and useful for various
consumers to ensure cleanup or debugging.
However, it doesn't offer any way to actually view the data.
## Solution
- Add a read-only view of the data.
- Don't add any (easy) way to mutate the data, as this presents a huge
footgun.
- Implement Reflect and register the component so you can see it in
inspectors nicely.
# Objective
Currently, the observer API looks like this:
```rust
app.add_observer(|trigger: Trigger<Explode>| {
info!("Entity {} exploded!", trigger.target());
});
```
Future plans for observers also include "multi-event observers" with a
trigger that looks like this (see [Cart's
example](https://github.com/bevyengine/bevy/issues/14649#issuecomment-2960402508)):
```rust
trigger: Trigger<(
OnAdd<Pressed>,
OnRemove<Pressed>,
OnAdd<InteractionDisabled>,
OnRemove<InteractionDisabled>,
OnInsert<Hovered>,
)>,
```
In scenarios like this, there is a lot of repetition of `On`. These are
expected to be very high-traffic APIs especially in UI contexts, so
ergonomics and readability are critical.
By renaming `Trigger` to `On`, we can make these APIs read more cleanly
and get rid of the repetition:
```rust
app.add_observer(|trigger: On<Explode>| {
info!("Entity {} exploded!", trigger.target());
});
```
```rust
trigger: On<(
Add<Pressed>,
Remove<Pressed>,
Add<InteractionDisabled>,
Remove<InteractionDisabled>,
Insert<Hovered>,
)>,
```
Names like `On<Add<Pressed>>` emphasize the actual event listener nature
more than `Trigger<OnAdd<Pressed>>`, and look cleaner. This *also* frees
up the `Trigger` name if we want to use it for the observer event type,
splitting them out from buffered events (bikeshedding this is out of
scope for this PR though).
For prior art:
[`bevy_eventlistener`](https://github.com/aevyrie/bevy_eventlistener)
used
[`On`](https://docs.rs/bevy_eventlistener/latest/bevy_eventlistener/event_listener/struct.On.html)
for its event listener type. Though in our case, the observer is the
event listener, and `On` is just a type containing information about the
triggered event.
## Solution
Steal from `bevy_event_listener` by @aevyrie and use `On`.
- Rename `Trigger` to `On`
- Rename `OnAdd` to `Add`
- Rename `OnInsert` to `Insert`
- Rename `OnReplace` to `Replace`
- Rename `OnRemove` to `Remove`
- Rename `OnDespawn` to `Despawn`
## Discussion
### Naming Conflicts??
Using a name like `Add` might initially feel like a very bad idea, since
it risks conflict with `core::ops::Add`. However, I don't expect this to
be a big problem in practice.
- You rarely need to actually implement the `Add` trait, especially in
modules that would use the Bevy ECS.
- In the rare cases where you *do* get a conflict, it is very easy to
fix by just disambiguating, for example using `ops::Add`.
- The `Add` event is a struct while the `Add` trait is a trait (duh), so
the compiler error should be very obvious.
For the record, renaming `OnAdd` to `Add`, I got exactly *zero* errors
or conflicts within Bevy itself. But this is of course not entirely
representative of actual projects *using* Bevy.
You might then wonder, why not use `Added`? This would conflict with the
`Added` query filter, so it wouldn't work. Additionally, the current
naming convention for observer events does not use past tense.
### Documentation
This does make documentation slightly more awkward when referring to
`On` or its methods. Previous docs often referred to `Trigger::target`
or "sends a `Trigger`" (which is... a bit strange anyway), which would
now be `On::target` and "sends an observer `Event`".
You can see the diff in this PR to see some of the effects. I think it
should be fine though, we may just need to reword more documentation to
read better.
# Objective
The documentation for observers is not very good. This poses a problem
to users, but *also* causes serious problems for engine devs, as they
attempt to improve assorted issues surrounding observers.
This PR:
- Fixes#14084.
- Fixes#14726.
- Fixes#16538.
- Closes#18914, by attempting to solve the same issue.
To keep this PR at all reviewable, I've opted to simply note the various
limitations (some may call them bugs!) in place, rather than attempting
to fix them. There is a huge amount of cleanup work to be done here: see
https://github.com/orgs/bevyengine/projects/17.
## Solution
- Write good module docs for observers, offering bread crumbs to the
most common methods and techniques and comparing-and-contrasting as
needed.
- Fix any actively misleading documentation.
- Try to explain how the various bits of the (public?!) internals are
related.
---------
Co-authored-by: Chris Biscardi <chris@christopherbiscardi.com>
Co-authored-by: Joona Aalto <jondolf.dev@gmail.com>
# Objective
As discussed in #19285, some of our names conflict. `Entry` in bevy_ecs
is one of those overly general names.
## Solution
Rename this type (and the related types) to `ComponentEntry`.
---------
Co-authored-by: urben1680 <55257931+urben1680@users.noreply.github.com>
# Objective
I set out with one simple goal: clearly document the differences between
each of the component lifecycle events via module docs.
Unfortunately, no such module existed: the various lifecycle code was
scattered to the wind.
Without a unified module, it's very hard to discover the related types,
and there's nowhere good to put my shiny new documentation.
## Solution
1. Unify the assorted types into a single
`bevy_ecs::component_lifecycle` module.
2. Write docs.
3. Write a migration guide.
## Testing
Thanks CI!
## Follow-up
1. The lifecycle event names are pretty confusing, especially
`OnReplace`. We should consider renaming those. No bikeshedding in my PR
though!
2. Observers need real module docs too :(
3. Any additional functional changes should be done elsewhere; this is a
simple docs and re-org PR.
---------
Co-authored-by: theotherphil <phil.j.ellison@gmail.com>
# Objective
Fixes#19403
As described in the issue, the objective is to support the use of
systems returning `Result<(), BevyError>` and
`Result<bool, BevyError>` as run conditions. In these cases, the run
condition would hold on `Ok(())` and `Ok(true)` respectively.
## Solution
`IntoSystem<In, bool, M>` cannot be implemented for systems returning
`Result<(), BevyError>` and `Result<bool, BevyError>` as that would
conflict with their trivial implementation of the trait. That led me to
add a method to the sealed trait `SystemCondition` that does the
conversion. In the original case of a system returning `bool`, the
system is returned as is. With the new types, the system is combined
with `map()` to obtain a `bool`.
By the way, I'm confused as to why `SystemCondition` has a generic `In`
parameter as it is only ever used with `In = ()` as far as I can tell.
## Testing
I added a simple test for both type of system. That's minimal but it
felt enough. I could not picture the more complicated tests passing for
a run condition returning `bool` and failing for the new types.
## Doc
I documenting the change on the page of the trait. I had trouble wording
it right but I'm not sure how to improve it. The phrasing "the condition
returns `true`" which reads naturally is now technically incorrect as
the new types return a `Result`. However, the underlying condition
system that the implementing system turns into does indeed return
`bool`. But talking about the implementation details felt too much.
Another possibility is to use another turn of phrase like "the condition
holds" or "the condition checks out". I've left "the condition returns
`true`" in the documentation of `run_if` and the provided methods for
now.
I'm perplexed about the examples. In the first one, why not implement
the condition directly instead of having a system returning it? Is it
from a time of Bevy where you had to implement your conditions that way?
In that case maybe that should be updated. And in the second example I'm
missing the point entirely. As I stated above, I've only seen conditions
used in contexts where they have no input parameter. Here we create a
condition with an input parameter (cannot be used by `run_if`) and we
are using it with `pipe()` which actually doesn't need our system to
implement `SystemCondition`. Both examples are also calling
`IntoSystem::into_system` which should not be encouraged. What am I
missing?
# Objective
- Cleanup related to #19495.
## Solution
- Delete `System::component_access()`. It is redundant with
`System::component_access_set().combined_access()`.
## Testing
- None. There are no callers of this function.
# Objective
In the past I had custom data structures containing `Tick`s. I learned
that these need to be regularly checked to clamp them. But there was no
way to hook into that logic so I abandoned storing ticks since then.
Another motivation to open this up some more is to be more able to do a
correct implementation of `System::check_ticks`.
## Solution
Add `CheckChangeTicks` and trigger it in `World::check_change_ticks`.
Make `Tick::check_tick` public.
This event makes it possible to store ticks in components or resources
and have them checked.
I also made `Schedules::check_change_ticks` public so users can store
schedules in custom resources/components for whatever reasons.
## Testing
The logic boils down to a single `World::trigger` call and I don't think
this needs more tests.
## Alternatives
Making this obsolete like with #15683.
---
## Showcase
From the added docs:
```rs
use bevy_ecs::prelude::*;
use bevy_ecs::component::CheckChangeTicks;
#[derive(Resource)]
struct CustomSchedule(Schedule);
let mut world = World::new();
world.add_observer(|tick: Trigger<CheckChangeTicks>, mut schedule: ResMut<CustomSchedule>| {
schedule.0.check_change_ticks(tick.get());
});
```
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
Fixes#19136
## Solution
- Add a new container attribute which when set does not emit
`BundleFromComponents`
## Testing
- Did you test these changes?
Yes, a new test was added.
- Are there any parts that need more testing?
Since `BundleFromComponents` is unsafe I made extra sure that I did not
misunderstand its purpose. As far as I can tell, _not_ implementing it
is ok.
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
Nope
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
I don't think the platform is relevant
---
One thing I am not sure about is how to document this? I'll gladly add
it
---------
Signed-off-by: Marcel Müller <neikos@neikos.email>
# Objective
`SystemSet`s are surprisingly rich and nuanced, but are extremely poorly
documented.
Fixes#19536.
## Solution
Explain the basic concept of system sets, how to create them, and give
some opinionated advice about their more advanced functionality.
## Follow-up
I'd like proper module level docs on system ordering that I can link to
here, but they don't exist. Punting to follow-up!
---------
Co-authored-by: theotherphil <phil.j.ellison@gmail.com>
# Objective
- Fixes#4381
## Solution
- Replace `component_access` with `component_access_set` when
determining conflicting systems during schedule building.
- All `component_access()` impls just forward to
`&component_access_set().combined_access`, so we are essentially trading
`Access::is_compatible` for `FilteredAccessSet::is_compatible`.
- `FilteredAccessSet::get_conflicts` internally calls
`combined_access.is_compatible` as the first step, so we can remove that
redundant check.
## Testing
- Un-ignored a previously failing test now that it passes!
- Ran the `build_schedule` benchmark and got basically no change in the
results. Perhaps are benchmarks are just not targetted towards this
situation.
```
$ critcmp main fix-ambiguity -f 'build_schedule'
group fix-ambiguity main
----- ------------- ----
build_schedule/1000_schedule 1.00 2.9±0.02s ? ?/sec 1.01 2.9±0.05s ? ?/sec
build_schedule/1000_schedule_no_constraints 1.02 48.3±1.48ms ? ?/sec 1.00 47.4±1.78ms ? ?/sec
build_schedule/100_schedule 1.00 9.9±0.17ms ? ?/sec 1.06 10.5±0.32ms ? ?/sec
build_schedule/100_schedule_no_constraints 1.00 804.7±21.85µs ? ?/sec 1.03 828.7±19.36µs ? ?/sec
build_schedule/500_schedule 1.00 451.7±7.25ms ? ?/sec 1.04 468.9±11.70ms ? ?/sec
build_schedule/500_schedule_no_constraints 1.02 12.7±0.46ms ? ?/sec 1.00 12.5±0.44ms ? ?/sec
```
# Objective
`Entity::PLACEHOLDER` acts as a magic number that will *probably* never
really exist, but it certainly could. And, `Entity` has a niche, so the
only reason to use `PLACEHOLDER` is as an alternative to `MaybeUninit`
that trades safety risks for logic risks.
As a result, bevy has generally advised against using `PLACEHOLDER`, but
we still use if for a lot internally. This pr starts removing internal
uses of it, starting from observers.
## Solution
Change all trigger target related types from `Entity` to
`Option<Entity>`
Small migration guide to come.
## Testing
CI
## Future Work
This turned a lot of code from
```rust
trigger.target()
```
to
```rust
trigger.target().unwrap()
```
The extra panic is no worse than before; it's just earlier than
panicking after passing the placeholder to something else.
But this is kinda annoying.
I would like to add a `TriggerMode` or something to `Event` that would
restrict what kinds of targets can be used for that event. Many events
like `Removed` etc, are always triggered with a target. We can make
those have a way to assume Some, etc. But I wanted to save that for a
future pr.
# Objective
At the moment, if someone wants to despawn all the children of an
entity, they would need to use `despawn_related::<Children>();`.
In my opinion, this makes a very common operation less easily
discoverable and require some understanding of Entity Relationships.
## Solution
Adding a `despawn_children ` makes a very simple, discoverable and
readable way to despawn all the children while maintaining cohesion with
other similar methods.
## Testing
The implementation itself is very simple as it simply wraps around
`despawn_related` with `Children` as the generic type.
I gave it a quick try by modifying the parenting example and it worked
as expected.
---------
Co-authored-by: Zachary Harrold <zac@harrold.com.au>
# Objective
- Fix https://github.com/bevyengine/bevy/issues/13843
- Clarify the difference between Mut and &mut when accessing query data
## Solution
- Mention `Mut` in `QueryData` docs as an example of a type that
implements this trait
- Give example of `iter_mut` vs `iter` access to `Mut` and `& mut`
parameters
## Testing
-
# Objective
Make the restrictions of `transmute_lens` and related functions clearer.
Related issue: https://github.com/bevyengine/bevy/issues/12156
Related PR: https://github.com/bevyengine/bevy/pull/12157
## Solution
* Make it clearer that the set of returned entities is a subset of those
from the original query
* Move description of read/write/required access to a table
* Reference the new table in `transmute_lens` docs from the other
`transmute_lens*` functions
## Testing
cargo doc --open locally to check this render correctly
---------
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
# Objective
- A preparation for the 'system as entities'
- The current system has a series of states such as `is_send`,
`is_exclusive`, `has_defered`, As `system as entites` landed, it may
have more states. Using Bitflags to unify all states is a more concise
and performant approach
## Solution
- Using Bitflags to unify system state.
# Objective
Deny missing docs for bevy_ecs_macros, towards
https://github.com/bevyengine/bevy/issues/3492.
## Solution
More docs of the form
```
/// Does the thing
fn do_the_thing() {}
```
But I don't think the derive macros are where anyone is going to be
looking for details of these concepts and deny(missing_docs) inevitably
results in some items having noddy docs.
# Objective
#19421 implemented `Ord` for `EntityGeneration` along the lines of [the
impl from
slotmap](https://docs.rs/slotmap/latest/src/slotmap/util.rs.html#8):
```rs
/// Returns if a is an older version than b, taking into account wrapping of
/// versions.
pub fn is_older_version(a: u32, b: u32) -> bool {
let diff = a.wrapping_sub(b);
diff >= (1 << 31)
}
```
But that PR and the slotmap impl are different:
**slotmap impl**
- if `(1u32 << 31)` is greater than `a.wrapping_sub(b)`, then `a` is
older than `b`
- if `(1u32 << 31)` is equal to `a.wrapping_sub(b)`, then `a` is older
than `b`
- if `(1u32 << 31)` is less than `a.wrapping_sub(b)`, then `a` is equal
or newer than `b`
**previous PR impl**
- if `(1u32 << 31)` is greater than `a.wrapping_sub(b)`, then `a` is
older than `b`
- if `(1u32 << 31)` is equal to `a.wrapping_sub(b)`, then `a` is equal
to `b` ⚠️
- if `(1u32 << 31)` is less than `a.wrapping_sub(b)`, then `a` is newer
than `b` ⚠️
This ordering is also not transitive, therefore it should not implement
`PartialOrd`.
## Solution
Fix the impl in a standalone method, remove the `Partialord`/`Ord`
implementation.
## Testing
Given the first impl was wrong and got past reviews, I think a new unit
test is justified.
# Objective
- Partial fix#19504
- As more features were added to Bevy ECS, certain core hot-path
function calls exceeded LLVM's automatic inlining threshold, leading to
significant performance regressions in some cases.
## Solution
- inline more functions.
## Performance
This brought nearly 3x improvement in Windows bench (using Sander's
testing code)
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
- #19504 showed a 11x regression in getting component values for
unregistered components. This pr should fix that and improve others a
little too.
- This is some cleanup work from #18173 .
## Solution
- Whenever we expect a component value to exist, we only care about
fully registered components, not queued to be registered components
since, for the value to exist, it must be registered.
- So we can use the faster `get_valid_*` instead of `get_*` in a lot of
places.
- Also found a bug where `valid_*` did not forward to `get_valid_*`
properly. That's fixed.
## Testing
CI
# Objective
Help users discover how to use `Option<T>` and `When<T>` to handle
failing parameters.
## Solution
Have the error message for a failed parameter mention that `Option<T>`
and `When<T>` can be used to handle the failure.
## Showcase
```
Encountered an error in system `system_name`: Parameter `Res<ResourceType>` failed validation: Resource does not exist
If this is an expected state, wrap the parameter in `Option<T>` and handle `None` when it happens, or wrap the parameter in `When<T>` to skip the system when it happens.
```
# Objective
- Enable hot patching systems with subsecond
- Fixes#19296
## Solution
- First commit is the naive thin layer
- Second commit only check the jump table when the code is hot patched
instead of on every system execution
- Depends on https://github.com/DioxusLabs/dioxus/pull/4153 for a nicer
API, but could be done without
- Everything in second commit is feature gated, it has no impact when
the feature is not enabled
## Testing
- Check dependencies without the feature enabled: nothing dioxus in tree
- Run the new example: text and color can be changed
---------
Co-authored-by: Jan Hohenheim <jan@hohenheim.ch>
Co-authored-by: JMS55 <47158642+JMS55@users.noreply.github.com>
# Objective
- Part 1 of #19454 .
- Split from PR #18860(authored by @notmd) for better review and limit
implementation impact. so all credit for this work belongs to @notmd .
## Solution
- Trigger `ArchetypeCreated ` when new archetype is createed
---------
Co-authored-by: mgi388 <135186256+mgi388@users.noreply.github.com>
# Objective
`Populated`, a loose wrapper around `Query`, does not implement
`IntoIterator`, requiring either a deref or `into_inner()` call to
access the `Query` and iterate over that.
## Solution
This pr implements `IntoIterator` for `Populated`, `&Populated`, and
`&mut Populated`, each of which forwards the call to the inner `Query`.
This allows the `Populated` to be used directly for any API that takes
an `impl IntoIterator`.
## Testing
`cargo test` was run on the `bevy_ecs` crate
```
test result: ok. 390 passed; 0 failed; 2 ignored; 0 measured; 0 filtered out; finished in 46.38s
```
# Objective
This is the first step of #19430 and is a follow up for #19132.
Now that `ArchetypeRow` has a niche, we can use `Option` instead of
needing `INVALID` everywhere.
This was especially concerning since `INVALID` *really was valid!*
Using options here made the code clearer and more data-driven.
## Solution
Replace all uses of `INVALID` entity locations (and archetype/table
rows) with `None`.
## Testing
CI
---------
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
Co-authored-by: François Mockers <francois.mockers@vleue.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
Fix#19324
## Solution
`EntityCloner` replaces required components when filtering. This is
unexpected when comparing with the way the rest of bevy handles required
components. This PR separates required components from explicit
components when filtering in `EntityClonerBuilder`.
## Testing
Added a regression test for this case.
# Objective
Fixes https://github.com/bevyengine/bevy/issues/17933
## Solution
Correct "value has changed'" in docs to "value has been added or mutably
dereferenced", with a note for emphasis copied from the docs for
Changed.
## Testing
-
# Objective
Recently the `u32` `Entity::generation` was replaced with the new
`EntityGeneration` in #19121.
This made meanings a lot more clear, and prevented accidental misuse.
One common misuse was assuming that `u32`s that were greater than others
came after those others.
Wrapping makes this assumption false.
When `EntityGeneration` was created, it retained the `u32` ordering,
which was useless at best and wrong at worst.
This pr fixes the ordering implementation, so new generations are
greater than older generations.
Some users were already accounting for this ordering issue (which was
still present in 0.16 and before) by manually accessing the `u32`
representation. This made migrating difficult for avian physics; see
[here](https://discord.com/channels/691052431525675048/749335865876021248/1377431569228103780).
I am generally of the opinion that this type should be kept opaque to
prevent accidental misuse.
As we find issues like this, the functionality should be added to
`EntityGeneration` directly.
## Solution
Fix the ordering implementation through `Ord`.
Alternatively, we could keep `Ord` the same and make a `cmp_age` method,
but I think this is better, even though sorting entity ids may be
*marginally* slower now (but more correct). This is a tradeoff.
## Testing
I improved documentation for aliasing and ordering, adding some doc
tests.
# Objective
There are several uninlined format args (seems to be in more formatting
macros and in more crates) that are not detected on stable, but are on
nightly.
## Solution
Fix them.
# Objective
#19047 added an `MaybeUninit` field to `EntityMeta`, but did not
guarantee that it will be initialized before access:
```rust
let mut world = World::new();
let id = world.entities().reserve_entity();
world.flush();
world.entity(id);
```
<details>
<summary>Miri Error</summary>
```
error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory
--> /home/vj/workspace/rust/bevy/crates/bevy_ecs/src/entity/mod.rs:1121:26
|
1121 | unsafe { meta.spawned_or_despawned.assume_init() }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ using uninitialized data, but this operation requires initialized memory
|
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
= note: BACKTRACE:
= note: inside closure at /home/vj/workspace/rust/bevy/crates/bevy_ecs/src/entity/mod.rs:1121:26: 1121:65
= note: inside `std::option::Option::<&bevy_ecs::entity::EntityMeta>::map::<bevy_ecs::entity::SpawnedOrDespawned, {closure@bevy_ecs::entity::Entities::entity_get_spawned_or_despawned::{closure#1}}>` at /home/vj/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/option.rs:1144:29: 1144:33
= note: inside `bevy_ecs::entity::Entities::entity_get_spawned_or_despawned` at /home/vj/workspace/rust/bevy/crates/bevy_ecs/src/entity/mod.rs:1112:9: 1122:15
= note: inside closure at /home/vj/workspace/rust/bevy/crates/bevy_ecs/src/entity/mod.rs:1094:13: 1094:57
= note: inside `bevy_ecs::change_detection::MaybeLocation::<std::option::Option<&std::panic::Location<'_>>>::new_with_flattened::<{closure@bevy_ecs::entity::Entities::entity_get_spawned_or_despawned_by::{closure#0}}>` at /home/vj/workspace/rust/bevy/crates/bevy_ecs/src/change_detection.rs:1371:20: 1371:24
= note: inside `bevy_ecs::entity::Entities::entity_get_spawned_or_despawned_by` at /home/vj/workspace/rust/bevy/crates/bevy_ecs/src/entity/mod.rs:1093:9: 1096:11
= note: inside `bevy_ecs::entity::Entities::entity_does_not_exist_error_details` at /home/vj/workspace/rust/bevy/crates/bevy_ecs/src/entity/mod.rs:1163:23: 1163:70
= note: inside `bevy_ecs::entity::EntityDoesNotExistError::new` at /home/vj/workspace/rust/bevy/crates/bevy_ecs/src/entity/mod.rs:1182:22: 1182:74
= note: inside `bevy_ecs::world::unsafe_world_cell::UnsafeWorldCell::<'_>::get_entity` at /home/vj/workspace/rust/bevy/crates/bevy_ecs/src/world/unsafe_world_cell.rs:368:20: 368:73
= note: inside `<bevy_ecs::entity::Entity as bevy_ecs::world::WorldEntityFetch>::fetch_ref` at /home/vj/workspace/rust/bevy/crates/bevy_ecs/src/world/entity_fetch.rs:207:21: 207:42
= note: inside `bevy_ecs::world::World::get_entity::<bevy_ecs::entity::Entity>` at /home/vj/workspace/rust/bevy/crates/bevy_ecs/src/world/mod.rs:911:18: 911:42
note: inside `main`
--> src/main.rs:12:15
|
12 | world.entity(id);
|
```
</details>
## Solution
- remove the existing `MaybeUninit` in `EntityMeta.spawned_or_despawned`
- initialize during flush. This is not needed for soundness, but not
doing this means we can't return a sensible location/tick for flushed
entities.
## Testing
Test via the snippet above (also added equivalent test).
---------
Co-authored-by: urben1680 <55257931+urben1680@users.noreply.github.com>
# Objective
Fixes#18905
## Solution
`world.commands().entity(target_entity).queue(command)` calls
`commands.with_entity` without an error handler, instead queue on
`Commands` with an error handler
## Testing
Added unit test
Co-authored-by: Heart <>
# Objective
Remove `ArchetypeComponentId` and `archetype_component_access`.
Following #16885, they are no longer used by the engine, so we can stop
spending time calculating them or space storing them.
## Solution
Remove `ArchetypeComponentId` and everything that touches it.
The `System::update_archetype_component_access` method no longer needs
to update `archetype_component_access`. We do still need to update query
caches, but we no longer need to do so *before* running the system. We'd
have to touch every caller anyway if we gave the method a better name,
so just remove `System::update_archetype_component_access` and
`SystemParam::new_archetype` entirely, and update the query cache in
`Query::get_param`.
The `Single` and `Populated` params also need their query caches updated
in `SystemParam::validate_param`, so change `validate_param` to take
`&mut Self::State` instead of `&Self::State`.
# Objective
- move SyncCell and SyncUnsafeCell to bevy_platform
## Solution
- move SyncCell and SyncUnsafeCell to bevy_platform
## Testing
- cargo clippy works
Fixes#19081.
Simply created a duplicate of the existing `insert_if_new` test, but
using sparse sets.
## Testing:
The test passes on main, but fails if #19059 is reverted.
# Objective
Fix some grammatical errors: it's -> its
Not the most useful commit in the world, but I saw a couple of these and
decided to fix the lot.
## Solution
-
## Testing
-
# Objective
Fix https://github.com/bevyengine/bevy/issues/13390
## Solution
The second parameter of the remove_reflect function is called
component_type_name in ReflectCommandExt but component_type_path in the
implementation for EntityCommands. Use component_type_path in both
places.
## Testing
None
# Objective
Fixes#18790.
Simpler alternative to #19195.
## Solution
As suggested by @PixelDust22, simply avoid overwriting the pass if the
schedule already has auto sync points enabled.
Leave pass logic untouched.
It still is probably a bad idea to add systems/set configs before
changing the build settings, but that is not important as long there are
no more complex build passes.
## Testing
Added a test.
---------
Co-authored-by: Thierry Berger <contact@thierryberger.com>
# Objective
Since #18704 is done, we can track the length of unique entity row
collections with only a `u32` and identify an index within that
collection with only a `NonMaxU32`. This leaves an opportunity for
performance improvements.
## Solution
- Use `EntityRow` in sparse sets.
- Change table, entity, and query lengths to be `u32` instead of
`usize`.
- Keep `batching` module `usize` based since that is reused for events,
which may exceed `u32::MAX`.
- Change according `Range<usize>` to `Range<u32>`. This is more
efficient and helps justify safety.
- Change `ArchetypeRow` and `TableRow` to wrap `NonMaxU32` instead of
`u32`.
Justifying `NonMaxU32::new_unchecked` everywhere is predicated on this
safety comment in `Entities::set`: "`location` must be valid for the
entity at `index` or immediately made valid afterwards before handing
control to unknown code." This ensures no entity is in two table rows
for example. That fact is used to argue uniqueness of the entity rows in
each table, archetype, sparse set, query, etc. So if there's no
duplicates, and a maximum total entities of `u32::MAX` none of the
corresponding row ids / indexes can exceed `NonMaxU32`.
## Testing
CI
---------
Co-authored-by: Christian Hughes <9044780+ItsDoot@users.noreply.github.com>
# Objective
- Currently, the error span for `get_struct_field` when encountering an
enum or union points to the macro invocation, rather than the `enum` or
`union` token. It also doesn't mention which macro reported the error.
## Solution
- Report the correct error span
- Add parameter for passing in the name of the macro invocation
## Testing
Bevy compiles fine with this change
## Migration Guide
```rs
// before
let fields = get_struct_fields(&ast.data);
// after
let fields = get_struct_fields(&ast.data, "derive(Bundle)");
```
---------
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
Hiya!
# Objective
- Remove upcasting methods that are no longer necessary since Rust 1.86.
- Cleanup the interned label code.
## Notes
- I didn't try to remove the upcasting methods from `bevy_reflect`, as
there appears to be some complexity related to remote type reflection.
- There are likely some other upcasting methods floating around.
## Testing
I ran the `breakout` example to check that the hashing/eq
implementations of the labels are still correct.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
similar to https://github.com/bevyengine/bevy/pull/12030
# Objective
`bevy_mod_debugdump` uses the `SystemTypeSet::system_type` to look up
constrains like `(system_1, system_2.after(system_1))`. For that it
needs to find the type id in `schedule.graph().systems()`
Now with systems being wrapped in an `InfallibleSystemWrapper` this
association was no longer possible.
## Solution
By forwarding the type id in `InfallibleSystemWrapper`,
`bevy_mod_debugdump` can resolve the dependencies as before, and the
wrapper is an unnoticable implementation detail.
## Testing
- `cargo test -p bevy_ecs`
I'm not sure what exactly could break otherwise.
# Objective
Now that `bevy_platform::cfg` is merged, we can start tidying up
features. This PR starts with `bevy_utils`.
## Solution
- Removed `serde` and `critical-section` features (they were just
re-exports of `bevy_platform` anyway)
- Removed `std`, `alloc` features, relying on `bevy_platform::cfg` to
check for availability.
- Added `parallel` feature to provide access to the `Parallel` type.
- Moved the `HashMap` type aliases into `map.rs` for better
organisation.
## Testing
- CI
# Objective
Remove errant "a" from docs.
(I'm assuming that this sort of trivial fix is easy enough to merge that
it's worth doing, but let me know if you'd prefer me to not bother.)
# Objective
allow serialization / deserialization on the `ChildOf` entity, for
example in network usage.
my usage was for the bevy_replicon crate, to replicate `ChildOf`.
## Solution
same implementation of serde as other types in the bevy repo
---------
Co-authored-by: Hennadii Chernyshchyk <genaloner@gmail.com>
## 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.
