# Objective
The current Entity Debug impl prints the bit representation. This is an
"overshare". Debug is in many ways the primary interface into Entity, as
people derive Debug on their entity-containing types when they want to
inspect them. The bits take up too much space in the console and
obfuscate the useful information (entity index and generation).
## Solution
Use the Display implementation in Debug as well. Direct people
interested in bits to `Entity::to_bits` in the docs.
# Objective
#19649 introduced new `*_if_new` and `*_by_bundle_id_*` variations to
`EntityClonerBuilder` filtering functionality, which resulted in
increase in method permutations - there are now 8 allow variants to
support various id types and 2 different insert modes.
## Solution
This PR introduces a new trait `FilterableIds` to unify all id types and
their `IntoIterator` implementations, which is somewhat similar to
`WorldEntityFetch`. It supports `TypeId`, `ComponentId` and `BundleId`,
allowing us to reduce the number of `allow` methods to 4: `allow`,
`allow_if_new`, `allow_by_ids`, `allow_by_ids_if_new`. The function
signature is a bit less readable now, but the docs mention types that
can be passed in.
## Testing
All existing tests pass, performance is unchanged.
---------
Co-authored-by: urben1680 <55257931+urben1680@users.noreply.github.com>
# Objective
- `MapEntities` is not implemented for arrays, `HashMap`, `BTreeMap`,
and `IndexMap`.
## Solution
- Implement `MapEntities` for arrays, `HashMap`, `BTreeMap`, `IndexMap`
## Testing
- I didn't add a test for this as the implementations seems pretty
trivial
# Objective
Further tests after #19326 showed that configuring `EntityCloner` with
required components is bug prone and the current design has several
weaknesses in it's API:
- Mixing `EntityClonerBuilder::allow` and `EntityClonerBuilder::deny`
requires extra care how to support that which has an impact on
surrounding code that has to keep edge cases in mind. This is especially
true for attempts to fix the following issues. There is no use-case
known (to me) why someone would mix those.
- A builder with `EntityClonerBuilder::allow_all` configuration tries to
support required components like `EntityClonerBuilder::deny_all` does,
but the meaning of that is conflicting with how you'd expect things to
work:
- If all components should be cloned except component `A`, do you also
want to exclude required components of `A` too? Or are these also valid
without `A` at the target entity?
- If `EntityClonerBuilder::allow_all` should ignore required components
and not add them to be filtered away, which purpose has
`EntityClonerBuilder::without_required_components` for this cloner?
- Other bugs found with the linked PR are:
- Denying `A` also denies required components of `A` even when `A` does
not exist at the source entity
- Allowing `A` also allows required components of `A` even when `A` does
not exist at the source entity
- Adding `allow_if_new` filters to the cloner faces the same issues and
require a common solution to dealing with source-archetype sensitive
cloning
Alternative to #19632 and #19635.
# Solution
`EntityClonerBuilder` is made generic and split into
`EntityClonerBuilder<OptOut>` and `EntityClonerBuilder<OptIn>`
For an overview of the changes, see the migration guide. It is generally
a good idea to start a review of that.
## Algorithm
The generic of `EntityClonerBuilder` contains the filter data that is
needed to build and clone the entity components.
As the filter needs to be borrowed mutably for the duration of the
clone, the borrow checker forced me to separate the filter value and all
other fields in `EntityCloner`. The latter are now in the
`EntityClonerConfig` struct. This caused many changed LOC, sorry.
To make reviewing easier:
1. Check the migration guide
2. Many methods of `EntityCloner` now just call identitcal
`EntityClonerConfig` methods with a mutable borrow of the filter
3. Check `EntityClonerConfig::clone_entity_internal` which changed a bit
regarding the filter usage that is now trait powered (`CloneByFilter`)
to support `OptOut`, `OptIn` and `EntityClonerFilter` (an enum combining
the first two)
4. Check `OptOut` type that no longer tracks required components but has
a `insert_mode` field
5. Check `OptIn` type that has the most logic changes
# Testing
I added a bunch of tests that cover the new logic parts and the fixed
issues.
Benchmarks are in a comment a bit below which shows ~4% to 9%
regressions, but it varied wildly for me. For example at one run the
reflection-based clonings were on-par with main while the other are not,
and redoing that swapped the situation for both.
It would be really cool if I could get some hints how to get better
benchmark results or if you could run them on your machine too.
Just be aware this is not a Performance PR but a Bugfix PR, even if I
smuggled in some more functionalities. So doing changes to
`EntityClonerBuilder` is kind of required here which might make us bite
the bullet.
---------
Co-authored-by: eugineerd <70062110+eugineerd@users.noreply.github.com>
# Objective
Fix https://github.com/bevyengine/bevy/issues/19617
# Solution
Add newlines before all impl blocks.
I suspect that at least some of these will be objectionable! If there's
a desired Bevy style for this then I'll update the PR. If not then we
can just close it - it's the work of a single find and replace.
# Objective
- Many strings in bevy_ecs are created but only used for debug: system
name, component name, ...
- Those strings make a significant part of the final binary and are no
use in a released game
## Solution
- Use [`strings`](https://linux.die.net/man/1/strings) to find ...
strings in a binary
- Try to find where they come from
- Many are made from `type_name::<T>()` and only used in error / debug
messages
- Add a new structure `DebugName` that holds no value if `debug` feature
is disabled
- Replace `core::any::type_name::<T>()` by `DebugName::type_name::<T>()`
## Testing
Measurements were taken without the new feature being enabled by
default, to help with commands
### File Size
I tried building the `breakout` example with `cargo run --release
--example breakout`
|`debug` enabled|`debug` disabled|
|-|-|
|81621776 B|77735728B|
|77.84MB|74.13MB|
### Compilation time
`hyperfine --min-runs 15 --prepare "cargo clean && sleep 5"
'RUSTC_WRAPPER="" cargo build --release --example breakout'
'RUSTC_WRAPPER="" cargo build --release --example breakout --features
debug'`
```
breakout' 'RUSTC_WRAPPER="" cargo build --release --example breakout --features debug'
Benchmark 1: RUSTC_WRAPPER="" cargo build --release --example breakout
Time (mean ± σ): 84.856 s ± 3.565 s [User: 1093.817 s, System: 32.547 s]
Range (min … max): 78.038 s … 89.214 s 15 runs
Benchmark 2: RUSTC_WRAPPER="" cargo build --release --example breakout --features debug
Time (mean ± σ): 92.303 s ± 2.466 s [User: 1193.443 s, System: 33.803 s]
Range (min … max): 90.619 s … 99.684 s 15 runs
Summary
RUSTC_WRAPPER="" cargo build --release --example breakout ran
1.09 ± 0.05 times faster than RUSTC_WRAPPER="" cargo build --release --example breakout --features debug
```
Follow-up of #19274.
Make the `check_change_tick` methods, of which some are now public, take
`CheckChangeTicks` to make it obvious where this tick comes from, see
other PR.
This also affects the `System` trait, hence the many changed files.
---------
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
# Objective
#19421 implemented `Ord` for `EntityGeneration` along the lines of [the
impl from
slotmap](https://docs.rs/slotmap/latest/src/slotmap/util.rs.html#8):
```rs
/// Returns if a is an older version than b, taking into account wrapping of
/// versions.
pub fn is_older_version(a: u32, b: u32) -> bool {
let diff = a.wrapping_sub(b);
diff >= (1 << 31)
}
```
But that PR and the slotmap impl are different:
**slotmap impl**
- if `(1u32 << 31)` is greater than `a.wrapping_sub(b)`, then `a` is
older than `b`
- if `(1u32 << 31)` is equal to `a.wrapping_sub(b)`, then `a` is older
than `b`
- if `(1u32 << 31)` is less than `a.wrapping_sub(b)`, then `a` is equal
or newer than `b`
**previous PR impl**
- if `(1u32 << 31)` is greater than `a.wrapping_sub(b)`, then `a` is
older than `b`
- if `(1u32 << 31)` is equal to `a.wrapping_sub(b)`, then `a` is equal
to `b` ⚠️
- if `(1u32 << 31)` is less than `a.wrapping_sub(b)`, then `a` is newer
than `b` ⚠️
This ordering is also not transitive, therefore it should not implement
`PartialOrd`.
## Solution
Fix the impl in a standalone method, remove the `Partialord`/`Ord`
implementation.
## Testing
Given the first impl was wrong and got past reviews, I think a new unit
test is justified.
# Objective
- #19504 showed a 11x regression in getting component values for
unregistered components. This pr should fix that and improve others a
little too.
- This is some cleanup work from #18173 .
## Solution
- Whenever we expect a component value to exist, we only care about
fully registered components, not queued to be registered components
since, for the value to exist, it must be registered.
- So we can use the faster `get_valid_*` instead of `get_*` in a lot of
places.
- Also found a bug where `valid_*` did not forward to `get_valid_*`
properly. That's fixed.
## Testing
CI
# Objective
This is the first step of #19430 and is a follow up for #19132.
Now that `ArchetypeRow` has a niche, we can use `Option` instead of
needing `INVALID` everywhere.
This was especially concerning since `INVALID` *really was valid!*
Using options here made the code clearer and more data-driven.
## Solution
Replace all uses of `INVALID` entity locations (and archetype/table
rows) with `None`.
## Testing
CI
---------
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
Co-authored-by: François Mockers <francois.mockers@vleue.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
Fix#19324
## Solution
`EntityCloner` replaces required components when filtering. This is
unexpected when comparing with the way the rest of bevy handles required
components. This PR separates required components from explicit
components when filtering in `EntityClonerBuilder`.
## Testing
Added a regression test for this case.
# Objective
Recently the `u32` `Entity::generation` was replaced with the new
`EntityGeneration` in #19121.
This made meanings a lot more clear, and prevented accidental misuse.
One common misuse was assuming that `u32`s that were greater than others
came after those others.
Wrapping makes this assumption false.
When `EntityGeneration` was created, it retained the `u32` ordering,
which was useless at best and wrong at worst.
This pr fixes the ordering implementation, so new generations are
greater than older generations.
Some users were already accounting for this ordering issue (which was
still present in 0.16 and before) by manually accessing the `u32`
representation. This made migrating difficult for avian physics; see
[here](https://discord.com/channels/691052431525675048/749335865876021248/1377431569228103780).
I am generally of the opinion that this type should be kept opaque to
prevent accidental misuse.
As we find issues like this, the functionality should be added to
`EntityGeneration` directly.
## Solution
Fix the ordering implementation through `Ord`.
Alternatively, we could keep `Ord` the same and make a `cmp_age` method,
but I think this is better, even though sorting entity ids may be
*marginally* slower now (but more correct). This is a tradeoff.
## Testing
I improved documentation for aliasing and ordering, adding some doc
tests.
# Objective
There are several uninlined format args (seems to be in more formatting
macros and in more crates) that are not detected on stable, but are on
nightly.
## Solution
Fix them.
# Objective
#19047 added an `MaybeUninit` field to `EntityMeta`, but did not
guarantee that it will be initialized before access:
```rust
let mut world = World::new();
let id = world.entities().reserve_entity();
world.flush();
world.entity(id);
```
<details>
<summary>Miri Error</summary>
```
error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory
--> /home/vj/workspace/rust/bevy/crates/bevy_ecs/src/entity/mod.rs:1121:26
|
1121 | unsafe { meta.spawned_or_despawned.assume_init() }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ using uninitialized data, but this operation requires initialized memory
|
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
= note: BACKTRACE:
= note: inside closure at /home/vj/workspace/rust/bevy/crates/bevy_ecs/src/entity/mod.rs:1121:26: 1121:65
= note: inside `std::option::Option::<&bevy_ecs::entity::EntityMeta>::map::<bevy_ecs::entity::SpawnedOrDespawned, {closure@bevy_ecs::entity::Entities::entity_get_spawned_or_despawned::{closure#1}}>` at /home/vj/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/option.rs:1144:29: 1144:33
= note: inside `bevy_ecs::entity::Entities::entity_get_spawned_or_despawned` at /home/vj/workspace/rust/bevy/crates/bevy_ecs/src/entity/mod.rs:1112:9: 1122:15
= note: inside closure at /home/vj/workspace/rust/bevy/crates/bevy_ecs/src/entity/mod.rs:1094:13: 1094:57
= note: inside `bevy_ecs::change_detection::MaybeLocation::<std::option::Option<&std::panic::Location<'_>>>::new_with_flattened::<{closure@bevy_ecs::entity::Entities::entity_get_spawned_or_despawned_by::{closure#0}}>` at /home/vj/workspace/rust/bevy/crates/bevy_ecs/src/change_detection.rs:1371:20: 1371:24
= note: inside `bevy_ecs::entity::Entities::entity_get_spawned_or_despawned_by` at /home/vj/workspace/rust/bevy/crates/bevy_ecs/src/entity/mod.rs:1093:9: 1096:11
= note: inside `bevy_ecs::entity::Entities::entity_does_not_exist_error_details` at /home/vj/workspace/rust/bevy/crates/bevy_ecs/src/entity/mod.rs:1163:23: 1163:70
= note: inside `bevy_ecs::entity::EntityDoesNotExistError::new` at /home/vj/workspace/rust/bevy/crates/bevy_ecs/src/entity/mod.rs:1182:22: 1182:74
= note: inside `bevy_ecs::world::unsafe_world_cell::UnsafeWorldCell::<'_>::get_entity` at /home/vj/workspace/rust/bevy/crates/bevy_ecs/src/world/unsafe_world_cell.rs:368:20: 368:73
= note: inside `<bevy_ecs::entity::Entity as bevy_ecs::world::WorldEntityFetch>::fetch_ref` at /home/vj/workspace/rust/bevy/crates/bevy_ecs/src/world/entity_fetch.rs:207:21: 207:42
= note: inside `bevy_ecs::world::World::get_entity::<bevy_ecs::entity::Entity>` at /home/vj/workspace/rust/bevy/crates/bevy_ecs/src/world/mod.rs:911:18: 911:42
note: inside `main`
--> src/main.rs:12:15
|
12 | world.entity(id);
|
```
</details>
## Solution
- remove the existing `MaybeUninit` in `EntityMeta.spawned_or_despawned`
- initialize during flush. This is not needed for soundness, but not
doing this means we can't return a sensible location/tick for flushed
entities.
## Testing
Test via the snippet above (also added equivalent test).
---------
Co-authored-by: urben1680 <55257931+urben1680@users.noreply.github.com>
# Objective
Fix some grammatical errors: it's -> its
Not the most useful commit in the world, but I saw a couple of these and
decided to fix the lot.
## Solution
-
## Testing
-
# Objective
In my own project I was encountering the issue to find out which
entities were spawned after applying commands. I began maintaining a
vector of all entities with generational information before and after
applying the command and diffing it. This was awfully complicated though
and has no constant complexity but grows with the number of entities.
## Solution
Looking at `EntyMeta` it seemed obvious to me that struct can track the
tick just as it does with `MaybeLocation`, updated from the same call.
After that it became almost a given to also introduce query data
`SpawnDetails` which offers methods to get the spawn tick and location,
and query filter `Spawned` that filters entities out that were not
spawned since the last run.
## Testing
I expanded a few tests and added new ones, though maybe I forgot a group
of tests that should be extended too. I basically searched `bevy_ecs`
for mentions of `Changed` and `Added` to see where the tests and docs
are.
Benchmarks of spawn/despawn can be found
[here](https://github.com/bevyengine/bevy/pull/19047#issuecomment-2852181374).
---
## Showcase
From the added docs, systems with equal complexity since the filter is
not archetypal:
```rs
fn system1(q: Query<Entity, Spawned>) {
for entity in &q { /* entity spawned */ }
}
fn system2(query: Query<(Entity, SpawnDetails)>) {
for (entity, spawned) in &query {
if spawned.is_spawned() { /* entity spawned */ }
}
}
```
`SpawnedDetails` has a few more methods:
```rs
fn print_spawn_details(query: Query<(Entity, SpawnDetails)>) {
for (entity, spawn_details) in &query {
if spawn_details.is_spawned() {
print!("new ");
}
println!(
"entity {:?} spawned at {:?} by {:?}",
entity,
spawn_details.spawned_at(),
spawn_details.spawned_by()
);
}
}
```
## Changes
No public api was changed, I only added to it. That is why I added no
migration guide.
- query data `SpawnDetails`
- query filter `Spawned`
- method `Entities::entity_get_spawned_or_despawned_at`
- method `EntityRef::spawned_at`
- method `EntityMut::spawned_at`
- method `EntityWorldMut::spawned_at`
- method `UnsafeEntityCell::spawned_at`
- method `FilteredEntityRef::spawned_at`
- method `FilteredEntityMut::spawned_at`
- method `EntityRefExcept::spawned_at`
- method `EntityMutExcept::spawned_at`
---------
Co-authored-by: Eagster <79881080+ElliottjPierce@users.noreply.github.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
This is a followup to #18704 . There's lots more followup work, but this
is the minimum to unblock #18670, etc.
This direction has been given the green light by Alice
[here](https://github.com/bevyengine/bevy/pull/18704#issuecomment-2853368129).
## Solution
I could have split this over multiple PRs, but I figured skipping
straight here would be easiest for everyone and would unblock things the
quickest.
This removes the now no longer needed `identifier` module and makes
`Entity::generation` go from `NonZeroU32` to `struct
EntityGeneration(u32)`.
## Testing
CI
---------
Co-authored-by: Mark Nokalt <marknokalt@live.com>
# Objective
There are two problems this aims to solve.
First, `Entity::index` is currently a `u32`. That means there are
`u32::MAX + 1` possible entities. Not only is that awkward, but it also
make `Entity` allocation more difficult. I discovered this while working
on remote entity reservation, but even on main, `Entities` doesn't
handle the `u32::MAX + 1` entity very well. It can not be batch reserved
because that iterator uses exclusive ranges, which has a maximum upper
bound of `u32::MAX - 1`. In other words, having `u32::MAX` as a valid
index can be thought of as a bug right now. We either need to make that
invalid (this PR), which makes Entity allocation cleaner and makes
remote reservation easier (because the length only needs to be u32
instead of u64, which, in atomics is a big deal), or we need to take
another pass at `Entities` to make it handle the `u32::MAX` index
properly.
Second, `TableRow`, `ArchetypeRow` and `EntityIndex` (a type alias for
u32) all have `u32` as the underlying type. That means using these as
the index type in a `SparseSet` uses 64 bits for the sparse list because
it stores `Option<IndexType>`. By using `NonMaxU32` here, we cut the
memory of that list in half. To my knowledge, `EntityIndex` is the only
thing that would really benefit from this niche. `TableRow` and
`ArchetypeRow` I think are not stored in an `Option` in bulk. But if
they ever are, this would help. Additionally this ensures
`TableRow::INVALID` and `ArchetypeRow::INVALID` never conflict with an
actual row, which in a nice bonus.
As a related note, if we do components as entities where `ComponentId`
becomes `Entity`, the the `SparseSet<ComponentId>` will see a similar
memory improvement too.
## Solution
Create a new type `EntityRow` that wraps `NonMaxU32`, similar to
`TableRow` and `ArchetypeRow`.
Change `Entity::index` to this type.
## Downsides
`NonMax` is implemented as a `NonZero` with a binary inversion. That
means accessing and storing the value takes one more instruction. I
don't think that's a big deal, but it's worth mentioning.
As a consequence, `to_bits` uses `transmute` to skip the inversion which
keeps it a nop. But that also means that ordering has now flipped. In
other words, higher indices are considered less than lower indices. I
don't think that's a problem, but it's also worth mentioning.
## Alternatives
We could keep the index as a u32 type and just document that `u32::MAX`
is invalid, modifying `Entities` to ensure it never gets handed out.
(But that's not enforced by the type system.) We could still take
advantage of the niche here in `ComponentSparseSet`. We'd just need some
unsafe manual conversions, which is probably fine, but opens up the
possibility for correctness problems later.
We could change `Entities` to fully support the `u32::MAX` index. (But
that makes `Entities` more complex and potentially slightly slower.)
## Testing
- CI
- A few tests were changed because they depend on different ordering and
`to_bits` values.
## Future Work
- It might be worth removing the niche on `Entity::generation` since
there is now a different niche.
- We could move `Entity::generation` into it's own type too for clarity.
- We should change `ComponentSparseSet` to take advantage of the new
niche. (This PR doesn't change that yet.)
- Consider removing or updating `Identifier`. This is only used for
`Entity`, so it might be worth combining since `Entity` is now more
unique.
---------
Co-authored-by: atlv <email@atlasdostal.com>
Co-authored-by: Zachary Harrold <zac@harrold.com.au>
# Objective
With the current `MapEntities` `impl`s, it is not possible to derive
things like this:
```rust
#[derive(Component)]
pub struct Inventory {
#[entities]
slots: Vec<Option<Entity>>,
}
```
This is because `MapEntities` is only implemented for `Vec<Entity>` &
`Option<Entity>`, and not arbitrary combinations of those.
It would be nice to also support those types.
## Solution
I replaced the `impl`s of the following types
- `Option<Entity>`: replaced with `Option<T>`
- `Vec<Entity>`: replaced with `Vec<T>`
- `HashSet<Entity, S>`: replaced with `HashSet<T, S>`
- `T` also had to be `Eq + core:#️⃣:Hash` here. **Not sure if this is
too restrictive?**
- `IndexSet<Entity, S>`: replaced with `IndexSet <T, S>`
- `T` also had to be `Eq + core:#️⃣:Hash` here. **Not sure if this is
too restrictive?**
- `BTreeSet<Entity>`: replaced with `BTreeSet<T>`
- `VecDeque<Entity>`: replaced with `VecDeque<T>`
- `SmallVec<A: smallvec::Array<Item = Entity>>`: replaced with
`SmallVec<A: smallvec::Array<Item = T>>`
(in all of the above, `T` is a generic type that implements
`MapEntities` (`Entity` being one of them).)
## Testing
I did not test any of this, but extended the `Component::map_entities`
doctest with an example usage of the newly supported types.
---
## Showcase
With these changes, this is now possible:
```rust
#[derive(Component)]
pub struct Inventory {
#[entities]
slots: Vec<Option<Entity>>,
}
```
# Objective
Based on and closes#18054, this PR builds on #18035 and #18147 to
remove:
- `Commands::insert_or_spawn_batch`
- `Entities::alloc_at_without_replacement`
- `Entities::alloc_at`
- `entity::AllocAtWithoutReplacement`
- `World::insert_or_spawn_batch`
- `World::insert_or_spawn_batch_with_caller`
## Testing
Just removing unused, deprecated code, so no new tests. Note that as of
writing, #18035 is still under testing and review.
## Future Work
Per
[this](https://github.com/bevyengine/bevy/issues/18054#issuecomment-2689088899)
comment on #18054, there may be additional performance improvements
possible to the entity allocator now that `alloc_at` no longer is
supported. At a glance, I don't see anything obvious to improve, but it
may be worth further investigation in the future.
---------
Co-authored-by: JaySpruce <jsprucebruce@gmail.com>
# Objective
`BTreeSet` doesn't implement `RelationshipSourceCollection`.
## Solution
Implement it.
## Testing
`cargo clippy`
---
## Showcase
You can now use `BTreeSet` in a `RelationshipTarget`
---------
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
Co-authored-by: François Mockers <mockersf@gmail.com>
# Objective
The goal of `bevy_platform_support` is to provide a set of platform
agnostic APIs, alongside platform-specific functionality. This is a high
traffic crate (providing things like HashMap and Instant). Especially in
light of https://github.com/bevyengine/bevy/discussions/18799, it
deserves a friendlier / shorter name.
Given that it hasn't had a full release yet, getting this change in
before Bevy 0.16 makes sense.
## Solution
- Rename `bevy_platform_support` to `bevy_platform`.
# Objective
- The `#[deprecated]` attributes supports a `since` field, which
documents in which version an item was deprecated. This field is visible
in `rustdoc`.
- We inconsistently use `since` throughout the project.
For an example of what `since` renders as, take a look at
`ChildOf::get()`:
```rust
/// The parent entity of this child entity.
#[deprecated(since = "0.16.0", note = "Use child_of.parent() instead")]
#[inline]
pub fn get(&self) -> Entity {
self.0
}
```
## Solution
- Add `since = "0.16.0"` to all `#[deprecated]` attributes that do not
already use it.
- Add an example of deprecating a struct with the `since` field in the
migration guide document.
I would appreciate if this could be included in 0.16's release, as its a
low-risk documentation improvement that is valuable for the release, but
I'd understand if this was cut.
## Testing
You can use `cargo doc` to inspect the rendered form of
`#[deprecated(since = "0.16.0", ...)]`.
# Objective
Newest installment of the #16547 series.
In #18319 we introduced `Entity` defaults to accomodate the most common
use case for these types, however that resulted in the switch of the `T`
and `N` generics of `UniqueEntityArray`.
Swapping generics might be somewhat acceptable for `UniqueEntityArray`,
it is not at all acceptable for map and set types, which we would make
generic over `T: EntityEquivalent` in #18408.
Leaving these defaults in place would result in a glaring inconsistency
between these set collections and the others.
Additionally, the current standard in the engine is for "entity" to mean
`Entity`. APIs could be changed to accept `EntityEquivalent`, however
that is a separate and contentious discussion.
## Solution
Name these set collections `UniqueEntityEquivalent*`, and retain the
`UniqueEntity*` name for an alias of the `Entity` case.
While more verbose, this allows for all generics to be in proper order,
full consistency between all set types*, and the "entity" name to be
restricted to `Entity`.
On top of that, `UniqueEntity*` now always have 1 generic less, when
previously this was not enforced for the default case.
*`UniqueEntityIter<I: Iterator<T: EntityEquivalent>>` is the sole
exception to this. Aliases are unable to enforce bounds
(`lazy_type_alias` is needed for this), so for this type, doing this
split would be a mere suggestion, and in no way enforced.
Iterator types are rarely ever named, and this specific one is intended
to be aliased when it sees more use, like we do for the corresponding
set collection iterators.
Furthermore, the `EntityEquivalent` precursor `Borrow<Entity>` was used
exactly because of such iterator bounds!
Because of that, we leave it as is.
While no migration guide for 0.15 users, for those that upgrade from
main:
`UniqueEntityVec<T>` -> `UniqueEntityEquivalentVec<T>`
`UniqueEntitySlice<T>` -> `UniqueEntityEquivalentSlice<T>`
`UniqueEntityArray<N, T>` -> `UniqueEntityEquivalentArray<T, N>`
# Objective
In #17905 we swapped to a named field on `ChildOf` to help resolve
variable naming ambiguity of child vs parent (ex: `child_of.parent`
clearly reads as "I am accessing the parent of the child_of
relationship", whereas `child_of.0` is less clear).
Unfortunately this has the side effect of making initialization less
ideal. `ChildOf { parent }` reads just as well as `ChildOf(parent)`, but
`ChildOf { parent: root }` doesn't read nearly as well as
`ChildOf(root)`.
## Solution
Move back to `ChildOf(pub Entity)` but add a `child_of.parent()`
function and use it for all accesses. The downside here is that users
are no longer "forced" to access the parent field with `parent`
nomenclature, but I think this strikes the right balance.
Take a look at the diff. I think the results provide strong evidence for
this change. Initialization has the benefit of reading much better _and_
of taking up significantly less space, as many lines go from 3 to 1, and
we're cutting out a bunch of syntax in some cases.
Sadly I do think this should land in 0.16 as the cost of doing this
_after_ the relationships migration is high.
# Objective
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
Requires are currently more verbose than they need to be. People would
like to define inline component values. Additionally, the current
`#[require(Foo(custom_constructor))]` and `#[require(Foo(|| Foo(10))]`
syntax doesn't really make sense within the context of the Rust type
system. #18309 was an attempt to improve ergonomics for some cases, but
it came at the cost of even more weirdness / unintuitive behavior. Our
approach as a whole needs a rethink.
## Solution
Rework the `#[require()]` syntax to make more sense. This is a breaking
change, but I think it will make the system easier to learn, while also
improving ergonomics substantially:
```rust
#[derive(Component)]
#[require(
A, // this will use A::default()
B(1), // inline tuple-struct value
C { value: 1 }, // inline named-struct value
D::Variant, // inline enum variant
E::SOME_CONST, // inline associated const
F::new(1), // inline constructor
G = returns_g(), // an expression that returns G
H = SomethingElse::new(), // expression returns SomethingElse, where SomethingElse: Into<H>
)]
struct Foo;
```
## Migration Guide
Custom-constructor requires should use the new expression-style syntax:
```rust
// before
#[derive(Component)]
#[require(A(returns_a))]
struct Foo;
// after
#[derive(Component)]
#[require(A = returns_a())]
struct Foo;
```
Inline-closure-constructor requires should use the inline value syntax
where possible:
```rust
// before
#[derive(Component)]
#[require(A(|| A(10))]
struct Foo;
// after
#[derive(Component)]
#[require(A(10)]
struct Foo;
```
In cases where that is not possible, use the expression-style syntax:
```rust
// before
#[derive(Component)]
#[require(A(|| A(10))]
struct Foo;
// after
#[derive(Component)]
#[require(A = A(10)]
struct Foo;
```
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: François Mockers <mockersf@gmail.com>
# Objective
- `bevy_ecs` has lint errors without some features
## Solution
- Fix `clippy::allow-attributes-without-reason` when `bevy_reflect` is
disabled by adding a reason
- Fix `clippy::needless_return` when `std` is disabled by adding a gated
`expect` attribute and a comment to remove it when the `no_std` stuff is
addressed
## Testing
- `cargo clippy -p bevy_ecs --no-default-features --no-deps -- --D
warnings`
- CI
# Objective
There are currently too many disparate ways to handle entity mapping,
especially after #17687. We now have MapEntities, VisitEntities,
VisitEntitiesMut, Component::visit_entities,
Component::visit_entities_mut.
Our only known use case at the moment for these is entity mapping. This
means we have significant consolidation potential.
Additionally, VisitEntitiesMut cannot be implemented for map-style
collections like HashSets, as you cant "just" mutate a `&mut Entity`.
Our current approach to Component mapping requires VisitEntitiesMut,
meaning this category of entity collection isn't mappable. `MapEntities`
is more generally applicable. Additionally, the _existence_ of the
blanket From impl on VisitEntitiesMut blocks us from implementing
MapEntities for HashSets (or any types we don't own), because the owner
could always add a conflicting impl in the future.
## Solution
Use `MapEntities` everywhere and remove all "visit entities" usages.
* Add `Component::map_entities`
* Remove `Component::visit_entities`, `Component::visit_entities_mut`,
`VisitEntities`, and `VisitEntitiesMut`
* Support deriving `Component::map_entities` in `#[derive(Coomponent)]`
* Add `#[derive(MapEntities)]`, and share logic with the
`Component::map_entities` derive.
* Add `ComponentCloneCtx::queue_deferred`, which is command-like logic
that runs immediately after normal clones. Reframe `FromWorld` fallback
logic in the "reflect clone" impl to use it. This cuts out a lot of
unnecessary work and I think justifies the existence of a pseudo-command
interface (given how niche, yet performance sensitive this is).
Note that we no longer auto-impl entity mapping for ` IntoIterator<Item
= &'a Entity>` types, as this would block our ability to implement cases
like `HashMap`. This means the onus is on us (or type authors) to add
explicit support for types that should be mappable.
Also note that the Component-related changes do not require a migration
guide as there hasn't been a release with them yet.
## Migration Guide
If you were previously implementing `VisitEntities` or
`VisitEntitiesMut` (likely via a derive), instead use `MapEntities`.
Those were almost certainly used in the context of Bevy Scenes or
reflection via `ReflectMapEntities`. If you have a case that uses
`VisitEntities` or `VisitEntitiesMut` directly, where `MapEntities` is
not a viable replacement, please let us know!
```rust
// before
#[derive(VisitEntities, VisitEntitiesMut)]
struct Inventory {
items: Vec<Entity>,
#[visit_entities(ignore)]
label: String,
}
// after
#[derive(MapEntities)]
struct Inventory {
#[entities]
items: Vec<Entity>,
label: String,
}
```
# Objective
Fixes#18357
## Solution
Generalize `RelationshipInsertHookMode` to `RelationshipHookMode`, wire
it up to on_replace execution, and use it in the
`Relationship::on_replace` hook.
# Objective
Continuation of #17449.
#17449 implemented the wrapper types around `IndexMap`/`Set` and co.,
however punted on the slice types.
They are needed to support creating `EntitySetIterator`s from their
slices, not just the base maps and sets.
## Solution
Add the wrappers, in the same vein as #17449 and #17589 before.
The `Index`/`IndexMut` implementations take up a lot of space, however
they cannot be merged because we'd then get overlaps.
They are simply named `Slice` to match the `indexmap` naming scheme, but
this means they cannot be differentiated properly until their modules
are made public, which is already a follow-up mentioned in #17954.
# Objective
Now that #13432 has been merged, it's important we update our reflected
types to properly opt into this feature. If we do not, then this could
cause issues for users downstream who want to make use of
reflection-based cloning.
## Solution
This PR is broken into 4 commits:
1. Add `#[reflect(Clone)]` on all types marked `#[reflect(opaque)]` that
are also `Clone`. This is mandatory as these types would otherwise cause
the cloning operation to fail for any type that contains it at any
depth.
2. Update the reflection example to suggest adding `#[reflect(Clone)]`
on opaque types.
3. Add `#[reflect(clone)]` attributes on all fields marked
`#[reflect(ignore)]` that are also `Clone`. This prevents the ignored
field from causing the cloning operation to fail.
Note that some of the types that contain these fields are also `Clone`,
and thus can be marked `#[reflect(Clone)]`. This makes the
`#[reflect(clone)]` attribute redundant. However, I think it's safer to
keep it marked in the case that the `Clone` impl/derive is ever removed.
I'm open to removing them, though, if people disagree.
4. Finally, I added `#[reflect(Clone)]` on all types that are also
`Clone`. While not strictly necessary, it enables us to reduce the
generated output since we can just call `Clone::clone` directly instead
of calling `PartialReflect::reflect_clone` on each variant/field. It
also means we benefit from any optimizations or customizations made in
the `Clone` impl, including directly dereferencing `Copy` values and
increasing reference counters.
Along with that change I also took the liberty of adding any missing
registrations that I saw could be applied to the type as well, such as
`Default`, `PartialEq`, and `Hash`. There were hundreds of these to
edit, though, so it's possible I missed quite a few.
That last commit is **_massive_**. There were nearly 700 types to
update. So it's recommended to review the first three before moving onto
that last one.
Additionally, I can break the last commit off into its own PR or into
smaller PRs, but I figured this would be the easiest way of doing it
(and in a timely manner since I unfortunately don't have as much time as
I used to for code contributions).
## Testing
You can test locally with a `cargo check`:
```
cargo check --workspace --all-features
```
# Objective
Installment of the #16547 series.
The vast majority of uses for these types will be the `Entity` case, so
it makes sense for it to be the default.
## Solution
`UniqueEntityVec`, `UniqueEntitySlice`, `UniqueEntityArray` and their
helper iterator aliases now have `Entity` as a default.
Unfortunately, this means the the `T` parameter for `UniqueEntityArray`
now has to be ordered after the `N` constant, which breaks the
consistency to `[T; N]`.
Same with about a dozen iterator aliases that take some `P`/`F`
predicate/function parameter.
This should however be an ergonomic improvement in most cases, so we'll
just have to live with this inconsistency.
## Migration Guide
Switch type parameter order for the relevant wrapper types/aliases.
# Objective
#13432 added proper reflection-based cloning. This is a better method
than cloning via `clone_value` for reasons detailed in the description
of that PR. However, it may not be immediately apparent to users why one
should be used over the other, and what the gotchas of `clone_value`
are.
## Solution
This PR marks `PartialReflect::clone_value` as deprecated, with the
deprecation notice pointing users to `PartialReflect::reflect_clone`.
However, it also suggests using a new method introduced in this PR:
`PartialReflect::to_dynamic`.
`PartialReflect::to_dynamic` is essentially a renaming of
`PartialReflect::clone_value`. By naming it `to_dynamic`, we make it
very obvious that what's returned is a dynamic type. The one caveat to
this is that opaque types still use `reflect_clone` as they have no
corresponding dynamic type.
Along with changing the name, the method is now optional, and comes with
a default implementation that calls out to the respective reflection
subtrait method. This was done because there was really no reason to
require manual implementors provide a method that almost always calls
out to a known set of methods.
Lastly, to make this default implementation work, this PR also did a
similar thing with the `clone_dynamic ` methods on the reflection
subtraits. For example, `Struct::clone_dynamic` has been marked
deprecated and is superseded by `Struct::to_dynamic_struct`. This was
necessary to avoid the "multiple names in scope" issue.
### Open Questions
This PR maintains the original signature of `clone_value` on
`to_dynamic`. That is, it takes `&self` and returns `Box<dyn
PartialReflect>`.
However, in order for this to work, it introduces a panic if the value
is opaque and doesn't override the default `reflect_clone`
implementation.
One thing we could do to avoid the panic would be to make the conversion
fallible, either returning `Option<Box<dyn PartialReflect>>` or
`Result<Box<dyn PartialReflect>, ReflectCloneError>`.
This makes using the method a little more involved (i.e. users have to
either unwrap or handle the rare possibility of an error), but it would
set us up for a world where opaque types don't strictly need to be
`Clone`. Right now this bound is sort of implied by the fact that
`clone_value` is a required trait method, and the default behavior of
the macro is to use `Clone` for opaque types.
Alternatively, we could keep the signature but make the method required.
This maintains that implied bound where manual implementors must provide
some way of cloning the value (or YOLO it and just panic), but also
makes the API simpler to use.
Finally, we could just leave it with the panic. It's unlikely this would
occur in practice since our macro still requires `Clone` for opaque
types, and thus this would only ever be an issue if someone were to
manually implement `PartialReflect` without a valid `to_dynamic` or
`reflect_clone` method.
## Testing
You can test locally using the following command:
```
cargo test --package bevy_reflect --all-features
```
---
## Migration Guide
`PartialReflect::clone_value` is being deprecated. Instead, use
`PartialReflect::to_dynamic` if wanting to create a new dynamic instance
of the reflected value. Alternatively, use
`PartialReflect::reflect_clone` to attempt to create a true clone of the
underlying value.
Similarly, the following methods have been deprecated and should be
replaced with these alternatives:
- `Array::clone_dynamic` → `Array::to_dynamic_array`
- `Enum::clone_dynamic` → `Enum::to_dynamic_enum`
- `List::clone_dynamic` → `List::to_dynamic_list`
- `Map::clone_dynamic` → `Map::to_dynamic_map`
- `Set::clone_dynamic` → `Set::to_dynamic_set`
- `Struct::clone_dynamic` → `Struct::to_dynamic_struct`
- `Tuple::clone_dynamic` → `Tuple::to_dynamic_tuple`
- `TupleStruct::clone_dynamic` → `TupleStruct::to_dynamic_tuple_struct`
# Objective
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
I was recently exploreing `Entities` and stumbled on something strange.
`Entities::len` (the field) has the comment `Stores the number of free
entities for [`len`](Entities::len)`, refering to the method. But that
method says `The count of currently allocated entities.` Looking at the
code, the field's comment is wrong, and the public `len()` is correct.
Phew!
## Solution
So, I was just going to fix the comment, so it didn't confuse anyone
else, but as it turns out, we can just remove the field entirely. As a
bonus, this saves some book keeping work too. We can just calculate it
on the fly.
Also, add additional length methods and documentation for completeness.
These new length methods might be useful debug tools in the future.
---------
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
# Objective
- Fixes#15460 (will open new issues for further `no_std` efforts)
- Supersedes #17715
## Solution
- Threaded in new features as required
- Made certain crates optional but default enabled
- Removed `compile-check-no-std` from internal `ci` tool since GitHub CI
can now simply check `bevy` itself now
- Added CI task to check `bevy` on `thumbv6m-none-eabi` to ensure
`portable-atomic` support is still valid [^1]
[^1]: This may be controversial, since it could be interpreted as
implying Bevy will maintain support for `thumbv6m-none-eabi` going
forward. In reality, just like `x86_64-unknown-none`, this is a
[canary](https://en.wiktionary.org/wiki/canary_in_a_coal_mine) target to
make it clear when `portable-atomic` no longer works as intended (fixing
atomic support on atomically challenged platforms). If a PR comes
through and makes supporting this class of platforms impossible, then
this CI task can be removed. I however wager this won't be a problem.
## Testing
- CI
---
## Release Notes
Bevy now has support for `no_std` directly from the `bevy` crate.
Users can disable default features and enable a new `default_no_std`
feature instead, allowing `bevy` to be used in `no_std` applications and
libraries.
```toml
# Bevy for `no_std` platforms
bevy = { version = "0.16", default-features = false, features = ["default_no_std"] }
```
`default_no_std` enables certain required features, such as `libm` and
`critical-section`, and as many optional crates as possible (currently
just `bevy_state`). For atomically-challenged platforms such as the
Raspberry Pi Pico, `portable-atomic` will be used automatically.
For library authors, we recommend depending on `bevy` with
`default-features = false` to allow `std` and `no_std` users to both
depend on your crate. Here are some recommended features a library crate
may want to expose:
```toml
[features]
# Most users will be on a platform which has `std` and can use the more-powerful `async_executor`.
default = ["std", "async_executor"]
# Features for typical platforms.
std = ["bevy/std"]
async_executor = ["bevy/async_executor"]
# Features for `no_std` platforms.
libm = ["bevy/libm"]
critical-section = ["bevy/critical-section"]
[dependencies]
# We disable default features to ensure we don't accidentally enable `std` on `no_std` targets, for example.
bevy = { version = "0.16", default-features = false }
```
While this is verbose, it gives the maximum control to end-users to
decide how they wish to use Bevy on their platform.
We encourage library authors to experiment with `no_std` support. For
libraries relying exclusively on `bevy` and no other dependencies, it
may be as simple as adding `#![no_std]` to your `lib.rs` and exposing
features as above! Bevy can also provide many `std` types, such as
`HashMap`, `Mutex`, and `Instant` on all platforms. See
`bevy::platform_support` for details on what's available out of the box!
## Migration Guide
- If you were previously relying on `bevy` with default features
disabled, you may need to enable the `std` and `async_executor`
features.
- `bevy_reflect` has had its `bevy` feature removed. If you were relying
on this feature, simply enable `smallvec` and `smol_str` instead.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
Based on #18054, this PR builds on #18035 to deprecate:
- `Commands::insert_or_spawn_batch`
- `Entities::alloc_at_without_replacement`
- `Entities::alloc_at`
- `World::insert_or_spawn_batch`
- `World::insert_or_spawn_batch_with_caller`
## Testing
Just deprecation, so no new tests. Note that as of writing #18035 is
still under testing and review.
## Open Questions
- [x] Should `entity::AllocAtWithoutReplacement` be deprecated? It is
internal and only used in `Entities::alloc_at_without_replacement`.
**EDIT:** Now deprecated.
## Migration Guide
The following functions have been deprecated:
- `Commands::insert_or_spawn_batch`
- `World::insert_or_spawn_batch`
- `World::insert_or_spawn_batch_with_caller`
These functions, when used incorrectly, can cause major performance
problems and are generally viewed as anti-patterns and foot guns. These
are planned to be removed altogether in 0.17.
Instead of these functions consider doing one of the following:
Option A) Instead of despawing entities and re-spawning them at a
particular id, insert the new `Disabled` component without despawning
the entity, and use `try_insert_batch` or `insert_batch` and remove
`Disabled` instead of re-spawning it.
Option B) Instead of giving special meaning to an entity id, simply use
`spawn_batch` and ensure entity references are valid when despawning.
---------
Co-authored-by: JaySpruce <jsprucebruce@gmail.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
Component `require()` IDE integration is fully broken, as of #16575.
## Solution
This reverts us back to the previous "put the docs on Component trait"
impl. This _does_ reduce the accessibility of the required components in
rust docs, but the complete erasure of "required component IDE
experience" is not worth the price of slightly increased prominence of
requires in docs.
Additionally, Rust Analyzer has recently started including derive
attributes in suggestions, so we aren't losing that benefit of the
proc_macro attribute impl.
Fixes#17720
## Objective
Spawning RelationshipTargets from scenes currently fails to preserve
RelationshipTarget ordering (ex: `Children` has an arbitrary order).
This is because it uses the normal hook flow to set up the collection,
which means we are pushing onto the collection in _spawn order_ (which
is currently in archetype order, which will often produce mismatched
orderings).
We need to preserve the ordering in the original RelationshipTarget
collection. Ideally without expensive checking / fixups.
## Solution
One solution would be to spawn in hierarchy-order. However this gets
complicated as there can be multiple hierarchies, and it also means we
can't spawn in more cache-friendly orders (ex: the current per-archetype
spawning, or future even-smarter per-table spawning). Additionally,
same-world cloning has _slightly_ more nuanced needs (ex: recursively
clone linked relationships, while maintaining _original_ relationships
outside of the tree via normal hooks).
The preferred approach is to directly spawn the remapped
RelationshipTarget collection, as this trivially preserves the ordering.
Unfortunately we can't _just_ do that, as when we spawn the children
with their Relationships (ex: `ChildOf`), that will insert a duplicate.
We could "fixup" the collection retroactively by just removing the back
half of duplicates, but this requires another pass / more lookups /
allocating twice as much space. Additionally, it becomes complicated
because observers could insert additional children, making it harder
(aka more expensive) to determine which children are dupes and which are
not.
The path I chose is to support "opting out" of the relationship target
hook in the contexts that need that, as this allows us to just cheaply
clone the mapped collection. The relationship hook can look for this
configuration when it runs and skip its logic when that happens. A
"simple" / small-amount-of-code way to do this would be to add a "skip
relationship spawn" flag to World. Sadly, any hook / observer that runs
_as the result of an insert_ would also read this flag. We really need a
way to scope this setting to a _specific_ insert.
Therefore I opted to add a new `RelationshipInsertHookMode` enum and an
`entity.insert_with_relationship_insert_hook_mode` variant. Obviously
this is verbose and ugly. And nobody wants _more_ insert variants. But
sadly this was the best I could come up with from a performance and
capability perspective. If you have alternatives let me know!
There are three variants:
1. `RelationshipInsertHookMode::Run`: always run relationship insert
hooks (this is the default)
2. `RelationshipInsertHookMode::Skip`: do not run any relationship
insert hooks for this insert (this is used by spawner code)
3. `RelationshipInsertHookMode::RunIfNotLinked`: only run hooks for
_unlinked_ relationships (this is used in same-world recursive entity
cloning to preserve relationships outside of the deep-cloned tree)
Note that I have intentionally only added "insert with relationship hook
mode" variants to the cases we absolutely need (everything else uses the
default `Run` mode), just to keep the code size in check. I do not think
we should add more without real _very necessary_ use cases.
I also made some other minor tweaks:
1. I split out `SourceComponent` from `ComponentCloneCtx`. Reading the
source component no longer needlessly blocks mutable access to
`ComponentCloneCtx`.
2. Thanks to (1), I've removed the `RefCell` wrapper over the cloned
component queue.
3. (1) also allowed me to write to the EntityMapper while queuing up
clones, meaning we can reserve entities during the component clone and
write them to the mapper _before_ inserting the component, meaning
cloned collections can be mapped on insert.
4. I've removed the closure from `write_target_component_ptr` to
simplify the API / make it compatible with the split `SourceComponent`
approach.
5. I've renamed `EntityCloner::recursive` to
`EntityCloner::linked_cloning` to connect that feature more directly
with `RelationshipTarget::LINKED_SPAWN`
6. I've removed `EntityCloneBehavior::RelationshipTarget`. This was
always intended to be temporary, and this new behavior removes the need
for it.
---------
Co-authored-by: Viktor Gustavsson <villor94@gmail.com>
# Objective
fixes#17896
## Solution
Change ChildOf ( Entity ) to ChildOf { parent: Entity }
by doing this we also allow users to use named structs for relationship
derives, When you have more than 1 field in a struct with named fields
the macro will look for a field with the attribute #[relationship] and
all of the other fields should implement the Default trait. Unnamed
fields are still supported.
When u have a unnamed struct with more than one field the macro will
fail.
Do we want to support something like this ?
```rust
#[derive(Component)]
#[relationship_target(relationship = ChildOf)]
pub struct Children (#[relationship] Entity, u8);
```
I could add this, it but doesn't seem nice.
## Testing
crates/bevy_ecs - cargo test
## Showcase
```rust
use bevy_ecs::component::Component;
use bevy_ecs::entity::Entity;
#[derive(Component)]
#[relationship(relationship_target = Children)]
pub struct ChildOf {
#[relationship]
pub parent: Entity,
internal: u8,
};
#[derive(Component)]
#[relationship_target(relationship = ChildOf)]
pub struct Children {
children: Vec<Entity>
};
```
---------
Co-authored-by: Tim Overbeek <oorbecktim@Tims-MacBook-Pro.local>
Co-authored-by: Tim Overbeek <oorbecktim@c-001-001-042.client.nl.eduvpn.org>
Co-authored-by: Tim Overbeek <oorbecktim@c-001-001-059.client.nl.eduvpn.org>
Co-authored-by: Tim Overbeek <oorbecktim@c-001-001-054.client.nl.eduvpn.org>
Co-authored-by: Tim Overbeek <oorbecktim@c-001-001-027.client.nl.eduvpn.org>
## Objective
The closure argument for
`EntityClonerBuilder::without_required_components` has `Send + Sync +
'static` bounds, but the closure immediately gets called and never needs
to be sent anywhere. (This was my fault :P )
## Solution
Remove the bounds so that users aren't unnecessarily restricted.
I also took the opportunity to expand the tests a little.
# Objective
Continuation of #17589 and #16547.
`get_many` is last of the `many` methods with a missing `unique`
counterpart.
It both takes and returns arrays, thus necessitates a matching
`UniqueEntityArray` type!
Plus, some slice methods involve returning arrays, which are currently
missing from `UniqueEntitySlice`.
## Solution
Add the type, the related methods and trait impls.
Note that for this PR, we abstain from some methods/trait impls that
create `&mut UniqueEntityArray`, because it can be successfully
mem-swapped. This can potentially invalidate a larger slice, which is
the same reason we punted on some mutable slice methods in #17589. We
can follow-up on all of these together in a following PR.
The new `unique_array` module is not glob-exported, because the trait
alias `unique_array::IntoIter` would conflict with
`unique_vec::IntoIter`.
The solution for this is to make the various `unique_*` modules public,
which I intend to do in yet another PR.
# 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
Update typos, fix new typos.
1.29.6 was just released to fix an
[issue](https://github.com/crate-ci/typos/issues/1228) where January's
corrections were not included in the binaries for the last release.
Reminder: typos can be tossed in the monthly [non-critical corrections
issue](https://github.com/crate-ci/typos/issues/1221).
## Solution
I chose to allow `implementors`, because a good argument seems to be
being made [here](https://github.com/crate-ci/typos/issues/1226) and
there is now a PR to address that.
## Discussion
Should I exclude `bevy_mikktspace`?
At one point I think we had an informal policy of "don't mess with
mikktspace until https://github.com/bevyengine/bevy/pull/9050 is merged"
but it doesn't seem like that is likely to be merged any time soon.
I think these particular corrections in mikktspace are fine because
- The same typo mistake seems to have been fixed in that PR
- The entire file containing these corrections was deleted in that PR
## Typo of the Month
correspindong -> corresponding
