# Objective
- There's been several changes to `Query` for this release cycle, and
`Query`'s top-level documentation has gotten slightly out-of-date.
- Alternative to #18615.
## Solution
- Edit `Query`'s docs for consistency, clarity, and correctness.
- Make sure to group `get()` and `get_many()` together instead of
`single()` and `get_many()`, to enforce the distinction from
https://github.com/bevyengine/bevy/pull/18615#issuecomment-2764355672.
- Reformat doc tests so they would be readable if extracted into their
own file. (Which mainly involves adding more spacing.)
- Move link definitions to be nearer where they are used.
- Fix the tables so they are up-to-date and correctly escape square
brackets `\[ \]`.
## Testing
I ran `cargo doc -p bevy_ecs --no-deps` to view the docs and `cargo test
-p bevy_ecs --doc` to test the doc comments.
## Reviewing
The diff is difficult to read, so I don't recommend _just_ looking at
that. Instead, run `cargo doc -p bevy_ecs --no-deps` locally and read
through the new version. It should theoretically read smoother with less
super-technical jargon. :)
## Follow-up
I want to go through some of `Query`'s methods, such as `single()`,
`get()`, and `get_many()`, but I'll leave that for another PR.
---------
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
# Objective
Fixes#9367.
Yet another follow-up to #16547.
These traits were initially based on `Borrow<Entity>` because that trait
was what they were replacing, and it felt close enough in meaning.
However, they ultimately don't quite match: `borrow` always returns
references, whereas `EntityBorrow` always returns a plain `Entity`.
Additionally, `EntityBorrow` can imply that we are borrowing an `Entity`
from the ECS, which is not what it does.
Due to its safety contract, `TrustedEntityBorrow` is important an
important and widely used trait for `EntitySet` functionality.
In contrast, the safe `EntityBorrow` does not see much use, because even
outside of `EntitySet`-related functionality, it is a better idea to
accept `TrustedEntityBorrow` over `EntityBorrow`.
Furthermore, as #9367 points out, abstracting over returning `Entity`
from pointers/structs that contain it can skip some ergonomic friction.
On top of that, there are aspects of #18319 and #18408 that are relevant
to naming:
We've run into the issue that relying on a type default can switch
generic order. This is livable in some contexts, but unacceptable in
others.
To remedy that, we'd need to switch to a type alias approach:
The "defaulted" `Entity` case becomes a
`UniqueEntity*`/`Entity*Map`/`Entity*Set` alias, and the base type
receives a more general name. `TrustedEntityBorrow` does not mesh
clearly with sensible base type names.
## Solution
Replace any `EntityBorrow` bounds with `TrustedEntityBorrow`.
+
Rename them as such:
`EntityBorrow` -> `ContainsEntity`
`TrustedEntityBorrow` -> `EntityEquivalent`
For `EntityBorrow` we produce a change in meaning; We designate it for
types that aren't necessarily strict wrappers around `Entity` or some
pointer to `Entity`, but rather any of the myriad of types that contain
a single associated `Entity`.
This pattern can already be seen in the common `entity`/`id` methods
across the engine.
We do not mean for `ContainsEntity` to be a trait that abstracts input
API (like how `AsRef<T>` is often used, f.e.), because eliding
`entity()` would be too implicit in the general case.
We prefix "Contains" to match the intuition of a struct with an `Entity`
field, like some contain a `length` or `capacity`.
It gives the impression of structure, which avoids the implication of a
relationship to the `ECS`.
`HasEntity` f.e. could be interpreted as "a currently live entity",
As an input trait for APIs like #9367 envisioned, `TrustedEntityBorrow`
is a better fit, because it *does* restrict itself to strict wrappers
and pointers. Which is why we replace any
`EntityBorrow`/`ContainsEntity` bounds with
`TrustedEntityBorrow`/`EntityEquivalent`.
Here, the name `EntityEquivalent` is a lot closer to its actual meaning,
which is "A type that is both equivalent to an `Entity`, and forms the
same total order when compared".
Prior art for this is the
[`Equivalent`](https://docs.rs/hashbrown/latest/hashbrown/trait.Equivalent.html)
trait in `hashbrown`, which utilizes both `Borrow` and `Eq` for its one
blanket impl!
Given that we lose the `Borrow` moniker, and `Equivalent` can carry
various meanings, we expand on the safety comment of `EntityEquivalent`
somewhat. That should help prevent the confusion we saw in
[#18408](https://github.com/bevyengine/bevy/pull/18408#issuecomment-2742094176).
The new name meshes a lot better with the type aliasing approach in
#18408, by aligning with the base name `EntityEquivalentHashMap`.
For a consistent scheme among all set types, we can use this scheme for
the `UniqueEntity*` wrapper types as well!
This allows us to undo the switched generic order that was introduced to
`UniqueEntityArray` by its `Entity` default.
Even without the type aliases, I think these renames are worth doing!
## Migration Guide
Any use of `EntityBorrow` becomes `ContainsEntity`.
Any use of `TrustedEntityBorrow` becomes `EntityEquivalent`.
# Objective
Unlike for their helper typers, the import paths for
`unique_array::UniqueEntityArray`, `unique_slice::UniqueEntitySlice`,
`unique_vec::UniqueEntityVec`, `hash_set::EntityHashSet`,
`hash_map::EntityHashMap`, `index_set::EntityIndexSet`,
`index_map::EntityIndexMap` are quite redundant.
When looking at the structure of `hashbrown`, we can also see that while
both `HashSet` and `HashMap` have their own modules, the main types
themselves are re-exported to the crate level.
## Solution
Re-export the types in their shared `entity` parent module, and simplify
the imports where they're used.
# Objective
Make it easier to short-circuit system parameter validation.
Simplify the API surface by combining `ValidationOutcome` with
`SystemParamValidationError`.
## Solution
Replace `ValidationOutcome` with `Result<(),
SystemParamValidationError>`. Move the docs from `ValidationOutcome` to
`SystemParamValidationError`.
Add a `skipped` field to `SystemParamValidationError` to distinguish the
`Skipped` and `Invalid` variants.
Use the `?` operator to short-circuit validation in tuples of system
params.
# Objective
When introduced, `Single` was intended to simply be silently skipped,
allowing for graceful and efficient handling of systems during invalid
game states (such as when the player is dead).
However, this also caused missing resources to *also* be silently
skipped, leading to confusing and very hard to debug failures. In
0.15.1, this behavior was reverted to a panic, making missing resources
easier to debug, but largely making `Single` (and `Populated`)
worthless, as they would panic during expected game states.
Ultimately, the consensus is that this behavior should differ on a
per-system-param basis. However, there was no sensible way to *do* that
before this PR.
## Solution
Swap `SystemParam::validate_param` from a `bool` to:
```rust
/// The outcome of system / system param validation,
/// used by system executors to determine what to do with a system.
pub enum ValidationOutcome {
/// All system parameters were validated successfully and the system can be run.
Valid,
/// At least one system parameter failed validation, and an error must be handled.
/// By default, this will result in1 a panic. See [crate::error] for more information.
///
/// This is the default behavior, and is suitable for system params that should *always* be valid,
/// either because sensible fallback behavior exists (like [`Query`] or because
/// failures in validation should be considered a bug in the user's logic that must be immediately addressed (like [`Res`]).
Invalid,
/// At least one system parameter failed validation, but the system should be skipped due to [`ValidationBehavior::Skip`].
/// This is suitable for system params that are intended to only operate in certain application states, such as [`Single`].
Skipped,
}
```
Then, inside of each `SystemParam` implementation, return either Valid,
Invalid or Skipped.
Currently, only `Single`, `Option<Single>` and `Populated` use the
`Skipped` behavior. Other params (like resources) retain their current
failing
## Testing
Messed around with the fallible_params example. Added a pair of tests:
one for panicking when resources are missing, and another for properly
skipping `Single` and `Populated` system params.
## To do
- [x] get https://github.com/bevyengine/bevy/pull/18454 merged
- [x] fix the todo!() in the macro-powered tuple implementation (please
help 🥺)
- [x] test
- [x] write a migration guide
- [x] update the example comments
## Migration Guide
Various system and system parameter validation methods
(`SystemParam::validate_param`, `System::validate_param` and
`System::validate_param_unsafe`) now return and accept a
`ValidationOutcome` enum, rather than a `bool`. The previous `true`
values map to `ValidationOutcome::Valid`, while `false` maps to
`ValidationOutcome::Invalid`.
However, if you wrote a custom schedule executor, you should now respect
the new `ValidationOutcome::Skipped` parameter, skipping any systems
whose validation was skipped. By contrast, `ValidationOutcome::Invalid`
systems should also be skipped, but you should call the
`default_error_handler` on them first, which by default will result in a
panic.
If you are implementing a custom `SystemParam`, you should consider
whether failing system param validation is an error or an expected
state, and choose between `Invalid` and `Skipped` accordingly. In Bevy
itself, `Single` and `Populated` now once again skip the system when
their conditions are not met. This is the 0.15.0 behavior, but stands in
contrast to the 0.15.1 behavior, where they would panic.
---------
Co-authored-by: MiniaczQ <xnetroidpl@gmail.com>
Co-authored-by: Dmytro Banin <banind@cs.washington.edu>
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
# Objective
Continuation to #16547 and #17954.
The `get_many` family are the last methods on `Query`/`QueryState` for
which we're still missing a `unique` version.
## Solution
Offer `get_many_unique`/`get_many_unique_mut` and
`get_many_unique_inner`!
Their implementation is the same as `get_many`, the difference lies in
their guaranteed-to-be unique inputs, meaning we never do any aliasing
checks.
To reduce confusion, we also rename `get_many_readonly` into
`get_many_inner` and the current `get_many_inner` into
`get_many_mut_inner` to clarify their purposes.
## Testing
Doc examples.
## Migration Guide
`get_many_inner` is now called `get_many_mut_inner`.
`get_many_readonly` is now called `get_many_inner`.
# Objective
Fix unsound lifetimes in `Query::join` and `Query::join_filtered`.
The joined query allowed access from either input query, but it only
took the `'world` lifetime from `self`, not from `other`. This meant
that after the borrow of `other` ended, the joined query would unsoundly
alias `other`.
## Solution
Change the lifetimes on `join` and `join_filtered` to require mutable
borrows of the *same* lifetime for the input queries. This ensures both
input queries are borrowed for the full lifetime of the joined query.
Change `join_inner` to take `other` by value instead of reference so
that the returned query is still usable without needing to borrow from a
local variable.
## Testing
Added a compile-fail test.
# Objective
- Allow `Query` methods such as `Query::get` to have their error
short-circuited using `?` in systems using Bevy's `Error` type
## Solution
- Removed `UnsafeWorldCell<'w>` from `QueryEntityError` and instead
store `ArchetypeId` (the information the error formatter was extracting
anyway).
- Replaced trait implementations with derives now that the type is
plain-old-data.
## Testing
- CI
---
## Migration Guide
- `QueryEntityError::QueryDoesNotMatch.1` is of type `ArchetypeId`
instead of `UnsafeWorldCell`. It is up to the caller to obtain an
`UnsafeWorldCell` now.
- `QueryEntityError` no longer has a lifetime parameter, remove it from
type signatures where required.
## Notes
This was discussed on Discord and accepted by Cart as the desirable path
forward in [this
message](https://discord.com/channels/691052431525675048/749335865876021248/1346611950527713310).
Scroll up from this point for context.
---------
Co-authored-by: SpecificProtagonist <30270112+SpecificProtagonist@users.noreply.github.com>
# Objective
Alternative to and closes#18120.
Sibling to #18082, see that PR for broader reasoning.
Folks weren't sold on the name `many` (get_many is clearer, and this is
rare), and that PR is much more complex.
## Solution
- Simply deprecate `Query::many` and `Query::many_mut`
- Clean up internal usages
Mentions of this in the docs can wait until it's fully removed in the
0.17 cycle IMO: it's much easier to catch the problems when doing that.
## Testing
CI!
## Migration Guide
`Query::many` and `Query::many_mut` have been deprecated to reduce
panics and API duplication. Use `Query::get_many` and
`Query::get_many_mut` instead, and handle the `Result`.
---------
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
# Objective
Fix unsound query transmutes on queries obtained from
`Query::as_readonly()`.
The following compiles, and the call to `transmute_lens()` should panic,
but does not:
```rust
fn bad_system(query: Query<&mut A>) {
let mut readonly = query.as_readonly();
let mut lens: QueryLens<&mut A> = readonly.transmute_lens();
let other_readonly: Query<&A> = query.as_readonly();
// `lens` and `other_readonly` alias, and are both alive here!
}
```
To make `Query::as_readonly()` zero-cost, we pointer-cast
`&QueryState<D, F>` to `&QueryState<D::ReadOnly, F>`. This means that
the `component_access` for a read-only query's state may include
accesses for the original mutable version, but the `Query` does not have
exclusive access to those components! `transmute` and `join` use that
access to ensure that a join is valid, and will incorrectly allow a
transmute that includes mutable access.
As a bonus, allow `Query::join`s that output `FilteredEntityRef` or
`FilteredEntityMut` to receive access from the `other` query. Currently
they only receive access from `self`.
## Solution
When transmuting or joining from a read-only query, remove any writes
before performing checking that the transmute is valid. For joins, be
sure to handle the case where one input query was the result of
`as_readonly()` but the other has valid mutable access.
This requires identifying read-only queries, so add a
`QueryData::IS_READ_ONLY` associated constant. Note that we only call
`QueryState::as_transmuted_state()` with `NewD: ReadOnlyQueryData`, so
checking for read-only queries is sufficient to check for
`as_transmuted_state()`.
Removing writes requires allocating a new `FilteredAccess`, so only do
so if the query is read-only and the state has writes. Otherwise, the
existing access is correct and we can continue using a reference to it.
Use the new read-only state to call `NewD::set_access`, so that
transmuting to a `FilteredAccessMut` results in a read-only
`FilteredAccessMut`. Otherwise, it would take the original write access,
and then the transmute would panic because it had too much access.
Note that `join` was previously passing `self.component_access` to
`NewD::set_access`. Switching it to `joined_component_access` also
allows a join that outputs `FilteredEntity(Ref|Mut)` to receive access
from `other`. The fact that it didn't do that before seems like an
oversight, so I didn't try to prevent that change.
## Testing
Added unit tests with the unsound transmute and join.
# Objective
As discussed in #14275, Bevy is currently too prone to panic, and makes
the easy / beginner-friendly way to do a large number of operations just
to panic on failure.
This is seriously frustrating in library code, but also slows down
development, as many of the `Query::single` panics can actually safely
be an early return (these panics are often due to a small ordering issue
or a change in game state.
More critically, in most "finished" products, panics are unacceptable:
any unexpected failures should be handled elsewhere. That's where the
new
With the advent of good system error handling, we can now remove this.
Note: I was instrumental in a) introducing this idea in the first place
and b) pushing to make the panicking variant the default. The
introduction of both `let else` statements in Rust and the fancy system
error handling work in 0.16 have changed my mind on the right balance
here.
## Solution
1. Make `Query::single` and `Query::single_mut` (and other random
related methods) return a `Result`.
2. Handle all of Bevy's internal usage of these APIs.
3. Deprecate `Query::get_single` and friends, since we've moved their
functionality to the nice names.
4. Add detailed advice on how to best handle these errors.
Generally I like the diff here, although `get_single().unwrap()` in
tests is a bit of a downgrade.
## Testing
I've done a global search for `.single` to track down any missed
deprecated usages.
As to whether or not all the migrations were successful, that's what CI
is for :)
## Future work
~~Rename `Query::get_single` and friends to `Query::single`!~~
~~I've opted not to do this in this PR, and smear it across two releases
in order to ease the migration. Successive deprecations are much easier
to manage than the semantics and types shifting under your feet.~~
Cart has convinced me to change my mind on this; see
https://github.com/bevyengine/bevy/pull/18082#discussion_r1974536085.
## Migration guide
`Query::single`, `Query::single_mut` and their `QueryState` equivalents
now return a `Result`. Generally, you'll want to:
1. Use Bevy 0.16's system error handling to return a `Result` using the
`?` operator.
2. Use a `let else Ok(data)` block to early return if it's an expected
failure.
3. Use `unwrap()` or `Ok` destructuring inside of tests.
The old `Query::get_single` (etc) methods which did this have been
deprecated.
## Objective
There's no general error for when an entity doesn't exist, and some
methods are going to need one when they get Resultified. The closest
thing is `EntityFetchError`, but that error has a slightly more specific
purpose.
## Solution
- Added `EntityDoesNotExistError`.
- Contains `Entity` and `EntityDoesNotExistDetails`.
- Changed `EntityFetchError` and `QueryEntityError`:
- Changed `NoSuchEntity` variant to wrap `EntityDoesNotExistError` and
renamed the variant to `EntityDoesNotExist`.
- Renamed `EntityFetchError` to `EntityMutableFetchError` to make its
purpose clearer.
- Renamed `TryDespawnError` to `EntityDespawnError` to make it more
general.
- Changed `World::inspect_entity` to return `Result<[ok],
EntityDoesNotExistError>` instead of panicking.
- Changed `World::get_entity` and `WorldEntityFetch::fetch_ref` to
return `Result<[ok], EntityDoesNotExistError>` instead of `Result<[ok],
Entity>`.
- Changed `UnsafeWorldCell::get_entity` to return
`Result<UnsafeEntityCell, EntityDoesNotExistError>` instead of
`Option<UnsafeEntityCell>`.
## Migration Guide
- `World::inspect_entity` now returns `Result<impl Iterator<Item =
&ComponentInfo>, EntityDoesNotExistError>` instead of `impl
Iterator<Item = &ComponentInfo>`.
- `World::get_entity` now returns `EntityDoesNotExistError` as an error
instead of `Entity`. You can still access the entity's ID through the
error's `entity` field.
- `UnsafeWorldCell::get_entity` now returns `Result<UnsafeEntityCell,
EntityDoesNotExistError>` instead of `Option<UnsafeEntityCell>`.
# Objective
Simplify the API surface by removing duplicated functionality between
`Query` and `QueryState`.
Reduce the amount of `unsafe` code required in `QueryState`.
This is a follow-up to #15858.
## Solution
Move implementations of `Query` methods from `QueryState` to `Query`.
Instead of the original methods being on `QueryState`, with `Query`
methods calling them by passing the individual parameters, the original
methods are now on `Query`, with `QueryState` methods calling them by
constructing a `Query`.
This also adds two `_inner` methods that were missed in #15858:
`iter_many_unique_inner` and `single_inner`.
One goal here is to be able to deprecate and eventually remove many of
the methods on `QueryState`, reducing the overall API surface. (I
expected to do that in this PR, but this change was large enough on its
own!) Now that the `QueryState` methods each consist of a simple
expression like `self.query(world).get_inner(entity)`, a future PR can
deprecate some or all of them with simple migration instructions.
The other goal is to reduce the amount of `unsafe` code. The current
implementation of a read-only method like `QueryState::get` directly
calls the `unsafe fn get_unchecked_manual` and needs to repeat the proof
that `&World` has enough access. With this change, `QueryState::get` is
entirely safe code, with the proof that `&World` has enough access done
by the `query()` method and shared across all read-only operations.
## Future Work
The next step will be to mark the `QueryState` methods as
`#[deprecated]` and migrate callers to the methods on `Query`.
# Objective
Continuation of #16547.
We do not yet have parallel versions of `par_iter_many` and
`par_iter_many_unique`. It is currently very painful to try and use
parallel iteration over entity lists. Even if a list is not long, each
operation might still be very expensive, and worth parallelizing.
Plus, it has been requested several times!
## Solution
Once again, we implement what we lack!
These parallel iterators collect their input entity list into a
`Vec`/`UniqueEntityVec`, then chunk that over the available threads,
inspired by the original `par_iter`.
Since no order guarantee is given to the caller, we could sort the input
list according to `EntityLocation`, but that would likely only be worth
it for very large entity lists.
There is some duplication which could likely be improved, but I'd like
to leave that for a follow-up.
## Testing
The doc tests on `for_each_init` of `QueryParManyIter` and
`QueryParManyUniqueIter`.
# Objective
Fix unsoundness introduced by #15858. `QueryLens::query()` would hand
out a `Query` with the full `'w` lifetime, and the new `_inner` methods
would let the results outlive the `Query`. This could be used to create
aliasing mutable references, like
```rust
fn bad<'w>(mut lens: QueryLens<'w, EntityMut>, entity: Entity) {
let one: EntityMut<'w> = lens.query().get_inner(entity).unwrap();
let two: EntityMut<'w> = lens.query().get_inner(entity).unwrap();
assert!(one.entity() == two.entity());
}
```
Fixes#17693
## Solution
Restrict the `'world` lifetime in the `Query` returned by
`QueryLens::query()` to `'_`, the lifetime of the borrow of the
`QueryLens`.
The model here is that `Query<'w, 's, D, F>` and `QueryLens<'w, D, F>`
have permission to access their components for the lifetime `'w`. So
going from `&'a mut QueryLens<'w>` to `Query<'w, 'a>` would borrow the
permission only for the `'a` lifetime, but incorrectly give it out for
the full `'w` lifetime.
To handle any cases where users were calling `get_inner()` or
`iter_inner()` on the `Query` and expecting the full `'w` lifetime, we
introduce a new `QueryLens::query_inner()` method. This is only valid
for `ReadOnlyQueryData`, so it may safely hand out a copy of the
permission for the full `'w` lifetime. Since `get_inner()` and
`iter_inner()` were only valid on `ReadOnlyQueryData` prior to #15858,
that should cover any uses that relied on the longer lifetime.
## Migration Guide
Users of `QueryLens::query()` who were calling `get_inner()` or
`iter_inner()` will need to replace the call with
`QueryLens::query_inner()`.
# Objective
Restore the behavior of `Query::get_many` prior to #15858.
When passed duplicate `Entity`s, `get_many` is supposed to return
results for all of them, since read-only queries don't alias. However,
#15858 merged the implementation with `get_many_mut` and caused it to
return `QueryEntityError::AliasedMutability`.
## Solution
Introduce a new `Query::get_many_readonly` method that consumes the
`Query` like `get_many_inner`, but that is constrained to `D:
ReadOnlyQueryData` so that it can skip the aliasing check. Implement
`Query::get_many` in terms of that new method. Add a test, and a comment
explaining why it doesn't match the pattern of the other `&self`
methods.
# Objective
Simplify and expand the API for `QueryState`.
`QueryState` has a lot of methods that mirror those on `Query`. These
are then multiplied by variants that take `&World`, `&mut World`, and
`UnsafeWorldCell`. In addition, many of them have `_manual` variants
that take `&QueryState` and avoid calling `update_archetypes()`. Not all
of the combinations exist, however, so some operations are not possible.
## Solution
Introduce methods to get a `Query` from a `QueryState`. That will reduce
duplication between the types, and ensure that the full `Query` API is
always available for `QueryState`.
Introduce methods on `Query` that consume the query to return types with
the full `'w` lifetime. This avoids issues with borrowing where things
like `query_state.query(&world).get(entity)` don't work because they
borrow from the temporary `Query`.
Finally, implement `Copy` for read-only `Query`s. `get_inner` and
`iter_inner` currently take `&self`, so changing them to consume `self`
would be a breaking change. By making `Query: Copy`, they can consume a
copy of `self` and continue to work.
The consuming methods also let us simplify the implementation of methods
on `Query`, by doing `fn foo(&self) { self.as_readonly().foo_inner() }`
and `fn foo_mut(&mut self) { self.reborrow().foo_inner() }`. That
structure makes it more difficult to accidentally extend lifetimes,
since the safe `as_readonly()` and `reborrow()` methods shrink them
appropriately. The optimizer is able to see that they are both identity
functions and inline them, so there should be no performance cost.
Note that this change would conflict with #15848. If `QueryState` is
stored as a `Cow`, then the consuming methods cannot be implemented, and
`Copy` cannot be implemented.
## Future Work
The next step is to mark the methods on `QueryState` as `#[deprecated]`,
and move the implementations into `Query`.
## Migration Guide
`Query::to_readonly` has been renamed to `Query::as_readonly`.
docs: enhance documentation in `query.rs` to clarify borrowing rules.
Please, let me know if you don't agree with the wording.. There is
always room for improvement.
Tested locally and it looks like this:
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
Fixes#16104
## Solution
I removed all instances of `:?` and put them back one by one where it
caused an error.
I removed some bevy_utils helper functions that were only used in 2
places and don't add value. See: #11478
## Testing
CI should catch the mistakes
## Migration Guide
`bevy::utils::{dbg,info,warn,error}` were removed. Use
`bevy::utils::tracing::{debug,info,warn,error}` instead.
---------
Co-authored-by: SpecificProtagonist <vincentjunge@posteo.net>
# Objective
Fixes: #16578
## Solution
This is a patch fix, proper fix requires a breaking change.
Added `Panic` enum variant and using is as the system meta default.
Warn once behavior can be enabled same way disabling panic (originally
disabling wans) is.
To fix an issue with the current architecture, where **all** combinator
system params get checked together,
combinator systems only check params of the first system.
This will result in old, panicking behavior on subsequent systems and
will be fixed in 0.16.
## Testing
Ran unit tests and `fallible_params` example.
---------
Co-authored-by: François Mockers <mockersf@gmail.com>
Co-authored-by: François Mockers <francois.mockers@vleue.com>
# Objective
In current Bevy, it is very inconvenient to mutably retrieve a
user-provided list of entities more than one element at a time.
If the list contains any duplicate entities, we risk mutable aliasing.
Users of `Query::iter_many_mut` do not have access to `Iterator` trait,
and thus miss out on common functionality, for instance collecting their
`QueryManyIter`.
We can circumvent this issue with validation, however that entails
checking every entity against all others for inequality, or utilizing an
`EntityHashSet`. Even if an entity list remains unchanged, this
validation is/would have to be redone every time we wish to fetch with
the list.
This presents a lot of wasted work, as we often trivially know an entity
list to be unique f.e.: `QueryIter` will fetch every `Entity` once and
only once.
As more things become entities – assets, components, queries – this
issue will become more pronounced.
`get_many`/`many`/`iter_many`/`par_iter_many`-like functionality is all
affected.
## Solution
The solution this PR proposes is to introduce functionality built around
a new trait: `EntitySet`.
The goal is to preserve the property of "uniqueness" in a list wherever
possible, and then rely on it as a bound within new `*_many_unique`
methods to avoid the need for validation.
This is achieved using `Iterator`:
`EntitySet` is blanket implemented for any `T` that implements
`IntoIterator<IntoIter: EntitySetIterator>`.
`EntitySetIterator` is the unsafe trait that actually guarantees an
iterator to be "unique" via its safety contract.
We define an "Iterator over unique entities" as: "No two entities
returned by the iterator may compare equal."
For iterators that cannot return more than 1 element, this is trivially
true.
Whether an iterator can satisfy this is up to the `EntitySetIterator`
implementor to ensure, hence the unsafe.
However, this is not yet a complete solution. Looking at the signature
of `iter_many`, we find that `IntoIterator::Item` is not `Entity`, but
is instead bounded by the `Borrow<Entity>` trait. That is because
iteration without consuming the collection will often yield us
references, not owned items.
`Borrow<Entity>` presents an issue: The `Borrow` docs state that `x = y`
should equal `x.borrow() = y.borrow()`, but unsafe cannot rely on this
for soundness. We run into similar problems with other trait
implementations of any `Borrow<Entity>` type: `PartialEq`, `Eq`,
`PartialOrd`, `Ord`, `Hash`, `Clone`, `Borrow`, and `BorrowMut`.
This PR solves this with the unsafe `TrustedEntityBorrow` trait:
Any implementor promises that the behavior of the aforementioned traits
matches that of the underlying entity.
While `Borrow<Entity>` was the inspiration, we use our own counterpart
trait `EntityBorrow` as the supertrait to `TrustedEntityBorrow`, so we
can circumvent the limitations of the existing `Borrow<T>` blanket
impls.
All together, these traits allow us to implement `*_many_unique`
functionality with a lone `EntitySet` bound.
`EntitySetIterator` is implemented for all the std iterators and
iterator adapters that guarantee or preserve uniqueness, so we can
filter, skip, take, step, reverse, ... our unique entity iterators
without worry!
Sadly, current `HashSet` iterators do not carry the necessary type
information with them to determine whether the source `HashSet` produces
logic errors; A malicious `Hasher` could compromise a `HashSet`.
`HashSet` iteration is generally discouraged in the first place, so we
also exclude the set operation iterators, even though they do carry the
`Hasher` type parameter.
`BTreeSet` implements `EntitySet` without any problems.
If an iterator type cannot guarantee uniqueness at compile time, then a
user can still attach `EntitySetIterator` to an individual instance of
that type via `UniqueEntityIter::from_iterator_unchecked`.
With this, custom types can use `UniqueEntityIter<I>` as their
`IntoIterator::IntoIter` type, if necessary.
This PR is focused on the base concept, and expansions on it are left
for follow-up PRs. See "Potential Future Work" below.
## Testing
Doctests on `iter_many_unique`/`iter_many_unique_mut` + 2 tests in
entity_set.rs.
## Showcase
```rust
// Before:
fn system(player_list: Res<SomeUniquePlayerList>, players: Query<&mut Player>) {
let value = 0;
while let Some(player) = players.iter_many_mut(player_list).fetch_next() {
value += mem::take(player.value_mut())
}
}
// After:
fn system(player_list: Res<SomeUniquePlayerList>, players: Query<&mut Player>) {
let value = players
.iter_many_unique_mut(player_list)
.map(|player| mem::take(player.value_mut()))
.sum();
}
```
## Changelog
- added `EntityBorrow`, `TrustedEntityBorrow`, `EntitySet` and
`EntitySetIterator` traits
- added `iter_many_unique`, `iter_many_unique_mut`,
`iter_many_unique_unsafe` methods on `Query`
- added `iter_many_unique`, `iter_many_unique_mut`,
`iter_many_unique_manual` and `iter_many_unique_unchecked_manual`
methods on `QueryState`
- added corresponding `QueryManyUniqueIter`
- added `UniqueEntityIter`
## Migration Guide
Any custom type used as a `Borrow<Entity>` entity list item for an
`iter_many` method now has to implement `EntityBorrow` instead. Any type
that implements `Borrow<Entity>` can trivially implement `EntityBorrow`.
## Potential Future Work
- `ToEntitySet` trait for converting any entity iterator into an
`EntitySetIterator`
- `EntityIndexSet/Map` to tie in hashing with `EntitySet`
- add `EntityIndexSetSlice/MapSlice`
- requires: `EntityIndexSet/Map`
- Implementing `par_iter_many_unique_mut` for parallel mutable iteration
- requires: `par_iter_many`
- allow collecting into `UniqueEntityVec` to store entity sets
- add `UniqueEntitySlice`s
- Doesn't require, but should be done after: `UniqueEntityVec`
- add `UniqueEntityArray`s
- Doesn't require, but should be done after: `UniqueEntitySlice`
- `get_many_unique`/`many_unique` methods
- requires: `UniqueEntityArray`
- `World::entity_unique` to match `World::entity` methods
- Doesn't require, but makes sense after:
`get_many_unique`/`many_unique`
- implement `TrustedEntityBorrow` for the `EntityRef` family
- Doesn't require, but makes sense after: `UniqueEntityVec`
# Objective
The documentation for `Query::transmute_lens` lists some allowed
transmutes, but the list is incomplete.
## Solution
Document the underlying rules for what transmutes are allowed.
Add a longer list of examples. Write them as doc tests to ensure that
those examples are actually allowed.
I'm assuming that anything that can be done today is intended to be
supported! If any of these examples are things we plan to prohibit in
the future then we can add some warnings to that effect.
# Objective
In the [*Similar parameters* section of
`Query`](https://dev-docs.bevyengine.org/bevy/ecs/prelude/struct.Query.html#similar-parameters),
the doc link for `Single` actually links to `Query::single`, and
`Option<Single>` just links to `Option`. They should both link to
`Single`!
The first link is broken because there is a reference-style link defined
for `single`, but not for `Single`, and rustdoc treats the link as
case-insensitive for some reason.
## Solution
Fix the links!
## Testing
I built the docs locally with `cargo doc` and tested the links.
# Objective
- closes#15866
## Solution
- Simply migrate where possible.
## Testing
- Expect that CI will do most of the work. Examples is another way of
testing this, as most of the work is in that area.
---
## Notes
For now, this PR doesn't migrate `QueryState::single` and friends as for
now, this look like another issue. So for example, QueryBuilders that
used single or `World::query` that used single wasn't migrated. If there
is a easy way to migrate those, please let me know.
Most of the uses of `Query::single` were removed, the only other uses
that I found was related to tests of said methods, so will probably be
removed when we remove `Query::single`.
# Objective
Add a `Populated` system parameter that acts like `Query`, but prevents
system from running if there are no matching entities.
Fixes: #15302
## Solution
Implement the system param which newtypes the `Query`.
The only change is new validation, which fails if query is empty.
The new system param is used in `fallible_params` example.
## Testing
Ran `fallible_params` example.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
Add the following system params:
- `QuerySingle<D, F>` - Valid if only one matching entity exists,
- `Option<QuerySingle<D, F>>` - Valid if zero or one matching entity
exists.
As @chescock pointed out, we don't need `Mut` variants.
Fixes: #15264
## Solution
Implement the type and both variants of system params.
Also implement `ReadOnlySystemParam` for readonly queries.
Added a new ECS example `fallible_params` which showcases `SingleQuery`
usage.
In the future we might want to add `NonEmptyQuery`,
`NonEmptyEventReader` and `Res` to it (or maybe just stop at mentioning
it).
## Testing
Tested with the example.
There is a lot of warning spam so we might want to implement #15391.
# Objective
- Fixes#6370
- Closes#6581
## Solution
- Added the following lints to the workspace:
- `std_instead_of_core`
- `std_instead_of_alloc`
- `alloc_instead_of_core`
- Used `cargo +nightly fmt` with [item level use
formatting](https://rust-lang.github.io/rustfmt/?version=v1.6.0&search=#Item%5C%3A)
to split all `use` statements into single items.
- Used `cargo clippy --workspace --all-targets --all-features --fix
--allow-dirty` to _attempt_ to resolve the new linting issues, and
intervened where the lint was unable to resolve the issue automatically
(usually due to needing an `extern crate alloc;` statement in a crate
root).
- Manually removed certain uses of `std` where negative feature gating
prevented `--all-features` from finding the offending uses.
- Used `cargo +nightly fmt` with [crate level use
formatting](https://rust-lang.github.io/rustfmt/?version=v1.6.0&search=#Crate%5C%3A)
to re-merge all `use` statements matching Bevy's previous styling.
- Manually fixed cases where the `fmt` tool could not re-merge `use`
statements due to conditional compilation attributes.
## Testing
- Ran CI locally
## Migration Guide
The MSRV is now 1.81. Please update to this version or higher.
## Notes
- This is a _massive_ change to try and push through, which is why I've
outlined the semi-automatic steps I used to create this PR, in case this
fails and someone else tries again in the future.
- Making this change has no impact on user code, but does mean Bevy
contributors will be warned to use `core` and `alloc` instead of `std`
where possible.
- This lint is a critical first step towards investigating `no_std`
options for Bevy.
---------
Co-authored-by: François Mockers <francois.mockers@vleue.com>
# Objective
- Sometimes some method or function takes an owned `Query`, but we don't
want to give up ours;
- transmuting it technically a solution, but it more costly than
necessary.
- Make query iterators more flexible
- this would allow the equivalent of
`slice::split_first`/`slice::split_first_mut` for query iterators
- helps with requests like #14685
## Solution
- Add a way for reborrowing queries, that is going from a `&'a mut
Query<'w, 's, D, F>` to a `Query<'a, 's, D, F>`:
- this is safe because the original query will be borrowed while the new
query exists and thus no aliased access can happen;
- it's basically the equivalent of going from `&'short mut &'long mut T`
to `&'short mut T` the the compiler automatically implements.
- Add a way for getting the remainder of a query iterator:
- this is interesting also because the original iterator keeps its
position, which was not possible before;
- this in turn requires a way to reborrow query fetches, which I had to
add to `WorldQuery`.
## Showcase
- You can now reborrow a `Query`, getting an equivalent `Query` with a
shorter lifetime. Previously this was possible for read-only queries by
using `Query::to_readonly`, now it's possible for mutable queries too;
- You can now separately iterate over the remainder of `QueryIter`.
## Migration Guide
- `WorldQuery` now has an additional `shrink_fetch` method you have to
implement if you were implementing `WorldQuery` manually.
# Objective
- Fix#14629
## Solution
- Make `QueryState::transmute`, `QueryState::transmute_filtered`,
`QueryState::join` and `QueryState::join_filtered` take a `impl
Into<UnsafeWorldCell>` instead of a `&Components` and validate their
`WorldId`
## Migration Guide
- `QueryState::transmute`, `QueryState::transmute_filtered`,
`QueryState::join` and `QueryState::join_filtered` now take a `impl
Into<UnsafeWorldCell>` instead of a `&Components`
---------
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
# Objective
- Fix a typo in documentation for `Query::single_mut`
## Solution
- Change `item` to `items`
## Testing
- I built the documentation and it looked fine.
- Since this only affects a doc comment, no further testing should be
necessary.
---
## Changelog
> This section is optional. If this was a trivial fix, or has no
externally-visible impact, you can delete this section.
- Fixed a typo in the documentation for Query.
# Objective
Passing `&World` in the `WorldQuery::get_state` method is unnecessary,
as all implementations of this method in the engine either only access
`Components` in `&World`, or do nothing with it.
It can introduce UB by necessitating the creation of a `&World` from a
`UnsafeWorldCell`.
This currently happens in `Query::transmute_lens`, which obtains a
`&World` from the internal `UnsafeWorldCell` solely to pass to
`get_state`. `Query::join` suffers from the same issue.
Other cases of UB come from allowing implementors of `WorldQuery` to
freely access `&World`, like in the `bevy-trait-query` crate, where a
[reference to a resource is
obtained](0c0e7dd646/src/lib.rs (L445))
inside of
[`get_state`](0c0e7dd646/src/one.rs (L245)),
potentially aliasing with a `ResMut` parameter in the same system.
`WorldQuery::init_state` currently requires `&mut World`, which doesn't
suffer from these issues.
But that too can be changed to receive a wrapper around `&mut
Components` and `&mut Storages` for consistency in a follow-up PR.
## Solution
Replace the `&World` parameter in `get_state` with `&Components`.
## Changelog
`WorldQuery::get_state` now takes `&Components` instead of `&World`.
The `transmute`, `transmute_filtered`, `join` and `join_filtered`
methods on `QueryState` now similarly take `&Components` instead of
`&World`.
## Migration Guide
Users of `WorldQuery::get_state` or `transmute`, `transmute_filtered`,
`join` and `join_filtered` methods on `QueryState` now need to pass
`&Components` instead of `&World`.
`&Components` can be trivially obtained from either `components` method
on `&World` or `UnsafeWorldCell`.
For implementors of `WorldQuery::get_state` that were accessing more
than the `Components` inside `&World` and its methods, this is no longer
allowed.
# Objective
- Closes#12958
## Solution
- Find all methods under `Query` that mention panicking, and add
`#[track_caller]` to them.
---
## Changelog
- Added `#[track_caller]` to `Query::many`, `Query::many_mut`,
`Query::transmute_lens`, and `Query::transmute_lens_filtered`.
## For reviewers
I'm unfamiliar with the depths of the `Query` struct. Please check
whether it makes since for the updated methods to have
`#[track_caller]`, and if I missed any!
# Objective
Allow parallel iteration over events, resolve#10766
## Solution
- Add `EventParIter` which works similarly to `QueryParIter`,
implementing a `for_each{_with_id}` operator.
I chose to not mirror `EventIteratorWithId` and instead implement both
operations on a single struct.
- Reuse `BatchingStrategy` from `QueryParIter`
## Changelog
- `EventReader` now supports parallel event iteration using
`par_read().for_each(|event| ...)`.
---------
Co-authored-by: James Liu <contact@jamessliu.com>
Co-authored-by: Pablo Reinhardt <126117294+pablo-lua@users.noreply.github.com>
# Objective
Fixes#12752. Fixes#12750. Document the runtime complexity of all of
the `O(1)` operations on the individual APIs.
## Solution
* Mirror `Query::contains` onto `QueryState::contains`
* Make `QueryState::as_nop` pub(crate)
* Make `NopWorldQuery` pub(crate)
* Document all of the O(1) operations on Query and QueryState.
# Objective
Fixes#12392, fixes#12393, and fixes#11387. Implement QueryData for
Archetype and EntityLocation.
## Solution
Add impls for both of the types.
---
## Changelog
Added: `&Archetype` now implements `QueryData`
Added: `EntityLocation` now implements `QueryData`
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
I'm reading through the ecs query code for the first time, and updating
the docs:
- fixed some typos
- added some docs about things I was confused about (in particular what
the difference between `matches_component_set` and
`update_component_access` was)
# Objective
- Add a way to combine 2 queries together in a similar way to
`Query::transmute_lens`
- Fixes#1658
## Solution
- Use a similar method to query transmute, but take the intersection of
matched archetypes between the 2 queries and the union of the accesses
to create the new underlying QueryState.
---
## Changelog
- Add query joins
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
Fix#10876. Improve `Query` and `QueryState`'s docs.
## Solution
Explicitly denote that Query is always guaranteed to return results from
all matching entities once and only once for each entity, and that
iteration order is not guaranteed in any way.
# 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
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.
