000c362de0
117 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Vic
|
59657ed1e2
|
remove unsound DerefMut impls from EntityHashMap/EntityHashSet (#17450)
# Objective Noticed while doing #17449, I had left these `DerefMut` impls in. Obtaining mutable references to those inner iterator types allows for `mem::swap`, which can be used to swap an incorrectly behaving instance into the wrappers. ## Solution Remove them! |
||
|
Alice Cecile
|
5a9bc28502
|
Support non-Vec data structures in relations (#17447)
# Objective
The existing `RelationshipSourceCollection` uses `Vec` as the only
possible backing for our relationships. While a reasonable choice,
benchmarking use cases might reveal that a different data type is better
or faster.
For example:
- Not all relationships require a stable ordering between the
relationship sources (i.e. children). In cases where we a) have many
such relations and b) don't care about the ordering between them, a hash
set is likely a better datastructure than a `Vec`.
- The number of children-like entities may be small on average, and a
`smallvec` may be faster
## Solution
- Implement `RelationshipSourceCollection` for `EntityHashSet`, our
custom entity-optimized `HashSet`.
-~~Implement `DoubleEndedIterator` for `EntityHashSet` to make things
compile.~~
- This implementation was cursed and very surprising.
- Instead, by moving the iterator type on `RelationshipSourceCollection`
from an erased RPTIT to an explicit associated type we can add a trait
bound on the offending methods!
- Implement `RelationshipSourceCollection` for `SmallVec`
## Testing
I've added a pair of new tests to make sure this pattern compiles
successfully in practice!
## Migration Guide
`EntityHashSet` and `EntityHashMap` are no longer re-exported in
`bevy_ecs::entity` directly. If you were not using `bevy_ecs` / `bevy`'s
`prelude`, you can access them through their now-public modules,
`hash_set` and `hash_map` instead.
## Notes to reviewers
The `EntityHashSet::Iter` type needs to be public for this impl to be
allowed. I initially renamed it to something that wasn't ambiguous and
re-exported it, but as @Victoronz pointed out, that was somewhat
unidiomatic.
In
|
||
|
Zachary Harrold
|
a64446b77e
|
Create bevy_platform_support Crate (#17250)
# Objective - Contributes to #16877 ## Solution - Initial creation of `bevy_platform_support` crate. - Moved `bevy_utils::Instant` into new `bevy_platform_support` crate. - Moved `portable-atomic`, `portable-atomic-util`, and `critical-section` into new `bevy_platform_support` crate. ## Testing - CI --- ## Showcase Instead of needing code like this to import an `Arc`: ```rust #[cfg(feature = "portable-atomic")] use portable_atomic_util::Arc; #[cfg(not(feature = "portable-atomic"))] use alloc::sync::Arc; ``` We can now use: ```rust use bevy_platform_support::sync::Arc; ``` This applies to many other types, but the goal is overall the same: allowing crates to use `std`-like types without the boilerplate of conditional compilation and platform-dependencies. ## Migration Guide - Replace imports of `bevy_utils::Instant` with `bevy_platform_support::time::Instant` - Replace imports of `bevy::utils::Instant` with `bevy::platform_support::time::Instant` ## Notes - `bevy_platform_support` hasn't been reserved on `crates.io` - ~~`bevy_platform_support` is not re-exported from `bevy` at this time. It may be worthwhile exporting this crate, but I am unsure of a reasonable name to export it under (`platform_support` may be a bit wordy for user-facing).~~ - I've included an implementation of `Instant` which is suitable for `no_std` platforms that are not Wasm for the sake of eliminating feature gates around its use. It may be a controversial inclusion, so I'm happy to remove it if required. - There are many other items (`spin`, `bevy_utils::Sync(Unsafe)Cell`, etc.) which should be added to this crate. I have kept the initial scope small to demonstrate utility without making this too unwieldy. --------- Co-authored-by: TimJentzsch <TimJentzsch@users.noreply.github.com> Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com> Co-authored-by: François Mockers <francois.mockers@vleue.com> |
||
|
Carter Anderson
|
21f1e3045c
|
Relationships (non-fragmenting, one-to-many) (#17398)
This adds support for one-to-many non-fragmenting relationships (with planned paths for fragmenting and non-fragmenting many-to-many relationships). "Non-fragmenting" means that entities with the same relationship type, but different relationship targets, are not forced into separate tables (which would cause "table fragmentation"). Functionally, this fills a similar niche as the current Parent/Children system. The biggest differences are: 1. Relationships have simpler internals and significantly improved performance and UX. Commands and specialized APIs are no longer necessary to keep everything in sync. Just spawn entities with the relationship components you want and everything "just works". 2. Relationships are generalized. Bevy can provide additional built in relationships, and users can define their own. **REQUEST TO REVIEWERS**: _please don't leave top level comments and instead comment on specific lines of code. That way we can take advantage of threaded discussions. Also dont leave comments simply pointing out CI failures as I can read those just fine._ ## Built on top of what we have Relationships are implemented on top of the Bevy ECS features we already have: components, immutability, and hooks. This makes them immediately compatible with all of our existing (and future) APIs for querying, spawning, removing, scenes, reflection, etc. The fewer specialized APIs we need to build, maintain, and teach, the better. ## Why focus on one-to-many non-fragmenting first? 1. This allows us to improve Parent/Children relationships immediately, in a way that is reasonably uncontroversial. Switching our hierarchy to fragmenting relationships would have significant performance implications. ~~Flecs is heavily considering a switch to non-fragmenting relations after careful considerations of the performance tradeoffs.~~ _(Correction from @SanderMertens: Flecs is implementing non-fragmenting storage specialized for asset hierarchies, where asset hierarchies are many instances of small trees that have a well defined structure)_ 2. Adding generalized one-to-many relationships is currently a priority for the [Next Generation Scene / UI effort](https://github.com/bevyengine/bevy/discussions/14437). Specifically, we're interested in building reactions and observers on top. ## The changes This PR does the following: 1. Adds a generic one-to-many Relationship system 3. Ports the existing Parent/Children system to Relationships, which now lives in `bevy_ecs::hierarchy`. The old `bevy_hierarchy` crate has been removed. 4. Adds on_despawn component hooks 5. Relationships can opt-in to "despawn descendants" behavior, meaning that the entire relationship hierarchy is despawned when `entity.despawn()` is called. The built in Parent/Children hierarchies enable this behavior, and `entity.despawn_recursive()` has been removed. 6. `world.spawn` now applies commands after spawning. This ensures that relationship bookkeeping happens immediately and removes the need to manually flush. This is in line with the equivalent behaviors recently added to the other APIs (ex: insert). 7. Removes the ValidParentCheckPlugin (system-driven / poll based) in favor of a `validate_parent_has_component` hook. ## Using Relationships The `Relationship` trait looks like this: ```rust pub trait Relationship: Component + Sized { type RelationshipSources: RelationshipSources<Relationship = Self>; fn get(&self) -> Entity; fn from(entity: Entity) -> Self; } ``` A relationship is a component that: 1. Is a simple wrapper over a "target" Entity. 2. Has a corresponding `RelationshipSources` component, which is a simple wrapper over a collection of entities. Every "target entity" targeted by a "source entity" with a `Relationship` has a `RelationshipSources` component, which contains every "source entity" that targets it. For example, the `Parent` component (as it currently exists in Bevy) is the `Relationship` component and the entity containing the Parent is the "source entity". The entity _inside_ the `Parent(Entity)` component is the "target entity". And that target entity has a `Children` component (which implements `RelationshipSources`). In practice, the Parent/Children relationship looks like this: ```rust #[derive(Relationship)] #[relationship(relationship_sources = Children)] pub struct Parent(pub Entity); #[derive(RelationshipSources)] #[relationship_sources(relationship = Parent)] pub struct Children(Vec<Entity>); ``` The Relationship and RelationshipSources derives automatically implement Component with the relevant configuration (namely, the hooks necessary to keep everything in sync). The most direct way to add relationships is to spawn entities with relationship components: ```rust let a = world.spawn_empty().id(); let b = world.spawn(Parent(a)).id(); assert_eq!(world.entity(a).get::<Children>().unwrap(), &[b]); ``` There are also convenience APIs for spawning more than one entity with the same relationship: ```rust world.spawn_empty().with_related::<Children>(|s| { s.spawn_empty(); s.spawn_empty(); }) ``` The existing `with_children` API is now a simpler wrapper over `with_related`. This makes this change largely non-breaking for existing spawn patterns. ```rust world.spawn_empty().with_children(|s| { s.spawn_empty(); s.spawn_empty(); }) ``` There are also other relationship APIs, such as `add_related` and `despawn_related`. ## Automatic recursive despawn via the new on_despawn hook `RelationshipSources` can opt-in to "despawn descendants" behavior, which will despawn all related entities in the relationship hierarchy: ```rust #[derive(RelationshipSources)] #[relationship_sources(relationship = Parent, despawn_descendants)] pub struct Children(Vec<Entity>); ``` This means that `entity.despawn_recursive()` is no longer required. Instead, just use `entity.despawn()` and the relevant related entities will also be despawned. To despawn an entity _without_ despawning its parent/child descendants, you should remove the `Children` component first, which will also remove the related `Parent` components: ```rust entity .remove::<Children>() .despawn() ``` This builds on the on_despawn hook introduced in this PR, which is fired when an entity is despawned (before other hooks). ## Relationships are the source of truth `Relationship` is the _single_ source of truth component. `RelationshipSources` is merely a reflection of what all the `Relationship` components say. By embracing this, we are able to significantly improve the performance of the system as a whole. We can rely on component lifecycles to protect us against duplicates, rather than needing to scan at runtime to ensure entities don't already exist (which results in quadratic runtime). A single source of truth gives us constant-time inserts. This does mean that we cannot directly spawn populated `Children` components (or directly add or remove entities from those components). I personally think this is a worthwhile tradeoff, both because it makes the performance much better _and_ because it means theres exactly one way to do things (which is a philosophy we try to employ for Bevy APIs). As an aside: treating both sides of the relationship as "equivalent source of truth relations" does enable building simple and flexible many-to-many relationships. But this introduces an _inherent_ need to scan (or hash) to protect against duplicates. [`evergreen_relations`](https://github.com/EvergreenNest/evergreen_relations) has a very nice implementation of the "symmetrical many-to-many" approach. Unfortunately I think the performance issues inherent to that approach make it a poor choice for Bevy's default relationship system. ## Followup Work * Discuss renaming `Parent` to `ChildOf`. I refrained from doing that in this PR to keep the diff reasonable, but I'm personally biased toward this change (and using that naming pattern generally for relationships). * [Improved spawning ergonomics](https://github.com/bevyengine/bevy/discussions/16920) * Consider adding relationship observers/triggers for "relationship targets" whenever a source is added or removed. This would replace the current "hierarchy events" system, which is unused upstream but may have existing users downstream. I think triggers are the better fit for this than a buffered event queue, and would prefer not to add that back. * Fragmenting relations: My current idea hinges on the introduction of "value components" (aka: components whose type _and_ value determines their ComponentId, via something like Hashing / PartialEq). By labeling a Relationship component such as `ChildOf(Entity)` as a "value component", `ChildOf(e1)` and `ChildOf(e2)` would be considered "different components". This makes the transition between fragmenting and non-fragmenting a single flag, and everything else continues to work as expected. * Many-to-many support * Non-fragmenting: We can expand Relationship to be a list of entities instead of a single entity. I have largely already written the code for this. * Fragmenting: With the "value component" impl mentioned above, we get many-to-many support "for free", as it would allow inserting multiple copies of a Relationship component with different target entities. Fixes #3742 (If this PR is merged, I think we should open more targeted followup issues for the work above, with a fresh tracking issue free of the large amount of less-directed historical context) Fixes #17301 Fixes #12235 Fixes #15299 Fixes #15308 ## Migration Guide * Replace `ChildBuilder` with `ChildSpawnerCommands`. * Replace calls to `.set_parent(parent_id)` with `.insert(Parent(parent_id))`. * Replace calls to `.replace_children()` with `.remove::<Children>()` followed by `.add_children()`. Note that you'll need to manually despawn any children that are not carried over. * Replace calls to `.despawn_recursive()` with `.despawn()`. * Replace calls to `.despawn_descendants()` with `.despawn_related::<Children>()`. * If you have any calls to `.despawn()` which depend on the children being preserved, you'll need to remove the `Children` component first. --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> |
||
|
MichiRecRoom
|
17c46f4add
|
bevy_ecs: Apply #![warn(clippy::allow_attributes, clippy::allow_attributes_without_reason)] (#17335)
# Objective - https://github.com/bevyengine/bevy/issues/17111 ## Solution Set the `clippy::allow_attributes` and `clippy::allow_attributes_without_reason` lints to `warn`, and bring `bevy_ecs` in line with the new restrictions. ## Testing This PR is a WIP; testing will happen after it's finished. |
||
|
SpecificProtagonist
|
f5d38f30cc
|
Fix entity does not exist message on index reuse (#17264)
# Objective
With the `track_location` feature, the error message of trying to
acquire an entity that was despawned pointed to the wrong line if the
entity index has been reused.
## Showcase
```rust
use bevy_ecs::prelude::*;
fn main() {
let mut world = World::new();
let e = world.spawn_empty().id();
world.despawn(e);
world.flush();
let _ = world.spawn_empty();
world.entity(e);
}
```
Old message:
```
Entity 0v1 was despawned by src/main.rs:8:19
```
New message:
```
Entity 0v1 does not exist (its index has been reused)
```
|
||
|
JaySpruce
|
4fde223831
|
Optimize Entities::entity_does_not_exist_error_details_message, remove UnsafeWorldCell from error (#17115)
## Objective The error `EntityFetchError::NoSuchEntity` has an `UnsafeWorldCell` inside it, which it uses to call `Entities::entity_does_not_exist_error_details_message` when being printed. That method returns a `String` that, if the `track_location` feature is enabled, contains the location of whoever despawned the relevant entity. I initially had to modify this error while working on #17043. The `UnsafeWorldCell` was causing borrow problems when being returned from a command, so I tried replacing it with the `String` that the method returns, since that was the world cell's only purpose. Unfortunately, `String`s are slow, and it significantly impacted performance (on top of that PR's performance hit): <details> <summary>17043 benchmarks</summary> ### With `String`  ### No `String`  </details> For that PR, I just removed the error details entirely, but I figured I'd try to find a way to keep them around. ## Solution - Replace the `String` with a helper struct that holds the location, and only turn it into a string when someone actually wants to print it. - Replace the `UnsafeWorldCell` with the aforementioned struct. - Do the same for `QueryEntityError::NoSuchEntity`. ## Benchmarking This had some interesting performance impact: <details> <summary>This PR vs main</summary>    </details> ## Other work `QueryEntityError::QueryDoesNotMatch` also has an `UnsafeWorldCell` inside it. This one would be more complicated to rework while keeping the same functionality. ## Migration Guide The errors `EntityFetchError::NoSuchEntity` and `QueryEntityError::NoSuchEntity` now contain an `EntityDoesNotExistDetails` struct instead of an `UnsafeWorldCell`. If you were just printing these, they should work identically. --------- Co-authored-by: Benjamin Brienen <benjamin.brienen@outlook.com> |
||
|
Zachary Harrold
|
0403948aa2
|
Remove Implicit std Prelude from no_std Crates (#17086)
# Background In `no_std` compatible crates, there is often an `std` feature which will allow access to the standard library. Currently, with the `std` feature _enabled_, the [`std::prelude`](https://doc.rust-lang.org/std/prelude/index.html) is implicitly imported in all modules. With the feature _disabled_, instead the [`core::prelude`](https://doc.rust-lang.org/core/prelude/index.html) is implicitly imported. This creates a subtle and pervasive issue where `alloc` items _may_ be implicitly included (if `std` is enabled), or must be explicitly included (if `std` is not enabled). # Objective - Make the implicit imports for `no_std` crates consistent regardless of what features are/not enabled. ## Solution - Replace the `cfg_attr` "double negative" `no_std` attribute with conditional compilation to _include_ `std` as an external crate. ```rust // Before #![cfg_attr(not(feature = "std"), no_std)] // After #![no_std] #[cfg(feature = "std")] extern crate std; ``` - Fix imports that are currently broken but are only now visible with the above fix. ## Testing - CI ## Notes I had previously used the "double negative" version of `no_std` based on general consensus that it was "cleaner" within the Rust embedded community. However, this implicit prelude issue likely was considered when forming this consensus. I believe the reason why is the items most affected by this issue are provided by the `alloc` crate, which is rarely used within embedded but extensively used within Bevy. |
||
|
Sean Kim
|
294e0db719
|
Rename track_change_detection flag to track_location (#17075)
# Objective - As stated in the related issue, this PR is to better align the feature flag name with what it actually does and the plans for the future. - Fixes #16852 ## Solution - Simple find / replace ## Testing - Local run of `cargo run -p ci` ## Migration Guide The `track_change_detection` feature flag has been renamed to `track_location` to better reflect its extended capabilities. |
||
|
Benjamin Brienen
|
847c3a1719
|
Fix random clippy warning (#17010)
# Objective Follow-up to #16984 ## Solution Fix the lint ## Testing ``` PS C:\Users\BenjaminBrienen\source\bevy> cargo clippy Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.71s PS C:\Users\BenjaminBrienen\source\bevy> cargo clippy -p bevy_ecs Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.21s ``` |
||
|
Benjamin Brienen
|
1675d68366
|
Fix beta CI (#16984)
# Objective Fixes #16607 ## Solution Satisfy clippy ## Testing Ran clippy |
||
|
Vic
|
8ac90ac542
|
make EntityHashMap and EntityHashSet proper types (#16912)
# Objective `EntityHashMap` and `EntityHashSet` iterators do not implement `EntitySetIterator`. ## Solution Make them newtypes instead of aliases. The methods that create the iterators can then produce their own newtypes that carry the `Hasher` generic and implement `EntitySetIterator`. Functionality remains the same otherwise. There are some other small benefits, f.e. the removal of `with_hasher` associated functions, and the ability to implement more traits ourselves. `MainEntityHashMap` and `MainEntityHashSet` are currently left as the previous type aliases, because supporting general `TrustedEntityBorrow` hashing is more complex. However, it can also be done. ## Testing Pre-existing `EntityHashMap` tests. ## Migration Guide Users of `with_hasher` and `with_capacity_and_hasher` on `EntityHashMap`/`Set` must now use `new` and `with_capacity` respectively. If the non-newtyped versions are required, they can be obtained via `Deref`, `DerefMut` or `into_inner` calls. |
||
|
Zachary Harrold
|
21786632c3
|
Remove bevy_core (#16897)
# Objective - Fixes #16892 ## Solution - Removed `TypeRegistryPlugin` (`Name` is now automatically registered with a default `App`) - Moved `TaskPoolPlugin` to `bevy_app` - Moved `FrameCountPlugin` to `bevy_diagnostic` - Deleted now-empty `bevy_core` ## Testing - CI ## Migration Guide - `TypeRegistryPlugin` no longer exists. If you can't use a default `App` but still need `Name` registered, do so manually with `app.register_type::<Name>()`. - References to `TaskPoolPlugin` and associated types will need to import it from `bevy_app` instead of `bevy_core` - References to `FrameCountPlugin` and associated types will need to import it from `bevy_diagnostic` instead of `bevy_core` ## Notes This strategy was agreed upon by Cart and several other members in [Discord](https://discord.com/channels/691052431525675048/692572690833473578/1319137218312278077). |
||
|
eugineerd
|
20049d4c34
|
Faster entity cloning (#16717)
# Objective #16132 introduced entity cloning functionality, and while it works and is useful, it can be made faster. This is the promised follow-up to improve performance. ## Solution **PREFACE**: This is my first time writing `unsafe` in rust and I have only vague idea about what I'm doing. I would encourage reviewers to scrutinize `unsafe` parts in particular. The solution is to clone component data to an intermediate buffer and use `EntityWorldMut::insert_by_ids` to insert components without additional archetype moves. To facilitate this, `EntityCloner::clone_entity` now reads all components of the source entity and provides clone handlers with the ability to read component data straight from component storage using `read_source_component` and write to an intermediate buffer using `write_target_component`. `ComponentId` is used to check that requested type corresponds to the type available on source entity. Reflect-based handler is a little trickier to pull of: we only have `&dyn Reflect` and no direct access to the underlying data. `ReflectFromPtr` can be used to get `&dyn Reflect` from concrete component data, but to write it we need to create a clone of the underlying data using `Reflect`. For this reason only components that have `ReflectDefault` or `ReflectFromReflect` or `ReflectFromWorld` can be cloned, all other components will be skipped. The good news is that this is actually only a temporary limitation: once #13432 lands we will be able to clone component without requiring one of these `type data`s. This PR also introduces `entity_cloning` benchmark to better compare changes between the PR and main, you can see the results in the **showcase** section. ## Testing - All previous tests passing - Added test for fast reflect clone path (temporary, will be removed after reflection-based cloning lands) - Ran miri ## Showcase Here's a table demonstrating the improvement: | **benchmark** | **main, avg** | **PR, avg** | **change, avg** | | ----------------------- | ------------- | ----------- | --------------- | | many components reflect | 18.505 µs | 2.1351 µs | -89.095% | | hierarchy wide reflect* | 22.778 ms | 4.1875 ms | -81.616% | | hierarchy tall reflect* | 107.24 µs | 26.322 µs | -77.141% | | hierarchy many reflect | 78.533 ms | 9.7415 ms | -87.596% | | many components clone | 1.3633 µs | 758.17 ns | -45.937% | | hierarchy wide clone* | 2.7716 ms | 3.3411 ms | +20.546% | | hierarchy tall clone* | 17.646 µs | 20.190 µs | +17.379% | | hierarchy many clone | 5.8779 ms | 4.2650 ms | -27.439% | *: these benchmarks have entities with only 1 component ## Considerations Once #10154 is resolved a large part of the functionality in this PR will probably become obsolete. It might still be a little bit faster than using command batching, but the complexity might not be worth it. ## Migration Guide - `&EntityCloner` in component clone handlers is changed to `&mut ComponentCloneCtx` to better separate data. - Changed `EntityCloneHandler` from enum to struct and added convenience functions to add default clone and reflect handler more easily. --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com> |
||
|
Vic
|
a4b89d0d5e
|
implement EntitySet and iter_many_unique methods (#16547)
# Objective
In current Bevy, it is very inconvenient to mutably retrieve a
user-provided list of entities more than one element at a time.
If the list contains any duplicate entities, we risk mutable aliasing.
Users of `Query::iter_many_mut` do not have access to `Iterator` trait,
and thus miss out on common functionality, for instance collecting their
`QueryManyIter`.
We can circumvent this issue with validation, however that entails
checking every entity against all others for inequality, or utilizing an
`EntityHashSet`. Even if an entity list remains unchanged, this
validation is/would have to be redone every time we wish to fetch with
the list.
This presents a lot of wasted work, as we often trivially know an entity
list to be unique f.e.: `QueryIter` will fetch every `Entity` once and
only once.
As more things become entities – assets, components, queries – this
issue will become more pronounced.
`get_many`/`many`/`iter_many`/`par_iter_many`-like functionality is all
affected.
## Solution
The solution this PR proposes is to introduce functionality built around
a new trait: `EntitySet`.
The goal is to preserve the property of "uniqueness" in a list wherever
possible, and then rely on it as a bound within new `*_many_unique`
methods to avoid the need for validation.
This is achieved using `Iterator`:
`EntitySet` is blanket implemented for any `T` that implements
`IntoIterator<IntoIter: EntitySetIterator>`.
`EntitySetIterator` is the unsafe trait that actually guarantees an
iterator to be "unique" via its safety contract.
We define an "Iterator over unique entities" as: "No two entities
returned by the iterator may compare equal."
For iterators that cannot return more than 1 element, this is trivially
true.
Whether an iterator can satisfy this is up to the `EntitySetIterator`
implementor to ensure, hence the unsafe.
However, this is not yet a complete solution. Looking at the signature
of `iter_many`, we find that `IntoIterator::Item` is not `Entity`, but
is instead bounded by the `Borrow<Entity>` trait. That is because
iteration without consuming the collection will often yield us
references, not owned items.
`Borrow<Entity>` presents an issue: The `Borrow` docs state that `x = y`
should equal `x.borrow() = y.borrow()`, but unsafe cannot rely on this
for soundness. We run into similar problems with other trait
implementations of any `Borrow<Entity>` type: `PartialEq`, `Eq`,
`PartialOrd`, `Ord`, `Hash`, `Clone`, `Borrow`, and `BorrowMut`.
This PR solves this with the unsafe `TrustedEntityBorrow` trait:
Any implementor promises that the behavior of the aforementioned traits
matches that of the underlying entity.
While `Borrow<Entity>` was the inspiration, we use our own counterpart
trait `EntityBorrow` as the supertrait to `TrustedEntityBorrow`, so we
can circumvent the limitations of the existing `Borrow<T>` blanket
impls.
All together, these traits allow us to implement `*_many_unique`
functionality with a lone `EntitySet` bound.
`EntitySetIterator` is implemented for all the std iterators and
iterator adapters that guarantee or preserve uniqueness, so we can
filter, skip, take, step, reverse, ... our unique entity iterators
without worry!
Sadly, current `HashSet` iterators do not carry the necessary type
information with them to determine whether the source `HashSet` produces
logic errors; A malicious `Hasher` could compromise a `HashSet`.
`HashSet` iteration is generally discouraged in the first place, so we
also exclude the set operation iterators, even though they do carry the
`Hasher` type parameter.
`BTreeSet` implements `EntitySet` without any problems.
If an iterator type cannot guarantee uniqueness at compile time, then a
user can still attach `EntitySetIterator` to an individual instance of
that type via `UniqueEntityIter::from_iterator_unchecked`.
With this, custom types can use `UniqueEntityIter<I>` as their
`IntoIterator::IntoIter` type, if necessary.
This PR is focused on the base concept, and expansions on it are left
for follow-up PRs. See "Potential Future Work" below.
## Testing
Doctests on `iter_many_unique`/`iter_many_unique_mut` + 2 tests in
entity_set.rs.
## Showcase
```rust
// Before:
fn system(player_list: Res<SomeUniquePlayerList>, players: Query<&mut Player>) {
let value = 0;
while let Some(player) = players.iter_many_mut(player_list).fetch_next() {
value += mem::take(player.value_mut())
}
}
// After:
fn system(player_list: Res<SomeUniquePlayerList>, players: Query<&mut Player>) {
let value = players
.iter_many_unique_mut(player_list)
.map(|player| mem::take(player.value_mut()))
.sum();
}
```
## Changelog
- added `EntityBorrow`, `TrustedEntityBorrow`, `EntitySet` and
`EntitySetIterator` traits
- added `iter_many_unique`, `iter_many_unique_mut`,
`iter_many_unique_unsafe` methods on `Query`
- added `iter_many_unique`, `iter_many_unique_mut`,
`iter_many_unique_manual` and `iter_many_unique_unchecked_manual`
methods on `QueryState`
- added corresponding `QueryManyUniqueIter`
- added `UniqueEntityIter`
## Migration Guide
Any custom type used as a `Borrow<Entity>` entity list item for an
`iter_many` method now has to implement `EntityBorrow` instead. Any type
that implements `Borrow<Entity>` can trivially implement `EntityBorrow`.
## Potential Future Work
- `ToEntitySet` trait for converting any entity iterator into an
`EntitySetIterator`
- `EntityIndexSet/Map` to tie in hashing with `EntitySet`
- add `EntityIndexSetSlice/MapSlice`
- requires: `EntityIndexSet/Map`
- Implementing `par_iter_many_unique_mut` for parallel mutable iteration
- requires: `par_iter_many`
- allow collecting into `UniqueEntityVec` to store entity sets
- add `UniqueEntitySlice`s
- Doesn't require, but should be done after: `UniqueEntityVec`
- add `UniqueEntityArray`s
- Doesn't require, but should be done after: `UniqueEntitySlice`
- `get_many_unique`/`many_unique` methods
- requires: `UniqueEntityArray`
- `World::entity_unique` to match `World::entity` methods
- Doesn't require, but makes sense after:
`get_many_unique`/`many_unique`
- implement `TrustedEntityBorrow` for the `EntityRef` family
- Doesn't require, but makes sense after: `UniqueEntityVec`
|
||
|
Zachary Harrold
|
1f2d0e6308
|
Add no_std support to bevy_ecs (#16758)
# Objective - Contributes to #15460 ## Solution - Added the following features: - `std` (default) - `async_executor` (default) - `edge_executor` - `critical-section` - `portable-atomic` - Gated `tracing` in `bevy_utils` to allow compilation on certain platforms - Switched from `tracing` to `log` for simple message logging within `bevy_ecs`. Note that `tracing` supports capturing from `log` so this should be an uncontroversial change. - Fixed imports and added feature gates as required - Made `bevy_tasks` optional within `bevy_ecs`. Turns out it's only needed for parallel operations which are already gated behind `multi_threaded` anyway. ## Testing - Added to `compile-check-no-std` CI command - `cargo check -p bevy_ecs --no-default-features --features edge_executor,critical-section,portable-atomic --target thumbv6m-none-eabi` - `cargo check -p bevy_ecs --no-default-features --features edge_executor,critical-section` - `cargo check -p bevy_ecs --no-default-features` ## Draft Release Notes Bevy's core ECS now supports `no_std` platforms. In prior versions of Bevy, it was not possible to work with embedded or niche platforms due to our reliance on the standard library, `std`. This has blocked a number of novel use-cases for Bevy, such as an embedded database for IoT devices, or for creating games on retro consoles. With this release, `bevy_ecs` no longer requires `std`. To use Bevy on a `no_std` platform, you must disable default features and enable the new `edge_executor` and `critical-section` features. You may also need to enable `portable-atomic` and `critical-section` if your platform does not natively support all atomic types and operations used by Bevy. ```toml [dependencies] bevy_ecs = { version = "0.16", default-features = false, features = [ # Required for platforms with incomplete atomics (e.g., Raspberry Pi Pico) "portable-atomic", "critical-section", # Optional "bevy_reflect", "serialize", "bevy_debug_stepping", "edge_executor" ] } ``` Currently, this has been tested on bare-metal x86 and the Raspberry Pi Pico. If you have trouble using `bevy_ecs` on a particular platform, please reach out either through a GitHub issue or in the `no_std` working group on the Bevy Discord server. Keep an eye out for future `no_std` updates as we continue to improve the parity between `std` and `no_std`. We look forward to seeing what kinds of applications are now possible with Bevy! ## Notes - Creating PR in draft to ensure CI is passing before requesting reviews. - This implementation has no support for multithreading in `no_std`, especially due to `NonSend` being unsound if allowed in multithreading. The reason is we cannot check the `ThreadId` in `no_std`, so we have no mechanism to at-runtime determine if access is sound. --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: Vic <59878206+Victoronz@users.noreply.github.com> |
||
|
SpecificProtagonist
|
21195a75e6
|
track_change_detection: Also track spawns/despawns (#16047)
# Objective
Expand `track_change_detection` feature to also track entity spawns and
despawns. Use this to create better error messages.
# Solution
Adds `Entities::entity_get_spawned_or_despawned_by` as well as `{all
entity reference types}::spawned_by`.
This also removes the deprecated `get_many_entities_mut` & co (and
therefore can't land in 0.15) because we don't yet have no Polonius.
## Testing
Added a test that checks that the locations get updated and these
updates are ordered correctly vs hooks & observers.
---
## Showcase
Access location:
```rust
let mut world = World::new();
let entity = world.spawn_empty().id();
println!("spawned by: {}", world.entity(entity).spawned_by());
```
```
spawned by: src/main.rs:5:24
```
Error message (with `track_change_detection`):
```rust
world.despawn(entity);
world.entity(entity);
```
```
thread 'main' panicked at src/main.rs:11:11:
Entity 0v1#4294967296 was despawned by src/main.rs:10:11
```
and without:
```
thread 'main' panicked at src/main.rs:11:11:
Entity 0v1#4294967296 does not exist (enable `track_change_detection` feature for more details)
```
Similar error messages now also exists for `Query::get`,
`World::entity_mut`, `EntityCommands` creation and everything that
causes `B0003`, e.g.
```
error[B0003]: Could not insert a bundle (of type `MaterialMeshBundle<StandardMaterial>`) for entity Entity { index: 7, generation: 1 }, which was despawned by src/main.rs:10:11. See: https://bevyengine.org/learn/errors/#b0003
```
---------
Co-authored-by: kurk070ff <108901106+kurk070ff@users.noreply.github.com>
Co-authored-by: Freya Pines <freya@MacBookAir.lan>
Co-authored-by: Freya Pines <freya@Freyas-MacBook-Air.local>
Co-authored-by: Matty Weatherley <weatherleymatthew@gmail.com>
|
||
|
JaySpruce
|
5a94beb239
|
Extend cloning functionality and add convenience methods to EntityWorldMut and EntityCommands (#16826)
## Objective Thanks to @eugineerd's work on entity cloning (#16132), we now have a robust way to copy components between entities. We can extend this to implement some useful functionality that would have been more complicated before. Closes #15350. ## Solution `EntityCloneBuilder` now automatically includes required components alongside any component added/removed from the component filter. Added the following methods to `EntityCloneBuilder`: - `move_components` - `without_required_components` Added the following methods to `EntityWorldMut` and `EntityCommands`: - `clone_with` - `clone_components` - `move_components` Also added `clone_and_spawn` and `clone_and_spawn_with` to `EntityWorldMut` (`EntityCommands` already had them). ## Showcase ``` assert_eq!(world.entity(entity_a).get::<B>(), Some(&B)); assert_eq!(world.entity(entity_b).get::<B>(), None); world.entity_mut(entity_a).clone_components::<B>(entity_b); assert_eq!(world.entity(entity_a).get::<B>(), Some(&B)); assert_eq!(world.entity(entity_b).get::<B>(), Some(&B)); assert_eq!(world.entity(entity_a).get::<C>(), Some(&C(5))); assert_eq!(world.entity(entity_b).get::<C>(), None); world.entity_mut(entity_a).move_components::<C>(entity_b); assert_eq!(world.entity(entity_a).get::<C>(), None); assert_eq!(world.entity(entity_b).get::<C>(), Some(&C(5))); ``` |
||
|
Clar Fon
|
711246aa34
|
Update hashbrown to 0.15 (#15801)
Updating dependencies; adopted version of #15696. (Supercedes #15696.) Long answer: hashbrown is no longer using ahash by default, meaning that we can't use the default-hasher methods with ahasher. So, we have to use the longer-winded versions instead. This takes the opportunity to also switch our default hasher as well, but without actually enabling the default-hasher feature for hashbrown, meaning that we'll be able to change our hasher more easily at the cost of all of these method calls being obnoxious forever. One large change from 0.15 is that `insert_unique_unchecked` is now `unsafe`, and for cases where unsafe code was denied at the crate level, I replaced it with `insert`. ## Migration Guide `bevy_utils` has updated its version of `hashbrown` to 0.15 and now defaults to `foldhash` instead of `ahash`. This means that if you've hard-coded your hasher to `bevy_utils::AHasher` or separately used the `ahash` crate in your code, you may need to switch to `foldhash` to ensure that everything works like it does in Bevy. |
||
|
Nuutti Kotivuori
|
76d610d465
|
Flush commands after every mutation in WorldEntityMut (#16219)
# Objective - Currently adding observers spawns an entity which implicitly flushes the command queue, which can cause undefined behaviour if the `WorldEntityMut` is used after this - The reason `WorldEntityMut` attempted to (unsuccessfully) avoid flushing commands until finished was that such commands may move or despawn the entity being referenced, invalidating the cached location. - With the introduction of hooks and observers, this isn't sensible anymore as running the commands generated by hooks immediately is required to maintain correct ordering of operations and to not expose the world in an inconsistent state - Objective is to make command flushing deterministic and fix the related issues - Fixes #16212 - Fixes #14621 - Fixes #16034 ## Solution - Allow `WorldEntityMut` to exist even when it refers to a despawned entity by allowing `EntityLocation` to be marked invalid - Add checks to all methods to panic if trying to access a despawned entity - Flush command queue after every operation that might trigger hooks or observers - Update entity location always after flushing command queue ## Testing - Added test cases for currently broken behaviour - Added test cases that flushes happen in all operations - Added test cases to ensure hooks and commands are run exactly in correct order when nested --- Todo: - [x] Write migration guide - [x] Add tests that using `EntityWorldMut` on a despawned entity panics - [x] Add tests that commands are flushed after every operation that is supposed to flush them - [x] Add tests that hooks, observers and their spawned commands are run in the correct order when nested --- ## Migration Guide Previously `EntityWorldMut` triggered command queue flushes in unpredictable places, which could interfere with hooks and observers. Now the command queue is flushed always immediately after any call in `EntityWorldMut` that spawns or despawns an entity, or adds, removes or replaces a component. This means hooks and observers will run their commands in the correct order. As a side effect, there is a possibility that a hook or observer could despawn the entity that is being referred to by `EntityWorldMut`. This could already currently happen if an observer was added while keeping an `EntityWorldMut` referece and would cause unsound behaviour. If the entity has been despawned, calling any methods which require the entity location will panic. This matches the behaviour that `Commands` will panic if called on an already despawned entity. In the extremely rare case where taking a new `EntityWorldMut` reference or otherwise restructuring the code so that this case does not happen is not possible, there's a new `is_despawned` method that can be used to check if the referred entity has been despawned. |
||
|
Zachary Harrold
|
a35811d088
|
Add Immutable Component Support (#16372)
# Objective - Fixes #16208 ## Solution - Added an associated type to `Component`, `Mutability`, which flags whether a component is mutable, or immutable. If `Mutability= Mutable`, the component is mutable. If `Mutability= Immutable`, the component is immutable. - Updated `derive_component` to default to mutable unless an `#[component(immutable)]` attribute is added. - Updated `ReflectComponent` to check if a component is mutable and, if not, panic when attempting to mutate. ## Testing - CI - `immutable_components` example. --- ## Showcase Users can now mark a component as `#[component(immutable)]` to prevent safe mutation of a component while it is attached to an entity: ```rust #[derive(Component)] #[component(immutable)] struct Foo { // ... } ``` This prevents creating an exclusive reference to the component while it is attached to an entity. This is particularly powerful when combined with component hooks, as you can now fully track a component's value, ensuring whatever invariants you desire are upheld. Before this would be done my making a component private, and manually creating a `QueryData` implementation which only permitted read access. <details> <summary>Using immutable components as an index</summary> ```rust /// This is an example of a component like [`Name`](bevy::prelude::Name), but immutable. #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Component)] #[component( immutable, on_insert = on_insert_name, on_replace = on_replace_name, )] pub struct Name(pub &'static str); /// This index allows for O(1) lookups of an [`Entity`] by its [`Name`]. #[derive(Resource, Default)] struct NameIndex { name_to_entity: HashMap<Name, Entity>, } impl NameIndex { fn get_entity(&self, name: &'static str) -> Option<Entity> { self.name_to_entity.get(&Name(name)).copied() } } fn on_insert_name(mut world: DeferredWorld<'_>, entity: Entity, _component: ComponentId) { let Some(&name) = world.entity(entity).get::<Name>() else { unreachable!() }; let Some(mut index) = world.get_resource_mut::<NameIndex>() else { return; }; index.name_to_entity.insert(name, entity); } fn on_replace_name(mut world: DeferredWorld<'_>, entity: Entity, _component: ComponentId) { let Some(&name) = world.entity(entity).get::<Name>() else { unreachable!() }; let Some(mut index) = world.get_resource_mut::<NameIndex>() else { return; }; index.name_to_entity.remove(&name); } // Setup our name index world.init_resource::<NameIndex>(); // Spawn some entities! let alyssa = world.spawn(Name("Alyssa")).id(); let javier = world.spawn(Name("Javier")).id(); // Check our index let index = world.resource::<NameIndex>(); assert_eq!(index.get_entity("Alyssa"), Some(alyssa)); assert_eq!(index.get_entity("Javier"), Some(javier)); // Changing the name of an entity is also fully capture by our index world.entity_mut(javier).insert(Name("Steven")); // Javier changed their name to Steven let steven = javier; // Check our index let index = world.resource::<NameIndex>(); assert_eq!(index.get_entity("Javier"), None); assert_eq!(index.get_entity("Steven"), Some(steven)); ``` </details> Additionally, users can use `Component<Mutability = ...>` in trait bounds to enforce that a component _is_ mutable or _is_ immutable. When using `Component` as a trait bound without specifying `Mutability`, any component is applicable. However, methods which only work on mutable or immutable components are unavailable, since the compiler must be pessimistic about the type. ## Migration Guide - When implementing `Component` manually, you must now provide a type for `Mutability`. The type `Mutable` provides equivalent behaviour to earlier versions of `Component`: ```rust impl Component for Foo { type Mutability = Mutable; // ... } ``` - When working with generic components, you may need to specify that your generic parameter implements `Component<Mutability = Mutable>` rather than `Component` if you require mutable access to said component. - The entity entry API has had to have some changes made to minimise friction when working with immutable components. Methods which previously returned a `Mut<T>` will now typically return an `OccupiedEntry<T>` instead, requiring you to add an `into_mut()` to get the `Mut<T>` item again. ## Draft Release Notes Components can now be made immutable while stored within the ECS. Components are the fundamental unit of data within an ECS, and Bevy provides a number of ways to work with them that align with Rust's rules around ownership and borrowing. One part of this is hooks, which allow for defining custom behavior at key points in a component's lifecycle, such as addition and removal. However, there is currently no way to respond to _mutation_ of a component using hooks. The reasons for this are quite technical, but to summarize, their addition poses a significant challenge to Bevy's core promises around performance. Without mutation hooks, it's relatively trivial to modify a component in such a way that breaks invariants it intends to uphold. For example, you can use `core::mem::swap` to swap the components of two entities, bypassing the insertion and removal hooks. This means the only way to react to this modification is via change detection in a system, which then begs the question of what happens _between_ that alteration and the next run of that system? Alternatively, you could make your component private to prevent mutation, but now you need to provide commands and a custom `QueryData` implementation to allow users to interact with your component at all. Immutable components solve this problem by preventing the creation of an exclusive reference to the component entirely. Without an exclusive reference, the only way to modify an immutable component is via removal or replacement, which is fully captured by component hooks. To make a component immutable, simply add `#[component(immutable)]`: ```rust #[derive(Component)] #[component(immutable)] struct Foo { // ... } ``` When implementing `Component` manually, there is an associated type `Mutability` which controls this behavior: ```rust impl Component for Foo { type Mutability = Mutable; // ... } ``` Note that this means when working with generic components, you may need to specify that a component is mutable to gain access to certain methods: ```rust // Before fn bar<C: Component>() { // ... } // After fn bar<C: Component<Mutability = Mutable>>() { // ... } ``` With this new tool, creating index components, or caching data on an entity should be more user friendly, allowing libraries to provide APIs relying on components and hooks to uphold their invariants. ## Notes - ~~I've done my best to implement this feature, but I'm not happy with how reflection has turned out. If any reflection SMEs know a way to improve this situation I'd greatly appreciate it.~~ There is an outstanding issue around the fallibility of mutable methods on `ReflectComponent`, but the DX is largely unchanged from `main` now. - I've attempted to prevent all safe mutable access to a component that does not implement `Component<Mutability = Mutable>`, but there may still be some methods I have missed. Please indicate so and I will address them, as they are bugs. - Unsafe is an escape hatch I am _not_ attempting to prevent. Whatever you do with unsafe is between you and your compiler. - I am marking this PR as ready, but I suspect it will undergo fairly major revisions based on SME feedback. - I've marked this PR as _Uncontroversial_ based on the feature, not the implementation. --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: Benjamin Brienen <benjamin.brienen@outlook.com> Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com> Co-authored-by: Nuutti Kotivuori <naked@iki.fi> |
||
|
SpecificProtagonist
|
1a6b94c5e8
|
Remove flush_and_reserve_invalid_assuming_no_entities (#16460)
# Objective `flush_and_reserve_invalid_assuming_no_entities` was made for the old rendering world (which was reset every frame) and is usused since the 0.15 retained rendering world, but wasn't removed yet. It is pub, but is undocumented apart from the safety comment. ## Solution Remove `flush_and_reserve_invalid_assuming_no_entities` and the safety invariants this method required for `EntityMeta`, `EntityLocation`, `TableId` and `TableRow`. This reduces the amount of unsafe code & safety invariants and makes #16047 easier. ## Alternatives - Document `flush_and_reserve_invalid_assuming_no_entities` and keep it unchanged - Document `flush_and_reserve_invalid_assuming_no_entities` and change it to be based on `EntityMeta::INVALID` ## Migration Guide - exchange `Entities::flush_and_reserve_invalid_assuming_no_entities` for `reserve` and `flush_as_invalid` and notify us if that's insufficient --------- Co-authored-by: Benjamin Brienen <benjamin.brienen@outlook.com> |
||
|
eugineerd
|
2e267bba5a
|
Entity cloning (#16132)
## Objective Fixes #1515 This PR implements a flexible entity cloning system. The primary use case for it is to clone dynamically-generated entities. Example: ```rs #[derive(Component, Clone)] pub struct Projectile; #[derive(Component, Clone)] pub struct Damage { value: f32, } fn player_input( mut commands: Commands, projectiles: Query<Entity, With<Projectile>>, input: Res<ButtonInput<KeyCode>>, ) { // Fire a projectile if input.just_pressed(KeyCode::KeyF) { commands.spawn((Projectile, Damage { value: 10.0 })); } // Triplicate all active projectiles if input.just_pressed(KeyCode::KeyT) { for projectile in projectiles.iter() { // To triplicate a projectile we need to create 2 more clones for _ in 0..2{ commands.clone_entity(projectile) } } } } ``` ## Solution ### Commands Add a `clone_entity` command to create a clone of an entity with all components that can be cloned. Components that can't be cloned will be ignored. ```rs commands.clone_entity(entity) ``` If there is a need to configure the cloning process (like set to clone recursively), there is a second command: ```rs commands.clone_entity_with(entity, |builder| { builder.recursive(true) }); ``` Both of these commands return `EntityCommands` of the cloned entity, so the copy can be modified afterwards. ### Builder All these commands use `EntityCloneBuilder` internally. If there is a need to clone an entity using `World` instead, it is also possible: ```rs let entity = world.spawn(Component).id(); let entity_clone = world.spawn_empty().id(); EntityCloneBuilder::new(&mut world).clone_entity(entity, entity_clone); ``` Builder has methods to `allow` or `deny` certain components during cloning if required and can be extended by implementing traits on it. This PR includes two `EntityCloneBuilder` extensions: `CloneEntityWithObserversExt` to configure adding cloned entity to observers of the original entity, and `CloneEntityRecursiveExt` to configure cloning an entity recursively. ### Clone implementations By default, all components that implement either `Clone` or `Reflect` will be cloned (with `Clone`-based implementation preferred in case component implements both). This can be overriden on a per-component basis: ```rs impl Component for SomeComponent { const STORAGE_TYPE: StorageType = StorageType::Table; fn get_component_clone_handler() -> ComponentCloneHandler { // Don't clone this component ComponentCloneHandler::Ignore } } ``` ### `ComponentCloneHandlers` Clone implementation specified in `get_component_clone_handler` will get registered in `ComponentCloneHandlers` (stored in `bevy_ecs::component::Components`) at component registration time. The clone handler implementation provided by a component can be overriden after registration like so: ```rs let component_id = world.components().component_id::<Component>().unwrap() world.get_component_clone_handlers_mut() .set_component_handler(component_id, ComponentCloneHandler::Custom(component_clone_custom)) ``` The default clone handler for all components that do not explicitly define one (or don't derive `Component`) is `component_clone_via_reflect` if `bevy_reflect` feature is enabled, and `component_clone_ignore` (noop) otherwise. Default handler can be overriden using `ComponentCloneHandlers::set_default_handler` ### Handlers Component clone handlers can be used to modify component cloning behavior. The general signature for a handler that can be used in `ComponentCloneHandler::Custom` is as follows: ```rs pub fn component_clone_custom( world: &mut DeferredWorld, entity_cloner: &EntityCloner, ) { // implementation } ``` The `EntityCloner` implementation (used internally by `EntityCloneBuilder`) assumes that after calling this custom handler, the `target` entity has the desired version of the component from the `source` entity. ### Builder handler overrides Besides component-defined and world-overriden handlers, `EntityCloneBuilder` also has a way to override handlers locally. It is mainly used to allow configuration methods like `recursive` and `add_observers`. ```rs // From observer clone handler implementation impl CloneEntityWithObserversExt for EntityCloneBuilder<'_> { fn add_observers(&mut self, add_observers: bool) -> &mut Self { if add_observers { self.override_component_clone_handler::<ObservedBy>(ComponentCloneHandler::Custom( component_clone_observed_by, )) } else { self.remove_component_clone_handler_override::<ObservedBy>() } } } ``` ## Testing Includes some basic functionality tests and doctests. Performance-wise this feature is the same as calling `clone` followed by `insert` for every entity component. There is also some inherent overhead due to every component clone handler having to access component data through `World`, but this can be reduced without breaking current public API in a later PR. |
||
|
Rob Parrett
|
30d84519a2
|
Use en-us locale for typos (#16037)
# Objective Bevy seems to want to standardize on "American English" spellings. Not sure if this is laid out anywhere in writing, but see also #15947. While perusing the docs for `typos`, I noticed that it has a `locale` config option and tried it out. ## Solution Switch to `en-us` locale in the `typos` config and run `typos -w` ## Migration Guide The following methods or fields have been renamed from `*dependants*` to `*dependents*`. - `ProcessorAssetInfo::dependants` - `ProcessorAssetInfos::add_dependant` - `ProcessorAssetInfos::non_existent_dependants` - `AssetInfo::dependants_waiting_on_load` - `AssetInfo::dependants_waiting_on_recursive_dep_load` - `AssetInfos::loader_dependants` - `AssetInfos::remove_dependants_and_labels` |
||
|
Gino Valente
|
da4e7769ad
|
bevy_ecs: Special-case Entity::PLACEHOLDER formatting (#15839)
# Objective
Oftentimes, users will store an entity on a component or resource. To
make this component/resource `Default`-able, they might initialize it
with `Entity::PLACEHOLDER`. This is sometimes done to avoid the need for
an `Option<Entity>`, especially if it complicates other logic.
For example, it's used in this `Selection` resource to denote "no
selection":
```rust
#[derive(Resource, Debug)]
struct Selection(Entity);
impl Default for Selection {
fn default() -> Self {
Self(Entity::PLACEHOLDER)
}
}
```
The problem is that if we try to `Debug` the current `Selection`, we get
back: `4294967295v1#8589934591`. It's not immediately obvious whether or
not the entity is an actual entity or the placeholder.
Now while it doesn't take long to realize that this is in fact just the
value of `Entity::PLACEHOLDER`, it would be a lot clearer if this was
made explicit, especially for these particular use cases.
## Solution
This PR makes the `Debug` and `Display` impls for `Entity` return
`PLACEHOLDER` for the `Entity::PLACEHOLDER` constant.
~~Feel free to bikeshed the actual value returned here. I think
`PLACEHOLDER` on its own could work too.~~ Swapped to `PLACEHOLDER` from
`Entity::PLACEHOLDER`.
## Testing
You can test locally by running:
```
cargo test --package bevy_ecs
```
---
## Migration Guide
The `Debug` and `Display` impls for `Entity` now return `PLACEHOLDER`
for the `Entity::PLACEHOLDER` constant. If you had any code relying on
these values, you may need to account for this change.
|
||
|
Josh Robson Chase
|
40e88dceff
|
Change ReflectMapEntities to operate on components before insertion (#15422)
Previous PR https://github.com/bevyengine/bevy/pull/14549 was closed in error and couldn't be reopened since I had updated the branch 😿 # Objective Fixes #14465 ## Solution `ReflectMapEntities` now works similarly to `MapEntities` in that it works on the reflected value itself rather than the component in the world after insertion. This makes it so that observers see the remapped entities on insertion rather than the entity IDs from the scene. `ReflectMapEntities` now works for both components and resources, so we only need the one. ## Testing * New unit test for `Observer`s + `DynamicScene`s * New unit test for `Observer`s + `Scene`s * Open to suggestions for other tests! --- ## Migration Guide - Consumers of `ReflectMapEntities` will need to call `map_entities` on values prior to inserting them into the world. - Implementors of `MapEntities` will need to remove the `mappings` method, which is no longer needed for `ReflectMapEntities` and has been removed from the trait. --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: Hennadii Chernyshchyk <genaloner@gmail.com> |
||
|
Josh Robson Chase
|
f97eba2082
|
Add VisitEntities for generic and reflectable Entity iteration (#15425)
# Objective - Provide a generic and _reflectable_ way to iterate over contained entities ## Solution Adds two new traits: * `VisitEntities`: Reflectable iteration, accepts a closure rather than producing an iterator. Implemented by default for `IntoIterator` implementing types. A proc macro is also provided. * A `Mut` variant of the above. Its derive macro uses the same field attribute to avoid repetition. ## Testing Added a test for `VisitEntities` that also transitively tests its derive macro as well as the default `MapEntities` impl. |
||
|
Zachary Harrold
|
d70595b667
|
Add core and alloc over std Lints (#15281)
# Objective - Fixes #6370 - Closes #6581 ## Solution - Added the following lints to the workspace: - `std_instead_of_core` - `std_instead_of_alloc` - `alloc_instead_of_core` - Used `cargo +nightly fmt` with [item level use formatting](https://rust-lang.github.io/rustfmt/?version=v1.6.0&search=#Item%5C%3A) to split all `use` statements into single items. - Used `cargo clippy --workspace --all-targets --all-features --fix --allow-dirty` to _attempt_ to resolve the new linting issues, and intervened where the lint was unable to resolve the issue automatically (usually due to needing an `extern crate alloc;` statement in a crate root). - Manually removed certain uses of `std` where negative feature gating prevented `--all-features` from finding the offending uses. - Used `cargo +nightly fmt` with [crate level use formatting](https://rust-lang.github.io/rustfmt/?version=v1.6.0&search=#Crate%5C%3A) to re-merge all `use` statements matching Bevy's previous styling. - Manually fixed cases where the `fmt` tool could not re-merge `use` statements due to conditional compilation attributes. ## Testing - Ran CI locally ## Migration Guide The MSRV is now 1.81. Please update to this version or higher. ## Notes - This is a _massive_ change to try and push through, which is why I've outlined the semi-automatic steps I used to create this PR, in case this fails and someone else tries again in the future. - Making this change has no impact on user code, but does mean Bevy contributors will be warned to use `core` and `alloc` instead of `std` where possible. - This lint is a critical first step towards investigating `no_std` options for Bevy. --------- Co-authored-by: François Mockers <francois.mockers@vleue.com> |
||
|
Josh Robson Chase
|
fcddb54ce5
|
Fix for SceneEntityMapper + hooks panic (#15405)
# Objective - Add a test case for #14300 Fixes #14300 ## Solution `SceneEntityMapper` relies on operations on `Entities` that require flushing in advance, such as `alloc` and `free`. Previously, it wasn't calling `world.flush_entities()` itself and relied on its caller having flushed beforehand. This wasn't an issue before observers and hooks were released, since entity reservation was happening at expected times. Now that hooks and observers are a thing, they can introduce a need to flush. We have a few options: * Flush after each observer/hook run * Flush between each paired observer/hook and operation that requires a flush * Flush before operations requiring it The first option for this case seemed trickier to reason about than I wanted, since it involved the `BundleInserter` and its `UnsafeWorldCell`, and the second is generally harder to track down. The third seemed the most straightforward and conventional, since we can see a flush occurring at the start of a number of `World` methods. Therefore, we're letting `SceneEntityMapper` be in charge of upholding its own invariants and calling `flush_entities` when it's created. ## Testing Added a new test case modeled after #14300 |
||
|
Clar Fon
|
efda7f3f9c
|
Simpler lint fixes: makes ci lints work but disables a lint for now (#15376)
Takes the first two commits from #15375 and adds suggestions from this comment: https://github.com/bevyengine/bevy/pull/15375#issuecomment-2366968300 See #15375 for more reasoning/motivation. ## Rebasing (rerunning) ```rust git switch simpler-lint-fixes git reset --hard main cargo fmt --all -- --unstable-features --config normalize_comments=true,imports_granularity=Crate cargo fmt --all git add --update git commit --message "rustfmt" cargo clippy --workspace --all-targets --all-features --fix cargo fmt --all -- --unstable-features --config normalize_comments=true,imports_granularity=Crate cargo fmt --all git add --update git commit --message "clippy" git cherry-pick e6c0b94f6795222310fb812fa5c4512661fc7887 ``` |
||
|
Gino Valente
|
83356b12c9
|
bevy_reflect: Replace "value" terminology with "opaque" (#15240)
# Objective Currently, the term "value" in the context of reflection is a bit overloaded. For one, it can be used synonymously with "data" or "variable". An example sentence would be "this function takes a reflected value". However, it is also used to refer to reflected types which are `ReflectKind::Value`. These types are usually either primitives, opaque types, or types that don't fall into any other `ReflectKind` (or perhaps could, but don't due to some limitation/difficulty). An example sentence would be "this function takes a reflected value type". This makes it difficult to write good documentation or other learning material without causing some amount of confusion to readers. Ideally, we'd be able to move away from the `ReflectKind::Value` usage and come up with a better term. ## Solution This PR replaces the terminology of "value" with "opaque" across `bevy_reflect`. This includes in documentation, type names, variant names, and macros. The term "opaque" was chosen because that's essentially how the type is treated within the reflection API. In other words, its internal structure is hidden. All we can do is work with the type itself. ### Primitives While primitives are not technically opaque types, I think it's still clearer to refer to them as "opaque" rather than keep the confusing "value" terminology. We could consider adding another concept for primitives (e.g. `ReflectKind::Primitive`), but I'm not sure that provides a lot of benefit right now. In most circumstances, they'll be treated just like an opaque type. They would also likely use the same macro (or two copies of the same macro but with different names). ## Testing You can test locally by running: ``` cargo test --package bevy_reflect --all-features ``` --- ## Migration Guide The reflection concept of "value type" has been replaced with a clearer "opaque type". The following renames have been made to account for this: - `ReflectKind::Value` → `ReflectKind::Opaque` - `ReflectRef::Value` → `ReflectRef::Opaque` - `ReflectMut::Value` → `ReflectMut::Opaque` - `ReflectOwned::Value` → `ReflectOwned::Opaque` - `TypeInfo::Value` → `TypeInfo::Opaque` - `ValueInfo` → `OpaqueInfo` - `impl_reflect_value!` → `impl_reflect_opaque!` - `impl_from_reflect_value!` → `impl_from_reflect_opaque!` Additionally, declaring your own opaque types no longer uses `#[reflect_value]`. This attribute has been replaced by `#[reflect(opaque)]`: ```rust // BEFORE #[derive(Reflect)] #[reflect_value(Default)] struct MyOpaqueType(u32); // AFTER #[derive(Reflect)] #[reflect(opaque)] #[reflect(Default)] struct MyOpaqueType(u32); ``` Note that the order in which `#[reflect(opaque)]` appears does not matter. |
||
|
Robert Walter
|
5484d2d6f8
|
Choose more descriptive field names for ReserveEntitiesIterator (#15168)
No hard feelings if you don't want to make this change. This is just something I stumbled over in my very first read of the `bevy_ecs` crate. # Objective - the general goal here is to improve DX slightly - make the code easier to read in general. The previous names make the code harder to read, especially since they are so similar. ## Solution - choose more specific names for the fields - `index_iter` -> `freelist_indices` : "freelist" is a well established term in the rest of the docs in this module, so we might want to reuse it - `index_range` -> `new_indices` : Nothing besides the doc comment stated that these indices were actually new/fresh ## Testing Note that the fields are private so that this is no breaking change. They are also only used in this one module. |
||
|
Benjamin Brienen
|
1b8c1c1242
|
simplify std::mem references (#15315)
# Objective - Fixes #15314 ## Solution - Remove unnecessary usings and simplify references to those functions. ## Testing CI |
||
|
Federico Rinaldi
|
262b068bc3
|
Substitute trivial fallible conversions with infallible function calls (#10846)
Clippy for rust 1.75 will introduce the [`unnecessary_fallible_conversions`][1] lint. This PR solves the trivial ones. [1]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_fallible_conversions |
||
|
Zachary Harrold
|
bc13161416
|
Migrated NonZero* to NonZero<*> (#14978)
# Objective - Fixes #14974 ## Solution - Replace all* instances of `NonZero*` with `NonZero<*>` ## Testing - CI passed locally. --- ## Notes Within the `bevy_reflect` implementations for `std` types, `impl_reflect_value!()` will continue to use the type aliases instead, as it inappropriately parses the concrete type parameter as a generic argument. If the `ZeroablePrimitive` trait was stable, or the macro could be modified to accept a finite list of types, then we could fully migrate. |
||
|
Carter Anderson
|
9cdb915809
|
Required Components (#14791)
## 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> |
||
|
EdJoPaTo
|
938d810766
|
Apply unused_qualifications lint (#14828)
# 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. |
||
|
Zeenobit
|
023e0e5bde
|
Fix Entity Debug Format (#14539)
# Objective Fixes #12139 ## Solution See this comment on original issue for my proposal: https://github.com/bevyengine/bevy/issues/12139#issuecomment-2241915791 This PR is an implementation of this proposal. I modified the implementation of `fmt::Debug` to instead display `0v0#12345` to ensure entity index, generation, and raw bits are all present in the output for debug purposes while still keeping log message concise. `fmt::Display` remains as is (`0v0`) to offer an even shorter output. To me, this is the most non-intrusive fix for this issue. ## Testing Add `fn entity_debug` test --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com> |
||
|
Giacomo Stevanato
|
d7080369a7
|
Fix intra-doc links and make CI test them (#14076)
# 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) |
||
|
Alice Cecile
|
52e5ad5da7
|
Don't show .to_bits in Display impl for Entity (#14011)
# Objective #12469 changed the `Debug` impl for `Entity`, making sure it's actually accurate for debugging. To ensure that its can still be readily logged in error messages and inspectors, this PR added a more concise and human-friendly `Display` impl. However, users found this form too verbose: the `to_bits` information was unhelpful and too long. Fixes #13980. ## Solution - Don't include `Entity::to_bits` in the `Display` implementation for `Entity`. This information can readily be accessed and logged for users who need it. - Also clean up the implementation of `Display` for `DebugName`, introduced in https://github.com/bevyengine/bevy/pull/13760, to simply use the new `Display` impl (since this was the desired format there). ## Testing I've updated an existing test to verify the output of `Entity::display`. --------- Co-authored-by: Kristoffer Søholm <k.soeholm@gmail.com> |
||
|
Periwink
|
93f3432400
|
Update serialize flag for bevy_ecs (#13740)
# Objective There were some issues with the `serialize` feature: - `bevy_app` had a `serialize` feature and a dependency on `serde` even there is no usage of serde at all inside `bevy_app` - the `bevy_app/serialize` feature enabled `bevy_ecs/serde`, which is strange - `bevy_internal/serialize` did not enable `bevy_app/serialize` so there was no way of serializing an Entity in bevy 0.14 ## Solution - Remove `serde` and `bevy_app/serialize` - Add a `serialize` flag on `bevy_ecs` that enables `serde` - ` bevy_internal/serialize` now enables `bevy_ecs/serialize` |
||
|
Zachary Harrold
|
3bfc427666
|
Add mappings to EntityMapper (#13727)
# Objective - Fixes #13703 ## Solution - Added `mappings` to the `EntityMapper` trait, which returns an iterator over currently tracked `Entity` to `Entity` mappings. - Added `DynEntityMapper` as an [object safe](https://doc.rust-lang.org/reference/items/traits.html#object-safety) alternative to `EntityMapper`. - Added `assert_object_safe` as a helper for ensuring traits are object safe. ## Testing - Added new unit test `entity_mapper_iteration` which tests the `SceneEntityMapper` implementation of `EntityMapper::mappings`. - Added unit tests to ensure `DynEntityMapper`, `DynEq` and `DynHash` are object safe. - Passed CI on my Windows 10 development environment --- ## Changelog - Added `mappings` to `EntityMapper` trait. ## Migration Guide - If you are implementing `EntityMapper` yourself, you can use the below as a stub implementation: ```rust fn mappings(&self) -> impl Iterator<Item = (Entity, Entity)> { unimplemented!() } ``` - If you were using `EntityMapper` as a trait object (`dyn EntityMapper`), instead use `dyn DynEntityMapper` and its associated methods. ## Notes - The original issue proposed returning a `Vec` from `EntityMapper` instead of an `impl Iterator` to preserve its object safety. This is a simpler option, but also forces an allocation where it isn't strictly needed. I've opted for this split into `DynEntityMapper` and `EntityMapper` as it's been done several times across Bevy already, and provides maximum flexibility to users. - `assert_object_safe` is an empty function, since the assertion actually happens once you try to use a `dyn T` for some trait `T`. I have still added this function to clearly document what object safety is within Bevy, and to create a standard way to communicate that a given trait must be object safe. - Other traits should have tests added to ensure object safety, but I've left those off to avoid cluttering this PR further. --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> |
||
|
Alice Cecile
|
ec7b3490f6
|
Add on_unimplemented Diagnostics to Most Public Traits (#13347) (#13662)
# Objective - #13414 did not have the intended effect. - #13404 is still blocked ## Solution - Re-adds #13347. Co-authored-by: Zachary Harrold <zac@harrold.com.au> Co-authored-by: Jamie Ridding <Themayu@users.noreply.github.com> Co-authored-by: BD103 <59022059+BD103@users.noreply.github.com> |
||
|
Dmytro Banin
|
7307d76fb3
|
Make SceneEntityMapper constructor/destructor public (#13630)
# Objective `SceneEntityMapper` seems like it could be generally useful. ## Solution Allow end users to call `SceneEntityMapper::new` and `SceneEntityMapper::finish`. |
||
|
Alice Cecile
|
ee6dfd35c9
|
Revert "Add on_unimplemented Diagnostics to Most Public Traits" (#13413)
# Objective - Rust 1.78 breaks all Android support, see https://github.com/bevyengine/bevy/issues/13331 - We should not bump the MSRV to 1.78 until that's resolved in #13366. ## Solution - Temporarily revert https://github.com/bevyengine/bevy/pull/13347 Co-authored-by: Alice Cecile <alice.i.cecil@gmail.com> |
||
|
Zachary Harrold
|
11f0a2dcde
|
Add on_unimplemented Diagnostics to Most Public Traits (#13347)
# Objective - Fixes #12377 ## Solution Added simple `#[diagnostic::on_unimplemented(...)]` attributes to some critical public traits providing a more approachable initial error message. Where appropriate, a `note` is added indicating that a `derive` macro is available. ## Examples <details> <summary>Examples hidden for brevity</summary> Below is a collection of examples showing the new error messages produced by this change. In general, messages will start with a more Bevy-centric error message (e.g., _`MyComponent` is not a `Component`_), and a note directing the user to an available derive macro where appropriate. ### Missing `#[derive(Resource)]` <details> <summary>Example Code</summary> ```rust use bevy::prelude::*; struct MyResource; fn main() { App::new() .insert_resource(MyResource) .run(); } ``` </details> <details> <summary>Error Generated</summary> ```error error[E0277]: `MyResource` is not a `Resource` --> examples/app/empty.rs:7:26 | 7 | .insert_resource(MyResource) | --------------- ^^^^^^^^^^ invalid `Resource` | | | required by a bound introduced by this call | = help: the trait `Resource` is not implemented for `MyResource` = note: consider annotating `MyResource` with `#[derive(Resource)]` = help: the following other types implement trait `Resource`: AccessibilityRequested ManageAccessibilityUpdates bevy::bevy_a11y::Focus DiagnosticsStore FrameCount bevy::prelude::State<S> SystemInfo bevy::prelude::Axis<T> and 141 others note: required by a bound in `bevy::prelude::App::insert_resource` --> C:\Users\Zac\Documents\GitHub\bevy\crates\bevy_app\src\app.rs:419:31 | 419 | pub fn insert_resource<R: Resource>(&mut self, resource: R) -> &mut Self { | ^^^^^^^^ required by this bound in `App::insert_resource` ``` </details> ### Putting A `QueryData` in a `QueryFilter` Slot <details> <summary>Example Code</summary> ```rust use bevy::prelude::*; #[derive(Component)] struct A; #[derive(Component)] struct B; fn my_system(_query: Query<&A, &B>) {} fn main() { App::new() .add_systems(Update, my_system) .run(); } ``` </details> <details> <summary>Error Generated</summary> ```error error[E0277]: `&B` is not a valid `Query` filter --> examples/app/empty.rs:9:22 | 9 | fn my_system(_query: Query<&A, &B>) {} | ^^^^^^^^^^^^^ invalid `Query` filter | = help: the trait `QueryFilter` is not implemented for `&B` = help: the following other types implement trait `QueryFilter`: With<T> Without<T> bevy::prelude::Or<()> bevy::prelude::Or<(F0,)> bevy::prelude::Or<(F0, F1)> bevy::prelude::Or<(F0, F1, F2)> bevy::prelude::Or<(F0, F1, F2, F3)> bevy::prelude::Or<(F0, F1, F2, F3, F4)> and 28 others note: required by a bound in `bevy::prelude::Query` --> C:\Users\Zac\Documents\GitHub\bevy\crates\bevy_ecs\src\system\query.rs:349:51 | 349 | pub struct Query<'world, 'state, D: QueryData, F: QueryFilter = ()> { | ^^^^^^^^^^^ required by this bound in `Query` ``` </details> ### Missing `#[derive(Component)]` <details> <summary>Example Code</summary> ```rust use bevy::prelude::*; struct A; fn my_system(mut commands: Commands) { commands.spawn(A); } fn main() { App::new() .add_systems(Startup, my_system) .run(); } ``` </details> <details> <summary>Error Generated</summary> ```error error[E0277]: `A` is not a `Bundle` --> examples/app/empty.rs:6:20 | 6 | commands.spawn(A); | ----- ^ invalid `Bundle` | | | required by a bound introduced by this call | = help: the trait `bevy::prelude::Component` is not implemented for `A`, which is required by `A: Bundle` = note: consider annotating `A` with `#[derive(Component)]` or `#[derive(Bundle)]` = help: the following other types implement trait `Bundle`: TransformBundle SceneBundle DynamicSceneBundle AudioSourceBundle<Source> SpriteBundle SpriteSheetBundle Text2dBundle MaterialMesh2dBundle<M> and 34 others = note: required for `A` to implement `Bundle` note: required by a bound in `bevy::prelude::Commands::<'w, 's>::spawn` --> C:\Users\Zac\Documents\GitHub\bevy\crates\bevy_ecs\src\system\commands\mod.rs:243:21 | 243 | pub fn spawn<T: Bundle>(&mut self, bundle: T) -> EntityCommands { | ^^^^^^ required by this bound in `Commands::<'w, 's>::spawn` ``` </details> ### Missing `#[derive(Asset)]` <details> <summary>Example Code</summary> ```rust use bevy::prelude::*; struct A; fn main() { App::new() .init_asset::<A>() .run(); } ``` </details> <details> <summary>Error Generated</summary> ```error error[E0277]: `A` is not an `Asset` --> examples/app/empty.rs:7:23 | 7 | .init_asset::<A>() | ---------- ^ invalid `Asset` | | | required by a bound introduced by this call | = help: the trait `Asset` is not implemented for `A` = note: consider annotating `A` with `#[derive(Asset)]` = help: the following other types implement trait `Asset`: Font AnimationGraph DynamicScene Scene AudioSource Pitch bevy::bevy_gltf::Gltf GltfNode and 17 others note: required by a bound in `init_asset` --> C:\Users\Zac\Documents\GitHub\bevy\crates\bevy_asset\src\lib.rs:307:22 | 307 | fn init_asset<A: Asset>(&mut self) -> &mut Self; | ^^^^^ required by this bound in `AssetApp::init_asset` ``` </details> ### Mismatched Input and Output on System Piping <details> <summary>Example Code</summary> ```rust use bevy::prelude::*; fn producer() -> u32 { 123 } fn consumer(_: In<u16>) {} fn main() { App::new() .add_systems(Update, producer.pipe(consumer)) .run(); } ``` </details> <details> <summary>Error Generated</summary> ```error error[E0277]: `fn(bevy::prelude::In<u16>) {consumer}` is not a valid system with input `u32` and output `_` --> examples/app/empty.rs:11:44 | 11 | .add_systems(Update, producer.pipe(consumer)) | ---- ^^^^^^^^ invalid system | | | required by a bound introduced by this call | = help: the trait `bevy::prelude::IntoSystem<u32, _, _>` is not implemented for fn item `fn(bevy::prelude::In<u16>) {consumer}` = note: expecting a system which consumes `u32` and produces `_` note: required by a bound in `pipe` --> C:\Users\Zac\Documents\GitHub\bevy\crates\bevy_ecs\src\system\mod.rs:168:12 | 166 | fn pipe<B, Final, MarkerB>(self, system: B) -> PipeSystem<Self::System, B::System> | ---- required by a bound in this associated function 167 | where 168 | B: IntoSystem<Out, Final, MarkerB>, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `IntoSystem::pipe` ``` </details> ### Missing Reflection <details> <summary>Example Code</summary> ```rust use bevy::prelude::*; #[derive(Component)] struct MyComponent; fn main() { App::new() .register_type::<MyComponent>() .run(); } ``` </details> <details> <summary>Error Generated</summary> ```error error[E0277]: `MyComponent` does not provide type registration information --> examples/app/empty.rs:8:26 | 8 | .register_type::<MyComponent>() | ------------- ^^^^^^^^^^^ the trait `GetTypeRegistration` is not implemented for `MyComponent` | | | required by a bound introduced by this call | = note: consider annotating `MyComponent` with `#[derive(Reflect)]` = help: the following other types implement trait `GetTypeRegistration`: bool char isize i8 i16 i32 i64 i128 and 443 others note: required by a bound in `bevy::prelude::App::register_type` --> C:\Users\Zac\Documents\GitHub\bevy\crates\bevy_app\src\app.rs:619:29 | 619 | pub fn register_type<T: bevy_reflect::GetTypeRegistration>(&mut self) -> &mut Self { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `App::register_type` ``` </details> ### Missing `#[derive(States)]` Implementation <details> <summary>Example Code</summary> ```rust use bevy::prelude::*; #[derive(Debug, Clone, Copy, Default, Eq, PartialEq, Hash)] enum AppState { #[default] Menu, InGame { paused: bool, turbo: bool, }, } fn main() { App::new() .init_state::<AppState>() .run(); } ``` </details> <details> <summary>Error Generated</summary> ```error error[E0277]: the trait bound `AppState: FreelyMutableState` is not satisfied --> examples/app/empty.rs:15:23 | 15 | .init_state::<AppState>() | ---------- ^^^^^^^^ the trait `FreelyMutableState` is not implemented for `AppState` | | | required by a bound introduced by this call | = note: consider annotating `AppState` with `#[derive(States)]` note: required by a bound in `bevy::prelude::App::init_state` --> C:\Users\Zac\Documents\GitHub\bevy\crates\bevy_app\src\app.rs:282:26 | 282 | pub fn init_state<S: FreelyMutableState + FromWorld>(&mut self) -> &mut Self { | ^^^^^^^^^^^^^^^^^^ required by this bound in `App::init_state` ``` </details> ### Adding a `System` with Unhandled Output <details> <summary>Example Code</summary> ```rust use bevy::prelude::*; fn producer() -> u32 { 123 } fn main() { App::new() .add_systems(Update, consumer) .run(); } ``` </details> <details> <summary>Error Generated</summary> ```error error[E0277]: `fn() -> u32 {producer}` does not describe a valid system configuration --> examples/app/empty.rs:9:30 | 9 | .add_systems(Update, producer) | ----------- ^^^^^^^^ invalid system configuration | | | required by a bound introduced by this call | = help: the trait `IntoSystem<(), (), _>` is not implemented for fn item `fn() -> u32 {producer}`, which is required by `fn() -> u32 {producer}: IntoSystemConfigs<_>` = help: the following other types implement trait `IntoSystemConfigs<Marker>`: <Box<(dyn bevy::prelude::System<In = (), Out = ()> + 'static)> as IntoSystemConfigs<()>> <NodeConfigs<Box<(dyn bevy::prelude::System<In = (), Out = ()> + 'static)>> as IntoSystemConfigs<()>> <(S0,) as IntoSystemConfigs<(SystemConfigTupleMarker, P0)>> <(S0, S1) as IntoSystemConfigs<(SystemConfigTupleMarker, P0, P1)>> <(S0, S1, S2) as IntoSystemConfigs<(SystemConfigTupleMarker, P0, P1, P2)>> <(S0, S1, S2, S3) as IntoSystemConfigs<(SystemConfigTupleMarker, P0, P1, P2, P3)>> <(S0, S1, S2, S3, S4) as IntoSystemConfigs<(SystemConfigTupleMarker, P0, P1, P2, P3, P4)>> <(S0, S1, S2, S3, S4, S5) as IntoSystemConfigs<(SystemConfigTupleMarker, P0, P1, P2, P3, P4, P5)>> and 14 others = note: required for `fn() -> u32 {producer}` to implement `IntoSystemConfigs<_>` note: required by a bound in `bevy::prelude::App::add_systems` --> C:\Users\Zac\Documents\GitHub\bevy\crates\bevy_app\src\app.rs:342:23 | 339 | pub fn add_systems<M>( | ----------- required by a bound in this associated function ... 342 | systems: impl IntoSystemConfigs<M>, | ^^^^^^^^^^^^^^^^^^^^ required by this bound in `App::add_systems` ``` </details> </details> ## Testing CI passed locally. ## Migration Guide Upgrade to version 1.78 (or higher) of Rust. ## Future Work - Currently, hints are not supported in this diagnostic. Ideally, suggestions like _"consider using ..."_ would be in a hint rather than a note, but that is the best option for now. - System chaining and other `all_tuples!(...)`-based traits have bad error messages due to the slightly different error message format. --------- Co-authored-by: Jamie Ridding <Themayu@users.noreply.github.com> Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: BD103 <59022059+BD103@users.noreply.github.com> |
||
|
BD103
|
7b8d502083
|
Fix beta lints (#12980)
# Objective - Fixes #12976 ## Solution This one is a doozy. - Run `cargo +beta clippy --workspace --all-targets --all-features` and fix all issues - This includes: - Moving inner attributes to be outer attributes, when the item in question has both inner and outer attributes - Use `ptr::from_ref` in more scenarios - Extend the valid idents list used by `clippy:doc_markdown` with more names - Use `Clone::clone_from` when possible - Remove redundant `ron` import - Add backticks to **so many** identifiers and items - I'm sorry whoever has to review this --- ## Changelog - Added links to more identifiers in documentation. |
||
|
James Liu
|
934f2cfadf
|
Clean up some low level dependencies (#12858)
# Objective Minimize the number of dependencies low in the tree. ## Solution * Remove the dependency on rustc-hash in bevy_ecs (not used) and bevy_macro_utils (only used in one spot). * Deduplicate the dependency on `sha1_smol` with the existing blake3 dependency already being used for bevy_asset. * Remove the unused `ron` dependency on `bevy_app` * Make the `serde` dependency for `bevy_ecs` optional. It's only used for serializing Entity. * Change the `wgpu` dependency to `wgpu-types`, and make it optional for `bevy_color`. * Remove the unused `thread-local` dependency on `bevy_render`. * Make multiple dependencies for `bevy_tasks` optional and enabled only when running with the `multi-threaded` feature. Preferably they'd be disabled all the time on wasm, but I couldn't find a clean way to do this. --- ## Changelog TODO ## Migration Guide TODO |
||
|
BD103
|
84363f2fab
|
Remove redundant imports (#12817)
# Objective - There are several redundant imports in the tests and examples that are not caught by CI because additional flags need to be passed. ## Solution - Run `cargo check --workspace --tests` and `cargo check --workspace --examples`, then fix all warnings. - Add `test-check` to CI, which will be run in the check-compiles job. This should catch future warnings for tests. Examples are already checked, but I'm not yet sure why they weren't caught. ## Discussion - Should the `--tests` and `--examples` flags be added to CI, so this is caught in the future? - If so, #12818 will need to be merged first. It was also a warning raised by checking the examples, but I chose to split off into a separate PR. --------- Co-authored-by: François Mockers <francois.mockers@vleue.com> |
||
|
Zeenobit
|
7d816aab04
|
Fix inconsistency between Debug and serialized representation of Entity (#12469)
# Objective Fixes #12139 ## Solution - Derive `Debug` impl for `Entity` - Add impl `Display` for `Entity` - Add `entity_display` test to check the output contains all required info I decided to go with `0v0|1234` format as opposed to the `0v0[1234]` which was initially discussed in the issue. My rationale for this is that `[1234]` may be confused for index values, which may be common in logs, and so searching for entities by text would become harder. I figured `|1234` would help the entity IDs stand out more. Additionally, I'm a little concerned that this change is gonna break existing logging for projects because `Debug` is now going to be a multi-line output. But maybe this is ok. We could implement `Debug` to be a single-line output, but then I don't see why it would be different from `Display` at all. @alice-i-cecile Let me know if we'd like to make any changes based on these points. |