# Objective
It's possible to create UB using an implementation of `QueryFilter` that
performs mutable access, but that does not violate any documented safety
invariants.
This code:
```rust
#[derive(Component)]
struct Foo(usize);
// This derive is a simple way to get a valid WorldQuery impl. The QueryData impl isn't used.
#[derive(QueryData)]
#[query_data(mutable)]
struct BadFilter<'w> {
foo: &'w mut Foo,
}
impl QueryFilter for BadFilter<'_> {
const IS_ARCHETYPAL: bool = false;
unsafe fn filter_fetch(
fetch: &mut Self::Fetch<'_>,
entity: Entity,
table_row: TableRow,
) -> bool {
// SAFETY: fetch and filter_fetch have the same safety requirements
let f: &mut usize = &mut unsafe { Self::fetch(fetch, entity, table_row) }.foo.0;
println!("Got &mut at {f:p}");
true
}
}
let mut world = World::new();
world.spawn(Foo(0));
world.run_system_once(|query: Query<&Foo, BadFilter>| {
let f: &usize = &query.iter().next().unwrap().0;
println!("Got & at {f:p}");
query.iter().next().unwrap();
println!("Still have & at {f:p}");
});
```
prints:
```
Got &mut at 0x1924b92dfb0
Got & at 0x1924b92dfb0
Got &mut at 0x1924b92dfb0
Still have & at 0x1924b92dfb0
```
Which means it had an `&` and `&mut` alive at the same time.
The only `unsafe` there is around `Self::fetch`, but I believe that call
correctly upholds the safety invariant, and matches what `Added` and
`Changed` do.
## Solution
Make `QueryFilter` an unsafe trait and document the requirement that the
`WorldQuery` implementation be read-only.
## Migration Guide
`QueryFilter` is now an `unsafe trait`. If you were manually
implementing it, you will need to verify that the `WorldQuery`
implementation is read-only and then add the `unsafe` keyword to the
`impl`.
# Objective
Allow `SystemParamBuilder` implementations for custom system parameters
created using `#[derive(SystemParam)]`.
## Solution
Extend the derive macro to accept a `#[system_param(builder)]`
attribute. When present, emit a builder type with a field corresponding
to each field of the param.
## Example
```rust
#[derive(SystemParam)]
#[system_param(builder)]
struct CustomParam<'w, 's> {
query: Query<'w, 's, ()>,
local: Local<'s, usize>,
}
let system = (CustomParamBuilder {
local: LocalBuilder(100),
query: QueryParamBuilder::new(|builder| {
builder.with::<A>();
}),
},)
.build_state(&mut world)
.build_system(|param: CustomParam| *param.local + param.query.iter().count());
```
## Introduction
This is the first step in my [Next Generation Scene / UI
Proposal](https://github.com/bevyengine/bevy/discussions/14437).
Fixes https://github.com/bevyengine/bevy/issues/7272#14800.
Bevy's current Bundles as the "unit of construction" hamstring the UI
user experience and have been a pain point in the Bevy ecosystem
generally when composing scenes:
* They are an additional _object defining_ concept, which must be
learned separately from components. Notably, Bundles _are not present at
runtime_, which is confusing and limiting.
* They can completely erase the _defining component_ during Bundle init.
For example, `ButtonBundle { style: Style::default(), ..default() }`
_makes no mention_ of the `Button` component symbol, which is what makes
the Entity a "button"!
* They are not capable of representing "dependency inheritance" without
completely non-viable / ergonomically crushing nested bundles. This
limitation is especially painful in UI scenarios, but it applies to
everything across the board.
* They introduce a bunch of additional nesting when defining scenes,
making them ugly to look at
* They introduce component name "stutter": `SomeBundle { component_name:
ComponentName::new() }`
* They require copious sprinklings of `..default()` when spawning them
in Rust code, due to the additional layer of nesting
**Required Components** solve this by allowing you to define which
components a given component needs, and how to construct those
components when they aren't explicitly provided.
This is what a `ButtonBundle` looks like with Bundles (the current
approach):
```rust
#[derive(Component, Default)]
struct Button;
#[derive(Bundle, Default)]
struct ButtonBundle {
pub button: Button,
pub node: Node,
pub style: Style,
pub interaction: Interaction,
pub focus_policy: FocusPolicy,
pub border_color: BorderColor,
pub border_radius: BorderRadius,
pub image: UiImage,
pub transform: Transform,
pub global_transform: GlobalTransform,
pub visibility: Visibility,
pub inherited_visibility: InheritedVisibility,
pub view_visibility: ViewVisibility,
pub z_index: ZIndex,
}
commands.spawn(ButtonBundle {
style: Style {
width: Val::Px(100.0),
height: Val::Px(50.0),
..default()
},
focus_policy: FocusPolicy::Block,
..default()
})
```
And this is what it looks like with Required Components:
```rust
#[derive(Component)]
#[require(Node, UiImage)]
struct Button;
commands.spawn((
Button,
Style {
width: Val::Px(100.0),
height: Val::Px(50.0),
..default()
},
FocusPolicy::Block,
));
```
With Required Components, we mention only the most relevant components.
Every component required by `Node` (ex: `Style`, `FocusPolicy`, etc) is
automatically brought in!
### Efficiency
1. At insertion/spawn time, Required Components (including recursive
required components) are initialized and inserted _as if they were
manually inserted alongside the given components_. This means that this
is maximally efficient: there are no archetype or table moves.
2. Required components are only initialized and inserted if they were
not manually provided by the developer. For the code example in the
previous section, because `Style` and `FocusPolicy` are inserted
manually, they _will not_ be initialized and inserted as part of the
required components system. Efficient!
3. The "missing required components _and_ constructors needed for an
insertion" are cached in the "archetype graph edge", meaning they aren't
computed per-insertion. When a component is inserted, the "missing
required components" list is iterated (and that graph edge (AddBundle)
is actually already looked up for us during insertion, because we need
that for "normal" insert logic too).
### IDE Integration
The `#[require(SomeComponent)]` macro has been written in such a way
that Rust Analyzer can provide type-inspection-on-hover and `F12` /
go-to-definition for required components.
### Custom Constructors
The `require` syntax expects a `Default` constructor by default, but it
can be overridden with a custom constructor:
```rust
#[derive(Component)]
#[require(
Node,
Style(button_style),
UiImage
)]
struct Button;
fn button_style() -> Style {
Style {
width: Val::Px(100.0),
..default()
}
}
```
### Multiple Inheritance
You may have noticed by now that this behaves a bit like "multiple
inheritance". One of the problems that this presents is that it is
possible to have duplicate requires for a given type at different levels
of the inheritance tree:
```rust
#[derive(Component)
struct X(usize);
#[derive(Component)]
#[require(X(x1))
struct Y;
fn x1() -> X {
X(1)
}
#[derive(Component)]
#[require(
Y,
X(x2),
)]
struct Z;
fn x2() -> X {
X(2)
}
// What version of X is inserted for Z?
commands.spawn(Z);
```
This is allowed (and encouraged), although this doesn't appear to occur
much in practice. First: only one version of `X` is initialized and
inserted for `Z`. In the case above, I think we can all probably agree
that it makes the most sense to use the `x2` constructor for `X`,
because `Y`'s `x1` constructor exists "beneath" `Z` in the inheritance
hierarchy; `Z`'s constructor is "more specific".
The algorithm is simple and predictable:
1. Use all of the constructors (including default constructors) directly
defined in the spawned component's require list
2. In the order the requires are defined in `#[require()]`, recursively
visit the require list of each of the components in the list (this is a
depth Depth First Search). When a constructor is found, it will only be
used if one has not already been found.
From a user perspective, just think about this as the following:
1. Specifying a required component constructor for `Foo` directly on a
spawned component `Bar` will result in that constructor being used (and
overriding existing constructors lower in the inheritance tree). This is
the classic "inheritance override" behavior people expect.
2. For cases where "multiple inheritance" results in constructor
clashes, Components should be listed in "importance order". List a
component earlier in the requirement list to initialize its inheritance
tree earlier.
Required Components _does_ generally result in a model where component
values are decoupled from each other at construction time. Notably, some
existing Bundle patterns use bundle constructors to initialize multiple
components with shared state. I think (in general) moving away from this
is necessary:
1. It allows Required Components (and the Scene system more generally)
to operate according to simple rules
2. The "do arbitrary init value sharing in Bundle constructors" approach
_already_ causes data consistency problems, and those problems would be
exacerbated in the context of a Scene/UI system. For cases where shared
state is truly necessary, I think we are better served by observers /
hooks.
3. If a situation _truly_ needs shared state constructors (which should
be rare / generally discouraged), Bundles are still there if they are
needed.
## Next Steps
* **Require Construct-ed Components**: I have already implemented this
(as defined in the [Next Generation Scene / UI
Proposal](https://github.com/bevyengine/bevy/discussions/14437). However
I've removed `Construct` support from this PR, as that has not landed
yet. Adding this back in requires relatively minimal changes to the
current impl, and can be done as part of a future Construct pr.
* **Port Built-in Bundles to Required Components**: This isn't something
we should do right away. It will require rethinking our public
interfaces, which IMO should be done holistically after the rest of Next
Generation Scene / UI lands. I think we should merge this PR first and
let people experiment _inside their own code with their own Components_
while we wait for the rest of the new scene system to land.
* **_Consider_ Automatic Required Component Removal**: We should
evaluate _if_ automatic Required Component removal should be done. Ex:
if all components that explicitly require a component are removed,
automatically remove that component. This issue has been explicitly
deferred in this PR, as I consider the insertion behavior to be
desirable on its own (and viable on its own). I am also doubtful that we
can find a design that has behavior we actually want. Aka: can we
_really_ distinguish between a component that is "only there because it
was automatically inserted" and "a component that was necessary / should
be kept". See my [discussion response
here](https://github.com/bevyengine/bevy/discussions/14437#discussioncomment-10268668)
for more details.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: BD103 <59022059+BD103@users.noreply.github.com>
Co-authored-by: Pascal Hertleif <killercup@gmail.com>
# Objective
Fixes#14782
## Solution
Enable the lint and fix all upcoming hints (`--fix`). Also tried to
figure out the false-positive (see review comment). Maybe split this PR
up into multiple parts where only the last one enables the lint, so some
can already be merged resulting in less many files touched / less
potential for merge conflicts?
Currently, there are some cases where it might be easier to read the
code with the qualifier, so perhaps remove the import of it and adapt
its cases? In the current stage it's just a plain adoption of the
suggestions in order to have a base to discuss.
## Testing
`cargo clippy` and `cargo run -p ci` are happy.
# Objective
- Sometimes some method or function takes an owned `Query`, but we don't
want to give up ours;
- transmuting it technically a solution, but it more costly than
necessary.
- Make query iterators more flexible
- this would allow the equivalent of
`slice::split_first`/`slice::split_first_mut` for query iterators
- helps with requests like #14685
## Solution
- Add a way for reborrowing queries, that is going from a `&'a mut
Query<'w, 's, D, F>` to a `Query<'a, 's, D, F>`:
- this is safe because the original query will be borrowed while the new
query exists and thus no aliased access can happen;
- it's basically the equivalent of going from `&'short mut &'long mut T`
to `&'short mut T` the the compiler automatically implements.
- Add a way for getting the remainder of a query iterator:
- this is interesting also because the original iterator keeps its
position, which was not possible before;
- this in turn requires a way to reborrow query fetches, which I had to
add to `WorldQuery`.
## Showcase
- You can now reborrow a `Query`, getting an equivalent `Query` with a
shorter lifetime. Previously this was possible for read-only queries by
using `Query::to_readonly`, now it's possible for mutable queries too;
- You can now separately iterate over the remainder of `QueryIter`.
## Migration Guide
- `WorldQuery` now has an additional `shrink_fetch` method you have to
implement if you were implementing `WorldQuery` manually.
# Objective
- Fix issue #2611
## Solution
- Add `--generate-link-to-definition` to all the `rustdoc-args` arrays
in the `Cargo.toml`s (for docs.rs)
- Add `--generate-link-to-definition` to the `RUSTDOCFLAGS` environment
variable in the docs workflow (for dev-docs.bevyengine.org)
- Document all the workspace crates in the docs workflow (needed because
otherwise only the source code of the `bevy` package will be included,
making the argument useless)
- I think this also fixes#3662, since it fixes the bug on
dev-docs.bevyengine.org, while on docs.rs it has been fixed for a while
on their side.
---
## Changelog
- The source code viewer on docs.rs now includes links to the
definitions.
# Objective
Fixes#14202
## Solution
Add `on_replaced` component hook and `OnReplaced` observer trigger
## Testing
- Did you test these changes? If so, how?
- Updated & added unit tests
---
## Changelog
- Added new `on_replaced` component hook and `OnReplaced` observer
trigger for performing cleanup on component values when they are
overwritten with `.insert()`
# Objective
Add basic bubbling to observers, modeled off `bevy_eventlistener`.
## Solution
- Introduce a new `Traversal` trait for components which point to other
entities.
- Provide a default `TraverseNone: Traversal` component which cannot be
constructed.
- Implement `Traversal` for `Parent`.
- The `Event` trait now has an associated `Traversal` which defaults to
`TraverseNone`.
- Added a field `bubbling: &mut bool` to `Trigger` which can be used to
instruct the runner to bubble the event to the entity specified by the
event's traversal type.
- Added an associated constant `SHOULD_BUBBLE` to `Event` which
configures the default bubbling state.
- Added logic to wire this all up correctly.
Introducing the new associated information directly on `Event` (instead
of a new `BubblingEvent` trait) lets us dispatch both bubbling and
non-bubbling events through the same api.
## Testing
I have added several unit tests to cover the common bugs I identified
during development. Running the unit tests should be enough to validate
correctness. The changes effect unsafe portions of the code, but should
not change any of the safety assertions.
## Changelog
Observers can now bubble up the entity hierarchy! To create a bubbling
event, change your `Derive(Event)` to something like the following:
```rust
#[derive(Component)]
struct MyEvent;
impl Event for MyEvent {
type Traverse = Parent; // This event will propagate up from child to parent.
const AUTO_PROPAGATE: bool = true; // This event will propagate by default.
}
```
You can dispatch a bubbling event using the normal
`world.trigger_targets(MyEvent, entity)`.
Halting an event mid-bubble can be done using
`trigger.propagate(false)`. Events with `AUTO_PROPAGATE = false` will
not propagate by default, but you can enable it using
`trigger.propagate(true)`.
If there are multiple observers attached to a target, they will all be
triggered by bubbling. They all share a bubbling state, which can be
accessed mutably using `trigger.propagation_mut()` (`trigger.propagate`
is just sugar for this).
You can choose to implement `Traversal` for your own types, if you want
to bubble along a different structure than provided by `bevy_hierarchy`.
Implementers must be careful never to produce loops, because this will
cause bevy to hang.
## Migration Guide
+ Manual implementations of `Event` should add associated type `Traverse
= TraverseNone` and associated constant `AUTO_PROPAGATE = false`;
+ `Trigger::new` has new field `propagation: &mut Propagation` which
provides the bubbling state.
+ `ObserverRunner` now takes the same `&mut Propagation` as a final
parameter.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Torstein Grindvik <52322338+torsteingrindvik@users.noreply.github.com>
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
# Objective
- Bevy currently has lot of invalid intra-doc links, let's fix them!
- Also make CI test them, to avoid future regressions.
- Helps with #1983 (but doesn't fix it, as there could still be explicit
links to docs.rs that are broken)
## Solution
- Make `cargo r -p ci -- doc-check` check fail on warnings (could also
be changed to just some specific lints)
- Manually fix all the warnings (note that in some cases it was unclear
to me what the fix should have been, I'll try to highlight them in a
self-review)
Bump version after release
This PR has been auto-generated
Co-authored-by: Bevy Auto Releaser <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: François Mockers <mockersf@gmail.com>
# Objective
Fixes https://github.com/bevyengine/bevy/issues/13972
## Solution
Added 3 new attributes to the `Component` macro.
## Testing
Added `component_hook_order_spawn_despawn_with_macro_hooks`, that makes
the same as `component_hook_order_spawn_despawn` but uses a struct, that
defines it's hooks with the `Component` macro.
---
---------
Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
# Objective
- Provide an expressive way to register dynamic behavior in response to
ECS changes that is consistent with existing bevy types and traits as to
provide a smooth user experience.
- Provide a mechanism for immediate changes in response to events during
command application in order to facilitate improved query caching on the
path to relations.
## Solution
- A new fundamental ECS construct, the `Observer`; inspired by flec's
observers but adapted to better fit bevy's access patterns and rust's
type system.
---
## Examples
There are 3 main ways to register observers. The first is a "component
observer" that looks like this:
```rust
world.observe(|trigger: Trigger<OnAdd, Transform>, query: Query<&Transform>| {
let transform = query.get(trigger.entity()).unwrap();
});
```
The above code will spawn a new entity representing the observer that
will run it's callback whenever the `Transform` component is added to an
entity. This is a system-like function that supports dependency
injection for all the standard bevy types: `Query`, `Res`, `Commands`
etc. It also has a `Trigger` parameter that provides information about
the trigger such as the target entity, and the event being triggered.
Importantly these systems run during command application which is key
for their future use to keep ECS internals up to date. There are similar
events for `OnInsert` and `OnRemove`, and this will be expanded with
things such as `ArchetypeCreated`, `TableEmpty` etc. in follow up PRs.
Another way to register an observer is an "entity observer" that looks
like this:
```rust
world.entity_mut(entity).observe(|trigger: Trigger<Resize>| {
// ...
});
```
Entity observers run whenever an event of their type is triggered
targeting that specific entity. This type of observer will de-spawn
itself if the entity (or entities) it is observing is ever de-spawned so
as to not leave dangling observers.
Entity observers can also be spawned from deferred contexts such as
other observers, systems, or hooks using commands:
```rust
commands.entity(entity).observe(|trigger: Trigger<Resize>| {
// ...
});
```
Observers are not limited to in built event types, they can be used with
any type that implements `Event` (which has been extended to implement
Component). This means events can also carry data:
```rust
#[derive(Event)]
struct Resize { x: u32, y: u32 }
commands.entity(entity).observe(|trigger: Trigger<Resize>, query: Query<&mut Size>| {
let event = trigger.event();
// ...
});
// Will trigger the observer when commands are applied.
commands.trigger_targets(Resize { x: 10, y: 10 }, entity);
```
You can also trigger events that target more than one entity at a time:
```rust
commands.trigger_targets(Resize { x: 10, y: 10 }, [e1, e2]);
```
Additionally, Observers don't _need_ entity targets:
```rust
app.observe(|trigger: Trigger<Quit>| {
})
commands.trigger(Quit);
```
In these cases, `trigger.entity()` will be a placeholder.
Observers are actually just normal entities with an `ObserverState` and
`Observer` component! The `observe()` functions above are just shorthand
for:
```rust
world.spawn(Observer::new(|trigger: Trigger<Resize>| {});
```
This will spawn the `Observer` system and use an `on_add` hook to add
the `ObserverState` component.
Dynamic components and trigger types are also fully supported allowing
for runtime defined trigger types.
## Possible Follow-ups
1. Deprecate `RemovedComponents`, observers should fulfill all use cases
while being more flexible and performant.
2. Queries as entities: Swap queries to entities and begin using
observers listening to archetype creation triggers to keep their caches
in sync, this allows unification of `ObserverState` and `QueryState` as
well as unlocking several API improvements for `Query` and the
management of `QueryState`.
3. Trigger bubbling: For some UI use cases in particular users are
likely to want some form of bubbling for entity observers, this is
trivial to implement naively but ideally this includes an acceleration
structure to cache hierarchy traversals.
4. All kinds of other in-built trigger types.
5. Optimization; in order to not bloat the complexity of the PR I have
kept the implementation straightforward, there are several areas where
performance can be improved. The focus for this PR is to get the
behavior implemented and not incur a performance cost for users who
don't use observers.
I am leaving each of these to follow up PR's in order to keep each of
them reviewable as this already includes significant changes.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: MiniaczQ <xnetroidpl@gmail.com>
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
…izer (#13442)"
This reverts commit 5cfb063d4a.
- This PR broke bevy-trait-query, which needs to be able to write a
resource in init_state. See #13798 for more details.
- Note this doesn't fix everything as transmutes for bevy-trait-query
will still be broken,. But the current usage in that crate is UB, so we
need to find another solution.
# Objective
In #13343, `WorldQuery::get_state` was constrained from `&World` as the
argument to `&Components`, but `WorldQuery::init_state` hasn't yet been
changed from `&mut World` to match.
Fixes#13358
## Solution
Create a wrapper around `&mut Components` and `&mut Storages` that can
be obtained from `&mut World` with a `component_initializer` method.
This new `ComponentInitializer` re-exposes the API on `&mut Components`
minus the `&mut Storages` parameter where it was present. For the
`&Components` API, it simply derefs to its `components` field.
## Changelog
### Added
The `World::component_initializer` method.
The `ComponentInitializer` struct that re-exposes `Components` API.
### Changed
`WorldQuery::init_state` now takes `&mut ComponentInitializer` instead
of `&mut World`.
## Migration Guide
Instead of passing `&mut World` to `WorldQuery::init_state` directly,
pass in a mutable reference to the struct returned from
`World::component_initializer`.
# Objective
Passing `&World` in the `WorldQuery::get_state` method is unnecessary,
as all implementations of this method in the engine either only access
`Components` in `&World`, or do nothing with it.
It can introduce UB by necessitating the creation of a `&World` from a
`UnsafeWorldCell`.
This currently happens in `Query::transmute_lens`, which obtains a
`&World` from the internal `UnsafeWorldCell` solely to pass to
`get_state`. `Query::join` suffers from the same issue.
Other cases of UB come from allowing implementors of `WorldQuery` to
freely access `&World`, like in the `bevy-trait-query` crate, where a
[reference to a resource is
obtained](0c0e7dd646/src/lib.rs (L445))
inside of
[`get_state`](0c0e7dd646/src/one.rs (L245)),
potentially aliasing with a `ResMut` parameter in the same system.
`WorldQuery::init_state` currently requires `&mut World`, which doesn't
suffer from these issues.
But that too can be changed to receive a wrapper around `&mut
Components` and `&mut Storages` for consistency in a follow-up PR.
## Solution
Replace the `&World` parameter in `get_state` with `&Components`.
## Changelog
`WorldQuery::get_state` now takes `&Components` instead of `&World`.
The `transmute`, `transmute_filtered`, `join` and `join_filtered`
methods on `QueryState` now similarly take `&Components` instead of
`&World`.
## Migration Guide
Users of `WorldQuery::get_state` or `transmute`, `transmute_filtered`,
`join` and `join_filtered` methods on `QueryState` now need to pass
`&Components` instead of `&World`.
`&Components` can be trivially obtained from either `components` method
on `&World` or `UnsafeWorldCell`.
For implementors of `WorldQuery::get_state` that were accessing more
than the `Components` inside `&World` and its methods, this is no longer
allowed.
## Summary/Description
This PR extends states to allow support for a wider variety of state
types and patterns, by providing 3 distinct types of state:
- Standard [`States`] can only be changed by manually setting the
[`NextState<S>`] resource. These states are the baseline on which the
other state types are built, and can be used on their own for many
simple patterns. See the [state
example](https://github.com/bevyengine/bevy/blob/latest/examples/ecs/state.rs)
for a simple use case - these are the states that existed so far in
Bevy.
- [`SubStates`] are children of other states - they can be changed
manually using [`NextState<S>`], but are removed from the [`World`] if
the source states aren't in the right state. See the [sub_states
example](https://github.com/lee-orr/bevy/blob/derived_state/examples/ecs/sub_states.rs)
for a simple use case based on the derive macro, or read the trait docs
for more complex scenarios.
- [`ComputedStates`] are fully derived from other states - they provide
a [`compute`](ComputedStates::compute) method that takes in the source
states and returns their derived value. They are particularly useful for
situations where a simplified view of the source states is necessary -
such as having an `InAMenu` computed state derived from a source state
that defines multiple distinct menus. See the [computed state
example](https://github.com/lee-orr/bevy/blob/derived_state/examples/ecs/computed_states.rscomputed_states.rs)
to see a sampling of uses for these states.
# Objective
This PR is another attempt at allowing Bevy to better handle complex
state objects in a manner that doesn't rely on strict equality. While my
previous attempts (https://github.com/bevyengine/bevy/pull/10088 and
https://github.com/bevyengine/bevy/pull/9957) relied on complex matching
capacities at the point of adding a system to application, this one
instead relies on deterministically deriving simple states from more
complex ones.
As a result, it does not require any special macros, nor does it change
any other interactions with the state system once you define and add
your derived state. It also maintains a degree of distinction between
`State` and just normal application state - your derivations have to end
up being discreet pre-determined values, meaning there is less of a
risk/temptation to place a significant amount of logic and data within a
given state.
### Addition - Sub States
closes#9942
After some conversation with Maintainers & SMEs, a significant concern
was that people might attempt to use this feature as if it were
sub-states, and find themselves unable to use it appropriately. Since
`ComputedState` is mainly a state matching feature, while `SubStates`
are more of a state mutation related feature - but one that is easy to
add with the help of the machinery introduced by `ComputedState`, it was
added here as well. The relevant discussion is here:
https://discord.com/channels/691052431525675048/1200556329803186316
## Solution
closes#11358
The solution is to create a new type of state - one implementing
`ComputedStates` - which is deterministically tied to one or more other
states. Implementors write a function to transform the source states
into the computed state, and it gets triggered whenever one of the
source states changes.
In addition, we added the `FreelyMutableState` trait , which is
implemented as part of the derive macro for `States`. This allows us to
limit use of `NextState<S>` to states that are actually mutable,
preventing mis-use of `ComputedStates`.
---
## Changelog
- Added `ComputedStates` trait
- Added `FreelyMutableState` trait
- Converted `NextState` resource to an Enum, with `Unchanged` and
`Pending`
- Added `App::add_computed_state::<S: ComputedStates>()`, to allow for
easily adding derived states to an App.
- Moved the `StateTransition` schedule label from `bevy_app` to
`bevy_ecs` - but maintained the export in `bevy_app` for continuity.
- Modified the process for updating states. Instead of just having an
`apply_state_transition` system that can be added anywhere, we now have
a multi-stage process that has to run within the `StateTransition`
label. First, all the state changes are calculated - manual transitions
rely on `apply_state_transition`, while computed transitions run their
computation process before both call `internal_apply_state_transition`
to apply the transition, send out the transition event, trigger
dependent states, and record which exit/transition/enter schedules need
to occur. Once all the states have been updated, the transition
schedules are called - first the exit schedules, then transition
schedules and finally enter schedules.
- Added `SubStates` trait
- Adjusted `apply_state_transition` to be a no-op if the `State<S>`
resource doesn't exist
## Migration Guide
If the user accessed the NextState resource's value directly or created
them from scratch they will need to adjust to use the new enum variants:
- if they created a `NextState(Some(S))` - they should now use
`NextState::Pending(S)`
- if they created a `NextState(None)` -they should now use
`NextState::Unchanged`
- if they matched on the `NextState` value, they would need to make the
adjustments above
If the user manually utilized `apply_state_transition`, they should
instead use systems that trigger the `StateTransition` schedule.
---
## Future Work
There is still some future potential work in the area, but I wanted to
keep these potential features and changes separate to keep the scope
here contained, and keep the core of it easy to understand and use.
However, I do want to note some of these things, both as inspiration to
others and an illustration of what this PR could unlock.
- `NextState::Remove` - Now that the `State` related mechanisms all
utilize options (#11417), it's fairly easy to add support for explicit
state removal. And while `ComputedStates` can add and remove themselves,
right now `FreelyMutableState`s can't be removed from within the state
system. While it existed originally in this PR, it is a different
question with a separate scope and usability concerns - so having it as
it's own future PR seems like the best approach. This feature currently
lives in a separate branch in my fork, and the differences between it
and this PR can be seen here: https://github.com/lee-orr/bevy/pull/5
- `NextState::ReEnter` - this would allow you to trigger exit & entry
systems for the current state type. We can potentially also add a
`NextState::ReEnterRecirsive` to also re-trigger any states that depend
on the current one.
- More mechanisms for `State` updates - This PR would finally make
states that aren't a set of exclusive Enums useful, and with that comes
the question of setting state more effectively. Right now, to update a
state you either need to fully create the new state, or include the
`Res<Option<State<S>>>` resource in your system, clone the state, mutate
it, and then use `NextState.set(my_mutated_state)` to make it the
pending next state. There are a few other potential methods that could
be implemented in future PRs:
- Inverse Compute States - these would essentially be compute states
that have an additional (manually defined) function that can be used to
nudge the source states so that they result in the computed states
having a given value. For example, you could use set the `IsPaused`
state, and it would attempt to pause or unpause the game by modifying
the `AppState` as needed.
- Closure-based state modification - this would involve adding a
`NextState.modify(f: impl Fn(Option<S> -> Option<S>)` method, and then
you can pass in closures or function pointers to adjust the state as
needed.
- Message-based state modification - this would involve either creating
states that can respond to specific messages, similar to Elm or Redux.
These could either use the `NextState` mechanism or the Event mechanism.
- ~`SubStates` - which are essentially a hybrid of computed and manual
states. In the simplest (and most likely) version, they would work by
having a computed element that determines whether the state should
exist, and if it should has the capacity to add a new version in, but
then any changes to it's content would be freely mutated.~ this feature
is now part of this PR. See above.
- Lastly, since states are getting more complex there might be value in
moving them out of `bevy_ecs` and into their own crate, or at least out
of the `schedule` module into a `states` module. #11087
As mentioned, all these future work elements are TBD and are explicitly
not part of this PR - I just wanted to provide them as potential
explorations for the future.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Marcel Champagne <voiceofmarcel@gmail.com>
Co-authored-by: MiniaczQ <xnetroidpl@gmail.com>
# Objective
Fix#2128. Both `Query::new_archetype` and `SystemParam::new_archetype`
do not check if the `Archetype` comes from the same World the state is
initialized from. This could result in unsoundness via invalid accesses
if called incorrectly.
## Solution
Make them `unsafe` functions and lift the invariant to the caller. This
also caught one instance of us not validating the World in
`SystemState::update_archetypes_unsafe_world_cell`'s implementation.
---
## Changelog
Changed: `QueryState::new_archetype` is now an unsafe function.
Changed: `SystemParam::new_archetype` is now an unsafe function.
## Migration Guide
`QueryState::new_archetype` and `SystemParam::new_archetype` are now an
unsafe functions that must be sure that the provided `Archetype` is from
the same `World` that the state was initialized from. Callers may need
to add additional assertions or propagate the safety invariant upwards
through the callstack to ensure safety.
# Objective
When doing a final pass for #3362, it appeared that `ComponentStorage`
as a trait, the two types implementing it, and the associated type on
`Component` aren't really necessary anymore. This likely was due to an
earlier constraint on the use of consts in traits, but that definitely
doesn't seem to be a problem in Rust 1.76.
## Solution
Remove them.
---
## Changelog
Changed: `Component::Storage` has been replaced with
`Component::STORAGE_TYPE` as a const.
Removed: `bevy::ecs::component::ComponentStorage` trait
Removed: `bevy::ecs::component::TableStorage` struct
Removed: `bevy::ecs::component::SparseSetStorage` struct
## Migration Guide
If you were manually implementing `Component` instead of using the
derive macro, replace the associated `Storage` associated type with the
`STORAGE_TYPE` const:
```rust
// in Bevy 0.13
impl Component for MyComponent {
type Storage = TableStorage;
}
// in Bevy 0.14
impl Component for MyComponent {
const STORAGE_TYPE: StorageType = StorageType::Table;
}
```
Component is no longer object safe. If you were relying on `&dyn
Component`, `Box<dyn Component>`, etc. please [file an issue
](https://github.com/bevyengine/bevy/issues) to get [this
change](https://github.com/bevyengine/bevy/pull/12311) reverted.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Fixes#12016.
Bump version after release
This PR has been auto-generated
Co-authored-by: Bevy Auto Releaser <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: François <mockersf@gmail.com>
[`ScheduleLabel`] derive macro uses "ScheduleName" as the trait name by
mistake. This only affects the error message when a user tries to use
the derive macro on a union type. No other code is affected.
# Objective
Currently the `missing_docs` lint is allowed-by-default and enabled at
crate level when their documentations is complete (see #3492).
This PR proposes to inverse this logic by making `missing_docs`
warn-by-default and mark crates with imcomplete docs allowed.
## Solution
Makes `missing_docs` warn at workspace level and allowed at crate level
when the docs is imcomplete.
# Objective
Expand the existing `Query` API to support more dynamic use cases i.e.
scripting.
## Prior Art
- #6390
- #8308
- #10037
## Solution
- Create a `QueryBuilder` with runtime methods to define the set of
component accesses for a built query.
- Create new `WorldQueryData` implementations `FilteredEntityMut` and
`FilteredEntityRef` as variants of `EntityMut` and `EntityRef` that
provide run time checked access to the components included in a given
query.
- Add new methods to `Query` to create "query lens" with a subset of the
access of the initial query.
### Query Builder
The `QueryBuilder` API allows you to define a query at runtime. At it's
most basic use it will simply create a query with the corresponding type
signature:
```rust
let query = QueryBuilder::<Entity, With<A>>::new(&mut world).build();
// is equivalent to
let query = QueryState::<Entity, With<A>>::new(&mut world);
```
Before calling `.build()` you also have the opportunity to add
additional accesses and filters. Here is a simple example where we add
additional filter terms:
```rust
let entity_a = world.spawn((A(0), B(0))).id();
let entity_b = world.spawn((A(0), C(0))).id();
let mut query_a = QueryBuilder::<Entity>::new(&mut world)
.with::<A>()
.without::<C>()
.build();
assert_eq!(entity_a, query_a.single(&world));
```
This alone is useful in that allows you to decide which archetypes your
query will match at runtime. However it is also very limited, consider a
case like the following:
```rust
let query_a = QueryBuilder::<&A>::new(&mut world)
// Add an additional access
.data::<&B>()
.build();
```
This will grant the query an additional read access to component B
however we have no way of accessing the data while iterating as the type
signature still only includes &A. For an even more concrete example of
this consider dynamic components:
```rust
let query_a = QueryBuilder::<Entity>::new(&mut world)
// Adding a filter is easy since it doesn't need be read later
.with_id(component_id_a)
// How do I access the data of this component?
.ref_id(component_id_b)
.build();
```
With this in mind the `QueryBuilder` API seems somewhat incomplete by
itself, we need some way method of accessing the components dynamically.
So here's one:
### Query Transmutation
If the problem is not having the component in the type signature why not
just add it? This PR also adds transmute methods to `QueryBuilder` and
`QueryState`. Here's a simple example:
```rust
world.spawn(A(0));
world.spawn((A(1), B(0)));
let mut query = QueryBuilder::<()>::new(&mut world)
.with::<B>()
.transmute::<&A>()
.build();
query.iter(&world).for_each(|a| assert_eq!(a.0, 1));
```
The `QueryState` and `QueryBuilder` transmute methods look quite similar
but are different in one respect. Transmuting a builder will always
succeed as it will just add the additional accesses needed for the new
terms if they weren't already included. Transmuting a `QueryState` will
panic in the case that the new type signature would give it access it
didn't already have, for example:
```rust
let query = QueryState::<&A, Option<&B>>::new(&mut world);
/// This is fine, the access for Option<&A> is less restrictive than &A
query.transmute::<Option<&A>>(&world);
/// Oh no, this would allow access to &B on entities that might not have it, so it panics
query.transmute::<&B>(&world);
/// This is right out
query.transmute::<&C>(&world);
```
This is quite an appealing API to also have available on `Query` however
it does pose one additional wrinkle: In order to to change the iterator
we need to create a new `QueryState` to back it. `Query` doesn't own
it's own state though, it just borrows it, so we need a place to borrow
it from. This is why `QueryLens` exists, it is a place to store the new
state so it can be borrowed when you call `.query()` leaving you with an
API like this:
```rust
fn function_that_takes_a_query(query: &Query<&A>) {
// ...
}
fn system(query: Query<(&A, &B)>) {
let lens = query.transmute_lens::<&A>();
let q = lens.query();
function_that_takes_a_query(&q);
}
```
Now you may be thinking: Hey, wait a second, you introduced the problem
with dynamic components and then described a solution that only works
for static components! Ok, you got me, I guess we need a bit more:
### Filtered Entity References
Currently the only way you can access dynamic components on entities
through a query is with either `EntityMut` or `EntityRef`, however these
can access all components and so conflict with all other accesses. This
PR introduces `FilteredEntityMut` and `FilteredEntityRef` as
alternatives that have additional runtime checking to prevent accessing
components that you shouldn't. This way you can build a query with a
`QueryBuilder` and actually access the components you asked for:
```rust
let mut query = QueryBuilder::<FilteredEntityRef>::new(&mut world)
.ref_id(component_id_a)
.with(component_id_b)
.build();
let entity_ref = query.single(&world);
// Returns Some(Ptr) as we have that component and are allowed to read it
let a = entity_ref.get_by_id(component_id_a);
// Will return None even though the entity does have the component, as we are not allowed to read it
let b = entity_ref.get_by_id(component_id_b);
```
For the most part these new structs have the exact same methods as their
non-filtered equivalents.
Putting all of this together we can do some truly dynamic ECS queries,
check out the `dynamic` example to see it in action:
```
Commands:
comp, c Create new components
spawn, s Spawn entities
query, q Query for entities
Enter a command with no parameters for usage.
> c A, B, C, Data 4
Component A created with id: 0
Component B created with id: 1
Component C created with id: 2
Component Data created with id: 3
> s A, B, Data 1
Entity spawned with id: 0v0
> s A, C, Data 0
Entity spawned with id: 1v0
> q &Data
0v0: Data: [1, 0, 0, 0]
1v0: Data: [0, 0, 0, 0]
> q B, &mut Data
0v0: Data: [2, 1, 1, 1]
> q B || C, &Data
0v0: Data: [2, 1, 1, 1]
1v0: Data: [0, 0, 0, 0]
```
## Changelog
- Add new `transmute_lens` methods to `Query`.
- Add new types `QueryBuilder`, `FilteredEntityMut`, `FilteredEntityRef`
and `QueryLens`
- `update_archetype_component_access` has been removed, archetype
component accesses are now determined by the accesses set in
`update_component_access`
- Added method `set_access` to `WorldQuery`, this is called before
`update_component_access` for queries that have a restricted set of
accesses, such as those built by `QueryBuilder` or `QueryLens`. This is
primarily used by the `FilteredEntity*` variants and has an empty trait
implementation.
- Added method `get_state` to `WorldQuery` as a fallible version of
`init_state` when you don't have `&mut World` access.
## Future Work
Improve performance of `FilteredEntityMut` and `FilteredEntityRef`,
currently they have to determine the accesses a query has in a given
archetype during iteration which is far from ideal, especially since we
already did the work when matching the archetype in the first place. To
avoid making more internal API changes I have left it out of this PR.
---------
Co-authored-by: Mike Hsu <mike.hsu@gmail.com>
# Objective
- There is an warning about non snake case on system_param.rs generated
by a macro
## Solution
- Allow non snake case on the function at fault
# Objective
- Shorten paths by removing unnecessary prefixes
## Solution
- Remove the prefixes from many paths which do not need them. Finding
the paths was done automatically using built-in refactoring tools in
Jetbrains RustRover.
# Objective
- Fixes#7680
- This is an updated for https://github.com/bevyengine/bevy/pull/8899
which had the same objective but fell a long way behind the latest
changes
## Solution
The traits `WorldQueryData : WorldQuery` and `WorldQueryFilter :
WorldQuery` have been added and some of the types and functions from
`WorldQuery` has been moved into them.
`ReadOnlyWorldQuery` has been replaced with `ReadOnlyWorldQueryData`.
`WorldQueryFilter` is safe (as long as `WorldQuery` is implemented
safely).
`WorldQueryData` is unsafe - safely implementing it requires that
`Self::ReadOnly` is a readonly version of `Self` (this used to be a
safety requirement of `WorldQuery`)
The type parameters `Q` and `F` of `Query` must now implement
`WorldQueryData` and `WorldQueryFilter` respectively.
This makes it impossible to accidentally use a filter in the data
position or vice versa which was something that could lead to bugs.
~~Compile failure tests have been added to check this.~~
It was previously sometimes useful to use `Option<With<T>>` in the data
position. Use `Has<T>` instead in these cases.
The `WorldQuery` derive macro has been split into separate derive macros
for `WorldQueryData` and `WorldQueryFilter`.
Previously it was possible to derive both `WorldQuery` for a struct that
had a mixture of data and filter items. This would not work correctly in
some cases but could be a useful pattern in others. *This is no longer
possible.*
---
## Notes
- The changes outside of `bevy_ecs` are all changing type parameters to
the new types, updating the macro use, or replacing `Option<With<T>>`
with `Has<T>`.
- All `WorldQueryData` types always returned `true` for `IS_ARCHETYPAL`
so I moved it to `WorldQueryFilter` and
replaced all calls to it with `true`. That should be the only logic
change outside of the macro generation code.
- `Changed<T>` and `Added<T>` were being generated by a macro that I
have expanded. Happy to revert that if desired.
- The two derive macros share some functions for implementing
`WorldQuery` but the tidiest way I could find to implement them was to
give them a ton of arguments and ask clippy to ignore that.
## Changelog
### Changed
- Split `WorldQuery` into `WorldQueryData` and `WorldQueryFilter` which
now have separate derive macros. It is not possible to derive both for
the same type.
- `Query` now requires that the first type argument implements
`WorldQueryData` and the second implements `WorldQueryFilter`
## Migration Guide
- Update derives
```rust
// old
#[derive(WorldQuery)]
#[world_query(mutable, derive(Debug))]
struct CustomQuery {
entity: Entity,
a: &'static mut ComponentA
}
#[derive(WorldQuery)]
struct QueryFilter {
_c: With<ComponentC>
}
// new
#[derive(WorldQueryData)]
#[world_query_data(mutable, derive(Debug))]
struct CustomQuery {
entity: Entity,
a: &'static mut ComponentA,
}
#[derive(WorldQueryFilter)]
struct QueryFilter {
_c: With<ComponentC>
}
```
- Replace `Option<With<T>>` with `Has<T>`
```rust
/// old
fn my_system(query: Query<(Entity, Option<With<ComponentA>>)>)
{
for (entity, has_a_option) in query.iter(){
let has_a:bool = has_a_option.is_some();
//todo!()
}
}
/// new
fn my_system(query: Query<(Entity, Has<ComponentA>)>)
{
for (entity, has_a) in query.iter(){
//todo!()
}
}
```
- Fix queries which had filters in the data position or vice versa.
```rust
// old
fn my_system(query: Query<(Entity, With<ComponentA>)>)
{
for (entity, _) in query.iter(){
//todo!()
}
}
// new
fn my_system(query: Query<Entity, With<ComponentA>>)
{
for entity in query.iter(){
//todo!()
}
}
// old
fn my_system(query: Query<AnyOf<(&ComponentA, With<ComponentB>)>>)
{
for (entity, _) in query.iter(){
//todo!()
}
}
// new
fn my_system(query: Query<Option<&ComponentA>, Or<(With<ComponentA>, With<ComponentB>)>>)
{
for entity in query.iter(){
//todo!()
}
}
```
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
- Fix adding `#![allow(clippy::type_complexity)]` everywhere. like #9796
## Solution
- Use the new [lints] table that will land in 1.74
(https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#lints)
- inherit lint to the workspace, crates and examples.
```
[lints]
workspace = true
```
## Changelog
- Bump rust version to 1.74
- Enable lints table for the workspace
```toml
[workspace.lints.clippy]
type_complexity = "allow"
```
- Allow type complexity for all crates and examples
```toml
[lints]
workspace = true
```
---------
Co-authored-by: Martín Maita <47983254+mnmaita@users.noreply.github.com>
Preparing next release
This PR has been auto-generated
---------
Co-authored-by: Bevy Auto Releaser <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: François <mockersf@gmail.com>
# Objective
First of all, this PR took heavy inspiration from #7760 and #5715. It
intends to also fix#5569, but with a slightly different approach.
This also fixes#9335 by reexporting `DynEq`.
## Solution
The advantage of this API is that we can intern a value without
allocating for zero-sized-types and for enum variants that have no
fields. This PR does this automatically in the `SystemSet` and
`ScheduleLabel` derive macros for unit structs and fieldless enum
variants. So this should cover many internal and external use cases of
`SystemSet` and `ScheduleLabel`. In these optimal use cases, no memory
will be allocated.
- The interning returns a `Interned<dyn SystemSet>`, which is just a
wrapper around a `&'static dyn SystemSet`.
- `Hash` and `Eq` are implemented in terms of the pointer value of the
reference, similar to my first approach of anonymous system sets in
#7676.
- Therefore, `Interned<T>` does not implement `Borrow<T>`, only `Deref`.
- The debug output of `Interned<T>` is the same as the interned value.
Edit:
- `AppLabel` is now also interned and the old
`derive_label`/`define_label` macros were replaced with the new
interning implementation.
- Anonymous set ids are reused for different `Schedule`s, reducing the
amount of leaked memory.
### Pros
- `InternedSystemSet` and `InternedScheduleLabel` behave very similar to
the current `BoxedSystemSet` and `BoxedScheduleLabel`, but can be copied
without an allocation.
- Many use cases don't allocate at all.
- Very fast lookups and comparisons when using `InternedSystemSet` and
`InternedScheduleLabel`.
- The `intern` module might be usable in other areas.
- `Interned{ScheduleLabel, SystemSet, AppLabel}` does implement
`{ScheduleLabel, SystemSet, AppLabel}`, increasing ergonomics.
### Cons
- Implementors of `SystemSet` and `ScheduleLabel` still need to
implement `Hash` and `Eq` (and `Clone`) for it to work.
## Changelog
### Added
- Added `intern` module to `bevy_utils`.
- Added reexports of `DynEq` to `bevy_ecs` and `bevy_app`.
### Changed
- Replaced `BoxedSystemSet` and `BoxedScheduleLabel` with
`InternedSystemSet` and `InternedScheduleLabel`.
- Replaced `impl AsRef<dyn ScheduleLabel>` with `impl ScheduleLabel`.
- Replaced `AppLabelId` with `InternedAppLabel`.
- Changed `AppLabel` to use `Debug` for error messages.
- Changed `AppLabel` to use interning.
- Changed `define_label`/`derive_label` to use interning.
- Replaced `define_boxed_label`/`derive_boxed_label` with
`define_label`/`derive_label`.
- Changed anonymous set ids to be only unique inside a schedule, not
globally.
- Made interned label types implement their label trait.
### Removed
- Removed `define_boxed_label` and `derive_boxed_label`.
## Migration guide
- Replace `BoxedScheduleLabel` and `Box<dyn ScheduleLabel>` with
`InternedScheduleLabel` or `Interned<dyn ScheduleLabel>`.
- Replace `BoxedSystemSet` and `Box<dyn SystemSet>` with
`InternedSystemSet` or `Interned<dyn SystemSet>`.
- Replace `AppLabelId` with `InternedAppLabel` or `Interned<dyn
AppLabel>`.
- Types manually implementing `ScheduleLabel`, `AppLabel` or `SystemSet`
need to implement:
- `dyn_hash` directly instead of implementing `DynHash`
- `as_dyn_eq`
- Pass labels to `World::try_schedule_scope`, `World::schedule_scope`,
`World::try_run_schedule`. `World::run_schedule`, `Schedules::remove`,
`Schedules::remove_entry`, `Schedules::contains`, `Schedules::get` and
`Schedules::get_mut` by value instead of by reference.
---------
Co-authored-by: Joseph <21144246+JoJoJet@users.noreply.github.com>
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
# Objective
The `States::variants` method was once used to construct `OnExit` and
`OnEnter` schedules for every possible value of a given `States` type.
[Since the switch to lazily initialized
schedules](https://github.com/bevyengine/bevy/pull/8028/files#diff-b2fba3a0c86e496085ce7f0e3f1de5960cb754c7d215ed0f087aa556e529f97f),
we no longer need to track every possible value.
This also opens the door to `States` types that aren't enums.
## Solution
- Remove the unused `States::variants` method and its associated type.
- Remove the enum-only restriction on derived States types.
---
## Changelog
- Removed `States::variants` and its associated type.
- Derived `States` can now be datatypes other than enums.
## Migration Guide
- `States::variants` no longer exists. If you relied on this function,
consider using a library that provides enum iterators.
# Objective
Replace instances of
```rust
for x in collection.iter{_mut}() {
```
with
```rust
for x in &{mut} collection {
```
This also changes CI to no longer suppress this lint. Note that since
this lint only shows up when using clippy in pedantic mode, it was
probably unnecessary to suppress this lint in the first place.
# Objective
[Rust 1.72.0](https://blog.rust-lang.org/2023/08/24/Rust-1.72.0.html) is
now stable.
# Notes
- `let-else` formatting has arrived!
- I chose to allow `explicit_iter_loop` due to
https://github.com/rust-lang/rust-clippy/issues/11074.
We didn't hit any of the false positives that prevent compilation, but
fixing this did produce a lot of the "symbol soup" mentioned, e.g. `for
image in &mut *image_events {`.
Happy to undo this if there's consensus the other way.
---------
Co-authored-by: François <mockersf@gmail.com>
# Objective
Cloning a `WorldQuery` type's "fetch" struct was made unsafe in #5593,
by adding the `unsafe fn clone_fetch` to `WorldQuery`. However, as that
method's documentation explains, it is not the right place to put the
safety invariant:
> While calling this method on its own cannot cause UB it is marked
`unsafe` as the caller must ensure that the returned value is not used
in any way that would cause two `QueryItem<Self>` for the same
`archetype_index` or `table_row` to be alive at the same time.
You can clone a fetch struct all you want and it will never cause
undefined behavior -- in order for something to go wrong, you need to
improperly call `WorldQuery::fetch` with it (which is marked unsafe).
Additionally, making it unsafe to clone a fetch struct does not even
prevent undefined behavior, since there are other ways to incorrectly
use a fetch struct. For example, you could just call fetch more than
once for the same entity, which is not currently forbidden by any
documented invariants.
## Solution
Document a safety invariant on `WorldQuery::fetch` that requires the
caller to not create aliased `WorldQueryItem`s for mutable types. Remove
the `clone_fetch` function, and add the bound `Fetch: Clone` instead.
---
## Changelog
- Removed the associated function `WorldQuery::clone_fetch`, and added a
`Clone` bound to `WorldQuery::Fetch`.
## Migration Guide
### `fetch` invariants
The function `WorldQuery::fetch` has had the following safety invariant
added:
> If this type does not implement `ReadOnlyWorldQuery`, then the caller
must ensure that it is impossible for more than one `Self::Item` to
exist for the same entity at any given time.
This invariant was always required for soundness, but was previously
undocumented. If you called this function manually anywhere, you should
check to make sure that this invariant is not violated.
### Removed `clone_fetch`
The function `WorldQuery::clone_fetch` has been removed. The associated
type `WorldQuery::Fetch` now has the bound `Clone`.
Before:
```rust
struct MyFetch<'w> { ... }
unsafe impl WorldQuery for MyQuery {
...
type Fetch<'w> = MyFetch<'w>
unsafe fn clone_fetch<'w>(fetch: &Self::Fetch<'w>) -> Self::Fetch<'w> {
MyFetch {
field1: fetch.field1,
field2: fetch.field2.clone(),
...
}
}
}
```
After:
```rust
#[derive(Clone)]
struct MyFetch<'w> { ... }
unsafe impl WorldQuery for MyQuery {
...
type Fetch<'w> = MyFetch<'w>;
}
```
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
CI-capable version of #9086
---------
Co-authored-by: Bevy Auto Releaser <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: François <mockersf@gmail.com>
# 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.
I created this manually as Github didn't want to run CI for the
workflow-generated PR. I'm guessing we didn't hit this in previous
releases because we used bors.
Co-authored-by: Bevy Auto Releaser <41898282+github-actions[bot]@users.noreply.github.com>
# 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());
```
# 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.
- Supress false positive `redundant_clone` lints.
- Supress inactionable `result_large_err` lint.
Most of the size(50 out of 68 bytes) is coming from
`naga::WithSpan<naga::valid::ValidationError>`
Fixes#8333
# Objective
Fixes issue which causes failure to compile if using
`#![deny(missing_docs)]`.
## Solution
Added some very basic commenting to the generated read-only fields.
honestly I feel this to be up for debate since the comments are very
basic and give very little useful information but the purpose of this PR
is to fix the issue at hand.
---
## Changelog
Added comments to the derive macro and the projects now successfully
compile.
---------
Co-authored-by: lupan <kallll5@hotmail.com>
# Objective
The `#[derive(WorldQuery)]` macro currently only supports structs with
named fields.
Same motivation as #6957. Remove sharp edges from the derive macro, make
it just work more often.
## Solution
Support tuple structs.
---
## Changelog
+ Added support for tuple structs to the `#[derive(WorldQuery)]` macro.
# 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.
