Commit graph

695 commits

Author SHA1 Message Date
Mike
d76b53bf4d Separate Extract from Sub App Schedule (#7046)
# Objective

- This pulls out some of the changes to Plugin setup and sub apps from #6503 to make that PR easier to review.
- Separate the extract stage from running the sub app's schedule to allow for them to be run on separate threads in the future
- Fixes #6990

## Solution

- add a run method to `SubApp` that runs the schedule
- change the name of `sub_app_runner` to extract to make it clear that this function is only for extracting data between the main app and the sub app
- remove the extract stage from the sub app schedule so it can be run separately. This is done by adding a `setup` method to the `Plugin` trait that runs after all plugin build methods run. This is required to allow the extract stage to be removed from the schedule after all the plugins have added their systems to the stage. We will also need the setup method for pipelined rendering to setup the render thread. See e3267965e1/crates/bevy_render/src/pipelined_rendering.rs (L57-L98)

## Changelog

- Separate SubApp Extract stage from running the sub app schedule.

## Migration Guide

### SubApp `runner` has conceptually been changed to an `extract` function.

The `runner` no longer is in charge of running the sub app schedule. It's only concern is now moving data between the main world and the sub app. The `sub_app.app.schedule` is now run for you after the provided function is called.

```rust
// before
fn main() {
    let sub_app = App::empty();
    sub_app.add_stage(MyStage, SystemStage::parallel());
    
    App::new().add_sub_app(MySubApp, sub_app, move |main_world, sub_app| {
        extract(app_world, render_app);
        render_app.app.schedule.run();
    });
}

// after
fn main() {
        let sub_app = App::empty();
    sub_app.add_stage(MyStage, SystemStage::parallel());
    
    App::new().add_sub_app(MySubApp, sub_app, move |main_world, sub_app| {
        extract(app_world, render_app);
        // schedule is automatically called for you after extract is run
    });
}
```
2023-01-09 19:24:54 +00:00
JoJoJet
1efdbb7e3e Remove the SystemParamState trait and remove types like ResState (#6919)
Spiritual successor to #5205.
Actual successor to #6865.

# Objective

Currently, system params are defined using three traits: `SystemParam`, `ReadOnlySystemParam`, `SystemParamState`. The behavior for each param is specified by the `SystemParamState` trait, while `SystemParam` simply defers to the state.

Splitting the traits in this way makes it easier to implement within macros, but it increases the cognitive load. Worst of all, this approach requires each `MySystemParam` to have a public `MySystemParamState` type associated with it.

## Solution

* Merge the trait `SystemParamState` into `SystemParam`.
* Remove all trivial `SystemParam` state types. 
  * `OptionNonSendMutState<T>`: you will not be missed.

---

- [x] Fix/resolve the remaining test failure.

## Changelog

* Removed the trait `SystemParamState`, merging its functionality into `SystemParam`.

## Migration Guide

**Note**: this should replace the migration guide for #6865.
This is relative to Bevy 0.9, not main.

The traits `SystemParamState` and `SystemParamFetch` have been removed, and their functionality has been transferred to `SystemParam`.


```rust
// Before (0.9)
impl SystemParam for MyParam<'_, '_> {
    type State = MyParamState;
}
unsafe impl SystemParamState for MyParamState {
    fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self { ... }
}
unsafe impl<'w, 's> SystemParamFetch<'w, 's> for MyParamState {
    type Item = MyParam<'w, 's>;
    fn get_param(&mut self, ...) -> Self::Item;
}
unsafe impl ReadOnlySystemParamFetch for MyParamState { }

// After (0.10)
unsafe impl SystemParam for MyParam<'_, '_> {
    type State = MyParamState;
    type Item<'w, 's> = MyParam<'w, 's>;
    fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State { ... }
    fn get_param<'w, 's>(state: &mut Self::State, ...) -> Self::Item<'w, 's>;
}
unsafe impl ReadOnlySystemParam for MyParam<'_, '_> { }
```

The trait `ReadOnlySystemParamFetch` has been replaced with `ReadOnlySystemParam`.

```rust
// Before
unsafe impl ReadOnlySystemParamFetch for MyParamState {}

// After
unsafe impl ReadOnlySystemParam for MyParam<'_, '_> {}
```
2023-01-07 23:20:32 +00:00
JoJoJet
076e6f780c Update an outdated example for Mut::map_unchanged (#7115)
# Objective

- The doctest for `Mut::map_unchanged` uses a fake function `set_if_not_equal` to demonstrate usage.
- Now that #6853 has been merged, we can use `Mut::set_if_neq` directly instead of mocking it.
2023-01-06 23:24:25 +00:00
iiYese
653c062ba3 Added missing details to SystemParam Local documentation. (#7106)
# Objective

`SystemParam` `Local`s documentation currently leaves out information that should be documented.
- What happens when multiple `SystemParam`s within the same system have the same `Local` type.
- What lifetime parameter is expected by `Local`.
 
## Solution

- Added sentences to documentation to communicate this information.
- Renamed `Local` lifetimes in code to `'s` where they previously were not. Users can get complicated incorrect suggested fixes if they pass the wrong lifetime. Some instance of the code had `'w` indicating the expected lifetime might not have been known to those that wrote the code either.

Co-authored-by: iiYese <83026177+iiYese@users.noreply.github.com>
2023-01-06 15:40:10 +00:00
Rob Parrett
3dd8b42f72 Fix various typos (#7096)
I stumbled across a typo in some docs. Fixed some more while I was in there.
2023-01-06 00:43:30 +00:00
JoJoJet
8ca3d0462c Allow SystemParams with private fields (#7056)
# Objective

- Fix #4200

Currently, `#[derive(SystemParam)]` publicly exposes each field type, which makes it impossible to encapsulate private fields.

## Solution

Previously, the fields were leaked because they were used as an input generic type to the macro-generated `SystemParam::State` struct. That type has been changed to store its state in a field with a specific type, instead of a generic type.

---

## Changelog

- Fixed a bug that caused `#[derive(SystemParam)]` to leak the types of private fields.
2023-01-04 23:25:36 +00:00
James Liu
a5b1c46d5b Extend EntityLocation with TableId and TableRow (#6681)
# Objective
`Query::get` and other random access methods require looking up `EntityLocation` for every provided entity, then always looking up the `Archetype` to get the table ID and table row. This requires 4 total random fetches from memory: the `Entities` lookup, the `Archetype` lookup, the table row lookup, and the final fetch from table/sparse sets. If `EntityLocation` contains the table ID and table row, only the `Entities` lookup and the final storage fetch are required.

## Solution
Add `TableId` and table row to `EntityLocation`. Ensure it's updated whenever entities are moved around. To ensure `EntityMeta` does not grow bigger, both `TableId` and `ArchetypeId` have been shrunk to u32, and the archetype index and table row are stored as u32s instead of as usizes. This should shrink `EntityMeta` by 4 bytes, from 24 to 20 bytes, as there is no padding anymore due to the change in alignment.

This idea was partially concocted by @BoxyUwU. 

## Performance
This should restore the `Query::get` "gains" lost to #6625 that were introduced in #4800 without being unsound, and also incorporates some of the memory usage reductions seen in #3678.

This also removes the same lookups during add/remove/spawn commands, so there may be a bit of a speedup in commands and `Entity{Ref,Mut}`.

---

## Changelog
Added: `EntityLocation::table_id`
Added: `EntityLocation::table_row`.
Changed: `World`s can now only hold a maximum of 2<sup>32</sup>- 1 archetypes.
Changed: `World`s can now only hold a maximum of 2<sup>32</sup> - 1 tables.

## Migration Guide

A `World` can only hold a maximum of 2<sup>32</sup> - 1 archetypes and tables now. If your use case requires more than this, please file an issue explaining your use case.
2023-01-02 21:25:04 +00:00
Nile
0ddaa7e83a Round out the untyped api s (#7009)
# Objective

Bevy uses custom `Ptr` types so the rust borrow checker can help ensure lifetimes are correct, even when types aren't known. However, these types don't benefit from the automatic lifetime coercion regular rust references enjoy

## Solution

Add a couple methods to Ptr, PtrMut, and MutUntyped to allow for easy usage of these types in more complex scenarios.

## Changelog

- Added `as_mut` and `as_ref` methods to `MutUntyped`.
- Added `shrink` and `as_ref` methods to `PtrMut`.

## Migration Guide

- `MutUntyped::into_inner` now marks things as changed.
2022-12-27 16:05:16 +00:00
JoJoJet
48b4a45d82 Add a const PipeSystem constructor (#7019)
# Objective

Fix #5914.

`PipeSystem` cannot be constructed in `const` contexts.

## Solution

Add a const `PipeSystem::new` function.
2022-12-25 00:51:19 +00:00
JoJoJet
a91f89db73 Add a basic example for system ordering (#7017)
# Objective

Fix #5653.

## Solution

- Add an example of how systems can be ordered from within a stage.
- Update some docs from before #4224
2022-12-25 00:51:17 +00:00
JoJoJet
65d390163f Add a trait for commands that run for a given Entity (#7015)
# Objective

Resolve #6156.

The most common type of command is one that runs for a single entity. Built-in commands like this can be ergonomically added to the command queue using the `EntityCommands` struct. However, adding custom entity commands to the queue is quite cumbersome. You must first spawn an entity, store its ID in a local, then construct a command using that ID and add it to the queue. This prevents method chaining, which is the main benefit of using `EntityCommands`.

### Example (before)

```rust
struct MyCustomCommand(Entity);

impl Command for MyCustomCommand { ... }

let id = commands.spawn((...)).id();
commmands.add(MyCustomCommand(id));
```

## Solution

Add the `EntityCommand` trait, which allows directly adding per-entity commands to the `EntityCommands` struct.

### Example (after)

```rust
struct MyCustomCommand;

impl EntityCommand for MyCustomCommand { ... }

commands.spawn((...)).add(MyCustomCommand);
```
---

## Changelog

- Added the trait `EntityCommand`. This is a counterpart of `Command` for types that execute code for a single entity.

## Future Work

If we feel its necessary, we can simplify built-in commands (such as `Despawn`) to use this trait.
2022-12-25 00:51:16 +00:00
JoJoJet
83b602a77c Relax Sync bound on anonymous Commands (#7014)
# Objective

Any closure with the signature `FnOnce(&mut World)` implicitly implements the trait `Command` due to a blanket implementation. However, this implementation unnecessarily has the `Sync` bound, which limits the types that can be used.

## Solution

Remove the bound.

---

## Changelog

- `Command` closures no longer need to implement the marker trait `std::marker::Sync`.
2022-12-25 00:51:14 +00:00
Aceeri
9717204aef Rework manual event iterator so we can actually name the type (#5735)
# Objective
- Be able to name the type that `ManualEventReader::iter/iter_with_id` returns and `EventReader::iter/iter_with_id` by proxy.
  Currently for the purpose of https://github.com/bevyengine/bevy/pull/5719

## Solution
- Create a custom `Iterator` type.
2022-12-25 00:39:27 +00:00
JoJoJet
0d98327ce7 Support SystemParam types with const generics (#7001)
# Objective

* Currently, the `SystemParam` derive does not support types with const generic parameters.
  * If you try to use const generics, the error message is cryptic and unhelpful.
* Continuation of the work started in #6867 and #6957.

## Solution

Allow const generic parameters to be used with `#[derive(SystemParam)]`.
2022-12-25 00:06:23 +00:00
JoJoJet
fa2b5f2b36 Add documentation to ParamSet (#6998)
# Objective

Fixes #4729.
Continuation of #4854.

## Solution

Add documentation to `ParamSet` and its methods. Includes examples suggested by community members in the original PR.


Co-authored-by: Nanox19435 <50684926+Nanox19435@users.noreply.github.com>
Co-authored-by: JoJoJet <21144246+JoJoJet@users.noreply.github.com>
2022-12-25 00:06:22 +00:00
JoJoJet
025996b18c Lift the 16-field limit from the SystemParam derive (#6867)
# Objective

* The `SystemParam` derive internally uses tuples, which means it is constrained by the 16-field limit on `all_tuples`.
    * The error message if you exceed this limit is abysmal.
* Supercedes #5965 -- this does the same thing, but is simpler.

## Solution

If any tuples have more than 16 fields, they are folded into tuples of tuples until they are under the 16-field limit.
2022-12-21 01:54:10 +00:00
JoJoJet
0363e0b32a Support tuple structs with #[derive(SystemParam)] (#6957)
# Objective

Currently, only named structs can be used with the `SystemParam` derive macro.

## Solution

Remove the restriction. Tuple structs and unit structs are now supported.

---

## Changelog

+ Added support for tuple structs and unit structs to the `SystemParam` derive macro.
2022-12-20 23:45:44 +00:00
ira
0761594dd8 Use World helper methods for sending HierarchyEvents (#6921)
A code-quality PR

Also cleans up the helper methods by just importing the `Event` type

Co-authored-by: devil-ira <justthecooldude@gmail.com>
2022-12-20 16:17:07 +00:00
Zeenobit
f8e4b755ff Add EntityMap::iter() (#6935)
# Objective

There is currently no way to iterate over key/value pairs inside an `EntityMap`, which makes the usage of this struct very awkward. I couldn't think of a good reason why the `iter()` function should not be exposed, considering the interface already exposes `keys()` and `values()`, so I made this PR.

## Solution

Implement `iter()` for `EntityMap` in terms of its inner map type.
2022-12-16 20:14:13 +00:00
Rob Parrett
ec0478d100 Fix clippy lints and failed test with Rust 1.66 (#6945)
# Objective

[Rust 1.66](https://blog.rust-lang.org/inside-rust/2022/12/12/1.66.0-prerelease.html) is coming in a few days, and bevy doesn't build with it.

Fix that.

## Solution

Replace output from a trybuild test, and fix a few new instances of `needless_borrow` and `unnecessary_cast` that are now caught.

## Note

Due to the trybuild test, this can't be merged until 1.66 is released.
2022-12-15 18:05:15 +00:00
James Liu
79b9231b74 Move system_commands spans into apply_buffers (#6900)
# Objective
A separate `tracing` span for running a system's commands is created, even if the system doesn't have commands. This is adding extra measuring overhead (see #4892) where it's not needed.

## Solution
Move the span into `ParallelCommandState` and `CommandQueue`'s `SystemParamState::apply`. To get the right metadata for the span, a additional `&SystemMeta` parameter was added to `SystemParamState::apply`.

---

## Changelog
Added: `SystemMeta::name`
Changed: Systems without `Commands` and  `ParallelCommands` will no longer show a "system_commands" span when profiling.
Changed: `SystemParamState::apply` now takes a `&SystemMeta` parameter in addition to the provided `&mut World`.
2022-12-11 23:04:04 +00:00
Zoey
4820917af6 Add set_if_neq method to DetectChanges trait (Rebased) (#6853)
# Objective

Change detection can be spuriously triggered by setting a field to the same value as before. As a result, a common pattern is to write:

```rust
if *foo != value {
  *foo = value;
}
```

This is confusing to read, and heavy on boilerplate.

Adopted from #5373, but untangled and rebased to current `bevy/main`.

## Solution

    1. Add a method to the `DetectChanges` trait that implements this boilerplate when the appropriate trait bounds are met.

    2. Document this minor footgun, and point users to it.


## Changelog

    * added the `set_if_neq` method to avoid triggering change detection when the new and previous values are equal. This will work on both components and resources.


## Migration Guide

If you are manually checking if a component or resource's value is equal to its new value before setting it to avoid triggering change detection, migrate to the clearer and more convenient `set_if_neq` method.
## Context

Related to #2363 as it avoids triggering change detection, but not a complete solution (as it still requires triggering it when real changes are made).



Co-authored-by: Zoey <Dessix@Dessix.net>
2022-12-11 19:24:19 +00:00
James Liu
87bf0e2664 Remove unnecessary branching from bundle insertion (#6902)
# Objective
Speed up bundle insertion and spawning from a bundle.

## Solution
Use the same technique used in #6800 to remove the branch on storage type when writing components from a `Bundle` into storage.

 - Add a `StorageType` argument to the closure on `Bundle::get_components`.
 - Pass `C::Storage::STORAGE_TYPE` into that argument.
 - Match on that argument instead of reading from a `Vec<StorageType>` in `BundleInfo`.
 - Marked all implementations of `Bundle::get_components` as inline to encourage dead code elimination.

The `Vec<StorageType>` in `BundleInfo` was also removed as it's no longer needed. If users were reliant on this, they can either use the compile time constants or fetch the information from `Components`. Should save a rather negligible amount of memory.

## Performance
Microbenchmarks show a slight improvement to inserting components into existing entities, as well as spawning from a bundle. Ranging about 8-16% faster depending on the benchmark.

```
group                                          main                                    soft-constant-write-components
-----                                          ----                                    ------------------------------
add_remove/sparse_set                          1.08  1019.0±80.10µs        ? ?/sec     1.00   944.6±66.86µs        ? ?/sec
add_remove/table                               1.07  1343.3±20.37µs        ? ?/sec     1.00  1257.3±18.13µs        ? ?/sec
add_remove_big/sparse_set                      1.08  1132.4±263.10µs        ? ?/sec    1.00  1050.8±240.74µs        ? ?/sec
add_remove_big/table                           1.02      2.6±0.05ms        ? ?/sec     1.00      2.5±0.08ms        ? ?/sec
get_or_spawn/batched                           1.15   401.4±17.76µs        ? ?/sec     1.00   349.3±11.26µs        ? ?/sec
get_or_spawn/individual                        1.13   732.1±43.35µs        ? ?/sec     1.00   645.6±41.44µs        ? ?/sec
insert_commands/insert                         1.12   623.9±37.48µs        ? ?/sec     1.00   557.4±34.99µs        ? ?/sec
insert_commands/insert_batch                   1.16   401.4±17.00µs        ? ?/sec     1.00   347.4±12.87µs        ? ?/sec
insert_simple/base                             1.08    416.9±5.60µs        ? ?/sec     1.00    385.2±4.14µs        ? ?/sec
insert_simple/unbatched                        1.06   934.5±44.58µs        ? ?/sec     1.00   881.3±47.86µs        ? ?/sec
spawn_commands/2000_entities                   1.09   190.7±11.41µs        ? ?/sec     1.00    174.7±9.15µs        ? ?/sec
spawn_commands/4000_entities                   1.10   386.5±25.33µs        ? ?/sec     1.00   352.3±18.81µs        ? ?/sec
spawn_commands/6000_entities                   1.10   586.2±34.42µs        ? ?/sec     1.00   535.3±27.25µs        ? ?/sec
spawn_commands/8000_entities                   1.08   778.5±45.15µs        ? ?/sec     1.00   718.0±33.66µs        ? ?/sec
spawn_world/10000_entities                     1.04  1026.4±195.46µs        ? ?/sec    1.00  985.8±253.37µs        ? ?/sec
spawn_world/1000_entities                      1.06   103.8±20.23µs        ? ?/sec     1.00    97.6±18.22µs        ? ?/sec
spawn_world/100_entities                       1.15     11.4±4.25µs        ? ?/sec     1.00      9.9±1.87µs        ? ?/sec
spawn_world/10_entities                        1.05  1030.8±229.78ns        ? ?/sec    1.00  986.2±231.12ns        ? ?/sec
spawn_world/1_entities                         1.01   105.1±23.33ns        ? ?/sec     1.00   104.6±31.84ns        ? ?/sec
```

---

## Changelog
Changed: `Bundle::get_components` now takes a `FnMut(StorageType, OwningPtr)`. The provided storage type must be correct for the component being fetched.
2022-12-11 18:46:43 +00:00
Mike
75880a0b17 run clear trackers on render world (#6878)
# Objective

- Fixes https://github.com/bevyengine/bevy/issues/6417

## Solution

- clear_trackers was not being called on the render world. This causes the removed components vecs to continuously grow. This PR adds clear trackers to the end of RenderStage::Cleanup

## Migration Guide

The call to `clear_trackers` in `App` has been moved from the schedule to App::update for the main world and calls to `clear_trackers` have been added for sub_apps in the same function. This was due to needing stronger guarantees. If clear_trackers isn't called on a world it can lead to memory leaks in `RemovedComponents`.
2022-12-11 18:34:15 +00:00
JoJoJet
1af73624fa Simplify trait hierarchy for SystemParam (#6865)
# Objective

* Implementing a custom `SystemParam` by hand requires implementing three traits -- four if it is read-only.
* The trait `SystemParamFetch<'w, 's>` is a workaround from before we had generic associated types, and is no longer necessary.

## Solution

* Combine the trait `SystemParamFetch` with `SystemParamState`.
    * I decided to remove the `Fetch` name and keep the `State` name, since the former was consistently conflated with the latter.
* Replace the trait `ReadOnlySystemParamFetch` with `ReadOnlySystemParam`, which simplifies trait bounds in generic code.

---

## Changelog

- Removed the trait `SystemParamFetch`, moving its functionality to `SystemParamState`.
- Replaced the trait `ReadOnlySystemParamFetch` with `ReadOnlySystemParam`.

## Migration Guide

The trait `SystemParamFetch` has been removed, and its functionality has been transferred to `SystemParamState`.

```rust
// Before
impl SystemParamState for MyParamState {
    fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self { ... }
}
impl<'w, 's> SystemParamFetch<'w, 's> for MyParamState {
    type Item = MyParam<'w, 's>;
    fn get_param(...) -> Self::Item;
}

// After
impl SystemParamState for MyParamState {
    type Item<'w, 's> = MyParam<'w, 's>; // Generic associated types!
    fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self { ... }
    fn get_param<'w, 's>(...) -> Self::Item<'w, 's>;
}
```

The trait `ReadOnlySystemParamFetch` has been replaced with `ReadOnlySystemParam`.

```rust
// Before
unsafe impl ReadOnlySystemParamFetch for MyParamState {}

// After
unsafe impl<'w, 's> ReadOnlySystemParam for MyParam<'w, 's> {}
```
2022-12-11 18:34:14 +00:00
James Liu
c16791ce67 Document options for !Sync types for Component and Resources (#6864)
# Objective
It's not clear to users how to handle `!Sync` types as components and resources in the absence of engine level support for them. 

## Solution
Added a section to `Component`'s and `Resource`'s type level docs on available options for making a type `Sync` when it holds `!Sync` fields, linking `bevy_utils::synccell::SyncCell` and the currently unstable `std::sync::Exclusive`.

Also added a compile_fail doctest that illustrates how to apply `SyncCell`. These will break when/if #6572 gets merged, at which point these docs should be updated.
2022-12-11 18:34:13 +00:00
Edvin Kjell
aea4c5b1a4 [Fixes #6224] Add logging variants of system piping (#6751)
# Objective

Fixes #6224, add ``dbg``, ``info``, ``warn`` and ``error`` system piping adapter variants to expand #5776, which call the corresponding re-exported [bevy_log macros](https://docs.rs/bevy/latest/bevy/log/macro.info.html) when the result is an error.

## Solution

* Added ``dbg``, ``info``, ``warn`` and ``error`` system piping adapter variants to ``system_piping.rs``. 
* Modified and added tests for these under examples in ``system_piping.rs``.
2022-12-11 18:10:03 +00:00
Jerome Humbert
07e7fa5a4d Document World::clear_trackers() (#6520)
# Objective

Document `World::clear_trackers()`.

## Solution

Document the `World::clear_trackers()` method, and briefly how it's related to change detection and `RemovedComponents`.

This is a follow-up from [this discussion](https://discord.com/channels/691052431525675048/749335865876021248/1039628807025479700) on Discord.
2022-12-11 18:10:01 +00:00
James Liu
6308041772 Fix Sparse Change Detection (#6896)
# Objective
#6547 accidentally broke change detection for SparseSet components by using `Ticks::from_tick_cells` with the wrong argument order.

## Solution
Use the right argument order. Add a regression test.
2022-12-10 09:25:53 +00:00
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