diff --git a/crates/bevy_ecs/Cargo.toml b/crates/bevy_ecs/Cargo.toml index 173de89ad3..125054041a 100644 --- a/crates/bevy_ecs/Cargo.toml +++ b/crates/bevy_ecs/Cargo.toml @@ -34,6 +34,7 @@ serde = { version = "1", optional = true, default-features = false } thiserror = "1.0" nonmax = "0.5" arrayvec = { version = "0.7.4", optional = true } +smallvec = "1" [dev-dependencies] rand = "0.8" diff --git a/crates/bevy_ecs/src/archetype.rs b/crates/bevy_ecs/src/archetype.rs index 5b56e1cff4..7df84b87c4 100644 --- a/crates/bevy_ecs/src/archetype.rs +++ b/crates/bevy_ecs/src/archetype.rs @@ -499,6 +499,16 @@ impl Archetype { self.components.len() } + /// Gets an iterator of all of the components in the archetype, along with + /// their archetype component ID. + pub(crate) fn components_with_archetype_component_id( + &self, + ) -> impl Iterator + '_ { + self.components + .iter() + .map(|(component_id, info)| (*component_id, info.archetype_component_id)) + } + /// Fetches an immutable reference to the archetype's [`Edges`], a cache of /// archetypal relationships. #[inline] diff --git a/crates/bevy_ecs/src/query/access.rs b/crates/bevy_ecs/src/query/access.rs index 6769a754ca..0f5fb318af 100644 --- a/crates/bevy_ecs/src/query/access.rs +++ b/crates/bevy_ecs/src/query/access.rs @@ -49,20 +49,22 @@ impl<'a, T: SparseSetIndex + Debug> Debug for FormattedBitSet<'a, T> { /// See the [`is_compatible`](Access::is_compatible) and [`get_conflicts`](Access::get_conflicts) functions. #[derive(Eq, PartialEq)] pub struct Access { - /// All accessed components. + /// All accessed components, or forbidden components if + /// `Self::component_read_and_writes_inverted` is set. component_read_and_writes: FixedBitSet, - /// The exclusively-accessed components. + /// All exclusively-accessed components, or components that may not be + /// exclusively accessed if `Self::component_writes_inverted` is set. component_writes: FixedBitSet, /// All accessed resources. resource_read_and_writes: FixedBitSet, /// The exclusively-accessed resources. resource_writes: FixedBitSet, - /// Is `true` if this has access to all components. - /// (Note that this does not include `Resources`) - reads_all_components: bool, - /// Is `true` if this has mutable access to all components. - /// (Note that this does not include `Resources`) - writes_all_components: bool, + /// Is `true` if this component can read all components *except* those + /// present in `Self::component_read_and_writes`. + component_read_and_writes_inverted: bool, + /// Is `true` if this component can write to all components *except* those + /// present in `Self::component_writes`. + component_writes_inverted: bool, /// Is `true` if this has access to all resources. /// This field is a performance optimization for `&World` (also harder to mess up for soundness). reads_all_resources: bool, @@ -82,8 +84,8 @@ impl Clone for Access { component_writes: self.component_writes.clone(), resource_read_and_writes: self.resource_read_and_writes.clone(), resource_writes: self.resource_writes.clone(), - reads_all_components: self.reads_all_components, - writes_all_components: self.writes_all_components, + component_read_and_writes_inverted: self.component_read_and_writes_inverted, + component_writes_inverted: self.component_writes_inverted, reads_all_resources: self.reads_all_resources, writes_all_resources: self.writes_all_resources, archetypal: self.archetypal.clone(), @@ -98,8 +100,8 @@ impl Clone for Access { self.resource_read_and_writes .clone_from(&source.resource_read_and_writes); self.resource_writes.clone_from(&source.resource_writes); - self.reads_all_components = source.reads_all_components; - self.writes_all_components = source.writes_all_components; + self.component_read_and_writes_inverted = source.component_read_and_writes_inverted; + self.component_writes_inverted = source.component_writes_inverted; self.reads_all_resources = source.reads_all_resources; self.writes_all_resources = source.writes_all_resources; self.archetypal.clone_from(&source.archetypal); @@ -125,8 +127,11 @@ impl Debug for Access { "resource_writes", &FormattedBitSet::::new(&self.resource_writes), ) - .field("reads_all_components", &self.reads_all_components) - .field("writes_all_components", &self.writes_all_components) + .field( + "component_read_and_writes_inverted", + &self.component_read_and_writes_inverted, + ) + .field("component_writes_inverted", &self.component_writes_inverted) .field("reads_all_resources", &self.reads_all_resources) .field("writes_all_resources", &self.writes_all_resources) .field("archetypal", &FormattedBitSet::::new(&self.archetypal)) @@ -146,8 +151,8 @@ impl Access { Self { reads_all_resources: false, writes_all_resources: false, - reads_all_components: false, - writes_all_components: false, + component_read_and_writes_inverted: false, + component_writes_inverted: false, component_read_and_writes: FixedBitSet::new(), component_writes: FixedBitSet::new(), resource_read_and_writes: FixedBitSet::new(), @@ -157,18 +162,33 @@ impl Access { } } + fn add_component_sparse_set_index_read(&mut self, index: usize) { + if !self.component_read_and_writes_inverted { + self.component_read_and_writes.grow_and_insert(index); + } else if index < self.component_read_and_writes.len() { + self.component_read_and_writes.remove(index); + } + } + + fn add_component_sparse_set_index_write(&mut self, index: usize) { + if !self.component_writes_inverted { + self.component_writes.grow_and_insert(index); + } else if index < self.component_writes.len() { + self.component_writes.remove(index); + } + } + /// Adds access to the component given by `index`. pub fn add_component_read(&mut self, index: T) { - self.component_read_and_writes - .grow_and_insert(index.sparse_set_index()); + let sparse_set_index = index.sparse_set_index(); + self.add_component_sparse_set_index_read(sparse_set_index); } /// Adds exclusive access to the component given by `index`. pub fn add_component_write(&mut self, index: T) { - self.component_read_and_writes - .grow_and_insert(index.sparse_set_index()); - self.component_writes - .grow_and_insert(index.sparse_set_index()); + let sparse_set_index = index.sparse_set_index(); + self.add_component_sparse_set_index_read(sparse_set_index); + self.add_component_sparse_set_index_write(sparse_set_index); } /// Adds access to the resource given by `index`. @@ -185,6 +205,49 @@ impl Access { .grow_and_insert(index.sparse_set_index()); } + fn remove_component_sparse_set_index_read(&mut self, index: usize) { + if self.component_read_and_writes_inverted { + self.component_read_and_writes.grow_and_insert(index); + } else if index < self.component_read_and_writes.len() { + self.component_read_and_writes.remove(index); + } + } + + fn remove_component_sparse_set_index_write(&mut self, index: usize) { + if self.component_writes_inverted { + self.component_writes.grow_and_insert(index); + } else if index < self.component_writes.len() { + self.component_writes.remove(index); + } + } + + /// Removes both read and write access to the component given by `index`. + /// + /// Because this method corresponds to the set difference operator ∖, it can + /// create complicated logical formulas that you should verify correctness + /// of. For example, A ∪ (B ∖ A) isn't equivalent to (A ∪ B) ∖ A, so you + /// can't replace a call to `remove_component_read` followed by a call to + /// `extend` with a call to `extend` followed by a call to + /// `remove_component_read`. + pub fn remove_component_read(&mut self, index: T) { + let sparse_set_index = index.sparse_set_index(); + self.remove_component_sparse_set_index_write(sparse_set_index); + self.remove_component_sparse_set_index_read(sparse_set_index); + } + + /// Removes write access to the component given by `index`. + /// + /// Because this method corresponds to the set difference operator ∖, it can + /// create complicated logical formulas that you should verify correctness + /// of. For example, A ∪ (B ∖ A) isn't equivalent to (A ∪ B) ∖ A, so you + /// can't replace a call to `remove_component_write` followed by a call to + /// `extend` with a call to `extend` followed by a call to + /// `remove_component_write`. + pub fn remove_component_write(&mut self, index: T) { + let sparse_set_index = index.sparse_set_index(); + self.remove_component_sparse_set_index_write(sparse_set_index); + } + /// Adds an archetypal (indirect) access to the component given by `index`. /// /// This is for components whose values are not accessed (and thus will never cause conflicts), @@ -199,25 +262,25 @@ impl Access { /// Returns `true` if this can access the component given by `index`. pub fn has_component_read(&self, index: T) -> bool { - self.reads_all_components - || self + self.component_read_and_writes_inverted + ^ self .component_read_and_writes .contains(index.sparse_set_index()) } /// Returns `true` if this can access any component. pub fn has_any_component_read(&self) -> bool { - self.reads_all_components || !self.component_read_and_writes.is_clear() + self.component_read_and_writes_inverted || !self.component_read_and_writes.is_clear() } /// Returns `true` if this can exclusively access the component given by `index`. pub fn has_component_write(&self, index: T) -> bool { - self.writes_all_components || self.component_writes.contains(index.sparse_set_index()) + self.component_writes_inverted ^ self.component_writes.contains(index.sparse_set_index()) } /// Returns `true` if this accesses any component mutably. pub fn has_any_component_write(&self) -> bool { - self.writes_all_components || !self.component_writes.is_clear() + self.component_writes_inverted || !self.component_writes.is_clear() } /// Returns `true` if this can access the resource given by `index`. @@ -258,14 +321,16 @@ impl Access { /// Sets this as having access to all components (i.e. `EntityRef`). #[inline] pub fn read_all_components(&mut self) { - self.reads_all_components = true; + self.component_read_and_writes_inverted = true; + self.component_read_and_writes.clear(); } /// Sets this as having mutable access to all components (i.e. `EntityMut`). #[inline] pub fn write_all_components(&mut self) { - self.reads_all_components = true; - self.writes_all_components = true; + self.read_all_components(); + self.component_writes_inverted = true; + self.component_writes.clear(); } /// Sets this as having access to all resources (i.e. `&World`). @@ -298,13 +363,13 @@ impl Access { /// Returns `true` if this has access to all components (i.e. `EntityRef`). #[inline] pub fn has_read_all_components(&self) -> bool { - self.reads_all_components + self.component_read_and_writes_inverted && self.component_read_and_writes.is_clear() } /// Returns `true` if this has write access to all components (i.e. `EntityMut`). #[inline] pub fn has_write_all_components(&self) -> bool { - self.writes_all_components + self.component_writes_inverted && self.component_writes.is_clear() } /// Returns `true` if this has access to all resources (i.e. `EntityRef`). @@ -332,7 +397,7 @@ impl Access { /// Removes all writes. pub fn clear_writes(&mut self) { self.writes_all_resources = false; - self.writes_all_components = false; + self.component_writes_inverted = false; self.component_writes.clear(); self.resource_writes.clear(); } @@ -341,8 +406,8 @@ impl Access { pub fn clear(&mut self) { self.reads_all_resources = false; self.writes_all_resources = false; - self.reads_all_components = false; - self.writes_all_components = false; + self.component_read_and_writes_inverted = false; + self.component_writes_inverted = false; self.component_read_and_writes.clear(); self.component_writes.clear(); self.resource_read_and_writes.clear(); @@ -351,13 +416,72 @@ impl Access { /// Adds all access from `other`. pub fn extend(&mut self, other: &Access) { + let component_read_and_writes_inverted = + self.component_read_and_writes_inverted || other.component_read_and_writes_inverted; + let component_writes_inverted = + self.component_writes_inverted || other.component_writes_inverted; + + match ( + self.component_read_and_writes_inverted, + other.component_read_and_writes_inverted, + ) { + (true, true) => { + self.component_read_and_writes + .intersect_with(&other.component_read_and_writes); + } + (true, false) => { + self.component_read_and_writes + .difference_with(&other.component_read_and_writes); + } + (false, true) => { + // We have to grow here because the new bits are going to get flipped to 1. + self.component_read_and_writes.grow( + self.component_read_and_writes + .len() + .max(other.component_read_and_writes.len()), + ); + self.component_read_and_writes.toggle_range(..); + self.component_read_and_writes + .intersect_with(&other.component_read_and_writes); + } + (false, false) => { + self.component_read_and_writes + .union_with(&other.component_read_and_writes); + } + } + + match ( + self.component_writes_inverted, + other.component_writes_inverted, + ) { + (true, true) => { + self.component_writes + .intersect_with(&other.component_writes); + } + (true, false) => { + self.component_writes + .difference_with(&other.component_writes); + } + (false, true) => { + // We have to grow here because the new bits are going to get flipped to 1. + self.component_writes.grow( + self.component_writes + .len() + .max(other.component_writes.len()), + ); + self.component_writes.toggle_range(..); + self.component_writes + .intersect_with(&other.component_writes); + } + (false, false) => { + self.component_writes.union_with(&other.component_writes); + } + } + self.reads_all_resources = self.reads_all_resources || other.reads_all_resources; self.writes_all_resources = self.writes_all_resources || other.writes_all_resources; - self.reads_all_components = self.reads_all_components || other.reads_all_components; - self.writes_all_components = self.writes_all_components || other.writes_all_components; - self.component_read_and_writes - .union_with(&other.component_read_and_writes); - self.component_writes.union_with(&other.component_writes); + self.component_read_and_writes_inverted = component_read_and_writes_inverted; + self.component_writes_inverted = component_writes_inverted; self.resource_read_and_writes .union_with(&other.resource_read_and_writes); self.resource_writes.union_with(&other.resource_writes); @@ -369,27 +493,48 @@ impl Access { /// [`Access`] instances are incompatible if one can write /// an element that the other can read or write. pub fn is_components_compatible(&self, other: &Access) -> bool { - if self.writes_all_components { - return !other.has_any_component_read(); + // We have a conflict if we write and they read or write, or if they + // write and we read or write. + for ( + lhs_writes, + rhs_reads_and_writes, + lhs_writes_inverted, + rhs_reads_and_writes_inverted, + ) in [ + ( + &self.component_writes, + &other.component_read_and_writes, + self.component_writes_inverted, + other.component_read_and_writes_inverted, + ), + ( + &other.component_writes, + &self.component_read_and_writes, + other.component_writes_inverted, + self.component_read_and_writes_inverted, + ), + ] { + match (lhs_writes_inverted, rhs_reads_and_writes_inverted) { + (true, true) => return false, + (false, true) => { + if !lhs_writes.is_subset(rhs_reads_and_writes) { + return false; + } + } + (true, false) => { + if !rhs_reads_and_writes.is_subset(lhs_writes) { + return false; + } + } + (false, false) => { + if !lhs_writes.is_disjoint(rhs_reads_and_writes) { + return false; + } + } + } } - if other.writes_all_components { - return !self.has_any_component_read(); - } - - if self.reads_all_components { - return !other.has_any_component_write(); - } - - if other.reads_all_components { - return !self.has_any_component_write(); - } - - self.component_writes - .is_disjoint(&other.component_read_and_writes) - && other - .component_writes - .is_disjoint(&self.component_read_and_writes) + true } /// Returns `true` if the access and `other` can be active at the same time, @@ -432,25 +577,48 @@ impl Access { /// Returns `true` if the set's component access is a subset of another, i.e. `other`'s component access /// contains at least all the values in `self`. pub fn is_subset_components(&self, other: &Access) -> bool { - if self.writes_all_components { - return other.writes_all_components; + for ( + our_components, + their_components, + our_components_inverted, + their_components_inverted, + ) in [ + ( + &self.component_read_and_writes, + &other.component_read_and_writes, + self.component_read_and_writes_inverted, + other.component_read_and_writes_inverted, + ), + ( + &self.component_writes, + &other.component_writes, + self.component_writes_inverted, + other.component_writes_inverted, + ), + ] { + match (our_components_inverted, their_components_inverted) { + (true, true) => { + if !their_components.is_subset(our_components) { + return false; + } + } + (true, false) => { + return false; + } + (false, true) => { + if !our_components.is_disjoint(their_components) { + return false; + } + } + (false, false) => { + if !our_components.is_subset(their_components) { + return false; + } + } + } } - if other.writes_all_components { - return true; - } - - if self.reads_all_components { - return other.reads_all_components; - } - - if other.reads_all_components { - return self.component_writes.is_subset(&other.component_writes); - } - - self.component_read_and_writes - .is_subset(&other.component_read_and_writes) - && self.component_writes.is_subset(&other.component_writes) + true } /// Returns `true` if the set's resource access is a subset of another, i.e. `other`'s resource access @@ -483,30 +651,52 @@ impl Access { self.is_subset_components(other) && self.is_subset_resources(other) } + fn get_component_conflicts(&self, other: &Access) -> AccessConflicts { + let mut conflicts = FixedBitSet::new(); + + // We have a conflict if we write and they read or write, or if they + // write and we read or write. + for ( + lhs_writes, + rhs_reads_and_writes, + lhs_writes_inverted, + rhs_reads_and_writes_inverted, + ) in [ + ( + &self.component_writes, + &other.component_read_and_writes, + self.component_writes_inverted, + other.component_read_and_writes_inverted, + ), + ( + &other.component_writes, + &self.component_read_and_writes, + other.component_writes_inverted, + self.component_read_and_writes_inverted, + ), + ] { + // There's no way that I can see to do this without a temporary. + // Neither CNF nor DNF allows us to avoid one. + let temp_conflicts: FixedBitSet = + match (lhs_writes_inverted, rhs_reads_and_writes_inverted) { + (true, true) => return AccessConflicts::All, + (false, true) => lhs_writes.difference(rhs_reads_and_writes).collect(), + (true, false) => rhs_reads_and_writes.difference(lhs_writes).collect(), + (false, false) => lhs_writes.intersection(rhs_reads_and_writes).collect(), + }; + conflicts.union_with(&temp_conflicts); + } + + AccessConflicts::Individual(conflicts) + } + /// Returns a vector of elements that the access and `other` cannot access at the same time. pub fn get_conflicts(&self, other: &Access) -> AccessConflicts { - let mut conflicts = FixedBitSet::new(); - if self.reads_all_components { - if other.writes_all_components { - return AccessConflicts::All; - } - conflicts.extend(other.component_writes.ones()); - } + let mut conflicts = match self.get_component_conflicts(other) { + AccessConflicts::All => return AccessConflicts::All, + AccessConflicts::Individual(conflicts) => conflicts, + }; - if other.reads_all_components { - if self.writes_all_components { - return AccessConflicts::All; - } - conflicts.extend(self.component_writes.ones()); - } - - if self.writes_all_components { - conflicts.extend(other.component_read_and_writes.ones()); - } - - if other.writes_all_components { - conflicts.extend(self.component_read_and_writes.ones()); - } if self.reads_all_resources { if other.writes_all_resources { return AccessConflicts::All; @@ -528,14 +718,6 @@ impl Access { conflicts.extend(self.resource_read_and_writes.ones()); } - conflicts.extend( - self.component_writes - .intersection(&other.component_read_and_writes), - ); - conflicts.extend( - self.component_read_and_writes - .intersection(&other.component_writes), - ); conflicts.extend( self.resource_writes .intersection(&other.resource_read_and_writes), @@ -547,25 +729,6 @@ impl Access { AccessConflicts::Individual(conflicts) } - /// Returns the indices of the components this has access to. - pub fn component_reads_and_writes(&self) -> impl Iterator + '_ { - self.component_read_and_writes - .ones() - .map(T::get_sparse_set_index) - } - - /// Returns the indices of the components this has non-exclusive access to. - pub fn component_reads(&self) -> impl Iterator + '_ { - self.component_read_and_writes - .difference(&self.component_writes) - .map(T::get_sparse_set_index) - } - - /// Returns the indices of the components this has exclusive access to. - pub fn component_writes(&self) -> impl Iterator + '_ { - self.component_writes.ones().map(T::get_sparse_set_index) - } - /// Returns the indices of the components that this has an archetypal access to. /// /// These are components whose values are not accessed (and thus will never cause conflicts), @@ -577,6 +740,40 @@ impl Access { pub fn archetypal(&self) -> impl Iterator + '_ { self.archetypal.ones().map(T::get_sparse_set_index) } + + /// Returns an iterator over the component IDs that this `Access` either + /// reads and writes or can't read or write. + /// + /// The returned flag specifies whether the list consists of the components + /// that the access *can* read or write (false) or whether the list consists + /// of the components that the access *can't* read or write (true). + /// + /// Because this method depends on internal implementation details of + /// `Access`, it's not recommended. Prefer to manage your own lists of + /// accessible components if your application needs to do that. + #[doc(hidden)] + #[deprecated] + pub fn component_reads_and_writes(&self) -> (impl Iterator + '_, bool) { + ( + self.component_read_and_writes + .ones() + .map(T::get_sparse_set_index), + self.component_read_and_writes_inverted, + ) + } + + /// Returns an iterator over the component IDs that this `Access` either + /// writes or can't write. + /// + /// The returned flag specifies whether the list consists of the components + /// that the access *can* write (false) or whether the list consists of the + /// components that the access *can't* write (true). + pub(crate) fn component_writes(&self) -> (impl Iterator + '_, bool) { + ( + self.component_writes.ones().map(T::get_sparse_set_index), + self.component_writes_inverted, + ) + } } /// An [`Access`] that has been filtered to include and exclude certain combinations of elements. diff --git a/crates/bevy_ecs/src/query/builder.rs b/crates/bevy_ecs/src/query/builder.rs index 94a8af27c5..2f4217e711 100644 --- a/crates/bevy_ecs/src/query/builder.rs +++ b/crates/bevy_ecs/src/query/builder.rs @@ -79,10 +79,14 @@ impl<'w, D: QueryData, F: QueryFilter> QueryBuilder<'w, D, F> { .map_or(false, |info| info.storage_type() == StorageType::Table) }; - self.access - .access() - .component_reads_and_writes() - .all(is_dense) + #[allow(deprecated)] + let (mut component_reads_and_writes, component_reads_and_writes_inverted) = + self.access.access().component_reads_and_writes(); + if component_reads_and_writes_inverted { + return false; + } + + component_reads_and_writes.all(is_dense) && self.access.access().archetypal().all(is_dense) && !self.access.access().has_read_all_components() && self.access.with_filters().all(is_dense) diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 4009523718..311ddcc9aa 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -1,17 +1,19 @@ use crate::{ archetype::{Archetype, Archetypes}, + bundle::Bundle, change_detection::{MaybeThinSlicePtrLocation, Ticks, TicksMut}, component::{Component, ComponentId, Components, StorageType, Tick}, entity::{Entities, Entity, EntityLocation}, query::{Access, DebugCheckedUnwrap, FilteredAccess, WorldQuery}, storage::{ComponentSparseSet, Table, TableRow}, world::{ - unsafe_world_cell::UnsafeWorldCell, EntityMut, EntityRef, FilteredEntityMut, - FilteredEntityRef, Mut, Ref, World, + unsafe_world_cell::UnsafeWorldCell, EntityMut, EntityMutExcept, EntityRef, EntityRefExcept, + FilteredEntityMut, FilteredEntityRef, Mut, Ref, World, }, }; use bevy_ptr::{ThinSlicePtr, UnsafeCellDeref}; use bevy_utils::all_tuples; +use smallvec::SmallVec; use std::{cell::UnsafeCell, marker::PhantomData}; /// Types that can be fetched from a [`World`] using a [`Query`]. @@ -626,27 +628,15 @@ unsafe impl<'a> WorldQuery for FilteredEntityRef<'a> { unsafe fn set_archetype<'w>( fetch: &mut Self::Fetch<'w>, state: &Self::State, - archetype: &'w Archetype, + _: &'w Archetype, _table: &Table, ) { - let mut access = Access::default(); - state.access.component_reads().for_each(|id| { - if archetype.contains(id) { - access.add_component_read(id); - } - }); - fetch.1 = access; + fetch.1.clone_from(&state.access); } #[inline] - unsafe fn set_table<'w>(fetch: &mut Self::Fetch<'w>, state: &Self::State, table: &'w Table) { - let mut access = Access::default(); - state.access.component_reads().for_each(|id| { - if table.has_column(id) { - access.add_component_read(id); - } - }); - fetch.1 = access; + unsafe fn set_table<'w>(fetch: &mut Self::Fetch<'w>, state: &Self::State, _: &'w Table) { + fetch.1.clone_from(&state.access); } #[inline] @@ -733,37 +723,15 @@ unsafe impl<'a> WorldQuery for FilteredEntityMut<'a> { unsafe fn set_archetype<'w>( fetch: &mut Self::Fetch<'w>, state: &Self::State, - archetype: &'w Archetype, + _: &'w Archetype, _table: &Table, ) { - let mut access = Access::default(); - state.access.component_reads().for_each(|id| { - if archetype.contains(id) { - access.add_component_read(id); - } - }); - state.access.component_writes().for_each(|id| { - if archetype.contains(id) { - access.add_component_write(id); - } - }); - fetch.1 = access; + fetch.1.clone_from(&state.access); } #[inline] - unsafe fn set_table<'w>(fetch: &mut Self::Fetch<'w>, state: &Self::State, table: &'w Table) { - let mut access = Access::default(); - state.access.component_reads().for_each(|id| { - if table.has_column(id) { - access.add_component_read(id); - } - }); - state.access.component_writes().for_each(|id| { - if table.has_column(id) { - access.add_component_write(id); - } - }); - fetch.1 = access; + unsafe fn set_table<'w>(fetch: &mut Self::Fetch<'w>, state: &Self::State, _: &'w Table) { + fetch.1.clone_from(&state.access); } #[inline] @@ -815,6 +783,201 @@ unsafe impl<'a> QueryData for FilteredEntityMut<'a> { type ReadOnly = FilteredEntityRef<'a>; } +/// SAFETY: `EntityRefExcept` guards access to all components in the bundle `B` +/// and populates `Access` values so that queries that conflict with this access +/// are rejected. +unsafe impl<'a, B> WorldQuery for EntityRefExcept<'a, B> +where + B: Bundle, +{ + type Fetch<'w> = UnsafeWorldCell<'w>; + type Item<'w> = EntityRefExcept<'w, B>; + type State = SmallVec<[ComponentId; 4]>; + + fn shrink<'wlong: 'wshort, 'wshort>(item: Self::Item<'wlong>) -> Self::Item<'wshort> { + item + } + + fn shrink_fetch<'wlong: 'wshort, 'wshort>(fetch: Self::Fetch<'wlong>) -> Self::Fetch<'wshort> { + fetch + } + + unsafe fn init_fetch<'w>( + world: UnsafeWorldCell<'w>, + _: &Self::State, + _: Tick, + _: Tick, + ) -> Self::Fetch<'w> { + world + } + + const IS_DENSE: bool = true; + + unsafe fn set_archetype<'w>( + _: &mut Self::Fetch<'w>, + _: &Self::State, + _: &'w Archetype, + _: &'w Table, + ) { + } + + unsafe fn set_table<'w>(_: &mut Self::Fetch<'w>, _: &Self::State, _: &'w Table) {} + + unsafe fn fetch<'w>( + world: &mut Self::Fetch<'w>, + entity: Entity, + _: TableRow, + ) -> Self::Item<'w> { + let cell = world.get_entity(entity).unwrap(); + EntityRefExcept::new(cell) + } + + fn update_component_access( + state: &Self::State, + filtered_access: &mut FilteredAccess, + ) { + let mut my_access = Access::new(); + my_access.read_all_components(); + for id in state { + my_access.remove_component_read(*id); + } + + let access = filtered_access.access_mut(); + assert!( + access.is_compatible(&my_access), + "`EntityRefExcept<{}>` conflicts with a previous access in this query.", + std::any::type_name::(), + ); + access.extend(&my_access); + } + + fn init_state(world: &mut World) -> Self::State { + Self::get_state(world.components()).unwrap() + } + + fn get_state(components: &Components) -> Option { + let mut ids = SmallVec::new(); + B::get_component_ids(components, &mut |maybe_id| { + if let Some(id) = maybe_id { + ids.push(id); + } + }); + Some(ids) + } + + fn matches_component_set(_: &Self::State, _: &impl Fn(ComponentId) -> bool) -> bool { + true + } +} + +/// SAFETY: `Self` is the same as `Self::ReadOnly`. +unsafe impl<'a, B> QueryData for EntityRefExcept<'a, B> +where + B: Bundle, +{ + type ReadOnly = Self; +} + +/// SAFETY: `EntityRefExcept` enforces read-only access to its contained +/// components. +unsafe impl<'a, B> ReadOnlyQueryData for EntityRefExcept<'a, B> where B: Bundle {} + +/// SAFETY: `EntityMutExcept` guards access to all components in the bundle `B` +/// and populates `Access` values so that queries that conflict with this access +/// are rejected. +unsafe impl<'a, B> WorldQuery for EntityMutExcept<'a, B> +where + B: Bundle, +{ + type Fetch<'w> = UnsafeWorldCell<'w>; + type Item<'w> = EntityMutExcept<'w, B>; + type State = SmallVec<[ComponentId; 4]>; + + fn shrink<'wlong: 'wshort, 'wshort>(item: Self::Item<'wlong>) -> Self::Item<'wshort> { + item + } + + fn shrink_fetch<'wlong: 'wshort, 'wshort>(fetch: Self::Fetch<'wlong>) -> Self::Fetch<'wshort> { + fetch + } + + unsafe fn init_fetch<'w>( + world: UnsafeWorldCell<'w>, + _: &Self::State, + _: Tick, + _: Tick, + ) -> Self::Fetch<'w> { + world + } + + const IS_DENSE: bool = true; + + unsafe fn set_archetype<'w>( + _: &mut Self::Fetch<'w>, + _: &Self::State, + _: &'w Archetype, + _: &'w Table, + ) { + } + + unsafe fn set_table<'w>(_: &mut Self::Fetch<'w>, _: &Self::State, _: &'w Table) {} + + unsafe fn fetch<'w>( + world: &mut Self::Fetch<'w>, + entity: Entity, + _: TableRow, + ) -> Self::Item<'w> { + let cell = world.get_entity(entity).unwrap(); + EntityMutExcept::new(cell) + } + + fn update_component_access( + state: &Self::State, + filtered_access: &mut FilteredAccess, + ) { + let mut my_access = Access::new(); + my_access.write_all_components(); + for id in state { + my_access.remove_component_read(*id); + } + + let access = filtered_access.access_mut(); + assert!( + access.is_compatible(&my_access), + "`EntityMutExcept<{}>` conflicts with a previous access in this query.", + std::any::type_name::() + ); + access.extend(&my_access); + } + + fn init_state(world: &mut World) -> Self::State { + Self::get_state(world.components()).unwrap() + } + + fn get_state(components: &Components) -> Option { + let mut ids = SmallVec::new(); + B::get_component_ids(components, &mut |maybe_id| { + if let Some(id) = maybe_id { + ids.push(id); + } + }); + Some(ids) + } + + fn matches_component_set(_: &Self::State, _: &impl Fn(ComponentId) -> bool) -> bool { + true + } +} + +/// SAFETY: All accesses that `EntityRefExcept` provides are also accesses that +/// `EntityMutExcept` provides. +unsafe impl<'a, B> QueryData for EntityMutExcept<'a, B> +where + B: Bundle, +{ + type ReadOnly = EntityRefExcept<'a, B>; +} + /// SAFETY: /// `update_component_access` and `update_archetype_component_access` do nothing. /// This is sound because `fetch` does not access components. diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 9b4e108624..61cd622d5a 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -508,22 +508,46 @@ impl QueryState { archetype: &Archetype, access: &mut Access, ) { - self.component_access - .access - .component_reads() - .for_each(|id| { + // As a fast path, we can iterate directly over the components involved + // if the `access` isn't inverted. + #[allow(deprecated)] + let (component_reads_and_writes, component_reads_and_writes_inverted) = + self.component_access.access.component_reads_and_writes(); + let (component_writes, component_writes_inverted) = + self.component_access.access.component_writes(); + + if !component_reads_and_writes_inverted && !component_writes_inverted { + component_reads_and_writes.for_each(|id| { if let Some(id) = archetype.get_archetype_component_id(id) { access.add_component_read(id); } }); - self.component_access - .access - .component_writes() - .for_each(|id| { + component_writes.for_each(|id| { if let Some(id) = archetype.get_archetype_component_id(id) { access.add_component_write(id); } }); + return; + } + + for (component_id, archetype_component_id) in + archetype.components_with_archetype_component_id() + { + if self + .component_access + .access + .has_component_read(component_id) + { + access.add_component_read(archetype_component_id); + } + if self + .component_access + .access + .has_component_write(component_id) + { + access.add_component_write(archetype_component_id); + } + } } /// Use this to transform a [`QueryState`] into a more generic [`QueryState`]. diff --git a/crates/bevy_ecs/src/system/mod.rs b/crates/bevy_ecs/src/system/mod.rs index 4eb8301d9b..9de3ab40a4 100644 --- a/crates/bevy_ecs/src/system/mod.rs +++ b/crates/bevy_ecs/src/system/mod.rs @@ -1493,13 +1493,10 @@ mod tests { // set up system and verify its access is empty system.initialize(&mut world); system.update_archetype_component_access(world.as_unsafe_world_cell()); - assert_eq!( - system - .archetype_component_access() - .component_reads() - .collect::>(), - expected_ids - ); + let archetype_component_access = system.archetype_component_access(); + assert!(expected_ids + .iter() + .all(|id| archetype_component_access.has_component_read(*id))); // add some entities with archetypes that should match and save their ids expected_ids.insert( @@ -1523,13 +1520,10 @@ mod tests { // update system and verify its accesses are correct system.update_archetype_component_access(world.as_unsafe_world_cell()); - assert_eq!( - system - .archetype_component_access() - .component_reads() - .collect::>(), - expected_ids - ); + let archetype_component_access = system.archetype_component_access(); + assert!(expected_ids + .iter() + .all(|id| archetype_component_access.has_component_read(*id))); // one more round expected_ids.insert( @@ -1541,13 +1535,10 @@ mod tests { ); world.spawn((A, B, D)); system.update_archetype_component_access(world.as_unsafe_world_cell()); - assert_eq!( - system - .archetype_component_access() - .component_reads() - .collect::>(), - expected_ids - ); + let archetype_component_access = system.archetype_component_access(); + assert!(expected_ids + .iter() + .all(|id| archetype_component_access.has_component_read(*id))); } #[test] diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 1671f073ce..e8b2c6f563 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -1927,12 +1927,6 @@ impl<'w> FilteredEntityRef<'w> { self.entity.archetype() } - /// Returns an iterator over the component ids that are accessed by self. - #[inline] - pub fn accessed_components(&self) -> impl Iterator + '_ { - self.access.component_reads_and_writes() - } - /// Returns a reference to the underlying [`Access`]. #[inline] pub fn access(&self) -> &Access { @@ -2184,12 +2178,6 @@ impl<'w> FilteredEntityMut<'w> { self.entity.archetype() } - /// Returns an iterator over the component ids that are accessed by self. - #[inline] - pub fn accessed_components(&self) -> impl Iterator + '_ { - self.access.component_reads_and_writes() - } - /// Returns a reference to the underlying [`Access`]. #[inline] pub fn access(&self) -> &Access { @@ -2384,6 +2372,180 @@ pub enum TryFromFilteredError { MissingWriteAllAccess, } +/// Provides read-only access to a single entity and all its components, save +/// for an explicitly-enumerated set. +#[derive(Clone)] +pub struct EntityRefExcept<'w, B> +where + B: Bundle, +{ + entity: UnsafeEntityCell<'w>, + phantom: PhantomData, +} + +impl<'w, B> EntityRefExcept<'w, B> +where + B: Bundle, +{ + pub(crate) unsafe fn new(entity: UnsafeEntityCell<'w>) -> Self { + Self { + entity, + phantom: PhantomData, + } + } + + /// Gets access to the component of type `C` for the current entity. Returns + /// `None` if the component doesn't have a component of that type or if the + /// type is one of the excluded components. + #[inline] + pub fn get(&self) -> Option<&'w C> + where + C: Component, + { + let components = self.entity.world().components(); + let id = components.component_id::()?; + if bundle_contains_component::(components, id) { + None + } else { + // SAFETY: We have read access for all components that weren't + // covered by the `contains` check above. + unsafe { self.entity.get() } + } + } + + /// Gets access to the component of type `C` for the current entity, + /// including change detection information. Returns `None` if the component + /// doesn't have a component of that type or if the type is one of the + /// excluded components. + #[inline] + pub fn get_ref(&self) -> Option> + where + C: Component, + { + let components = self.entity.world().components(); + let id = components.component_id::()?; + if bundle_contains_component::(components, id) { + None + } else { + // SAFETY: We have read access for all components that weren't + // covered by the `contains` check above. + unsafe { self.entity.get_ref() } + } + } +} + +impl<'a, B> From<&'a EntityMutExcept<'_, B>> for EntityRefExcept<'a, B> +where + B: Bundle, +{ + fn from(entity_mut: &'a EntityMutExcept<'_, B>) -> Self { + // SAFETY: All accesses that `EntityRefExcept` provides are also + // accesses that `EntityMutExcept` provides. + unsafe { EntityRefExcept::new(entity_mut.entity) } + } +} + +/// Provides mutable access to all components of an entity, with the exception +/// of an explicit set. +/// +/// This is a rather niche type that should only be used if you need access to +/// *all* components of an entity, while still allowing you to consult other +/// queries that might match entities that this query also matches. If you don't +/// need access to all components, prefer a standard query with a +/// [`crate::query::Without`] filter. +#[derive(Clone)] +pub struct EntityMutExcept<'w, B> +where + B: Bundle, +{ + entity: UnsafeEntityCell<'w>, + phantom: PhantomData, +} + +impl<'w, B> EntityMutExcept<'w, B> +where + B: Bundle, +{ + pub(crate) unsafe fn new(entity: UnsafeEntityCell<'w>) -> Self { + Self { + entity, + phantom: PhantomData, + } + } + + /// Returns a new instance with a shorter lifetime. + /// + /// This is useful if you have `&mut EntityMutExcept`, but you need + /// `EntityMutExcept`. + pub fn reborrow(&mut self) -> EntityMutExcept<'_, B> { + // SAFETY: We have exclusive access to the entire entity and the + // applicable components. + unsafe { Self::new(self.entity) } + } + + /// Gets read-only access to all of the entity's components, except for the + /// ones in `CL`. + #[inline] + pub fn as_readonly(&self) -> EntityRefExcept<'_, B> { + EntityRefExcept::from(self) + } + + /// Gets access to the component of type `C` for the current entity. Returns + /// `None` if the component doesn't have a component of that type or if the + /// type is one of the excluded components. + #[inline] + pub fn get(&self) -> Option<&'_ C> + where + C: Component, + { + self.as_readonly().get() + } + + /// Gets access to the component of type `C` for the current entity, + /// including change detection information. Returns `None` if the component + /// doesn't have a component of that type or if the type is one of the + /// excluded components. + #[inline] + pub fn get_ref(&self) -> Option> + where + C: Component, + { + self.as_readonly().get_ref() + } + + /// Gets mutable access to the component of type `C` for the current entity. + /// Returns `None` if the component doesn't have a component of that type or + /// if the type is one of the excluded components. + #[inline] + pub fn get_mut(&mut self) -> Option> + where + C: Component, + { + let components = self.entity.world().components(); + let id = components.component_id::()?; + if bundle_contains_component::(components, id) { + None + } else { + // SAFETY: We have write access for all components that weren't + // covered by the `contains` check above. + unsafe { self.entity.get_mut() } + } + } +} + +fn bundle_contains_component(components: &Components, query_id: ComponentId) -> bool +where + B: Bundle, +{ + let mut found = false; + B::get_component_ids(components, &mut |maybe_id| { + if let Some(id) = maybe_id { + found = found || id == query_id; + } + }); + found +} + /// Inserts a dynamic [`Bundle`] into the entity. /// /// # Safety @@ -2598,9 +2760,12 @@ mod tests { use bevy_ptr::OwningPtr; use std::panic::AssertUnwindSafe; + use crate::system::RunSystemOnce as _; use crate::world::{FilteredEntityMut, FilteredEntityRef}; use crate::{self as bevy_ecs, component::ComponentId, prelude::*, system::assert_is_system}; + use super::{EntityMutExcept, EntityRefExcept}; + #[test] fn sorted_remove() { let mut a = vec![1, 2, 3, 4, 5, 6, 7]; @@ -2993,6 +3158,164 @@ mod tests { world.spawn_empty().remove_by_id(test_component_id); } + /// Tests that components can be accessed through an `EntityRefExcept`. + #[test] + fn entity_ref_except() { + let mut world = World::new(); + world.init_component::(); + world.init_component::(); + + world.spawn(TestComponent(0)).insert(TestComponent2(0)); + + let mut query = world.query::>(); + + let mut found = false; + for entity_ref in query.iter_mut(&mut world) { + found = true; + assert!(entity_ref.get::().is_none()); + assert!(entity_ref.get_ref::().is_none()); + assert!(matches!( + entity_ref.get::(), + Some(TestComponent2(0)) + )); + } + + assert!(found); + } + + // Test that a single query can't both contain a mutable reference to a + // component C and an `EntityRefExcept` that doesn't include C among its + // exclusions. + #[test] + #[should_panic] + fn entity_ref_except_conflicts_with_self() { + let mut world = World::new(); + world.spawn(TestComponent(0)).insert(TestComponent2(0)); + + // This should panic, because we have a mutable borrow on + // `TestComponent` but have a simultaneous indirect immutable borrow on + // that component via `EntityRefExcept`. + world.run_system_once(system); + + fn system(_: Query<(&mut TestComponent, EntityRefExcept)>) {} + } + + // Test that an `EntityRefExcept` that doesn't include a component C among + // its exclusions can't coexist with a mutable query for that component. + #[test] + #[should_panic] + fn entity_ref_except_conflicts_with_other() { + let mut world = World::new(); + world.spawn(TestComponent(0)).insert(TestComponent2(0)); + + // This should panic, because we have a mutable borrow on + // `TestComponent` but have a simultaneous indirect immutable borrow on + // that component via `EntityRefExcept`. + world.run_system_once(system); + + fn system(_: Query<&mut TestComponent>, _: Query>) {} + } + + // Test that an `EntityRefExcept` with an exception for some component C can + // coexist with a query for that component C. + #[test] + fn entity_ref_except_doesnt_conflict() { + let mut world = World::new(); + world.spawn(TestComponent(0)).insert(TestComponent2(0)); + + world.run_system_once(system); + + fn system(_: Query<&mut TestComponent>, query: Query>) { + for entity_ref in query.iter() { + assert!(matches!( + entity_ref.get::(), + Some(TestComponent2(0)) + )); + } + } + } + + /// Tests that components can be mutably accessed through an + /// `EntityMutExcept`. + #[test] + fn entity_mut_except() { + let mut world = World::new(); + world.spawn(TestComponent(0)).insert(TestComponent2(0)); + + let mut query = world.query::>(); + + let mut found = false; + for mut entity_mut in query.iter_mut(&mut world) { + found = true; + assert!(entity_mut.get::().is_none()); + assert!(entity_mut.get_ref::().is_none()); + assert!(entity_mut.get_mut::().is_none()); + assert!(matches!( + entity_mut.get::(), + Some(TestComponent2(0)) + )); + } + + assert!(found); + } + + // Test that a single query can't both contain a mutable reference to a + // component C and an `EntityMutExcept` that doesn't include C among its + // exclusions. + #[test] + #[should_panic] + fn entity_mut_except_conflicts_with_self() { + let mut world = World::new(); + world.spawn(TestComponent(0)).insert(TestComponent2(0)); + + // This should panic, because we have a mutable borrow on + // `TestComponent` but have a simultaneous indirect immutable borrow on + // that component via `EntityRefExcept`. + world.run_system_once(system); + + fn system(_: Query<(&mut TestComponent, EntityMutExcept)>) {} + } + + // Test that an `EntityMutExcept` that doesn't include a component C among + // its exclusions can't coexist with a query for that component. + #[test] + #[should_panic] + fn entity_mut_except_conflicts_with_other() { + let mut world = World::new(); + world.spawn(TestComponent(0)).insert(TestComponent2(0)); + + // This should panic, because we have a mutable borrow on + // `TestComponent` but have a simultaneous indirect immutable borrow on + // that component via `EntityRefExcept`. + world.run_system_once(system); + + fn system(_: Query<&mut TestComponent>, mut query: Query>) { + for mut entity_mut in query.iter_mut() { + assert!(entity_mut + .get_mut::() + .is_some_and(|component| component.0 == 0)); + } + } + } + + // Test that an `EntityMutExcept` with an exception for some component C can + // coexist with a query for that component C. + #[test] + fn entity_mut_except_doesnt_conflict() { + let mut world = World::new(); + world.spawn(TestComponent(0)).insert(TestComponent2(0)); + + world.run_system_once(system); + + fn system(_: Query<&mut TestComponent>, mut query: Query>) { + for mut entity_mut in query.iter_mut() { + assert!(entity_mut + .get_mut::() + .is_some_and(|component| component.0 == 0)); + } + } + } + #[derive(Component)] struct A; diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 01e9132a1c..cc89646b46 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -19,8 +19,8 @@ pub use crate::{ pub use component_constants::*; pub use deferred_world::DeferredWorld; pub use entity_ref::{ - EntityMut, EntityRef, EntityWorldMut, Entry, FilteredEntityMut, FilteredEntityRef, - OccupiedEntry, VacantEntry, + EntityMut, EntityMutExcept, EntityRef, EntityRefExcept, EntityWorldMut, Entry, + FilteredEntityMut, FilteredEntityRef, OccupiedEntry, VacantEntry, }; pub use identifier::WorldId; pub use spawn_batch::*; diff --git a/examples/ecs/dynamic.rs b/examples/ecs/dynamic.rs index e7d20a8997..7bcd4e01d3 100644 --- a/examples/ecs/dynamic.rs +++ b/examples/ecs/dynamic.rs @@ -150,8 +150,11 @@ fn main() { let mut query = builder.build(); query.iter_mut(&mut world).for_each(|filtered_entity| { + #[allow(deprecated)] let terms = filtered_entity - .accessed_components() + .access() + .component_reads_and_writes() + .0 .map(|id| { let ptr = filtered_entity.get_by_id(id).unwrap(); let info = component_info.get(&id).unwrap();