ea9a3bee7f
120 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
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> |
||
|
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> |
||
|
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` |
||
|
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 |
||
|
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. |
||
|
James Liu
|
0d172b2914
|
Document instability of Entity's internal representation (#12249)
# Objective bevy_ecs has been developed with a de facto assumption that `Entity` is to be treated as an opaque identifier by external users, and that its internal representation is readable but in no way guaranteed to be stable between versions of bevy_ecs. This hasn't been clear to users, and the functions on the type that expose its guts speak a different story. ## Solution Explicitly document the lack of stability here and define internal representation changes as a non-breaking change under SemVer. Give it the same treatment that the standard lib gives `TypeId`. |
||
|
eri
|
5f8f3b532c
|
Check cfg during CI and fix feature typos (#12103)
# Objective - Add the new `-Zcheck-cfg` checks to catch more warnings - Fixes #12091 ## Solution - Create a new `cfg-check` to the CI that runs `cargo check -Zcheck-cfg --workspace` using cargo nightly (and fails if there are warnings) - Fix all warnings generated by the new check --- ## Changelog - Remove all redundant imports - Fix cfg wasm32 targets - Add 3 dead code exceptions (should StandardColor be unused?) - Convert ios_simulator to a feature (I'm not sure if this is the right way to do it, but the check complained before) ## Migration Guide No breaking changes --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> |
||
|
Tristan Guichaoua
|
33c7a2251e
|
bevy_ecs address trivial cases of unsafe_op_in_unsafe_fn (#11861)
# Objective - Part of #11590 - Fix `unsafe_op_in_unsafe_fn` for trivial cases in bevy_ecs ## Solution Fix `unsafe_op_in_unsafe_fn` in bevy_ecs for trivial cases, i.e., add an `unsafe` block when the safety comment already exists or add a comment like "The invariants are uphold by the caller". --------- Co-authored-by: James Liu <contact@jamessliu.com> |
||
|
Doonv
|
1c67e020f7
|
Move EntityHash related types into bevy_ecs (#11498)
# Objective Reduce the size of `bevy_utils` (https://github.com/bevyengine/bevy/issues/11478) ## Solution Move `EntityHash` related types into `bevy_ecs`. This also allows us access to `Entity`, which means we no longer need `EntityHashMap`'s first generic argument. --- ## Changelog - Moved `bevy::utils::{EntityHash, EntityHasher, EntityHashMap, EntityHashSet}` into `bevy::ecs::entity::hash` . - Removed `EntityHashMap`'s first generic argument. It is now hardcoded to always be `Entity`. ## Migration Guide - Uses of `bevy::utils::{EntityHash, EntityHasher, EntityHashMap, EntityHashSet}` now have to be imported from `bevy::ecs::entity::hash`. - Uses of `EntityHashMap` no longer have to specify the first generic parameter. It is now hardcoded to always be `Entity`. |
||
|
Gonçalo Rica Pais da Silva
|
e6a324a11a
|
Unified identifer for entities & relations (#9797)
# Objective The purpose of this PR is to begin putting together a unified identifier structure that can be used by entities and later components (as entities) as well as relationship pairs for relations, to enable all of these to be able to use the same storages. For the moment, to keep things small and focused, only `Entity` is being changed to make use of the new `Identifier` type, keeping `Entity`'s API and serialization/deserialization the same. Further changes are for follow-up PRs. ## Solution `Identifier` is a wrapper around `u64` split into two `u32` segments with the idea of being generalised to not impose restrictions on variants. That is for `Entity` to do. Instead, it is a general API for taking bits to then merge and map into a `u64` integer. It exposes low/high methods to return the two value portions as `u32` integers, with then the MSB masked for usage as a type flag, enabling entity kind discrimination and future activation/deactivation semantics. The layout in this PR for `Identifier` is described as below, going from MSB -> LSB. ``` |F| High value | Low value | |_|_______________________________|________________________________| |1| 31 | 32 | F = Bit Flags ``` The high component in this implementation has only 31 bits, but that still leaves 2^31 or 2,147,483,648 values that can be stored still, more than enough for any generation/relation kinds/etc usage. The low part is a full 32-bit index. The flags allow for 1 bit to be used for entity/pair discrimination, as these have different usages for the low/high portions of the `Identifier`. More bits can be reserved for more variants or activation/deactivation purposes, but this currently has no use in bevy. More bits could be reserved for future features at the cost of bits for the high component, so how much to reserve is up for discussion. Also, naming of the struct and methods are also subject to further bikeshedding and feedback. Also, because IDs can have different variants, I wonder if `Entity::from_bits` needs to return a `Result` instead of potentially panicking on receiving an invalid ID. PR is provided as an early WIP to obtain feedback and notes on whether this approach is viable. --- ## Changelog ### Added New `Identifier` struct for unifying IDs. ### Changed `Entity` changed to use new `Identifier`/`IdentifierMask` as the underlying ID logic. --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: vero <email@atlasdostal.com> |
||
|
Natalie Bonnibel Baker
|
b257fffef8
|
Change Entity::generation from u32 to NonZeroU32 for niche optimization (#9907)
# Objective - Implements change described in https://github.com/bevyengine/bevy/issues/3022 - Goal is to allow Entity to benefit from niche optimization, especially in the case of Option<Entity> to reduce memory overhead with structures with empty slots ## Discussion - First PR attempt: https://github.com/bevyengine/bevy/pull/3029 - Discord: https://discord.com/channels/691052431525675048/1154573759752183808/1154573764240093224 ## Solution - Change `Entity::generation` from u32 to NonZeroU32 to allow for niche optimization. - The reason for changing generation rather than index is so that the costs are only encountered on Entity free, instead of on Entity alloc - There was some concern with generations being used, due to there being some desire to introduce flags. This was more to do with the original retirement approach, however, in reality even if generations were reduced to 24-bits, we would still have 16 million generations available before wrapping and current ideas indicate that we would be using closer to 4-bits for flags. - Additionally, another concern was the representation of relationships where NonZeroU32 prevents us using the full address space, talking with Joy it seems unlikely to be an issue. The majority of the time these entity references will be low-index entries (ie. `ChildOf`, `Owes`), these will be able to be fast lookups, and the remainder of the range can use slower lookups to map to the address space. - It has the additional benefit of being less visible to most users, since generation is only ever really set through `from_bits` type methods. - `EntityMeta` was changed to match - On free, generation now explicitly wraps: - Originally, generation would panic in debug mode and wrap in release mode due to using regular ops. - The first attempt at this PR changed the behavior to "retire" slots and remove them from use when generations overflowed. This change was controversial, and likely needs a proper RFC/discussion. - Wrapping matches current release behaviour, and should therefore be less controversial. - Wrapping also more easily migrates to the retirement approach, as users likely to exhaust the exorbitant supply of generations will code defensively against aliasing and that defensive code is less likely to break than code assuming that generations don't wrap. - We use some unsafe code here when wrapping generations, to avoid branch on NonZeroU32 construction. It's guaranteed safe due to how we perform wrapping and it results in significantly smaller ASM code. - https://godbolt.org/z/6b6hj8PrM ## Migration - Previous `bevy_scene` serializations have a high likelihood of being broken, as they contain 0th generation entities. ## Current Issues - `Entities::reserve_generations` and `EntityMapper` wrap now, even in debug - although they technically did in release mode already so this probably isn't a huge issue. It just depends if we need to change anything here? --------- Co-authored-by: Natalie Baker <natalie.baker@advancednavigation.com> |
||
|
Mike
|
786abbf3f5
|
Fix ci xvfb (#11143)
# Objective Fix ci hang, so we can merge pr's again. ## Solution - switch ppa action to use mesa stable versions https://launchpad.net/~kisak/+archive/ubuntu/turtle - use commit from #11123 --------- Co-authored-by: Stepan Koltsov <stepan.koltsov@gmail.com> |
||
|
Tygyh
|
63d17e8494
|
Simplify equality assertions (#10988)
# Objective - Shorten assertions. ## Solution - Replace '==' assertions with 'assert_eq()' and '!=' assertions with 'assert_ne()' . |
||
|
tygyh
|
fd308571c4
|
Remove unnecessary path prefixes (#10749)
# Objective - Shorten paths by removing unnecessary prefixes ## Solution - Remove the prefixes from many paths which do not need them. Finding the paths was done automatically using built-in refactoring tools in Jetbrains RustRover. |
||
|
scottmcm
|
a902ea6f85
|
Save an instruction in EntityHasher (#10648)
# Objective Keep essentially the same structure of `EntityHasher` from #9903, but rephrase the multiplication slightly to save an instruction. cc @superdump Discord thread: https://discord.com/channels/691052431525675048/1172033156845674507/1174969772522356756 ## Solution Today, the hash is ```rust self.hash = i | (i.wrapping_mul(FRAC_U64MAX_PI) << 32); ``` with `i` being `(generation << 32) | index`. Expanding things out, we get ```rust i | ( (i * CONST) << 32 ) = (generation << 32) | index | ((((generation << 32) | index) * CONST) << 32) = (generation << 32) | index | ((index * CONST) << 32) // because the generation overflowed = (index * CONST | generation) << 32 | index ``` What if we do the same thing, but with `+` instead of `|`? That's almost the same thing, except that it has carries, which are actually often better in a hash function anyway, since it doesn't saturate. (`|` can be dangerous, since once something becomes `-1` it'll stay that, and there's no mixing available.) ```rust (index * CONST + generation) << 32 + index = (CONST << 32 + 1) * index + generation << 32 = (CONST << 32 + 1) * index + (WHATEVER << 32 + generation) << 32 // because the extra overflows and thus can be anything = (CONST << 32 + 1) * index + ((CONST * generation) << 32 + generation) << 32 // pick "whatever" to be something convenient = (CONST << 32 + 1) * index + ((CONST << 32 + 1) * generation) << 32 = (CONST << 32 + 1) * index +((CONST << 32 + 1) * (generation << 32) = (CONST << 32 + 1) * (index + generation << 32) = (CONST << 32 + 1) * (generation << 32 | index) = (CONST << 32 + 1) * i ``` So we can do essentially the same thing using a single multiplication instead of doing multiply-shift-or. LLVM was already smart enough to merge the shifting into a multiplication, but this saves the extra `or`:  <https://rust.godbolt.org/z/MEvbz4eo4> It's a very small change, and often will disappear in load latency anyway, but it's a couple percent faster in lookups:  (There was more of an improvement here before #10558, but with `to_bits` being a single `qword` load now, keeping things mostly as it is turned out to be better than the bigger changes I'd tried in #10605.) --- ## Changelog (Probably skip it) ## Migration Guide (none needed) |
||
|
Gonçalo Rica Pais da Silva
|
9c0fca072d
|
Optimise Entity with repr align & manual PartialOrd/Ord (#10558)
# Objective - Follow up on https://github.com/bevyengine/bevy/pull/10519, diving deeper into optimising `Entity` due to the `derive`d `PartialOrd` `partial_cmp` not being optimal with codegen: https://github.com/rust-lang/rust/issues/106107 - Fixes #2346. ## Solution Given the previous PR's solution and the other existing LLVM codegen bug, there seemed to be a potential further optimisation possible with `Entity`. In exploring providing manual `PartialOrd` impl, it turned out initially that the resulting codegen was not immediately better than the derived version. However, once `Entity` was given `#[repr(align(8)]`, the codegen improved remarkably, even more once the fields in `Entity` were rearranged to correspond to a `u64` layout (Rust doesn't automatically reorder fields correctly it seems). The field order and `align(8)` additions also improved `to_bits` codegen to be a single `mov` op. In turn, this led me to replace the previous "non-shortcircuiting" impl of `PartialEq::eq` to use direct `to_bits` comparison. The result was remarkably better codegen across the board, even for hastable lookups. The current baseline codegen is as follows: https://godbolt.org/z/zTW1h8PnY Assuming the following example struct that mirrors with the existing `Entity` definition: ```rust #[derive(Clone, Copy, Eq, PartialEq, PartialOrd, Ord)] pub struct FakeU64 { high: u32, low: u32, } ``` the output for `to_bits` is as follows: ``` example::FakeU64::to_bits: shl rdi, 32 mov eax, esi or rax, rdi ret ``` Changing the struct to: ```rust #[derive(Clone, Copy, Eq)] #[repr(align(8))] pub struct FakeU64 { low: u32, high: u32, } ``` and providing manual implementations for `PartialEq`/`PartialOrd`/`Ord`, `to_bits` now optimises to: ``` example::FakeU64::to_bits: mov rax, rdi ret ``` The full codegen example for this PR is here for reference: https://godbolt.org/z/n4Mjx165a To highlight, `gt` comparison goes from ``` example::greater_than: cmp edi, edx jae .LBB3_2 xor eax, eax ret .LBB3_2: setne dl cmp esi, ecx seta al or al, dl ret ``` to ``` example::greater_than: cmp rdi, rsi seta al ret ``` As explained on Discord by @scottmcm : >The root issue here, as far as I understand it, is that LLVM's middle-end is inexplicably unwilling to merge loads if that would make them under-aligned. It leaves that entirely up to its target-specific back-end, and thus a bunch of the things that you'd expect it to do that would fix this just don't happen. ## Benchmarks Before discussing benchmarks, everything was tested on the following specs: AMD Ryzen 7950X 16C/32T CPU 64GB 5200 RAM AMD RX7900XT 20GB Gfx card Manjaro KDE on Wayland I made use of the new entity hashing benchmarks to see how this PR would improve things there. With the changes in place, I first did an implementation keeping the existing "non shortcircuit" `PartialEq` implementation in place, but with the alignment and field ordering changes, which in the benchmark is the `ord_shortcircuit` column. The `to_bits` `PartialEq` implementation is the `ord_to_bits` column. The main_ord column is the current existing baseline from `main` branch.  My machine is not super set-up for benchmarking, so some results are within noise, but there's not just a clear improvement between the non-shortcircuiting implementation, but even further optimisation taking place with the `to_bits` implementation. On my machine, a fair number of the stress tests were not showing any difference (indicating other bottlenecks), but I was able to get a clear difference with `many_foxes` with a fox count of 10,000: Test with `cargo run --example many_foxes --features bevy/trace_tracy,wayland --release -- --count 10000`:  On avg, a framerate of about 28-29FPS was improved to 30-32FPS. "This trace" represents the current PR's perf, while "External trace" represents the `main` branch baseline. ## Changelog Changed: micro-optimized Entity align and field ordering as well as providing manual `PartialOrd`/`Ord` impls to help LLVM optimise further. ## Migration Guide Any `unsafe` code relying on field ordering of `Entity` or sufficiently cursed shenanigans should change to reflect the different internal representation and alignment requirements of `Entity`. Co-authored-by: james7132 <contact@jamessliu.com> Co-authored-by: NathanW <nathansward@comcast.net> |
||
|
scottmcm
|
713b1d8fa4
|
Optimize Entity::eq (#10519)
(This is my first PR here, so I've probably missed some things. Please let me know what else I should do to help you as a reviewer!) # Objective Due to https://github.com/rust-lang/rust/issues/117800, the `derive`'d `PartialEq::eq` on `Entity` isn't as good as it could be. Since that's used in hashtable lookup, let's improve it. ## Solution The derived `PartialEq::eq` short-circuits if the generation doesn't match. However, having a branch there is sub-optimal, especially on 64-bit systems like x64 that could just load the whole `Entity` in one load anyway. Due to complications around `poison` in LLVM and the exact details of what unsafe code is allowed to do with reference in Rust (https://github.com/rust-lang/unsafe-code-guidelines/issues/346), LLVM isn't allowed to completely remove the short-circuiting. `&Entity` is marked `dereferencable(8)` so LLVM knows it's allowed to *load* all 8 bytes -- and does so -- but it has to assume that the `index` might be undef/poison if the `generation` doesn't match, and thus while it finds a way to do it without needing a branch, it has to do something slightly more complicated than optimal to combine the results. (LLVM is allowed to change non-short-circuiting code to use branches, but not the other way around.) Here's a link showing the codegen today: <https://rust.godbolt.org/z/9WzjxrY7c> ```rust #[no_mangle] pub fn demo_eq_ref(a: &Entity, b: &Entity) -> bool { a == b } ``` ends up generating the following assembly: ```asm demo_eq_ref: movq xmm0, qword ptr [rdi] movq xmm1, qword ptr [rsi] pcmpeqd xmm1, xmm0 pshufd xmm0, xmm1, 80 movmskpd eax, xmm0 cmp eax, 3 sete al ret ``` (It's usually not this bad in real uses after inlining and LTO, but it makes a strong demo.) This PR manually implements `PartialEq::eq` *without* short-circuiting, and because that tells LLVM that neither the generations nor the index can be poison, it doesn't need to be so careful and can generate the "just compare the two 64-bit values" code you'd have probably already expected: ```asm demo_eq_ref: mov rax, qword ptr [rsi] cmp qword ptr [rdi], rax sete al ret ``` Since this doesn't change the representation of `Entity`, if it's instead passed by *value*, then each `Entity` is two `u32` registers, and the old and the new code do exactly the same thing. (Other approaches, like changing `Entity` to be `[u32; 2]` or `u64`, affect this case.) This should hopefully merge easily with changes like https://github.com/bevyengine/bevy/pull/9907 that also want to change `Entity`. ## Benchmarks I'm not super-confident that I got my machine fully consistent for benchmarking, but whether I run the old or the new one first I get reasonably consistent results. Here's a fairly typical example of the benchmarks I added in this PR:  Building the sets seems to be basically the same. It's usually reported as noise, but sometimes I see a few percent slower or faster. But lookup hits in particular -- since a hit checks that the key is equal -- consistently shows around 10% improvement. `cargo run --example many_cubes --features bevy/trace_tracy --release -- --benchmark` showed as slightly faster with this change, though if I had to bet I'd probably say it's more noise than meaningful (but at least it's not worse either):  This is my first PR here -- and my first time running Tracy -- so please let me know what else I should run, or run things on your own more reliable machines to double-check. --- ## Changelog (probably not worth including) Changed: micro-optimized `Entity::eq` to help LLVM slightly. ## Migration Guide (I really hope nobody was using this on uninitialized entities where sufficiently tortured `unsafe` could could technically notice that this has changed.) |
||
|
Robert Swain
|
b6ead2be95
|
Use EntityHashMap<Entity, T> for render world entity storage for better performance (#9903)
# Objective - Improve rendering performance, particularly by avoiding the large system commands costs of using the ECS in the way that the render world does. ## Solution - Define `EntityHasher` that calculates a hash from the `Entity.to_bits()` by `i | (i.wrapping_mul(0x517cc1b727220a95) << 32)`. `0x517cc1b727220a95` is something like `u64::MAX / N` for N that gives a value close to π and that works well for hashing. Thanks for @SkiFire13 for the suggestion and to @nicopap for alternative suggestions and discussion. This approach comes from `rustc-hash` (a.k.a. `FxHasher`) with some tweaks for the case of hashing an `Entity`. `FxHasher` and `SeaHasher` were also tested but were significantly slower. - Define `EntityHashMap` type that uses the `EntityHashser` - Use `EntityHashMap<Entity, T>` for render world entity storage, including: - `RenderMaterialInstances` - contains the `AssetId<M>` of the material associated with the entity. Also for 2D. - `RenderMeshInstances` - contains mesh transforms, flags and properties about mesh entities. Also for 2D. - `SkinIndices` and `MorphIndices` - contains the skin and morph index for an entity, respectively - `ExtractedSprites` - `ExtractedUiNodes` ## Benchmarks All benchmarks have been conducted on an M1 Max connected to AC power. The tests are run for 1500 frames. The 1000th frame is captured for comparison to check for visual regressions. There were none. ### 2D Meshes `bevymark --benchmark --waves 160 --per-wave 1000 --mode mesh2d` #### `--ordered-z` This test spawns the 2D meshes with z incrementing back to front, which is the ideal arrangement allocation order as it matches the sorted render order which means lookups have a high cache hit rate. <img width="1112" alt="Screenshot 2023-09-27 at 07 50 45" src="https://github.com/bevyengine/bevy/assets/302146/e140bc98-7091-4a3b-8ae1-ab75d16d2ccb"> -39.1% median frame time. #### Random This test spawns the 2D meshes with random z. This not only makes the batching and transparent 2D pass lookups get a lot of cache misses, it also currently means that the meshes are almost certain to not be batchable. <img width="1108" alt="Screenshot 2023-09-27 at 07 51 28" src="https://github.com/bevyengine/bevy/assets/302146/29c2e813-645a-43ce-982a-55df4bf7d8c4"> -7.2% median frame time. ### 3D Meshes `many_cubes --benchmark` <img width="1112" alt="Screenshot 2023-09-27 at 07 51 57" src="https://github.com/bevyengine/bevy/assets/302146/1a729673-3254-4e2a-9072-55e27c69f0fc"> -7.7% median frame time. ### Sprites **NOTE: On `main` sprites are using `SparseSet<Entity, T>`!** `bevymark --benchmark --waves 160 --per-wave 1000 --mode sprite` #### `--ordered-z` This test spawns the sprites with z incrementing back to front, which is the ideal arrangement allocation order as it matches the sorted render order which means lookups have a high cache hit rate. <img width="1116" alt="Screenshot 2023-09-27 at 07 52 31" src="https://github.com/bevyengine/bevy/assets/302146/bc8eab90-e375-4d31-b5cd-f55f6f59ab67"> +13.0% median frame time. #### Random This test spawns the sprites with random z. This makes the batching and transparent 2D pass lookups get a lot of cache misses. <img width="1109" alt="Screenshot 2023-09-27 at 07 53 01" src="https://github.com/bevyengine/bevy/assets/302146/22073f5d-99a7-49b0-9584-d3ac3eac3033"> +0.6% median frame time. ### UI **NOTE: On `main` UI is using `SparseSet<Entity, T>`!** `many_buttons` <img width="1111" alt="Screenshot 2023-09-27 at 07 53 26" src="https://github.com/bevyengine/bevy/assets/302146/66afd56d-cbe4-49e7-8b64-2f28f6043d85"> +15.1% median frame time. ## Alternatives - Cart originally suggested trying out `SparseSet<Entity, T>` and indeed that is slightly faster under ideal conditions. However, `PassHashMap<Entity, T>` has better worst case performance when data is randomly distributed, rather than in sorted render order, and does not have the worst case memory usage that `SparseSet`'s dense `Vec<usize>` that maps from the `Entity` index to sparse index into `Vec<T>`. This dense `Vec` has to be as large as the largest Entity index used with the `SparseSet`. - I also tested `PassHashMap<u32, T>`, intending to use `Entity.index()` as the key, but this proved to sometimes be slower and mostly no different. - The only outstanding approach that has not been implemented and tested is to _not_ clear the render world of its entities each frame. That has its own problems, though they could perhaps be solved. - Performance-wise, if the entities and their component data were not cleared, then they would incur table moves on spawn, and should not thereafter, rather just their component data would be overwritten. Ideally we would have a neat way of either updating data in-place via `&mut T` queries, or inserting components if not present. This would likely be quite cumbersome to have to remember to do everywhere, but perhaps it only needs to be done in the more performance-sensitive systems. - The main problem to solve however is that we want to both maintain a mapping between main world entities and render world entities, be able to run the render app and world in parallel with the main app and world for pipelined rendering, and at the same time be able to spawn entities in the render world in such a way that those Entity ids do not collide with those spawned in the main world. This is potentially quite solvable, but could well be a lot of ECS work to do it in a way that makes sense. --- ## Changelog - Changed: Component data for entities to be drawn are no longer stored on entities in the render world. Instead, data is stored in a `EntityHashMap<Entity, T>` in various resources. This brings significant performance benefits due to the way the render app clears entities every frame. Resources of most interest are `RenderMeshInstances` and `RenderMaterialInstances`, and their 2D counterparts. ## Migration Guide Previously the render app extracted mesh entities and their component data from the main world and stored them as entities and components in the render world. Now they are extracted into essentially `EntityHashMap<Entity, T>` where `T` are structs containing an appropriate group of data. This means that while extract set systems will continue to run extract queries against the main world they will store their data in hash maps. Also, systems in later sets will either need to look up entities in the available resources such as `RenderMeshInstances`, or maintain their own `EntityHashMap<Entity, T>` for their own data. Before: ```rust fn queue_custom( material_meshes: Query<(Entity, &MeshTransforms, &Handle<Mesh>), With<InstanceMaterialData>>, ) { ... for (entity, mesh_transforms, mesh_handle) in &material_meshes { ... } } ``` After: ```rust fn queue_custom( render_mesh_instances: Res<RenderMeshInstances>, instance_entities: Query<Entity, With<InstanceMaterialData>>, ) { ... for entity in &instance_entities { let Some(mesh_instance) = render_mesh_instances.get(&entity) else { continue; }; // The mesh handle in `AssetId<Mesh>` form, and the `MeshTransforms` can now // be found in `mesh_instance` which is a `RenderMeshInstance` ... } } ``` --------- Co-authored-by: robtfm <50659922+robtfm@users.noreply.github.com> |
||
|
Joseph
|
bc8bf34818
|
Allow disjoint mutable world access via EntityMut (#9419)
# Objective Fix #4278 Fix #5504 Fix #9422 Provide safe ways to borrow an entire entity, while allowing disjoint mutable access. `EntityRef` and `EntityMut` are not suitable for this, since they provide access to the entire world -- they are just helper types for working with `&World`/`&mut World`. This has potential uses for reflection and serialization ## Solution Remove `EntityRef::world`, which allows it to soundly be used within queries. `EntityMut` no longer supports structural world mutations, which allows multiple instances of it to exist for different entities at once. Structural world mutations are performed using the new type `EntityWorldMut`. ```rust fn disjoint_system( q2: Query<&mut A>, q1: Query<EntityMut, Without<A>>, ) { ... } let [entity1, entity2] = world.many_entities_mut([id1, id2]); *entity1.get_mut::<T>().unwrap() = *entity2.get().unwrap(); for entity in world.iter_entities_mut() { ... } ``` --- ## Changelog - Removed `EntityRef::world`, to fix a soundness issue with queries. + Removed the ability to structurally mutate the world using `EntityMut`, which allows it to be used in queries. + Added `EntityWorldMut`, which is used to perform structural mutations that are no longer allowed using `EntityMut`. ## Migration Guide **Note for maintainers: ensure that the guide for #9604 is updated accordingly.** Removed the method `EntityRef::world`, to fix a soundness issue with queries. If you need access to `&World` while using an `EntityRef`, consider passing the world as a separate parameter. `EntityMut` can no longer perform 'structural' world mutations, such as adding or removing components, or despawning the entity. Additionally, `EntityMut::world`, `EntityMut::world_mut` , and `EntityMut::world_scope` have been removed. Instead, use the newly-added type `EntityWorldMut`, which is a helper type for working with `&mut World`. --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> |
||
|
ClayenKitten
|
ffc572728f
|
Fix typos throughout the project (#9090)
# Objective
Fix typos throughout the project.
## Solution
[`typos`](https://github.com/crate-ci/typos) project was used for
scanning, but no automatic corrections were applied. I checked
everything by hand before fixing.
Most of the changes are documentation/comments corrections. Also, there
are few trivial changes to code (variable name, pub(crate) function name
and a few error/panic messages).
## Unsolved
`bevy_reflect_derive` has
[typo](
|
||
|
JoJoJet
|
32faf4cb5c
|
Document every public item in bevy_ecs (#8731)
# Objective Title. --------- Co-authored-by: François <mockersf@gmail.com> Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: James Liu <contact@jamessliu.com> |
||
|
Illiux
|
eebc92a7d4
|
Make scene handling of entity references robust (#7335)
# Objective - Handle dangling entity references inside scenes - Handle references to entities with generation > 0 inside scenes - Fix a latent bug in `Parent`'s `MapEntities` implementation, which would, if the parent was outside the scene, cause the scene to be loaded into the new world with a parent reference potentially pointing to some random entity in that new world. - Fixes #4793 and addresses #7235 ## Solution - DynamicScenes now identify entities with a `Entity` instead of a u32, therefore including generation - `World` exposes a new `reserve_generations` function that despawns an entity and advances its generation by some extra amount. - `MapEntities` implementations have a new `get_or_reserve` function available that will always return an `Entity`, establishing a new mapping to a dead entity when the entity they are called with is not in the `EntityMap`. Subsequent calls with that same `Entity` will return the same newly created dead entity reference, preserving equality semantics. - As a result, after loading a scene containing references to dead entities (or entities otherwise outside the scene), those references will all point to different generations on a single entity id in the new world. --- ## Changelog ### Changed - In serialized scenes, entities are now identified by a u64 instead of a u32. - In serialized scenes, components with entity references now have those references serialize as u64s instead of structs. ### Fixed - Scenes containing components with entity references will now deserialize and add to a world reliably. ## Migration Guide - `MapEntities` implementations must change from a `&EntityMap` parameter to a `&mut EntityMapper` parameter and can no longer return a `Result`. Finally, they should switch from calling `EntityMap::get` to calling `EntityMapper::get_or_reserve`. --------- Co-authored-by: Nicola Papale <nicopap@users.noreply.github.com> |
||
|
Wybe Westra
|
abf12f3b3b
|
Fixed several missing links in docs. (#8117)
Links in the api docs are nice. I noticed that there were several places where structs / functions and other things were referenced in the docs, but weren't linked. I added the links where possible / logical. --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: François <mockersf@gmail.com> |
||
|
James Liu
|
2ec38d1467
|
Inline more ECS functions (#8083)
# Objective Upon closer inspection, there are a few functions in the ECS that are not being inlined, even with the highest optimizations and LTO enabled: - Almost all [WorldQuery::init_fetch]( |
||
|
Mat Hostetter
|
f3d6c2d90b
|
Remove redundant bounds check in Entities::get (#8108)
|
||
|
shuo
|
609ad5aaa6 |
introduce EntityLocation::INVALID const and adjust Entities::get comment (#7623)
also remove one un-needed mut. |
||
|
Alice Cecile
|
39e14a4a40 |
Make EntityRef::new unsafe (#7222)
# Objective - We rely on the construction of `EntityRef` to be valid elsewhere in unsafe code. This construction is not checked (for performance reasons), and thus this private method must be unsafe. - Fixes #7218. ## Solution - Make the method unsafe. - Add safety docs. - Improve safety docs slightly for the sibling `EntityMut::new`. - Add debug asserts to start to verify these assumptions in debug mode. ## Context for reviewers I attempted to verify the `EntityLocation` more thoroughly, but this turned out to be more work than expected. I've spun that off into #7221 as a result. |
||
|
Rob Parrett
|
3dd8b42f72 |
Fix various typos (#7096)
I stumbled across a typo in some docs. Fixed some more while I was in there. |
||
|
James Liu
|
a5b1c46d5b |
Extend EntityLocation with TableId and TableRow (#6681)
# Objective `Query::get` and other random access methods require looking up `EntityLocation` for every provided entity, then always looking up the `Archetype` to get the table ID and table row. This requires 4 total random fetches from memory: the `Entities` lookup, the `Archetype` lookup, the table row lookup, and the final fetch from table/sparse sets. If `EntityLocation` contains the table ID and table row, only the `Entities` lookup and the final storage fetch are required. ## Solution Add `TableId` and table row to `EntityLocation`. Ensure it's updated whenever entities are moved around. To ensure `EntityMeta` does not grow bigger, both `TableId` and `ArchetypeId` have been shrunk to u32, and the archetype index and table row are stored as u32s instead of as usizes. This should shrink `EntityMeta` by 4 bytes, from 24 to 20 bytes, as there is no padding anymore due to the change in alignment. This idea was partially concocted by @BoxyUwU. ## Performance This should restore the `Query::get` "gains" lost to #6625 that were introduced in #4800 without being unsound, and also incorporates some of the memory usage reductions seen in #3678. This also removes the same lookups during add/remove/spawn commands, so there may be a bit of a speedup in commands and `Entity{Ref,Mut}`. --- ## Changelog Added: `EntityLocation::table_id` Added: `EntityLocation::table_row`. Changed: `World`s can now only hold a maximum of 2<sup>32</sup>- 1 archetypes. Changed: `World`s can now only hold a maximum of 2<sup>32</sup> - 1 tables. ## Migration Guide A `World` can only hold a maximum of 2<sup>32</sup> - 1 archetypes and tables now. If your use case requires more than this, please file an issue explaining your use case. |
||
|
James Liu
|
530be10e72 |
Newtype ArchetypeRow and TableRow (#4878)
# Objective Prevent future unsoundness that was seen in #6623. ## Solution Newtype both indexes in `Archetype` and `Table` as `ArchetypeRow` and `TableRow`. This avoids weird numerical manipulation on the indices, and can be stored and treated opaquely. Also enforces the source and destination of where these indices at a type level. --- ## Changelog Changed: `Archetype` indices and `Table` rows have been newtyped as `ArchetypeRow` and `TableRow`. |
||
|
James Liu
|
e954b8573c |
Lock down access to Entities (#6740)
# Objective The soundness of the ECS `World` partially relies on the correctness of the state of `Entities` stored within it. We're currently allowing users to (unsafely) mutate it, as well as readily construct it without using a `World`. While this is not strictly unsound so long as users (including `bevy_render`) safely use the APIs, it's a fairly easy path to unsoundness without much of a guard rail. Addresses #3362 for `bevy_ecs::entity`. Incorporates the changes from #3985. ## Solution Remove `Entities`'s `Default` implementation and force access to the type to only be through a properly constructed `World`. Additional cleanup for other parts of `bevy_ecs::entity`: - `Entity::index` and `Entity::generation` are no longer `pub(crate)`, opting to force the rest of bevy_ecs to use the public interface to access these values. - `EntityMeta` is no longer `pub` and also not `pub(crate)` to attempt to cut down on updating `generation` without going through an `Entities` API. It's currently inaccessible except via the `pub(crate)` Vec on `Entities`, there was no way for an outside user to use it. - Added `Entities::set`, an unsafe `pub(crate)` API for setting the location of an Entity (parallel to `Entities::get`) that replaces the internal case where we need to set the location of an entity when it's been spawned, moved, or despawned. - `Entities::alloc_at_without_replacement` is only used in `World::get_or_spawn` within the first party crates, and I cannot find a public use of this API in any ecosystem crate that I've checked (via GitHub search). - Attempted to document the few remaining undocumented public APIs in the module. --- ## Changelog Removed: `Entities`'s `Default` implementation. Removed: `EntityMeta` Removed: `Entities::alloc_at_without_replacement` and `AllocAtWithoutReplacement`. Co-authored-by: james7132 <contact@jamessliu.com> Co-authored-by: James Liu <contact@jamessliu.com> |
||
|
mareq
|
bbb652a438 |
Fix documentation on spawining an entity (#6775)
# Objective - The documentation describing different ways to spawn an Entity is missing reference to "method" for "Spawn an entity with components". ## Solution - Update the documentation to add the reference to `World::spawn`. |
||
|
JoJoJet
|
416a33e613 |
Add const Entity::PLACEHOLDER (#6761)
# Objective One of the use-cases for the function `Entity::from_raw` is creating placeholder entity ids, which are meant to be overwritten later. If we use a constant for this instead of `from_raw`, it is more ergonomic and self-documenting. ## Solution Add a constant that returns an entity ID with an index of `u32::MAX` and a generation of zero. Users are instructed to overwrite this value before using it. |
||
|
James Liu
|
2179a3ebf4 |
Make Entity::to_bits const (#6559)
# Objective Fix #6548. Most of these methods were already made `const` in #5688. `Entity::to_bits` is the only one that remained. ## Solution Make it const. |
||
|
Edvin Kjell
|
a8a62fcf3d |
[Fixes #6059] `Entity`'s “ID” should be named “index” instead (#6107)
# Objective Fixes #6059, changing all incorrect occurrences of ``id`` in the ``entity`` module to ``index``: * struct level documentation, * ``id`` struct field, * ``id`` method and its documentation. ## Solution Renaming and verifying using CI. Co-authored-by: Edvin Kjell <43633999+Edwox@users.noreply.github.com> |
||
|
Hennadii Chernyshchyk
|
71f8b4a92f |
Use default serde impls for Entity (#6194)
# Objective
Currently for entities we serialize only `id`. But this is not very expected behavior. For example, in networking, when the server sends its state, it contains entities and components. On the client, I create new objects and map them (using `EntityMap`) to those received from the server (to know which one matches which). And if `generation` field is missing, this mapping can be broken. Example:
1. Server sends an entity `Entity{ id: 2, generation: 1}` with components.
2. Client puts the received entity in a map and create a new entity that maps to this received entity. The new entity have different `id` and `generation`. Let's call it `Entity{ id: 12, generation: 4}`.
3. Client sends a command for `Entity{ id: 12, generation: 4}`. To do so, it maps local entity to the one from server. But `generation` field is 0 because it was omitted for serialization on the server. So it maps to `Entity{ id: 2, generation: 0}`.
4. Server receives `Entity{ id: 2, generation: 0}` which is invalid.
In my game I worked around it by [writing custom serialization](https://github.com/dollisgame/dollis/blob/master/src/core/network/entity_serde.rs) and using `serde(with = "...")`. But it feels like a bad default to me.
Using `Entity` over a custom `NetworkId` also have the following advantages:
1. Re-use `MapEntities` trait to map `Entity`s in replicated components.
2. Instead of server `Entity <-> NetworkId ` and `Entity <-> NetworkId`, we map entities only on client.
3. No need to handling uniqueness. It's a rare case, but makes things simpler. For example, I don't need to query for a resource to create an unique ID.
Closes #6143.
## Solution
Use default serde impls. If anyone want to avoid wasting memory on `generation`, they can create a new type that holds `u32`. This is what Bevy do for [DynamicEntity](https://docs.rs/bevy/latest/bevy/scene/struct.DynamicEntity.html) to serialize scenes. And I don't see any use case to serialize an entity id expect this one.
---
## Changelog
### Changed
- Entity now serializes / deserializes `generation` field.
## Migration Guide
- Entity now fully serialized. If you want to serialze only `id`, as it was before, you can create a new type that wraps `u32`.
|
||
|
TheRawMeatball
|
3c13c75036 |
Optimize rendering slow-down at high entity counts (#5509)
# Objective - Improve #3953 ## Solution - The very specific circumstances under which the render world is reset meant that the flush_as_invalid function could be replaced with one that had a noop as its init method. - This removes a double-writing issue leading to greatly increased performance. Running the reproduction code in the linked issue, this change nearly doubles the framerate. Co-authored-by: Carter Anderson <mcanders1@gmail.com> |
||
|
Carter Anderson
|
dc3f801239 |
Exclusive Systems Now Implement System. Flexible Exclusive System Params (#6083)
# Objective The [Stageless RFC](https://github.com/bevyengine/rfcs/pull/45) involves allowing exclusive systems to be referenced and ordered relative to parallel systems. We've agreed that unifying systems under `System` is the right move. This is an alternative to #4166 (see rationale in the comments I left there). Note that this builds on the learnings established there (and borrows some patterns). ## Solution This unifies parallel and exclusive systems under the shared `System` trait, removing the old `ExclusiveSystem` trait / impls. This is accomplished by adding a new `ExclusiveFunctionSystem` impl similar to `FunctionSystem`. It is backed by `ExclusiveSystemParam`, which is similar to `SystemParam`. There is a new flattened out SystemContainer api (which cuts out a lot of trait and type complexity). This means you can remove all cases of `exclusive_system()`: ```rust // before commands.add_system(some_system.exclusive_system()); // after commands.add_system(some_system); ``` I've also implemented `ExclusiveSystemParam` for `&mut QueryState` and `&mut SystemState`, which makes this possible in exclusive systems: ```rust fn some_exclusive_system( world: &mut World, transforms: &mut QueryState<&Transform>, state: &mut SystemState<(Res<Time>, Query<&Player>)>, ) { for transform in transforms.iter(world) { println!("{transform:?}"); } let (time, players) = state.get(world); for player in players.iter() { println!("{player:?}"); } } ``` Note that "exclusive function systems" assume `&mut World` is present (and the first param). I think this is a fair assumption, given that the presence of `&mut World` is what defines the need for an exclusive system. I added some targeted SystemParam `static` constraints, which removed the need for this: ``` rust fn some_exclusive_system(state: &mut SystemState<(Res<'static, Time>, Query<&'static Player>)>) {} ``` ## Related - #2923 - #3001 - #3946 ## Changelog - `ExclusiveSystem` trait (and implementations) has been removed in favor of sharing the `System` trait. - `ExclusiveFunctionSystem` and `ExclusiveSystemParam` were added, enabling flexible exclusive function systems - `&mut SystemState` and `&mut QueryState` now implement `ExclusiveSystemParam` - Exclusive and parallel System configuration is now done via a unified `SystemDescriptor`, `IntoSystemDescriptor`, and `SystemContainer` api. ## Migration Guide Calling `.exclusive_system()` is no longer required (or supported) for converting exclusive system functions to exclusive systems: ```rust // Old (0.8) app.add_system(some_exclusive_system.exclusive_system()); // New (0.9) app.add_system(some_exclusive_system); ``` Converting "normal" parallel systems to exclusive systems is done by calling the exclusive ordering apis: ```rust // Old (0.8) app.add_system(some_system.exclusive_system().at_end()); // New (0.9) app.add_system(some_system.at_end()); ``` Query state in exclusive systems can now be cached via ExclusiveSystemParams, which should be preferred for clarity and performance reasons: ```rust // Old (0.8) fn some_system(world: &mut World) { let mut transforms = world.query::<&Transform>(); for transform in transforms.iter(world) { } } // New (0.9) fn some_system(world: &mut World, transforms: &mut QueryState<&Transform>) { for transform in transforms.iter(world) { } } ``` |
||
|
Carter Anderson
|
01aedc8431 |
Spawn now takes a Bundle (#6054)
# Objective Now that we can consolidate Bundles and Components under a single insert (thanks to #2975 and #6039), almost 100% of world spawns now look like `world.spawn().insert((Some, Tuple, Here))`. Spawning an entity without any components is an extremely uncommon pattern, so it makes sense to give spawn the "first class" ergonomic api. This consolidated api should be made consistent across all spawn apis (such as World and Commands). ## Solution All `spawn` apis (`World::spawn`, `Commands:;spawn`, `ChildBuilder::spawn`, and `WorldChildBuilder::spawn`) now accept a bundle as input: ```rust // before: commands .spawn() .insert((A, B, C)); world .spawn() .insert((A, B, C); // after commands.spawn((A, B, C)); world.spawn((A, B, C)); ``` All existing instances of `spawn_bundle` have been deprecated in favor of the new `spawn` api. A new `spawn_empty` has been added, replacing the old `spawn` api. By allowing `world.spawn(some_bundle)` to replace `world.spawn().insert(some_bundle)`, this opened the door to removing the initial entity allocation in the "empty" archetype / table done in `spawn()` (and subsequent move to the actual archetype in `.insert(some_bundle)`). This improves spawn performance by over 10%:  To take this measurement, I added a new `world_spawn` benchmark. Unfortunately, optimizing `Commands::spawn` is slightly less trivial, as Commands expose the Entity id of spawned entities prior to actually spawning. Doing the optimization would (naively) require assurances that the `spawn(some_bundle)` command is applied before all other commands involving the entity (which would not necessarily be true, if memory serves). Optimizing `Commands::spawn` this way does feel possible, but it will require careful thought (and maybe some additional checks), which deserves its own PR. For now, it has the same performance characteristics of the current `Commands::spawn_bundle` on main. **Note that 99% of this PR is simple renames and refactors. The only code that needs careful scrutiny is the new `World::spawn()` impl, which is relatively straightforward, but it has some new unsafe code (which re-uses battle tested BundlerSpawner code path).** --- ## Changelog - All `spawn` apis (`World::spawn`, `Commands:;spawn`, `ChildBuilder::spawn`, and `WorldChildBuilder::spawn`) now accept a bundle as input - All instances of `spawn_bundle` have been deprecated in favor of the new `spawn` api - World and Commands now have `spawn_empty()`, which is equivalent to the old `spawn()` behavior. ## Migration Guide ```rust // Old (0.8): commands .spawn() .insert_bundle((A, B, C)); // New (0.9) commands.spawn((A, B, C)); // Old (0.8): commands.spawn_bundle((A, B, C)); // New (0.9) commands.spawn((A, B, C)); // Old (0.8): let entity = commands.spawn().id(); // New (0.9) let entity = commands.spawn_empty().id(); // Old (0.8) let entity = world.spawn().id(); // New (0.9) let entity = world.spawn_empty(); ``` |
||
|
targrub
|
a09dd034a2 |
Fix CI issues arising from use of Rust 1.64 (#6067)
## Objective Fixes https://github.com/bevyengine/bevy/issues/6063 ## Solution - Use `then_some(x)` instead of `then( || x)`. - Updated error logs from `bevy_ecs_compile_fail_tests`. ## Migration Guide From Rust 1.63 to 1.64, a new Clippy error was added; now one should use `then_some(x)` instead of `then( || x)`. |
||
|
Carter Anderson
|
cd15f0f5be |
Accept Bundles for insert and remove. Deprecate insert/remove_bundle (#6039)
# Objective Take advantage of the "impl Bundle for Component" changes in #2975 / add the follow up changes discussed there. ## Solution - Change `insert` and `remove` to accept a Bundle instead of a Component (for both Commands and World) - Deprecate `insert_bundle`, `remove_bundle`, and `remove_bundle_intersection` - Add `remove_intersection` --- ## Changelog - Change `insert` and `remove` now accept a Bundle instead of a Component (for both Commands and World) - `insert_bundle` and `remove_bundle` are deprecated ## Migration Guide Replace `insert_bundle` with `insert`: ```rust // Old (0.8) commands.spawn().insert_bundle(SomeBundle::default()); // New (0.9) commands.spawn().insert(SomeBundle::default()); ``` Replace `remove_bundle` with `remove`: ```rust // Old (0.8) commands.entity(some_entity).remove_bundle::<SomeBundle>(); // New (0.9) commands.entity(some_entity).remove::<SomeBundle>(); ``` Replace `remove_bundle_intersection` with `remove_intersection`: ```rust // Old (0.8) world.entity_mut(some_entity).remove_bundle_intersection::<SomeBundle>(); // New (0.9) world.entity_mut(some_entity).remove_intersection::<SomeBundle>(); ``` Consider consolidating as many operations as possible to improve ergonomics and cut down on archetype moves: ```rust // Old (0.8) commands.spawn() .insert_bundle(SomeBundle::default()) .insert(SomeComponent); // New (0.9) - Option 1 commands.spawn().insert(( SomeBundle::default(), SomeComponent, )) // New (0.9) - Option 2 commands.spawn_bundle(( SomeBundle::default(), SomeComponent, )) ``` ## Next Steps Consider changing `spawn` to accept a bundle and deprecate `spawn_bundle`. |
||
|
Jerome Humbert
|
fe6246dac6 |
Make most Entity methods const (#5688)
# Objective Fixes #5687 ## Solution Update the methods on the `Entity` struct to be `const`, so we can define compile-time constants and more generally use them in a const context. --- ## Changelog ### Added - Most `Entity` methods are now `const fn`. |
||
|
Ian Chamberlain
|
cde5ae8104 |
bevy_ecs: Use 32-bit entity ID cursor on platforms without AtomicI64 (#4452)
# Objective - Fixes #4451 ## Solution - Conditionally compile entity ID cursor as `AtomicI32` when compiling on a platform that does not support 64-bit atomics. - This effectively raises the MSRV to 1.60 as it uses a `#[cfg]` that was only just stabilized there. (should this be noted in changelog?) --- ## Changelog - Added `bevy_ecs` support for platforms without 64-bit atomic ints ## Migration Guide N/A |
||
|
ira
|
4847f7e3ad |
Update codebase to use IntoIterator where possible. (#5269)
Remove unnecessary calls to `iter()`/`iter_mut()`.
Mainly updates the use of queries in our code, docs, and examples.
```rust
// From
for _ in list.iter() {
for _ in list.iter_mut() {
// To
for _ in &list {
for _ in &mut list {
```
We already enable the pedantic lint [clippy::explicit_iter_loop](https://rust-lang.github.io/rust-clippy/stable/) inside of Bevy. However, this only warns for a few known types from the standard library.
## Note for reviewers
As you can see the additions and deletions are exactly equal.
Maybe give it a quick skim to check I didn't sneak in a crypto miner, but you don't have to torture yourself by reading every line.
I already experienced enough pain making this PR :)
Co-authored-by: devil-ira <justthecooldude@gmail.com>
|
||
|
Jakob Hellermann
|
d38a8dfdd7 |
add more SAFETY comments and lint for missing ones in bevy_ecs (#4835)
# Objective `SAFETY` comments are meant to be placed before `unsafe` blocks and should contain the reasoning of why in this case the usage of unsafe is okay. This is useful when reading the code because it makes it clear which assumptions are required for safety, and makes it easier to spot possible unsoundness holes. It also forces the code writer to think of something to write and maybe look at the safety contracts of any called unsafe methods again to double-check their correct usage. There's a clippy lint called `undocumented_unsafe_blocks` which warns when using a block without such a comment. ## Solution - since clippy expects `SAFETY` instead of `SAFE`, rename those - add `SAFETY` comments in more places - for the last remaining 3 places, add an `#[allow()]` and `// TODO` since I wasn't comfortable enough with the code to justify their safety - add ` #![warn(clippy::undocumented_unsafe_blocks)]` to `bevy_ecs` ### Note for reviewers The first commit only renames `SAFETY` to `SAFE` so it doesn't need a thorough review. |
||
|
Federico Rinaldi
|
511bcc9633 |
Improve entity and component API docs (#4767)
# Objective The descriptions included in the API docs of `entity` module, `Entity` struct, and `Component` trait have some issues: 1. the concept of entity is not clearly defined, 2. descriptions are a little bit out of place, 3. in a case the description leak too many details about the implementation, 4. some descriptions are not exhaustive, 5. there are not enough examples, 6. the content can be formatted in a much better way. ## Solution 1. ~~Stress the fact that entity is an abstract and elementary concept. Abstract because the concept of entity is not hardcoded into the library but emerges from the interaction of `Entity` with every other part of `bevy_ecs`, like components and world methods. Elementary because it is a fundamental concept that cannot be defined with other terms (like point in euclidean geometry, or time in classical physics).~~ We decided to omit the definition of entity in the API docs ([see why]). It is only described in its relationship with components. 2. Information has been moved to relevant places and links are used instead in the other places. 3. Implementation details about `Entity` have been reduced. 4. Descriptions have been made more exhaustive by stating how to obtain and use items. Entity operations are enriched with `World` methods. 5. Examples have been added or enriched. 6. Sections have been added to organize content. Entity operations are now laid out in a table. ### Todo list - [x] Break lines at sentence-level. ## For reviewers - ~~I added a TODO over `Component` docs, make sure to check it out and discuss it if necessary.~~ ([Resolved]) - You can easily check the rendered documentation by doing `cargo doc -p bevy_ecs --no-deps --open`. [see why]: https://github.com/bevyengine/bevy/pull/4767#discussion_r875106329 [Resolved]: https://github.com/bevyengine/bevy/pull/4767#discussion_r874127825 |
||
|
James Liu
|
f2b545049c |
Implement FusedIterator for eligible Iterator types (#4942)
# Objective Most of our `Iterator` impls satisfy the requirements of `std::iter::FusedIterator`, which has internal specialization that optimizes `Interator::fuse`. The std lib iterator combinators do have a few that rely on `fuse`, so this could optimize those use cases. I don't think we're using any of them in the engine itself, but beyond a light increase in compile time, it doesn't hurt to implement the trait. ## Solution Implement the trait for all eligible iterators in first party crates. Also add a missing `ExactSizeIterator` on an iterator that could use it. |
||
|
Boxy
|
637a149910 |
unsafeify World::entities_mut (#4093)
# Objective
make bevy ecs a lil bit less unsound
## Solution
make unsound API unsafe so that there is an unsafe block to blame:
```rust
use bevy_ecs::prelude::*;
#[derive(Debug, Component)]
struct Foo(u8);
fn main() {
let mut world = World::new();
let e1 = world.spawn().id();
let e2 = world.spawn().insert(Foo(2)).id();
world.entities_mut().meta[0] = world.entities_mut().meta[1].clone();
let foo = world.entity(e1).get::<Foo>().unwrap();
// whoo i love having components i dont have
dbg!(foo);
}
```
This is not _strictly_ speaking UB, however:
- `Query::get_multiple` cannot work if this is allowed
- bevy_ecs is a pile of unsafe code whose soundness generally depends on the world being in a "correct" state with "no funny business" so it seems best to disallow this
- it is trivial to get bevy to panic inside of functions with safety invariants that have been violated (the entity location is not valid)
- it seems to violate what the safety invariant on `Entities::flush` is trying to ensure
|
||
|
danieleades
|
d8974e7c3d |
small and mostly pointless refactoring (#2934)
What is says on the tin. This has got more to do with making `clippy` slightly more *quiet* than it does with changing anything that might greatly impact readability or performance. that said, deriving `Default` for a couple of structs is a nice easy win |
||
|
Michael Dorst
|
507441d96f |
Fix doc_markdown lints in bevy_ecs (#3473)
#3457 adds the `doc_markdown` clippy lint, which checks doc comments to make sure code identifiers are escaped with backticks. This causes a lot of lint errors, so this is one of a number of PR's that will fix those lint errors one crate at a time. This PR fixes lints in the `bevy_ecs` crate. |
||
|
David Sugar
|
8a8293b266 |
Renamed Entity::new to Entity::from_raw (#3465)
# Objective - Rename `Entity::new(id: u32)` to `Entity::from_raw(id: u32)`. - Add further documentation. - fixes #3108 ## Solution - Renamed `Entity::new(id: u32)` to `Entity::from_raw(id: u32)`. - Docs extended. I derived the examples from the discussion of issue #3108 . The [first case](https://github.com/bevyengine/bevy/issues/3108#issuecomment-966669781) mentioned in the linked issue is quite obvious but the [second one](https://github.com/bevyengine/bevy/issues/3108#issuecomment-967093902) probably needs further explanation. Co-authored-by: r4gus <david@thesugar.de> |
||
|
Federico Rinaldi
|
615d43b998 |
Improve bevy_ecs and bevy_app API docs where referenced by the new Bevy Book (#2365)
## Objective
The upcoming Bevy Book makes many references to the API documentation of bevy.
Most references belong to the first two chapters of the Bevy Book:
- bevyengine/bevy-website#176
- bevyengine/bevy-website#182
This PR attempts to improve the documentation of `bevy_ecs` and `bevy_app` in order to help readers of the Book who want to delve deeper into technical details.
## Solution
- Add crate and level module documentation
- Document the most important items (basically those included in the preludes), with the following style, where applicable:
- **Summary.** Short description of the item.
- **Second paragraph.** Detailed description of the item, without going too much in the implementation.
- **Code example(s).**
- **Safety or panic notes.**
## Collaboration
Any kind of collaboration is welcome, especially corrections, wording, new ideas and guidelines on where the focus should be put in.
---
### Related issues
- Fixes #2246
|
||
|
Daniel McNab
|
af20cad830 |
Add error messages for the spooky insertions (#2581)
# Objective
Sometimes, the unwraps in `entity_mut` could fail here, if the entity was despawned *before* this command was applied.
The simplest case involves two command buffers:
```rust
use bevy::prelude::*;
fn b(mut commands1: Commands, mut commands2: Commands) {
let id = commands2.spawn().insert_bundle(()).id();
commands1.entity(id).despawn();
}
fn main() {
App::build().add_system(b.system()).run();
}
```
However, a more complicated version arises in the case of ambiguity:
```rust
use std::time::Duration;
use bevy::{app::ScheduleRunnerPlugin, prelude::*};
use rand::Rng;
fn cleanup(mut e: ResMut<Option<Entity>>) {
*e = None;
}
fn sleep_randomly() {
let mut rng = rand::thread_rng();
std:🧵:sleep(Duration::from_millis(rng.gen_range(0..50)));
}
fn spawn(mut commands: Commands, mut e: ResMut<Option<Entity>>) {
*e = Some(commands.spawn().insert_bundle(()).id());
}
fn despawn(mut commands: Commands, e: Res<Option<Entity>>) {
let mut rng = rand::thread_rng();
std:🧵:sleep(Duration::from_millis(rng.gen_range(0..50)));
if let Some(e) = *e {
commands.entity(e).despawn();
}
}
fn main() {
App::build()
.add_system(cleanup.system().label("cleanup"))
.add_system(sleep_randomly.system().label("before_despawn"))
.add_system(despawn.system().after("cleanup").after("before_despawn"))
.add_system(sleep_randomly.system().label("before_spawn"))
.add_system(spawn.system().after("cleanup").after("before_spawn"))
.insert_resource(None::<Entity>)
.add_plugin(ScheduleRunnerPlugin::default())
.run();
}
```
In the cases where this example crashes, it's because `despawn` was ordered before `spawn` in the topological ordering of systems (which determines when buffers are applied). However, `despawn` actually ran *after* `spawn`, because these systems are ambiguous, so the jiggles in the sleeping time triggered a case where this works.
## Solution
- Give a better error message
|
||
|
Carter Anderson
|
b47217bfab |
Spawn specific entities: spawn or insert operations, refactor spawn internals, world clearing (#2673)
This upstreams the code changes used by the new renderer to enable cross-app Entity reuse: * Spawning at specific entities * get_or_spawn: spawns an entity if it doesn't already exist and returns an EntityMut * insert_or_spawn_batch: the batched equivalent to `world.get_or_spawn(entity).insert_bundle(bundle)` * Clearing entities and storages * Allocating Entities with "invalid" archetypes. These entities cannot be queried / are treated as "non existent". They serve as "reserved" entities that won't show up when calling `spawn()`. They must be "specifically spawned at" using apis like `get_or_spawn(entity)`. In combination, these changes enable the "render world" to clear entities / storages each frame and reserve all "app world entities". These can then be spawned during the "render extract step". This refactors "spawn" and "insert" code in a way that I think is a massive improvement to legibility and re-usability. It also yields marginal performance wins by reducing some duplicate lookups (less than a percentage point improvement on insertion benchmarks). There is also some potential for future unsafe reduction (by making BatchSpawner and BatchInserter generic). But for now I want to cut down generic usage to a minimum to encourage smaller binaries and faster compiles. This is currently a draft because it needs more tests (although this code has already had some real-world testing on my custom-shaders branch). I also fixed the benchmarks (which currently don't compile!) / added new ones to illustrate batching wins. After these changes, Bevy ECS is basically ready to accommodate the new renderer. I think the biggest missing piece at this point is "sub apps". |
||
|
François
|
234b2efa71 |
Inline world get (#2520)
# Objective While looking at the code of `World`, I noticed two basic functions (`get` and `get_mut`) that are probably called a lot and with simple code that are not `inline` ## Solution - Add benchmark to check impact - Add `#[inline]` ``` group this pr main ----- ---- ---- world_entity/50000_entities 1.00 115.9±11.90µs ? ?/sec 1.71 198.5±29.54µs ? ?/sec world_get/50000_entities_SparseSet 1.00 409.9±46.96µs ? ?/sec 1.18 483.5±36.41µs ? ?/sec world_get/50000_entities_Table 1.00 391.3±29.83µs ? ?/sec 1.16 455.6±57.85µs ? ?/sec world_query_for_each/50000_entities_SparseSet 1.02 121.3±18.36µs ? ?/sec 1.00 119.4±13.88µs ? ?/sec world_query_for_each/50000_entities_Table 1.03 13.8±0.96µs ? ?/sec 1.00 13.3±0.54µs ? ?/sec world_query_get/50000_entities_SparseSet 1.00 666.9±54.36µs ? ?/sec 1.03 687.1±57.77µs ? ?/sec world_query_get/50000_entities_Table 1.01 584.4±55.12µs ? ?/sec 1.00 576.3±36.13µs ? ?/sec world_query_iter/50000_entities_SparseSet 1.01 169.7±19.50µs ? ?/sec 1.00 168.6±32.56µs ? ?/sec world_query_iter/50000_entities_Table 1.00 26.2±1.38µs ? ?/sec 1.91 50.0±4.40µs ? ?/sec ``` I didn't add benchmarks for the mutable path but I don't see how it could hurt to make it inline too... |
||
|
Lukas Wirth
|
7c274e5a44 |
Improve bevy_ecs query docs (#1935)
Mainly documents Query, WorldQuery and the various Query Filter types as well as some smaller doc changes. |
||
|
davier
|
8acb0d2012 |
Fix cargo doc warnings (#1640)
Fixes all warnings from `cargo doc --all`. Those related to code blocks were introduced in #1612, but re-formatting using the experimental features in `rustfmt.toml` doesn't seem to reintroduce them. |
||
|
Alice Cecile
|
03601db51c |
Basic documentation for Entities, Components and Systems (#1578)
These are largely targeted at beginners, as `Entity`, `Component` and `System` are the most obvious terms to search when first getting introduced to Bevy. |
||
|
Carter Anderson
|
b17f8a4bce |
format comments (#1612)
Uses the new unstable comment formatting features added to rustfmt.toml. |
||
|
Carter Anderson
|
3a2a68852c |
Bevy ECS V2 (#1525)
# Bevy ECS V2
This is a rewrite of Bevy ECS (basically everything but the new executor/schedule, which are already awesome). The overall goal was to improve the performance and versatility of Bevy ECS. Here is a quick bulleted list of changes before we dive into the details:
* Complete World rewrite
* Multiple component storage types:
* Tables: fast cache friendly iteration, slower add/removes (previously called Archetypes)
* Sparse Sets: fast add/remove, slower iteration
* Stateful Queries (caches query results for faster iteration. fragmented iteration is _fast_ now)
* Stateful System Params (caches expensive operations. inspired by @DJMcNab's work in #1364)
* Configurable System Params (users can set configuration when they construct their systems. once again inspired by @DJMcNab's work)
* Archetypes are now "just metadata", component storage is separate
* Archetype Graph (for faster archetype changes)
* Component Metadata
* Configure component storage type
* Retrieve information about component size/type/name/layout/send-ness/etc
* Components are uniquely identified by a densely packed ComponentId
* TypeIds are now totally optional (which should make implementing scripting easier)
* Super fast "for_each" query iterators
* Merged Resources into World. Resources are now just a special type of component
* EntityRef/EntityMut builder apis (more efficient and more ergonomic)
* Fast bitset-backed `Access<T>` replaces old hashmap-based approach everywhere
* Query conflicts are determined by component access instead of archetype component access (to avoid random failures at runtime)
* With/Without are still taken into account for conflicts, so this should still be comfy to use
* Much simpler `IntoSystem` impl
* Significantly reduced the amount of hashing throughout the ecs in favor of Sparse Sets (indexed by densely packed ArchetypeId, ComponentId, BundleId, and TableId)
* Safety Improvements
* Entity reservation uses a normal world reference instead of unsafe transmute
* QuerySets no longer transmute lifetimes
* Made traits "unsafe" where relevant
* More thorough safety docs
* WorldCell
* Exposes safe mutable access to multiple resources at a time in a World
* Replaced "catch all" `System::update_archetypes(world: &World)` with `System::new_archetype(archetype: &Archetype)`
* Simpler Bundle implementation
* Replaced slow "remove_bundle_one_by_one" used as fallback for Commands::remove_bundle with fast "remove_bundle_intersection"
* Removed `Mut<T>` query impl. it is better to only support one way: `&mut T`
* Removed with() from `Flags<T>` in favor of `Option<Flags<T>>`, which allows querying for flags to be "filtered" by default
* Components now have is_send property (currently only resources support non-send)
* More granular module organization
* New `RemovedComponents<T>` SystemParam that replaces `query.removed::<T>()`
* `world.resource_scope()` for mutable access to resources and world at the same time
* WorldQuery and QueryFilter traits unified. FilterFetch trait added to enable "short circuit" filtering. Auto impled for cases that don't need it
* Significantly slimmed down SystemState in favor of individual SystemParam state
* System Commands changed from `commands: &mut Commands` back to `mut commands: Commands` (to allow Commands to have a World reference)
Fixes #1320
## `World` Rewrite
This is a from-scratch rewrite of `World` that fills the niche that `hecs` used to. Yes, this means Bevy ECS is no longer a "fork" of hecs. We're going out our own!
(the only shared code between the projects is the entity id allocator, which is already basically ideal)
A huge shout out to @SanderMertens (author of [flecs](https://github.com/SanderMertens/flecs)) for sharing some great ideas with me (specifically hybrid ecs storage and archetype graphs). He also helped advise on a number of implementation details.
## Component Storage (The Problem)
Two ECS storage paradigms have gained a lot of traction over the years:
* **Archetypal ECS**:
* Stores components in "tables" with static schemas. Each "column" stores components of a given type. Each "row" is an entity.
* Each "archetype" has its own table. Adding/removing an entity's component changes the archetype.
* Enables super-fast Query iteration due to its cache-friendly data layout
* Comes at the cost of more expensive add/remove operations for an Entity's components, because all components need to be copied to the new archetype's "table"
* **Sparse Set ECS**:
* Stores components of the same type in densely packed arrays, which are sparsely indexed by densely packed unsigned integers (Entity ids)
* Query iteration is slower than Archetypal ECS because each entity's component could be at any position in the sparse set. This "random access" pattern isn't cache friendly. Additionally, there is an extra layer of indirection because you must first map the entity id to an index in the component array.
* Adding/removing components is a cheap, constant time operation
Bevy ECS V1, hecs, legion, flec, and Unity DOTS are all "archetypal ecs-es". I personally think "archetypal" storage is a good default for game engines. An entity's archetype doesn't need to change frequently in general, and it creates "fast by default" query iteration (which is a much more common operation). It is also "self optimizing". Users don't need to think about optimizing component layouts for iteration performance. It "just works" without any extra boilerplate.
Shipyard and EnTT are "sparse set ecs-es". They employ "packing" as a way to work around the "suboptimal by default" iteration performance for specific sets of components. This helps, but I didn't think this was a good choice for a general purpose engine like Bevy because:
1. "packs" conflict with each other. If bevy decides to internally pack the Transform and GlobalTransform components, users are then blocked if they want to pack some custom component with Transform.
2. users need to take manual action to optimize
Developers selecting an ECS framework are stuck with a hard choice. Select an "archetypal" framework with "fast iteration everywhere" but without the ability to cheaply add/remove components, or select a "sparse set" framework to cheaply add/remove components but with slower iteration performance.
## Hybrid Component Storage (The Solution)
In Bevy ECS V2, we get to have our cake and eat it too. It now has _both_ of the component storage types above (and more can be added later if needed):
* **Tables** (aka "archetypal" storage)
* The default storage. If you don't configure anything, this is what you get
* Fast iteration by default
* Slower add/remove operations
* **Sparse Sets**
* Opt-in
* Slower iteration
* Faster add/remove operations
These storage types complement each other perfectly. By default Query iteration is fast. If developers know that they want to add/remove a component at high frequencies, they can set the storage to "sparse set":
```rust
world.register_component(
ComponentDescriptor:🆕:<MyComponent>(StorageType::SparseSet)
).unwrap();
```
## Archetypes
Archetypes are now "just metadata" ... they no longer store components directly. They do store:
* The `ComponentId`s of each of the Archetype's components (and that component's storage type)
* Archetypes are uniquely defined by their component layouts
* For example: entities with "table" components `[A, B, C]` _and_ "sparse set" components `[D, E]` will always be in the same archetype.
* The `TableId` associated with the archetype
* For now each archetype has exactly one table (which can have no components),
* There is a 1->Many relationship from Tables->Archetypes. A given table could have any number of archetype components stored in it:
* Ex: an entity with "table storage" components `[A, B, C]` and "sparse set" components `[D, E]` will share the same `[A, B, C]` table as an entity with `[A, B, C]` table component and `[F]` sparse set components.
* This 1->Many relationship is how we preserve fast "cache friendly" iteration performance when possible (more on this later)
* A list of entities that are in the archetype and the row id of the table they are in
* ArchetypeComponentIds
* unique densely packed identifiers for (ArchetypeId, ComponentId) pairs
* used by the schedule executor for cheap system access control
* "Archetype Graph Edges" (see the next section)
## The "Archetype Graph"
Archetype changes in Bevy (and a number of other archetypal ecs-es) have historically been expensive to compute. First, you need to allocate a new vector of the entity's current component ids, add or remove components based on the operation performed, sort it (to ensure it is order-independent), then hash it to find the archetype (if it exists). And thats all before we get to the _already_ expensive full copy of all components to the new table storage.
The solution is to build a "graph" of archetypes to cache these results. @SanderMertens first exposed me to the idea (and he got it from @gjroelofs, who came up with it). They propose adding directed edges between archetypes for add/remove component operations. If `ComponentId`s are densely packed, you can use sparse sets to cheaply jump between archetypes.
Bevy takes this one step further by using add/remove `Bundle` edges instead of `Component` edges. Bevy encourages the use of `Bundles` to group add/remove operations. This is largely for "clearer game logic" reasons, but it also helps cut down on the number of archetype changes required. `Bundles` now also have densely-packed `BundleId`s. This allows us to use a _single_ edge for each bundle operation (rather than needing to traverse N edges ... one for each component). Single component operations are also bundles, so this is strictly an improvement over a "component only" graph.
As a result, an operation that used to be _heavy_ (both for allocations and compute) is now two dirt-cheap array lookups and zero allocations.
## Stateful Queries
World queries are now stateful. This allows us to:
1. Cache archetype (and table) matches
* This resolves another issue with (naive) archetypal ECS: query performance getting worse as the number of archetypes goes up (and fragmentation occurs).
2. Cache Fetch and Filter state
* The expensive parts of fetch/filter operations (such as hashing the TypeId to find the ComponentId) now only happen once when the Query is first constructed
3. Incrementally build up state
* When new archetypes are added, we only process the new archetypes (no need to rebuild state for old archetypes)
As a result, the direct `World` query api now looks like this:
```rust
let mut query = world.query::<(&A, &mut B)>();
for (a, mut b) in query.iter_mut(&mut world) {
}
```
Requiring `World` to generate stateful queries (rather than letting the `QueryState` type be constructed separately) allows us to ensure that _all_ queries are properly initialized (and the relevant world state, such as ComponentIds). This enables QueryState to remove branches from its operations that check for initialization status (and also enables query.iter() to take an immutable world reference because it doesn't need to initialize anything in world).
However in systems, this is a non-breaking change. State management is done internally by the relevant SystemParam.
## Stateful SystemParams
Like Queries, `SystemParams` now also cache state. For example, `Query` system params store the "stateful query" state mentioned above. Commands store their internal `CommandQueue`. This means you can now safely use as many separate `Commands` parameters in your system as you want. `Local<T>` system params store their `T` value in their state (instead of in Resources).
SystemParam state also enabled a significant slim-down of SystemState. It is much nicer to look at now.
Per-SystemParam state naturally insulates us from an "aliased mut" class of errors we have hit in the past (ex: using multiple `Commands` system params).
(credit goes to @DJMcNab for the initial idea and draft pr here #1364)
## Configurable SystemParams
@DJMcNab also had the great idea to make SystemParams configurable. This allows users to provide some initial configuration / values for system parameters (when possible). Most SystemParams have no config (the config type is `()`), but the `Local<T>` param now supports user-provided parameters:
```rust
fn foo(value: Local<usize>) {
}
app.add_system(foo.system().config(|c| c.0 = Some(10)));
```
## Uber Fast "for_each" Query Iterators
Developers now have the choice to use a fast "for_each" iterator, which yields ~1.5-3x iteration speed improvements for "fragmented iteration", and minor ~1.2x iteration speed improvements for unfragmented iteration.
```rust
fn system(query: Query<(&A, &mut B)>) {
// you now have the option to do this for a speed boost
query.for_each_mut(|(a, mut b)| {
});
// however normal iterators are still available
for (a, mut b) in query.iter_mut() {
}
}
```
I think in most cases we should continue to encourage "normal" iterators as they are more flexible and more "rust idiomatic". But when that extra "oomf" is needed, it makes sense to use `for_each`.
We should also consider using `for_each` for internal bevy systems to give our users a nice speed boost (but that should be a separate pr).
## Component Metadata
`World` now has a `Components` collection, which is accessible via `world.components()`. This stores mappings from `ComponentId` to `ComponentInfo`, as well as `TypeId` to `ComponentId` mappings (where relevant). `ComponentInfo` stores information about the component, such as ComponentId, TypeId, memory layout, send-ness (currently limited to resources), and storage type.
## Significantly Cheaper `Access<T>`
We used to use `TypeAccess<TypeId>` to manage read/write component/archetype-component access. This was expensive because TypeIds must be hashed and compared individually. The parallel executor got around this by "condensing" type ids into bitset-backed access types. This worked, but it had to be re-generated from the `TypeAccess<TypeId>`sources every time archetypes changed.
This pr removes TypeAccess in favor of faster bitset access everywhere. We can do this thanks to the move to densely packed `ComponentId`s and `ArchetypeComponentId`s.
## Merged Resources into World
Resources had a lot of redundant functionality with Components. They stored typed data, they had access control, they had unique ids, they were queryable via SystemParams, etc. In fact the _only_ major difference between them was that they were unique (and didn't correlate to an entity).
Separate resources also had the downside of requiring a separate set of access controls, which meant the parallel executor needed to compare more bitsets per system and manage more state.
I initially got the "separate resources" idea from `legion`. I think that design was motivated by the fact that it made the direct world query/resource lifetime interactions more manageable. It certainly made our lives easier when using Resources alongside hecs/bevy_ecs. However we already have a construct for safely and ergonomically managing in-world lifetimes: systems (which use `Access<T>` internally).
This pr merges Resources into World:
```rust
world.insert_resource(1);
world.insert_resource(2.0);
let a = world.get_resource::<i32>().unwrap();
let mut b = world.get_resource_mut::<f64>().unwrap();
*b = 3.0;
```
Resources are now just a special kind of component. They have their own ComponentIds (and their own resource TypeId->ComponentId scope, so they don't conflict wit components of the same type). They are stored in a special "resource archetype", which stores components inside the archetype using a new `unique_components` sparse set (note that this sparse set could later be used to implement Tags). This allows us to keep the code size small by reusing existing datastructures (namely Column, Archetype, ComponentFlags, and ComponentInfo). This allows us the executor to use a single `Access<ArchetypeComponentId>` per system. It should also make scripting language integration easier.
_But_ this merge did create problems for people directly interacting with `World`. What if you need mutable access to multiple resources at the same time? `world.get_resource_mut()` borrows World mutably!
## WorldCell
WorldCell applies the `Access<ArchetypeComponentId>` concept to direct world access:
```rust
let world_cell = world.cell();
let a = world_cell.get_resource_mut::<i32>().unwrap();
let b = world_cell.get_resource_mut::<f64>().unwrap();
```
This adds cheap runtime checks (a sparse set lookup of `ArchetypeComponentId` and a counter) to ensure that world accesses do not conflict with each other. Each operation returns a `WorldBorrow<'w, T>` or `WorldBorrowMut<'w, T>` wrapper type, which will release the relevant ArchetypeComponentId resources when dropped.
World caches the access sparse set (and only one cell can exist at a time), so `world.cell()` is a cheap operation.
WorldCell does _not_ use atomic operations. It is non-send, does a mutable borrow of world to prevent other accesses, and uses a simple `Rc<RefCell<ArchetypeComponentAccess>>` wrapper in each WorldBorrow pointer.
The api is currently limited to resource access, but it can and should be extended to queries / entity component access.
## Resource Scopes
WorldCell does not yet support component queries, and even when it does there are sometimes legitimate reasons to want a mutable world ref _and_ a mutable resource ref (ex: bevy_render and bevy_scene both need this). In these cases we could always drop down to the unsafe `world.get_resource_unchecked_mut()`, but that is not ideal!
Instead developers can use a "resource scope"
```rust
world.resource_scope(|world: &mut World, a: &mut A| {
})
```
This temporarily removes the `A` resource from `World`, provides mutable pointers to both, and re-adds A to World when finished. Thanks to the move to ComponentIds/sparse sets, this is a cheap operation.
If multiple resources are required, scopes can be nested. We could also consider adding a "resource tuple" to the api if this pattern becomes common and the boilerplate gets nasty.
## Query Conflicts Use ComponentId Instead of ArchetypeComponentId
For safety reasons, systems cannot contain queries that conflict with each other without wrapping them in a QuerySet. On bevy `main`, we use ArchetypeComponentIds to determine conflicts. This is nice because it can take into account filters:
```rust
// these queries will never conflict due to their filters
fn filter_system(a: Query<&mut A, With<B>>, b: Query<&mut B, Without<B>>) {
}
```
But it also has a significant downside:
```rust
// these queries will not conflict _until_ an entity with A, B, and C is spawned
fn maybe_conflicts_system(a: Query<(&mut A, &C)>, b: Query<(&mut A, &B)>) {
}
```
The system above will panic at runtime if an entity with A, B, and C is spawned. This makes it hard to trust that your game logic will run without crashing.
In this pr, I switched to using `ComponentId` instead. This _is_ more constraining. `maybe_conflicts_system` will now always fail, but it will do it consistently at startup. Naively, it would also _disallow_ `filter_system`, which would be a significant downgrade in usability. Bevy has a number of internal systems that rely on disjoint queries and I expect it to be a common pattern in userspace.
To resolve this, I added a new `FilteredAccess<T>` type, which wraps `Access<T>` and adds with/without filters. If two `FilteredAccess` have with/without values that prove they are disjoint, they will no longer conflict.
## EntityRef / EntityMut
World entity operations on `main` require that the user passes in an `entity` id to each operation:
```rust
let entity = world.spawn((A, )); // create a new entity with A
world.get::<A>(entity);
world.insert(entity, (B, C));
world.insert_one(entity, D);
```
This means that each operation needs to look up the entity location / verify its validity. The initial spawn operation also requires a Bundle as input. This can be awkward when no components are required (or one component is required).
These operations have been replaced by `EntityRef` and `EntityMut`, which are "builder-style" wrappers around world that provide read and read/write operations on a single, pre-validated entity:
```rust
// spawn now takes no inputs and returns an EntityMut
let entity = world.spawn()
.insert(A) // insert a single component into the entity
.insert_bundle((B, C)) // insert a bundle of components into the entity
.id() // id returns the Entity id
// Returns EntityMut (or panics if the entity does not exist)
world.entity_mut(entity)
.insert(D)
.insert_bundle(SomeBundle::default());
{
// returns EntityRef (or panics if the entity does not exist)
let d = world.entity(entity)
.get::<D>() // gets the D component
.unwrap();
// world.get still exists for ergonomics
let d = world.get::<D>(entity).unwrap();
}
// These variants return Options if you want to check existence instead of panicing
world.get_entity_mut(entity)
.unwrap()
.insert(E);
if let Some(entity_ref) = world.get_entity(entity) {
let d = entity_ref.get::<D>().unwrap();
}
```
This _does not_ affect the current Commands api or terminology. I think that should be a separate conversation as that is a much larger breaking change.
## Safety Improvements
* Entity reservation in Commands uses a normal world borrow instead of an unsafe transmute
* QuerySets no longer transmutes lifetimes
* Made traits "unsafe" when implementing a trait incorrectly could cause unsafety
* More thorough safety docs
## RemovedComponents SystemParam
The old approach to querying removed components: `query.removed:<T>()` was confusing because it had no connection to the query itself. I replaced it with the following, which is both clearer and allows us to cache the ComponentId mapping in the SystemParamState:
```rust
fn system(removed: RemovedComponents<T>) {
for entity in removed.iter() {
}
}
```
## Simpler Bundle implementation
Bundles are no longer responsible for sorting (or deduping) TypeInfo. They are just a simple ordered list of component types / data. This makes the implementation smaller and opens the door to an easy "nested bundle" implementation in the future (which i might even add in this pr). Duplicate detection is now done once per bundle type by World the first time a bundle is used.
## Unified WorldQuery and QueryFilter types
(don't worry they are still separate type _parameters_ in Queries .. this is a non-breaking change)
WorldQuery and QueryFilter were already basically identical apis. With the addition of `FetchState` and more storage-specific fetch methods, the overlap was even clearer (and the redundancy more painful).
QueryFilters are now just `F: WorldQuery where F::Fetch: FilterFetch`. FilterFetch requires `Fetch<Item = bool>` and adds new "short circuit" variants of fetch methods. This enables a filter tuple like `(With<A>, Without<B>, Changed<C>)` to stop evaluating the filter after the first mismatch is encountered. FilterFetch is automatically implemented for `Fetch` implementations that return bool.
This forces fetch implementations that return things like `(bool, bool, bool)` (such as the filter above) to manually implement FilterFetch and decide whether or not to short-circuit.
## More Granular Modules
World no longer globs all of the internal modules together. It now exports `core`, `system`, and `schedule` separately. I'm also considering exporting `core` submodules directly as that is still pretty "glob-ey" and unorganized (feedback welcome here).
## Remaining Draft Work (to be done in this pr)
* ~~panic on conflicting WorldQuery fetches (&A, &mut A)~~
* ~~bevy `main` and hecs both currently allow this, but we should protect against it if possible~~
* ~~batch_iter / par_iter (currently stubbed out)~~
* ~~ChangedRes~~
* ~~I skipped this while we sort out #1313. This pr should be adapted to account for whatever we land on there~~.
* ~~The `Archetypes` and `Tables` collections use hashes of sorted lists of component ids to uniquely identify each archetype/table. This hash is then used as the key in a HashMap to look up the relevant ArchetypeId or TableId. (which doesn't handle hash collisions properly)~~
* ~~It is currently unsafe to generate a Query from "World A", then use it on "World B" (despite the api claiming it is safe). We should probably close this gap. This could be done by adding a randomly generated WorldId to each world, then storing that id in each Query. They could then be compared to each other on each `query.do_thing(&world)` operation. This _does_ add an extra branch to each query operation, so I'm open to other suggestions if people have them.~~
* ~~Nested Bundles (if i find time)~~
## Potential Future Work
* Expand WorldCell to support queries.
* Consider not allocating in the empty archetype on `world.spawn()`
* ex: return something like EntityMutUninit, which turns into EntityMut after an `insert` or `insert_bundle` op
* this actually regressed performance last time i tried it, but in theory it should be faster
* Optimize SparseSet::insert (see `PERF` comment on insert)
* Replace SparseArray `Option<T>` with T::MAX to cut down on branching
* would enable cheaper get_unchecked() operations
* upstream fixedbitset optimizations
* fixedbitset could be allocation free for small block counts (store blocks in a SmallVec)
* fixedbitset could have a const constructor
* Consider implementing Tags (archetype-specific by-value data that affects archetype identity)
* ex: ArchetypeA could have `[A, B, C]` table components and `[D(1)]` "tag" component. ArchetypeB could have `[A, B, C]` table components and a `[D(2)]` tag component. The archetypes are different, despite both having D tags because the value inside D is different.
* this could potentially build on top of the `archetype.unique_components` added in this pr for resource storage.
* Consider reverting `all_tuples` proc macro in favor of the old `macro_rules` implementation
* all_tuples is more flexible and produces cleaner documentation (the macro_rules version produces weird type parameter orders due to parser constraints)
* but unfortunately all_tuples also appears to make Rust Analyzer sad/slow when working inside of `bevy_ecs` (does not affect user code)
* Consider "resource queries" and/or "mixed resource and entity component queries" as an alternative to WorldCell
* this is basically just "systems" so maybe it's not worth it
* Add more world ops
* `world.clear()`
* `world.reserve<T: Bundle>(count: usize)`
* Try using the old archetype allocation strategy (allocate new memory on resize and copy everything over). I expect this to improve batch insertion performance at the cost of unbatched performance. But thats just a guess. I'm not an allocation perf pro :)
* Adapt Commands apis for consistency with new World apis
## Benchmarks
key:
* `bevy_old`: bevy `main` branch
* `bevy`: this branch
* `_foreach`: uses an optimized for_each iterator
* ` _sparse`: uses sparse set storage (if unspecified assume table storage)
* `_system`: runs inside a system (if unspecified assume test happens via direct world ops)
### Simple Insert (from ecs_bench_suite)
### Simpler Iter (from ecs_bench_suite)
### Fragment Iter (from ecs_bench_suite)
### Sparse Fragmented Iter
Iterate a query that matches 5 entities from a single matching archetype, but there are 100 unmatching archetypes
### Schedule (from ecs_bench_suite)
### Add Remove Component (from ecs_bench_suite)
### Add Remove Component Big
Same as the test above, but each entity has 5 "large" matrix components and 1 "large" matrix component is added and removed
### Get Component
Looks up a single component value a large number of times
|
