From af0cc64d98e8234c579762443c475331c41b365a Mon Sep 17 00:00:00 2001 From: Chris Russell <8494645+chescock@users.noreply.github.com> Date: Fri, 11 Jul 2025 16:32:26 -0400 Subject: [PATCH 1/5] Borrow Access in `FilteredEntity(Ref|Mut)` to avoid cloning. --- crates/bevy_ecs/src/query/access.rs | 26 +++ crates/bevy_ecs/src/query/fetch.rs | 86 +++++---- crates/bevy_ecs/src/query/iter.rs | 4 +- crates/bevy_ecs/src/reflect/component.rs | 17 +- crates/bevy_ecs/src/world/entity_ref.rs | 220 +++++++++-------------- 5 files changed, 161 insertions(+), 192 deletions(-) diff --git a/crates/bevy_ecs/src/query/access.rs b/crates/bevy_ecs/src/query/access.rs index 0c5b29f715c7e..5a175e369549c 100644 --- a/crates/bevy_ecs/src/query/access.rs +++ b/crates/bevy_ecs/src/query/access.rs @@ -165,6 +165,32 @@ impl Access { } } + /// Creates an [`Access`] with read access to all components. + /// This is equivalent to calling `read_all()` on `Access::new()`, + /// but is available in a `const` context. + pub(crate) const fn new_read_all() -> Self { + let mut access = Self::new(); + access.reads_all_resources = true; + // Note that we cannot use `read_all_components()` + // because `FixedBitSet::clear()` is not `const`. + access.component_read_and_writes_inverted = true; + access + } + + /// Creates an [`Access`] with read access to all components. + /// This is equivalent to calling `read_all()` on `Access::new()`, + /// but is available in a `const` context. + pub(crate) const fn new_write_all() -> Self { + let mut access = Self::new(); + access.reads_all_resources = true; + access.writes_all_resources = true; + // Note that we cannot use `write_all_components()` + // because `FixedBitSet::clear()` is not `const`. + access.component_read_and_writes_inverted = true; + access.component_writes_inverted = true; + 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); diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 2564223972130..c75d52d3566f7 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -921,8 +921,8 @@ impl ReleaseStateQueryData for EntityMut<'_> { } /// SAFETY: The accesses of `Self::ReadOnly` are a subset of the accesses of `Self` -unsafe impl<'a> WorldQuery for FilteredEntityRef<'a> { - type Fetch<'w> = (EntityFetch<'w>, Access); +unsafe impl WorldQuery for FilteredEntityRef<'_, '_> { + type Fetch<'w> = EntityFetch<'w>; type State = Access; fn shrink_fetch<'wlong: 'wshort, 'wshort>(fetch: Self::Fetch<'wlong>) -> Self::Fetch<'wshort> { @@ -937,31 +937,28 @@ unsafe impl<'a> WorldQuery for FilteredEntityRef<'a> { last_run: Tick, this_run: Tick, ) -> Self::Fetch<'w> { - let mut access = Access::default(); - access.read_all_components(); - ( - EntityFetch { - world, - last_run, - this_run, - }, - access, - ) + EntityFetch { + world, + last_run, + this_run, + } } #[inline] unsafe fn set_archetype<'w, 's>( - fetch: &mut Self::Fetch<'w>, - state: &'s Self::State, + _fetch: &mut Self::Fetch<'w>, + _state: &'s Self::State, _: &'w Archetype, _table: &Table, ) { - fetch.1.clone_from(state); } #[inline] - unsafe fn set_table<'w, 's>(fetch: &mut Self::Fetch<'w>, state: &'s Self::State, _: &'w Table) { - fetch.1.clone_from(state); + unsafe fn set_table<'w, 's>( + _fetch: &mut Self::Fetch<'w>, + _state: &'s Self::State, + _: &'w Table, + ) { } fn update_component_access( @@ -992,10 +989,10 @@ unsafe impl<'a> WorldQuery for FilteredEntityRef<'a> { } /// SAFETY: `Self` is the same as `Self::ReadOnly` -unsafe impl<'a> QueryData for FilteredEntityRef<'a> { +unsafe impl QueryData for FilteredEntityRef<'_, '_> { const IS_READ_ONLY: bool = true; type ReadOnly = Self; - type Item<'w, 's> = FilteredEntityRef<'w>; + type Item<'w, 's> = FilteredEntityRef<'w, 's>; fn shrink<'wlong: 'wshort, 'wshort, 's>( item: Self::Item<'wlong, 's>, @@ -1024,8 +1021,8 @@ unsafe impl<'a> QueryData for FilteredEntityRef<'a> { #[inline(always)] unsafe fn fetch<'w, 's>( - _state: &'s Self::State, - (fetch, access): &mut Self::Fetch<'w>, + access: &'s Self::State, + fetch: &mut Self::Fetch<'w>, entity: Entity, _table_row: TableRow, ) -> Self::Item<'w, 's> { @@ -1037,16 +1034,16 @@ unsafe impl<'a> QueryData for FilteredEntityRef<'a> { .debug_checked_unwrap() }; // SAFETY: mutable access to every component has been registered. - unsafe { FilteredEntityRef::new(cell, access.clone()) } + unsafe { FilteredEntityRef::new(cell, &access) } } } /// SAFETY: Access is read-only. -unsafe impl ReadOnlyQueryData for FilteredEntityRef<'_> {} +unsafe impl ReadOnlyQueryData for FilteredEntityRef<'_, '_> {} /// SAFETY: The accesses of `Self::ReadOnly` are a subset of the accesses of `Self` -unsafe impl<'a> WorldQuery for FilteredEntityMut<'a> { - type Fetch<'w> = (EntityFetch<'w>, Access); +unsafe impl WorldQuery for FilteredEntityMut<'_, '_> { + type Fetch<'w> = EntityFetch<'w>; type State = Access; fn shrink_fetch<'wlong: 'wshort, 'wshort>(fetch: Self::Fetch<'wlong>) -> Self::Fetch<'wshort> { @@ -1061,31 +1058,28 @@ unsafe impl<'a> WorldQuery for FilteredEntityMut<'a> { last_run: Tick, this_run: Tick, ) -> Self::Fetch<'w> { - let mut access = Access::default(); - access.write_all_components(); - ( - EntityFetch { - world, - last_run, - this_run, - }, - access, - ) + EntityFetch { + world, + last_run, + this_run, + } } #[inline] unsafe fn set_archetype<'w, 's>( - fetch: &mut Self::Fetch<'w>, - state: &'s Self::State, + _fetch: &mut Self::Fetch<'w>, + _state: &'s Self::State, _: &'w Archetype, _table: &Table, ) { - fetch.1.clone_from(state); } #[inline] - unsafe fn set_table<'w, 's>(fetch: &mut Self::Fetch<'w>, state: &'s Self::State, _: &'w Table) { - fetch.1.clone_from(state); + unsafe fn set_table<'w, 's>( + _fetch: &mut Self::Fetch<'w>, + _state: &'s Self::State, + _: &'w Table, + ) { } fn update_component_access( @@ -1116,10 +1110,10 @@ unsafe impl<'a> WorldQuery for FilteredEntityMut<'a> { } /// SAFETY: access of `FilteredEntityRef` is a subset of `FilteredEntityMut` -unsafe impl<'a> QueryData for FilteredEntityMut<'a> { +unsafe impl<'a, 'b> QueryData for FilteredEntityMut<'a, 'b> { const IS_READ_ONLY: bool = false; - type ReadOnly = FilteredEntityRef<'a>; - type Item<'w, 's> = FilteredEntityMut<'w>; + type ReadOnly = FilteredEntityRef<'a, 'b>; + type Item<'w, 's> = FilteredEntityMut<'w, 's>; fn shrink<'wlong: 'wshort, 'wshort, 's>( item: Self::Item<'wlong, 's>, @@ -1146,8 +1140,8 @@ unsafe impl<'a> QueryData for FilteredEntityMut<'a> { #[inline(always)] unsafe fn fetch<'w, 's>( - _state: &'s Self::State, - (fetch, access): &mut Self::Fetch<'w>, + access: &'s Self::State, + fetch: &mut Self::Fetch<'w>, entity: Entity, _table_row: TableRow, ) -> Self::Item<'w, 's> { @@ -1159,7 +1153,7 @@ unsafe impl<'a> QueryData for FilteredEntityMut<'a> { .debug_checked_unwrap() }; // SAFETY: mutable access to every component has been registered. - unsafe { FilteredEntityMut::new(cell, access.clone()) } + unsafe { FilteredEntityMut::new(cell, &access) } } } diff --git a/crates/bevy_ecs/src/query/iter.rs b/crates/bevy_ecs/src/query/iter.rs index eb49204434b6f..606990f6b3f8b 100644 --- a/crates/bevy_ecs/src/query/iter.rs +++ b/crates/bevy_ecs/src/query/iter.rs @@ -958,13 +958,13 @@ unsafe impl<'w, 's, F: QueryFilter> EntitySetIterator for QueryIter<'w, 's, Enti // SAFETY: [`QueryIter`] is guaranteed to return every matching entity once and only once. unsafe impl<'w, 's, F: QueryFilter> EntitySetIterator - for QueryIter<'w, 's, FilteredEntityRef<'_>, F> + for QueryIter<'w, 's, FilteredEntityRef<'_, '_>, F> { } // SAFETY: [`QueryIter`] is guaranteed to return every matching entity once and only once. unsafe impl<'w, 's, F: QueryFilter> EntitySetIterator - for QueryIter<'w, 's, FilteredEntityMut<'_>, F> + for QueryIter<'w, 's, FilteredEntityMut<'_, '_>, F> { } diff --git a/crates/bevy_ecs/src/reflect/component.rs b/crates/bevy_ecs/src/reflect/component.rs index 8a5c17179e1ff..c38deeb31683c 100644 --- a/crates/bevy_ecs/src/reflect/component.rs +++ b/crates/bevy_ecs/src/reflect/component.rs @@ -118,9 +118,9 @@ pub struct ReflectComponentFns { /// Function pointer implementing [`ReflectComponent::contains()`]. pub contains: fn(FilteredEntityRef) -> bool, /// Function pointer implementing [`ReflectComponent::reflect()`]. - pub reflect: fn(FilteredEntityRef) -> Option<&dyn Reflect>, + pub reflect: for<'w> fn(FilteredEntityRef<'w, '_>) -> Option<&'w dyn Reflect>, /// Function pointer implementing [`ReflectComponent::reflect_mut()`]. - pub reflect_mut: fn(FilteredEntityMut) -> Option>, + pub reflect_mut: for<'w> fn(FilteredEntityMut<'w, '_>) -> Option>, /// Function pointer implementing [`ReflectComponent::map_entities()`]. pub map_entities: fn(&mut dyn Reflect, &mut dyn EntityMapper), /// Function pointer implementing [`ReflectComponent::reflect_unchecked_mut()`]. @@ -189,12 +189,15 @@ impl ReflectComponent { } /// Returns whether entity contains this [`Component`] - pub fn contains<'a>(&self, entity: impl Into>) -> bool { + pub fn contains<'w, 's>(&self, entity: impl Into>) -> bool { (self.0.contains)(entity.into()) } /// Gets the value of this [`Component`] type from the entity as a reflected reference. - pub fn reflect<'a>(&self, entity: impl Into>) -> Option<&'a dyn Reflect> { + pub fn reflect<'w, 's>( + &self, + entity: impl Into>, + ) -> Option<&'w dyn Reflect> { (self.0.reflect)(entity.into()) } @@ -203,10 +206,10 @@ impl ReflectComponent { /// # Panics /// /// Panics if [`Component`] is immutable. - pub fn reflect_mut<'a>( + pub fn reflect_mut<'w, 's>( &self, - entity: impl Into>, - ) -> Option> { + entity: impl Into>, + ) -> Option> { (self.0.reflect_mut)(entity.into()) } diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 90438b8d6e1e6..3f57d77c10675 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -338,10 +338,10 @@ impl<'a> From<&'a EntityMut<'_>> for EntityRef<'a> { } } -impl<'a> TryFrom> for EntityRef<'a> { +impl<'a> TryFrom> for EntityRef<'a> { type Error = TryFromFilteredError; - fn try_from(entity: FilteredEntityRef<'a>) -> Result { + fn try_from(entity: FilteredEntityRef<'a, '_>) -> Result { if !entity.access.has_read_all() { Err(TryFromFilteredError::MissingReadAllAccess) } else { @@ -351,10 +351,10 @@ impl<'a> TryFrom> for EntityRef<'a> { } } -impl<'a> TryFrom<&'a FilteredEntityRef<'_>> for EntityRef<'a> { +impl<'a> TryFrom<&'a FilteredEntityRef<'_, '_>> for EntityRef<'a> { type Error = TryFromFilteredError; - fn try_from(entity: &'a FilteredEntityRef<'_>) -> Result { + fn try_from(entity: &'a FilteredEntityRef<'_, '_>) -> Result { if !entity.access.has_read_all() { Err(TryFromFilteredError::MissingReadAllAccess) } else { @@ -364,10 +364,10 @@ impl<'a> TryFrom<&'a FilteredEntityRef<'_>> for EntityRef<'a> { } } -impl<'a> TryFrom> for EntityRef<'a> { +impl<'a> TryFrom> for EntityRef<'a> { type Error = TryFromFilteredError; - fn try_from(entity: FilteredEntityMut<'a>) -> Result { + fn try_from(entity: FilteredEntityMut<'a, '_>) -> Result { if !entity.access.has_read_all() { Err(TryFromFilteredError::MissingReadAllAccess) } else { @@ -377,10 +377,10 @@ impl<'a> TryFrom> for EntityRef<'a> { } } -impl<'a> TryFrom<&'a FilteredEntityMut<'_>> for EntityRef<'a> { +impl<'a> TryFrom<&'a FilteredEntityMut<'_, '_>> for EntityRef<'a> { type Error = TryFromFilteredError; - fn try_from(entity: &'a FilteredEntityMut<'_>) -> Result { + fn try_from(entity: &'a FilteredEntityMut<'_, '_>) -> Result { if !entity.access.has_read_all() { Err(TryFromFilteredError::MissingReadAllAccess) } else { @@ -1020,10 +1020,10 @@ impl<'a> From<&'a mut EntityWorldMut<'_>> for EntityMut<'a> { } } -impl<'a> TryFrom> for EntityMut<'a> { +impl<'a> TryFrom> for EntityMut<'a> { type Error = TryFromFilteredError; - fn try_from(entity: FilteredEntityMut<'a>) -> Result { + fn try_from(entity: FilteredEntityMut<'a, '_>) -> Result { if !entity.access.has_read_all() { Err(TryFromFilteredError::MissingReadAllAccess) } else if !entity.access.has_write_all() { @@ -1035,10 +1035,10 @@ impl<'a> TryFrom> for EntityMut<'a> { } } -impl<'a> TryFrom<&'a mut FilteredEntityMut<'_>> for EntityMut<'a> { +impl<'a> TryFrom<&'a mut FilteredEntityMut<'_, '_>> for EntityMut<'a> { type Error = TryFromFilteredError; - fn try_from(entity: &'a mut FilteredEntityMut<'_>) -> Result { + fn try_from(entity: &'a mut FilteredEntityMut<'_, '_>) -> Result { if !entity.access.has_read_all() { Err(TryFromFilteredError::MissingReadAllAccess) } else if !entity.access.has_write_all() { @@ -3310,20 +3310,23 @@ impl<'w, 'a, T: Component> VacantComponentEntry<'w, 'a, T> { /// let filtered_entity: FilteredEntityRef = query.single(&mut world).unwrap(); /// let component: &A = filtered_entity.get().unwrap(); /// ``` -#[derive(Clone)] -pub struct FilteredEntityRef<'w> { +#[derive(Clone, Copy)] +pub struct FilteredEntityRef<'w, 's> { entity: UnsafeEntityCell<'w>, - access: Access, + access: &'s Access, } -impl<'w> FilteredEntityRef<'w> { +impl<'w, 's> FilteredEntityRef<'w, 's> { /// # Safety /// - No `&mut World` can exist from the underlying `UnsafeWorldCell` /// - If `access` takes read access to a component no mutable reference to that /// component can exist at the same time as the returned [`FilteredEntityMut`] /// - If `access` takes any access for a component `entity` must have that component. #[inline] - pub(crate) unsafe fn new(entity: UnsafeEntityCell<'w>, access: Access) -> Self { + pub(crate) unsafe fn new( + entity: UnsafeEntityCell<'w>, + access: &'s Access, + ) -> Self { Self { entity, access } } @@ -3349,7 +3352,7 @@ impl<'w> FilteredEntityRef<'w> { /// Returns a reference to the underlying [`Access`]. #[inline] pub fn access(&self) -> &Access { - &self.access + self.access } /// Returns `true` if the current entity has a component of type `T`. @@ -3482,123 +3485,91 @@ impl<'w> FilteredEntityRef<'w> { } } -impl<'w> From> for FilteredEntityRef<'w> { +impl<'w, 's> From> for FilteredEntityRef<'w, 's> { #[inline] - fn from(entity: FilteredEntityMut<'w>) -> Self { + fn from(entity: FilteredEntityMut<'w, 's>) -> Self { // SAFETY: // - `FilteredEntityMut` guarantees exclusive access to all components in the new `FilteredEntityRef`. unsafe { FilteredEntityRef::new(entity.entity, entity.access) } } } -impl<'a> From<&'a FilteredEntityMut<'_>> for FilteredEntityRef<'a> { +impl<'w, 's> From<&'w FilteredEntityMut<'_, 's>> for FilteredEntityRef<'w, 's> { #[inline] - fn from(entity: &'a FilteredEntityMut<'_>) -> Self { + fn from(entity: &'w FilteredEntityMut<'_, 's>) -> Self { // SAFETY: // - `FilteredEntityMut` guarantees exclusive access to all components in the new `FilteredEntityRef`. - unsafe { FilteredEntityRef::new(entity.entity, entity.access.clone()) } + unsafe { FilteredEntityRef::new(entity.entity, entity.access) } } } -impl<'a> From> for FilteredEntityRef<'a> { +impl<'a> From> for FilteredEntityRef<'a, 'static> { fn from(entity: EntityRef<'a>) -> Self { // SAFETY: // - `EntityRef` guarantees exclusive access to all components in the new `FilteredEntityRef`. - unsafe { - let mut access = Access::default(); - access.read_all(); - FilteredEntityRef::new(entity.cell, access) - } + unsafe { FilteredEntityRef::new(entity.cell, const { &Access::new_read_all() }) } } } -impl<'a> From<&'a EntityRef<'_>> for FilteredEntityRef<'a> { +impl<'a> From<&'a EntityRef<'_>> for FilteredEntityRef<'a, 'static> { fn from(entity: &'a EntityRef<'_>) -> Self { // SAFETY: // - `EntityRef` guarantees exclusive access to all components in the new `FilteredEntityRef`. - unsafe { - let mut access = Access::default(); - access.read_all(); - FilteredEntityRef::new(entity.cell, access) - } + unsafe { FilteredEntityRef::new(entity.cell, const { &Access::new_read_all() }) } } } -impl<'a> From> for FilteredEntityRef<'a> { +impl<'a> From> for FilteredEntityRef<'a, 'static> { fn from(entity: EntityMut<'a>) -> Self { // SAFETY: // - `EntityMut` guarantees exclusive access to all components in the new `FilteredEntityRef`. - unsafe { - let mut access = Access::default(); - access.read_all(); - FilteredEntityRef::new(entity.cell, access) - } + unsafe { FilteredEntityRef::new(entity.cell, const { &Access::new_read_all() }) } } } -impl<'a> From<&'a EntityMut<'_>> for FilteredEntityRef<'a> { +impl<'a> From<&'a EntityMut<'_>> for FilteredEntityRef<'a, 'static> { fn from(entity: &'a EntityMut<'_>) -> Self { // SAFETY: // - `EntityMut` guarantees exclusive access to all components in the new `FilteredEntityRef`. - unsafe { - let mut access = Access::default(); - access.read_all(); - FilteredEntityRef::new(entity.cell, access) - } + unsafe { FilteredEntityRef::new(entity.cell, const { &Access::new_read_all() }) } } } -impl<'a> From> for FilteredEntityRef<'a> { +impl<'a> From> for FilteredEntityRef<'a, 'static> { fn from(entity: EntityWorldMut<'a>) -> Self { // SAFETY: // - `EntityWorldMut` guarantees exclusive access to the entire world. unsafe { - let mut access = Access::default(); - access.read_all(); - FilteredEntityRef::new(entity.into_unsafe_entity_cell(), access) + FilteredEntityRef::new( + entity.into_unsafe_entity_cell(), + const { &Access::new_read_all() }, + ) } } } -impl<'a> From<&'a EntityWorldMut<'_>> for FilteredEntityRef<'a> { +impl<'a> From<&'a EntityWorldMut<'_>> for FilteredEntityRef<'a, 'static> { fn from(entity: &'a EntityWorldMut<'_>) -> Self { // SAFETY: // - `EntityWorldMut` guarantees exclusive access to the entire world. unsafe { - let mut access = Access::default(); - access.read_all(); - FilteredEntityRef::new(entity.as_unsafe_entity_cell_readonly(), access) - } - } -} - -impl<'a, B: Bundle> From<&'a EntityRefExcept<'_, B>> for FilteredEntityRef<'a> { - fn from(value: &'a EntityRefExcept<'_, B>) -> Self { - // SAFETY: - // - The FilteredEntityRef has the same component access as the given EntityRefExcept. - unsafe { - let mut access = Access::default(); - access.read_all(); - let components = value.entity.world().components(); - B::get_component_ids(components, &mut |maybe_id| { - if let Some(id) = maybe_id { - access.remove_component_read(id); - } - }); - FilteredEntityRef::new(value.entity, access) + FilteredEntityRef::new( + entity.as_unsafe_entity_cell_readonly(), + const { &Access::new_read_all() }, + ) } } } -impl PartialEq for FilteredEntityRef<'_> { +impl PartialEq for FilteredEntityRef<'_, '_> { fn eq(&self, other: &Self) -> bool { self.entity() == other.entity() } } -impl Eq for FilteredEntityRef<'_> {} +impl Eq for FilteredEntityRef<'_, '_> {} -impl PartialOrd for FilteredEntityRef<'_> { +impl PartialOrd for FilteredEntityRef<'_, '_> { /// [`FilteredEntityRef`]'s comparison trait implementations match the underlying [`Entity`], /// and cannot discern between different worlds. fn partial_cmp(&self, other: &Self) -> Option { @@ -3606,26 +3577,26 @@ impl PartialOrd for FilteredEntityRef<'_> { } } -impl Ord for FilteredEntityRef<'_> { +impl Ord for FilteredEntityRef<'_, '_> { fn cmp(&self, other: &Self) -> Ordering { self.entity().cmp(&other.entity()) } } -impl Hash for FilteredEntityRef<'_> { +impl Hash for FilteredEntityRef<'_, '_> { fn hash(&self, state: &mut H) { self.entity().hash(state); } } -impl ContainsEntity for FilteredEntityRef<'_> { +impl ContainsEntity for FilteredEntityRef<'_, '_> { fn entity(&self) -> Entity { self.id() } } // SAFETY: This type represents one Entity. We implement the comparison traits based on that Entity. -unsafe impl EntityEquivalent for FilteredEntityRef<'_> {} +unsafe impl EntityEquivalent for FilteredEntityRef<'_, '_> {} /// Provides mutable access to a single entity and some of its components defined by the contained [`Access`]. /// @@ -3650,12 +3621,12 @@ unsafe impl EntityEquivalent for FilteredEntityRef<'_> {} /// let mut filtered_entity: FilteredEntityMut = query.single_mut(&mut world).unwrap(); /// let component: Mut = filtered_entity.get_mut().unwrap(); /// ``` -pub struct FilteredEntityMut<'w> { +pub struct FilteredEntityMut<'w, 's> { entity: UnsafeEntityCell<'w>, - access: Access, + access: &'s Access, } -impl<'w> FilteredEntityMut<'w> { +impl<'w, 's> FilteredEntityMut<'w, 's> { /// # Safety /// - No `&mut World` can exist from the underlying `UnsafeWorldCell` /// - If `access` takes read access to a component no mutable reference to that @@ -3664,20 +3635,23 @@ impl<'w> FilteredEntityMut<'w> { /// may exist at the same time as the returned [`FilteredEntityMut`] /// - If `access` takes any access for a component `entity` must have that component. #[inline] - pub(crate) unsafe fn new(entity: UnsafeEntityCell<'w>, access: Access) -> Self { + pub(crate) unsafe fn new( + entity: UnsafeEntityCell<'w>, + access: &'s Access, + ) -> Self { Self { entity, access } } /// Returns a new instance with a shorter lifetime. /// This is useful if you have `&mut FilteredEntityMut`, but you need `FilteredEntityMut`. - pub fn reborrow(&mut self) -> FilteredEntityMut<'_> { + pub fn reborrow(&mut self) -> FilteredEntityMut<'_, 's> { // SAFETY: We have exclusive access to the entire entity and its components. - unsafe { Self::new(self.entity, self.access.clone()) } + unsafe { Self::new(self.entity, self.access) } } /// Gets read-only access to all of the entity's components. #[inline] - pub fn as_readonly(&self) -> FilteredEntityRef<'_> { + pub fn as_readonly(&self) -> FilteredEntityRef<'_, 's> { FilteredEntityRef::from(self) } @@ -3703,7 +3677,7 @@ impl<'w> FilteredEntityMut<'w> { /// Returns a reference to the underlying [`Access`]. #[inline] pub fn access(&self) -> &Access { - &self.access + self.access } /// Returns `true` if the current entity has a component of type `T`. @@ -3868,85 +3842,57 @@ impl<'w> FilteredEntityMut<'w> { } } -impl<'a> From> for FilteredEntityMut<'a> { +impl<'a> From> for FilteredEntityMut<'a, 'static> { fn from(entity: EntityMut<'a>) -> Self { // SAFETY: // - `EntityMut` guarantees exclusive access to all components in the new `FilteredEntityMut`. - unsafe { - let mut access = Access::default(); - access.read_all(); - access.write_all(); - FilteredEntityMut::new(entity.cell, access) - } + unsafe { FilteredEntityMut::new(entity.cell, const { &Access::new_write_all() }) } } } -impl<'a> From<&'a mut EntityMut<'_>> for FilteredEntityMut<'a> { +impl<'a> From<&'a mut EntityMut<'_>> for FilteredEntityMut<'a, 'static> { fn from(entity: &'a mut EntityMut<'_>) -> Self { // SAFETY: // - `EntityMut` guarantees exclusive access to all components in the new `FilteredEntityMut`. - unsafe { - let mut access = Access::default(); - access.read_all(); - access.write_all(); - FilteredEntityMut::new(entity.cell, access) - } + unsafe { FilteredEntityMut::new(entity.cell, const { &Access::new_write_all() }) } } } -impl<'a> From> for FilteredEntityMut<'a> { +impl<'a> From> for FilteredEntityMut<'a, 'static> { fn from(entity: EntityWorldMut<'a>) -> Self { // SAFETY: // - `EntityWorldMut` guarantees exclusive access to the entire world. unsafe { - let mut access = Access::default(); - access.read_all(); - access.write_all(); - FilteredEntityMut::new(entity.into_unsafe_entity_cell(), access) + FilteredEntityMut::new( + entity.into_unsafe_entity_cell(), + const { &Access::new_write_all() }, + ) } } } -impl<'a> From<&'a mut EntityWorldMut<'_>> for FilteredEntityMut<'a> { +impl<'a> From<&'a mut EntityWorldMut<'_>> for FilteredEntityMut<'a, 'static> { fn from(entity: &'a mut EntityWorldMut<'_>) -> Self { // SAFETY: // - `EntityWorldMut` guarantees exclusive access to the entire world. unsafe { - let mut access = Access::default(); - access.read_all(); - access.write_all(); - FilteredEntityMut::new(entity.as_unsafe_entity_cell(), access) - } - } -} - -impl<'a, B: Bundle> From<&'a EntityMutExcept<'_, B>> for FilteredEntityMut<'a> { - fn from(value: &'a EntityMutExcept<'_, B>) -> Self { - // SAFETY: - // - The FilteredEntityMut has the same component access as the given EntityMutExcept. - unsafe { - let mut access = Access::default(); - access.write_all(); - let components = value.entity.world().components(); - B::get_component_ids(components, &mut |maybe_id| { - if let Some(id) = maybe_id { - access.remove_component_read(id); - } - }); - FilteredEntityMut::new(value.entity, access) + FilteredEntityMut::new( + entity.as_unsafe_entity_cell(), + const { &Access::new_write_all() }, + ) } } } -impl PartialEq for FilteredEntityMut<'_> { +impl PartialEq for FilteredEntityMut<'_, '_> { fn eq(&self, other: &Self) -> bool { self.entity() == other.entity() } } -impl Eq for FilteredEntityMut<'_> {} +impl Eq for FilteredEntityMut<'_, '_> {} -impl PartialOrd for FilteredEntityMut<'_> { +impl PartialOrd for FilteredEntityMut<'_, '_> { /// [`FilteredEntityMut`]'s comparison trait implementations match the underlying [`Entity`], /// and cannot discern between different worlds. fn partial_cmp(&self, other: &Self) -> Option { @@ -3954,26 +3900,26 @@ impl PartialOrd for FilteredEntityMut<'_> { } } -impl Ord for FilteredEntityMut<'_> { +impl Ord for FilteredEntityMut<'_, '_> { fn cmp(&self, other: &Self) -> Ordering { self.entity().cmp(&other.entity()) } } -impl Hash for FilteredEntityMut<'_> { +impl Hash for FilteredEntityMut<'_, '_> { fn hash(&self, state: &mut H) { self.entity().hash(state); } } -impl ContainsEntity for FilteredEntityMut<'_> { +impl ContainsEntity for FilteredEntityMut<'_, '_> { fn entity(&self) -> Entity { self.id() } } // SAFETY: This type represents one Entity. We implement the comparison traits based on that Entity. -unsafe impl EntityEquivalent for FilteredEntityMut<'_> {} +unsafe impl EntityEquivalent for FilteredEntityMut<'_, '_> {} /// Error type returned by [`TryFrom`] conversions from filtered entity types /// ([`FilteredEntityRef`]/[`FilteredEntityMut`]) to full-access entity types From 4bb6066a90b29107cfbf9444869475b1e7ff8a88 Mon Sep 17 00:00:00 2001 From: Chris Russell <8494645+chescock@users.noreply.github.com> Date: Sat, 12 Jul 2025 10:15:27 -0400 Subject: [PATCH 2/5] Have `Entity(Ref|Mut)Except` look up the component ids on initialization and store them in an Access that can be borrowed by each row. --- crates/bevy_animation/src/animation_curves.rs | 15 +--- crates/bevy_animation/src/lib.rs | 4 +- crates/bevy_ecs/src/query/fetch.rs | 85 +++++++++++-------- crates/bevy_ecs/src/query/iter.rs | 4 +- crates/bevy_ecs/src/world/entity_ref.rs | 82 ++++++++++++------ 5 files changed, 109 insertions(+), 81 deletions(-) diff --git a/crates/bevy_animation/src/animation_curves.rs b/crates/bevy_animation/src/animation_curves.rs index 45fa393e05965..782daf0f552bc 100644 --- a/crates/bevy_animation/src/animation_curves.rs +++ b/crates/bevy_animation/src/animation_curves.rs @@ -408,10 +408,7 @@ impl AnimationCurveEvaluator for AnimatableCurveEvaluator { self.evaluator.push_blend_register(weight, graph_node) } - fn commit<'a>( - &mut self, - mut entity: AnimationEntityMut<'a>, - ) -> Result<(), AnimationEvaluationError> { + fn commit(&mut self, mut entity: AnimationEntityMut) -> Result<(), AnimationEvaluationError> { let property = self.property.get_mut(&mut entity)?; *property = self .evaluator @@ -596,10 +593,7 @@ impl AnimationCurveEvaluator for WeightsCurveEvaluator { Ok(()) } - fn commit<'a>( - &mut self, - mut entity: AnimationEntityMut<'a>, - ) -> Result<(), AnimationEvaluationError> { + fn commit(&mut self, mut entity: AnimationEntityMut) -> Result<(), AnimationEvaluationError> { if self.stack_morph_target_weights.is_empty() { return Ok(()); } @@ -905,10 +899,7 @@ pub trait AnimationCurveEvaluator: Downcast + Send + Sync + 'static { /// /// The property on the component must be overwritten with the value from /// the stack, not blended with it. - fn commit<'a>( - &mut self, - entity: AnimationEntityMut<'a>, - ) -> Result<(), AnimationEvaluationError>; + fn commit(&mut self, entity: AnimationEntityMut) -> Result<(), AnimationEvaluationError>; } impl_downcast!(AnimationCurveEvaluator); diff --git a/crates/bevy_animation/src/lib.rs b/crates/bevy_animation/src/lib.rs index ae7ce42ed67a2..585f5d122982c 100644 --- a/crates/bevy_animation/src/lib.rs +++ b/crates/bevy_animation/src/lib.rs @@ -1021,8 +1021,8 @@ pub fn advance_animations( } /// A type alias for [`EntityMutExcept`] as used in animation. -pub type AnimationEntityMut<'w> = - EntityMutExcept<'w, (AnimationTarget, AnimationPlayer, AnimationGraphHandle)>; +pub type AnimationEntityMut<'w, 's> = + EntityMutExcept<'w, 's, (AnimationTarget, AnimationPlayer, AnimationGraphHandle)>; /// A system that modifies animation targets (e.g. bones in a skinned mesh) /// according to the currently-playing animations. diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index c75d52d3566f7..e66606d11ca1b 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -14,7 +14,6 @@ use crate::{ use bevy_ptr::{ThinSlicePtr, UnsafeCellDeref}; use bevy_utils::prelude::DebugName; use core::{cell::UnsafeCell, marker::PhantomData, panic::Location}; -use smallvec::SmallVec; use variadics_please::all_tuples; /// Types that can be fetched from a [`World`] using a [`Query`]. @@ -1160,12 +1159,12 @@ unsafe impl<'a, 'b> QueryData for FilteredEntityMut<'a, 'b> { /// 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> +unsafe impl<'a, 'b, B> WorldQuery for EntityRefExcept<'a, 'b, B> where B: Bundle, { type Fetch<'w> = EntityFetch<'w>; - type State = SmallVec<[ComponentId; 4]>; + type State = Access; fn shrink_fetch<'wlong: 'wshort, 'wshort>(fetch: Self::Fetch<'wlong>) -> Self::Fetch<'wshort> { fetch @@ -1197,15 +1196,9 @@ where unsafe fn set_table<'w, 's>(_: &mut Self::Fetch<'w>, _: &'s Self::State, _: &'w Table) {} fn update_component_access( - state: &Self::State, + my_access: &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), @@ -1216,17 +1209,29 @@ where } fn init_state(world: &mut World) -> Self::State { - Self::get_state(world.components()).unwrap() + let mut access = Access::new(); + access.read_all_components(); + B::component_ids(&mut world.components_registrator(), &mut |id| { + access.remove_component_read(id); + }); + access } fn get_state(components: &Components) -> Option { - let mut ids = SmallVec::new(); + let mut access = Access::new(); + access.read_all_components(); B::get_component_ids(components, &mut |maybe_id| { + // If the component isn't registered, we don't have a `ComponentId` + // to use to exclude its access. + // Rather than fail, just try to take additional access. + // This is sound because access checks will run on the resulting access. + // Since the component isn't registered, there are no entities with that + // component, and the extra access will usually have no effect. if let Some(id) = maybe_id { - ids.push(id); + access.remove_component_read(id); } }); - Some(ids) + Some(access) } fn matches_component_set(_: &Self::State, _: &impl Fn(ComponentId) -> bool) -> bool { @@ -1235,13 +1240,13 @@ where } /// SAFETY: `Self` is the same as `Self::ReadOnly`. -unsafe impl<'a, B> QueryData for EntityRefExcept<'a, B> +unsafe impl<'a, 'b, B> QueryData for EntityRefExcept<'a, 'b, B> where B: Bundle, { const IS_READ_ONLY: bool = true; type ReadOnly = Self; - type Item<'w, 's> = EntityRefExcept<'w, B>; + type Item<'w, 's> = EntityRefExcept<'w, 's, B>; fn shrink<'wlong: 'wshort, 'wshort, 's>( item: Self::Item<'wlong, 's>, @@ -1250,7 +1255,7 @@ where } unsafe fn fetch<'w, 's>( - _state: &'s Self::State, + access: &'s Self::State, fetch: &mut Self::Fetch<'w>, entity: Entity, _: TableRow, @@ -1259,23 +1264,23 @@ where .world .get_entity_with_ticks(entity, fetch.last_run, fetch.this_run) .unwrap(); - EntityRefExcept::new(cell) + EntityRefExcept::new(cell, access) } } /// SAFETY: `EntityRefExcept` enforces read-only access to its contained /// components. -unsafe impl<'a, B> ReadOnlyQueryData for EntityRefExcept<'a, B> where B: Bundle {} +unsafe impl ReadOnlyQueryData for EntityRefExcept<'_, '_, 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> +unsafe impl<'a, 'b, B> WorldQuery for EntityMutExcept<'a, 'b, B> where B: Bundle, { type Fetch<'w> = EntityFetch<'w>; - type State = SmallVec<[ComponentId; 4]>; + type State = Access; fn shrink_fetch<'wlong: 'wshort, 'wshort>(fetch: Self::Fetch<'wlong>) -> Self::Fetch<'wshort> { fetch @@ -1307,15 +1312,9 @@ where unsafe fn set_table<'w, 's>(_: &mut Self::Fetch<'w>, _: &'s Self::State, _: &'w Table) {} fn update_component_access( - state: &Self::State, + my_access: &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), @@ -1326,17 +1325,29 @@ where } fn init_state(world: &mut World) -> Self::State { - Self::get_state(world.components()).unwrap() + let mut access = Access::new(); + access.write_all_components(); + B::component_ids(&mut world.components_registrator(), &mut |id| { + access.remove_component_read(id); + }); + access } fn get_state(components: &Components) -> Option { - let mut ids = SmallVec::new(); + let mut access = Access::new(); + access.write_all_components(); B::get_component_ids(components, &mut |maybe_id| { + // If the component isn't registered, we don't have a `ComponentId` + // to use to exclude its access. + // Rather than fail, just try to take additional access. + // This is sound because access checks will run on the resulting access. + // Since the component isn't registered, there are no entities with that + // component, and the extra access will usually have no effect. if let Some(id) = maybe_id { - ids.push(id); + access.remove_component_read(id); } }); - Some(ids) + Some(access) } fn matches_component_set(_: &Self::State, _: &impl Fn(ComponentId) -> bool) -> bool { @@ -1346,13 +1357,13 @@ where /// SAFETY: All accesses that `EntityRefExcept` provides are also accesses that /// `EntityMutExcept` provides. -unsafe impl<'a, B> QueryData for EntityMutExcept<'a, B> +unsafe impl<'a, 'b, B> QueryData for EntityMutExcept<'a, 'b, B> where B: Bundle, { const IS_READ_ONLY: bool = false; - type ReadOnly = EntityRefExcept<'a, B>; - type Item<'w, 's> = EntityMutExcept<'w, B>; + type ReadOnly = EntityRefExcept<'a, 'b, B>; + type Item<'w, 's> = EntityMutExcept<'w, 's, B>; fn shrink<'wlong: 'wshort, 'wshort, 's>( item: Self::Item<'wlong, 's>, @@ -1361,7 +1372,7 @@ where } unsafe fn fetch<'w, 's>( - _state: &'s Self::State, + access: &'s Self::State, fetch: &mut Self::Fetch<'w>, entity: Entity, _: TableRow, @@ -1370,7 +1381,7 @@ where .world .get_entity_with_ticks(entity, fetch.last_run, fetch.this_run) .unwrap(); - EntityMutExcept::new(cell) + EntityMutExcept::new(cell, access) } } diff --git a/crates/bevy_ecs/src/query/iter.rs b/crates/bevy_ecs/src/query/iter.rs index 606990f6b3f8b..cab2ee9c9391d 100644 --- a/crates/bevy_ecs/src/query/iter.rs +++ b/crates/bevy_ecs/src/query/iter.rs @@ -970,13 +970,13 @@ unsafe impl<'w, 's, F: QueryFilter> EntitySetIterator // SAFETY: [`QueryIter`] is guaranteed to return every matching entity once and only once. unsafe impl<'w, 's, F: QueryFilter, B: Bundle> EntitySetIterator - for QueryIter<'w, 's, EntityRefExcept<'_, B>, F> + for QueryIter<'w, 's, EntityRefExcept<'_, '_, B>, F> { } // SAFETY: [`QueryIter`] is guaranteed to return every matching entity once and only once. unsafe impl<'w, 's, F: QueryFilter, B: Bundle> EntitySetIterator - for QueryIter<'w, 's, EntityMutExcept<'_, B>, F> + for QueryIter<'w, 's, EntityMutExcept<'_, '_, B>, F> { } diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 3f57d77c10675..0b850f816bdd5 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -3561,6 +3561,14 @@ impl<'a> From<&'a EntityWorldMut<'_>> for FilteredEntityRef<'a, 'static> { } } +impl<'w, 's, B: Bundle> From<&'w EntityRefExcept<'_, 's, B>> for FilteredEntityRef<'w, 's> { + fn from(value: &'w EntityRefExcept<'_, 's, B>) -> Self { + // SAFETY: + // - The FilteredEntityRef has the same component access as the given EntityRefExcept. + unsafe { FilteredEntityRef::new(value.entity, value.access) } + } +} + impl PartialEq for FilteredEntityRef<'_, '_> { fn eq(&self, other: &Self) -> bool { self.entity() == other.entity() @@ -3884,6 +3892,14 @@ impl<'a> From<&'a mut EntityWorldMut<'_>> for FilteredEntityMut<'a, 'static> { } } +impl<'w, 's, B: Bundle> From<&'w EntityMutExcept<'_, 's, B>> for FilteredEntityMut<'w, 's> { + fn from(value: &'w EntityMutExcept<'_, 's, B>) -> Self { + // SAFETY: + // - The FilteredEntityMut has the same component access as the given EntityMutExcept. + unsafe { FilteredEntityMut::new(value.entity, value.access) } + } +} + impl PartialEq for FilteredEntityMut<'_, '_> { fn eq(&self, other: &Self) -> bool { self.entity() == other.entity() @@ -3938,23 +3954,28 @@ pub enum TryFromFilteredError { /// Provides read-only access to a single entity and all its components, save /// for an explicitly-enumerated set. -pub struct EntityRefExcept<'w, B> +pub struct EntityRefExcept<'w, 's, B> where B: Bundle, { entity: UnsafeEntityCell<'w>, + access: &'s Access, phantom: PhantomData, } -impl<'w, B> EntityRefExcept<'w, B> +impl<'w, 's, B> EntityRefExcept<'w, 's, B> where B: Bundle, { /// # Safety /// Other users of `UnsafeEntityCell` must only have mutable access to the components in `B`. - pub(crate) unsafe fn new(entity: UnsafeEntityCell<'w>) -> Self { + pub(crate) unsafe fn new( + entity: UnsafeEntityCell<'w>, + access: &'s Access, + ) -> Self { Self { entity, + access, phantom: PhantomData, } } @@ -4107,34 +4128,34 @@ where } } -impl<'a, B> From<&'a EntityMutExcept<'_, B>> for EntityRefExcept<'a, B> +impl<'w, 's, B> From<&'w EntityMutExcept<'_, 's, B>> for EntityRefExcept<'w, 's, B> where B: Bundle, { - fn from(entity: &'a EntityMutExcept<'_, B>) -> Self { + fn from(entity: &'w EntityMutExcept<'_, 's, B>) -> Self { // SAFETY: All accesses that `EntityRefExcept` provides are also // accesses that `EntityMutExcept` provides. - unsafe { EntityRefExcept::new(entity.entity) } + unsafe { EntityRefExcept::new(entity.entity, entity.access) } } } -impl Clone for EntityRefExcept<'_, B> { +impl Clone for EntityRefExcept<'_, '_, B> { fn clone(&self) -> Self { *self } } -impl Copy for EntityRefExcept<'_, B> {} +impl Copy for EntityRefExcept<'_, '_, B> {} -impl PartialEq for EntityRefExcept<'_, B> { +impl PartialEq for EntityRefExcept<'_, '_, B> { fn eq(&self, other: &Self) -> bool { self.entity() == other.entity() } } -impl Eq for EntityRefExcept<'_, B> {} +impl Eq for EntityRefExcept<'_, '_, B> {} -impl PartialOrd for EntityRefExcept<'_, B> { +impl PartialOrd for EntityRefExcept<'_, '_, B> { /// [`EntityRefExcept`]'s comparison trait implementations match the underlying [`Entity`], /// and cannot discern between different worlds. fn partial_cmp(&self, other: &Self) -> Option { @@ -4142,26 +4163,26 @@ impl PartialOrd for EntityRefExcept<'_, B> { } } -impl Ord for EntityRefExcept<'_, B> { +impl Ord for EntityRefExcept<'_, '_, B> { fn cmp(&self, other: &Self) -> Ordering { self.entity().cmp(&other.entity()) } } -impl Hash for EntityRefExcept<'_, B> { +impl Hash for EntityRefExcept<'_, '_, B> { fn hash(&self, state: &mut H) { self.entity().hash(state); } } -impl ContainsEntity for EntityRefExcept<'_, B> { +impl ContainsEntity for EntityRefExcept<'_, '_, B> { fn entity(&self) -> Entity { self.id() } } // SAFETY: This type represents one Entity. We implement the comparison traits based on that Entity. -unsafe impl EntityEquivalent for EntityRefExcept<'_, B> {} +unsafe impl EntityEquivalent for EntityRefExcept<'_, '_, B> {} /// Provides mutable access to all components of an entity, with the exception /// of an explicit set. @@ -4171,23 +4192,28 @@ unsafe impl EntityEquivalent for EntityRefExcept<'_, B> {} /// 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. -pub struct EntityMutExcept<'w, B> +pub struct EntityMutExcept<'w, 's, B> where B: Bundle, { entity: UnsafeEntityCell<'w>, + access: &'s Access, phantom: PhantomData, } -impl<'w, B> EntityMutExcept<'w, B> +impl<'w, 's, B> EntityMutExcept<'w, 's, B> where B: Bundle, { /// # Safety /// Other users of `UnsafeEntityCell` must not have access to any components not in `B`. - pub(crate) unsafe fn new(entity: UnsafeEntityCell<'w>) -> Self { + pub(crate) unsafe fn new( + entity: UnsafeEntityCell<'w>, + access: &'s Access, + ) -> Self { Self { entity, + access, phantom: PhantomData, } } @@ -4203,16 +4229,16 @@ where /// /// This is useful if you have `&mut EntityMutExcept`, but you need /// `EntityMutExcept`. - pub fn reborrow(&mut self) -> EntityMutExcept<'_, B> { + pub fn reborrow(&mut self) -> EntityMutExcept<'_, 's, B> { // SAFETY: We have exclusive access to the entire entity and the // applicable components. - unsafe { Self::new(self.entity) } + unsafe { Self::new(self.entity, self.access) } } /// 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> { + pub fn as_readonly(&self) -> EntityRefExcept<'_, 's, B> { EntityRefExcept::from(self) } @@ -4341,15 +4367,15 @@ where } } -impl PartialEq for EntityMutExcept<'_, B> { +impl PartialEq for EntityMutExcept<'_, '_, B> { fn eq(&self, other: &Self) -> bool { self.entity() == other.entity() } } -impl Eq for EntityMutExcept<'_, B> {} +impl Eq for EntityMutExcept<'_, '_, B> {} -impl PartialOrd for EntityMutExcept<'_, B> { +impl PartialOrd for EntityMutExcept<'_, '_, B> { /// [`EntityMutExcept`]'s comparison trait implementations match the underlying [`Entity`], /// and cannot discern between different worlds. fn partial_cmp(&self, other: &Self) -> Option { @@ -4357,26 +4383,26 @@ impl PartialOrd for EntityMutExcept<'_, B> { } } -impl Ord for EntityMutExcept<'_, B> { +impl Ord for EntityMutExcept<'_, '_, B> { fn cmp(&self, other: &Self) -> Ordering { self.entity().cmp(&other.entity()) } } -impl Hash for EntityMutExcept<'_, B> { +impl Hash for EntityMutExcept<'_, '_, B> { fn hash(&self, state: &mut H) { self.entity().hash(state); } } -impl ContainsEntity for EntityMutExcept<'_, B> { +impl ContainsEntity for EntityMutExcept<'_, '_, B> { fn entity(&self) -> Entity { self.id() } } // SAFETY: This type represents one Entity. We implement the comparison traits based on that Entity. -unsafe impl EntityEquivalent for EntityMutExcept<'_, B> {} +unsafe impl EntityEquivalent for EntityMutExcept<'_, '_, B> {} fn bundle_contains_component(components: &Components, query_id: ComponentId) -> bool where From e8aa1c12b1cdd2ee7cb2a3c0fb3622714eb8dd42 Mon Sep 17 00:00:00 2001 From: Chris Russell <8494645+chescock@users.noreply.github.com> Date: Sun, 13 Jul 2025 11:56:21 -0400 Subject: [PATCH 3/5] Remove `&` that are no longer needed to fix clippy lints. --- crates/bevy_ecs/src/query/fetch.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index e66606d11ca1b..57babe6f95f7b 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -1033,7 +1033,7 @@ unsafe impl QueryData for FilteredEntityRef<'_, '_> { .debug_checked_unwrap() }; // SAFETY: mutable access to every component has been registered. - unsafe { FilteredEntityRef::new(cell, &access) } + unsafe { FilteredEntityRef::new(cell, access) } } } @@ -1152,7 +1152,7 @@ unsafe impl<'a, 'b> QueryData for FilteredEntityMut<'a, 'b> { .debug_checked_unwrap() }; // SAFETY: mutable access to every component has been registered. - unsafe { FilteredEntityMut::new(cell, &access) } + unsafe { FilteredEntityMut::new(cell, access) } } } @@ -1196,16 +1196,16 @@ where unsafe fn set_table<'w, 's>(_: &mut Self::Fetch<'w>, _: &'s Self::State, _: &'w Table) {} fn update_component_access( - my_access: &Self::State, + state: &Self::State, filtered_access: &mut FilteredAccess, ) { let access = filtered_access.access_mut(); assert!( - access.is_compatible(&my_access), + access.is_compatible(state), "`EntityRefExcept<{}>` conflicts with a previous access in this query.", DebugName::type_name::(), ); - access.extend(&my_access); + access.extend(state); } fn init_state(world: &mut World) -> Self::State { @@ -1312,16 +1312,16 @@ where unsafe fn set_table<'w, 's>(_: &mut Self::Fetch<'w>, _: &'s Self::State, _: &'w Table) {} fn update_component_access( - my_access: &Self::State, + state: &Self::State, filtered_access: &mut FilteredAccess, ) { let access = filtered_access.access_mut(); assert!( - access.is_compatible(&my_access), + access.is_compatible(state), "`EntityMutExcept<{}>` conflicts with a previous access in this query.", DebugName::type_name::() ); - access.extend(&my_access); + access.extend(state); } fn init_state(world: &mut World) -> Self::State { From cf6f317693aa37a608b09836dac6ec2a1287fab3 Mon Sep 17 00:00:00 2001 From: Chris Russell <8494645+chescock@users.noreply.github.com> Date: Sun, 13 Jul 2025 12:11:52 -0400 Subject: [PATCH 4/5] Remove some `FilteredEntityRef::clone()` calls now that it's `Copy` to fix clippy lints. --- crates/bevy_remote/src/builtin_methods.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_remote/src/builtin_methods.rs b/crates/bevy_remote/src/builtin_methods.rs index e847da08ed067..847043eef9cef 100644 --- a/crates/bevy_remote/src/builtin_methods.rs +++ b/crates/bevy_remote/src/builtin_methods.rs @@ -880,7 +880,7 @@ pub fn process_remote_query_request(In(params): In>, world: &mut W // The map of boolean-valued component presences: let has_map = build_has_map( - row.clone(), + row, has_paths_and_reflect_components.iter().copied(), &unregistered_in_has, ); @@ -1466,7 +1466,7 @@ fn build_has_map<'a>( let mut has_map = >::default(); for (type_path, reflect_component) in paths_and_reflect_components { - let has = reflect_component.contains(entity_ref.clone()); + let has = reflect_component.contains(entity_ref); has_map.insert(type_path.to_owned(), Value::Bool(has)); } unregistered_components.iter().for_each(|component| { From eb881a4b7798bb2f9766063e00e7b6aa10d15295 Mon Sep 17 00:00:00 2001 From: Chris Russell <8494645+chescock@users.noreply.github.com> Date: Mon, 14 Jul 2025 20:19:33 -0400 Subject: [PATCH 5/5] Add unit test that would have failed before the `init_state` changes. --- crates/bevy_ecs/src/world/entity_ref.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 0b850f816bdd5..e2977c814d74f 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -5374,6 +5374,19 @@ mod tests { } } + #[test] + fn entity_mut_except_registers_components() { + // Checks for a bug where `EntityMutExcept` would not register the component and + // would therefore not include an exception, causing it to conflict with the later query. + fn system1(_query: Query>, _: Query<&mut TestComponent>) {} + let mut world = World::new(); + world.run_system_once(system1).unwrap(); + + fn system2(_: Query<&mut TestComponent>, _query: Query>) {} + let mut world = World::new(); + world.run_system_once(system2).unwrap(); + } + #[derive(Component)] struct A;