# 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
Prevents duplicate implementation between IntoSystemConfigs and
IntoSystemSetConfigs using a generic, adds a NodeType trait for more
config flexibility (opening the door to implement
https://github.com/bevyengine/bevy/issues/14195?).
## Solution
Followed writeup by @ItsDoot:
https://hackmd.io/@doot/rJeefFHc1x
Removes IntoSystemConfigs and IntoSystemSetConfigs, instead using
IntoNodeConfigs with generics.
## Testing
Pending
---
## Showcase
N/A
## Migration Guide
SystemSetConfigs -> NodeConfigs<InternedSystemSet>
SystemConfigs -> NodeConfigs<ScheduleSystem>
IntoSystemSetConfigs -> IntoNodeConfigs<InternedSystemSet, M>
IntoSystemConfigs -> IntoNodeConfigs<ScheduleSystem, M>
---------
Co-authored-by: Christian Hughes <9044780+ItsDoot@users.noreply.github.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
- Prevent usage of `println!`, `eprintln!` and the like because they
require `std`
- Fixes#17446
## Solution
- Enable the `print_stdout` and `print_stderr` clippy lints
- Replace all `println!` and `eprintln!` occurrences with `log::*` where
applicable or alternatively ignore the warnings
## Testing
- Run `cargo clippy --workspace` to ensure that there are no warnings
relating to printing to `stdout` or `stderr`
# Objective
Part of the #16547 series.
The entity wrapper types often have some associated types an aliases
with them that cannot be re-exported into an outer module together.
Some helper types are best used with part of their path:
`bevy::ecs::entity::index_set::Slice` as `index_set::Slice`.
This has already been done for `entity::hash_set` and
`entity::hash_map`.
## Solution
Publicize the `index_set`, `index_map`, `unique_vec`, `unique_slice`,
and `unique_array` modules.
## Migration Guide
Any mention or import of types in the affected modules have to add the
respective module name to the import path.
F.e.:
`bevy::ecs::entity::EntityIndexSet` ->
`bevy::ecs::entity::index_set::EntityIndexSet`
# Objective
This is an alternative to #17871 and #17701 for tracking issue #18155.
This thanks to @maniwani for help with this design.
The goal is to enable component ids to be reserved from multiple threads
concurrently and with only `&World`. This contributes to assets as
entities, read-only query and system parameter initialization, etc.
## What's wrong with #17871 ?
In #17871, I used my proposed staging utilities to allow *fully*
registering components from any thread concurrently with only
`&Components`. However, if we want to pursue components as entities
(which is desirable for a great many reasons. See
[here](https://discord.com/channels/691052431525675048/692572690833473578/1346499196655505534)
on discord), this staging isn't going to work. After all, if registering
a component requires spawning an entity, and spawning an entity requires
`&mut World`, it is impossible to register a component fully with only
`&World`.
## Solution
But what if we don't have to register it all the way? What if it's
enough to just know the `ComponentId` it will have once it is registered
and to queue it to be registered at a later time? Spoiler alert: That is
all we need for these features.
Here's the basic design:
Queue a registration:
1. Check if it has already been registered.
2. Check if it has already been queued.
3. Reserve a `ComponentId`.
4. Queue the registration at that id.
Direct (normal) registration:
1. Check if this registration has been queued.
2. If it has, use the queued registration instead.
3. Otherwise, proceed like normal.
Appllying the queue:
1. Pop queued items off one by one.
2. Register them directly.
One other change:
The whole point of this design over #17871 is to facilitate coupling
component registration with the World. To ensure that this would fully
work with that, I went ahead and moved the `ComponentId` generator onto
the world itself. That stemmed a couple of minor organizational changes
(see migration guide). As we do components as entities, we will replace
this generator with `Entities`, which lives on `World` too. Doing this
move early let me verify the design and will reduce migration headaches
in the future. If components as entities is as close as I think it is, I
don't think splitting this up into different PRs is worth it. If it is
not as close as it is, it might make sense to still do #17871 in the
meantime (see the risks section). I'll leave it up to y'all what we end
up doing though.
## Risks and Testing
The biggest downside of this compared to #17871 is that now we have to
deal with correct but invalid `ComponentId`s. They are invalid because
the component still isn't registered, but they are correct because, once
registered, the component will have exactly that id.
However, the only time this becomes a problem is if some code violates
safety rules by queuing a registration and using the returned id as if
it was valid. As this is a new feature though, nothing in Bevy does
this, so no new tests were added for it. When we do use it, I left
detailed docs to help mitigate issues here, and we can test those
usages. Ex: we will want some tests on using queries initialized from
queued registrations.
## Migration Guide
Component registration can now be queued with only `&World`. To
facilitate this, a few APIs needed to be moved around.
The following functions have moved from `Components` to
`ComponentsRegistrator`:
- `register_component`
- `register_component_with_descriptor`
- `register_resource_with_descriptor`
- `register_non_send`
- `register_resource`
- `register_required_components_manual`
Accordingly, functions in `Bundle` and `Component` now take
`ComponentsRegistrator` instead of `Components`.
You can obtain `ComponentsRegistrator` from the new
`World::components_registrator`.
You can obtain `ComponentsQueuedRegistrator` from the new
`World::components_queue`, and use it to stage component registration if
desired.
# Open Question
Can we verify that it is enough to queue registration with `&World`? I
don't think it would be too difficult to package this up into a
`Arc<MyComponentsManager>` type thing if we need to, but keeping this on
`&World` certainly simplifies things. If we do need the `Arc`, we'll
need to look into partitioning `Entities` for components as entities, so
we can keep most of the allocation fast on `World` and only keep a
smaller partition in the `Arc`. I'd love an SME on assets as entities to
shed some light on this.
---------
Co-authored-by: andriyDev <andriydzikh@gmail.com>
# 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
Fixes#18092
Bevy's current error type is a simple type alias for `Box<dyn Error +
Send + Sync + 'static>`. This largely works as a catch-all error, but it
is missing a critical feature: the ability to capture a backtrace at the
point that the error occurs. The best way to do this is `anyhow`-style
error handling: a new error type that takes advantage of the fact that
the `?` `From` conversion happens "inline" to capture the backtrace at
the point of the error.
## Solution
This PR adds a new `BevyError` type (replacing our old
`std::error::Error` type alias), which uses the "from conversion
backtrace capture" approach:
```rust
fn oh_no() -> Result<(), BevyError> {
// this fails with Rust's built in ParseIntError, which
// is converted into the catch-all BevyError type
let number: usize = "hi".parse()?;
println!("parsed {number}");
Ok(())
}
```
This also updates our exported `Result` type alias to default to
`BevyError`, meaning you can write this instead:
```rust
fn oh_no() -> Result {
let number: usize = "hi".parse()?;
println!("parsed {number}");
Ok(())
}
```
When a BevyError is encountered in a system, it will use Bevy's default
system error handler (which panics by default). BevyError does custom
"backtrace filtering" by default, meaning we can cut out the _massive_
amount of "rust internals", "async executor internals", and "bevy system
scheduler internals" that show up in backtraces. It also trims out the
first generally-unnecssary `From` conversion backtrace lines that make
it harder to locate the real error location. The result is a blissfully
simple backtrace by default:
The full backtrace can be shown by setting the `BEVY_BACKTRACE=full`
environment variable. Non-BevyError panics still use the default Rust
backtrace behavior.
One issue that prevented the truly noise-free backtrace during panics
that you see above is that Rust's default panic handler will print the
unfiltered (and largely unhelpful real-panic-point) backtrace by
default, in _addition_ to our filtered BevyError backtrace (with the
helpful backtrace origin) that we capture and print. To resolve this, I
have extended Bevy's existing PanicHandlerPlugin to wrap the default
panic handler. If we panic from the result of a BevyError, we will skip
the default "print full backtrace" panic handler. This behavior can be
enabled and disabled using the new `error_panic_hook` cargo feature in
`bevy_app` (which is enabled by default).
One downside to _not_ using `Box<dyn Error>` directly is that we can no
longer take advantage of the built-in `Into` impl for strings to errors.
To resolve this, I have added the following:
```rust
// Before
Err("some error")?
// After
Err(BevyError::message("some error"))?
```
We can discuss adding shorthand methods or macros for this (similar to
anyhow's `anyhow!("some error")` macro), but I'd prefer to discuss that
later.
I have also added the following extension method:
```rust
// Before
some_option.ok_or("some error")?;
// After
some_option.ok_or_message("some error")?;
```
I've also moved all of our existing error infrastructure from
`bevy_ecs::result` to `bevy_ecs::error`, as I think that is the better
home for it
## Why not anyhow (or eyre)?
The biggest reason is that `anyhow` needs to be a "generically useful
error type", whereas Bevy is a much narrower scope. By using our own
error, we can be significantly more opinionated. For example, anyhow
doesn't do the extensive (and invasive) backtrace filtering that
BevyError does because it can't operate on Bevy-specific context, and
needs to be generically useful.
Bevy also has a lot of operational context (ex: system info) that could
be useful to attach to errors. If we have control over the error type,
we can add whatever context we want to in a structured way. This could
be increasingly useful as we add more visual / interactive error
handling tools and editor integrations.
Additionally, the core approach used is simple and requires almost no
code. anyhow clocks in at ~2500 lines of code, but the impl here uses
160. We are able to boil this down to exactly what we need, and by doing
so we improve our compile times and the understandability of our code.
# 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
- Fixes#16339
## Solution
- Replaced `component_reads_and_writes` and `component_writes` with
`try_iter_component_access`.
## Testing
- Ran `dynamic` example to confirm behaviour is unchanged.
- CI
---
## Migration Guide
The following methods (some removed in previous PRs) are now replaced by
`Access::try_iter_component_access`:
* `Access::component_reads_and_writes`
* `Access::component_reads`
* `Access::component_writes`
As `try_iter_component_access` returns a `Result`, you'll now need to
handle the failing case (e.g., `unwrap()`). There is currently a single
failure mode, `UnboundedAccess`, which occurs when the `Access` is for
all `Components` _except_ certain exclusions. Since this list is
infinite, there is no meaningful way for `Access` to provide an
iterator. Instead, get a list of components (e.g., from the `Components`
structure) and iterate over that instead, filtering using
`Access::has_component_read`, `Access::has_component_write`, etc.
Additionally, you'll need to `filter_map` the accesses based on which
method you're attempting to replace:
* `Access::component_reads_and_writes` -> `Exclusive(_) | Shared(_)`
* `Access::component_reads` -> `Shared(_)`
* `Access::component_writes` -> `Exclusive(_)`
To ease migration, please consider the below extension trait which you
can include in your project:
```rust
pub trait AccessCompatibilityExt {
/// Returns the indices of the components this has access to.
fn component_reads_and_writes(&self) -> impl Iterator<Item = T> + '_;
/// Returns the indices of the components this has non-exclusive access to.
fn component_reads(&self) -> impl Iterator<Item = T> + '_;
/// Returns the indices of the components this has exclusive access to.
fn component_writes(&self) -> impl Iterator<Item = T> + '_;
}
impl<T: SparseSetIndex> AccessCompatibilityExt for Access<T> {
fn component_reads_and_writes(&self) -> impl Iterator<Item = T> + '_ {
self
.try_iter_component_access()
.expect("Access is unbounded. Please refactor the usage of this method to directly use try_iter_component_access")
.filter_map(|component_access| {
let index = component_access.index().sparse_set_index();
match component_access {
ComponentAccessKind::Archetypal(_) => None,
ComponentAccessKind::Shared(_) => Some(index),
ComponentAccessKind::Exclusive(_) => Some(index),
}
})
}
fn component_reads(&self) -> impl Iterator<Item = T> + '_ {
self
.try_iter_component_access()
.expect("Access is unbounded. Please refactor the usage of this method to directly use try_iter_component_access")
.filter_map(|component_access| {
let index = component_access.index().sparse_set_index();
match component_access {
ComponentAccessKind::Archetypal(_) => None,
ComponentAccessKind::Shared(_) => Some(index),
ComponentAccessKind::Exclusive(_) => None,
}
})
}
fn component_writes(&self) -> impl Iterator<Item = T> + '_ {
self
.try_iter_component_access()
.expect("Access is unbounded. Please refactor the usage of this method to directly use try_iter_component_access")
.filter_map(|component_access| {
let index = component_access.index().sparse_set_index();
match component_access {
ComponentAccessKind::Archetypal(_) => None,
ComponentAccessKind::Shared(_) => None,
ComponentAccessKind::Exclusive(_) => Some(index),
}
})
}
}
```
Please take note of the use of `expect(...)` in these methods. You
should consider using these as a starting point for a more appropriate
migration based on your specific needs.
## Notes
- This new method is fallible based on whether the `Access` is bounded
or unbounded (unbounded occurring with inverted component sets). If
bounded, will return an iterator of every item and its access level. I
believe this makes sense without exposing implementation details around
`Access`.
- The access level is defined by an `enum` `ComponentAccessKind<T>`,
either `Archetypical`, `Shared`, or `Exclusive`. As a convenience, this
`enum` has a method `index` to get the inner `T` value without a match
statement. It does add more code, but the API is clearer.
- Within `QueryBuilder` this new method simplifies several pieces of
logic without changing behaviour.
- Within `QueryState` the logic is simplified and the amount of
iteration is reduced, potentially improving performance.
- Within the `dynamic` example it has identical behaviour, with the
inversion footgun explicitly highlighted by an `unwrap`.
---------
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
Co-authored-by: Mike <2180432+hymm@users.noreply.github.com>
The test case `query_iter_sorts` was doing lots of comparisons to ensure
that various query arrays were sorted, but the arrays were all empty.
This PR spawns some entities so that the entity lists to compare not
empty, and sorting can actually be tested for correctness.
# 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
`QueryIter::sort_by()` is unsound. It passes the lens items with the
full `'w` lifetime, and a malicious user could smuggle them out of the
closure where they could alias with the query results.
## Solution
Make the sort closures generic in the lifetime parameter of the lens
item. This ensures the lens items cannot outlive the call to the
closure.
## Testing
Added a compile-fail test that demonstrates the unsound pattern.
## Migration Guide
The `sort` family of methods on `QueryIter` unsoundly gave access
`L::Item<'w>` with the full `'w` lifetime. It has been shortened to
`L::Item<'w>` so that items cannot escape the comparer. If you get
lifetime errors using these methods, you will need to make the comparer
generic in the new lifetime. Often this can be done by replacing named
`'w` with `'_`, or by replacing the use of a function item with a
closure.
```rust
// Before: Now fails with "error: implementation of `FnMut` is not general enough"
query.iter().sort_by::<&C>(Ord::cmp);
// After: Wrap in a closure
query.iter().sort_by::<&C>(|l, r| Ord::cmp(l, r));
query.iter().sort_by::<&C>(comparer);
// Before: Uses specific `'w` lifetime from some outer scope
// now fails with "error: implementation of `FnMut` is not general enough"
fn comparer(left: &&'w C, right: &&'w C) -> Ordering { /* ... */ }
// After: Accepts any lifetime using inferred lifetime parameter
fn comparer(left: &&C, right: &&C) -> Ordering { /* ... */ }
# Objective
- Fixes#17960
## Solution
- Followed the [edition upgrade
guide](https://doc.rust-lang.org/edition-guide/editions/transitioning-an-existing-project-to-a-new-edition.html)
## Testing
- CI
---
## Summary of Changes
### Documentation Indentation
When using lists in documentation, proper indentation is now linted for.
This means subsequent lines within the same list item must start at the
same indentation level as the item.
```rust
/* Valid */
/// - Item 1
/// Run-on sentence.
/// - Item 2
struct Foo;
/* Invalid */
/// - Item 1
/// Run-on sentence.
/// - Item 2
struct Foo;
```
### Implicit `!` to `()` Conversion
`!` (the never return type, returned by `panic!`, etc.) no longer
implicitly converts to `()`. This is particularly painful for systems
with `todo!` or `panic!` statements, as they will no longer be functions
returning `()` (or `Result<()>`), making them invalid systems for
functions like `add_systems`. The ideal fix would be to accept functions
returning `!` (or rather, _not_ returning), but this is blocked on the
[stabilisation of the `!` type
itself](https://doc.rust-lang.org/std/primitive.never.html), which is
not done.
The "simple" fix would be to add an explicit `-> ()` to system
signatures (e.g., `|| { todo!() }` becomes `|| -> () { todo!() }`).
However, this is _also_ banned, as there is an existing lint which (IMO,
incorrectly) marks this as an unnecessary annotation.
So, the "fix" (read: workaround) is to put these kinds of `|| -> ! { ...
}` closuers into variables and give the variable an explicit type (e.g.,
`fn()`).
```rust
// Valid
let system: fn() = || todo!("Not implemented yet!");
app.add_systems(..., system);
// Invalid
app.add_systems(..., || todo!("Not implemented yet!"));
```
### Temporary Variable Lifetimes
The order in which temporary variables are dropped has changed. The
simple fix here is _usually_ to just assign temporaries to a named
variable before use.
### `gen` is a keyword
We can no longer use the name `gen` as it is reserved for a future
generator syntax. This involved replacing uses of the name `gen` with
`r#gen` (the raw-identifier syntax).
### Formatting has changed
Use statements have had the order of imports changed, causing a
substantial +/-3,000 diff when applied. For now, I have opted-out of
this change by amending `rustfmt.toml`
```toml
style_edition = "2021"
```
This preserves the original formatting for now, reducing the size of
this PR. It would be a simple followup to update this to 2024 and run
`cargo fmt`.
### New `use<>` Opt-Out Syntax
Lifetimes are now implicitly included in RPIT types. There was a handful
of instances where it needed to be added to satisfy the borrow checker,
but there may be more cases where it _should_ be added to avoid
breakages in user code.
### `MyUnitStruct { .. }` is an invalid pattern
Previously, you could match against unit structs (and unit enum
variants) with a `{ .. }` destructuring. This is no longer valid.
### Pretty much every use of `ref` and `mut` are gone
Pattern binding has changed to the point where these terms are largely
unused now. They still serve a purpose, but it is far more niche now.
### `iter::repeat(...).take(...)` is bad
New lint recommends using the more explicit `iter::repeat_n(..., ...)`
instead.
## Migration Guide
The lifetimes of functions using return-position impl-trait (RPIT) are
likely _more_ conservative than they had been previously. If you
encounter lifetime issues with such a function, please create an issue
to investigate the addition of `+ use<...>`.
## Notes
- Check the individual commits for a clearer breakdown for what
_actually_ changed.
---------
Co-authored-by: François Mockers <francois.mockers@vleue.com>
## 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
Currently, default query filters, as added in #13120 / #17514 are
hardcoded to only use a single query filter.
This is limiting, as multiple distinct disabling components can serve
important distinct roles. I ran into this limitation when experimenting
with a workflow for prefabs, which don't represent the same state as "an
entity which is temporarily nonfunctional".
## Solution
1. Change `DefaultQueryFilters` to store a SmallVec of ComponentId,
rather than an Option.
2. Expose methods on `DefaultQueryFilters`, `World` and `App` to
actually configure this.
3. While we're here, improve the docs, write some tests, make use of
FromWorld and make some method names more descriptive.
## Follow-up
I'm not convinced that supporting sparse set disabling components is
useful, given the hit to iteration performance and runtime checks
incurred. That's disjoint from this PR though, so I'm not doing it here.
The existing warnings are fine for now.
## Testing
I've added both a doc test and an mid-level unit test to verify that
this works!
# 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
Eliminate the need to write `cfg(feature = "track_location")` every time
one uses an API that may use location tracking. It's verbose, and a
little intimidating. And it requires code outside of `bevy_ecs` that
wants to use location tracking needs to either unconditionally enable
the feature, or include conditional compilation of its own. It would be
good for users to be able to log locations when they are available
without needing to add feature flags to their own crates.
Reduce the number of cases where code compiles with the `track_location`
feature enabled, but not with it disabled, or vice versa. It can be hard
to remember to test it both ways!
Remove the need to store a `None` in `HookContext` when the
`track_location` feature is disabled.
## Solution
Create an `MaybeLocation<T>` type that contains a `T` if the
`track_location` feature is enabled, and is a ZST if it is not. The
overall API is similar to `Option`, but whether the value is `Some` or
`None` is set at compile time and is the same for all values.
Default `T` to `&'static Location<'static>`, since that is the most
common case.
Remove all `cfg(feature = "track_location")` blocks outside of the
implementation of that type, and instead call methods on it.
When `track_location` is disabled, `MaybeLocation` is a ZST and all
methods are `#[inline]` and empty, so they should be entirely removed by
the compiler. But the code will still be visible to the compiler and
checked, so if it compiles with the feature disabled then it should also
compile with it enabled, and vice versa.
## Open Questions
Where should these types live? I put them in `change_detection` because
that's where the existing `MaybeLocation` types were, but we now use
these outside of change detection.
While I believe that the compiler should be able to remove all of these
calls, I have not actually tested anything. If we want to take this
approach, what testing is required to ensure it doesn't impact
performance?
## Migration Guide
Methods like `Ref::changed_by()` that return a `&'static
Location<'static>` will now be available even when the `track_location`
feature is disabled, but they will return a new `MaybeLocation` type.
`MaybeLocation` wraps a `&'static Location<'static>` when the feature is
enabled, and is a ZST when the feature is disabled.
Existing code that needs a `&Location` can call `into_option().unwrap()`
to recover it. Many trait impls are forwarded, so if you only need
`Display` then no changes will be necessary.
If that code was conditionally compiled, you may instead want to use the
methods on `MaybeLocation` to remove the need for conditional
compilation.
Code that constructs a `Ref`, `Mut`, `Res`, or `ResMut` will now need to
provide location information unconditionally. If you are creating them
from existing Bevy types, you can obtain a `MaybeLocation` from methods
like `Table::get_changed_by_slice_for()` or
`ComponentSparseSet::get_with_ticks`. Otherwise, you will need to store
a `MaybeLocation` next to your data and use methods like `as_ref()` or
`as_mut()` to obtain wrapped references.
This pr uses the `extern crate self as` trick to make proc macros behave
the same way inside and outside bevy.
# Objective
- Removes noise introduced by `crate as` in the whole bevy repo.
- Fixes#17004.
- Hardens proc macro path resolution.
## TODO
- [x] `BevyManifest` needs cleanup.
- [x] Cleanup remaining `crate as`.
- [x] Add proper integration tests to the ci.
## Notes
- `cargo-manifest-proc-macros` is written by me and based/inspired by
the old `BevyManifest` implementation and
[`bkchr/proc-macro-crate`](https://github.com/bkchr/proc-macro-crate).
- What do you think about the new integration test machinery I added to
the `ci`?
More and better integration tests can be added at a later stage.
The goal of these integration tests is to simulate an actual separate
crate that uses bevy. Ideally they would lightly touch all bevy crates.
## Testing
- Needs RA test
- Needs testing from other users
- Others need to run at least `cargo run -p ci integration-test` and
verify that they work.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
Progresses #17569. The end goal here is to synchronize component
registration. See the other PR for details for the motivation behind
that.
For this PR specifically, the objective is to decouple `Components` from
`Storages`. What components are registered etc should have nothing to do
with what Storages looks like. Storages should only care about what
entity archetypes have been spawned.
## Solution
Previously, this was used to create sparse sets for relevant components
when those components were registered. Now, we do that when the
component is inserted/spawned.
This PR proposes doing that in `BundleInfo::new`, but there may be a
better place.
## Testing
In theory, this shouldn't have changed any functionality, so no new
tests were created. I'm not aware of any examples that make heavy use of
sparse set components either.
## Migration Guide
- Remove storages from functions where it is no longer needed.
- Note that SparseSets are no longer present for all registered sparse
set components, only those that have been spawned.
---------
Co-authored-by: SpecificProtagonist <vincentjunge@posteo.net>
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
# Objective
Fixes#17662
## Solution
Moved `Item` and `fetch` from `WorldQuery` to `QueryData`, and adjusted
their implementations accordingly.
Currently, documentation related to `fetch` is written under
`WorldQuery`. It would be more appropriate to move it to the `QueryData`
documentation for clarity.
I am not very experienced with making contributions. If there are any
mistakes or areas for improvement, I would appreciate any suggestions
you may have.
## Migration Guide
The `WorldQuery::Item` type and `WorldQuery::fetch` method have been
moved to `QueryData`, as they were not useful for `QueryFilter` types.
---------
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
# 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`.
# Objective
While working on #17649, I found the docs for `WorldQuery` and the
related traits frustratingly vague.
## Solution
Clarify them and add some more tangible advice.
Also fix a copy-pasted typo in related comments.
---------
Co-authored-by: James O'Brien <james.obrien@drafly.net>
# Objective
Prevent unsound uses of `DeferredWorld` as a `SystemParam`. It is
currently unsound because it does not check for existing access, and
because it incorrectly registers filtered access.
## Solution
Have `DeferredWorld` panic if a previous parameter has conflicting
access.
Have `DeferredWorld` update `archetype_component_access` so that the
multi-threaded executor sees the access.
Fix `FilteredAccessSet::read_all()` and `write_all()` to correctly add a
`FilteredAccess` with no filter so that `Query` is able to detect the
conflicts.
Remove redundant `read_all()` call, since `write_all()` already declares
read access.
Remove unnecessary `set_has_deferred()` call, since `<DeferredWorld as
SystemParam>::apply_deferred()` does nothing. Previously we were
inserting unnecessary `apply_deferred` systems in the schedule.
## Testing
Added unit tests for systems where `DeferredWorld` conflicts with a
`Query` in the same system.
# Objective
The various `Query::sort()` methods have a lot of duplicated code
between them, including some unsafe code. Reduce the duplication to make
the code easier to read and maintain.
## Solution
Extract the duplicated code to a private method, and pass in the sorting
strategy as a closure.
## Testing
I used `cargo-show-asm` to verify that the closures were inlined, but I
didn't run anything through a profiler. The `sort()` method itself even
had identical assembly before and after this change, although the others
did not.
# Objective
Some usecases in the ecosystems are blocked by the inability to stop
bevy internals and third party plugins from touching their entities.
However the specifics of a general purpose entity disabling system are
controversial and further complicated by hierarchies. We can partially
unblock these usecases with an opt-in approach: default query filters.
## Solution
- Introduce DefaultQueryFilters, these filters are automatically applied
to queries that don't otherwise mention the filtered component.
- End users and third party plugins can register default filters and are
responsible for handling entities they have hidden this way.
- Extra features can be left for after user feedback
- The default value could later include official ways to hide entities
---
## Changelog
- Add DefaultQueryFilters
# Objective
- Fix issue identified on the [Discord
server](https://discord.com/channels/691052431525675048/691052431974465548/1328922812530036839)
## Solution
- Implement `Clone` for `QueryIter` using the existing
`QueryIter::remaining` method
## Testing
- CI
---
## Showcase
Users can now explicitly clone a read-only `QueryIter`:
```rust
fn combinations(query: Query<&ComponentA>) {
let mut iter = query.iter();
while let Some(a) = iter.next() {
// Can now clone rather than use remaining
for b in iter.clone() {
// Check every combination (a, b)
}
}
}
```
## Notes
This doesn't add any new functionality outside the context of generic
code (e.g., `T: Iterator<...> + Clone`), it's mostly for
discoverability. Users are more likely to be familiar with
`Clone::clone` than they are with the methods on `QueryIter`.
# Objective
- https://github.com/bevyengine/bevy/issues/17111
## Solution
Set the `clippy::allow_attributes` and
`clippy::allow_attributes_without_reason` lints to `warn`, and bring
`bevy_ecs` in line with the new restrictions.
## Testing
This PR is a WIP; testing will happen after it's finished.
# Objective
With the `track_location` feature, the error message of trying to
acquire an entity that was despawned pointed to the wrong line if the
entity index has been reused.
## Showcase
```rust
use bevy_ecs::prelude::*;
fn main() {
let mut world = World::new();
let e = world.spawn_empty().id();
world.despawn(e);
world.flush();
let _ = world.spawn_empty();
world.entity(e);
}
```
Old message:
```
Entity 0v1 was despawned by src/main.rs:8:19
```
New message:
```
Entity 0v1 does not exist (its index has been reused)
```
# Objective
Stumbled upon a `from <-> form` transposition while reviewing a PR,
thought it was interesting, and went down a bit of a rabbit hole.
## Solution
Fix em
Related to https://github.com/bevyengine/bevy/pull/16843
Since `WorldQuery::Fetch` is `Clone`, it can't store mutable references
to resources, so it doesn't make sense to mutably access resources. In
that sense, it is hard to find usecases of mutably accessing resources
and to clearly define, what mutably accessing resources would mean, so
it's been decided to disallow write resource access.
Also changed documentation of safety requirements of
`WorldQuery::init_fetch` and `WorldQuery::fetch` to clearly state to the
caller, what safety invariants they need to uphold.
# Objective
- Contributes to #11478
## Solution
- Made `bevy_utils::tracing` `doc(hidden)`
- Re-exported `tracing` from `bevy_log` for end-users
- Added `tracing` directly to crates that need it.
## Testing
- CI
---
## Migration Guide
If you were importing `tracing` via `bevy::utils::tracing`, instead use
`bevy::log::tracing`. Note that many items within `tracing` are also
directly re-exported from `bevy::log` as well, so you may only need
`bevy::log` for the most common items (e.g., `warn!`, `trace!`, etc.).
This also applies to the `log_once!` family of macros.
## Notes
- While this doesn't reduce the line-count in `bevy_utils`, it further
decouples the internal crates from `bevy_utils`, making its eventual
removal more feasible in the future.
- I have just imported `tracing` as we do for all dependencies. However,
a workspace dependency may be more appropriate for version management.
## Objective
The error `EntityFetchError::NoSuchEntity` has an `UnsafeWorldCell`
inside it, which it uses to call
`Entities::entity_does_not_exist_error_details_message` when being
printed. That method returns a `String` that, if the `track_location`
feature is enabled, contains the location of whoever despawned the
relevant entity.
I initially had to modify this error while working on #17043. The
`UnsafeWorldCell` was causing borrow problems when being returned from a
command, so I tried replacing it with the `String` that the method
returns, since that was the world cell's only purpose.
Unfortunately, `String`s are slow, and it significantly impacted
performance (on top of that PR's performance hit):
<details>
<summary>17043 benchmarks</summary>
### With `String`
### No `String`
</details>
For that PR, I just removed the error details entirely, but I figured
I'd try to find a way to keep them around.
## Solution
- Replace the `String` with a helper struct that holds the location, and
only turn it into a string when someone actually wants to print it.
- Replace the `UnsafeWorldCell` with the aforementioned struct.
- Do the same for `QueryEntityError::NoSuchEntity`.
## Benchmarking
This had some interesting performance impact:
<details>
<summary>This PR vs main</summary>
</details>
## Other work
`QueryEntityError::QueryDoesNotMatch` also has an `UnsafeWorldCell`
inside it. This one would be more complicated to rework while keeping
the same functionality.
## Migration Guide
The errors `EntityFetchError::NoSuchEntity` and
`QueryEntityError::NoSuchEntity` now contain an
`EntityDoesNotExistDetails` struct instead of an `UnsafeWorldCell`. If
you were just printing these, they should work identically.
---------
Co-authored-by: Benjamin Brienen <benjamin.brienen@outlook.com>
# Background
In `no_std` compatible crates, there is often an `std` feature which
will allow access to the standard library. Currently, with the `std`
feature _enabled_, the
[`std::prelude`](https://doc.rust-lang.org/std/prelude/index.html) is
implicitly imported in all modules. With the feature _disabled_, instead
the [`core::prelude`](https://doc.rust-lang.org/core/prelude/index.html)
is implicitly imported. This creates a subtle and pervasive issue where
`alloc` items _may_ be implicitly included (if `std` is enabled), or
must be explicitly included (if `std` is not enabled).
# Objective
- Make the implicit imports for `no_std` crates consistent regardless of
what features are/not enabled.
## Solution
- Replace the `cfg_attr` "double negative" `no_std` attribute with
conditional compilation to _include_ `std` as an external crate.
```rust
// Before
#![cfg_attr(not(feature = "std"), no_std)]
// After
#![no_std]
#[cfg(feature = "std")]
extern crate std;
```
- Fix imports that are currently broken but are only now visible with
the above fix.
## Testing
- CI
## Notes
I had previously used the "double negative" version of `no_std` based on
general consensus that it was "cleaner" within the Rust embedded
community. However, this implicit prelude issue likely was considered
when forming this consensus. I believe the reason why is the items most
affected by this issue are provided by the `alloc` crate, which is
rarely used within embedded but extensively used within Bevy.
# Objective
- As stated in the related issue, this PR is to better align the feature
flag name with what it actually does and the plans for the future.
- Fixes#16852
## Solution
- Simple find / replace
## Testing
- Local run of `cargo run -p ci`
## Migration Guide
The `track_change_detection` feature flag has been renamed to
`track_location` to better reflect its extended capabilities.
# Objective
- Made certain methods public for advanced use cases. Methods that
returns mutable references are marked as unsafe due to the possibility
of violating internal lifetime constraint assumptions.
- Fixes an issue introduced by #15184
# 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
Some types like `RenderEntity` and `MainEntity` are just wrappers around
`Entity`, so they should be able to implement
`EntityBorrow`/`TrustedEntityBorrow`. This allows using them with
`EntitySet` functionality.
The `EntityRef` family are more than direct wrappers around `Entity`,
but can still benefit from being unique in a collection.
## Solution
Implement `EntityBorrow` and `TrustedEntityBorrow` for simple `Entity`
newtypes and `EntityRef` types.
These impls are an explicit decision to have the `EntityRef` types
compare like just `Entity`.
`EntityWorldMut` is omitted from this impl, because it explicitly
contains a `&mut World` as well, and we do not ever use more than one at
a time.
Add `EntityBorrow` to the `bevy_ecs` prelude.
## Migration Guide
`NormalizedWindowRef::entity` has been replaced with an
`EntityBorrow::entity` impl.
# 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
- Contributes to #15460
## Solution
- Added the following features:
- `std` (default)
- `async_executor` (default)
- `edge_executor`
- `critical-section`
- `portable-atomic`
- Gated `tracing` in `bevy_utils` to allow compilation on certain
platforms
- Switched from `tracing` to `log` for simple message logging within
`bevy_ecs`. Note that `tracing` supports capturing from `log` so this
should be an uncontroversial change.
- Fixed imports and added feature gates as required
- Made `bevy_tasks` optional within `bevy_ecs`. Turns out it's only
needed for parallel operations which are already gated behind
`multi_threaded` anyway.
## Testing
- Added to `compile-check-no-std` CI command
- `cargo check -p bevy_ecs --no-default-features --features
edge_executor,critical-section,portable-atomic --target
thumbv6m-none-eabi`
- `cargo check -p bevy_ecs --no-default-features --features
edge_executor,critical-section`
- `cargo check -p bevy_ecs --no-default-features`
## Draft Release Notes
Bevy's core ECS now supports `no_std` platforms.
In prior versions of Bevy, it was not possible to work with embedded or
niche platforms due to our reliance on the standard library, `std`. This
has blocked a number of novel use-cases for Bevy, such as an embedded
database for IoT devices, or for creating games on retro consoles.
With this release, `bevy_ecs` no longer requires `std`. To use Bevy on a
`no_std` platform, you must disable default features and enable the new
`edge_executor` and `critical-section` features. You may also need to
enable `portable-atomic` and `critical-section` if your platform does
not natively support all atomic types and operations used by Bevy.
```toml
[dependencies]
bevy_ecs = { version = "0.16", default-features = false, features = [
# Required for platforms with incomplete atomics (e.g., Raspberry Pi Pico)
"portable-atomic",
"critical-section",
# Optional
"bevy_reflect",
"serialize",
"bevy_debug_stepping",
"edge_executor"
] }
```
Currently, this has been tested on bare-metal x86 and the Raspberry Pi
Pico. If you have trouble using `bevy_ecs` on a particular platform,
please reach out either through a GitHub issue or in the `no_std`
working group on the Bevy Discord server.
Keep an eye out for future `no_std` updates as we continue to improve
the parity between `std` and `no_std`. We look forward to seeing what
kinds of applications are now possible with Bevy!
## Notes
- Creating PR in draft to ensure CI is passing before requesting
reviews.
- This implementation has no support for multithreading in `no_std`,
especially due to `NonSend` being unsound if allowed in multithreading.
The reason is we cannot check the `ThreadId` in `no_std`, so we have no
mechanism to at-runtime determine if access is sound.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Vic <59878206+Victoronz@users.noreply.github.com>
# Objective
Expand `track_change_detection` feature to also track entity spawns and
despawns. Use this to create better error messages.
# Solution
Adds `Entities::entity_get_spawned_or_despawned_by` as well as `{all
entity reference types}::spawned_by`.
This also removes the deprecated `get_many_entities_mut` & co (and
therefore can't land in 0.15) because we don't yet have no Polonius.
## Testing
Added a test that checks that the locations get updated and these
updates are ordered correctly vs hooks & observers.
---
## Showcase
Access location:
```rust
let mut world = World::new();
let entity = world.spawn_empty().id();
println!("spawned by: {}", world.entity(entity).spawned_by());
```
```
spawned by: src/main.rs:5:24
```
Error message (with `track_change_detection`):
```rust
world.despawn(entity);
world.entity(entity);
```
```
thread 'main' panicked at src/main.rs:11:11:
Entity 0v1#4294967296 was despawned by src/main.rs:10:11
```
and without:
```
thread 'main' panicked at src/main.rs:11:11:
Entity 0v1#4294967296 does not exist (enable `track_change_detection` feature for more details)
```
Similar error messages now also exists for `Query::get`,
`World::entity_mut`, `EntityCommands` creation and everything that
causes `B0003`, e.g.
```
error[B0003]: Could not insert a bundle (of type `MaterialMeshBundle<StandardMaterial>`) for entity Entity { index: 7, generation: 1 }, which was despawned by src/main.rs:10:11. See: https://bevyengine.org/learn/errors/#b0003
```
---------
Co-authored-by: kurk070ff <108901106+kurk070ff@users.noreply.github.com>
Co-authored-by: Freya Pines <freya@MacBookAir.lan>
Co-authored-by: Freya Pines <freya@Freyas-MacBook-Air.local>
Co-authored-by: Matty Weatherley <weatherleymatthew@gmail.com>
# Objective
When calling any of the `sort` methods on a `QueryManyIter` with mutable
data, `collect_inner()` must be called before fetching items. Remove the
need for that call.
## Solution
Have the `sort` methods `collect()` the entity list into a `Vec` before
returning.
# Objective
Allow resources to be accessed soundly by `QueryData` and `QueryFilter`
implementations.
This mostly works today, and is used in `bevy-trait-query` and will be
used by #16810. The problem is that the access is not made visible to
the executor, so it would be possible for a system with resource access
in a query to run concurrently with a system that accesses the resource
with `ResMut`, resulting in Undefined Behavior.
## Solution
Define calling `add_resource_read` or `add_resource_write` in
`WorldQuery::update_component_access` to be a supported way to declare
resource access in a query.
Modify `QueryState::new_with_access` to check for resource access and
report it in `archetype_component_acccess`.
Modify `FilteredAccess::is_compatible` to consider resource access
conflicting even on queries with disjoint filters.
# Objective
- Remove `derive_more`'s error derivation and replace it with
`thiserror`
## Solution
- Added `derive_more`'s `error` feature to `deny.toml` to prevent it
sneaking back in.
- Reverted to `thiserror` error derivation
## Notes
Merge conflicts were too numerous to revert the individual changes, so
this reversion was done manually. Please scrutinise carefully during
review.
# Objective
- Fixes#16208
## Solution
- Added an associated type to `Component`, `Mutability`, which flags
whether a component is mutable, or immutable. If `Mutability= Mutable`,
the component is mutable. If `Mutability= Immutable`, the component is
immutable.
- Updated `derive_component` to default to mutable unless an
`#[component(immutable)]` attribute is added.
- Updated `ReflectComponent` to check if a component is mutable and, if
not, panic when attempting to mutate.
## Testing
- CI
- `immutable_components` example.
---
## Showcase
Users can now mark a component as `#[component(immutable)]` to prevent
safe mutation of a component while it is attached to an entity:
```rust
#[derive(Component)]
#[component(immutable)]
struct Foo {
// ...
}
```
This prevents creating an exclusive reference to the component while it
is attached to an entity. This is particularly powerful when combined
with component hooks, as you can now fully track a component's value,
ensuring whatever invariants you desire are upheld. Before this would be
done my making a component private, and manually creating a `QueryData`
implementation which only permitted read access.
<details>
<summary>Using immutable components as an index</summary>
```rust
/// This is an example of a component like [`Name`](bevy::prelude::Name), but immutable.
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Component)]
#[component(
immutable,
on_insert = on_insert_name,
on_replace = on_replace_name,
)]
pub struct Name(pub &'static str);
/// This index allows for O(1) lookups of an [`Entity`] by its [`Name`].
#[derive(Resource, Default)]
struct NameIndex {
name_to_entity: HashMap<Name, Entity>,
}
impl NameIndex {
fn get_entity(&self, name: &'static str) -> Option<Entity> {
self.name_to_entity.get(&Name(name)).copied()
}
}
fn on_insert_name(mut world: DeferredWorld<'_>, entity: Entity, _component: ComponentId) {
let Some(&name) = world.entity(entity).get::<Name>() else {
unreachable!()
};
let Some(mut index) = world.get_resource_mut::<NameIndex>() else {
return;
};
index.name_to_entity.insert(name, entity);
}
fn on_replace_name(mut world: DeferredWorld<'_>, entity: Entity, _component: ComponentId) {
let Some(&name) = world.entity(entity).get::<Name>() else {
unreachable!()
};
let Some(mut index) = world.get_resource_mut::<NameIndex>() else {
return;
};
index.name_to_entity.remove(&name);
}
// Setup our name index
world.init_resource::<NameIndex>();
// Spawn some entities!
let alyssa = world.spawn(Name("Alyssa")).id();
let javier = world.spawn(Name("Javier")).id();
// Check our index
let index = world.resource::<NameIndex>();
assert_eq!(index.get_entity("Alyssa"), Some(alyssa));
assert_eq!(index.get_entity("Javier"), Some(javier));
// Changing the name of an entity is also fully capture by our index
world.entity_mut(javier).insert(Name("Steven"));
// Javier changed their name to Steven
let steven = javier;
// Check our index
let index = world.resource::<NameIndex>();
assert_eq!(index.get_entity("Javier"), None);
assert_eq!(index.get_entity("Steven"), Some(steven));
```
</details>
Additionally, users can use `Component<Mutability = ...>` in trait
bounds to enforce that a component _is_ mutable or _is_ immutable. When
using `Component` as a trait bound without specifying `Mutability`, any
component is applicable. However, methods which only work on mutable or
immutable components are unavailable, since the compiler must be
pessimistic about the type.
## Migration Guide
- When implementing `Component` manually, you must now provide a type
for `Mutability`. The type `Mutable` provides equivalent behaviour to
earlier versions of `Component`:
```rust
impl Component for Foo {
type Mutability = Mutable;
// ...
}
```
- When working with generic components, you may need to specify that
your generic parameter implements `Component<Mutability = Mutable>`
rather than `Component` if you require mutable access to said component.
- The entity entry API has had to have some changes made to minimise
friction when working with immutable components. Methods which
previously returned a `Mut<T>` will now typically return an
`OccupiedEntry<T>` instead, requiring you to add an `into_mut()` to get
the `Mut<T>` item again.
## Draft Release Notes
Components can now be made immutable while stored within the ECS.
Components are the fundamental unit of data within an ECS, and Bevy
provides a number of ways to work with them that align with Rust's rules
around ownership and borrowing. One part of this is hooks, which allow
for defining custom behavior at key points in a component's lifecycle,
such as addition and removal. However, there is currently no way to
respond to _mutation_ of a component using hooks. The reasons for this
are quite technical, but to summarize, their addition poses a
significant challenge to Bevy's core promises around performance.
Without mutation hooks, it's relatively trivial to modify a component in
such a way that breaks invariants it intends to uphold. For example, you
can use `core::mem::swap` to swap the components of two entities,
bypassing the insertion and removal hooks.
This means the only way to react to this modification is via change
detection in a system, which then begs the question of what happens
_between_ that alteration and the next run of that system?
Alternatively, you could make your component private to prevent
mutation, but now you need to provide commands and a custom `QueryData`
implementation to allow users to interact with your component at all.
Immutable components solve this problem by preventing the creation of an
exclusive reference to the component entirely. Without an exclusive
reference, the only way to modify an immutable component is via removal
or replacement, which is fully captured by component hooks. To make a
component immutable, simply add `#[component(immutable)]`:
```rust
#[derive(Component)]
#[component(immutable)]
struct Foo {
// ...
}
```
When implementing `Component` manually, there is an associated type
`Mutability` which controls this behavior:
```rust
impl Component for Foo {
type Mutability = Mutable;
// ...
}
```
Note that this means when working with generic components, you may need
to specify that a component is mutable to gain access to certain
methods:
```rust
// Before
fn bar<C: Component>() {
// ...
}
// After
fn bar<C: Component<Mutability = Mutable>>() {
// ...
}
```
With this new tool, creating index components, or caching data on an
entity should be more user friendly, allowing libraries to provide APIs
relying on components and hooks to uphold their invariants.
## Notes
- ~~I've done my best to implement this feature, but I'm not happy with
how reflection has turned out. If any reflection SMEs know a way to
improve this situation I'd greatly appreciate it.~~ There is an
outstanding issue around the fallibility of mutable methods on
`ReflectComponent`, but the DX is largely unchanged from `main` now.
- I've attempted to prevent all safe mutable access to a component that
does not implement `Component<Mutability = Mutable>`, but there may
still be some methods I have missed. Please indicate so and I will
address them, as they are bugs.
- Unsafe is an escape hatch I am _not_ attempting to prevent. Whatever
you do with unsafe is between you and your compiler.
- I am marking this PR as ready, but I suspect it will undergo fairly
major revisions based on SME feedback.
- I've marked this PR as _Uncontroversial_ based on the feature, not the
implementation.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Benjamin Brienen <benjamin.brienen@outlook.com>
Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
Co-authored-by: Nuutti Kotivuori <naked@iki.fi>
