Commit graph

666 commits

Author SHA1 Message Date
Boxy
bac0d89059 remove a doc(hidden) on read only version of derive(WorldQuery) (#6877)
having `doc(hidden)` on the read only version of a generated mutable world query leads to docs on the readonly item having a dead link. It also makes it annoying to have nice docs for libraries attempting to expose derived `WorldQuery` structs as re-exporting the read only item does not cause it to appear in docs even though it would be intended for users to know about the read only world query and use it.
2022-12-07 23:10:26 +00:00
James Liu
530be10e72 Newtype ArchetypeRow and TableRow (#4878)
# Objective
Prevent future unsoundness that was seen in #6623.

## Solution
Newtype both indexes in `Archetype` and `Table` as `ArchetypeRow` and `TableRow`. This avoids weird numerical manipulation on the indices, and can be stored and treated opaquely. Also enforces the source and destination of where these indices at a type level.

---

## Changelog
Changed: `Archetype` indices and `Table` rows have been newtyped as `ArchetypeRow` and `TableRow`.
2022-12-06 01:38:21 +00:00
James Liu
a3f203b504 Use T::Storage::STORAGE_TYPE to optimize out unused branches (#6800)
# Objective
`EntityRef::get` and friends all type erase calls to fetch the target components by using passing in the `TypeId` instead of using generics. This is forcing a lookup to `Components` to fetch the storage type. This adds an extra memory lookup and forces a runtime branch instead of allowing the compiler to optimize out the unused branch.

## Solution
Leverage `Component::Storage::STORAGE_TYPE` as a constant instead of fetching the metadata from `Components`.

## Performance
This has a near 2x speedup for all calls to `World::get`. Microbenchmark results from my local machine. `Query::get_component`, which uses `EntityRef::get` internally also show a slight speed up. This has closed the gap between `World::get` and `Query::get` for the same use case.

```
group                                                             entity-ref-generics                     main
-----                                                             -------------------                     ----
query_get_component/50000_entities_sparse                         1.00   890.6±40.42µs        ? ?/sec     1.10   980.6±28.22µs        ? ?/sec
query_get_component/50000_entities_table                          1.00   968.5±73.73µs        ? ?/sec     1.08  1048.8±31.76µs        ? ?/sec
query_get_component_simple/system                                 1.00    703.2±4.37µs        ? ?/sec     1.00    702.1±6.13µs        ? ?/sec
query_get_component_simple/unchecked                              1.02    855.8±8.98µs        ? ?/sec     1.00    843.1±8.19µs        ? ?/sec
world_get/50000_entities_sparse                                   1.00    202.3±3.15µs        ? ?/sec     1.85   374.0±20.96µs        ? ?/sec
world_get/50000_entities_table                                    1.00    193.0±1.78µs        ? ?/sec     2.02   389.2±26.55µs        ? ?/sec
world_query_get/50000_entities_sparse                             1.01    162.4±2.23µs        ? ?/sec     1.00    161.3±0.95µs        ? ?/sec
world_query_get/50000_entities_table                              1.00    199.9±0.63µs        ? ?/sec     1.00    200.2±0.74µs        ? ?/sec
```

This should also, by proxy, speed up the `ReflectComponent` APIs as most of those use `World::get` variants internally.
2022-12-05 23:56:33 +00:00
JoJoJet
83e8224694 Add missing docs to World::change_tick and World::read_change_tick (#6765)
# Objective

The methods `World::change_tick` and `World::read_change_tick` lack documentation and have confusingly similar behavior.

## Solution

Add documentation and clarify the distinction between the two functions.
2022-12-05 23:23:14 +00:00
Vladyslav Batyrenko
b337ed63ad Borrow instead of consuming in EventReader::clear (#6851)
The PR fixes the interface of `EventReader::clear`. Currently, the method consumes the reader, which makes it unusable.

## Changelog

- `EventReader::clear` now takes a mutable reference instead of consuming the event reader. 

## Migration Guide

`EventReader::clear` now takes a mutable reference instead of consuming the event reader. This means that `clear` now needs explicit mutable access to the reader variable, which previously could have been omitted in some cases:

```rust
// Old (0.9)
fn clear_events(reader: EventReader<SomeEvent>) {
  reader.clear();
}

// New (0.10)
fn clear_events(mut reader: EventReader<SomeEvent>) {
  reader.clear();
}
``` 

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2022-12-05 23:07:20 +00:00
Martín Maita
eff632dac8 Replace World::read_change_ticks with World::change_ticks within bevy_ecs crate (#6816)
# Objective

- Fixes #6812.

## Solution

- Replaced `World::read_change_ticks` with `World::change_ticks` within `bevy_ecs` crate in places where `World` references were mutable.

---
2022-12-05 22:49:05 +00:00
James Liu
17480b2d89 Remove APIs deprecated in 0.9 (#6801)
# Objective
These functions were deprecated in 0.9. They should be removed in 0.10.

## Solution
Remove them.
2022-12-05 22:49:04 +00:00
James Liu
e8c0df9e1e Allow iterating over with EntityRef over the entire World (#6843)
# Objective
Partially addresses #5504. Allow users to get an `Iterator<Item = EntityRef<'a>>` over all entities in the `World`.

## Solution
Change `World::iter_entities` to return an iterator of `EntityRef` instead of `Entity`.

Not sure how to tackle making an `Iterator<Item = EntityMut<'_>>` without being horribly unsound. Might need to wait for `LendingIterator` to stabilize so we can ensure only one of them is valid at a given time.

---

## Changelog
Changed: `World::iter_entities` now returns an iterator of `EntityRef` instead of `Entity`.
2022-12-05 22:35:02 +00:00
JoJoJet
05b498a224 Make the SystemParam derive macro more flexible (#6694)
# Objective

Currently, the `SystemParam` derive forces you to declare the lifetime parameters `<'w, 's>`, even if you don't use them.
If you don't follow this structure, the error message is quite nasty.

### Example (before):

```rust
#[derive(SystemParam)]
pub struct EventWriter<'w, 's, E: Event> {
    events: ResMut<'w, Events<E>>,
    // The derive forces us to declare the `'s` lifetime even though we don't use it,
    // so we have to add this `PhantomData` to please rustc.
    #[system_param(ignore)]
    _marker: PhantomData<&'s ()>,
}
```


## Solution

* Allow the user to omit either lifetime.
* Emit a descriptive error if any lifetimes used are invalid.

### Example (after):

```rust
#[derive(SystemParam)]
pub struct EventWriter<'w, E: Event> {
    events: ResMut<'w, Events<E>>,
}
```

---

## Changelog

* The `SystemParam` derive is now more flexible, allowing you to omit unused lifetime parameters.
2022-12-05 20:15:03 +00:00
Aleksandr Belkin
9b72780b82 Provide public EntityRef::get_change_ticks_by_id that takes ComponentId (#6683)
# Objective

Fixes #6682 

## Solution

Add `EntityRef::get_change_ticks_by_id`
Add `EntityMut::get_change_ticks_by_id`


Co-authored-by: Aleksandr Belkin <sQu1rr@users.noreply.github.com>
2022-12-02 02:21:22 +00:00
James Liu
e954b8573c Lock down access to Entities (#6740)
# Objective
The soundness of the ECS `World` partially relies on the correctness of the state of `Entities` stored within it. We're currently allowing users to (unsafely) mutate it, as well as readily construct it without using a `World`. While this is not strictly unsound so long as users (including `bevy_render`) safely use the APIs, it's a fairly easy path to unsoundness without much of a guard rail.

Addresses #3362 for `bevy_ecs::entity`. Incorporates the changes from #3985.

## Solution
Remove `Entities`'s  `Default` implementation and force access to the type to only be through a properly constructed `World`.

Additional cleanup for other parts of `bevy_ecs::entity`:

 - `Entity::index` and `Entity::generation` are no longer `pub(crate)`, opting to force the rest of bevy_ecs to use the public interface to access these values.
 - `EntityMeta` is no longer `pub` and also not `pub(crate)` to attempt to cut down on updating `generation` without going through an `Entities` API. It's currently inaccessible except via the `pub(crate)` Vec on `Entities`, there was no way for an outside user to use it.
 - Added `Entities::set`, an unsafe `pub(crate)` API for setting the location of an Entity (parallel to `Entities::get`) that replaces the internal case where we need to set the location of an entity when it's been spawned, moved, or despawned.
 - `Entities::alloc_at_without_replacement` is only used in `World::get_or_spawn` within the first party crates, and I cannot find a public use of this API in any ecosystem crate that I've checked (via GitHub search).
 - Attempted to document the few remaining undocumented public APIs in the module.

---

## Changelog
Removed: `Entities`'s `Default` implementation.
Removed: `EntityMeta`
Removed: `Entities::alloc_at_without_replacement` and `AllocAtWithoutReplacement`.

Co-authored-by: james7132 <contact@jamessliu.com>
Co-authored-by: James Liu <contact@jamessliu.com>
2022-11-28 20:39:02 +00:00
James Liu
d79888bdae Document and lock down types in bevy_ecs::archetype (#6742)
# Objective
Document `bevy_ecs::archetype` and and declutter the public documentation for the module by making types non-`pub`.

Addresses #3362 for `bevy_ecs::archetype`.

## Solution
 - Add module level documentation.
 - Add type and API level documentation for all public facing types.
 - Make `ArchetypeId`, `ArchetypeGeneration`, and `ArchetypeComponentId` truly opaque IDs that are not publicly constructable. 
 - Make `AddBundle` non-pub, make `Edges::get_add_bundle` return a `Option<ArchetypeId>` and fork the existing function into `Edges::get_add_bundle_internal`.
 - Remove `pub(crate)` on fields that have a corresponding pub accessor function.
 - Removed the `Archetypes: Default` impl, opting for a `pub(crate) fn new` alternative instead.

---

## Changelog
Added: `ArchetypeGeneration` now implements `Ord` and `PartialOrd`.
Removed: `Archetypes`'s `Default` implementation.
Removed: `Archetype::new` and `Archetype::is_empty`.
Removed: `ArchetypeId::new` and `ArchetypeId::value`.
Removed: `ArchetypeGeneration::value`
Removed: `ArchetypeIdentity`.
Removed: `ArchetypeComponentId::new` and `ArchetypeComponentId::value`.
Removed: `AddBundle`. `Edges::get_add_bundle` now returns `Option<ArchetypeId>`
2022-11-28 13:54:12 +00:00
mareq
bbb652a438 Fix documentation on spawining an entity (#6775)
# Objective

- The documentation describing different ways to spawn an Entity is missing reference to "method" for "Spawn an entity with components".

## Solution

- Update the documentation to add the reference to `World::spawn`.
2022-11-28 13:40:31 +00:00
JoJoJet
1615834536 Fix an incorrect safety comment in World::get_resource (#6764)
# Objective

* Fix #6307

## Solution

* Rewrite the safety comment to reflect the actual invariants being asserted.
2022-11-28 13:40:26 +00:00
JoJoJet
416a33e613 Add const Entity::PLACEHOLDER (#6761)
# Objective

One of the use-cases for the function `Entity::from_raw` is creating placeholder entity ids, which are meant to be overwritten later. If we use a constant for this instead of `from_raw`, it is more ergonomic and self-documenting.

## Solution

Add a constant that returns an entity ID with an index of `u32::MAX` and a generation of zero. Users are instructed to overwrite this value before using it.
2022-11-28 13:40:10 +00:00
Mitchell Henry
ca74271d07 Fix docs typo (#6771)
# Objective

- Fix a small typo

## Solution

- Type them correctly :D
2022-11-27 01:28:17 +00:00
Alice Cecile
3433a7bd68 Remove warning about missed events due to false positives (#6730)
# Objective

- Reverts #5730.
- Fixes #6173, fixes #6596.

## Solution

Remove the warning entirely.

## Changelog

You will no longer be spammed about

> Missed 31 `bevy_input:🐭:MouseMotion` events. Consider
reading from the `EventReader` more often (generally the best
solution) or calling Events::update() less frequently
(normally this is called once per frame). This problem is most
likely due to run criteria/fixed timesteps or consuming events
conditionally. See the Events documentation for
more information.

when you miss events. These warnings were often (but not always) a false positive. You can still check this manually by using `ManualEventReader::missed_events`
2022-11-23 00:27:29 +00:00
ira
a1607b8065 Rename EntityId to EntityIndex (#6732)
Continuation of #6107

Co-authored-by: devil-ira <justthecooldude@gmail.com>
2022-11-22 20:38:35 +00:00
Jakob Hellermann
cf46dd2e7e fix mutable aliases for a very short time if WorldCell is already borrowed (#6639)
# Objective

Consider the test
```rust
let cell = world.cell();
let _value_a = cell.resource_mut::<A>();
let _value_b = cell.resource_mut::<A>();
```

Currently, this will roughly execute

```rust
// first call
let value = unsafe {
    self.world
    .get_non_send_unchecked_mut_with_id(component_id)?
};
return Some(WorldBorrowMut::new(value, archetype_component_id, self.access)))

// second call
let value = unsafe {
    self.world
    .get_non_send_unchecked_mut_with_id(component_id)?
};
return Some(WorldBorrowMut::new(value, archetype_component_id, self.access)))
```
where `WorldBorrowMut::new` will panic if the resource is already borrowed.

This means, that `_value_a` will be created, the access checked (OK), then `value_b` will be created, and the access checked (`panic`).
For a moment, both `_value_a` and `_value_b` existed as `&mut T` to the same location, which is insta-UB as far as I understand it.

## Solution
Flip the order so that `WorldBorrowMut::new` first checks the access, _then_ fetches creates the value. To do that, we pass a `impl FnOnce() -> Mut<T>` instead of the `Mut<T>` directly:

```rust
let get_value = || unsafe {
    self.world
    .get_non_send_unchecked_mut_with_id(component_id)?
};
return Some(WorldBorrowMut::new(get_value, archetype_component_id, self.access)))
```
2022-11-22 15:31:18 +00:00
Ida Iyes
96e09f004b Fix PipeSystem panicking with exclusive systems (#6698)
Without this fix, piped systems containing exclusive systems fail to run, giving a runtime panic.
With this PR, running piped systems that contain exclusive systems now works.

## Explanation of the bug

This is because, unless overridden, the default implementation of `run` from the `System` trait simply calls `run_unsafe`. That is not valid for exclusive systems. They must always be called via `run`, as `run_unsafe` takes `&World` instead of `&mut World`.

Trivial reproduction example:
```rust
fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_system(exclusive.pipe(another))
        .run();
}

fn exclusive(_world: &mut World) {}
fn another() {}
```
If you run this, you will get a panic 'Cannot run exclusive systems with a shared World reference' and the backtrace shows how bevy (correctly) tries to call the `run` method (because the system is exclusive), but it is the implementation from the `System` trait (because `PipeSystem` does not have its own), which calls `run_unsafe` (incorrect):
 - 3: <bevy_ecs::system::system_piping::PipeSystem<SystemA,SystemB> as bevy_ecs::system::system::System>::run_unsafe
 - 4: bevy_ecs::system::system::System::run
2022-11-21 14:23:21 +00:00
James Liu
55ca7fc88e Split Component Ticks (#6547)
# Objective
Fixes #4884. `ComponentTicks` stores both added and changed ticks contiguously in the same 8 bytes. This is convenient when passing around both together, but causes half the bytes fetched from memory for the purposes of change detection to effectively go unused. This is inefficient when most queries (no filter, mutating *something*) only write out to the changed ticks.

## Solution
Split the storage for change detection ticks into two separate `Vec`s inside `Column`. Fetch only what is needed during iteration.

This also potentially also removes one blocker from autovectorization of dense queries.

EDIT: This is confirmed to enable autovectorization of dense queries in `for_each` and `par_for_each`  where possible.  Unfortunately `iter` has other blockers that prevent it.

### TODO

 - [x] Microbenchmark
 - [x] Check if this allows query iteration to autovectorize simple loops.
 - [x] Clean up all of the spurious tuples now littered throughout the API

### Open Questions

 - ~~Is `Mut::is_added` absolutely necessary? Can we not just use `Added` or `ChangeTrackers`?~~ It's optimized out if unused.
 - ~~Does the fetch of the added ticks get optimized out if not used?~~ Yes it is.

---

## Changelog
Added: `Tick`, a wrapper around a single change detection tick.
Added: `Column::get_added_ticks`
Added: `Column::get_column_ticks`
Added: `SparseSet::get_added_ticks`
Added: `SparseSet::get_column_ticks`
Changed: `Column` now stores added and changed ticks separately internally.
Changed: Most APIs returning `&UnsafeCell<ComponentTicks>` now returns `TickCells` instead, which contains two separate `&UnsafeCell<Tick>` for either component ticks.
Changed: `Query::for_each(_mut)`, `Query::par_for_each(_mut)` will now leverage autovectorization to speed up query iteration where possible.

## Migration Guide
TODO
2022-11-21 12:59:09 +00:00
Nicola Papale
15ea93a348 Fix size_hint for partially consumed QueryIter and QueryCombinationIter (#5214)
# Objective

Fix #5149

## Solution

Instead of returning the **total count** of elements in the `QueryIter` in
`size_hint`, we return the **count of remaining elements**. This
Fixes #5149 even when #5148 gets merged.

- https://github.com/bevyengine/bevy/issues/5149
- https://github.com/bevyengine/bevy/pull/5148

---

## Changelog

- Fix partially consumed `QueryIter` and `QueryCombinationIter` having invalid `size_hint`


Co-authored-by: Nicola Papale <nicopap@users.noreply.github.com>
2022-11-21 12:37:31 +00:00
James Liu
9f51651eac Replace BlobVec's swap_scratch with a swap_nonoverlapping (#4853)
# Objective
BlobVec currently relies on a scratch piece of memory allocated at initialization to make a temporary copy of a component when using `swap_remove_and_{forget/drop}`. This is potentially suboptimal as it writes to a, well-known, but random part of memory instead of using the stack.

## Solution
As the `FIXME` in the file states, replace `swap_scratch` with a call to `swap_nonoverlapping::<u8>`. The swapped last entry is returned as a `OwnedPtr`.

In theory, this should be faster as the temporary swap is allocated on the stack, `swap_nonoverlapping` allows for easier vectorization for bigger types, and the same memory is used between the swap and the returned `OwnedPtr`.
2022-11-16 20:57:43 +00:00
Nicola Papale
00684d95f7 Fix FilteredAccessSet get_conflicts inconsistency (#5105)
# Objective

* Enable `Res` and `Query` parameter mutual exclusion
* Required for https://github.com/bevyengine/bevy/pull/5080

The `FilteredAccessSet::get_conflicts` methods didn't work properly with
`Res` and `ResMut` parameters. Because those added their access by using
the `combined_access_mut` method and directly modifying the global
access state of the FilteredAccessSet. This caused an inconsistency,
because get_conflicts assumes that ALL added access have a corresponding
`FilteredAccess` added to the `filtered_accesses` field.

In practice, that means that SystemParam that adds their access through
the `Access` returned by `combined_access_mut` and the ones that add
their access using the `add` method lived in two different universes. As
a result, they could never be mutually exclusive.

## Solution

This commit fixes it by removing the `combined_access_mut` method. This
ensures that the `combined_access` field of FilteredAccessSet is always
updated consistently with the addition of a filter. When checking for
filtered access, it is now possible to account for `Res` and `ResMut`
invalid access. This is currently not needed, but might be in the
future.

We add the `add_unfiltered_{read,write}` methods to replace previous
usages of `combined_access_mut`.

We also add improved Debug implementations on FixedBitSet so that their
meaning is much clearer in debug output.


---

## Changelog

* Fix `Res` and `Query` parameter never being mutually exclusive.

## Migration Guide

Note: this mostly changes ECS internals, but since the API is public, it is technically breaking:
* Removed `FilteredAccessSet::combined_access_mut`
  * Replace _immutable_ usage of those by `combined_access`
  * For _mutable_ usages, use the new `add_unfiltered_{read,write}` methods instead of `combined_access_mut` followed by `add_{read,write}`
2022-11-16 11:05:48 +00:00
James Liu
6763b31479 Immutable sparse sets for metadata storage (#4928)
# Objective
Make core types in ECS smaller. The column sparse set in Tables is never updated after creation.

## Solution
Create `ImmutableSparseSet` which removes the capacity fields in the backing vec's and the APIs for inserting or removing elements. Drops the size of the sparse set by 3 usizes (24 bytes on 64-bit systems)

## Followup
~~After #4809, Archetype's component SparseSet should be replaced with it.~~ This has been done.

---

## Changelog
Removed: `Table::component_capacity`

## Migration Guide
`Table::component_capacity()` has been removed as Tables do not support adding/removing columns after construction.

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2022-11-15 22:21:19 +00:00
James Liu
11c544c29a Remove redundant table and sparse set component IDs from Archetype (#4927)
# Objective
Archetype is a deceptively large type in memory. It stores metadata about which components are in which storage in multiple locations, which is only used when creating new Archetypes while moving entities.

## Solution
Remove the redundant `Box<[ComponentId]>`s and iterate over the sparse set of component metadata instead. Reduces Archetype's size by 4 usizes (32 bytes on 64-bit systems), as well as the additional allocations for holding these slices.

It'd seem like there's a downside that the origin archetype has it's component metadata iterated over twice when creating a new archetype, but this change also removes the extra `Vec<ArchetypeComponentId>` allocations when creating a new archetype which may amortize out to a net gain here. This change likely negatively impacts creating new archetypes with a large number of components, but that's a cost mitigated by the fact that these archetypal relationships are cached in Edges and is incurred only once for each edge created.

## Additional Context
There are several other in-flight PRs that shrink Archetype:

 - #4800 merges the entities and rows Vecs together (shaves off 24 bytes per archetype) 
 - #4809 removes unique_components and moves it to it's own dedicated storage (shaves off 72 bytes per archetype)

---

## Changelog
Changed: `Archetype::table_components` and `Archetype::sparse_set_components` return iterators instead of slices. `Archetype::new` requires iterators instead of parallel slices/vecs.

## Migration Guide
Do I still need to do this? I really hope people were not relying on the public facing APIs changed here.
2022-11-15 21:39:21 +00:00
James Liu
688f13cd83 Fix get_unchecked_manual using archetype index instead of table row. (#6625)
# Objective
Fix #6623.

## Solution
Use the right table row instead of the `EntityLocation` archetype index.
2022-11-15 00:19:11 +00:00
Alessandro Salvatore Nicosia
13abb1fc16 derived Debug on EventReader (#6600)
# Objective
Fixes #6588 

## Solution

Added Debug to the derived traits of EventReader.
2022-11-14 23:08:28 +00:00
Jakob Hellermann
b2090e3a8d add Resources::iter to iterate over all resource IDs (#6592)
# Objective

In bevy 0.8 you could list all resources using `world.archetypes().resource().components()`. As far as I can tell the resource archetype has been replaced with the `Resources` storage, and it would be nice if it could be used to iterate over all resource component IDs as well.

## Solution

- add `fn Resources::iter(&self) -> impl Iterator<Item = (ComponentId, &ResourceData)>`
2022-11-14 23:08:27 +00:00
JoJoJet
3ac06b57e9 Respect alignment for zero-sized types stored in the world (#6618)
# Objective

Fixes #6615.

`BlobVec` does not respect alignment for zero-sized types, which results in UB whenever a ZST with alignment other than 1 is used in the world.

## Solution

Add the fn `bevy_ptr::dangling_with_align`.

---

## Changelog

+ Added the function `dangling_with_align` to `bevy_ptr`, which creates a well-aligned dangling pointer to a type whose alignment is not known at compile time.
2022-11-14 21:16:53 +00:00
Nicola Papale
1967c3ddef Fix Entity hygiene in WorldQuery (#6614)
# Objective

Fix #6593

## Solution

Fully qualify `Entity` in the `WorldQuery` macro
2022-11-14 14:01:16 +00:00
github-actions[bot]
920543c824 Release 0.9.0 (#6568)
Preparing next release
This PR has been auto-generated
2022-11-12 20:01:29 +00:00
James Liu
2179a3ebf4 Make Entity::to_bits const (#6559)
# Objective
Fix #6548. Most of these methods were already made `const` in #5688. `Entity::to_bits` is the only one that remained.

## Solution
Make it const.
2022-11-12 16:15:04 +00:00
ira
d688ba5f29 Add send_event and friends to WorldCell (#6515)
# Objective

Copy `send_event` and friends from `World` to `WorldCell`.

Clean up `bevy_winit` using `WorldCell::send_event`.

## Changelog

Added `send_event`, `send_event_default`, and `send_event_batch` to `WorldCell`.

Co-authored-by: devil-ira <justthecooldude@gmail.com>
2022-11-07 21:25:31 +00:00
ira
944b311c67 Improve logging consistency for entity despawning (#6501)
* Move the despawn debug log from `World::despawn` to `EntityMut::despawn`.
 * Move the despawn non-existent warning log from `Commands::despawn` to `World::despawn`.

This should make logging consistent regardless of which of the three `despawn` methods is used.

Co-authored-by: devil-ira <justthecooldude@gmail.com>
2022-11-07 19:23:34 +00:00
TimJentzsch
694c980c82 Fix clippy::iter_with_drain (#6485)
# Objective

Fixes #6483.

- Fix the [`clippy::iter_with_drain`](https://rust-lang.github.io/rust-clippy/master/index.html#iter_with_drain) warnings
- From the docs: "`.into_iter()` is simpler with better performance"

## Solution

- Replace `.drain(..)` for `Vec` with `.into_iter()`
2022-11-06 01:42:15 +00:00
JoJoJet
0e41b79a35 debug_checked_unwrap should track its caller (#6452)
# Objective

When an error causes `debug_checked_unreachable` to be called, the panic message unhelpfully points to the function definition instead of the place that caused the error.

## Solution

Add the `#[track_caller]` attribute in debug mode.
2022-11-05 16:15:08 +00:00
ira
b0bd8722f3 Fix unsound EntityMut::remove_children. Add EntityMut::world_scope (#6464)
`EntityMut::remove_children` does not call `self.update_location()` which is unsound.
Verified by adding the following assertion, which fails when running the tests.
```rust
let before = self.location();
self.update_location();
assert_eq!(before, self.location());
```

I also removed incorrect messages like "parent entity is not modified" and the unhelpful "Inserting a bundle in the children entities may change the parent entity's location if they were of the same archetype" which might lead people to think that's the *only* thing that can change the entity's location.

# Changelog
Added `EntityMut::world_scope`.

Co-authored-by: devil-ira <justthecooldude@gmail.com>
2022-11-04 17:30:40 +00:00
James Liu
ec8c8fbc8a Remove unnecesary branches/panics from Query accesses (#6461)
# Objective
Supercedes #6452. Upon inspection of the [generated assembly](https://gist.github.com/james7132/c2740c6941b80d7912f1e8888e223cbb#file-original-s) of a [simple Bevy binary](https://gist.github.com/james7132/c2740c6941b80d7912f1e8888e223cbb#file-source-rs) compiled with `cargo rustc --release -- --emit asm`, it's apparent that there are multiple unnecessary branches in the generated assembly:

```assembly
.LBB5_5:
	cmpq	%r10, %r11
	je	.LBB5_15
	movq	(%r11), %rcx
	movq	328(%r15), %rdx
	cmpq	%rdx, %rcx
	jae	.LBB5_14
	movq	312(%r15), %rdi
	leaq	(%rcx,%rcx,2), %rcx
	shlq	$5, %rcx
	movq	336(%r12), %rdx
	movq	64(%rdi,%rcx), %rax
	cmpq	%rdx, %rax
	jbe	.LBB5_4
	leaq	(%rdi,%rcx), %rsi
	movq	48(%rsi), %rbp
	shlq	$4, %rdx
	cmpq	$0, (%rbp,%rdx)
	je	.LBB5_4
	movq	344(%r12), %rbx
	cmpq	%rbx, %rax
	jbe	.LBB5_4
	shlq	$4, %rbx
	cmpq	$0, (%rbp,%rbx)
	je	.LBB5_4
	addq	$8, %r11
	movq	88(%rdi,%rcx), %rcx
	testq	%rcx, %rcx
	je	.LBB5_5
	movq	(%rsi), %rax
	movq	8(%rbp,%rdx), %rdx
	leaq	(%rdx,%rdx,4), %rdi
	shlq	$4, %rdi
	movq	32(%rax,%rdi), %rdx
	movq	56(%rax,%rdi), %r8
	movq	8(%rbp,%rbx), %rbp
	leaq	(%rbp,%rbp,4), %rbp
	shlq	$4, %rbp
	movq	32(%rax,%rbp), %r9
	xorl	%ebp, %ebp
	jmp	.LBB5_13
	.p2align	4, 0x90
```

Almost every one of the instructions starting with `j` is a potential branch, which can significantly slow down accesses. Of these, two labels are both common and never used:

```asm
.LBB5_14:
	leaq	__unnamed_2(%rip), %r8
	callq	_ZN4core9panicking18panic_bounds_check17h70367088e72af65aE
	ud2
.LBB5_4:
	callq	_ZN8bevy_ecs5query25debug_checked_unreachable17h0855ff520ceaea77E
	ud2
	.seh_endproc
```

These correpsond to subprocedure calls to panicking due to out of bounds from indexing `Tables` and `debug_checked_unreadable`. Both of which should be inlined and optimized out, but are not.

## Solution
Make `debug_checked_unreachable` a macro to forcibly inline either `unreachable!()` in debug builds, and `std::hint::unreachable_unchecked()` in release mode. Replace the `Tables` and `Archetype` index access with `get(id).unwrap_or_else(|| debug_checked_unreachable!())` to assume that the table or archetype provided exists.

This has no external breaking change of any kind.

The equivalent section of code with these changes removes most of the conditional jump instructions:

```asm
.LBB5_5:
	movss	(%rbx,%rbp,4), %xmm0
	movl	%r14d, 4(%r8,%rbp,8)
	addss	(%rdi,%rbp,4), %xmm0
	movss	%xmm0, (%rdi,%rbp,4)
	incq	%rbp
.LBB5_1:
	cmpq	%rdx, %rbp
	jne	.LBB5_5
	.p2align	4, 0x90
.LBB5_2:
	cmpq	%rcx, %rax
	je	.LBB5_6
	movq	(%rax), %rdx
	addq	$8, %rax
	movq	312(%rsi), %rbp
	leaq	(%rdx,%rdx,2), %rbx
	shlq	$5, %rbx
	movq	88(%rbp,%rbx), %rdx
	testq	%rdx, %rdx
	je	.LBB5_2
	leaq	(%rbx,%rbp), %r8
	movq	336(%r15), %rdi
	movq	344(%r15), %r9
	movq	48(%rbp,%rbx), %r10
	shlq	$4, %rdi
	movq	(%r8), %rbx
	movq	8(%r10,%rdi), %rdi
	leaq	(%rdi,%rdi,4), %rbp
	shlq	$4, %rbp
	movq	32(%rbx,%rbp), %rdi
	movq	56(%rbx,%rbp), %r8
	shlq	$4, %r9
	movq	8(%r10,%r9), %rbp
	leaq	(%rbp,%rbp,4), %rbp
	shlq	$4, %rbp
	movq	32(%rbx,%rbp), %rbx
	xorl	%ebp, %ebp
	jmp	.LBB5_5
.LBB5_6:
	addq	$40, %rsp
	popq	%rbx
	popq	%rbp
	popq	%rdi
	popq	%rsi
	popq	%r14
	popq	%r15
	retq
	.seh_endproc

```

## Performance

Microbenchmarks results:

<details>

```
group                                                    main                                     no-panic-query
-----                                                    ----                                     --------------
busy_systems/01x_entities_03_systems                     1.20     42.4±2.66µs        ? ?/sec      1.00     35.3±1.68µs        ? ?/sec
busy_systems/01x_entities_06_systems                     1.32     83.8±3.50µs        ? ?/sec      1.00     63.6±1.72µs        ? ?/sec
busy_systems/01x_entities_09_systems                     1.15    113.3±8.90µs        ? ?/sec      1.00     98.2±6.15µs        ? ?/sec
busy_systems/01x_entities_12_systems                     1.27   160.8±32.44µs        ? ?/sec      1.00    126.6±4.70µs        ? ?/sec
busy_systems/01x_entities_15_systems                     1.12    179.6±3.71µs        ? ?/sec      1.00   160.3±11.03µs        ? ?/sec
busy_systems/02x_entities_03_systems                     1.18     76.8±3.14µs        ? ?/sec      1.00     65.2±3.17µs        ? ?/sec
busy_systems/02x_entities_06_systems                     1.16    144.6±6.10µs        ? ?/sec      1.00    124.5±5.14µs        ? ?/sec
busy_systems/02x_entities_09_systems                     1.19    215.3±9.18µs        ? ?/sec      1.00    181.5±5.67µs        ? ?/sec
busy_systems/02x_entities_12_systems                     1.20    266.7±8.33µs        ? ?/sec      1.00    222.0±9.53µs        ? ?/sec
busy_systems/02x_entities_15_systems                     1.23   338.8±10.53µs        ? ?/sec      1.00    276.3±6.94µs        ? ?/sec
busy_systems/03x_entities_03_systems                     1.43    113.5±5.06µs        ? ?/sec      1.00     79.6±1.49µs        ? ?/sec
busy_systems/03x_entities_06_systems                     1.38   217.3±12.67µs        ? ?/sec      1.00    157.5±3.07µs        ? ?/sec
busy_systems/03x_entities_09_systems                     1.23   308.8±24.75µs        ? ?/sec      1.00    251.6±8.93µs        ? ?/sec
busy_systems/03x_entities_12_systems                     1.05   347.7±12.43µs        ? ?/sec      1.00   330.6±11.43µs        ? ?/sec
busy_systems/03x_entities_15_systems                     1.13   455.5±13.88µs        ? ?/sec      1.00   401.7±17.29µs        ? ?/sec
busy_systems/04x_entities_03_systems                     1.24    144.7±5.89µs        ? ?/sec      1.00    116.9±6.29µs        ? ?/sec
busy_systems/04x_entities_06_systems                     1.24   282.8±21.40µs        ? ?/sec      1.00   228.6±21.31µs        ? ?/sec
busy_systems/04x_entities_09_systems                     1.35   431.8±14.10µs        ? ?/sec      1.00    319.6±9.83µs        ? ?/sec
busy_systems/04x_entities_12_systems                     1.16   493.8±22.87µs        ? ?/sec      1.00   424.9±15.24µs        ? ?/sec
busy_systems/04x_entities_15_systems                     1.10   587.5±23.25µs        ? ?/sec      1.00   531.7±16.32µs        ? ?/sec
busy_systems/05x_entities_03_systems                     1.14    148.2±9.61µs        ? ?/sec      1.00    129.5±4.32µs        ? ?/sec
busy_systems/05x_entities_06_systems                     1.31   359.7±17.46µs        ? ?/sec      1.00   273.6±10.55µs        ? ?/sec
busy_systems/05x_entities_09_systems                     1.22   473.5±23.11µs        ? ?/sec      1.00   389.3±13.62µs        ? ?/sec
busy_systems/05x_entities_12_systems                     1.05   562.9±20.76µs        ? ?/sec      1.00   536.5±24.35µs        ? ?/sec
busy_systems/05x_entities_15_systems                     1.23   818.5±28.70µs        ? ?/sec      1.00   666.6±45.87µs        ? ?/sec
contrived/01x_entities_03_systems                        1.27     27.5±0.49µs        ? ?/sec      1.00     21.6±1.71µs        ? ?/sec
contrived/01x_entities_06_systems                        1.22     49.9±1.18µs        ? ?/sec      1.00     40.7±2.62µs        ? ?/sec
contrived/01x_entities_09_systems                        1.30     72.3±2.39µs        ? ?/sec      1.00     55.4±2.60µs        ? ?/sec
contrived/01x_entities_12_systems                        1.28     94.3±9.44µs        ? ?/sec      1.00     73.7±3.62µs        ? ?/sec
contrived/01x_entities_15_systems                        1.25    118.0±2.43µs        ? ?/sec      1.00     94.1±3.99µs        ? ?/sec
contrived/02x_entities_03_systems                        1.23     41.6±1.71µs        ? ?/sec      1.00     33.7±2.30µs        ? ?/sec
contrived/02x_entities_06_systems                        1.19     78.6±2.63µs        ? ?/sec      1.00     65.9±2.35µs        ? ?/sec
contrived/02x_entities_09_systems                        1.28    113.6±3.60µs        ? ?/sec      1.00     88.6±3.60µs        ? ?/sec
contrived/02x_entities_12_systems                        1.20    146.4±5.75µs        ? ?/sec      1.00    121.7±3.35µs        ? ?/sec
contrived/02x_entities_15_systems                        1.23    178.5±4.86µs        ? ?/sec      1.00    145.7±4.00µs        ? ?/sec
contrived/03x_entities_03_systems                        1.42     58.3±2.77µs        ? ?/sec      1.00     41.1±1.54µs        ? ?/sec
contrived/03x_entities_06_systems                        1.32    108.5±7.30µs        ? ?/sec      1.00     82.4±4.86µs        ? ?/sec
contrived/03x_entities_09_systems                        1.23    153.7±4.61µs        ? ?/sec      1.00    125.0±4.76µs        ? ?/sec
contrived/03x_entities_12_systems                        1.18    197.5±5.12µs        ? ?/sec      1.00    166.8±8.14µs        ? ?/sec
contrived/03x_entities_15_systems                        1.23    238.8±6.38µs        ? ?/sec      1.00    194.6±4.55µs        ? ?/sec
contrived/04x_entities_03_systems                        1.34     66.4±3.42µs        ? ?/sec      1.00     49.5±1.98µs        ? ?/sec
contrived/04x_entities_06_systems                        1.27    134.3±4.86µs        ? ?/sec      1.00    105.8±3.58µs        ? ?/sec
contrived/04x_entities_09_systems                        1.26    193.2±3.83µs        ? ?/sec      1.00    153.0±5.60µs        ? ?/sec
contrived/04x_entities_12_systems                        1.16    237.1±5.78µs        ? ?/sec      1.00   204.9±18.77µs        ? ?/sec
contrived/04x_entities_15_systems                        1.17    289.2±4.76µs        ? ?/sec      1.00    246.3±8.57µs        ? ?/sec
contrived/05x_entities_03_systems                        1.26     80.4±2.90µs        ? ?/sec      1.00     63.7±3.07µs        ? ?/sec
contrived/05x_entities_06_systems                        1.27   161.6±13.47µs        ? ?/sec      1.00    127.2±5.59µs        ? ?/sec
contrived/05x_entities_09_systems                        1.22    228.0±7.76µs        ? ?/sec      1.00    186.2±7.68µs        ? ?/sec
contrived/05x_entities_12_systems                        1.20    289.5±6.21µs        ? ?/sec      1.00    241.8±7.52µs        ? ?/sec
contrived/05x_entities_15_systems                        1.18   357.3±11.24µs        ? ?/sec      1.00    302.7±7.21µs        ? ?/sec
heavy_compute/base                                       1.01    302.4±3.52µs        ? ?/sec      1.00    300.2±3.40µs        ? ?/sec
iter_fragmented/base                                     1.00    348.1±7.51ns        ? ?/sec      1.01    351.9±8.32ns        ? ?/sec
iter_fragmented/foreach                                  1.03   239.8±23.78ns        ? ?/sec      1.00   233.8±18.12ns        ? ?/sec
iter_fragmented/foreach_wide                             1.00      3.9±0.13µs        ? ?/sec      1.02      4.0±0.22µs        ? ?/sec
iter_fragmented/wide                                     1.18      4.6±0.15µs        ? ?/sec      1.00      3.9±0.10µs        ? ?/sec
iter_fragmented_sparse/base                              1.02      8.1±0.15ns        ? ?/sec      1.00      7.9±0.56ns        ? ?/sec
iter_fragmented_sparse/foreach                           1.00      7.8±0.22ns        ? ?/sec      1.01      7.9±0.62ns        ? ?/sec
iter_fragmented_sparse/foreach_wide                      1.00     37.2±1.17ns        ? ?/sec      1.10     40.9±0.95ns        ? ?/sec
iter_fragmented_sparse/wide                              1.09     48.4±2.13ns        ? ?/sec      1.00    44.5±18.34ns        ? ?/sec
iter_simple/base                                         1.02      8.4±0.10µs        ? ?/sec      1.00      8.2±0.14µs        ? ?/sec
iter_simple/foreach                                      1.01      8.3±0.07µs        ? ?/sec      1.00      8.2±0.09µs        ? ?/sec
iter_simple/foreach_sparse_set                           1.00     25.3±0.32µs        ? ?/sec      1.02     25.7±0.42µs        ? ?/sec
iter_simple/foreach_wide                                 1.03     41.1±0.94µs        ? ?/sec      1.00     39.9±0.41µs        ? ?/sec
iter_simple/foreach_wide_sparse_set                      1.05    123.6±2.05µs        ? ?/sec      1.00    118.1±2.78µs        ? ?/sec
iter_simple/sparse_set                                   1.14     30.5±1.40µs        ? ?/sec      1.00     26.9±0.64µs        ? ?/sec
iter_simple/system                                       1.01      8.4±0.25µs        ? ?/sec      1.00      8.4±0.11µs        ? ?/sec
iter_simple/wide                                         1.18     48.2±0.62µs        ? ?/sec      1.00     40.7±0.38µs        ? ?/sec
iter_simple/wide_sparse_set                              1.12   140.8±21.56µs        ? ?/sec      1.00    126.0±2.30µs        ? ?/sec
query_get/50000_entities_sparse                          1.17    378.6±7.60µs        ? ?/sec      1.00   324.1±23.17µs        ? ?/sec
query_get/50000_entities_table                           1.08   330.9±10.90µs        ? ?/sec      1.00    306.8±4.98µs        ? ?/sec
query_get_component/50000_entities_sparse                1.00   976.7±19.55µs        ? ?/sec      1.00   979.8±35.87µs        ? ?/sec
query_get_component/50000_entities_table                 1.00  1029.0±15.11µs        ? ?/sec      1.05  1080.0±59.18µs        ? ?/sec
query_get_component_simple/system                        1.13   839.7±14.18µs        ? ?/sec      1.00   742.8±10.72µs        ? ?/sec
query_get_component_simple/unchecked                     1.01   909.0±15.17µs        ? ?/sec      1.00   898.0±13.56µs        ? ?/sec
query_get_many_10/50000_calls_sparse                     1.04      5.5±0.54ms        ? ?/sec      1.00      5.3±0.67ms        ? ?/sec
query_get_many_10/50000_calls_table                      1.01      4.9±0.49ms        ? ?/sec      1.00      4.8±0.45ms        ? ?/sec
query_get_many_2/50000_calls_sparse                      1.28  848.4±210.89µs        ? ?/sec      1.00   664.8±47.69µs        ? ?/sec
query_get_many_2/50000_calls_table                       1.05   779.0±73.85µs        ? ?/sec      1.00   739.2±83.02µs        ? ?/sec
query_get_many_5/50000_calls_sparse                      1.05      2.4±0.37ms        ? ?/sec      1.00      2.3±0.33ms        ? ?/sec
query_get_many_5/50000_calls_table                       1.00  1939.9±75.22µs        ? ?/sec      1.04      2.0±0.19ms        ? ?/sec
run_criteria/yes_using_query/001_systems                 1.00      3.7±0.38µs        ? ?/sec      1.30      4.9±0.14µs        ? ?/sec
run_criteria/yes_using_query/006_systems                 1.00      8.9±0.40µs        ? ?/sec      1.17     10.3±0.57µs        ? ?/sec
run_criteria/yes_using_query/011_systems                 1.00     13.9±0.49µs        ? ?/sec      1.08     15.0±0.89µs        ? ?/sec
run_criteria/yes_using_query/016_systems                 1.00     18.8±0.74µs        ? ?/sec      1.00     18.8±1.43µs        ? ?/sec
run_criteria/yes_using_query/021_systems                 1.07     24.1±0.87µs        ? ?/sec      1.00     22.6±1.58µs        ? ?/sec
run_criteria/yes_using_query/026_systems                 1.04     27.9±0.62µs        ? ?/sec      1.00     26.8±1.71µs        ? ?/sec
run_criteria/yes_using_query/031_systems                 1.09     33.3±1.03µs        ? ?/sec      1.00     30.5±2.18µs        ? ?/sec
run_criteria/yes_using_query/036_systems                 1.14     38.7±0.80µs        ? ?/sec      1.00     33.9±1.75µs        ? ?/sec
run_criteria/yes_using_query/041_systems                 1.18     43.7±1.07µs        ? ?/sec      1.00     37.0±2.39µs        ? ?/sec
run_criteria/yes_using_query/046_systems                 1.14     47.6±1.16µs        ? ?/sec      1.00     41.9±2.09µs        ? ?/sec
run_criteria/yes_using_query/051_systems                 1.17     52.9±2.04µs        ? ?/sec      1.00     45.3±1.75µs        ? ?/sec
run_criteria/yes_using_query/056_systems                 1.25     59.2±2.38µs        ? ?/sec      1.00     47.2±2.01µs        ? ?/sec
run_criteria/yes_using_query/061_systems                 1.28    66.1±15.84µs        ? ?/sec      1.00     51.5±2.47µs        ? ?/sec
run_criteria/yes_using_query/066_systems                 1.28     70.2±2.57µs        ? ?/sec      1.00     54.7±2.58µs        ? ?/sec
run_criteria/yes_using_query/071_systems                 1.30     75.5±2.27µs        ? ?/sec      1.00     58.2±3.31µs        ? ?/sec
run_criteria/yes_using_query/076_systems                 1.26     81.5±2.66µs        ? ?/sec      1.00     64.5±3.13µs        ? ?/sec
run_criteria/yes_using_query/081_systems                 1.29     89.7±2.58µs        ? ?/sec      1.00     69.3±3.47µs        ? ?/sec
run_criteria/yes_using_query/086_systems                 1.33     95.6±3.39µs        ? ?/sec      1.00     71.8±3.48µs        ? ?/sec
run_criteria/yes_using_query/091_systems                 1.25    102.0±3.67µs        ? ?/sec      1.00     81.4±4.82µs        ? ?/sec
run_criteria/yes_using_query/096_systems                 1.33    111.7±3.29µs        ? ?/sec      1.00     83.8±4.15µs        ? ?/sec
run_criteria/yes_using_query/101_systems                 1.29   113.2±12.04µs        ? ?/sec      1.00     87.7±5.15µs        ? ?/sec
world_query_for_each/50000_entities_sparse               1.00     47.4±0.51µs        ? ?/sec      1.00     47.3±0.33µs        ? ?/sec
world_query_for_each/50000_entities_table                1.00     27.2±0.50µs        ? ?/sec      1.00     27.2±0.17µs        ? ?/sec
world_query_get/50000_entities_sparse_wide               1.09    210.5±1.78µs        ? ?/sec      1.00    192.5±2.61µs        ? ?/sec
world_query_get/50000_entities_table                     1.00    127.7±2.09µs        ? ?/sec      1.07    136.2±5.95µs        ? ?/sec
world_query_get/50000_entities_table_wide                1.00    209.8±2.37µs        ? ?/sec      1.15    240.6±2.04µs        ? ?/sec
world_query_iter/50000_entities_sparse                   1.00     54.2±0.36µs        ? ?/sec      1.01     54.7±0.61µs        ? ?/sec
world_query_iter/50000_entities_table                    1.00     27.2±0.31µs        ? ?/sec      1.00     27.3±0.64µs        ? ?/sec
```
</details>

NOTE: This PR includes a change to enable LTO on our benchmarks to get a "fully optimized" baseline for our benchmarks. Both the main and the current PR's results were with LTO enabled.
2022-11-04 06:04:55 +00:00
Carter Anderson
2e653e5774 Fix spawning empty bundles (#6425)
# Objective

Alternative to #6424 
Fixes #6226

Fixes spawning empty bundles

## Solution

Add `BundleComponentStatus` trait and implement it for `AddBundle` and a new `SpawnBundleStatus` type (which always returns an Added status). `write_components` is now generic on `BundleComponentStatus` instead of taking `AddBundle` directly. This means BundleSpawner can now avoid needing AddBundle from the Empty archetype, which means BundleSpawner no longer needs a reference to the original archetype.

In theory this cuts down on the work done in `write_components` when spawning, but I'm seeing no change in the spawn benchmarks.
2022-11-03 22:50:41 +00:00
Boxy
30e35764a1 Replace WorldQueryGats trait with actual gats (#6319)
# Objective

Replace `WorldQueryGats` trait with actual gats

## Solution

Replace `WorldQueryGats` trait with actual gats

---

## Changelog

- Replaced `WorldQueryGats` trait with actual gats

## Migration Guide

- Replace usage of `WorldQueryGats` assoc types with the actual gats on `WorldQuery` trait
2022-11-03 16:33:05 +00:00
James Liu
54a1e51623 TaskPool Panic Handling (#6443)
# Objective
Right now, the `TaskPool` implementation allows panics to permanently kill worker threads upon panicking. This is currently non-recoverable without using a `std::panic::catch_unwind` in every scheduled task. This is poor ergonomics and even poorer developer experience. This is exacerbated by #2250 as these threads are global and cannot be replaced after initialization.

Removes the need for temporary fixes like #4998. Fixes #4996. Fixes #6081. Fixes #5285. Fixes #5054. Supersedes #2307.

## Solution

The current solution is to wrap `Executor::run` in `TaskPool` with a `catch_unwind`, and discarding the potential panic. This was taken straight from [smol](404c7bcc0a/src/spawn.rs (L44))'s current implementation. ~~However, this is not entirely ideal as:~~
 
 - ~~the signaled to the awaiting task. We would need to change `Task<T>` to use `async_task::FallibleTask` internally, and even then it doesn't signal *why* it panicked, just that it did.~~ (See below).
 - ~~no error is logged of any kind~~ (See below)
 - ~~it's unclear if it drops other tasks in the executor~~ (it does not)
 - ~~This allows the ECS parallel executor to keep chugging even though a system's task has been dropped. This inevitably leads to deadlock in the executor.~~ Assuming we don't catch the unwind in ParallelExecutor, this will naturally kill the main thread.

### Alternatives
A final solution likely will incorporate elements of any or all of the following.

#### ~~Log and Ignore~~
~~Log the panic, drop the task, keep chugging. This only addresses the discoverability of the panic. The process will continue to run, probably deadlocking the executor. tokio's detatched tasks operate in this fashion.~~

Panics already do this by default, even when caught by `catch_unwind`.

#### ~~`catch_unwind` in `ParallelExecutor`~~
~~Add another layer catching system-level panics into the `ParallelExecutor`. How the executor continues when a core dependency of many systems fails to run is up for debate.~~

`async_task::Task`  bubbles up panics already, this will transitively push panics all the way to the main thread.

#### ~~Emulate/Copy `tokio::JoinHandle` with `Task<T>`~~
~~`tokio::JoinHandle<T>` bubbles up the panic from the underlying task when awaited. This can be transitively applied across other APIs that also use `Task<T>` like `Query::par_for_each` and `TaskPool::scope`, bubbling up the panic until it's either caught or it reaches the main thread.~~

`async_task::Task`  bubbles up panics already, this will transitively push panics all the way to the main thread.

#### Abort on Panic
The nuclear option. Log the error, abort the entire process on any thread in the task pool panicking. Definitely avoids any additional infrastructure for passing the panic around, and might actually lead to more efficient code as any unwinding is optimized out. However gives the developer zero options for dealing with the issue, a seemingly poor choice for debuggability, and prevents graceful shutdown of the process. Potentially an option for handling very low-level task management (a la #4740). Roughly takes the shape of:

```rust
struct AbortOnPanic;

impl Drop for AbortOnPanic {
   fn drop(&mut self) {
     abort!();
   }
}

let guard = AbortOnPanic;
// Run task
std::mem::forget(AbortOnPanic);
```

---

## Changelog

Changed: `bevy_tasks::TaskPool`'s threads  will no longer terminate permanently when a task scheduled onto them panics.
Changed: `bevy_tasks::Task` and`bevy_tasks::Scope` will propagate panics in the spawned tasks/scopes to the parent thread.
2022-11-02 23:40:08 +00:00
Alice Cecile
334e09892b Revert "Show prelude re-exports in docs (#6448)" (#6449)
This reverts commit 53d387f340.

# Objective

Reverts #6448. This didn't have the intended effect: we're now getting bevy::prelude shown in the docs again.

Co-authored-by: Alejandro Pascual <alejandro.pascual.pozo@gmail.com>
2022-11-02 20:40:45 +00:00
Alejandro Pascual
53d387f340 Show prelude re-exports in docs (#6448)
# Objective

- Right now re-exports are completely hidden in prelude docs.
- Fixes #6433

## Solution

- We could show the re-exports without inlining their documentation.
2022-11-02 19:35:06 +00:00
Edvin Kjell
a8a62fcf3d [Fixes #6059] `Entity`'s “ID” should be named “index” instead (#6107)
# Objective

Fixes #6059, changing all incorrect occurrences of ``id`` in the ``entity`` module to ``index``:

* struct level documentation,
* ``id`` struct field,
* ``id`` method and its documentation.

## Solution

Renaming and verifying using CI. 


Co-authored-by: Edvin Kjell <43633999+Edwox@users.noreply.github.com>
2022-11-02 15:19:50 +00:00
CGMossa
1f22d54489 Fixed docs for derive(WorldQuery). (#5283)
For `derive(WorldQuery)`, there are three structs generated, `Item`, `Fetch` and `State`. 
These inherit the visibility of the derived structure, thus `#![warn(missing_docs)]` would
warn about missing documentation for these structures.

- [ ] I'd like some advice on what to write here, as I personally don't really understand `Fetch` nor `State`.
2022-11-01 23:44:55 +00:00
JoJoJet
3d6706f86d Speed up Query::get_many and add benchmarks (#6400)
# Objective

* Add benchmarks for `Query::get_many`.
* Speed up `Query::get_many`.

## Solution

Previously, `get_many` and `get_many_mut` used the method `array::map`, which tends to optimize very poorly. This PR replaces uses of that method with loops.

## Benchmarks

| Benchmark name                       | Execution time | Change from this PR |
|--------------------------------------|----------------|---------------------|
| query_get_many_2/50000_calls_table   | 1.3732 ms      | -24.967%            |
| query_get_many_2/50000_calls_sparse  | 1.3826 ms      | -24.572%            |
| query_get_many_5/50000_calls_table   | 2.6833 ms      | -30.681%            |
| query_get_many_5/50000_calls_sparse  | 2.9936 ms      | -30.672%            |
| query_get_many_10/50000_calls_table  | 5.7771 ms      | -36.950%            |
| query_get_many_10/50000_calls_sparse | 7.4345 ms      | -36.987%            |
2022-11-01 03:51:41 +00:00
Lucas Jenß
e7719bf245 Mention world_query(ignore) attribute for WorldQuery derivation (#6309)
# Objective

Add documentation `#[world_query(ignore)]`. Fixes #6283.

---

I've only described it's behavior so far (which appears to be the same as with `system_param`). Is there another use-case for this besides with `PhantomData`? I could only find a single usage of this construct on GitHub, which is [here](ffcb816927/bevy/examples/ecs/custom_query_param.rs (L102)). 

I was also wondering if it would make sense to add a usage example to the `custom_query_example`? 🤔 That's why it's currently still in there.




Co-authored-by: Lucas Jenß <243719+x3ro@users.noreply.github.com>
2022-11-01 03:15:34 +00:00
JoJoJet
336049da68 Remove outdated uses of single-tuple bundles (#6406)
# Objective

Bevy still has many instances of using single-tuples `(T,)` to create a bundle. Due to #2975, this is no longer necessary.

## Solution

Search for regex `\(.+\s*,\)`. This should have found every instance.
2022-10-29 18:15:28 +00:00
Carter Anderson
dfb80ee74f Fix query.to_readonly().get_component_mut() soundness bug (#6401)
# Objective

Fix the soundness issue outlined in #5866. In short the problem is that `query.to_readonly().get_component_mut::<T>()` can provide unsound mutable access to the component. This PR is an alternative to just removing the offending api. Given that `to_readonly` is a useful tool, I think this approach is a preferable short term solution. Long term I think theres a better solution out there, but we can find that on its own time.

## Solution

Add what amounts to a "dirty flag" that marks Queries that have been converted to their read-only variant via `to_readonly` as dirty. When this flag is set to true, `get_component_mut` will fail with an error, preventing the unsound access.
2022-10-29 04:13:54 +00:00