Commit graph

426 commits

Author SHA1 Message Date
Joseph
8cc255c2f0
Hide UnsafeWorldCell::unsafe_world (#9741)
# Objective

We've done a lot of work to remove the pattern of a `&World` with
interior mutability (#6404, #8833). However, this pattern still persists
within `bevy_ecs` via the `unsafe_world` method.

## Solution

* Make `unsafe_world` private. Adjust any callsites to use
`UnsafeWorldCell` for interior mutability.
* Add `UnsafeWorldCell::removed_components`, since it is always safe to
access the removed components collection through `UnsafeWorldCell`.

## Future Work

Remove/hide `UnsafeWorldCell::world_metadata`, once we have provided
safe ways of accessing all world metadata.

---

## Changelog

+ Added `UnsafeWorldCell::removed_components`, which provides read-only
access to a world's collection of removed components.
2023-10-02 12:46:43 +00:00
SADIK KUZU
483f2464a8
Fix typos (#9965)
# Objective

- There were a few typos in the project.
- This PR fixes these typos.

## Solution

- Fixing the typos.

Signed-off-by: SADIK KUZU <sadikkuzu@hotmail.com>
2023-09-29 12:26:41 +00:00
Bruce Mitchener
a5a457c3c8
docs: Use intradoc links for method references. (#9958)
# Objective

- Use intradoc links to let the compiler verify correctness.

## Solution

- Use intradoc links.
2023-09-29 07:09:14 +00:00
Rob Parrett
7063c86ed4
Fix some typos (#9934)
# Objective

To celebrate the turning of the seasons, I took a small walk through the
codebase guided by the "[code spell
checker](https://marketplace.visualstudio.com/items?itemName=streetsidesoftware.code-spell-checker)"
VS Code extension and fixed a few typos.
2023-09-26 19:46:24 +00:00
Joseph
e60249e59d
Improve codegen for world validation (#9464)
# Objective

Improve code-gen for `QueryState::validate_world` and
`SystemState::validate_world`.

## Solution

* Move panics into separate, non-inlined functions, to reduce the code
size of the outer methods.
* Mark the panicking functions with `#[cold]` to help the compiler
optimize for the happy path.
* Mark the functions with `#[track_caller]` to make debugging easier.

---------

Co-authored-by: James Liu <contact@jamessliu.com>
2023-09-21 20:57:06 +00:00
Ethereumdegen
3ee9edf280
add try_insert to entity commands (#9844)
# Objective

- I spoke with some users in the ECS channel of bevy discord today and
they suggested that I implement a fallible form of .insert for
components.

- In my opinion, it would be nice to have a fallible .insert like
.try_insert (or to just make insert be fallible!) because it was causing
a lot of panics in my game. In my game, I am spawning terrain chunks and
despawning them in the Update loop. However, this was causing bevy_xpbd
to panic because it was trying to .insert some physics components on my
chunks and a race condition meant that its check to see if the entity
exists would pass but then the next execution step it would not exist
and would do an .insert and then panic. This means that there is no way
to avoid a panic with conditionals.

Luckily, bevy_xpbd does not care about inserting these components if the
entity is being deleted and so if there were a .try_insert, like this PR
provides it could use that instead in order to NOT panic.

( My interim solution for my own game has been to run the entity despawn
events in the Last schedule but really this is just a hack and I should
not be expected to manage the scheduling of despawns like this - it
should just be easy and simple. IF it just so happened that bevy_xpbd
ran .inserts in the Last schedule also, this would be an untenable soln
overall )

## Solution

- Describe the solution used to achieve the objective above.

Add a new command named TryInsert (entitycommands.try_insert) which
functions exactly like .insert except if the entity does not exist it
will not panic. Instead, it will log to info. This way, crates that are
attaching components in ways which they do not mind that the entity no
longer exists can just use try_insert instead of insert.

---

## Changelog

 

## Additional Thoughts

In my opinion, NOT panicing should really be the default and having an
.insert that does panic should be the odd edgecase but removing the
panic! from .insert seems a bit above my paygrade -- although i would
love to see it. My other thought is it would be good for .insert to
return an Option AND not panic but it seems it uses an event bus right
now so that seems to be impossible w the current architecture.
2023-09-20 19:34:30 +00:00
Trashtalk217
e4b368721d
One Shot Systems (#8963)
I'm adopting this ~~child~~ PR.

# Objective

- Working with exclusive world access is not always easy: in many cases,
a standard system or three is more ergonomic to write, and more
modularly maintainable.
- For small, one-off tasks (commonly handled with scripting), running an
event-reader system incurs a small but flat overhead cost and muddies
the schedule.
- Certain forms of logic (e.g. turn-based games) want very fine-grained
linear and/or branching control over logic.
- SystemState is not automatically cached, and so performance can suffer
and change detection breaks.
- Fixes https://github.com/bevyengine/bevy/issues/2192.
- Partial workaround for https://github.com/bevyengine/bevy/issues/279.

## Solution

- Adds a SystemRegistry resource to the World, which stores initialized
systems keyed by their SystemSet.
- Allows users to call world.run_system(my_system) and
commands.run_system(my_system), without re-initializing or losing state
(essential for change detection).
- Add a Callback type to enable convenient use of dynamic one shot
systems and reduce the mental overhead of working with Box<dyn
SystemSet>.
- Allow users to run systems based on their SystemSet, enabling more
complex user-made abstractions.

## Future work

- Parameterized one-shot systems would improve reusability and bring
them closer to events and commands. The API could be something like
run_system_with_input(my_system, my_input) and use the In SystemParam.
- We should evaluate the unification of commands and one-shot systems
since they are two different ways to run logic on demand over a World.

### Prior attempts

- https://github.com/bevyengine/bevy/pull/2234
- https://github.com/bevyengine/bevy/pull/2417
- https://github.com/bevyengine/bevy/pull/4090
- https://github.com/bevyengine/bevy/pull/7999

This PR continues the work done in
https://github.com/bevyengine/bevy/pull/7999.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Federico Rinaldi <gisquerin@gmail.com>
Co-authored-by: MinerSebas <66798382+MinerSebas@users.noreply.github.com>
Co-authored-by: Aevyrie <aevyrie@gmail.com>
Co-authored-by: Alejandro Pascual Pozo <alejandro.pascual.pozo@gmail.com>
Co-authored-by: Rob Parrett <robparrett@gmail.com>
Co-authored-by: François <mockersf@gmail.com>
Co-authored-by: Dmytro Banin <banind@cs.washington.edu>
Co-authored-by: James Liu <contact@jamessliu.com>
2023-09-19 20:17:05 +00:00
louis-le-cam
9ee9d627d7
Rename RemovedComponents::iter/iter_with_id to read/read_with_id (#9778)
# Objective

Rename RemovedComponents::iter/iter_with_id to read/read_with_id to make
it clear that it consume the data

Fixes #9755.

(It's my first pull request, if i've made any mistake, please let me
know)

## Solution

Refactor RemovedComponents::iter/iter_with_id to read/read_with_id



## Changelog

Refactor RemovedComponents::iter/iter_with_id to read/read_with_id

Deprecate RemovedComponents::iter/iter_with_id

Remove IntoIterator implementation

Update removal_detection example accordingly

---

## Migration Guide

Rename calls of RemovedComponents::iter/iter_with_id to
read/read_with_id

Replace IntoIterator iteration (&mut <RemovedComponents>) with .read()

---------

Co-authored-by: denshi_ika <mojang2824@gmail.com>
2023-09-15 12:37:20 +00:00
Mike
324c057b71
Cache System Tracing Spans (#9390)
# Objective

- Reduce the overhead of tracing by caching the system spans.

Yellow is this pr. Red is main.


![image](https://github.com/bevyengine/bevy/assets/2180432/fe9bb7c2-ae9a-4522-80a9-75a943a562b6)
2023-09-13 19:10:11 +00:00
Zachary Harrold
4c6b6fc24a
Moved get_component(_unchecked_mut) from Query to QueryState (#9686)
# Objective

- Fixes #9683

## Solution

- Moved `get_component` from `Query` to `QueryState`.
- Moved `get_component_unchecked_mut` from `Query` to `QueryState`.
- Moved `QueryComponentError` from `bevy_ecs::system` to
`bevy_ecs::query`. Minor Breaking Change.
- Narrowed scope of `unsafe` blocks in `Query` methods.

---

## Migration Guide

- `use bevy_ecs::system::QueryComponentError;` -> `use
bevy_ecs::query::QueryComponentError;`

## Notes

I am not very familiar with unsafe Rust nor its use within Bevy, so I
may have committed a Rust faux pas during the migration.

---------

Co-authored-by: Zac Harrold <zharrold@c5prosolutions.com>
Co-authored-by: Tristan Guichaoua <33934311+tguichaoua@users.noreply.github.com>
2023-09-11 19:04:22 +00:00
Edgar Geier
118509e4aa
Replace IntoSystemSetConfig with IntoSystemSetConfigs (#9247)
# Objective

- Fixes #9244.

## Solution


- Changed the `(Into)SystemSetConfigs` traits and structs be more like
the `(Into)SystemConfigs` traits and structs.
- Replaced uses of `IntoSystemSetConfig` with `IntoSystemSetConfigs`
- Added generic `ItemConfig` and `ItemConfigs` types.
- Changed `SystemConfig(s)` and `SystemSetConfig(s)` to be type aliases
to `ItemConfig(s)`.
- Added generic `process_configs` to `ScheduleGraph`.
- Changed `configure_sets_inner` and `add_systems_inner` to reuse
`process_configs`.

---

## Changelog

- Added `run_if` to `IntoSystemSetConfigs`
- Deprecated `Schedule::configure_set` and `App::configure_set`
- Removed `IntoSystemSetConfig`

## Migration Guide

- Use `App::configure_sets` instead of `App::configure_set`
- Use `Schedule::configure_sets` instead of `Schedule::configure_set`

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
2023-09-05 17:15:27 +00:00
Hennadii Chernyshchyk
9309d89bb8
Add panicking helpers for getting components from Query (#9659)
# Objective

- Currently we don't have panicking alternative for getting components
from `Query` like for resources. Partially addresses #9443.

## Solution

- Add these functions.

---

## Changelog

### Added

- `Query::component` and `Query::component_mut` to get specific
component from query and panic on error.
2023-09-04 12:31:12 +00:00
Joseph
58f7dac689
Fix unsoundness in QueryState::is_empty (#9463)
# Objective

`QueryState::is_empty` is unsound, as it does not validate the world. If
a mismatched world is passed in, then the query filter may cast a
component to an incorrect type, causing undefined behavior.

## Solution

Add world validation. To prevent a performance regression in `Query`
(whose world does not need to be validated), the unchecked function
`is_empty_unsafe_world_cell` has been added. This also allows us to
remove one of the last usages of the private function
`UnsafeWorldCell::unsafe_world`, which takes us a step towards being
able to remove that method entirely.
2023-09-02 23:43:22 +00:00
Joseph
bc8bf34818
Allow disjoint mutable world access via EntityMut (#9419)
# Objective

Fix #4278
Fix #5504
Fix #9422

Provide safe ways to borrow an entire entity, while allowing disjoint
mutable access. `EntityRef` and `EntityMut` are not suitable for this,
since they provide access to the entire world -- they are just helper
types for working with `&World`/`&mut World`.

This has potential uses for reflection and serialization

## Solution

Remove `EntityRef::world`, which allows it to soundly be used within
queries.

`EntityMut` no longer supports structural world mutations, which allows
multiple instances of it to exist for different entities at once.
Structural world mutations are performed using the new type
`EntityWorldMut`.

```rust
fn disjoint_system(
     q2: Query<&mut A>,
     q1: Query<EntityMut, Without<A>>,
) { ... }

let [entity1, entity2] = world.many_entities_mut([id1, id2]);
*entity1.get_mut::<T>().unwrap() = *entity2.get().unwrap();

for entity in world.iter_entities_mut() {
    ...
}
```

---

## Changelog

- Removed `EntityRef::world`, to fix a soundness issue with queries.
+ Removed the ability to structurally mutate the world using
`EntityMut`, which allows it to be used in queries.
+ Added `EntityWorldMut`, which is used to perform structural mutations
that are no longer allowed using `EntityMut`.

## Migration Guide

**Note for maintainers: ensure that the guide for #9604 is updated
accordingly.**

Removed the method `EntityRef::world`, to fix a soundness issue with
queries. If you need access to `&World` while using an `EntityRef`,
consider passing the world as a separate parameter.

`EntityMut` can no longer perform 'structural' world mutations, such as
adding or removing components, or despawning the entity. Additionally,
`EntityMut::world`, `EntityMut::world_mut` , and
`EntityMut::world_scope` have been removed.
Instead, use the newly-added type `EntityWorldMut`, which is a helper
type for working with `&mut World`.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
2023-08-28 23:30:59 +00:00
Mike
33fdc5f5db
Move schedule name into Schedule (#9600)
# Objective

- Move schedule name into `Schedule` to allow the schedule name to be
used for errors and tracing in Schedule methods
- Fixes #9510

## Solution

- Move label onto `Schedule` and adjust api's on `World` and `Schedule`
to not pass explicit label where it makes sense to.
- add name to errors and tracing.
- `Schedule::new` now takes a label so either add the label or use
`Schedule::default` which uses a default label. `default` is mostly used
in doc examples and tests.

---

## Changelog

- move label onto `Schedule` to improve error message and logging for
schedules.

## Migration Guide

`Schedule::new` and `App::add_schedule`
```rust
// old
let schedule = Schedule::new();
app.add_schedule(MyLabel, schedule);

// new
let schedule = Schedule::new(MyLabel);
app.add_schedule(schedule);
```

if you aren't using a label and are using the schedule struct directly
you can use the default constructor.
```rust
// old
let schedule = Schedule::new();
schedule.run(world);

// new
let schedule = Schedule::default();
schedule.run(world);
```

`Schedules:insert`
```rust
// old
let schedule = Schedule::new();
schedules.insert(MyLabel, schedule);

// new
let schedule = Schedule::new(MyLabel);
schedules.insert(schedule);
```

`World::add_schedule`
```rust
// old
let schedule = Schedule::new();
world.add_schedule(MyLabel, schedule);

// new
let schedule = Schedule::new(MyLabel);
world.add_schedule(schedule);
```
2023-08-28 20:44:48 +00:00
Noah
72fc63e594
implement insert and remove reflected entity commands (#8895)
# Objective

To enable non exclusive system usage of reflected components and make
reflection more ergonomic to use by making it more in line with standard
entity commands.

## Solution

- Implements a new `EntityCommands` extension trait for reflection
related functions in the reflect module of bevy_ecs.
- Implements 4 new commands, `insert_reflect`,
`insert_reflect_with_registry`, `remove_reflect`, and
`remove_reflect_with_registry`. Both insert commands take a `Box<dyn
Reflect>` component while the remove commands take the component type
name.

- Made `EntityCommands` fields pub(crate) to allow access in the reflect
module. (Might be worth making these just public to enable user end
custom entity commands in a different pr)
- Added basic tests to ensure the commands are actually working.
- Documentation of functions.

---

## Changelog

Added:
-  Implements 4 new commands on the new entity commands extension.
- `insert_reflect`
- `remove_reflect`
- `insert_reflect_with_registry`
- `remove_reflect_with_registry`

The commands operate the same except the with_registry commands take a
generic parameter for a resource that implements `AsRef<TypeRegistry>`.
Otherwise the default commands use the `AppTypeRegistry` for reflection
data.

Changed:

- Made `EntityCommands` fields pub(crate) to allow access in the reflect
module.

> Hopefully this time it works. Please don't make me rebase again ☹
2023-08-28 18:21:20 +00:00
Zachary Harrold
1839ff7e2a
Replaced EntityCommand Implementation for FnOnce (#9604)
# Objective

- Fixes #4917
- Replaces #9602

## Solution

- Replaced `EntityCommand` implementation for `FnOnce` to apply to
`FnOnce(EntityMut)` instead of `FnOnce(Entity, &mut World)`

---

## Changelog

- `FnOnce(Entity, &mut World)` no longer implements `EntityCommand`.
This is a breaking change.

## Migration Guide

### 1. New-Type `FnOnce`

Create an `EntityCommand` type which implements the method you
previously wrote:

```rust
pub struct ClassicEntityCommand<F>(pub F);

impl<F> EntityCommand for ClassicEntityCommand<F>
where
    F: FnOnce(Entity, &mut World) + Send + 'static,
{
    fn apply(self, id: Entity, world: &mut World) {
        (self.0)(id, world);
    }
}

commands.add(ClassicEntityCommand(|id: Entity, world: &mut World| {
    /* ... */
}));
```

### 2. Extract `(Entity, &mut World)` from `EntityMut`

The method `into_world_mut` can be used to gain access to the `World`
from an `EntityMut`.

```rust
let old = |id: Entity, world: &mut World| {
    /* ... */
};

let new = |mut entity: EntityMut| {
    let id = entity.id();
    let world = entity.into_world_mut();
    /* ... */
};
```
2023-08-28 18:08:53 +00:00
Joseph
da8ab16d83
Relax more Sync bounds on Local (#9589)
# Objective

#5483 allows for the creation of non-`Sync` locals. However, it's not
actually possible to use these types as there is a `Sync` bound on the
`Deref` impls.

## Solution

Remove the unnecessary bounds.
2023-08-28 17:47:15 +00:00
Joseph
474b55a29c
Add system.map(...) for transforming the output of a system (#8526)
# Objective

Any time we wish to transform the output of a system, we currently use
system piping to do so:

```rust
my_system.pipe(|In(x)| do_something(x))
```

Unfortunately, system piping is not a zero cost abstraction. Each call
to `.pipe` requires allocating two extra access sets: one for the second
system and one for the combined accesses of both systems. This also adds
extra work to each call to `update_archetype_component_access`, which
stacks as one adds multiple layers of system piping.

## Solution

Add the `AdapterSystem` abstraction: similar to `CombinatorSystem`, this
allows you to implement a trait to generically control how a system is
run and how its inputs and outputs are processed. Unlike
`CombinatorSystem`, this does not have any overhead when computing world
accesses which makes it ideal for simple operations such as inverting or
ignoring the output of a system.

Add the extension method `.map(...)`: this is similar to `.pipe(...)`,
only it accepts a closure as an argument instead of an `In<T>` system.

```rust
my_system.map(do_something)
```

This has the added benefit of making system names less messy: a system
that ignores its output will just be called `my_system`, instead of
`Pipe(my_system, ignore)`

---

## Changelog

TODO

## Migration Guide

The `system_adapter` functions have been deprecated: use `.map` instead,
which is a lightweight alternative to `.pipe`.

```rust
// Before:
my_system.pipe(system_adapter::ignore)
my_system.pipe(system_adapter::unwrap)
my_system.pipe(system_adapter::new(T::from))

// After:
my_system.map(std::mem::drop)
my_system.map(Result::unwrap)
my_system.map(T::from)

// Before:
my_system.pipe(system_adapter::info)
my_system.pipe(system_adapter::dbg)
my_system.pipe(system_adapter::warn)
my_system.pipe(system_adapter::error)

// After:
my_system.map(bevy_utils::info)
my_system.map(bevy_utils::dbg)
my_system.map(bevy_utils::warn)
my_system.map(bevy_utils::error)
```

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
2023-08-28 16:36:46 +00:00
Joseph
e8b3892517
Improve various Debug implementations (#9588)
# Objective

* `Local` and `SystemName` implement `Debug` manually, but they could
derive it.
* `QueryState` and `dyn System` have unconventional debug formatting.
2023-08-26 21:27:41 +00:00
Christian Hughes
349dd8bd45
Relax In/Out bounds on impl Debug for dyn System (#9581)
# Objective

Resolves #9580

## Solution

Relaxed the bounds on the `Debug` impl for `dyn System`
2023-08-26 13:57:19 +00:00
Joseph
69750a03ca
Implement Debug for UnsafeWorldCell (#9460)
# Objective

* Add more ways to use `UnsafeWorldCell` in safe code -- you no longer
need to call `.world_metadata()` to format it.
2023-08-20 22:00:12 +00:00
Zeenobit
1e170d2e90
Add RunSystem (#9366)
Add a `RunSystem` extension trait to allow for immediate execution of
systems on a `World` for debugging and/or testing purposes.

# Objective

Fixes #6184

Initially, I made this CL as `ApplyCommands`. After a discussion with
@cart , we decided a more generic implementation would be better to
support all systems. This is the new revised CL. Sorry for the long
delay! 😅

This CL allows users to do this:
```rust
use bevy::prelude::*;
use bevy::ecs::system::RunSystem;

struct T(usize);

impl Resource for T {}

fn system(In(n): In<usize>, mut commands: Commands) -> usize {
    commands.insert_resource(T(n));
    n + 1
}

let mut world = World::default();
let n = world.run_system_with(1, system);
assert_eq!(n, 2);
assert_eq!(world.resource::<T>().0, 1);
```

## Solution

This is implemented as a trait extension and not included in any
preludes to ensure it's being used consciously.
Internally, it just initializes and runs a systems, and applies any
deferred parameters all "in place".
The trait has 2 functions (one of which calls the other by default):
- `run_system_with` is the general implementation, which allows user to
pass system input parameters
- `run_system` is the ergonomic wrapper for systems with no input
parameter (to avoid having the user pass `()` as input).

~~Additionally, this trait is also implemented for `&mut App`. I added
this mainly for ergonomics (`app.run_system` vs.
`app.world.run_system`).~~ (Removed based on feedback)

---------

Co-authored-by: Pascal Hertleif <killercup@gmail.com>
2023-08-11 20:41:48 +00:00
Nicola Papale
77fa8a9a9c
Add a paragraph to the lifetimeless module doc (#9312)
# Objective

The `lifetimeless` module has been a source of confusion for bevy users
for a while now.

## Solution

Add a couple paragraph explaining that, yes, you can use one of the type
alias safely, without ever leaking any memory.
2023-08-02 22:01:56 +00:00
Ryan Devenney
264195ed77
replace parens with square brackets when referencing _mut on Query docs #9200 (#9223)
# Objective

Fixes #9200 
Switches ()'s to []'s when talking about the optional `_mut` suffix in
the ECS Query Struct page to have more idiomatic docs.

## Solution

Replace `()` with `[]` in appropriate doc pages.
2023-07-20 21:41:07 +00:00
ClayenKitten
ffc572728f
Fix typos throughout the project (#9090)
# Objective

Fix typos throughout the project.

## Solution

[`typos`](https://github.com/crate-ci/typos) project was used for
scanning, but no automatic corrections were applied. I checked
everything by hand before fixing.

Most of the changes are documentation/comments corrections. Also, there
are few trivial changes to code (variable name, pub(crate) function name
and a few error/panic messages).

## Unsolved

`bevy_reflect_derive` has
[typo](1b51053f19/crates/bevy_reflect/bevy_reflect_derive/src/type_path.rs (L76))
in enum variant name that I didn't fix. Enum is `pub(crate)`, so there
shouldn't be any trouble if fixed. However, code is tightly coupled with
macro usage, so I decided to leave it for more experienced contributor
just in case.
2023-07-10 00:11:51 +00:00
0xc0001a2040
15be0d1a61
Add/fix track_caller attribute on panicking entity accessor methods (#8951)
# Objective

`World::entity`, `World::entity_mut` and `Commands::entity` should be
marked with `track_caller` to display where (in user code) the call with
the invalid `Entity` was made. `Commands::entity` already has the
attibute, but it does nothing due to the call to `unwrap_or_else`.

## Solution

- Apply the `track_caller` attribute to the `World::entity_mut` and
`World::entity`.
- Remove the call to `unwrap_or_else` which makes the `track_caller`
attribute useless (because `unwrap_or_else` is not `track_caller`
itself). The avoid eager evaluation of the panicking branch it is never
inlined.

---------

Co-authored-by: Giacomo Stevanato <giaco.stevanato@gmail.com>
2023-06-26 18:35:11 +00:00
James Liu
70f91b2b9e
Implement WorldQuery for EntityRef (#6960)
# Objective
Partially address #5504. Fix #4278. Provide "whole entity" access in
queries. This can be useful when you don't know at compile time what
you're accessing (i.e. reflection via `ReflectComponent`).

## Solution
Implement `WorldQuery` for `EntityRef`. 

- This provides read-only access to the entire entity, and supports
anything that `EntityRef` can normally do.
- It matches all archetypes and tables and will densely iterate when
possible.
- It marks all of the ArchetypeComponentIds of a matched archetype as
read.
- Adding it to a query will cause it to panic if used in conjunction
with any other mutable access.
 - Expanded the docs on Query to advertise this feature.
 - Added tests to ensure the panics were working as intended.
 - Added `EntityRef` to the ECS prelude.

To make this safe, `EntityRef::world` was removed as it gave potential
`UnsafeCell`-like access to other parts of the `World` including aliased
mutable access to the components it would otherwise read safely.

## Performance
Not great beyond the additional parallelization opportunity over
exclusive systems. The `EntityRef` is fetched from `Entities` like any
other call to `World::entity`, which can be very random access heavy.
This could be simplified if `ArchetypeRow` is available in
`WorldQuery::fetch`'s arguments, but that's likely not something we
should optimize for.

## Future work
An equivalent API where it gives mutable access to all components on a
entity can be done with a scoped version of `EntityMut` where it does
not provide `&mut World` access nor allow for structural changes to the
entity is feasible as well. This could be done as a safe alternative to
exclusive system when structural mutation isn't required or the target
set of entities is scoped.

---

## Changelog
Added: `Access::has_any_write`
Added: `EntityRef` now implements `WorldQuery`. Allows read-only access
to the entire entity, incompatible with any other mutable access, can be
mixed with `With`/`Without` filters for more targeted use.
Added: `EntityRef` to `bevy::ecs::prelude`.
Removed: `EntityRef::world`

## Migration Guide
TODO

---------

Co-authored-by: Carter Weinberg <weinbergcarter@gmail.com>
Co-authored-by: Jakob Hellermann <jakob.hellermann@protonmail.com>
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2023-06-22 21:20:00 +00:00
Thierry Berger
17e1d211c5
doc: update a reference from add_system to add_systems (#8881)
Small fix for a forgotten documentation comment.
2023-06-18 17:17:02 +00:00
JoJoJet
8ec81496ff
Add a method to run read-only systems using &World (#8849)
# Objective

Resolves #7558.

Systems that are known to never modify the world implement the trait
`ReadOnlySystem`. This is a perfect place to add a safe API for running
a system with a shared reference to a World.

---

## Changelog

- Added the trait method `ReadOnlySystem::run_readonly`, which allows a
system to be run using `&World`.
2023-06-15 22:54:53 +00:00
JoJoJet
db8d3651e0
Migrate the rest of the engine to UnsafeWorldCell (#8833)
# Objective

Follow-up to #6404 and #8292.

Mutating the world through a shared reference is surprising, and it
makes the meaning of `&World` unclear: sometimes it gives read-only
access to the entire world, and sometimes it gives interior mutable
access to only part of it.

This is an up-to-date version of #6972.

## Solution

Use `UnsafeWorldCell` for all interior mutability. Now, `&World`
*always* gives you read-only access to the entire world.

---

## Changelog

TODO - do we still care about changelogs?

## Migration Guide

Mutating any world data using `&World` is now considered unsound -- the
type `UnsafeWorldCell` must be used to achieve interior mutability. The
following methods now accept `UnsafeWorldCell` instead of `&World`:

- `QueryState`: `get_unchecked`, `iter_unchecked`,
`iter_combinations_unchecked`, `for_each_unchecked`,
`get_single_unchecked`, `get_single_unchecked_manual`.
- `SystemState`: `get_unchecked_manual`

```rust
let mut world = World::new();
let mut query = world.query::<&mut T>();

// Before:
let t1 = query.get_unchecked(&world, entity_1);
let t2 = query.get_unchecked(&world, entity_2);

// After:
let world_cell = world.as_unsafe_world_cell();
let t1 = query.get_unchecked(world_cell, entity_1);
let t2 = query.get_unchecked(world_cell, entity_2);
```

The methods `QueryState::validate_world` and
`SystemState::matches_world` now take a `WorldId` instead of `&World`:

```rust
// Before:
query_state.validate_world(&world);

// After:
query_state.validate_world(world.id());
```

The methods `QueryState::update_archetypes` and
`SystemState::update_archetypes` now take `UnsafeWorldCell` instead of
`&World`:

```rust
// Before:
query_state.update_archetypes(&world);

// After:
query_state.update_archetypes(world.as_unsafe_world_cell_readonly());
```
2023-06-15 01:31:56 +00:00
Jonathan
c475e271be
Implement Clone for CombinatorSystem (#8826)
# Objective

Make a combined system cloneable if both systems are cloneable on their
own. This is necessary for using chained conditions (e.g
`cond1.and_then(cond2)`) with `distributive_run_if()`.

## Solution

Implement `Clone` for `CombinatorSystem<Func, A, B>` where `A, B:
Clone`.
2023-06-12 19:44:51 +00:00
Natanael Mojica
f135535cd6
Rename Command's "write" method to "apply" (#8814)
# Objective

- Fixes #8811 .

## Solution

- Rename "write" method to "apply" in Command trait definition.
- Rename other implementations of command trait throughout bevy's code
base.

---

## Changelog

- Changed: `Command::write` has been changed to `Command::apply`
- Changed: `EntityCommand::write` has been changed to
`EntityCommand::apply`

## Migration Guide

- `Command::write` implementations need to be changed to implement
`Command::apply` instead. This is a mere name change, with no further
actions needed.
- `EntityCommand::write` implementations need to be changed to implement
`EntityCommand::apply` instead. This is a mere name change, with no
further actions needed.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
2023-06-12 17:53:47 +00:00
JoJoJet
32faf4cb5c
Document every public item in bevy_ecs (#8731)
# Objective

Title.

---------

Co-authored-by: François <mockersf@gmail.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: James Liu <contact@jamessliu.com>
2023-06-10 23:23:48 +00:00
CatThingy
89cbc78d3d
Require #[derive(Event)] on all Events (#7086)
# Objective

Be consistent with `Resource`s and `Components` and have `Event` types
be more self-documenting.
Although not susceptible to accidentally using a function instead of a
value due to `Event`s only being initialized by their type, much of the
same reasoning for removing the blanket impl on `Resource` also applies
here.

* Not immediately obvious if a type is intended to be an event
* Prevent invisible conflicts if the same third-party or primitive types
are used as events
* Allows for further extensions (e.g. opt-in warning for missed events)

## Solution

Remove the blanket impl for the `Event` trait. Add a derive macro for
it.

---

## Changelog

- `Event` is no longer implemented for all applicable types. Add the
`#[derive(Event)]` macro for events.

## Migration Guide

* Add the `#[derive(Event)]` macro for events. Third-party types used as
events should be wrapped in a newtype.
2023-06-06 14:44:32 +00:00
Alice Cecile
cbd4abf0fc
Rename apply_system_buffers to apply_deferred (#8726)
# Objective

- `apply_system_buffers` is an unhelpful name: it introduces a new
internal-only concept
- this is particularly rough for beginners as reasoning about how
commands work is a critical stumbling block

## Solution

- rename `apply_system_buffers` to the more descriptive `apply_deferred`
- rename related fields, arguments and methods in the internals fo
bevy_ecs for consistency
- update the docs


## Changelog

`apply_system_buffers` has been renamed to `apply_deferred`, to more
clearly communicate its intent and relation to `Deferred` system
parameters like `Commands`.

## Migration Guide

- `apply_system_buffers` has been renamed to `apply_deferred`
- the `apply_system_buffers` method on the `System` trait has been
renamed to `apply_deferred`
- the `is_apply_system_buffers` function has been replaced by
`is_apply_deferred`
- `Executor::set_apply_final_buffers` is now
`Executor::set_apply_final_deferred`
- `Schedule::apply_system_buffers` is now `Schedule::apply_deferred`

---------

Co-authored-by: JoJoJet <21144246+JoJoJet@users.noreply.github.com>
2023-06-02 14:04:13 +00:00
JoJoJet
5472ea4a14
Improve encapsulation for commands and add docs (#8725)
# Objective

Several of our built-in `Command` types are too public:
- `GetOrSpawn` is public, even though it only makes sense to call it
from within `Commands::get_or_spawn`.
- `Remove` and `RemoveResource` contain public `PhantomData` marker
fields.

## Solution

Remove `GetOrSpawn` and use an anonymous command. Make the marker fields
private.

---

## Migration Guide

The `Command` types `Remove` and `RemoveResource` may no longer be
constructed manually.

```rust
// Before:
commands.add(Remove::<T> {
    entity: id,
    phantom: PhantomData,
});

// After:
commands.add(Remove::<T>::new(id));

// Before:
commands.add(RemoveResource::<T> { phantom: PhantomData });

// After:
commands.add(RemoveResource::<T>::new());
```

The command type `GetOrSpawn` has been removed. It was not possible to
use this type outside of `bevy_ecs`.
2023-05-31 16:45:46 +00:00
Sélène Amanita
ca81d3e435
Document query errors (#8692)
# Objective

Add documentation to `Query` and `QueryState` errors in bevy_ecs
(`QuerySingleError`, `QueryEntityError`, `QueryComponentError`)

## Solution

- Change display message for `QueryEntityError::QueryDoesNotMatch`: this
error can also happen when the entity has a component which is filtered
out (with `Without<C>`)
- Fix wrong reference in the documentation of `Query::get_component` and
`Query::get_component_mut` from `QueryEntityError` to
`QueryComponentError`
- Complete the documentation of the three error enum variants.
- Add examples for `QueryComponentError::MissingReadAccess` and
`QueryComponentError::MissingWriteAccess`
- Add reference to `QueryState` in `QueryEntityError`'s documentation.

---

## Migration Guide

Expect `QueryEntityError::QueryDoesNotMatch`'s display message to
change? Not sure that counts.

---------

Co-authored-by: harudagondi <giogdeasis@gmail.com>
2023-05-30 14:41:14 +00:00
JoJoJet
85a918a8dd
Improve safety for the multi-threaded executor using UnsafeWorldCell (#8292)
# Objective

Fix #7833.

Safety comments in the multi-threaded executor don't really talk about
system world accesses, which makes it unclear if the code is actually
valid.

## Solution

Update the `System` trait to use `UnsafeWorldCell`. This type's API is
written in a way that makes it much easier to cleanly maintain safety
invariants. Use this type throughout the multi-threaded executor, with a
liberal use of safety comments.

---

## Migration Guide

The `System` trait now uses `UnsafeWorldCell` instead of `&World`. This
type provides a robust API for interior mutable world access.
- The method `run_unsafe` uses this type to manage world mutations
across multiple threads.
- The method `update_archetype_component_access` uses this type to
ensure that only world metadata can be used.

```rust
let mut system = IntoSystem::into_system(my_system);
system.initialize(&mut world);

// Before:
system.update_archetype_component_access(&world);
unsafe { system.run_unsafe(&world) }

// After:
system.update_archetype_component_access(world.as_unsafe_world_cell_readonly());
unsafe { system.run_unsafe(world.as_unsafe_world_cell()) }
```

---------

Co-authored-by: James Liu <contact@jamessliu.com>
2023-05-29 15:22:10 +00:00
Troels Jessen
55d6c7d56e
Added reference to ParallelCommands in the Commands doc (#8510)
# Objective

Fixes #8414

---------

Co-authored-by: Sheepyhead <trojes@tuta.io>
Co-authored-by: Nicola Papale <nicopap@users.noreply.github.com>
2023-05-01 15:31:33 +00:00
Wybe Westra
abf12f3b3b
Fixed several missing links in docs. (#8117)
Links in the api docs are nice. I noticed that there were several places
where structs / functions and other things were referenced in the docs,
but weren't linked. I added the links where possible / logical.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: François <mockersf@gmail.com>
2023-04-23 17:28:36 +00:00
JoJoJet
ce252f8cf7
Reorganize system modules (#8419)
# Objective

Follow-up to #8377.

As the system module has been refactored, there are many types that no
longer make sense to live in the files that they do:
- The `IntoSystem` trait is in `function_system.rs`, even though this
trait is relevant to all kinds of systems. Same for the `In<T>` type.
- `PipeSystem` is now just an implementation of `CombinatorSystem`, so
`system_piping.rs` no longer needs its own file.

## Solution

- Move `IntoSystem`, `In<T>`, and system piping combinators & tests into
the top-level `mod.rs` file for `bevy_ecs::system`.
- Move `PipeSystem` into `combinator.rs`.
2023-04-17 21:10:57 +00:00
JoJoJet
b03b7b557e
Simplify system piping and make it more flexible (#8377)
# Objective

- Currently, it is not possible to call `.pipe` on a system that takes
any input other than `()`.
- The `IntoPipeSystem` trait is currently very difficult to parse due to
its use of generics.

## Solution

Remove the `IntoPipeSystem` trait, and move the `pipe` method to
`IntoSystem`.

---

## Changelog

- System piping has been made more flexible: it is now possible to call
`.pipe` on a system that takes an input.

## Migration Guide

The `IntoPipeSystem` trait has been removed, and the `pipe` method has
been moved to the `IntoSystem` trait.

```rust

// Before:
use bevy_ecs::system::IntoPipeSystem;
schedule.add_systems(first.pipe(second));

// After:
use bevy_ecs::system::IntoSystem;
schedule.add_systems(first.pipe(second));
```
2023-04-17 16:08:32 +00:00
JoJoJet
328347f44c
Add a missing safety invariant to System::run_unsafe (#7778)
# Objective

The implementation of `System::run_unsafe` for `FunctionSystem` requires
that the world is the same one used to initialize the system. However,
the `System` trait has no requirements that the world actually matches,
which makes this implementation unsound.

This was previously mentioned in
https://github.com/bevyengine/bevy/pull/7605#issuecomment-1426491871

Fixes part of #7833.

## Solution

Add the safety invariant that
`System::update_archetype_component_access` must be called prior to
`System::run_unsafe`. Since
`FunctionSystem::update_archetype_component_access` properly validates
the world, this ensures that `run_unsafe` is not called with a
mismatched world.

Most exclusive systems are not required to be run on the same world that
they are initialized with, so this is not a concern for them. Systems
formed by combining an exclusive system with a regular system *do*
require the world to match, however the validation is done inside of
`System::run` when needed.
2023-04-17 15:20:42 +00:00
Vladyslav Batyrenko
71fccb2897
Improve or-with disjoint checks (#7085)
# Objective

This PR attempts to improve query compatibility checks in scenarios
involving `Or` filters.

Currently, for the following two disjoint queries, Bevy will throw a
panic:

```
fn sys(_: Query<&mut C, Or<(With<A>, With<B>)>>, _: Query<&mut C, (Without<A>, Without<B>)>) {}
``` 

This PR addresses this particular scenario.

## Solution

`FilteredAccess::with` now stores a vector of `AccessFilters`
(representing a pair of `with` and `without` bitsets), where each member
represents an `Or` "variant".
Filters like `(With<A>, Or<(With<B>, Without<C>)>` are expected to be
expanded into `A * B + A * !C`.

When calculating whether queries are compatible, every `AccessFilters`
of a query is tested for incompatibility with every `AccessFilters` of
another query.

---

## Changelog

- Improved system and query data access compatibility checks in
scenarios involving `Or` filters

---------

Co-authored-by: MinerSebas <66798382+MinerSebas@users.noreply.github.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
2023-04-17 15:16:58 +00:00
JoJoJet
c37a53d52d
Add a regression test for change detection between piped systems (#8368)
# Objective

The behavior of change detection within `PipeSystem` is very tricky and
subtle, and is not currently covered by any of our tests as far as I'm
aware.
2023-04-13 17:59:29 +00:00
Mikkel Rasmussen
e9312254d8
Non-breaking change* from UK spellings to US (#8291)
Fixes issue mentioned in PR #8285.

_Note: By mistake, this is currently dependent on #8285_
# Objective

Ensure consistency in the spelling of the documentation.

Exceptions:
`crates/bevy_mikktspace/src/generated.rs` - Has not been changed from
licence to license as it is part of a licensing agreement.

Maybe for further consistency,
https://github.com/bevyengine/bevy-website should also be given a look.

## Solution

### Changed the spelling of the current words (UK/CN/AU -> US) :
cancelled -> canceled (Breaking API changes in #8285)
behaviour -> behavior (Breaking API changes in #8285)
neighbour -> neighbor
grey -> gray
recognise -> recognize
centre -> center
metres -> meters
colour -> color

### ~~Update [`engine_style_guide.md`]~~ Moved to #8324 

---

## Changelog

Changed UK spellings in documentation to US

## Migration Guide

Non-breaking changes*

\* If merged after #8285
2023-04-08 16:22:46 +00:00
JoJoJet
3442a13d2c
Use UnsafeWorldCell to increase code quality for SystemParam (#8174)
# Objective

The type `&World` is currently in an awkward place, since it has two
meanings:
1. Read-only access to the entire world.
2. Interior mutable access to the world; immutable and/or mutable access
to certain portions of world data.

This makes `&World` difficult to reason about, and surprising to see in
function signatures if one does not know about the interior mutable
property.

The type `UnsafeWorldCell` was added in #6404, which is meant to
alleviate this confusion by adding a dedicated type for interior mutable
world access. However, much of the engine still treats `&World` as an
interior mutable-ish type. One of those places is `SystemParam`.

## Solution

Modify `SystemParam::get_param` to accept `UnsafeWorldCell` instead of
`&World`. Simplify the safety invariants, since the `UnsafeWorldCell`
type encapsulates the concept of constrained world access.

---

## Changelog

`SystemParam::get_param` now accepts an `UnsafeWorldCell` instead of
`&World`. This type provides a high-level API for unsafe interior
mutable world access.

## Migration Guide

For manual implementers of `SystemParam`: the function `get_item` now
takes `UnsafeWorldCell` instead of `&World`. To access world data, use:

* `.get_entity()`, which returns an `UnsafeEntityCell` which can be used
to access component data.
* `get_resource()` and its variants, to access resource data.
2023-04-01 15:45:07 +00:00
JoJoJet
a954f3e150
Remove #[system_param(ignore)] and #[world_query(ignore)] (#8265)
# Objective

Follow-up to #8030.

Now that `SystemParam` and `WorldQuery` are implemented for
`PhantomData`, the `ignore` attributes are now unnecessary.

---

## Changelog

- Removed the attributes `#[system_param(ignore)]` and
`#[world_query(ignore)]`.

## Migration Guide

The attributes `#[system_param(ignore)]` and `#[world_query]` ignore
have been removed. If you were using either of these with `PhantomData`
fields, you can simply remove the attribute:

```rust
#[derive(SystemParam)]
struct MyParam<'w, 's, Marker> {
    ...
    // Before:
    #[system_param(ignore)
    _marker: PhantomData<Marker>,

    // After:
    _marker: PhantomData<Marker>,
}
#[derive(WorldQuery)]
struct MyQuery<Marker> {
    ...
    // Before:
    #[world_query(ignore)
    _marker: PhantomData<Marker>,

    // After:
    _marker: PhantomData<Marker>,
}
```

If you were using this for another type that implements `Default`,
consider wrapping that type in `Local<>` (this only works for
`SystemParam`):

```rust
#[derive(SystemParam)]
struct MyParam<'w, 's> {
    // Before:
    #[system_param(ignore)]
    value: MyDefaultType, // This will be initialized using `Default` each time `MyParam` is created.

    // After:
    value: Local<MyDefaultType>, // This will be initialized using `Default` the first time `MyParam` is created.
}
```

If you are implementing either trait and need to preserve the exact
behavior of the old `ignore` attributes, consider manually implementing
`SystemParam` or `WorldQuery` for a wrapper struct that uses the
`Default` trait:

```rust
// Before:

#[derive(WorldQuery)
struct MyQuery {
   #[world_query(ignore)]
    str: String,
}

// After:

#[derive(WorldQuery)
struct MyQuery {
    str: DefaultQuery<String>,
}

pub struct DefaultQuery<T: Default>(pub T);

unsafe impl<T: Default> WorldQuery for DefaultQuery<T> {
    type Item<'w> = Self;
    ...
    unsafe fn fetch<'w>(...) -> Self::Item<'w> {
        Self(T::default())
    }
}
```
2023-03-30 23:02:25 +00:00
JoJoJet
98954311b3
Use actual tests for SystemParam regression tests (#8270)
# Objective

Our regression tests for `SystemParam` currently consist of a bunch of
loosely dispersed struct definitions. This is messy, and doesn't fully
test their functionality.

## Solution

Group the struct definitions into functions annotated with `#[test]`.
This not only makes the module more organized, but it allows us to call
`assert_is_system`, which has the potential to catch some bugs that
would have been missed with the old approach. Also, this approach is
consistent with how `WorldQuery` regression tests are organized.
2023-03-30 21:47:36 +00:00
JoJoJet
d9113cca6f
Make #[system_param(ignore)] and #[world_query(ignore)] unnecessary (#8030)
# Objective

When using `PhantomData` fields with the `#[derive(SystemParam)]` or
`#[derive(WorldQuery)]` macros, the user is required to add the
`#[system_param(ignore)]` attribute so that the macro knows to treat
that field specially. This is undesirable, since it makes the macro more
fragile and less consistent.

## Solution

Implement `SystemParam` and `WorldQuery` for `PhantomData`. This makes
the `ignore` attributes unnecessary.

Some internal changes make the derive macro compatible with types that
have invariant lifetimes, which fixes #8192. From what I can tell, this
fix requires `PhantomData` to implement `SystemParam` in order to ensure
that all of a type's generic parameters are always constrained.

---

## Changelog

+ Implemented `SystemParam` and `WorldQuery` for `PhantomData<T>`.
+ Fixed a miscompilation caused when invariant lifetimes were used with
the `SystemParam` macro.
2023-03-30 15:12:26 +00:00
Rob Parrett
a141aed67d
Format SystemState doc example code (#8261)
# Objective

I noticed some funky whitespace in the examples in the
[`SystemState`](https://docs.rs/bevy/latest/bevy/ecs/system/struct.SystemState.html#example)
doc examples.

## Solution

Semi-manually reformatted these particular code blocks and hid some of
the imports and boilerplate not relevant to the example.


([format_code_in_doc_comments](https://rust-lang.github.io/rustfmt/?version=v1.5.1&search=#format_code_in_doc_comments)
seemingly has a bunch of issues and has not been stabilized)
2023-03-30 10:06:05 +00:00
Robert Walter
5f0abbfd15
Make standard commands more ergonomic (in niche cases) (#8249)
# Objective

I ran into a case where I need to create a `CommandQueue` and push
standard `Command` actions like `Insert` or `Remove` to it manually. I
saw that `Remove` looked as follows:

```rust
struct Remove<T> {
  entity: Entity,
  phantom: PhantomData<T>
}
```

so naturally, I tried to use `Remove::<Foo>::from(entity)` but it didn't
exist. We need to specify the `PhantomData` explicitly when creating
this command action. The same goes for `RemoveResource` and
`InitResource`

## Solution

This PR implements the following:

- `From<Entity>` for `Remove<T>`
- `Default` for `RemoveResource` and `InitResource`
- use these traits in the implementation of methods of `Commands`
- rename `phantom` field on the structs above to `_phantom` to have a
more uniform field naming scheme for the command actions

---

## Changelog

> This section is optional. If this was a trivial fix, or has no
externally-visible impact, you can delete this section.

- Added: implemented `From<Entity>` for `Remove<T>` and `Default` for
`RemoveResource` and `InitResource` for ergonomics

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
2023-03-29 18:27:48 +00:00
Trevor Lovell
464d35aef5
docs: update docs and comments that still refer to stages (#8156)
# Objective
Documentation should no longer be using pre-stageless terminology to
avoid confusion.

## Solution
- update all docs referring to stages to instead refer to sets/schedules
where appropriate
- also mention `apply_system_buffers` for anything system-buffer-related
that previously referred to buffers being applied "at the end of a
stage"
2023-03-27 21:50:21 +00:00
JoJoJet
27f2265e11
Fix name conflicts caused by the SystemParam and WorldQuery macros (#8012)
# Objective

Fix #1727
Fix #8010

Meta types generated by the `SystemParam` and `WorldQuery` derive macros
can conflict with user-defined types if they happen to have the same
name.

## Solution

In order to check if an identifier would conflict with user-defined
types, we can just search the original `TokenStream` passed to the macro
to see if it contains the identifier (since the meta types are defined
in an anonymous scope, it's only possible for them to conflict with the
struct definition itself). When generating an identifier for meta types,
we can simply check if it would conflict, and then add additional
characters to the name until it no longer conflicts with anything.

The `WorldQuery` "Item" and read-only structs are a part of a module's
public API, and thus it is intended for them to conflict with
user-defined types.
2023-03-22 15:45:25 +00:00
JoJoJet
daa1b0209a
Check for conflicting accesses in assert_is_system (#8154)
# Objective

The function `assert_is_system` is used in documentation tests to ensure
that example code actually produces valid systems. Currently,
`assert_is_system` just checks that each function parameter implements
`SystemParam`. To further check the validity of the system, we should
initialize the passed system so that it will be checked for conflicting
accesses. Not only does this enforce the validity of our examples, but
it provides a convenient way to demonstrate conflicting accesses via a
`should_panic` example, which is nicely rendered by rustdoc:

![should_panic
example](https://user-images.githubusercontent.com/21144246/226767682-d1c2f6b9-fc9c-4a4f-a4c4-c7f6070a115f.png)

## Solution

Initialize the system with an empty world to trigger its internal access
conflict checks.

---

## Changelog

The function `bevy::ecs::system::assert_is_system` now panics when
passed a system with conflicting world accesses, as does
`assert_is_read_only_system`.

## Migration Guide

The functions `assert_is_system` and `assert_is_read_only_system` (in
`bevy_ecs::system`) now panic if the passed system has invalid world
accesses. Any tests that called this function on a system with invalid
accesses will now fail. Either fix the system's conflicting accesses, or
specify that the test is meant to fail:

1. For regular tests (that is, functions annotated with `#[test]`), add
the `#[should_panic]` attribute to the function.
2. For documentation tests, add `should_panic` to the start of the code
block: ` ```should_panic`
2023-03-22 13:35:55 +00:00
Carter Anderson
aefe1f0739
Schedule-First: the new and improved add_systems (#8079)
Co-authored-by: Mike <mike.hsu@gmail.com>
2023-03-18 01:45:34 +00:00
Bruce Reif (Buswolley)
bca4b36d4d
change not implemation to custom system struct (#8105)
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
2023-03-17 21:37:16 +00:00
Turki Jamaan
4a62afb97b
Doc and test get_many(_mut) ordering (#8045) 2023-03-16 12:55:44 +00:00
Chris Russell
a63881905a
Pass query change ticks to QueryParIter instead of always using change ticks from World. (#8029)
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
Co-authored-by: James Liu <contact@jamessliu.com>
2023-03-13 22:06:16 +00:00
JoJoJet
ed97c621b8
Move docs for !Sync resources onto the correct trait (#8066) 2023-03-13 16:16:14 +00:00
JoJoJet
fd1af7c8b8
Replace multiple calls to add_system with add_systems (#8001) 2023-03-10 18:15:22 +00:00
JoJoJet
2e7b915ba4
Increase type safety and clarity for change detection (#7905) 2023-03-09 17:17:02 +00:00
Peter Gavin
10e6122c64 make Query::par_iter accept &self (#7912)
This seems to have been a typo/mistake.
2023-03-06 00:00:38 +00:00
JoJoJet
15c55a12ff Fix a typo in CombinatorSystem (#7823)
# Objective

I made a typo in #7605.

## Solution

Fix it.
2023-02-26 02:27:41 +00:00
Cameron
be22569db7 EntityMut: rename remove_intersection to remove and remove to take (#7810)
# Objective

- A more intuitive distinction between the two. `remove_intersection` is verbose and unclear.
- `EntityMut::remove` and `Commands::remove` should match.


## Solution

- What the title says.

---

## Migration Guide

Before
```rust
fn clear_children(parent: Entity, world: &mut World) {
    if let Some(children) = world.entity_mut(parent).remove::<Children>() {
        for &child in &children.0 {
            world.entity_mut(child).remove_intersection::<Parent>();
        }
    }
}
```

After
```rust
fn clear_children(parent: Entity, world: &mut World) {
    if let Some(children) = world.entity_mut(parent).take::<Children>() {
        for &child in &children.0 {
            world.entity_mut(child).remove::<Parent>();
        }
    }
}
```
2023-02-26 00:09:19 +00:00
JoJoJet
695d30bd54 Clean up marker generics for systems (#7789)
# Objective

While we use `#[doc(hidden)]` to try and hide marker generics from the user, these types reveal themselves in compiler errors, adding visual noise and confusion.

## Solution

Replace the `AlreadyWasSystem` marker generic with `()`, to reduce visual noise in error messages. This also makes it possible to return `impl Condition<()>` from combinators.

For function systems, use their function signature as the marker type. This should drastically improve the legibility of some error messages.  
The `InputMarker` type has been removed, since it is unnecessary.
2023-02-23 05:11:12 +00:00
JoJoJet
ee4c8c5ecd Use consistent names for marker generics (#7788)
# Objective

Several places in the ECS use marker generics to avoid overlapping trait implementations, but different places alternately refer to it as `Params` and `Marker`. This is potentially confusing, since it might not be clear that the same pattern is being used. Additionally, users might be misled into thinking that the `Params` type corresponds to the `SystemParam`s of a system.

## Solution

Rename `Params` to `Marker`.
2023-02-23 04:37:08 +00:00
JoJoJet
0c98f9a225 Add AND/OR combinators for run conditions (#7605)
# Objective

Fix #7584.

## Solution

Add an abstraction for creating custom system combinators with minimal boilerplate. Use this to implement AND/OR combinators. Use this to simplify the implementation of `PipeSystem`.

## Example

Feel free to bikeshed on the syntax.

I chose the names `and_then`/`or_else` to emphasize the fact that these short-circuit, while I chose method syntax to empasize that the arguments are *not* treated equally.

```rust
app.add_systems((
    my_system.run_if(resource_exists::<R>().and_then(resource_equals(R(0)))),
    our_system.run_if(resource_exists::<R>().or_else(resource_exists::<S>())),
));
```

---

## Todo

- [ ] Decide on a syntax
- [x] Write docs
- [x] Write tests

## Changelog

+ Added the extension methods `.and_then(...)` and `.or_else(...)` to run conditions, which allows combining run conditions with short-circuiting behavior.
+ Added the trait `Combine`, which can be used with the new `CombinatorSystem` to create system combinators with custom behavior.
2023-02-20 18:16:11 +00:00
dis-da-moe
8853bef6df implement TypeUuid for primitives and fix multiple-parameter generics having the same TypeUuid (#6633)
# Objective

- Fixes #5432 
- Fixes #6680

## Solution

- move code responsible for generating the `impl TypeUuid` from `type_uuid_derive` into a new function, `gen_impl_type_uuid`.
- this allows the new proc macro, `impl_type_uuid`, to call the code for generation.
- added struct `TypeUuidDef` and implemented `syn::Parse` to allow parsing of the input for the new macro.
- finally, used the new macro `impl_type_uuid` to implement `TypeUuid` for the standard library (in `crates/bevy_reflect/src/type_uuid_impl.rs`).
- fixes #6680 by doing a wrapping add of the param's index to its `TYPE_UUID`

Co-authored-by: dis-da-moe <84386186+dis-da-moe@users.noreply.github.com>
2023-02-16 17:09:44 +00:00
JoJoJet
4f57f380c7 Simplify generics for the SystemParamFunction trait (#7675)
# Objective

The `SystemParamFunction` (and `ExclusiveSystemParamFunction`) trait is very cumbersome to use, due to it requiring four generic type parameters. These are currently all used as marker parameters to satisfy rust's trait coherence rules.

### Example (before)

```rust
pub fn pipe<AIn, Shared, BOut, A, AParam, AMarker, B, BParam, BMarker>(
    mut system_a: A,
    mut system_b: B,
) -> impl FnMut(In<AIn>, ParamSet<(AParam, BParam)>) -> BOut
where
    A: SystemParamFunction<AIn, Shared, AParam, AMarker>,
    B: SystemParamFunction<Shared, BOut, BParam, BMarker>,
    AParam: SystemParam,
    BParam: SystemParam,
```

## Solution

Turn the `In`, `Out`, and `Param` generics into associated types. Merge the marker types together to retain coherence.

### Example (after)

```rust
pub fn pipe<A, B, AMarker, BMarker>(
    mut system_a: A,
    mut system_b: B,
) -> impl FnMut(In<A::In>, ParamSet<(A::Param, B::Param)>) -> B::Out
where
    A: SystemParamFunction<AMarker>,
    B: SystemParamFunction<BMarker, In = A::Out>,
```

---

## Changelog

+ Simplified the `SystemParamFunction` and `ExclusiveSystemParamFunction` traits.

## Migration Guide

For users of the `SystemParamFunction` trait, the generic type parameters `In`, `Out`, and `Param` have been turned into associated types. The same has been done with the `ExclusiveSystemParamFunction` trait.
2023-02-15 19:41:15 +00:00
JoJoJet
5efc226290 Allow piping run conditions (#7547)
# Objective

Run conditions are a special type of system that do not modify the world, and which return a bool. Due to the way they are currently implemented, you can *only* use bare function systems as a run condition. Among other things, this prevents the use of system piping with run conditions. This make very basic constructs impossible, such as `my_system.run_if(my_condition.pipe(not))`.

Unblocks a basic solution for #7202.

## Solution

Add the trait `ReadOnlySystem`, which is implemented for any system whose parameters all implement `ReadOnlySystemParam`. Allow any `-> bool` system implementing this trait to be used as a run condition.

---

## Changelog

+ Added the trait `ReadOnlySystem`, which is implemented for any `System` type whose parameters all implement `ReadOnlySystemParam`.
+ Added the function `bevy::ecs::system::assert_is_read_only_system`.
2023-02-07 22:22:16 +00:00
JoJoJet
d26b63a04d Add a SystemParam primitive for deferred mutations; allow #[derive]ing more types of SystemParam (#6817)
# Objective

One pattern to increase parallelism is deferred mutation: instead of directly mutating the world (and preventing other systems from running at the same time), you queue up operations to be applied to the world at the end of the stage. The most common example of this pattern uses the `Commands` SystemParam.

In order to avoid the overhead associated with commands, some power users may want to add their own deferred mutation behavior. To do this, you must implement the unsafe trait `SystemParam`, which interfaces with engine internals in a way that we'd like users to be able to avoid.

## Solution

Add the `Deferred<T>` primitive `SystemParam`, which encapsulates the deferred mutation pattern.
This can be combined with other types of `SystemParam` to safely and ergonomically create powerful custom types.

Essentially, this is just a variant of `Local<T>` which can run code at the end of the stage.

This type is used in the engine to derive `Commands` and `ParallelCommands`, which removes a bunch of unsafe boilerplate.

### Example

```rust
use bevy_ecs::system::{Deferred, SystemBuffer};

/// Sends events with a delay, but may run in parallel with other event writers.
#[derive(SystemParam)]
pub struct BufferedEventWriter<'s, E: Event> {
    queue: Deferred<'s, EventQueue<E>>,
}

struct EventQueue<E>(Vec<E>);

impl<'s, E: Event> BufferedEventWriter<'s, E> {
    /// Queues up an event to be sent at the end of this stage.
    pub fn send(&mut self, event: E) {
        self.queue.0.push(event);
    }
}

// The `SystemBuffer` trait controls how [`Deferred`] gets applied at the end of the stage.
impl<E: Event> SystemBuffer for EventQueue<E> {
    fn apply(&mut self, world: &mut World) {
        let mut events = world.resource_mut::<Events<E>>();
        for e in self.0.drain(..) {
            events.send(e);
        }
    }
}
```

---

## Changelog

+ Added the `SystemParam` type `Deferred<T>`, which can be used to defer `World` mutations. Powered by the new trait `SystemBuffer`.
2023-02-06 21:57:57 +00:00
Boxy
5c680a38a7 Move all logic to UnsafeWorldCell (#7381)
# Objective

- Implementing logic used by system params and `UnsafeWorldCell` on `&World` is sus since `&World` generally denotes shared read only access to world but this is a lie in the above situations. Move most/all logic that uses `&World` to mean `UnsafeWorldCell` onto `UnsafeWorldCell`
-  Add a way to take a `&mut World` out of `UnsafeWorldCell` and use this in `WorldCell`'s `Drop` impl instead of a `UnsafeCell` field

---

## Changelog

- changed some `UnsafeWorldCell` methods to take `self` instead of `&self`/`&mut self` since there is literally no point to them doing that
- `UnsafeWorldCell::world` is now used to get immutable access to the whole world instead of just the metadata which can now be done via `UnsafeWorldCell::world_metadata`
- `UnsafeWorldCell::world_mut` now exists and can be used to get a `&mut World` out of `UnsafeWorldCell`
- removed `UnsafeWorldCell::storages` since that is probably unsound since storages contains the actual component/resource data not just metadata

## Migration guide

N/A none of the breaking changes here make any difference for a 0.9->0.10 transition since `UnsafeWorldCell` did not exist in 0.9
2023-02-06 19:02:52 +00:00
张林伟
aa4170d9a4 Rename schedule v3 to schedule (#7519)
# Objective

- Follow up of https://github.com/bevyengine/bevy/pull/7267

## Solution

- Rename schedule_v3 to schedule
- Suppress "module inception" lint
2023-02-06 18:44:40 +00:00
Alice Cecile
206c7ce219 Migrate engine to Schedule v3 (#7267)
Huge thanks to @maniwani, @devil-ira, @hymm, @cart, @superdump and @jakobhellermann for the help with this PR.

# Objective

- Followup #6587.
- Minimal integration for the Stageless Scheduling RFC: https://github.com/bevyengine/rfcs/pull/45

## Solution

- [x]  Remove old scheduling module
- [x] Migrate new methods to no longer use extension methods
- [x] Fix compiler errors
- [x] Fix benchmarks
- [x] Fix examples
- [x] Fix docs
- [x] Fix tests

## Changelog

### Added

- a large number of methods on `App` to work with schedules ergonomically
- the `CoreSchedule` enum
- `App::add_extract_system` via the `RenderingAppExtension` trait extension method
- the private `prepare_view_uniforms` system now has a public system set for scheduling purposes, called `ViewSet::PrepareUniforms`

### Removed

- stages, and all code that mentions stages
- states have been dramatically simplified, and no longer use a stack
- `RunCriteriaLabel`
- `AsSystemLabel` trait
- `on_hierarchy_reports_enabled` run criteria (now just uses an ad hoc resource checking run condition)
- systems in `RenderSet/Stage::Extract` no longer warn when they do not read data from the main world
- `RunCriteriaLabel`
- `transform_propagate_system_set`: this was a nonstandard pattern that didn't actually provide enough control. The systems are already `pub`: the docs have been updated to ensure that the third-party usage is clear.

### Changed

- `System::default_labels` is now `System::default_system_sets`.
- `App::add_default_labels` is now `App::add_default_sets`
- `CoreStage` and `StartupStage` enums are now `CoreSet` and `StartupSet`
- `App::add_system_set` was renamed to `App::add_systems`
- The `StartupSchedule` label is now defined as part of the `CoreSchedules` enum
-  `.label(SystemLabel)` is now referred to as `.in_set(SystemSet)`
- `SystemLabel` trait was replaced by `SystemSet`
- `SystemTypeIdLabel<T>` was replaced by `SystemSetType<T>`
- The `ReportHierarchyIssue` resource now has a public constructor (`new`), and implements `PartialEq`
- Fixed time steps now use a schedule (`CoreSchedule::FixedTimeStep`) rather than a run criteria.
- Adding rendering extraction systems now panics rather than silently failing if no subapp with the `RenderApp` label is found.
- the `calculate_bounds` system, with the `CalculateBounds` label, is now in `CoreSet::Update`, rather than in `CoreSet::PostUpdate` before commands are applied. 
- `SceneSpawnerSystem` now runs under `CoreSet::Update`, rather than `CoreStage::PreUpdate.at_end()`.
- `bevy_pbr::add_clusters` is no longer an exclusive system
- the top level `bevy_ecs::schedule` module was replaced with `bevy_ecs::scheduling`
- `tick_global_task_pools_on_main_thread` is no longer run as an exclusive system. Instead, it has been replaced by `tick_global_task_pools`, which uses a `NonSend` resource to force running on the main thread.

## Migration Guide

- Calls to `.label(MyLabel)` should be replaced with `.in_set(MySet)`
- Stages have been removed. Replace these with system sets, and then add command flushes using the `apply_system_buffers` exclusive system where needed.
- The `CoreStage`, `StartupStage, `RenderStage` and `AssetStage`  enums have been replaced with `CoreSet`, `StartupSet, `RenderSet` and `AssetSet`. The same scheduling guarantees have been preserved.
  - Systems are no longer added to `CoreSet::Update` by default. Add systems manually if this behavior is needed, although you should consider adding your game logic systems to `CoreSchedule::FixedTimestep` instead for more reliable framerate-independent behavior.
  - Similarly, startup systems are no longer part of `StartupSet::Startup` by default. In most cases, this won't matter to you.
  - For example, `add_system_to_stage(CoreStage::PostUpdate, my_system)` should be replaced with 
  - `add_system(my_system.in_set(CoreSet::PostUpdate)`
- When testing systems or otherwise running them in a headless fashion, simply construct and run a schedule using `Schedule::new()` and `World::run_schedule` rather than constructing stages
- Run criteria have been renamed to run conditions. These can now be combined with each other and with states.
- Looping run criteria and state stacks have been removed. Use an exclusive system that runs a schedule if you need this level of control over system control flow.
- For app-level control flow over which schedules get run when (such as for rollback networking), create your own schedule and insert it under the `CoreSchedule::Outer` label.
- Fixed timesteps are now evaluated in a schedule, rather than controlled via run criteria. The `run_fixed_timestep` system runs this schedule between `CoreSet::First` and `CoreSet::PreUpdate` by default.
- Command flush points introduced by `AssetStage` have been removed. If you were relying on these, add them back manually.
- Adding extract systems is now typically done directly on the main app. Make sure the `RenderingAppExtension` trait is in scope, then call `app.add_extract_system(my_system)`.
- the `calculate_bounds` system, with the `CalculateBounds` label, is now in `CoreSet::Update`, rather than in `CoreSet::PostUpdate` before commands are applied. You may need to order your movement systems to occur before this system in order to avoid system order ambiguities in culling behavior.
- the `RenderLabel` `AppLabel` was renamed to `RenderApp` for clarity
- `App::add_state` now takes 0 arguments: the starting state is set based on the `Default` impl.
- Instead of creating `SystemSet` containers for systems that run in stages, simply use `.on_enter::<State::Variant>()` or its `on_exit` or `on_update` siblings.
- `SystemLabel` derives should be replaced with `SystemSet`. You will also need to add the `Debug`, `PartialEq`, `Eq`, and `Hash` traits to satisfy the new trait bounds.
- `with_run_criteria` has been renamed to `run_if`. Run criteria have been renamed to run conditions for clarity, and should now simply return a bool.
- States have been dramatically simplified: there is no longer a "state stack". To queue a transition to the next state, call `NextState::set`

## TODO

- [x] remove dead methods on App and World
- [x] add `App::add_system_to_schedule` and `App::add_systems_to_schedule`
- [x] avoid adding the default system set at inappropriate times
- [x] remove any accidental cycles in the default plugins schedule
- [x] migrate benchmarks
- [x] expose explicit labels for the built-in command flush points
- [x] migrate engine code
- [x] remove all mentions of stages from the docs
- [x] verify docs for States
- [x] fix uses of exclusive systems that use .end / .at_start / .before_commands
- [x] migrate RenderStage and AssetStage
- [x] migrate examples
- [x] ensure that transform propagation is exported in a sufficiently public way (the systems are already pub)
- [x] ensure that on_enter schedules are run at least once before the main app
- [x] re-enable opt-in to execution order ambiguities
- [x] revert change to `update_bounds` to ensure it runs in `PostUpdate`
- [x] test all examples
  - [x] unbreak directional lights
  - [x] unbreak shadows (see 3d_scene, 3d_shape, lighting, transparaency_3d examples)
  - [x] game menu example shows loading screen and menu simultaneously
  - [x] display settings menu is a blank screen
  - [x] `without_winit` example panics
- [x] ensure all tests pass
  - [x] SubApp doc test fails
  - [x] runs_spawn_local tasks fails
  - [x] [Fix panic_when_hierachy_cycle test hanging](https://github.com/alice-i-cecile/bevy/pull/120)

## Points of Difficulty and Controversy

**Reviewers, please give feedback on these and look closely**

1.  Default sets, from the RFC, have been removed. These added a tremendous amount of implicit complexity and result in hard to debug scheduling errors. They're going to be tackled in the form of "base sets" by @cart in a followup.
2. The outer schedule controls which schedule is run when `App::update` is called.
3. I implemented `Label for `Box<dyn Label>` for our label types. This enables us to store schedule labels in concrete form, and then later run them. I ran into the same set of problems when working with one-shot systems. We've previously investigated this pattern in depth, and it does not appear to lead to extra indirection with nested boxes.
4. `SubApp::update` simply runs the default schedule once. This sucks, but this whole API is incomplete and this was the minimal changeset.
5. `time_system` and `tick_global_task_pools_on_main_thread` no longer use exclusive systems to attempt to force scheduling order
6. Implemetnation strategy for fixed timesteps
7. `AssetStage` was migrated to `AssetSet` without reintroducing command flush points. These did not appear to be used, and it's nice to remove these bottlenecks.
8. Migration of `bevy_render/lib.rs` and pipelined rendering. The logic here is unusually tricky, as we have complex scheduling requirements.

## Future Work (ideally before 0.10)

- Rename schedule_v3 module to schedule or scheduling
- Add a derive macro to states, and likely a `EnumIter` trait of some form
- Figure out what exactly to do with the "systems added should basically work by default" problem
- Improve ergonomics for working with fixed timesteps and states
- Polish FixedTime API to match Time
- Rebase and merge #7415
- Resolve all internal ambiguities (blocked on better tools, especially #7442)
- Add "base sets" to replace the removed default sets.
2023-02-06 02:04:50 +00:00
Aceeri
67826b21d4 Replace RemovedComponents<T> backing with Events<Entity> (#5680)
# Objective
Removal events are unwieldy and require some knowledge of when to put systems that need to catch events for them, it is very easy to end up missing one and end up with memory leak-ish issues where you don't clean up after yourself.

## Solution
Consolidate removals with the benefits of `Events<...>` (such as double buffering and per system ticks for reading the events) and reduce the special casing of it, ideally I was hoping to move the removals to a `Resource` in the world, but that seems a bit more rough to implement/maintain because of double mutable borrowing issues.

This doesn't go the full length of change detection esque removal detection a la https://github.com/bevyengine/rfcs/pull/44.
Just tries to make the current workflow a bit more user friendly so detecting removals isn't such a scheduling nightmare.

---

## Changelog
- RemovedComponents<T> is now backed by an `Events<Entity>` for the benefits of double buffering.

## Migration Guide
- Add a `mut` for `removed: RemovedComponents<T>` since we are now modifying an event reader internally.
- Iterating over removed components now requires `&mut removed_components` or `removed_components.iter()` instead of `&removed_components`.
2023-02-04 20:53:37 +00:00
JoJoJet
e0bf4311d3 Remove ExclusiveSystemParam::apply (#7489)
# Objective

The trait method `SystemParam::apply` allows a `SystemParam` type to defer world mutations, which is internally used to apply `Commands` at the end of the stage. Any operations that require `&mut World` access must be deferred in this way, since parallel systems do not have exclusive access to the world.

The `ExclusiveSystemParam` trait (added in #6083) has an `apply` method which serves the same purpose. However, deferring mutations in this way does not make sense for exclusive systems since they already have `&mut World` access: there is no need to wait until a hard sync point, as the system *is* a hard sync point. World mutations can and should be performed within the body of the system.

## Solution

Remove the method. There were no implementations of this method in the engine.

---

## Changelog

*Note for maintainers: this changelog makes more sense if it's placed above the one for #6919.*

- Removed the method `ExclusiveSystemParamState::apply`.

## Migration Guide

*Note for maintainers: this migration guide makes more sense if it's placed above the one for #6919.*

The trait method `ExclusiveSystemParamState::apply` has been removed. If you have an exclusive system with buffers that must be applied, you should apply them within the body of the exclusive system.
2023-02-04 00:25:09 +00:00
Björn Tegelund
6506ea4d83 Add unit test with system that panics (#7491)
# Objective

Fixes #7434.

This is my first time contributing to a Rust project, so please let me know if this wasn't the change intended by the linked issue.

## Solution

Adds a test with a system that panics to `bevy_ecs`.

I'm not sure if this is the intended panic message, but this is what the test currently results in:
```
thread 'system::tests::panic_inside_system' panicked at 'called `Option::unwrap()` on a `None` value', /Users/bjorn/workplace/bevy/crates/bevy_tasks/src/task_pool.rs:354:49
```
2023-02-03 21:05:47 +00:00
JoJoJet
44a572e4e6 Fix ignored lifetimes in #[derive(SystemParam)] (#7458)
# Objective

Fix #7447.

The `SystemParam` derive uses the wrong lifetimes for ignored fields.

## Solution

Use type inference instead of explicitly naming the types of ignored fields. This allows the compiler to automatically use the correct lifetime.
2023-02-03 09:17:48 +00:00
JoJoJet
5d912a2f35 Speed up CommandQueue by storing commands more densely (#6391)
# Objective

* Speed up inserting and applying commands. 
* Halve the stack size of `CommandQueue` to 24 bytes.
* Require fewer allocations.

## Solution

Store commands and metadata densely within the same buffer. Each command takes up 1 `usize` of metadata, plus the bytes to store the command itself. Zero-sized types take up no space except for the metadata.

# Benchmarks

All of the benchmarks related to commands.

| Bench                                  | Time      |  % Change         |    p-value |
|----------------------------------------|-----------|--------------|-----------------|
| empty_commands/0_entities              | 4.7780 ns | -18.381% | 0.00  |
| spawn_commands/2000_entities           | 233.11 us | -0.9961%     | 0.00  |
| spawn_commands/4000_entities           | 448.38 us | -3.1466%     | 0.00  |
| spawn_commands/6000_entities           | 693.12 us | -0.3978%     | _0.52_ |
| spawn_commands/8000_entities           | 889.48 us | -2.8802%     | 0.00  |
| insert_commands/insert                 | 609.95 us | -4.8604%     | 0.00  |
| insert_commands/insert_batch           | 355.54 us | -2.8165%     |  0.00 |
| fake_commands/2000_commands            | 4.8018 us | **-17.802%** | 0.00  |
| fake_commands/4000_commands            | 9.5969 us | **-17.337%** | 0.00  |
| fake_commands/6000_commands            | 14.421 us | **-18.454%** | 0.00  |
| fake_commands/8000_commands            | 19.192 us | **-18.261%** | 0.00 |
| sized_commands_0_bytes/2000_commands   | 4.0593 us | -4.7145%     |  0.00 |
| sized_commands_0_bytes/4000_commands   | 8.1541 us | -4.9470%     |  0.00  |
| sized_commands_0_bytes/6000_commands   | 12.806 us | -12.017%     | 0.00 |
| sized_commands_0_bytes/8000_commands   | 17.096 us | -14.070% |  0.00 |
| sized_commands_12_bytes/2000_commands  | 5.3425 us | **-27.632%** | 0.00 |
| sized_commands_12_bytes/4000_commands  | 10.283 us | **-31.158%** |  0.00  |
| sized_commands_12_bytes/6000_commands  | 15.339 us | **-31.418%** |  0.00 |
| sized_commands_12_bytes/8000_commands  | 20.206 us | **-33.133%** |  0.00 |
| sized_commands_512_bytes/2000_commands | 99.118 us | -9.9655%     |  0.00  |
| sized_commands_512_bytes/4000_commands | 201.96 us | -8.8235%     |  0.00 |
| sized_commands_512_bytes/6000_commands | 300.95 us | -9.2344%     |  0.00  |
| sized_commands_512_bytes/8000_commands | 404.69 us | -8.4578%     |  0.00  |
2023-01-28 01:15:51 +00:00
Chris Ohk
3281aea5c2 Fix minor typos in code and docs (#7378)
# Objective

I found several words in code and docs are incorrect. This should be fixed.

## Solution

- Fix several minor typos

Co-authored-by: Chris Ohk <utilforever@gmail.com>
2023-01-27 12:12:53 +00:00
Jakob Hellermann
0cb0d8b55d add UnsafeWorldCell abstraction (#6404)
alternative to #5922, implements #5956 
builds on top of https://github.com/bevyengine/bevy/pull/6402

# Objective

https://github.com/bevyengine/bevy/issues/5956 goes into more detail, but the TLDR is:
- bevy systems ensure disjoint accesses to resources and components, and for that to work there are methods `World::get_resource_unchecked_mut(&self)`, ..., `EntityRef::get_mut_unchecked(&self)` etc.
- we don't have these unchecked methods for `by_id` variants, so third-party crate authors cannot build their own safe disjoint-access abstractions with these
- having `_unchecked_mut` methods is not great, because in their presence safe code can accidentally violate subtle invariants. Having to go through `world.as_unsafe_world_cell().unsafe_method()` forces you to stop and think about what you want to write in your `// SAFETY` comment.

The alternative is to keep exposing `_unchecked_mut` variants for every operation that we want third-party crates to build upon, but we'd prefer to avoid using these methods alltogether: https://github.com/bevyengine/bevy/pull/5922#issuecomment-1241954543

Also, this is something that **cannot be implemented outside of bevy**, so having either this PR or #5922 as an escape hatch with lots of discouraging comments would be great.

## Solution

- add `UnsafeWorldCell` with `unsafe fn get_resource(&self)`, `unsafe fn get_resource_mut(&self)`
- add `fn World::as_unsafe_world_cell(&mut self) -> UnsafeWorldCell<'_>` (and `as_unsafe_world_cell_readonly(&self)`)
- add `UnsafeWorldCellEntityRef` with `unsafe fn get`, `unsafe fn get_mut` and the other utilities on `EntityRef` (no methods for spawning, despawning, insertion)
- use the `UnsafeWorldCell` abstraction in `ReflectComponent`, `ReflectResource` and `ReflectAsset`, so these APIs are easier to reason about
- remove `World::get_resource_mut_unchecked`, `EntityRef::get_mut_unchecked` and use `unsafe { world.as_unsafe_world_cell().get_mut() }` and `unsafe { world.as_unsafe_world_cell().get_entity(entity)?.get_mut() }` instead

This PR does **not** make use of `UnsafeWorldCell` for anywhere else in `bevy_ecs` such as `SystemParam` or `Query`. That is a much larger change, and I am convinced that having `UnsafeWorldCell` is already useful for third-party crates.

Implemented API:

```rust
struct World { .. }
impl World {
  fn as_unsafe_world_cell(&self) -> UnsafeWorldCell<'_>;
}

struct UnsafeWorldCell<'w>(&'w World);
impl<'w> UnsafeWorldCell {
  unsafe fn world(&self) -> &World;

  fn get_entity(&self) -> UnsafeWorldCellEntityRef<'w>; // returns 'w which is `'self` of the `World::as_unsafe_world_cell(&'w self)`

  unsafe fn get_resource<T>(&self) -> Option<&'w T>;
  unsafe fn get_resource_by_id(&self, ComponentId) -> Option<&'w T>;
  unsafe fn get_resource_mut<T>(&self) -> Option<Mut<'w, T>>;
  unsafe fn get_resource_mut_by_id(&self) -> Option<MutUntyped<'w>>;
  unsafe fn get_non_send_resource<T>(&self) -> Option<&'w T>;
  unsafe fn get_non_send_resource_mut<T>(&self) -> Option<Mut<'w, T>>>;

  // not included: remove, remove_resource, despawn, anything that might change archetypes
}

struct UnsafeWorldCellEntityRef<'w> { .. }
impl UnsafeWorldCellEntityRef<'w> {
  unsafe fn get<T>(&self, Entity) -> Option<&'w T>;
  unsafe fn get_by_id(&self, Entity, ComponentId) -> Option<Ptr<'w>>;
  unsafe fn get_mut<T>(&self, Entity) -> Option<Mut<'w, T>>;
  unsafe fn get_mut_by_id(&self, Entity, ComponentId) -> Option<MutUntyped<'w>>;
  unsafe fn get_change_ticks<T>(&self, Entity) -> Option<Mut<'w, T>>;
  // fn id, archetype, contains, contains_id, containts_type_id
}
```

<details>
<summary>UnsafeWorldCell docs</summary>

Variant of the [`World`] where resource and component accesses takes a `&World`, and the responsibility to avoid
aliasing violations are given to the caller instead of being checked at compile-time by rust's unique XOR shared rule.

### Rationale
In rust, having a `&mut World` means that there are absolutely no other references to the safe world alive at the same time,
without exceptions. Not even unsafe code can change this.

But there are situations where careful shared mutable access through a type is possible and safe. For this, rust provides the [`UnsafeCell`](std::cell::UnsafeCell)
escape hatch, which allows you to get a `*mut T` from a `&UnsafeCell<T>` and around which safe abstractions can be built.

Access to resources and components can be done uniquely using [`World::resource_mut`] and [`World::entity_mut`], and shared using [`World::resource`] and [`World::entity`].
These methods use lifetimes to check at compile time that no aliasing rules are being broken.

This alone is not enough to implement bevy systems where multiple systems can access *disjoint* parts of the world concurrently. For this, bevy stores all values of
resources and components (and [`ComponentTicks`](crate::component::ComponentTicks)) in [`UnsafeCell`](std::cell::UnsafeCell)s, and carefully validates disjoint access patterns using
APIs like [`System::component_access`](crate::system::System::component_access).

A system then can be executed using [`System::run_unsafe`](crate::system::System::run_unsafe) with a `&World` and use methods with interior mutability to access resource values.
access resource values.

### Example Usage

[`UnsafeWorldCell`] can be used as a building block for writing APIs that safely allow disjoint access into the world.
In the following example, the world is split into a resource access half and a component access half, where each one can
safely hand out mutable references.

```rust
use bevy_ecs::world::World;
use bevy_ecs::change_detection::Mut;
use bevy_ecs::system::Resource;
use bevy_ecs::world::unsafe_world_cell_world::UnsafeWorldCell;

// INVARIANT: existance of this struct means that users of it are the only ones being able to access resources in the world
struct OnlyResourceAccessWorld<'w>(UnsafeWorldCell<'w>);
// INVARIANT: existance of this struct means that users of it are the only ones being able to access components in the world
struct OnlyComponentAccessWorld<'w>(UnsafeWorldCell<'w>);

impl<'w> OnlyResourceAccessWorld<'w> {
    fn get_resource_mut<T: Resource>(&mut self) -> Option<Mut<'w, T>> {
        // SAFETY: resource access is allowed through this UnsafeWorldCell
        unsafe { self.0.get_resource_mut::<T>() }
    }
}
// impl<'w> OnlyComponentAccessWorld<'w> {
//     ...
// }

// the two interior mutable worlds borrow from the `&mut World`, so it cannot be accessed while they are live
fn split_world_access(world: &mut World) -> (OnlyResourceAccessWorld<'_>, OnlyComponentAccessWorld<'_>) {
    let resource_access = OnlyResourceAccessWorld(unsafe { world.as_unsafe_world_cell() });
    let component_access = OnlyComponentAccessWorld(unsafe { world.as_unsafe_world_cell() });
    (resource_access, component_access)
}
```


</details>
2023-01-27 00:12:13 +00:00
JoJoJet
e84907fc55 Add context to compile tests (#7342)
# Objective

`bevy_ecs/system_param.rs` contains many seemingly-arbitrary struct definitions which serve as compile tests.

## Solution

Add a comment to each one, linking the issue or PR that motivated its addition.
2023-01-23 22:34:32 +00:00
JoJoJet
5d5a504685 Revise SystemParam docs (#7274)
# Objective

Increase clarity in a few places for the `SystemParam` docs.
2023-01-20 13:39:23 +00:00
James Liu
dfea88c64d Basic adaptive batching for parallel query iteration (#4777)
# Objective
Fixes #3184. Fixes #6640. Fixes #4798. Using `Query::par_for_each(_mut)` currently requires a `batch_size` parameter, which affects how it chunks up large archetypes and tables into smaller chunks to run in parallel. Tuning this value is difficult, as the performance characteristics entirely depends on the state of the `World` it's being run on. Typically, users will just use a flat constant and just tune it by hand until it performs well in some benchmarks. However, this is both error prone and risks overfitting the tuning on that benchmark.

This PR proposes a naive automatic batch-size computation based on the current state of the `World`.

## Background
`Query::par_for_each(_mut)` schedules a new Task for every archetype or table that it matches. Archetypes/tables larger than the batch size are chunked into smaller tasks. Assuming every entity matched by the query has an identical workload, this makes the worst case scenario involve using a batch size equal to the size of the largest matched archetype or table. Conversely, a batch size of `max {archetype, table} size / thread count * COUNT_PER_THREAD` is likely the sweetspot where the overhead of scheduling tasks is minimized, at least not without grouping small archetypes/tables together.

There is also likely a strict minimum batch size below which the overhead of scheduling these tasks is heavier than running the entire thing single-threaded.

## Solution

- [x] Remove the `batch_size` from `Query(State)::par_for_each`  and friends.
- [x] Add a check to compute `batch_size = max {archeytpe/table} size / thread count  * COUNT_PER_THREAD`
- [x] ~~Panic if thread count is 0.~~ Defer to `for_each` if the thread count is 1 or less.
- [x] Early return if there is no matched table/archetype. 
- [x] Add override option for users have queries that strongly violate the initial assumption that all iterated entities have an equal workload.

---

## Changelog
Changed: `Query::par_for_each(_mut)` has been changed to `Query::par_iter(_mut)` and will now automatically try to produce a batch size for callers based on the current `World` state.

## Migration Guide
The `batch_size` parameter for `Query(State)::par_for_each(_mut)` has been removed. These calls will automatically compute a batch size for you. Remove these parameters from all calls to these functions.

Before:
```rust
fn parallel_system(query: Query<&MyComponent>) {
   query.par_for_each(32, |comp| {
        ...
   });
}
```

After:

```rust
fn parallel_system(query: Query<&MyComponent>) {
   query.par_iter().for_each(|comp| {
        ...
   });
}
```

Co-authored-by: Arnav Choubey <56453634+x-52@users.noreply.github.com>
Co-authored-by: Robert Swain <robert.swain@gmail.com>
Co-authored-by: François <mockersf@gmail.com>
Co-authored-by: Corey Farwell <coreyf@rwell.org>
Co-authored-by: Aevyrie <aevyrie@gmail.com>
2023-01-20 08:47:20 +00:00
JoJoJet
629cfab135 Improve safety for CommandQueue internals (#7039)
# Objective

- Safety comments for the `CommandQueue` type are quite sparse and very imprecise. Sometimes, they are right for the wrong reasons or use circular reasoning.

## Solution

- Document previously-implicit safety invariants.
- Rewrite safety comments to actually reflect the specific invariants of each operation.
- Use `OwningPtr` instead of raw pointers, to encode an invariant in the type system instead of via comments.
- Use typed pointer methods when possible to increase reliability.

---

## Changelog

+ Added the function `OwningPtr::read_unaligned`.
2023-01-19 03:04:39 +00:00
James Liu
88b353c4b1 Reduce the use of atomics in the render phase (#7084)
# Objective
Speed up the render phase of rendering. An extension of #6885.

`SystemState::get` increments the `World`'s change tick atomically every time it's called. This is notably more expensive than a unsynchronized increment, even without contention. It also updates the archetypes, even when there has been nothing to update when it's called repeatedly.

## Solution
Piggyback off of #6885. Split `SystemState::validate_world_and_update_archetypes` into `SystemState::validate_world` and `SystemState::update_archetypes`, and make the later `pub`. Then create safe variants of `SystemState::get_unchecked_manual` that still validate the `World` but do not update archetypes and do not increment the change tick using `World::read_change_tick` and `World::change_tick`. Update `RenderCommandState` to call `SystemState::update_archetypes` in `Draw::prepare` and `SystemState::get_manual` in `Draw::draw`.

## Performance
There's a slight perf benefit (~2%) for `main_opaque_pass_3d` on `many_foxes` (340.39 us -> 333.32 us)

![image](https://user-images.githubusercontent.com/3137680/210643746-25320b98-3e2b-4a95-8084-892c23bb8b4e.png)

## Alternatives
We can change `SystemState::get` to not increment the `World`'s change tick. Though this would still put updating the archetypes and an atomic read on the hot-path.

---

## Changelog
Added: `SystemState::get_manual`
Added: `SystemState::get_manual_mut`
Added: `SystemState::update_archetypes`
2023-01-18 02:19:19 +00:00
JoJoJet
b5893e570d Add a missing impl of ReadOnlySystemParam for Option<NonSend<>> (#7245)
# Objective

The trait `ReadOnlySystemParam` is not implemented for `Option<NonSend<>>`, even though it should be.

Follow-up to #7243. This fixes another mistake made in #6919.

## Solution

Add the missing impl.
2023-01-17 03:29:08 +00:00
JoJoJet
0efe66b081 Remove an incorrect impl of ReadOnlySystemParam for NonSendMut (#7243)
# Objective

The trait `ReadOnlySystemParam` is implemented for `NonSendMut`, when it should not be. This mistake was made in #6919.

## Solution

Remove the incorrect impl.
2023-01-17 01:39:19 +00:00
Cameron
684f07595f Add bevy_ecs::schedule_v3 module (#6587)
# Objective

Complete the first part of the migration detailed in bevyengine/rfcs#45.

## Solution

Add all the new stuff.

### TODO

- [x] Impl tuple methods.
- [x] Impl chaining.
- [x] Port ambiguity detection.
- [x] Write docs.
- [x] ~~Write more tests.~~(will do later)
- [ ] Write changelog and examples here?
- [x] ~~Replace `petgraph`.~~ (will do later)



Co-authored-by: james7132 <contact@jamessliu.com>
Co-authored-by: Michael Hsu <mike.hsu@gmail.com>
Co-authored-by: Mike Hsu <mike.hsu@gmail.com>
2023-01-17 01:39:17 +00:00
JoJoJet
38005b0702 Support piping exclusive systems (#7023)
# Objective

Fix #5248.

## Solution

Support `In<T>` parameters and allow returning arbitrary types in exclusive systems.

---

## Changelog

- Exclusive systems may now be used with system piping.

## Migration Guide

Exclusive systems (systems that access `&mut World`) now support system piping, so the `ExclusiveSystemParamFunction` trait now has generics for the `In`put and `Out`put types.

```rust
// Before
fn my_generic_system<T, Param>(system_function: T)
where T: ExclusiveSystemParamFunction<Param>
{ ... }

// After
fn my_generic_system<T, In, Out, Param>(system_function: T)
where T: ExclusiveSystemParamFunction<In, Out, Param>
{ ... }
```
2023-01-16 15:22:38 +00:00
Joshua Chapman
9dd8fbc570 Added Ref to allow immutable access with change detection (#7097)
# Objective

- Fixes #7066 

## Solution

- Split the ChangeDetection trait into ChangeDetection and ChangeDetectionMut
- Added Ref as equivalent to &T with change detection

---

## Changelog

- Support for Ref which allow inspecting change detection flags in an immutable way

## Migration Guide

- While bevy prelude includes both ChangeDetection and ChangeDetectionMut any code explicitly referencing ChangeDetection might need to be updated to ChangeDetectionMut or both. Specifically any reading logic requires ChangeDetection while writes requires ChangeDetectionMut.

use bevy_ecs::change_detection::DetectChanges -> use bevy_ecs::change_detection::{DetectChanges, DetectChangesMut}

- Previously Res had methods to access change detection `is_changed` and `is_added` those methods have been moved to the `DetectChanges` trait. If you are including bevy prelude you will have access to these types otherwise you will need to `use bevy_ecs::change_detection::DetectChanges` to continue using them.
2023-01-11 15:41:54 +00:00
张林伟
0d2cdb450d Fix beta clippy lints (#7154)
# Objective

- When I run `cargo run -p ci` for my pr locally using latest beta toolchain, the ci failed due to [uninlined_format_args](https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args) and [needless_lifetimes](https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes) lints

## Solution

- Fix lints according to clippy suggestions.
2023-01-11 09:51:22 +00:00
Boxy
d4babafe81 Make Query fields private (#7149)
`Query`'s fields being `pub(crate)` means that the struct can be constructed via safe code from anywhere in `bevy_ecs` . This is Not Good since it is intended that all construction of this type goes through `Query::new` which is an `unsafe fn` letting various `Query` methods rely on those invariants holding even though they can be trivially bypassed.

This has no user facing impact
2023-01-10 18:55:23 +00:00
JoJoJet
fa40e2badb Fix a miscompilation with #[derive(SystemParam)] (#7105)
# Objective

- Fix #7103.
- The issue is caused because I forgot to add a where clause to a generated struct in #7056.

## Solution

- Add the where clause.
2023-01-10 18:41:50 +00:00
Tirth Patel
a207178344 Add wrapping_add to change_tick (#7146)
# Objective

Fixes #7140


## Solution

As discussed in the issue, added wrapping_add

---
2023-01-10 17:48:34 +00:00
Boxy
d03c1a0687 Ensure Query does not use the wrong World (#7150)
`Query` relies on the `World` it stores being the same as the world used for creating the `QueryState` it stores. If they are not the same then everything is very unsound. This was not actually being checked anywhere, `Query::new` did not have a safety invariant or even an assertion that the `WorldId`'s are the same.

This shouldn't have any user facing impact unless we have really messed up in bevy and have unsoundness elsewhere (in which case we would now get a panic instead of being unsound).
2023-01-10 17:25:45 +00:00
zeroacez
aaaf357dbb Added docs for `.apply()in basic usage of systemState` (#7138)
# Objective

Fixes #5940 

## Solution

Added the suggested comment.

Co-authored-by: zeroacez <43633834+zeroacez@users.noreply.github.com>
2023-01-10 17:25:44 +00:00
JoJoJet
afe0a0650b Relax Sync bound on Local<T> as ExclusiveSystemParam (#7040)
# Objective

The type `Local<T>` unnecessarily has the bound `T: Sync` when the local is used in an exclusive system.

## Solution

Lift the bound.

---

## Changelog

Removed the bound `T: Sync` from `Local<T>` when used as an `ExclusiveSystemParam`.
2023-01-09 20:56:06 +00:00