# Objective
- In #9623 I forgot to change the `FromWorld` requirement for
`ReflectResource`, fix that;
- Fix#12129
## Solution
- Use the same approach as in #9623 to try using `FromReflect` and
falling back to the `ReflectFromWorld` contained in the `TypeRegistry`
provided
- Just reflect `Resource` on `State<S>` since now that's possible
without introducing new bounds.
---
## Changelog
- `ReflectResource`'s `FromType<T>` implementation no longer requires
`T: FromWorld`, but instead now requires `FromReflect`.
- `ReflectResource::insert`, `ReflectResource::apply_or_insert` and
`ReflectResource::copy` now take an extra `&TypeRegistry` parameter.
## Migration Guide
- Users of `#[reflect(Resource)]` will need to also implement/derive
`FromReflect` (should already be the default).
- Users of `#[reflect(Resource)]` may now want to also add `FromWorld`
to the list of reflected traits in case their `FromReflect`
implementation may fail.
- Users of `ReflectResource` will now need to pass a `&TypeRegistry` to
its `insert`, `apply_or_insert` and `copy` methods.
# Objective
`downcast-rs` is not used within bevy_ecs. This is probably a remnant
from before Schedule v3 landed, since stages needed the downcasting.
## Solution
Remove it.
# Objective
Memory usage optimisation
## Solution
`HashMap` and `HashSet`'s keys are immutable. So using mutable types
like `String`, `Vec<T>`, or `PathBuf` as a key is a waste of memory:
they have an extra `usize` for their capacity and may have spare
capacity.
This PR replaces these types by their immutable equivalents `Box<str>`,
`Box<[T]>`, and `Box<Path>`.
For more context, I recommend watching the [Use Arc Instead of
Vec](https://www.youtube.com/watch?v=A4cKi7PTJSs) video.
---------
Co-authored-by: James Liu <contact@jamessliu.com>
# Objective
- Avoid misspellings throughout the codebase by using
[`typos`](https://github.com/crate-ci/typos) in CI
Inspired by https://github.com/gfx-rs/wgpu/pull/5191
Typos is a minimal code speller written in rust that finds and corrects
spelling mistakes among source code.
- Fast enough to run on monorepos
- Low false positives so you can run on PRs
## Solution
- Use
[typos-action](https://github.com/marketplace/actions/typos-action) in
CI
- Add how to use typos in the Contribution Guide
---------
Co-authored-by: François <mockersf@gmail.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Joona Aalto <jondolf.dev@gmail.com>
# Objective
Right now when using egui, systems are inserted without any identifier
and to the root. I'd like to name those systems and insert them as
children to a root entity. This helps to keep the editor organized.
## Solution
- Although the `SystemId` is documented as an opaque type, examples
depicted above benefit from tear down of the abstraction.
---
## Changelog
### Added
- Implemented `From<SystemId>` for `Entity`
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Add Archetype::component_count utility method
# Objective
I wanted a method to count components on an archetype without iterating
over them.
## Solution
Added `Archetype::component_count`
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
The multi-threaded executor currently runs in a dedicated task on a
single thread. When a system finishes running, it needs to notify that
task and wait for the thread to be available and running before the
executor can process the completion.
See #8304
## Solution
Run the multi-threaded executor at the end of each system task. This
allows it to run immediately instead of needing to wait for the main
thread to wake up. Move the mutable executor state into a separate
struct and wrap it in a mutex so it can be shared among the worker
threads.
While this should be faster in theory, I don't actually know how to
measure the performance impact myself.
---------
Co-authored-by: James Liu <contact@jamessliu.com>
Co-authored-by: Mike <mike.hsu@gmail.com>
# Objective
- A tiny nit I noticed; I think the type of these function is
`EntityCommand`, not `Command`
Co-authored-by: Charles Bournhonesque <cbournhonesque@snapchat.com>
# Objective
- Add the new `-Zcheck-cfg` checks to catch more warnings
- Fixes#12091
## Solution
- Create a new `cfg-check` to the CI that runs `cargo check -Zcheck-cfg
--workspace` using cargo nightly (and fails if there are warnings)
- Fix all warnings generated by the new check
---
## Changelog
- Remove all redundant imports
- Fix cfg wasm32 targets
- Add 3 dead code exceptions (should StandardColor be unused?)
- Convert ios_simulator to a feature (I'm not sure if this is the right
way to do it, but the check complained before)
## Migration Guide
No breaking changes
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
Fixes#11821.
## Solution
* Run `System::apply_deferred` in `System::run` after executing the
system.
* Switch to using `System::run_unsafe` in `SingleThreadedExecutor` to
preserve the current behavior.
* Remove the `System::apply_deferred` in `SimpleExecutor` as it's now
redundant.
* Remove the `System::apply_deferred` when running one-shot systems, as
it's now redundant.
---
## Changelog
Changed: `System::run` will now immediately apply deferred system params
after running the system.
## Migration Guide
`System::run` will now always run `System::apply_deferred` immediately
after running the system now. If you were running systems and then
applying their deferred buffers at a later point in time, you can
eliminate the latter.
```rust
// in 0.13
system.run(world);
// .. sometime later ...
system.apply_deferred(world);
// in 0.14
system.run(world);
```
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
Since #9822, `SimpleExecutor` panics when an automatic sync point is
inserted:
```rust
let mut sched = Schedule::default();
sched.set_executor_kind(ExecutorKind::Simple);
sched.add_systems((|_: Commands| (), || ()).chain());
sched.run(&mut World::new());
```
```
System's param_state was not found. Did you forget to initialize this system before running it?
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Encountered a panic in system `bevy_ecs::schedule::executor::apply_deferred`!
```
## Solution
Don't try to run the `apply_deferred` system.
# Objective
- Part of #11590
- Fix `unsafe_op_in_unsafe_fn` for trivial cases in bevy_ecs
## Solution
Fix `unsafe_op_in_unsafe_fn` in bevy_ecs for trivial cases, i.e., add an
`unsafe` block when the safety comment already exists or add a comment
like "The invariants are uphold by the caller".
---------
Co-authored-by: James Liu <contact@jamessliu.com>
## Objective
Always have `some_system.into_system().type_id() ==
some_system.into_system_set().system_type().unwrap()`.
System sets have a `fn system_type() -> Option<TypeId>` that is
implemented by `SystemTypeSet` to returning the TypeId of the system's
function type. This was implemented in
https://github.com/bevyengine/bevy/pull/7715 and is used in
`bevy_mod_debugdump` to handle `.after(function)` constraints.
Back then, `System::type_id` always also returned the type id of the
function item, not of `FunctionSystem<M, F>`.
https://github.com/bevyengine/bevy/pull/11728 changes the behaviour of
`System::type_id` so that it returns the id of the
`FunctionSystem`/`ExclusiveFunctionSystem` wrapper, but it did not
change `SystemTypeSet::system_type`, so doing the lookup breaks in
`bevy_mod_debugdump`.
## Solution
Change `IntoSystemSet` for functions to return a
`SystemTypeSet<FunctionSystem>` /
`SystemTypeSet<ExclusiveFunctionSystem>` instead of `SystemTypeSet<F>`.
Fixes#12016.
Bump version after release
This PR has been auto-generated
Co-authored-by: Bevy Auto Releaser <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: François <mockersf@gmail.com>
# Objective
We deprecated quite a few APIs in 0.13. 0.13 has shipped already. It
should be OK to remove them in 0.14's release. Fixes#4059. Fixes#9011.
## Solution
Remove them.
# Objective
* Fixes#11932 (performance impact when stepping is disabled)
## Solution
The `Option<FixedBitSet>` argument added to `ScheduleExecutor::run()` in
#8453 caused a measurable performance impact even when stepping is
disabled. This can be seen by the benchmark of running `Schedule:run()`
on an empty schedule in a tight loop
(https://github.com/bevyengine/bevy/issues/11932#issuecomment-1950970236).
I was able to get the same performance results as on 0.12.1 by changing
the argument
`ScheduleExecutor::run()` from `Option<FixedBitSet>` to
`Option<&FixedBitSet>`. The down-side of this change is that
`Schedule::run()` now takes about 6% longer (3.7319 ms vs 3.9855ns) when
stepping is enabled
---
## Changelog
* Change `ScheduleExecutor::run()` `_skipped_systems` from
`Option<FixedBitSet>` to `Option<&FixedBitSet>`
* Added a few benchmarks to measure `Schedule::run()` performance with
various executors
# Objective
There's a repeating pattern of `ThreadLocal<Cell<Vec<T>>>` which is very
useful for low overhead, low contention multithreaded queues that have
cropped up in a few places in the engine. This pattern is surprisingly
useful when building deferred mutation across multiple threads, as noted
by it's use in `ParallelCommands`.
However, `ThreadLocal<Cell<Vec<T>>>` is not only a mouthful, it's also
hard to ensure the thread-local queue is replaced after it's been
temporarily removed from the `Cell`.
## Solution
Wrap the pattern into `bevy_utils::Parallel<T>` which codifies the
entire pattern and ensures the user follows the contract. Instead of
fetching indivdual cells, removing the value, mutating it, and replacing
it, `Parallel::get` returns a `ParRef<'a, T>` which contains the
temporarily removed value and a reference back to the cell, and will
write the mutated value back to the cell upon being dropped.
I would like to use this to simplify the remaining part of #4899 that
has not been adopted/merged.
---
## Changelog
TODO
---------
Co-authored-by: Joseph <21144246+JoJoJet@users.noreply.github.com>
# Objective
`update_archetype_component_access` was removed from queries in #9774,
but some documentation still refers to it.
## Solution
Update the documentation. Since a bunch of these were in SAFETY comments
it would be nice if someone who knows the details better could check
that the rest of those comments are still valid.
# Objective
- There are multiple instances of `let Some(x) = ... else { None };`
throughout the project.
- Because `Option<T>` implements
[`Try`](https://doc.rust-lang.org/stable/std/ops/trait.Try.html), it can
use the question mark `?` operator.
## Solution
- Use question mark operator instead of `let Some(x) = ... else { None
}`.
---
There was another PR that did a similar thing a few weeks ago, but I
couldn't find it.
# Objective
At the start of every schedule run, there's currently a guaranteed piece
of overhead as the async executor spawns the MultithreadeExecutor task
onto one of the ComputeTaskPool threads.
## Solution
Poll the executor once to immediately schedule systems without waiting
for the async executor, then spawn the task if and only if the executor
does not immediately terminate.
On a similar note, having the executor task immediately start executing
a system in the same async task might yield similar results over a
broader set of cases. However, this might be more involved, and may need
a solution like #8304.
# Objective
When applying a command, we currently use double indirection for the
world reference `&mut Option<&mut World>`. Since this is used across a
`fn` pointer boundary, this can't get optimized away.
## Solution
Reborrow the world reference and pass `Option<&mut World>` instead.
# Objective
Bevy's change detection functionality is invaluable for writing robust
apps, but it only works in the context of systems and exclusive systems.
Oftentimes it is necessary to detect changes made in earlier code
without having to place the code in separate systems, but it is not
currently possible to do so since there is no way to set the value of
`World::last_change_tick`.
`World::clear_trackers` allows you to update the change tick, but this
has unintended side effects, since it irreversibly affects the behavior
of change and removal detection for the entire app.
## Solution
Add a method `World::last_change_tick_scope`. This allows you to set
`last_change_tick` to a specific value for a region of code. To ensure
that misuse doesn't break unrelated functions, we restore the world's
original change tick at the end of the provided scope.
### Example
A function that uses this to run an update loop repeatedly, allowing
each iteration of the loop to react to changes made in the previous loop
iteration.
```rust
fn update_loop(
world: &mut World,
mut update_fn: impl FnMut(&mut World) -> std::ops::ControlFlow<()>,
) {
let mut last_change_tick = world.last_change_tick();
// Repeatedly run the update function until it requests a break.
loop {
// Update once.
let control_flow = world.last_change_tick_scope(last_change_tick, |world| {
update_fn(world)
});
// End the loop when the closure returns `ControlFlow::Break`.
if control_flow.is_break() {
break;
}
// Increment the change tick so the next update can detect changes from this update.
last_change_tick = world.change_tick();
world.increment_change_tick();
}
}
```
---
## Changelog
+ Added `World::last_change_tick_scope`, which allows you to specify the
reference for change detection within a certain scope.
# Objective
Reduce the size of `bevy_utils`
(https://github.com/bevyengine/bevy/issues/11478)
## Solution
Move `EntityHash` related types into `bevy_ecs`. This also allows us
access to `Entity`, which means we no longer need `EntityHashMap`'s
first generic argument.
---
## Changelog
- Moved `bevy::utils::{EntityHash, EntityHasher, EntityHashMap,
EntityHashSet}` into `bevy::ecs::entity::hash` .
- Removed `EntityHashMap`'s first generic argument. It is now hardcoded
to always be `Entity`.
## Migration Guide
- Uses of `bevy::utils::{EntityHash, EntityHasher, EntityHashMap,
EntityHashSet}` now have to be imported from `bevy::ecs::entity::hash`.
- Uses of `EntityHashMap` no longer have to specify the first generic
parameter. It is now hardcoded to always be `Entity`.
# Objective
It would be useful to be able to inspect a `QueryState`'s accesses so we
can detect when the data it accesses changes without having to iterate
it. However there are two things preventing this:
* These accesses are unnecessarily encapsulated.
* `Has<T>` indirectly accesses `T`, but does not register it.
## Solution
* Expose accesses and matches used by `QueryState`.
* Add the notion of "archetypal" accesses, which are not accessed
directly, but whose presence in an archetype affects a query result.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
I want to keep track of despawned entities.
I am aware of
[`RemovedComponents`](https://docs.rs/bevy/0.12.1/bevy/ecs/prelude/struct.RemovedComponents.html).
However, the docs don't explicitly mention that despawned entities are
also included in this event iterator.
I searched through the bevy tests to find `removal_tracking` in
`crates/bevy_ecs/src/system/mod.rs` that confirmed the behavior:
```rust
...
assert_eq!(
removed_i32.read().collect::<Vec<_>>(),
&[despawned.0],
"despawning causes the correct entity to show up in the 'RemovedComponent' system parameter."
);
...
```
## Solution
- Explicitly mention this behavior in docs.
[`ScheduleLabel`] derive macro uses "ScheduleName" as the trait name by
mistake. This only affects the error message when a user tries to use
the derive macro on a union type. No other code is affected.
# Objective
- Fixes#11679
## Solution
- Added `IntoSystem::system_type_id` which returns the equivalent of
`system.into_system().type_id()` without construction. This allows for
getting the `TypeId` of functions (a function is an unnamed type and
therefore you cannot call `TypeId::of::<apply_deferred::System>()`)
- Added default implementation of `System::type_id` to ensure
consistency between implementations. Some returned `Self`, while others
were returning an inner value instead. This ensures consistency with
`IntoSystem::system_type_id`.
## Migration Guide
If you use `System::type_id()` on function systems (exclusive or not),
ensure you are comparing its value to other `System::type_id()` calls,
or `IntoSystem::system_type_id()`.
This code wont require any changes, because `IntoSystem`'s are directly
compared to each other.
```rust
fn test_system() {}
let type_id = test_system.type_id();
// ...
// No change required
assert_eq!(test_system.type_id(), type_id);
```
Likewise, this code wont, because `System`'s are directly compared.
```rust
fn test_system() {}
let type_id = IntoSystem::into_system(test_system).type_id();
// ...
// No change required
assert_eq!(IntoSystem::into_system(test_system).type_id(), type_id);
```
The below _does_ require a change, since you're comparing a `System`
type to a `IntoSystem` type.
```rust
fn test_system() {}
// Before
assert_eq!(test_system.type_id(), IntoSystem::into_system(test_system).type_id());
// After
assert_eq!(test_system.system_type_id(), IntoSystem::into_system(test_system).type_id());
```
# Objective
- (Partially) Fixes#9904
- Acts on #9910
## Solution
- Deprecated the relevant methods from `Query`, cascading changes as
required across Bevy.
---
## Changelog
- Deprecated `QueryState::get_component_unchecked_mut` method
- Deprecated `Query::get_component` method
- Deprecated `Query::get_component_mut` method
- Deprecated `Query::component` method
- Deprecated `Query::component_mut` method
- Deprecated `Query::get_component_unchecked_mut` method
## Migration Guide
### `QueryState::get_component_unchecked_mut`
Use `QueryState::get_unchecked_manual` and select for the exact
component based on the structure of the exact query as required.
### `Query::(get_)component(_unchecked)(_mut)`
Use `Query::get` and select for the exact component based on the
structure of the exact query as required.
- For mutable access (`_mut`), use `Query::get_mut`
- For unchecked access (`_unchecked`), use `Query::get_unchecked`
- For panic variants (non-`get_`), add `.unwrap()`
## Notes
- `QueryComponentError` can be removed once these deprecated methods are
also removed. Due to an interaction with `thiserror`'s derive macro, it
is not marked as deprecated.
Use `TypeIdMap<T>` instead of `HashMap<TypeId, T>`
- ~~`TypeIdMap` was in `bevy_ecs`. I've kept it there because of
#11478~~
- ~~I haven't swapped `bevy_reflect` over because it doesn't depend on
`bevy_ecs`, but I'd also be happy with moving `TypeIdMap` to
`bevy_utils` and then adding a dependency to that~~
- ~~this is a slight change in the public API of
`DrawFunctionsInternal`, does this need to go in the changelog?~~
## Changelog
- moved `TypeIdMap` to `bevy_utils`
- changed `DrawFunctionsInternal::indices` to `TypeIdMap`
## Migration Guide
- `TypeIdMap` now lives in `bevy_utils`
- `DrawFunctionsInternal::indices` now uses a `TypeIdMap`.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
Currently the `missing_docs` lint is allowed-by-default and enabled at
crate level when their documentations is complete (see #3492).
This PR proposes to inverse this logic by making `missing_docs`
warn-by-default and mark crates with imcomplete docs allowed.
## Solution
Makes `missing_docs` warn at workspace level and allowed at crate level
when the docs is imcomplete.
# Objective
Add interactive system debugging capabilities to bevy, providing
step/break/continue style capabilities to running system schedules.
* Original implementation: #8063
- `ignore_stepping()` everywhere was too much complexity
* Schedule-config & Resource discussion: #8168
- Decided on selective adding of Schedules & Resource-based control
## Solution
Created `Stepping` Resource. This resource can be used to enable
stepping on a per-schedule basis. Systems within schedules can be
individually configured to:
* AlwaysRun: Ignore any stepping state and run every frame
* NeverRun: Never run while stepping is enabled
- this allows for disabling of systems while debugging
* Break: If we're running the full frame, stop before this system is run
Stepping provides two modes of execution that reflect traditional
debuggers:
* Step-based: Only execute one system at a time
* Continue/Break: Run all systems, but stop before running a system
marked as Break
### Demo
https://user-images.githubusercontent.com/857742/233630981-99f3bbda-9ca6-4cc4-a00f-171c4946dc47.mov
Breakout has been modified to use Stepping. The game runs normally for a
couple of seconds, then stepping is enabled and the game appears to
pause. A list of Schedules & Systems appears with a cursor at the first
System in the list. The demo then steps forward full frames using the
spacebar until the ball is about to hit a brick. Then we step system by
system as the ball impacts a brick, showing the cursor moving through
the individual systems. Finally the demo switches back to frame stepping
as the ball changes course.
### Limitations
Due to architectural constraints in bevy, there are some cases systems
stepping will not function as a user would expect.
#### Event-driven systems
Stepping does not support systems that are driven by `Event`s as events
are flushed after 1-2 frames. Although game systems are not running
while stepping, ignored systems are still running every frame, so events
will be flushed.
This presents to the user as stepping the event-driven system never
executes the system. It does execute, but the events have already been
flushed.
This can be resolved by changing event handling to use a buffer for
events, and only dropping an event once all readers have read it.
The work-around to allow these systems to properly execute during
stepping is to have them ignore stepping:
`app.add_systems(event_driven_system.ignore_stepping())`. This was done
in the breakout example to ensure sound played even while stepping.
#### Conditional Systems
When a system is stepped, it is given an opportunity to run. If the
conditions of the system say it should not run, it will not.
Similar to Event-driven systems, if a system is conditional, and that
condition is only true for a very small time window, then stepping the
system may not execute the system. This includes depending on any sort
of external clock.
This exhibits to the user as the system not always running when it is
stepped.
A solution to this limitation is to ensure any conditions are consistent
while stepping is enabled. For example, all systems that modify any
state the condition uses should also enable stepping.
#### State-transition Systems
Stepping is configured on the per-`Schedule` level, requiring the user
to have a `ScheduleLabel`.
To support state-transition systems, bevy generates needed schedules
dynamically. Currently it’s very difficult (if not impossible, I haven’t
verified) for the user to get the labels for these schedules.
Without ready access to the dynamically generated schedules, and a
resolution for the `Event` lifetime, **stepping of the state-transition
systems is not supported**
---
## Changelog
- `Schedule::run()` updated to consult `Stepping` Resource to determine
which Systems to run each frame
- Added `Schedule.label` as a `BoxedSystemLabel`, along with supporting
`Schedule::set_label()` and `Schedule::label()` methods
- `Stepping` needed to know which `Schedule` was running, and prior to
this PR, `Schedule` didn't track its own label
- Would have preferred to add `Schedule::with_label()` and remove
`Schedule::new()`, but this PR touches enough already
- Added calls to `Schedule.set_label()` to `App` and `World` as needed
- Added `Stepping` resource
- Added `Stepping::begin_frame()` system to `MainSchedulePlugin`
- Run before `Main::run_main()`
- Notifies any `Stepping` Resource a new render frame is starting
## Migration Guide
- Add a call to `Schedule::set_label()` for any custom `Schedule`
- This is only required if the `Schedule` will be stepped
---------
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
# Objective
Fix an issue where events are not being dropped after being read. I
believe #10077 introduced this issue. The code currently works as
follows:
1. `EventUpdateSignal` is **shared for all event types**
2. During the fixed update phase, `EventUpdateSignal` is set to true
3. `event_update_system`, **unique per event type**, runs to update
Events<T>
4. `event_update_system` reads value of `EventUpdateSignal` to check if
it should update, and then **resets** the value to false
If there are multiple event types, the first `event_update_system` run
will reset the shared `EventUpdateSignal` signal, preventing other
events from being cleared.
## Solution
I've updated the code to have separate signals per event type and added
a shared signal to notify all systems that the time plugin is installed.
## Changelog
- Fixed bug where events were not being dropped
# Objective
- Deriving `Reflect` for some public ChangeDetection/Tick structs in
bevy_ecs
---------
Co-authored-by: Charles Bournhonesque <cbournhonesque@snapchat.com>
# Objective
Fixes: https://github.com/bevyengine/bevy/issues/11549
Add a doctest example of what a custom implementation of an
`EntityMapper` would look like.
(need to wait until https://github.com/bevyengine/bevy/pull/11428 is
merged)
---------
Co-authored-by: Charles Bournhonesque <cbournhonesque@snapchat.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Hennadii Chernyshchyk <genaloner@gmail.com>
# Objective
- Sending and receiving events of the same type in the same system is a
reasonably common need, generally due to event filtering.
- However, actually doing so is non-trivial, as the borrow checker
simultaneous hates mutable and immutable access.
## Solution
- Demonstrate two sensible patterns for doing so.
- Update the `ManualEventReader` docs to be more clear and link to this
example.
---------
Co-authored-by: Alice Cecile <alice.i.cecil@gmail.com>
Co-authored-by: Joona Aalto <jondolf.dev@gmail.com>
Co-authored-by: ickk <git@ickk.io>
# Objective
My motivation are to resolve some of the issues I describe in this
[PR](https://github.com/bevyengine/bevy/issues/11415):
- not being able to easily mapping entities because the current
EntityMapper requires `&mut World` access
- not being able to create my own `EntityMapper` because some components
(`Parent` or `Children`) do not provide any public way of modifying the
inner entities
This PR makes the `MapEntities` trait accept a generic type that
implements `Mapper` to perform the mapping.
This means we don't need to use `EntityMapper` to perform our mapping,
we can use any type that implements `Mapper`. Basically this change is
very similar to what `serde` does. Instead of specifying directly how to
map entities for a given type, we have 2 distinct steps:
- the user implements `MapEntities` to define how the type will be
traversed and which `Entity`s will be mapped
- the `Mapper` defines how the mapping is actually done
This is similar to the distinction between `Serialize` (`MapEntities`)
and `Serializer` (`Mapper`).
This allows networking library to map entities without having to use the
existing `EntityMapper` (which requires `&mut World` access and the use
of `world_scope()`)
## Migration Guide
- The existing `EntityMapper` (notably used to replicate `Scenes` across
different `World`s) has been renamed to `SceneEntityMapper`
- The `MapEntities` trait now works with a generic `EntityMapper`
instead of the specific struct `EntityMapper`.
Calls to `fn map_entities(&mut self, entity_mapper: &mut EntityMapper)`
need to be updated to
`fn map_entities<M: EntityMapper>(&mut self, entity_mapper: &mut M)`
- The new trait `EntityMapper` has been added to the prelude
---------
Co-authored-by: Charles Bournhonesque <cbournhonesque@snapchat.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: UkoeHB <37489173+UkoeHB@users.noreply.github.com>
# Objective
Fixes#11311
## Solution
Adds an example to the documentation for `par_iter_mut`. I didn't add
any examples to `par_iter`, because I couldn't think of a good example
and I figure users can infer that `par_iter` and `par_iter_mut` are
similar.
# Objective
It's sometimes desirable to get a `Res<T>` rather than `&T` from
`World::get_resource`.
Alternative to #9940, partly adresses #9926
## Solution
added additional methods to `World` and `UnsafeWorldCell` to retrieve a
resource wrapped in a `Res`.
- `UnsafeWorldCell::get_resource_ref`
- `World::get_resource_ref`
- `World::resource_ref`
I can change it so `World::resource_mut` returns `ResMut` instead of
`Mut` as well if that's desired, but that could also be added later in a
seperate pr.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Mike <mike.hsu@gmail.com>
Co-authored-by: MinerSebas <66798382+MinerSebas@users.noreply.github.com>
# Objective
While working on #11527 I spotted that the internal field for the label
of a `Schedule` is called `name`. Using `label` seems more in line with
the other naming across Bevy.
## Solution
Renaming the field was straightforward since it's not exposed outside of
the module. This also means a changelog or migration guide isn't
necessary.
# Objective
- `World::get_resource`'s comment on it's `unsafe` usage meant to say
"mutably" but instead said "immutably."
- Fixes#11430.
## Solution
- Replace "immutably" with "mutably."
# Objective
It would be convenient to be able to call functions with `Commands` as a
parameter without having to move your own instance of `Commands`. Since
this struct is composed entirely of references, we can easily get an
owned instance of `Commands` by shortening the lifetime.
## Solution
Add `Commands::reborrow`, `EntiyCommands::reborrow`, and
`Deferred::reborrow`, which returns an owned version of themselves with
a shorter lifetime.
Remove unnecessary lifetimes from `EntityCommands`. The `'w` and `'s`
lifetimes only have to be separate for `Commands` because it's used as a
`SystemParam` -- this is not the case for `EntityCommands`.
---
## Changelog
Added `Commands::reborrow`. This is useful if you have `&mut Commands`
but need `Commands`. Also added `EntityCommands::reborrow` and
`Deferred:reborrow` which serve the same purpose.
## Migration Guide
The lifetimes for `EntityCommands` have been simplified.
```rust
// Before (Bevy 0.12)
struct MyStruct<'w, 's, 'a> {
commands: EntityCommands<'w, 's, 'a>,
}
// After (Bevy 0.13)
struct MyStruct<'a> {
commands: EntityCommands<'a>,
}
```
The method `EntityCommands::commands` now returns `Commands` rather than
`&mut Commands`.
```rust
// Before (Bevy 0.12)
let commands = entity_commands.commands();
commands.spawn(...);
// After (Bevy 0.13)
let mut commands = entity_commands.commands();
commands.spawn(...);
```
# Objective
Document a few common cases of which lifetime is required when using
SystemParam Derive
## Solution
Added a table in the doc comment
---------
Co-authored-by: laund <me@laund.moe>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
Adjust bevy internals to utilize `Option<Res<State<S>>>` instead of
`Res<State<S>>`, to allow for adding/removing states at runtime and
avoid unexpected panics.
As requested here:
https://github.com/bevyengine/bevy/pull/10088#issuecomment-1869185413
---
## Changelog
- Changed the use of `world.resource`/`world.resource_mut` to
`world.get_resource`/`world.get_resource_mut` in the
`run_enter_schedule` and `apply_state_transition` systems and handled
the `None` option.
- `in_state` now returns a ` FnMut(Option<Res<State<S>>>) -> bool +
Clone`, returning `false` if the resource doesn't exist.
- `state_exists_and_equals` was marked as deprecated, and now just runs
and returns `in_state`, since their bevhaviour is now identical
- `state_changed` now takes an `Option<Res<State<S>>>` and returns
`false` if it does not exist.
I would like to remove `state_exists_and_equals` fully, but wanted to
ensure that is acceptable before doing so.
---------
Co-authored-by: Mike <mike.hsu@gmail.com>
# Objective
- `FromType<T>` for `ReflectComponent` and `ReflectBundle` currently
require `T: FromWorld` for two reasons:
- they include a `from_world` method;
- they create dummy `T`s using `FromWorld` and then `apply` a `&dyn
Reflect` to it to simulate `FromReflect`.
- However `FromWorld`/`Default` may be difficult/weird/impractical to
implement, while `FromReflect` is easier and also more natural for the
job.
- See also
https://discord.com/channels/691052431525675048/1146022009554337792
## Solution
- Split `from_world` from `ReflectComponent` and `ReflectBundle` into
its own `ReflectFromWorld` struct.
- Replace the requirement on `FromWorld` in `ReflectComponent` and
`ReflectBundle` with `FromReflect`
---
## Changelog
- `ReflectComponent` and `ReflectBundle` no longer offer a `from_world`
method.
- `ReflectComponent` and `ReflectBundle`'s `FromType<T>` implementation
no longer requires `T: FromWorld`, but now requires `FromReflect`.
- `ReflectComponent::insert`, `ReflectComponent::apply_or_insert` and
`ReflectComponent::copy` now take an extra `&TypeRegistry` parameter.
- There is now a new `ReflectFromWorld` struct.
## Migration Guide
- Existing uses of `ReflectComponent::from_world` and
`ReflectBundle::from_world` will have to be changed to
`ReflectFromWorld::from_world`.
- Users of `#[reflect(Component)]` and `#[reflect(Bundle)]` will need to
also implement/derive `FromReflect`.
- Users of `#[reflect(Component)]` and `#[reflect(Bundle)]` may now want
to also add `FromWorld` to the list of reflected traits in case their
`FromReflect` implementation may fail.
- Users of `ReflectComponent` will now need to pass a `&TypeRegistry` to
its `insert`, `apply_or_insert` and `copy` methods.
# Objective
- Add methods to get Change Ticks for a given resource by type or
ComponentId
- Fixes#11390
The `is_resource_id_changed` requested in the Issue already exists, this
adds their request for `get_resource_change_ticks`
## Solution
- Added two methods to get change ticks by Type or ComponentId
# Objective
Expand the existing `Query` API to support more dynamic use cases i.e.
scripting.
## Prior Art
- #6390
- #8308
- #10037
## Solution
- Create a `QueryBuilder` with runtime methods to define the set of
component accesses for a built query.
- Create new `WorldQueryData` implementations `FilteredEntityMut` and
`FilteredEntityRef` as variants of `EntityMut` and `EntityRef` that
provide run time checked access to the components included in a given
query.
- Add new methods to `Query` to create "query lens" with a subset of the
access of the initial query.
### Query Builder
The `QueryBuilder` API allows you to define a query at runtime. At it's
most basic use it will simply create a query with the corresponding type
signature:
```rust
let query = QueryBuilder::<Entity, With<A>>::new(&mut world).build();
// is equivalent to
let query = QueryState::<Entity, With<A>>::new(&mut world);
```
Before calling `.build()` you also have the opportunity to add
additional accesses and filters. Here is a simple example where we add
additional filter terms:
```rust
let entity_a = world.spawn((A(0), B(0))).id();
let entity_b = world.spawn((A(0), C(0))).id();
let mut query_a = QueryBuilder::<Entity>::new(&mut world)
.with::<A>()
.without::<C>()
.build();
assert_eq!(entity_a, query_a.single(&world));
```
This alone is useful in that allows you to decide which archetypes your
query will match at runtime. However it is also very limited, consider a
case like the following:
```rust
let query_a = QueryBuilder::<&A>::new(&mut world)
// Add an additional access
.data::<&B>()
.build();
```
This will grant the query an additional read access to component B
however we have no way of accessing the data while iterating as the type
signature still only includes &A. For an even more concrete example of
this consider dynamic components:
```rust
let query_a = QueryBuilder::<Entity>::new(&mut world)
// Adding a filter is easy since it doesn't need be read later
.with_id(component_id_a)
// How do I access the data of this component?
.ref_id(component_id_b)
.build();
```
With this in mind the `QueryBuilder` API seems somewhat incomplete by
itself, we need some way method of accessing the components dynamically.
So here's one:
### Query Transmutation
If the problem is not having the component in the type signature why not
just add it? This PR also adds transmute methods to `QueryBuilder` and
`QueryState`. Here's a simple example:
```rust
world.spawn(A(0));
world.spawn((A(1), B(0)));
let mut query = QueryBuilder::<()>::new(&mut world)
.with::<B>()
.transmute::<&A>()
.build();
query.iter(&world).for_each(|a| assert_eq!(a.0, 1));
```
The `QueryState` and `QueryBuilder` transmute methods look quite similar
but are different in one respect. Transmuting a builder will always
succeed as it will just add the additional accesses needed for the new
terms if they weren't already included. Transmuting a `QueryState` will
panic in the case that the new type signature would give it access it
didn't already have, for example:
```rust
let query = QueryState::<&A, Option<&B>>::new(&mut world);
/// This is fine, the access for Option<&A> is less restrictive than &A
query.transmute::<Option<&A>>(&world);
/// Oh no, this would allow access to &B on entities that might not have it, so it panics
query.transmute::<&B>(&world);
/// This is right out
query.transmute::<&C>(&world);
```
This is quite an appealing API to also have available on `Query` however
it does pose one additional wrinkle: In order to to change the iterator
we need to create a new `QueryState` to back it. `Query` doesn't own
it's own state though, it just borrows it, so we need a place to borrow
it from. This is why `QueryLens` exists, it is a place to store the new
state so it can be borrowed when you call `.query()` leaving you with an
API like this:
```rust
fn function_that_takes_a_query(query: &Query<&A>) {
// ...
}
fn system(query: Query<(&A, &B)>) {
let lens = query.transmute_lens::<&A>();
let q = lens.query();
function_that_takes_a_query(&q);
}
```
Now you may be thinking: Hey, wait a second, you introduced the problem
with dynamic components and then described a solution that only works
for static components! Ok, you got me, I guess we need a bit more:
### Filtered Entity References
Currently the only way you can access dynamic components on entities
through a query is with either `EntityMut` or `EntityRef`, however these
can access all components and so conflict with all other accesses. This
PR introduces `FilteredEntityMut` and `FilteredEntityRef` as
alternatives that have additional runtime checking to prevent accessing
components that you shouldn't. This way you can build a query with a
`QueryBuilder` and actually access the components you asked for:
```rust
let mut query = QueryBuilder::<FilteredEntityRef>::new(&mut world)
.ref_id(component_id_a)
.with(component_id_b)
.build();
let entity_ref = query.single(&world);
// Returns Some(Ptr) as we have that component and are allowed to read it
let a = entity_ref.get_by_id(component_id_a);
// Will return None even though the entity does have the component, as we are not allowed to read it
let b = entity_ref.get_by_id(component_id_b);
```
For the most part these new structs have the exact same methods as their
non-filtered equivalents.
Putting all of this together we can do some truly dynamic ECS queries,
check out the `dynamic` example to see it in action:
```
Commands:
comp, c Create new components
spawn, s Spawn entities
query, q Query for entities
Enter a command with no parameters for usage.
> c A, B, C, Data 4
Component A created with id: 0
Component B created with id: 1
Component C created with id: 2
Component Data created with id: 3
> s A, B, Data 1
Entity spawned with id: 0v0
> s A, C, Data 0
Entity spawned with id: 1v0
> q &Data
0v0: Data: [1, 0, 0, 0]
1v0: Data: [0, 0, 0, 0]
> q B, &mut Data
0v0: Data: [2, 1, 1, 1]
> q B || C, &Data
0v0: Data: [2, 1, 1, 1]
1v0: Data: [0, 0, 0, 0]
```
## Changelog
- Add new `transmute_lens` methods to `Query`.
- Add new types `QueryBuilder`, `FilteredEntityMut`, `FilteredEntityRef`
and `QueryLens`
- `update_archetype_component_access` has been removed, archetype
component accesses are now determined by the accesses set in
`update_component_access`
- Added method `set_access` to `WorldQuery`, this is called before
`update_component_access` for queries that have a restricted set of
accesses, such as those built by `QueryBuilder` or `QueryLens`. This is
primarily used by the `FilteredEntity*` variants and has an empty trait
implementation.
- Added method `get_state` to `WorldQuery` as a fallible version of
`init_state` when you don't have `&mut World` access.
## Future Work
Improve performance of `FilteredEntityMut` and `FilteredEntityRef`,
currently they have to determine the accesses a query has in a given
archetype during iteration which is far from ideal, especially since we
already did the work when matching the archetype in the first place. To
avoid making more internal API changes I have left it out of this PR.
---------
Co-authored-by: Mike Hsu <mike.hsu@gmail.com>
# Objective
- Update async channel to v2.
## Solution
- async channel doesn't support `send_blocking` on wasm anymore. So
don't compile the pipelined rendering plugin on wasm anymore.
- Replaces https://github.com/bevyengine/bevy/pull/10405
## Migration Guide
- The `PipelinedRendering` plugin is no longer exported on wasm. If you
are including it in your wasm builds you should remove it.
```rust
#[cfg(all(not(target_arch = "wasm32"))]
app.add_plugins(bevy_render::pipelined_rendering::PipelinedRenderingPlugin);
```
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Based on discussion after #11268 was merged:
Instead of panicking should the impl of `TypeId::hash` change
significantly, have a fallback and detect this in a test.
# Objective
`TypeId` contains a high-quality hash. Whenever a lookup based on a
`TypeId` is performed (e.g. to insert/remove components), the hash is
run through a second hash function. This is unnecessary.
## Solution
Skip re-hashing `TypeId`s.
In my
[testing](https://gist.github.com/SpecificProtagonist/4b49ad74c6b82b0aedd3b4ea35121be8),
this improves lookup performance consistently by 10%-15% (of course, the
lookup is only a small part of e.g. a bundle insertion).
# Objective
The purpose of this PR is to begin putting together a unified identifier
structure that can be used by entities and later components (as
entities) as well as relationship pairs for relations, to enable all of
these to be able to use the same storages. For the moment, to keep
things small and focused, only `Entity` is being changed to make use of
the new `Identifier` type, keeping `Entity`'s API and
serialization/deserialization the same. Further changes are for
follow-up PRs.
## Solution
`Identifier` is a wrapper around `u64` split into two `u32` segments
with the idea of being generalised to not impose restrictions on
variants. That is for `Entity` to do. Instead, it is a general API for
taking bits to then merge and map into a `u64` integer. It exposes
low/high methods to return the two value portions as `u32` integers,
with then the MSB masked for usage as a type flag, enabling entity kind
discrimination and future activation/deactivation semantics.
The layout in this PR for `Identifier` is described as below, going from
MSB -> LSB.
```
|F| High value | Low value |
|_|_______________________________|________________________________|
|1| 31 | 32 |
F = Bit Flags
```
The high component in this implementation has only 31 bits, but that
still leaves 2^31 or 2,147,483,648 values that can be stored still, more
than enough for any generation/relation kinds/etc usage. The low part is
a full 32-bit index. The flags allow for 1 bit to be used for
entity/pair discrimination, as these have different usages for the
low/high portions of the `Identifier`. More bits can be reserved for
more variants or activation/deactivation purposes, but this currently
has no use in bevy.
More bits could be reserved for future features at the cost of bits for
the high component, so how much to reserve is up for discussion. Also,
naming of the struct and methods are also subject to further
bikeshedding and feedback.
Also, because IDs can have different variants, I wonder if
`Entity::from_bits` needs to return a `Result` instead of potentially
panicking on receiving an invalid ID.
PR is provided as an early WIP to obtain feedback and notes on whether
this approach is viable.
---
## Changelog
### Added
New `Identifier` struct for unifying IDs.
### Changed
`Entity` changed to use new `Identifier`/`IdentifierMask` as the
underlying ID logic.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: vero <email@atlasdostal.com>
# Objective
This dependency is seemingly no longer used directly after #7267.
Unfortunately, this doesn't fix us having versions of `event-listener`
in our tree.
Closes#10654
## Solution
Remove it, see if anything breaks.
# Objective
- Implements change described in
https://github.com/bevyengine/bevy/issues/3022
- Goal is to allow Entity to benefit from niche optimization, especially
in the case of Option<Entity> to reduce memory overhead with structures
with empty slots
## Discussion
- First PR attempt: https://github.com/bevyengine/bevy/pull/3029
- Discord:
https://discord.com/channels/691052431525675048/1154573759752183808/1154573764240093224
## Solution
- Change `Entity::generation` from u32 to NonZeroU32 to allow for niche
optimization.
- The reason for changing generation rather than index is so that the
costs are only encountered on Entity free, instead of on Entity alloc
- There was some concern with generations being used, due to there being
some desire to introduce flags. This was more to do with the original
retirement approach, however, in reality even if generations were
reduced to 24-bits, we would still have 16 million generations available
before wrapping and current ideas indicate that we would be using closer
to 4-bits for flags.
- Additionally, another concern was the representation of relationships
where NonZeroU32 prevents us using the full address space, talking with
Joy it seems unlikely to be an issue. The majority of the time these
entity references will be low-index entries (ie. `ChildOf`, `Owes`),
these will be able to be fast lookups, and the remainder of the range
can use slower lookups to map to the address space.
- It has the additional benefit of being less visible to most users,
since generation is only ever really set through `from_bits` type
methods.
- `EntityMeta` was changed to match
- On free, generation now explicitly wraps:
- Originally, generation would panic in debug mode and wrap in release
mode due to using regular ops.
- The first attempt at this PR changed the behavior to "retire" slots
and remove them from use when generations overflowed. This change was
controversial, and likely needs a proper RFC/discussion.
- Wrapping matches current release behaviour, and should therefore be
less controversial.
- Wrapping also more easily migrates to the retirement approach, as
users likely to exhaust the exorbitant supply of generations will code
defensively against aliasing and that defensive code is less likely to
break than code assuming that generations don't wrap.
- We use some unsafe code here when wrapping generations, to avoid
branch on NonZeroU32 construction. It's guaranteed safe due to how we
perform wrapping and it results in significantly smaller ASM code.
- https://godbolt.org/z/6b6hj8PrM
## Migration
- Previous `bevy_scene` serializations have a high likelihood of being
broken, as they contain 0th generation entities.
## Current Issues
- `Entities::reserve_generations` and `EntityMapper` wrap now, even in
debug - although they technically did in release mode already so this
probably isn't a huge issue. It just depends if we need to change
anything here?
---------
Co-authored-by: Natalie Baker <natalie.baker@advancednavigation.com>
# Objective
`Column` unconditionally requires three separate allocations: one for
the data, and two for the tick Vecs. The tick Vecs aren't really needed
for Resources, so we're allocating a bunch of one-element Vecs, and it
costs two extra dereferences when fetching/inserting/removing resources.
## Solution
Drop one level lower in `ResourceData` and directly store a `BlobVec`
and two `UnsafeCell<Tick>`s. This should significantly shrink
`ResourceData` (exchanging 6 usizes for 2 u32s), removes the need to
dereference two separate ticks when inserting/removing/fetching
resources, and can significantly decrease the number of small
allocations the ECS makes by default.
This tentatively might have a non-insignificant impact on the CPU cost
for rendering since we're constantly fetching resources in draw
functions, depending on how aggressively inlined the functions are.
This requires reimplementing some of the unsafe functions that `Column`
wraps, but it also allows us to delete a few Column APIs that were only
used for Resources, so the total amount of unsafe we're maintaining
shouldn't change significantly.
---------
Co-authored-by: Joseph <21144246+JoJoJet@users.noreply.github.com>
# Objective
In #9604 we removed the ability to define an `EntityCommand` as
`fn(Entity, &mut World)`. However I have since realized that `fn(Entity,
&mut World)` is an incredibly expressive and powerful way to define a
command for an entity that may or may not exist (`fn(EntityWorldMut)`
only works on entities that are alive).
## Solution
Support `EntityCommand`s in the style of `fn(Entity, &mut World)`, as
well as `fn(EntityWorldMut)`. Use a marker generic on the
`EntityCommand` trait to allow multiple impls.
The second commit in this PR replaces all of the internal command
definitions with ones using `fn` definitions. This is mostly just to
show off how expressive this style of command is -- we can revert this
commit if we'd rather avoid breaking changes.
---
## Changelog
Re-added support for expressively defining an `EntityCommand` as a
function that takes `Entity, &mut World`.
## Migration Guide
All `Command` types in `bevy_ecs`, such as `Spawn`, `SpawnBatch`,
`Insert`, etc., have been made private. Use the equivalent methods on
`Commands` or `EntityCommands` instead.
# Objective
- Make it possible to react to arbitrary state changes
- this will be useful regardless of the other changes to states
currently being discussed
## Solution
- added `StateTransitionEvent<S>` struct
- previously, this would have been impossible:
```rs
#[derive(States, Eq, PartialEq, Hash, Copy, Clone, Default)]
enum MyState {
#[default]
Foo,
Bar(MySubState),
}
enum MySubState {
Spam,
Eggs,
}
app.add_system(Update, on_enter_bar);
fn on_enter_bar(trans: EventReader<StateTransition<MyState>>){
for (befoare, after) in trans.read() {
match before, after {
MyState::Foo, MyState::Bar(_) => info!("detected transition foo => bar");
_, _ => ();
}
}
}
```
---
## Changelog
- Added
- `StateTransitionEvent<S>` - Fired on state changes of `S`
## Migration Guide
N/A no breaking changes
---------
Co-authored-by: Federico Rinaldi <gisquerin@gmail.com>
# Objective
When `BlobVec::reserve` is called with an argument causing capacity
overflow, in release build capacity overflow is ignored, and capacity is
decreased.
I'm not sure it is possible to exploit this issue using public API of
`bevy_ecs`, but better fix it anyway.
## Solution
Check for capacity overflow.
# Objective
`SystemName` might be useful in systems which accept `&mut World`.
## Solution
- `impl ExclusiveSystemParam for SystemName`
- move `SystemName` into a separate file, because it no longer belongs
to a file which defines `SystemParam`
- add a test for new impl, and for existing impl
## Changelog
- `impl ExclusiveSystemParam for SystemName`
# Objective
There are a lot of doctests that are `ignore`d for no documented reason.
And that should be fixed.
## Solution
I searched the bevy repo with the regex ` ```[a-z,]*ignore ` in order to
find all `ignore`d doctests. For each one of the `ignore`d doctests, I
did the following steps:
1. Attempt to remove the `ignored` attribute while still passing the
test. I did this by adding hidden dummy structs and imports.
2. If step 1 doesn't work, attempt to replace the `ignored` attribute
with the `no_run` attribute while still passing the test.
3. If step 2 doesn't work, keep the `ignored` attribute but add
documentation for why the `ignored` attribute was added.
---------
Co-authored-by: François <mockersf@gmail.com>
# Objective
Fixes#11050
Rename ArchetypeEntity::entity to ArchetypeEntity::id to be consistent
with `EntityWorldMut`, `EntityMut` and `EntityRef`.
## Migration Guide
The method `ArchetypeEntity::entity` has been renamed to
`ArchetypeEntity::id`
# Objective
- There is an warning about non snake case on system_param.rs generated
by a macro
## Solution
- Allow non snake case on the function at fault
# Objective
Implement `ExclusiveSystemParam` for `PhantomData`.
For the same reason `SystemParam` impl exists: to simplify writing
generic code.
786abbf3f5/crates/bevy_ecs/src/system/system_param.rs (L1557)
Also for consistency.
## Solution
`impl ExclusiveSystemParam for PhantomData`.
## Changelog
Added: PhantomData<T> now implements ExclusiveSystemParam.
# Objective
Mostly for consistency.
## Solution
```rust
impl ExclusiveSystemParam for WorldId
```
- Also add a test for `SystemParam for WorldId`
## Changelog
Added: Worldd now implements ExclusiveSystemParam.
# Objective
Fix ci hang, so we can merge pr's again.
## Solution
- switch ppa action to use mesa stable versions
https://launchpad.net/~kisak/+archive/ubuntu/turtle
- use commit from #11123
---------
Co-authored-by: Stepan Koltsov <stepan.koltsov@gmail.com>
# Objective
The documentation for the `States` trait contains an error! There is a
single colon missing from `OnExit<T:Variant>`.
## Solution
Replace `OnExit<T:Variant>` with `OnExit<T::Variant>`. (Notice the added
colon.)
---
## Changelog
### Added
- Added missing colon in `States` documentation.
---
Bevy community, you may now rest easy.
# Objective
Fix#10731.
## Solution
Rename `App::add_state<T>(&mut self)` to `init_state`, and add
`App::insert_state<T>(&mut self, state: T)`. I decided on these names
because they are more similar to `init_resource` and `insert_resource`.
I also removed the `States` trait's requirement for `Default`. Instead,
`init_state` requires `FromWorld`.
---
## Changelog
- Renamed `App::add_state` to `init_state`.
- Added `App::insert_state`.
- Removed the `States` trait's requirement for `Default`.
## Migration Guide
- Renamed `App::add_state` to `init_state`.
# Objective
`Has<T>` in some niche cases may behave in an unexpected way.
Specifically, when using `Query::get` on a `Has<T>` with a despawned
entity.
## Solution
Add precision about cases wehre `Query::get` could return an `Err`.
Use `'w` for world lifetime consistently.
When implementing system params, useful to look at how other params are
implemented. `'w` makes it clear it is world, not state.
# Objective
- Allow checking if a resource has changed by its ComponentId
---
## Changelog
- Added `World::is_resource_changed_by_id()` and
`World::is_resource_added_by_id()`.
# Objective
The definition of several `QueryState` methods use unnecessary explicit
lifetimes, which adds to visual noise.
## Solution
Elide the lifetimes.
# Objective
- Users are often confused when their command effects are not visible in
the next system. This PR auto inserts sync points if there are deferred
buffers on a system and there are dependents on that system (systems
with after relationships).
- Manual sync points can lead to users adding more than needed and it's
hard for the user to have a global understanding of their system graph
to know which sync points can be merged. However we can easily calculate
which sync points can be merged automatically.
## Solution
1. Add new edge types to allow opting out of new behavior
2. Insert an sync point for each edge whose initial node has deferred
system params.
3. Reuse nodes if they're at the number of sync points away.
* add opt outs for specific edges with `after_ignore_deferred`,
`before_ignore_deferred` and `chain_ignore_deferred`. The
`auto_insert_apply_deferred` boolean on `ScheduleBuildSettings` can be
set to false to opt out for the whole schedule.
## Perf
This has a small negative effect on schedule build times.
```text
group auto-sync main-for-auto-sync
----- ----------- ------------------
build_schedule/1000_schedule 1.06 2.8±0.15s ? ?/sec 1.00 2.7±0.06s ? ?/sec
build_schedule/1000_schedule_noconstraints 1.01 26.2±0.88ms ? ?/sec 1.00 25.8±0.36ms ? ?/sec
build_schedule/100_schedule 1.02 13.1±0.33ms ? ?/sec 1.00 12.9±0.28ms ? ?/sec
build_schedule/100_schedule_noconstraints 1.08 505.3±29.30µs ? ?/sec 1.00 469.4±12.48µs ? ?/sec
build_schedule/500_schedule 1.00 485.5±6.29ms ? ?/sec 1.00 485.5±9.80ms ? ?/sec
build_schedule/500_schedule_noconstraints 1.00 6.8±0.10ms ? ?/sec 1.02 6.9±0.16ms ? ?/sec
```
---
## Changelog
- Auto insert sync points and added `after_ignore_deferred`,
`before_ignore_deferred`, `chain_no_deferred` and
`auto_insert_apply_deferred` APIs to opt out of this behavior
## Migration Guide
- `apply_deferred` points are added automatically when there is ordering
relationship with a system that has deferred parameters like `Commands`.
If you want to opt out of this you can switch from `after`, `before`,
and `chain` to the corresponding `ignore_deferred` API,
`after_ignore_deferred`, `before_ignore_deferred` or
`chain_ignore_deferred` for your system/set ordering.
- You can also set `ScheduleBuildSettings::auto_insert_sync_points` to
`false` if you want to do it for the whole schedule. Note that in this
mode you can still add `apply_deferred` points manually.
- For most manual insertions of `apply_deferred` you should remove them
as they cannot be merged with the automatically inserted points and
might reduce parallelizability of the system graph.
## TODO
- [x] remove any apply_deferred used in the engine
- [x] ~~decide if we should deprecate manually using apply_deferred.~~
We'll still allow inserting manual sync points for now for whatever edge
cases users might have.
- [x] Update migration guide
- [x] rerun schedule build benchmarks
---------
Co-authored-by: Joseph <21144246+JoJoJet@users.noreply.github.com>
# Objective
- Make the implementation order consistent between all sources to fit
the order in the trait.
## Solution
- Change the implementation order.
# Objective
Since #10776 split `WorldQuery` to `WorldQueryData` and
`WorldQueryFilter`, it should be clear that the query is actually
composed of two parts. It is not factually correct to call "query" only
the data part. Therefore I suggest to rename the `Q` parameter to `D` in
`Query` and related items.
As far as I know, there shouldn't be breaking changes from renaming
generic type parameters.
## Solution
I used a combination of rust-analyzer go to reference and `Ctrl-F`ing
various patterns to catch as many cases as possible. Hopefully I got
them all. Feel free to check if you're concerned of me having missed
some.
## Notes
This and #10779 have many lines in common, so merging one will cause a
lot of merge conflicts to the other.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
- The example in the docs is unsound.
Demo:
```rust
#[derive(Resource)]
struct MyRes(u32);
fn main() {
let mut w = World::new();
w.insert_resource(MyRes(0));
let (mut res, comp) = split_world_access(&mut w);
let mut r1 = res.get_resource_mut::<MyRes>().unwrap();
let mut r2 = res.get_resource_mut::<MyRes>().unwrap();
*r1 = MyRes(1);
*r2 = MyRes(2);
}
```
The API in the example allows aliasing mutable references to the same
resource. Miri also complains when running this.
## Solution
- Change the example API to make the returned `Mut` borrow from the
`OnlyResourceAccessWorld` instead of borrowing from the world via `'w`.
This prevents obtaining more than one `Mut` at the same time from it.
# Objective
The `Despawn` command breaks the hierarchy whenever you use it if the
despawned entity has a parent or any children. This is a serious footgun
because the `Despawn` command has the shortest name, the behavior is
unexpected and not likely to be what you want, and the crash that it
causes can be very difficult to track down.
## Solution
Until this can be fixed by relations, add a note mentioning the footgun
in the documentation.
## Solution
`Commands.remove` and `.retain` (because I copied `remove`s doc)
referenced `EntityWorldMut.remove` and `retain` for more detail but the
`Commands` docs are much more detailed (which makes sense because it is
the most common api), so I have instead inverted this so that
`EntityWorldMut` docs link to `Commands`.
I also made `EntityWorldMut.despawn` reference `World.despawn` for more
details, like `Commands.despawn` does.
# Objective
Test more complex function signatures for exclusive systems, and test
that `StaticSystemParam` is indeed a `SystemParam`.
I mean, it currently works, but might as well add a test for it.
# Objective
Adds `EntityCommands.retain` and `EntityWorldMut.retain` to remove all
components except the given bundle from the entity.
Fixes#10865.
## Solution
I added a private unsafe function in `EntityWorldMut` called
`remove_bundle_info` which performs the shared behaviour of `remove` and
`retain`, namely taking a `BundleInfo` of components to remove, and
removing them from the given entity. Then `retain` simply gets all the
components on the entity and filters them by whether they are in the
bundle it was passed, before passing this `BundleInfo` into
`remove_bundle_info`.
`EntityCommands.retain` just creates a new type `Retain` which runs
`EntityWorldMut.retain` when run.
---
## Changelog
Added `EntityCommands.retain` and `EntityWorldMut.retain`, which remove
all components except the given bundle from the entity, they can also be
used to remove all components by passing `()` as the bundle.
# Objective
- Fixes#10806
## Solution
Replaced `new` and `index` methods for both `TableRow` and `TableId`
with `from_*` and `as_*` methods. These remove the need to perform
casting at call sites, reducing the total number of casts in the Bevy
codebase. Within these methods, an appropriate `debug_assertion` ensures
the cast will behave in an expected manner (no wrapping, etc.). I am
using a `debug_assertion` instead of an `assert` to reduce any possible
runtime overhead, however minimal. This choice is something I am open to
changing (or leaving up to another PR) if anyone has any strong
arguments for it.
---
## Changelog
- `ComponentSparseSet::sparse` stores a `TableRow` instead of a `u32`
(private change)
- Replaced `TableRow::new` and `TableRow::index` methods with
`TableRow::from_*` and `TableRow::as_*`, with `debug_assertions`
protecting any internal casting.
- Replaced `TableId::new` and `TableId::index` methods with
`TableId::from_*` and `TableId::as_*`, with `debug_assertions`
protecting any internal casting.
- All `TableId` methods are now `const`
## Migration Guide
- `TableRow::new` -> `TableRow::from_usize`
- `TableRow::index` -> `TableRow::as_usize`
- `TableId::new` -> `TableId::from_usize`
- `TableId::index` -> `TableId::as_usize`
---
## Notes
I have chosen to remove the `index` and `new` methods for the following
chain of reasoning:
- Across the codebase, `new` was called with a mixture of `u32` and
`usize` values. Likewise for `index`.
- Choosing `new` to either be `usize` or `u32` would break half of these
call-sites, requiring `as` casting at the site.
- Adding a second method `new_u32` or `new_usize` avoids the above, bu
looks visually inconsistent.
- Therefore, they should be replaced with `from_*` and `as_*` methods
instead.
Worth noting is that by updating `ComponentSparseSet`, there are now
zero instances of interacting with the inner value of `TableRow` as a
`u32`, it is exclusively used as a `usize` value (due to interactions
with methods like `len` and slice indexing). I have left the `as_u32`
and `from_u32` methods as the "proper" constructors/getters.
# Objective
Resolves Issue #10772.
## Solution
Added the deprecated warning for QueryState::for_each_unchecked, as
noted in the comments of PR #6773.
Followed the wording in the deprecation messages for `for_each` and
`for_each_mut`
# Objective
After #6547, `Query::for_each` has been capable of automatic
vectorization on certain queries, which is seeing a notable (>50% CPU
time improvements) for iteration. However, `Query::for_each` isn't
idiomatic Rust, and lacks the flexibility of iterator combinators.
Ideally, `Query::iter` and friends should be able to achieve the same
results. However, this does seem to blocked upstream
(rust-lang/rust#104914) by Rust's loop optimizations.
## Solution
This is an intermediate solution and refactor. This moves the
`Query::for_each` implementation onto the `Iterator::fold`
implementation for `QueryIter` instead. This should result in the same
automatic vectorization optimization on all `Iterator` functions that
internally use fold, including `Iterator::for_each`, `Iterator::count`,
etc.
With this, it should close the gap between the two completely.
Internally, this PR changes `Query::for_each` to use
`query.iter().for_each(..)` instead of the duplicated implementation.
Separately, the duplicate implementations of internal iteration (i.e.
`Query::par_for_each`) now use portions of the current `Query::for_each`
implementation factored out into their own functions.
This also massively cleans up our internal fragmentation of internal
iteration options, deduplicating the iteration code used in `for_each`
and `par_iter().for_each()`.
---
## Changelog
Changed: `Query::for_each`, `Query::for_each_mut`, `Query::for_each`,
and `Query::for_each_mut` have been moved to `QueryIter`'s
`Iterator::for_each` implementation, and still retains their performance
improvements over normal iteration. These APIs are deprecated in 0.13
and will be removed in 0.14.
---------
Co-authored-by: JoJoJet <21144246+JoJoJet@users.noreply.github.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
- Shorten paths by removing unnecessary prefixes
## Solution
- Remove the prefixes from many paths which do not need them. Finding
the paths was done automatically using built-in refactoring tools in
Jetbrains RustRover.
# Objective
Keep essentially the same structure of `EntityHasher` from #9903, but
rephrase the multiplication slightly to save an instruction.
cc @superdump
Discord thread:
https://discord.com/channels/691052431525675048/1172033156845674507/1174969772522356756
## Solution
Today, the hash is
```rust
self.hash = i | (i.wrapping_mul(FRAC_U64MAX_PI) << 32);
```
with `i` being `(generation << 32) | index`.
Expanding things out, we get
```rust
i | ( (i * CONST) << 32 )
= (generation << 32) | index | ((((generation << 32) | index) * CONST) << 32)
= (generation << 32) | index | ((index * CONST) << 32) // because the generation overflowed
= (index * CONST | generation) << 32 | index
```
What if we do the same thing, but with `+` instead of `|`? That's almost
the same thing, except that it has carries, which are actually often
better in a hash function anyway, since it doesn't saturate. (`|` can be
dangerous, since once something becomes `-1` it'll stay that, and
there's no mixing available.)
```rust
(index * CONST + generation) << 32 + index
= (CONST << 32 + 1) * index + generation << 32
= (CONST << 32 + 1) * index + (WHATEVER << 32 + generation) << 32 // because the extra overflows and thus can be anything
= (CONST << 32 + 1) * index + ((CONST * generation) << 32 + generation) << 32 // pick "whatever" to be something convenient
= (CONST << 32 + 1) * index + ((CONST << 32 + 1) * generation) << 32
= (CONST << 32 + 1) * index +((CONST << 32 + 1) * (generation << 32)
= (CONST << 32 + 1) * (index + generation << 32)
= (CONST << 32 + 1) * (generation << 32 | index)
= (CONST << 32 + 1) * i
```
So we can do essentially the same thing using a single multiplication
instead of doing multiply-shift-or.
LLVM was already smart enough to merge the shifting into a
multiplication, but this saves the extra `or`:
<https://rust.godbolt.org/z/MEvbz4eo4>
It's a very small change, and often will disappear in load latency
anyway, but it's a couple percent faster in lookups:
(There was more of an improvement here before #10558, but with `to_bits`
being a single `qword` load now, keeping things mostly as it is turned
out to be better than the bigger changes I'd tried in #10605.)
---
## Changelog
(Probably skip it)
## Migration Guide
(none needed)
# Objective
Related to #10612.
Enable the
[`clippy::manual_let_else`](https://rust-lang.github.io/rust-clippy/master/#manual_let_else)
lint as a warning. The `let else` form seems more idiomatic to me than a
`match`/`if else` that either match a pattern or diverge, and from the
clippy doc, the lint doesn't seem to have any possible false positive.
## Solution
Add the lint as warning in `Cargo.toml`, refactor places where the lint
triggers.
# Objective
- Fixes#7680
- This is an updated for https://github.com/bevyengine/bevy/pull/8899
which had the same objective but fell a long way behind the latest
changes
## Solution
The traits `WorldQueryData : WorldQuery` and `WorldQueryFilter :
WorldQuery` have been added and some of the types and functions from
`WorldQuery` has been moved into them.
`ReadOnlyWorldQuery` has been replaced with `ReadOnlyWorldQueryData`.
`WorldQueryFilter` is safe (as long as `WorldQuery` is implemented
safely).
`WorldQueryData` is unsafe - safely implementing it requires that
`Self::ReadOnly` is a readonly version of `Self` (this used to be a
safety requirement of `WorldQuery`)
The type parameters `Q` and `F` of `Query` must now implement
`WorldQueryData` and `WorldQueryFilter` respectively.
This makes it impossible to accidentally use a filter in the data
position or vice versa which was something that could lead to bugs.
~~Compile failure tests have been added to check this.~~
It was previously sometimes useful to use `Option<With<T>>` in the data
position. Use `Has<T>` instead in these cases.
The `WorldQuery` derive macro has been split into separate derive macros
for `WorldQueryData` and `WorldQueryFilter`.
Previously it was possible to derive both `WorldQuery` for a struct that
had a mixture of data and filter items. This would not work correctly in
some cases but could be a useful pattern in others. *This is no longer
possible.*
---
## Notes
- The changes outside of `bevy_ecs` are all changing type parameters to
the new types, updating the macro use, or replacing `Option<With<T>>`
with `Has<T>`.
- All `WorldQueryData` types always returned `true` for `IS_ARCHETYPAL`
so I moved it to `WorldQueryFilter` and
replaced all calls to it with `true`. That should be the only logic
change outside of the macro generation code.
- `Changed<T>` and `Added<T>` were being generated by a macro that I
have expanded. Happy to revert that if desired.
- The two derive macros share some functions for implementing
`WorldQuery` but the tidiest way I could find to implement them was to
give them a ton of arguments and ask clippy to ignore that.
## Changelog
### Changed
- Split `WorldQuery` into `WorldQueryData` and `WorldQueryFilter` which
now have separate derive macros. It is not possible to derive both for
the same type.
- `Query` now requires that the first type argument implements
`WorldQueryData` and the second implements `WorldQueryFilter`
## Migration Guide
- Update derives
```rust
// old
#[derive(WorldQuery)]
#[world_query(mutable, derive(Debug))]
struct CustomQuery {
entity: Entity,
a: &'static mut ComponentA
}
#[derive(WorldQuery)]
struct QueryFilter {
_c: With<ComponentC>
}
// new
#[derive(WorldQueryData)]
#[world_query_data(mutable, derive(Debug))]
struct CustomQuery {
entity: Entity,
a: &'static mut ComponentA,
}
#[derive(WorldQueryFilter)]
struct QueryFilter {
_c: With<ComponentC>
}
```
- Replace `Option<With<T>>` with `Has<T>`
```rust
/// old
fn my_system(query: Query<(Entity, Option<With<ComponentA>>)>)
{
for (entity, has_a_option) in query.iter(){
let has_a:bool = has_a_option.is_some();
//todo!()
}
}
/// new
fn my_system(query: Query<(Entity, Has<ComponentA>)>)
{
for (entity, has_a) in query.iter(){
//todo!()
}
}
```
- Fix queries which had filters in the data position or vice versa.
```rust
// old
fn my_system(query: Query<(Entity, With<ComponentA>)>)
{
for (entity, _) in query.iter(){
//todo!()
}
}
// new
fn my_system(query: Query<Entity, With<ComponentA>>)
{
for entity in query.iter(){
//todo!()
}
}
// old
fn my_system(query: Query<AnyOf<(&ComponentA, With<ComponentB>)>>)
{
for (entity, _) in query.iter(){
//todo!()
}
}
// new
fn my_system(query: Query<Option<&ComponentA>, Or<(With<ComponentA>, With<ComponentB>)>>)
{
for entity in query.iter(){
//todo!()
}
}
```
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
- `insert_reflect` relies on `reflect_type_path`, which doesn't gives
the actual type path for object created by `clone_value`, leading to an
unexpected panic. This is a workaround for it.
- Fix#10590
## Solution
- Tries to get type path from `get_represented_type_info` if get failed
from `reflect_type_path`.
---
## Defect remaining
- `get_represented_type_info` implies a shortage on performance than
using `TypeRegistry`.
# Objective
- Fixes#10676, preventing a possible memory leak for commands which
owned resources.
## Solution
Implemented `Drop` for `CommandQueue`. This has been done entirely in
the private API of `CommandQueue`, ensuring no breaking changes. Also
added a unit test, `test_command_queue_inner_drop_early`, based on the
reproduction steps as outlined in #10676.
## Notes
I believe this can be applied to `0.12.1` as well, but I am uncertain of
the process to make that kind of change. Please let me know if there's
anything I can do to help with the back-porting of this change.
---------
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
# Objective
Resolves#10743.
## Solution
Copied over the documentation written by @stepancheng from PR #10718.
I left out the lines from the doctest where `<()>` is removed, as that
seemed to be the part people couldn't decide on whether to keep or not.
## Objective
Currently, events are dropped after two frames. This cadence wasn't
*chosen* for a specific reason, double buffering just lets events
persist for at least two frames. Events only need to be dropped at a
predictable point so that the event queues don't grow forever (i.e.
events should never cause a memory leak).
Events (and especially input events) need to be observable by systems in
`FixedUpdate`, but as-is events are dropped before those systems even
get a chance to see them.
## Solution
Instead of unconditionally dropping events in `First`, require
`FixedUpdate` to first queue the buffer swap (if the `TimePlugin` has
been installed). This way, events are only dropped after a frame that
runs `FixedUpdate`.
## Future Work
In the same way we have independent copies of `Time` for tracking time
in `Main` and `FixedUpdate`, we will need independent copies of `Input`
for tracking press/release status correctly in `Main` and `FixedUpdate`.
--
Every run of `FixedUpdate` covers a specific timespan. For example, if
the fixed timestep `Δt` is 10ms, the first three `FixedUpdate` runs
cover `[0ms, 10ms)`, `[10ms, 20ms)`, and `[20ms, 30ms)`.
`FixedUpdate` can run many times in one frame. For truly
framerate-independent behavior, each `FixedUpdate` should only see the
events that occurred in its covered timespan, but what happens right now
is the first step in the frame reads all pending events.
Fixing that will require timestamped events.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Explain https://github.com/bevyengine/bevy/issues/10625.
This might be obvious to those familiar with Bevy internals, but it
surprised me.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
- I've been experimenting with different patterns to try and make async
tasks more convenient. One of the better ones I've found is to return a
command queue to allow for deferred &mut World access. It can be
convenient to check for task completion in a normal system, but it is
hard to do something with the command queue after getting it back. This
pr adds a `append` to Commands. This allows appending the returned
command queue onto the system's commands.
## Solution
- I edited the async compute example to use the new `append`, but not
sure if I should keep the example changed as this might be too
opinionated.
## Future Work
- It would be very easy to pull the pattern used in the example out into
a plugin or a external crate, so users wouldn't have to add the checking
system.
---
## Changelog
- add `append` to `Commands` and `CommandQueue`
# Objective
Enables warning on `clippy::undocumented_unsafe_blocks` across the
workspace rather than only in `bevy_ecs`, `bevy_transform` and
`bevy_utils`. This adds a little awkwardness in a few areas of code that
have trivial safety or explain safety for multiple unsafe blocks with
one comment however automatically prevents these comments from being
missed.
## Solution
This adds `undocumented_unsafe_blocks = "warn"` to the workspace
`Cargo.toml` and fixes / adds a few missed safety comments. I also added
`#[allow(clippy::undocumented_unsafe_blocks)]` where the safety is
explained somewhere above.
There are a couple of safety comments I added I'm not 100% sure about in
`bevy_animation` and `bevy_render/src/view` and I'm not sure about the
use of `#[allow(clippy::undocumented_unsafe_blocks)]` compared to adding
comments like `// SAFETY: See above`.
# Objective
Make the impl block for RemovedSystem generic so that the methods can be
called for systems that have inputs or outputs.
## Solution
Simply adding generics to the impl block.
# Objective
Adds `.entry` to `EntityWorldMut` with `Entry`, `OccupiedEntry` and
`VacantEntry` for easier in-situ modification, based on `HashMap.entry`.
Fixes#10635
## Solution
This adds the `entry` method to `EntityWorldMut` which returns an
`Entry`. This is an enum of `OccupiedEntry` and `VacantEntry` and has
the methods `and_modify`, `insert_entry`, `or_insert`, `or_insert_with`
and `or_default`. The only difference between `OccupiedEntry` and
`VacantEntry` is the type, they are both a mutable reference to the
`EntityWorldMut` and a marker for the component type, `HashMap` also
stores things to make it quicker to access the data in `OccupiedEntry`
but I wasn't sure if we had anything it would be logical to store to
make accessing/modifying the component faster? As such, the differences
are that `OccupiedEntry` assumes the entity has the component (because
nothing else can have an `EntityWorldMut` so it can't be changed outside
the entry api) and has different methods.
All the methods are based very closely off `hashbrown::HashMap` (because
its easier to read the source of) with a couple of quirks like
`OccupiedEntry.insert` doesn't return the old value because we don't
appear to have an api for mem::replacing components.
---
## Changelog
- Added a new function `EntityWorldMut.entry` which returns an `Entry`,
allowing easier in-situ modification of a component.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Pascal Hertleif <killercup@gmail.com>
# Objective
- Fix adding `#![allow(clippy::type_complexity)]` everywhere. like #9796
## Solution
- Use the new [lints] table that will land in 1.74
(https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#lints)
- inherit lint to the workspace, crates and examples.
```
[lints]
workspace = true
```
## Changelog
- Bump rust version to 1.74
- Enable lints table for the workspace
```toml
[workspace.lints.clippy]
type_complexity = "allow"
```
- Allow type complexity for all crates and examples
```toml
[lints]
workspace = true
```
---------
Co-authored-by: Martín Maita <47983254+mnmaita@users.noreply.github.com>
# Objective
- Follow up on https://github.com/bevyengine/bevy/pull/10519, diving
deeper into optimising `Entity` due to the `derive`d `PartialOrd`
`partial_cmp` not being optimal with codegen:
https://github.com/rust-lang/rust/issues/106107
- Fixes#2346.
## Solution
Given the previous PR's solution and the other existing LLVM codegen
bug, there seemed to be a potential further optimisation possible with
`Entity`. In exploring providing manual `PartialOrd` impl, it turned out
initially that the resulting codegen was not immediately better than the
derived version. However, once `Entity` was given `#[repr(align(8)]`,
the codegen improved remarkably, even more once the fields in `Entity`
were rearranged to correspond to a `u64` layout (Rust doesn't
automatically reorder fields correctly it seems). The field order and
`align(8)` additions also improved `to_bits` codegen to be a single
`mov` op. In turn, this led me to replace the previous
"non-shortcircuiting" impl of `PartialEq::eq` to use direct `to_bits`
comparison.
The result was remarkably better codegen across the board, even for
hastable lookups.
The current baseline codegen is as follows:
https://godbolt.org/z/zTW1h8PnY
Assuming the following example struct that mirrors with the existing
`Entity` definition:
```rust
#[derive(Clone, Copy, Eq, PartialEq, PartialOrd, Ord)]
pub struct FakeU64 {
high: u32,
low: u32,
}
```
the output for `to_bits` is as follows:
```
example::FakeU64::to_bits:
shl rdi, 32
mov eax, esi
or rax, rdi
ret
```
Changing the struct to:
```rust
#[derive(Clone, Copy, Eq)]
#[repr(align(8))]
pub struct FakeU64 {
low: u32,
high: u32,
}
```
and providing manual implementations for `PartialEq`/`PartialOrd`/`Ord`,
`to_bits` now optimises to:
```
example::FakeU64::to_bits:
mov rax, rdi
ret
```
The full codegen example for this PR is here for reference:
https://godbolt.org/z/n4Mjx165a
To highlight, `gt` comparison goes from
```
example::greater_than:
cmp edi, edx
jae .LBB3_2
xor eax, eax
ret
.LBB3_2:
setne dl
cmp esi, ecx
seta al
or al, dl
ret
```
to
```
example::greater_than:
cmp rdi, rsi
seta al
ret
```
As explained on Discord by @scottmcm :
>The root issue here, as far as I understand it, is that LLVM's
middle-end is inexplicably unwilling to merge loads if that would make
them under-aligned. It leaves that entirely up to its target-specific
back-end, and thus a bunch of the things that you'd expect it to do that
would fix this just don't happen.
## Benchmarks
Before discussing benchmarks, everything was tested on the following
specs:
AMD Ryzen 7950X 16C/32T CPU
64GB 5200 RAM
AMD RX7900XT 20GB Gfx card
Manjaro KDE on Wayland
I made use of the new entity hashing benchmarks to see how this PR would
improve things there. With the changes in place, I first did an
implementation keeping the existing "non shortcircuit" `PartialEq`
implementation in place, but with the alignment and field ordering
changes, which in the benchmark is the `ord_shortcircuit` column. The
`to_bits` `PartialEq` implementation is the `ord_to_bits` column. The
main_ord column is the current existing baseline from `main` branch.
My machine is not super set-up for benchmarking, so some results are
within noise, but there's not just a clear improvement between the
non-shortcircuiting implementation, but even further optimisation taking
place with the `to_bits` implementation.
On my machine, a fair number of the stress tests were not showing any
difference (indicating other bottlenecks), but I was able to get a clear
difference with `many_foxes` with a fox count of 10,000:
Test with `cargo run --example many_foxes --features
bevy/trace_tracy,wayland --release -- --count 10000`:
On avg, a framerate of about 28-29FPS was improved to 30-32FPS. "This
trace" represents the current PR's perf, while "External trace"
represents the `main` branch baseline.
## Changelog
Changed: micro-optimized Entity align and field ordering as well as
providing manual `PartialOrd`/`Ord` impls to help LLVM optimise further.
## Migration Guide
Any `unsafe` code relying on field ordering of `Entity` or sufficiently
cursed shenanigans should change to reflect the different internal
representation and alignment requirements of `Entity`.
Co-authored-by: james7132 <contact@jamessliu.com>
Co-authored-by: NathanW <nathansward@comcast.net>
# Objective
- Fixes#10532
## Solution
I've updated the various `Event` send methods to return the sent
`EventId`(s). Since these methods previously returned nothing, and this
information is cheap to copy, there should be minimal negative
consequences to providing this additional information. In the case of
`send_batch`, an iterator is returned built from `Range` and `Map`,
which only consumes 16 bytes on the stack with no heap allocations for
all batch sizes. As such, the cost of this information is negligible.
These changes are reflected for `EventWriter` and `World`. For `World`,
the return types are optional to account for the possible lack of an
`Events` resource. Again, these methods previously returned no
information, so its inclusion should only be a benefit.
## Usage
Now when sending events, the IDs of those events is available for
immediate use:
```rust
// Example of a request-response system where the requester can track handled requests.
/// A system which can make and track requests
fn requester(
mut requests: EventWriter<Request>,
mut handled: EventReader<Handled>,
mut pending: Local<HashSet<EventId<Request>>>,
) {
// Check status of previous requests
for Handled(id) in handled.read() {
pending.remove(&id);
}
if !pending.is_empty() {
error!("Not all my requests were handled on the previous frame!");
pending.clear();
}
// Send a new request and remember its ID for later
let request_id = requests.send(Request::MyRequest { /* ... */ });
pending.insert(request_id);
}
/// A system which handles requests
fn responder(
mut requests: EventReader<Request>,
mut handled: EventWriter<Handled>,
) {
for (request, id) in requests.read_with_id() {
if handle(request).is_ok() {
handled.send(Handled(id));
}
}
}
```
In the above example, a `requester` system can send request events, and
keep track of which ones are currently pending by `EventId`. Then, a
`responder` system can act on that event, providing the ID as a
reference that the `requester` can use. Before this PR, it was not
trivial for a system sending events to keep track of events by ID. This
is unfortunate, since for a system reading events, it is trivial to
access the ID of a event.
---
## Changelog
- Updated `Events`:
- Added `send_batch`
- Modified `send` to return the sent `EventId`
- Modified `send_default` to return the sent `EventId`
- Updated `EventWriter`
- Modified `send_batch` to return all sent `EventId`s
- Modified `send` to return the sent `EventId`
- Modified `send_default` to return the sent `EventId`
- Updated `World`
- Modified `send_event` to return the sent `EventId` if sent, otherwise
`None`.
- Modified `send_event_default` to return the sent `EventId` if sent,
otherwise `None`.
- Modified `send_event_batch` to return all sent `EventId`s if sent,
otherwise `None`.
- Added unit test `test_send_events_ids` to ensure returned `EventId`s
match the sent `Event`s
- Updated uses of modified methods.
## Migration Guide
### `send` / `send_default` / `send_batch`
For the following methods:
- `Events::send`
- `Events::send_default`
- `Events::send_batch`
- `EventWriter::send`
- `EventWriter::send_default`
- `EventWriter::send_batch`
- `World::send_event`
- `World::send_event_default`
- `World::send_event_batch`
Ensure calls to these methods either handle the returned value, or
suppress the result with `;`.
```rust
// Now fails to compile due to mismatched return type
fn send_my_event(mut events: EventWriter<MyEvent>) {
events.send_default()
}
// Fix
fn send_my_event(mut events: EventWriter<MyEvent>) {
events.send_default();
}
```
This will most likely be noticed within `match` statements:
```rust
// Before
match is_pressed {
true => events.send(PlayerAction::Fire),
// ^--^ No longer returns ()
false => {}
}
// After
match is_pressed {
true => {
events.send(PlayerAction::Fire);
},
false => {}
}
```
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Nicola Papale <nicopap@users.noreply.github.com>
# Objective
Allows chained systems taking an `In<_>` input parameter to be run as
one-shot systems. This API was mentioned in #8963.
In addition, `run_system(_with_input)` returns the system output, for
any `'static` output type.
## Solution
A new function, `World::run_system_with_input` allows a `SystemId<I, O>`
to be run by providing an `I` value as input and producing `O` as an
output.
`SystemId<I, O>` is now generic over the input type `I` and output type
`O`, along with the related functions and types `RegisteredSystem`,
`RemovedSystem`, `register_system`, `remove_system`, and
`RegisteredSystemError`. These default to `()`, preserving the existing
API, for all of the public types.
---
## Changelog
- Added `World::run_system_with_input` function to allow one-shot
systems that take `In<_>` input parameters
- Changed `World::run_system` and `World::register_system` to support
systems with return types beyond `()`
- Added `Commands::run_system_with_input` command that schedules a
one-shot system with an `In<_>` input parameter
(This is my first PR here, so I've probably missed some things. Please
let me know what else I should do to help you as a reviewer!)
# Objective
Due to https://github.com/rust-lang/rust/issues/117800, the `derive`'d
`PartialEq::eq` on `Entity` isn't as good as it could be. Since that's
used in hashtable lookup, let's improve it.
## Solution
The derived `PartialEq::eq` short-circuits if the generation doesn't
match. However, having a branch there is sub-optimal, especially on
64-bit systems like x64 that could just load the whole `Entity` in one
load anyway.
Due to complications around `poison` in LLVM and the exact details of
what unsafe code is allowed to do with reference in Rust
(https://github.com/rust-lang/unsafe-code-guidelines/issues/346), LLVM
isn't allowed to completely remove the short-circuiting. `&Entity` is
marked `dereferencable(8)` so LLVM knows it's allowed to *load* all 8
bytes -- and does so -- but it has to assume that the `index` might be
undef/poison if the `generation` doesn't match, and thus while it finds
a way to do it without needing a branch, it has to do something slightly
more complicated than optimal to combine the results. (LLVM is allowed
to change non-short-circuiting code to use branches, but not the other
way around.)
Here's a link showing the codegen today:
<https://rust.godbolt.org/z/9WzjxrY7c>
```rust
#[no_mangle]
pub fn demo_eq_ref(a: &Entity, b: &Entity) -> bool {
a == b
}
```
ends up generating the following assembly:
```asm
demo_eq_ref:
movq xmm0, qword ptr [rdi]
movq xmm1, qword ptr [rsi]
pcmpeqd xmm1, xmm0
pshufd xmm0, xmm1, 80
movmskpd eax, xmm0
cmp eax, 3
sete al
ret
```
(It's usually not this bad in real uses after inlining and LTO, but it
makes a strong demo.)
This PR manually implements `PartialEq::eq` *without* short-circuiting,
and because that tells LLVM that neither the generations nor the index
can be poison, it doesn't need to be so careful and can generate the
"just compare the two 64-bit values" code you'd have probably already
expected:
```asm
demo_eq_ref:
mov rax, qword ptr [rsi]
cmp qword ptr [rdi], rax
sete al
ret
```
Since this doesn't change the representation of `Entity`, if it's
instead passed by *value*, then each `Entity` is two `u32` registers,
and the old and the new code do exactly the same thing. (Other
approaches, like changing `Entity` to be `[u32; 2]` or `u64`, affect
this case.)
This should hopefully merge easily with changes like
https://github.com/bevyengine/bevy/pull/9907 that also want to change
`Entity`.
## Benchmarks
I'm not super-confident that I got my machine fully consistent for
benchmarking, but whether I run the old or the new one first I get
reasonably consistent results.
Here's a fairly typical example of the benchmarks I added in this PR:
Building the sets seems to be basically the same. It's usually reported
as noise, but sometimes I see a few percent slower or faster.
But lookup hits in particular -- since a hit checks that the key is
equal -- consistently shows around 10% improvement.
`cargo run --example many_cubes --features bevy/trace_tracy --release --
--benchmark` showed as slightly faster with this change, though if I had
to bet I'd probably say it's more noise than meaningful (but at least
it's not worse either):
This is my first PR here -- and my first time running Tracy -- so please
let me know what else I should run, or run things on your own more
reliable machines to double-check.
---
## Changelog
(probably not worth including)
Changed: micro-optimized `Entity::eq` to help LLVM slightly.
## Migration Guide
(I really hope nobody was using this on uninitialized entities where
sufficiently tortured `unsafe` could could technically notice that this
has changed.)
# Objective
- `CommandQueue::apply` calculates the address of the end of the
internal buffer as a `usize` rather than as a pointer, requiring two
casts of `cursor` to `usize`. Casting pointers to integers is generally
discouraged and may also prevent optimizations. It's also unnecessary
here.
## Solution
- Calculate the end address as a pointer rather than a `usize`.
Small note:
A trivial translation of the old code to use pointers would have
computed `end_addr` as `cursor.add(self.bytes.len())`, which is not
wrong but is an additional `unsafe` operation that also needs to be
properly documented and proven correct. However this operation is
already implemented in the form of the safe `as_mut_ptr_range`, so I
just used that.
# Objective
There is an if statement checking if a node is present in a graph
moments after it explicitly being added.
Unless the edge function has super weird side effects and the tests
don't pass, this is unnecessary.
## Solution
Removed it
# Objective
- Allow registration of one-shot systems when those systems have already
been `Box`ed.
- Needed for `bevy_eventlisteners` which allows adding event listeners
with callbacks in normal systems. The current one shot system
implementation requires systems be registered from an exclusive system,
and that those systems be passed in as types that implement
`IntoSystem`. However, the eventlistener callback crate allows users to
define their callbacks in normal systems, by boxing the system and
deferring initialization to an exclusive system.
## Solution
- Separate the registration of the system from the boxing of the system.
This is non-breaking, and adds a new method.
---
## Changelog
- Added `World::register_boxed_system` to allow registration of
already-boxed one shot systems.
# Objective
- There is a specialized hasher for entities:
[`EntityHashMap`](https://docs.rs/bevy/latest/bevy/utils/type.EntityHashMap.html)
- [`EntityMapper`] currently uses a normal `HashMap<Entity, Entity>`
- Fixes#10391
## Solution
- Replace the normal `HashMap` with the more performant `EntityHashMap`
## Questions
- This does change public API. Should a system be implemented to help
migrate code?
- Perhaps an `impl From<HashMap<K, V, S>> for EntityHashMap<K, V>`
- I updated to docs for each function that I changed, but I may have
missed something
---
## Changelog
- Changed `EntityMapper` to use `EntityHashMap` instead of normal
`HashMap`
## Migration Guide
If you are using the following types, update their listed methods to use
the new `EntityHashMap`. `EntityHashMap` has the same methods as the
normal `HashMap`, so you just need to replace the name.
### `EntityMapper`
- `get_map`
- `get_mut_map`
- `new`
- `world_scope`
### `ReflectMapEntities`
- `map_all_entities`
- `map_entities`
- `write_to_world`
### `InstanceInfo`
- `entity_map`
- This is a property, not a method.
---
This is my first time contributing in a while, and I'm not familiar with
the usage of `EntityMapper`. I changed the type definition and fixed all
errors, but there may have been things I've missed. Please keep an eye
out for me!
Preparing next release
This PR has been auto-generated
---------
Co-authored-by: Bevy Auto Releaser <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: François <mockersf@gmail.com>
# Objective
Align all error-like types to implement `Error`.
Fixes #10176
## Solution
- Derive `Error` on more types
- Refactor instances of manual implementations that could be derived
This adds thiserror as a dependency to bevy_transform, which might
increase compilation time -- but I don't know of any situation where you
might only use that but not any other crate that pulls in bevy_utils.
The `contributors` example has a `LoadContributorsError` type, but as
it's an example I have not updated it. Doing that would mean either
having a `use bevy_internal::utils::thiserror::Error;` in an example
file, or adding `thiserror` as a dev-dependency to the main `bevy`
crate.
---
## Changelog
- All `…Error` types now implement the `Error` trait
# Objective
First of all, this PR took heavy inspiration from #7760 and #5715. It
intends to also fix#5569, but with a slightly different approach.
This also fixes#9335 by reexporting `DynEq`.
## Solution
The advantage of this API is that we can intern a value without
allocating for zero-sized-types and for enum variants that have no
fields. This PR does this automatically in the `SystemSet` and
`ScheduleLabel` derive macros for unit structs and fieldless enum
variants. So this should cover many internal and external use cases of
`SystemSet` and `ScheduleLabel`. In these optimal use cases, no memory
will be allocated.
- The interning returns a `Interned<dyn SystemSet>`, which is just a
wrapper around a `&'static dyn SystemSet`.
- `Hash` and `Eq` are implemented in terms of the pointer value of the
reference, similar to my first approach of anonymous system sets in
#7676.
- Therefore, `Interned<T>` does not implement `Borrow<T>`, only `Deref`.
- The debug output of `Interned<T>` is the same as the interned value.
Edit:
- `AppLabel` is now also interned and the old
`derive_label`/`define_label` macros were replaced with the new
interning implementation.
- Anonymous set ids are reused for different `Schedule`s, reducing the
amount of leaked memory.
### Pros
- `InternedSystemSet` and `InternedScheduleLabel` behave very similar to
the current `BoxedSystemSet` and `BoxedScheduleLabel`, but can be copied
without an allocation.
- Many use cases don't allocate at all.
- Very fast lookups and comparisons when using `InternedSystemSet` and
`InternedScheduleLabel`.
- The `intern` module might be usable in other areas.
- `Interned{ScheduleLabel, SystemSet, AppLabel}` does implement
`{ScheduleLabel, SystemSet, AppLabel}`, increasing ergonomics.
### Cons
- Implementors of `SystemSet` and `ScheduleLabel` still need to
implement `Hash` and `Eq` (and `Clone`) for it to work.
## Changelog
### Added
- Added `intern` module to `bevy_utils`.
- Added reexports of `DynEq` to `bevy_ecs` and `bevy_app`.
### Changed
- Replaced `BoxedSystemSet` and `BoxedScheduleLabel` with
`InternedSystemSet` and `InternedScheduleLabel`.
- Replaced `impl AsRef<dyn ScheduleLabel>` with `impl ScheduleLabel`.
- Replaced `AppLabelId` with `InternedAppLabel`.
- Changed `AppLabel` to use `Debug` for error messages.
- Changed `AppLabel` to use interning.
- Changed `define_label`/`derive_label` to use interning.
- Replaced `define_boxed_label`/`derive_boxed_label` with
`define_label`/`derive_label`.
- Changed anonymous set ids to be only unique inside a schedule, not
globally.
- Made interned label types implement their label trait.
### Removed
- Removed `define_boxed_label` and `derive_boxed_label`.
## Migration guide
- Replace `BoxedScheduleLabel` and `Box<dyn ScheduleLabel>` with
`InternedScheduleLabel` or `Interned<dyn ScheduleLabel>`.
- Replace `BoxedSystemSet` and `Box<dyn SystemSet>` with
`InternedSystemSet` or `Interned<dyn SystemSet>`.
- Replace `AppLabelId` with `InternedAppLabel` or `Interned<dyn
AppLabel>`.
- Types manually implementing `ScheduleLabel`, `AppLabel` or `SystemSet`
need to implement:
- `dyn_hash` directly instead of implementing `DynHash`
- `as_dyn_eq`
- Pass labels to `World::try_schedule_scope`, `World::schedule_scope`,
`World::try_run_schedule`. `World::run_schedule`, `Schedules::remove`,
`Schedules::remove_entry`, `Schedules::contains`, `Schedules::get` and
`Schedules::get_mut` by value instead of by reference.
---------
Co-authored-by: Joseph <21144246+JoJoJet@users.noreply.github.com>
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
# Objective
Reduce code duplication and improve APIs of Bevy's [global
taskpools](https://github.com/bevyengine/bevy/blob/main/crates/bevy_tasks/src/usages.rs).
## Solution
- As all three of the global taskpools have identical implementations
and only differ in their identifiers, this PR moves the implementation
into a macro to reduce code duplication.
- The `init` method is renamed to `get_or_init` to more accurately
reflect what it really does.
- Add a new `try_get` method that just returns `None` when the pool is
uninitialized, to complement the other getter methods.
- Minor documentation improvements to accompany the above changes.
---
## Changelog
- Added a new `try_get` method to the global TaskPools
- The global TaskPools' `init` method has been renamed to `get_or_init`
for clarity
- Documentation improvements
## Migration Guide
- Uses of `ComputeTaskPool::init`, `AsyncComputeTaskPool::init` and
`IoTaskPool::init` should be changed to `::get_or_init`.
# Objective
Closes#9955.
Use the same interface for all "pure" builder types: taking and
returning `Self` (and not `&mut Self`).
## Solution
Changed `DynamicSceneBuilder`, `SceneFilter` and `TableBuilder` to take
and return `Self`.
## Changelog
### Changed
- `DynamicSceneBuilder` and `SceneBuilder` methods in `bevy_ecs` now
take and return `Self`.
## Migration guide
When using `bevy_ecs::DynamicSceneBuilder` and `bevy_ecs::SceneBuilder`,
instead of binding the builder to a variable, directly use it. Methods
on those types now consume `Self`, so you will need to re-bind the
builder if you don't `build` it immediately.
Before:
```rust
let mut scene_builder = DynamicSceneBuilder::from_world(&world);
let scene = scene_builder.extract_entity(a).extract_entity(b).build();
```
After:
```rust
let scene = DynamicSceneBuilder::from_world(&world)
.extract_entity(a)
.extract_entity(b)
.build();
```
