diff --git a/crates/bevy_asset/src/asset_changed.rs b/crates/bevy_asset/src/asset_changed.rs index b1d1042571..a01e64663c 100644 --- a/crates/bevy_asset/src/asset_changed.rs +++ b/crates/bevy_asset/src/asset_changed.rs @@ -246,28 +246,16 @@ unsafe impl WorldQuery for AssetChanged { component_access_set: &mut FilteredAccessSet, _world: UnsafeWorldCell, ) { - let combined_access = component_access_set.combined_access(); - assert!( - !combined_access.has_resource_write(state.resource_id), - "error[B0002]: AssetChanged<{}> in system {:?} conflicts with a previous system param. Consider removing the duplicate access. See: https://bevy.org/learn/errors/b0002", - DebugName::type_name::(), - system_name, - ); - let mut filter = FilteredAccess::default(); - filter.add_component_read(state.resource_id); - filter.add_resource_read(state.resource_id); + filter.add_read(state.resource_id); filter.and_with(IS_RESOURCE); - assert!(component_access_set - .get_conflicts_single(&filter) - .is_empty(), - "error[B0002]: AssetChanged<{}> in system {:?} conflicts with a previous system param. Consider removing the duplicate access. See: https://bevy.org/learn/errors/b0002", - DebugName::type_name::(), - system_name, - ); - - component_access_set.add(filter); + let conflicts = component_access_set.get_conflicts_single(&filter); + if conflicts.is_empty() { + component_access_set.add(filter); + return; + } + panic!("error[B0002]: AssetChanged<{}> in system {:?} conflicts with a previous system parameter. Consider removing the duplicate access. See: https://bevy.org/learn/errors/b0002", DebugName::type_name::(), system_name); } fn init_state(world: &mut World) -> AssetChangedState { @@ -326,6 +314,7 @@ mod tests { use crate::tests::create_app; use crate::{AssetEventSystems, Handle}; use alloc::{vec, vec::Vec}; + use bevy_ecs::system::assert_is_system; use core::num::NonZero; use std::println; @@ -336,7 +325,7 @@ mod tests { component::Component, message::MessageWriter, resource::Resource, - system::{assert_is_system, Commands, IntoSystem, Local, Query, Res, ResMut}, + system::{Commands, IntoSystem, Local, Query, Res, ResMut}, }; use bevy_reflect::TypePath; diff --git a/crates/bevy_ecs/src/entity_disabling.rs b/crates/bevy_ecs/src/entity_disabling.rs index 29c692f708..1e751070ac 100644 --- a/crates/bevy_ecs/src/entity_disabling.rs +++ b/crates/bevy_ecs/src/entity_disabling.rs @@ -257,9 +257,7 @@ mod tests { // A component access with an unrelated component let mut component_access = FilteredAccess::default(); - component_access - .access_mut() - .add_component_read(ComponentId::new(2)); + component_access.access_mut().add_read(ComponentId::new(2)); let mut applied_access = component_access.clone(); filters.modify_access(&mut applied_access); diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 1c46a4ca1c..f2fd313234 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -1542,8 +1542,8 @@ mod tests { let mut expected = FilteredAccess::default(); let a_id = world.components.get_id(TypeId::of::()).unwrap(); let b_id = world.components.get_id(TypeId::of::()).unwrap(); - expected.add_component_write(a_id); - expected.add_component_read(b_id); + expected.add_write(a_id); + expected.add_read(b_id); assert!( query.component_access.eq(&expected), "ComponentId access from query fetch and query filter should be combined" diff --git a/crates/bevy_ecs/src/query/access.rs b/crates/bevy_ecs/src/query/access.rs index 45521e3e6b..43d599ad11 100644 --- a/crates/bevy_ecs/src/query/access.rs +++ b/crates/bevy_ecs/src/query/access.rs @@ -1,5 +1,5 @@ -use crate::component::ComponentId; use crate::world::unsafe_world_cell::UnsafeWorldCell; +use crate::{component::ComponentId, resource::IS_RESOURCE}; use alloc::{format, string::String, vec, vec::Vec}; use core::{fmt, fmt::Debug}; use derive_more::From; @@ -47,26 +47,16 @@ impl<'a> Debug for FormattedBitSet<'a> { pub struct Access { /// All accessed components, or forbidden components if /// `Self::component_read_and_writes_inverted` is set. - component_read_and_writes: FixedBitSet, + read_and_writes: FixedBitSet, /// 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, + writes: FixedBitSet, /// Is `true` if this component can read all components *except* those - /// present in `Self::component_read_and_writes`. - component_read_and_writes_inverted: bool, + /// present in `Self::read_and_writes`. + 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, - /// Is `true` if this has mutable access to all resources. - /// If this is true, then `reads_all` must also be true. - writes_all_resources: bool, + /// present in `Self::writes`. + writes_inverted: bool, // Components that are not accessed, but whose presence in an archetype affect query results. archetypal: FixedBitSet, } @@ -75,29 +65,19 @@ pub struct Access { impl Clone for Access { fn clone(&self) -> Self { Self { - component_read_and_writes: self.component_read_and_writes.clone(), - component_writes: self.component_writes.clone(), - resource_read_and_writes: self.resource_read_and_writes.clone(), - resource_writes: self.resource_writes.clone(), - 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, + read_and_writes: self.read_and_writes.clone(), + writes: self.writes.clone(), + read_and_writes_inverted: self.read_and_writes_inverted, + writes_inverted: self.writes_inverted, archetypal: self.archetypal.clone(), } } fn clone_from(&mut self, source: &Self) { - self.component_read_and_writes - .clone_from(&source.component_read_and_writes); - self.component_writes.clone_from(&source.component_writes); - self.resource_read_and_writes - .clone_from(&source.resource_read_and_writes); - self.resource_writes.clone_from(&source.resource_writes); - 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.read_and_writes.clone_from(&source.read_and_writes); + self.writes.clone_from(&source.writes); + self.read_and_writes_inverted = source.read_and_writes_inverted; + self.writes_inverted = source.writes_inverted; self.archetypal.clone_from(&source.archetypal); } } @@ -106,28 +86,12 @@ impl Debug for Access { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("Access") .field( - "component_read_and_writes", - &FormattedBitSet::new(&self.component_read_and_writes), + "read_and_writes", + &FormattedBitSet::new(&self.read_and_writes), ) - .field( - "component_writes", - &FormattedBitSet::new(&self.component_writes), - ) - .field( - "resource_read_and_writes", - &FormattedBitSet::new(&self.resource_read_and_writes), - ) - .field( - "resource_writes", - &FormattedBitSet::new(&self.resource_writes), - ) - .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("writes", &FormattedBitSet::new(&self.writes)) + .field("read_and_writes_inverted", &self.read_and_writes_inverted) + .field("writes_inverted", &self.writes_inverted) .field("archetypal", &FormattedBitSet::new(&self.archetypal)) .finish() } @@ -137,14 +101,10 @@ impl Access { /// Creates an empty [`Access`] collection. pub const fn new() -> Self { Self { - reads_all_resources: false, - writes_all_resources: 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(), - resource_writes: FixedBitSet::new(), + read_and_writes_inverted: false, + writes_inverted: false, + read_and_writes: FixedBitSet::new(), + writes: FixedBitSet::new(), archetypal: FixedBitSet::new(), } } @@ -154,10 +114,9 @@ impl Access { /// 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()` + // Note that we cannot use `read_all()` // because `FixedBitSet::clear()` is not `const`. - access.component_read_and_writes_inverted = true; + access.read_and_writes_inverted = true; access } @@ -166,71 +125,94 @@ impl Access { /// 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()` + // Note that we cannot use `write_all()` // because `FixedBitSet::clear()` is not `const`. - access.component_read_and_writes_inverted = true; - access.component_writes_inverted = true; + access.read_and_writes_inverted = true; + access.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); - } else if index < self.component_read_and_writes.len() { - self.component_read_and_writes.remove(index); + fn add_sparse_set_index_read(&mut self, index: usize) { + if !self.read_and_writes_inverted { + self.read_and_writes.grow_and_insert(index); + } else if index < self.read_and_writes.len() { + self.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); + fn add_sparse_set_index_write(&mut self, index: usize) { + if !self.writes_inverted { + self.writes.grow_and_insert(index); + } else if index < self.writes.len() { + self.writes.remove(index); } } /// Adds access to the component given by `index`. + #[deprecated(since = "0.19.0", note = "use Access::add_read")] pub fn add_component_read(&mut self, index: ComponentId) { + self.add_read(index); + } + + /// Adds access to the component given by `index`. + pub fn add_read(&mut self, index: ComponentId) { let sparse_set_index = index.index(); - self.add_component_sparse_set_index_read(sparse_set_index); + self.add_sparse_set_index_read(sparse_set_index); } /// Adds exclusive access to the component given by `index`. + #[deprecated(since = "0.19.0", note = "use Access::add_write")] pub fn add_component_write(&mut self, index: ComponentId) { + self.add_write(index); + } + + /// Adds exclusive access to the component given by `index`. + pub fn add_write(&mut self, index: ComponentId) { let sparse_set_index = index.index(); - self.add_component_sparse_set_index_read(sparse_set_index); - self.add_component_sparse_set_index_write(sparse_set_index); + self.add_sparse_set_index_read(sparse_set_index); + self.add_sparse_set_index_write(sparse_set_index); } /// Adds access to the resource given by `index`. + #[deprecated( + since = "0.19.0", + note = "Call `FilteredAccessSet::add_resource_read`. If this is called in a `WorldQuery` impl, then you will need to implement `init_nested_access` to modify the `FilteredAccessSet`." + )] pub fn add_resource_read(&mut self, index: ComponentId) { - self.resource_read_and_writes.grow_and_insert(index.index()); + self.add_read(index); } /// Adds exclusive access to the resource given by `index`. + #[deprecated( + since = "0.19.0", + note = "Call `FilteredAccessSet::add_resource_write`. If this is called in a `WorldQuery` impl, then you will need to implement `init_nested_access` to modify the `FilteredAccessSet`." + )] pub fn add_resource_write(&mut self, index: ComponentId) { - self.resource_read_and_writes.grow_and_insert(index.index()); - self.resource_writes.grow_and_insert(index.index()); + self.add_write(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_sparse_set_index_read(&mut self, index: usize) { + if self.read_and_writes_inverted { + self.read_and_writes.grow_and_insert(index); + } else if index < self.read_and_writes.len() { + self.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); + fn remove_sparse_set_index_write(&mut self, index: usize) { + if self.writes_inverted { + self.writes.grow_and_insert(index); + } else if index < self.writes.len() { + self.writes.remove(index); } } + /// Removes both read and write access to the component given by `index`. + #[deprecated(since = "0.19.0", note = "use Access::remove_read")] + pub fn remove_component_read(&mut self, index: ComponentId) { + self.remove_read(index); + } + /// Removes both read and write access to the component given by `index`. /// /// Because this method corresponds to the set difference operator ∖, it can @@ -238,11 +220,17 @@ impl Access { /// 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: ComponentId) { + /// `remove_read`. + pub fn remove_read(&mut self, index: ComponentId) { let sparse_set_index = index.index(); - self.remove_component_sparse_set_index_write(sparse_set_index); - self.remove_component_sparse_set_index_read(sparse_set_index); + self.remove_sparse_set_index_write(sparse_set_index); + self.remove_sparse_set_index_read(sparse_set_index); + } + + /// Removes write access to the component given by `index`. + #[deprecated(since = "0.19.0", note = "use Access::remove_write")] + pub fn remove_component_write(&mut self, index: ComponentId) { + self.remove_write(index); } /// Removes write access to the component given by `index`. @@ -250,12 +238,12 @@ impl Access { /// 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 + /// can't replace a call to `remove_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: ComponentId) { + /// `remove_write`. + pub fn remove_write(&mut self, index: ComponentId) { let sparse_set_index = index.index(); - self.remove_component_sparse_set_index_write(sparse_set_index); + self.remove_sparse_set_index_write(sparse_set_index); } /// Adds an archetypal (indirect) access to the component given by `index`. @@ -272,54 +260,71 @@ impl Access { } /// Returns `true` if this can access the component given by `index`. + #[deprecated(since = "0.19.0", note = "use Access::has_read")] pub fn has_component_read(&self, index: ComponentId) -> bool { - self.component_read_and_writes_inverted - ^ self.component_read_and_writes.contains(index.index()) + self.has_read(index) + } + + /// Returns `true` if this can access the component given by `index`. + pub fn has_read(&self, index: ComponentId) -> bool { + self.read_and_writes_inverted ^ self.read_and_writes.contains(index.index()) } /// Returns `true` if this can access any component. + #[deprecated(since = "0.19.0", note = "use Access::has_any_read")] pub fn has_any_component_read(&self) -> bool { - self.component_read_and_writes_inverted || !self.component_read_and_writes.is_clear() + self.has_any_read() + } + + /// Returns `true` if this can access any component. + pub fn has_any_read(&self) -> bool { + self.read_and_writes_inverted || !self.read_and_writes.is_clear() } /// Returns `true` if this can exclusively access the component given by `index`. + #[deprecated(since = "0.19.0", note = "use Access::has_write")] pub fn has_component_write(&self, index: ComponentId) -> bool { - self.component_writes_inverted ^ self.component_writes.contains(index.index()) + self.has_write(index) + } + + /// Returns `true` if this can exclusively access the component given by `index`. + pub fn has_write(&self, index: ComponentId) -> bool { + self.writes_inverted ^ self.writes.contains(index.index()) } /// Returns `true` if this accesses any component mutably. + #[deprecated(since = "0.19.0", note = "use Access::has_any_write")] pub fn has_any_component_write(&self) -> bool { - self.component_writes_inverted || !self.component_writes.is_clear() + self.has_any_write() + } + + /// Returns `true` if this accesses any component mutably. + pub fn has_any_write(&self) -> bool { + self.writes_inverted || !self.writes.is_clear() } /// Returns `true` if this can access the resource given by `index`. + #[deprecated(since = "0.19.0", note = "use Access::has_component_read")] pub fn has_resource_read(&self, index: ComponentId) -> bool { - self.reads_all_resources || self.resource_read_and_writes.contains(index.index()) + self.has_read(index) } /// Returns `true` if this can access any resource. + #[deprecated(since = "0.19.0", note = "use Access::has_any_component_read")] pub fn has_any_resource_read(&self) -> bool { - self.reads_all_resources || !self.resource_read_and_writes.is_clear() + self.has_any_read() } /// Returns `true` if this can exclusively access the resource given by `index`. + #[deprecated(since = "0.19.0", note = "use Access::has_component_write")] pub fn has_resource_write(&self, index: ComponentId) -> bool { - self.writes_all_resources || self.resource_writes.contains(index.index()) + self.has_write(index) } /// Returns `true` if this accesses any resource mutably. + #[deprecated(since = "0.19.0", note = "use Access::has_any_component_write")] pub fn has_any_resource_write(&self) -> bool { - self.writes_all_resources || !self.resource_writes.is_clear() - } - - /// Returns `true` if this accesses any resources or components. - pub fn has_any_read(&self) -> bool { - self.has_any_component_read() || self.has_any_resource_read() - } - - /// Returns `true` if this accesses any resources or components mutably. - pub fn has_any_write(&self) -> bool { - self.has_any_component_write() || self.has_any_resource_write() + self.has_any_write() } /// Returns true if this has an archetypal (indirect) access to the component given by `index`. @@ -335,121 +340,84 @@ impl Access { } /// Sets this as having access to all components (i.e. `EntityRef`). - #[inline] + #[deprecated(since = "0.19.0", note = "use Access::read_all")] pub fn read_all_components(&mut self) { - self.component_read_and_writes_inverted = true; - self.component_read_and_writes.clear(); + self.read_all(); } - /// Sets this as having mutable access to all components (i.e. `EntityMut`). - #[inline] - pub fn write_all_components(&mut self) { - self.read_all_components(); - self.component_writes_inverted = true; - self.component_writes.clear(); - } - - /// Sets this as having access to all resources (i.e. `&World`). - #[inline] - pub const fn read_all_resources(&mut self) { - self.reads_all_resources = true; - } - - /// Sets this as having mutable access to all resources (i.e. `&mut World`). - #[inline] - pub const fn write_all_resources(&mut self) { - self.reads_all_resources = true; - self.writes_all_resources = true; - } - - /// Sets this as having access to all indexed elements (i.e. `&World`). + /// Sets this as having access to all components (i.e. `EntityRef` and `&World`). #[inline] pub fn read_all(&mut self) { - self.read_all_components(); - self.read_all_resources(); + self.read_and_writes_inverted = true; + self.read_and_writes.clear(); } - /// Sets this as having mutable access to all indexed elements (i.e. `&mut World`). + /// Sets this as having mutable access to all components (i.e. `EntityMut` and `&mut World`). + #[deprecated(since = "0.19.0", note = "use Access::write_all")] + pub fn write_all_components(&mut self) { + self.write_all(); + } + + /// Sets this as having mutable access to all components (i.e. `EntityMut` and `&mut World`). #[inline] pub fn write_all(&mut self) { - self.write_all_components(); - self.write_all_resources(); + self.read_all(); + self.writes_inverted = true; + self.writes.clear(); } - /// Returns `true` if this has access to all components (i.e. `EntityRef`). - #[inline] + /// Returns `true` if this has access to all components (i.e. `EntityRef` and `&World`). + #[deprecated(since = "0.19.0", note = "use Access::has_read_all")] pub fn has_read_all_components(&self) -> bool { - self.component_read_and_writes_inverted && self.component_read_and_writes.is_clear() + self.has_read_all() } - /// Returns `true` if this has write access to all components (i.e. `EntityMut`). + /// Returns `true` if this has access to all components (i.e. `EntityRef` and `&World`). #[inline] - pub fn has_write_all_components(&self) -> bool { - self.component_writes_inverted && self.component_writes.is_clear() - } - - /// Returns `true` if this has access to all resources (i.e. `EntityRef`). - #[inline] - pub fn has_read_all_resources(&self) -> bool { - self.reads_all_resources - } - - /// Returns `true` if this has write access to all resources (i.e. `EntityMut`). - #[inline] - pub fn has_write_all_resources(&self) -> bool { - self.writes_all_resources - } - - /// Returns `true` if this has access to all indexed elements (i.e. `&World`). pub fn has_read_all(&self) -> bool { - self.has_read_all_components() && self.has_read_all_resources() + self.read_and_writes_inverted && self.read_and_writes.is_clear() } - /// Returns `true` if this has write access to all indexed elements (i.e. `&mut World`). + /// Returns `true` if this has write access to all components (i.e. `EntityMut` and `&mut World`). + #[deprecated(since = "0.19.0", note = "use Access::has_write_all")] + pub fn has_write_all_components(&self) -> bool { + self.has_write_all() + } + + /// Returns `true` if this has write access to all components (i.e. `EntityMut` and `&mut World`). + #[inline] pub fn has_write_all(&self) -> bool { - self.has_write_all_components() && self.has_write_all_resources() + self.writes_inverted && self.writes.is_clear() } /// Removes all writes. pub fn clear_writes(&mut self) { - self.writes_all_resources = false; - self.component_writes_inverted = false; - self.component_writes.clear(); - self.resource_writes.clear(); + self.writes_inverted = false; + self.writes.clear(); } /// Removes all accesses. pub fn clear(&mut self) { - self.reads_all_resources = false; - self.writes_all_resources = 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(); - self.resource_writes.clear(); + self.read_and_writes_inverted = false; + self.writes_inverted = false; + self.read_and_writes.clear(); + self.writes.clear(); } /// Adds all access from `other`. pub fn extend(&mut self, other: &Access) { invertible_union_with( - &mut self.component_read_and_writes, - &mut self.component_read_and_writes_inverted, - &other.component_read_and_writes, - other.component_read_and_writes_inverted, + &mut self.read_and_writes, + &mut self.read_and_writes_inverted, + &other.read_and_writes, + other.read_and_writes_inverted, ); invertible_union_with( - &mut self.component_writes, - &mut self.component_writes_inverted, - &other.component_writes, - other.component_writes_inverted, + &mut self.writes, + &mut self.writes_inverted, + &other.writes, + other.writes_inverted, ); - - 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.resource_read_and_writes - .union_with(&other.resource_read_and_writes); - self.resource_writes.union_with(&other.resource_writes); self.archetypal.union_with(&other.archetypal); } @@ -458,38 +426,30 @@ impl Access { /// and removes any writes for any component read by `other`. pub fn remove_conflicting_access(&mut self, other: &Access) { invertible_difference_with( - &mut self.component_read_and_writes, - &mut self.component_read_and_writes_inverted, - &other.component_writes, - other.component_writes_inverted, + &mut self.read_and_writes, + &mut self.read_and_writes_inverted, + &other.writes, + other.writes_inverted, ); invertible_difference_with( - &mut self.component_writes, - &mut self.component_writes_inverted, - &other.component_read_and_writes, - other.component_read_and_writes_inverted, + &mut self.writes, + &mut self.writes_inverted, + &other.read_and_writes, + other.read_and_writes_inverted, ); - - if other.reads_all_resources { - self.writes_all_resources = false; - self.resource_writes.clear(); - } - if other.writes_all_resources { - self.reads_all_resources = false; - self.resource_read_and_writes.clear(); - } - self.resource_read_and_writes - .difference_with(&other.resource_writes); - self.resource_writes - .difference_with(&other.resource_read_and_writes); } - /// Returns `true` if the access and `other` can be active at the same time, - /// only looking at their component access. + /// Returns `true` if the access and `other` can be active at the same time. + #[deprecated(since = "0.19.0", note = "use Access::is_compatible")] + pub fn is_components_compatible(&self, other: &Access) -> bool { + self.is_compatible(other) + } + + /// Returns `true` if the access and `other` can be active at the same time. /// /// [`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 { + pub fn is_compatible(&self, other: &Access) -> bool { // We have a conflict if we write and they read or write, or if they // write and we read or write. for ( @@ -499,16 +459,16 @@ impl Access { rhs_reads_and_writes_inverted, ) in [ ( - &self.component_writes, - &other.component_read_and_writes, - self.component_writes_inverted, - other.component_read_and_writes_inverted, + &self.writes, + &other.read_and_writes, + self.writes_inverted, + other.read_and_writes_inverted, ), ( - &other.component_writes, - &self.component_read_and_writes, - other.component_writes_inverted, - self.component_read_and_writes_inverted, + &other.writes, + &self.read_and_writes, + other.writes_inverted, + self.read_and_writes_inverted, ), ] { match (lhs_writes_inverted, rhs_reads_and_writes_inverted) { @@ -534,46 +494,16 @@ impl Access { true } - /// Returns `true` if the access and `other` can be active at the same time, - /// only looking at their resource access. - /// - /// [`Access`] instances are incompatible if one can write - /// an element that the other can read or write. - pub fn is_resources_compatible(&self, other: &Access) -> bool { - if self.writes_all_resources { - return !other.has_any_resource_read(); - } - - if other.writes_all_resources { - return !self.has_any_resource_read(); - } - - if self.reads_all_resources { - return !other.has_any_resource_write(); - } - - if other.reads_all_resources { - return !self.has_any_resource_write(); - } - - self.resource_writes - .is_disjoint(&other.resource_read_and_writes) - && other - .resource_writes - .is_disjoint(&self.resource_read_and_writes) - } - - /// Returns `true` if the access and `other` can be active at the same time. - /// - /// [`Access`] instances are incompatible if one can write - /// an element that the other can read or write. - pub fn is_compatible(&self, other: &Access) -> bool { - self.is_components_compatible(other) && self.is_resources_compatible(other) - } - - /// 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`. + /// Returns `true` if the set is a subset of another, i.e. `other` contains + /// at least all the values in `self`. + #[deprecated(since = "0.19.0", note = "use Access::is_subset")] pub fn is_subset_components(&self, other: &Access) -> bool { + self.is_subset(other) + } + + /// Returns `true` if the set is a subset of another, i.e. `other` contains + /// at least all the values in `self`. + pub fn is_subset(&self, other: &Access) -> bool { for ( our_components, their_components, @@ -581,16 +511,16 @@ impl Access { 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.read_and_writes, + &other.read_and_writes, + self.read_and_writes_inverted, + other.read_and_writes_inverted, ), ( - &self.component_writes, - &other.component_writes, - self.component_writes_inverted, - other.component_writes_inverted, + &self.writes, + &other.writes, + self.writes_inverted, + other.writes_inverted, ), ] { match (our_components_inverted, their_components_inverted) { @@ -618,37 +548,9 @@ impl Access { true } - /// Returns `true` if the set's resource access is a subset of another, i.e. `other`'s resource access - /// contains at least all the values in `self`. - pub fn is_subset_resources(&self, other: &Access) -> bool { - if self.writes_all_resources { - return other.writes_all_resources; - } - - if other.writes_all_resources { - return true; - } - - if self.reads_all_resources { - return other.reads_all_resources; - } - - if other.reads_all_resources { - return self.resource_writes.is_subset(&other.resource_writes); - } - - self.resource_read_and_writes - .is_subset(&other.resource_read_and_writes) - && self.resource_writes.is_subset(&other.resource_writes) - } - - /// Returns `true` if the set is a subset of another, i.e. `other` contains - /// at least all the values in `self`. - pub fn is_subset(&self, other: &Access) -> bool { - self.is_subset_components(other) && self.is_subset_resources(other) - } - - fn get_component_conflicts(&self, other: &Access) -> AccessConflicts { + /// Returns a vector of elements that the access and `other` cannot access at the same time. + #[inline] + pub fn get_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 @@ -660,16 +562,16 @@ impl Access { rhs_reads_and_writes_inverted, ) in [ ( - &self.component_writes, - &other.component_read_and_writes, - self.component_writes_inverted, - other.component_read_and_writes_inverted, + &self.writes, + &other.read_and_writes, + self.writes_inverted, + other.read_and_writes_inverted, ), ( - &other.component_writes, - &self.component_read_and_writes, - other.component_writes_inverted, - self.component_read_and_writes_inverted, + &other.writes, + &self.read_and_writes, + other.writes_inverted, + self.read_and_writes_inverted, ), ] { // There's no way that I can see to do this without a temporary. @@ -687,62 +589,6 @@ impl Access { 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 = match self.get_component_conflicts(other) { - AccessConflicts::All => return AccessConflicts::All, - AccessConflicts::Individual(conflicts) => conflicts, - }; - - if self.reads_all_resources { - if other.writes_all_resources { - return AccessConflicts::All; - } - conflicts.extend(other.resource_writes.ones()); - } - - if other.reads_all_resources { - if self.writes_all_resources { - return AccessConflicts::All; - } - conflicts.extend(self.resource_writes.ones()); - } - if self.writes_all_resources { - conflicts.extend(other.resource_read_and_writes.ones()); - } - - if other.writes_all_resources { - conflicts.extend(self.resource_read_and_writes.ones()); - } - - conflicts.extend( - self.resource_writes - .intersection(&other.resource_read_and_writes), - ); - conflicts.extend( - self.resource_read_and_writes - .intersection(&other.resource_writes), - ); - AccessConflicts::Individual(conflicts) - } - - /// Returns the indices of the resources this has access to. - pub fn resource_reads_and_writes(&self) -> impl Iterator + '_ { - self.resource_read_and_writes.ones().map(ComponentId::new) - } - - /// Returns the indices of the resources this has non-exclusive access to. - pub fn resource_reads(&self) -> impl Iterator + '_ { - self.resource_read_and_writes - .difference(&self.resource_writes) - .map(ComponentId::new) - } - - /// Returns the indices of the resources this has exclusive access to. - pub fn resource_writes(&self) -> impl Iterator + '_ { - self.resource_writes.ones().map(ComponentId::new) - } - /// 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), @@ -755,6 +601,14 @@ impl Access { self.archetypal.ones().map(ComponentId::new) } + /// Returns an iterator over the component IDs and their [`ComponentAccessKind`]. + #[deprecated(since = "0.19.0", note = "use Access::try_iter_access")] + pub fn try_iter_component_access( + &self, + ) -> Result + '_, UnboundedAccessError> { + self.try_iter_access() + } + /// Returns an iterator over the component IDs and their [`ComponentAccessKind`]. /// /// Returns `Err(UnboundedAccess)` if the access is unbounded. @@ -768,12 +622,12 @@ impl Access { /// # use bevy_ecs::component::ComponentId; /// let mut access = Access::default(); /// - /// access.add_component_read(ComponentId::new(1)); - /// access.add_component_write(ComponentId::new(2)); + /// access.add_read(ComponentId::new(1)); + /// access.add_write(ComponentId::new(2)); /// access.add_archetypal(ComponentId::new(3)); /// /// let result = access - /// .try_iter_component_access() + /// .try_iter_access() /// .map(Iterator::collect::>); /// /// assert_eq!( @@ -785,22 +639,22 @@ impl Access { /// ]), /// ); /// ``` - pub fn try_iter_component_access( + pub fn try_iter_access( &self, ) -> Result + '_, UnboundedAccessError> { - // component_writes_inverted is only ever true when component_read_and_writes_inverted is - // also true. Therefore it is sufficient to check just component_read_and_writes_inverted. - if self.component_read_and_writes_inverted { + // writes_inverted is only ever true when read_and_writes_inverted is + // also true. Therefore it is sufficient to check just read_and_writes_inverted. + if self.read_and_writes_inverted { return Err(UnboundedAccessError { - writes_inverted: self.component_writes_inverted, - read_and_writes_inverted: self.component_read_and_writes_inverted, + writes_inverted: self.writes_inverted, + read_and_writes_inverted: self.read_and_writes_inverted, }); } - let reads_and_writes = self.component_read_and_writes.ones().map(|index| { + let reads_and_writes = self.read_and_writes.ones().map(|index| { let sparse_index = ComponentId::new(index); - if self.component_writes.contains(index) { + if self.writes.contains(index) { ComponentAccessKind::Exclusive(sparse_index) } else { ComponentAccessKind::Shared(sparse_index) @@ -810,10 +664,7 @@ impl Access { let archetypal = self .archetypal .ones() - .filter(|&index| { - !self.component_writes.contains(index) - && !self.component_read_and_writes.contains(index) - }) + .filter(|&index| !self.writes.contains(index) && !self.read_and_writes.contains(index)) .map(|index| ComponentAccessKind::Archetypal(ComponentId::new(index))); Ok(reads_and_writes.chain(archetypal)) @@ -1057,29 +908,31 @@ impl FilteredAccess { } /// Adds access to the component given by `index`. + #[deprecated(since = "0.19.0", note = "use FilteredAccess::add_read")] pub fn add_component_read(&mut self, index: ComponentId) { - self.access.add_component_read(index); + self.add_read(index); + } + + /// Adds access to the component given by `index`. + pub fn add_read(&mut self, index: ComponentId) { + self.access.add_read(index); self.add_required(index); self.and_with(index); } /// Adds exclusive access to the component given by `index`. + #[deprecated(since = "0.19.0", note = "use FilteredAccess::add_write")] pub fn add_component_write(&mut self, index: ComponentId) { - self.access.add_component_write(index); + self.add_write(index); + } + + /// Adds exclusive access to the component given by `index`. + pub fn add_write(&mut self, index: ComponentId) { + self.access.add_write(index); self.add_required(index); self.and_with(index); } - /// Adds access to the resource given by `index`. - pub fn add_resource_read(&mut self, index: ComponentId) { - self.access.add_resource_read(index); - } - - /// Adds exclusive access to the resource given by `index`. - pub fn add_resource_write(&mut self, index: ComponentId) { - self.access.add_resource_write(index); - } - fn add_required(&mut self, index: ComponentId) { self.required.grow_and_insert(index.index()); } @@ -1120,13 +973,7 @@ impl FilteredAccess { /// Returns `true` if this and `other` can be active at the same time. pub fn is_compatible(&self, other: &FilteredAccess) -> bool { - // Resources are read from the world rather than the filtered archetypes, - // so they must be compatible even if the filters are disjoint. - if !self.access.is_resources_compatible(&other.access) { - return false; - } - - if self.access.is_components_compatible(&other.access) { + if self.access.is_compatible(&other.access) { return true; } @@ -1186,24 +1033,26 @@ impl FilteredAccess { self.filter_sets = new_filters; } - /// Sets the underlying unfiltered access as having access to all indexed elements. + /// Sets the underlying unfiltered access as having access to all components. pub fn read_all(&mut self) { self.access.read_all(); } - /// Sets the underlying unfiltered access as having mutable access to all indexed elements. + /// Sets the underlying unfiltered access as having mutable access to all components. pub fn write_all(&mut self) { self.access.write_all(); } /// Sets the underlying unfiltered access as having access to all components. + #[deprecated(since = "0.19.0", note = "use FilteredAccess::read_all")] pub fn read_all_components(&mut self) { - self.access.read_all_components(); + self.read_all(); } /// Sets the underlying unfiltered access as having mutable access to all components. + #[deprecated(since = "0.19.0", note = "use FilteredAccess::write_all")] pub fn write_all_components(&mut self) { - self.access.write_all_components(); + self.write_all(); } /// Returns `true` if the set is a subset of another, i.e. `other` contains @@ -1387,30 +1236,58 @@ impl FilteredAccessSet { } /// Adds a read access to a resource to the set. + #[deprecated(since = "0.19.0", note = "FilteredAccessSet::add_resource_read")] pub fn add_unfiltered_resource_read(&mut self, index: ComponentId) { + self.add_resource_read(index); + } + + /// Adds a read access to a resource to the set. + pub fn add_resource_read(&mut self, index: ComponentId) { let mut filter = FilteredAccess::default(); - filter.add_resource_read(index); + filter.add_read(index); + filter.and_with(IS_RESOURCE); + self.add(filter); + } + + /// Adds a read access to a component to the set. + pub(crate) fn add_unfiltered_component_read(&mut self, index: ComponentId) { + let mut filter = FilteredAccess::default(); + filter.add_read(index); + self.add(filter); + } + + /// Adds read access to all components to the set. + pub fn add_unfiltered_read_all_components(&mut self) { + let mut filter = FilteredAccess::default(); + filter.access.read_all(); self.add(filter); } /// Adds a write access to a resource to the set. + #[deprecated(since = "0.19.0", note = "FilteredAccessSet::add_resource_write")] pub fn add_unfiltered_resource_write(&mut self, index: ComponentId) { + self.add_resource_write(index); + } + + /// Adds a write access to a resource to the set. + pub fn add_resource_write(&mut self, index: ComponentId) { let mut filter = FilteredAccess::default(); - filter.add_resource_write(index); + filter.add_write(index); + filter.and_with(IS_RESOURCE); self.add(filter); } - /// Adds read access to all resources to the set. - pub fn add_unfiltered_read_all_resources(&mut self) { + /// Adds a write access to a resource to the set. + pub(crate) fn add_unfiltered_component_write(&mut self, index: ComponentId) { let mut filter = FilteredAccess::default(); - filter.access.read_all_resources(); + filter.add_write(index); self.add(filter); } - /// Adds write access to all resources to the set. - pub fn add_unfiltered_write_all_resources(&mut self) { + /// Adds write access to all components to the set. + pub fn add_unfiltered_write_all_components(&mut self) { let mut filter = FilteredAccess::default(); - filter.access.write_all_resources(); + filter.write_all(); self.add(filter); } @@ -1459,9 +1336,9 @@ mod tests { fn create_sample_access() -> Access { let mut access = Access::default(); - access.add_component_read(ComponentId::new(1)); - access.add_component_read(ComponentId::new(2)); - access.add_component_write(ComponentId::new(3)); + access.add_read(ComponentId::new(1)); + access.add_read(ComponentId::new(2)); + access.add_write(ComponentId::new(3)); access.add_archetypal(ComponentId::new(5)); access.read_all(); @@ -1471,8 +1348,8 @@ mod tests { fn create_sample_filtered_access() -> FilteredAccess { let mut filtered_access = FilteredAccess::default(); - filtered_access.add_component_write(ComponentId::new(1)); - filtered_access.add_component_read(ComponentId::new(2)); + filtered_access.add_write(ComponentId::new(1)); + filtered_access.add_read(ComponentId::new(2)); filtered_access.add_required(ComponentId::new(3)); filtered_access.and_with(ComponentId::new(4)); @@ -1491,8 +1368,8 @@ mod tests { fn create_sample_filtered_access_set() -> FilteredAccessSet { let mut filtered_access_set = FilteredAccessSet::default(); - filtered_access_set.add_unfiltered_resource_read(ComponentId::new(2)); - filtered_access_set.add_unfiltered_resource_write(ComponentId::new(4)); + filtered_access_set.add_unfiltered_component_read(ComponentId::new(2)); + filtered_access_set.add_unfiltered_component_write(ComponentId::new(4)); filtered_access_set.read_all(); filtered_access_set @@ -1511,8 +1388,8 @@ mod tests { let original = create_sample_access(); let mut cloned = Access::default(); - cloned.add_component_write(ComponentId::new(7)); - cloned.add_component_read(ComponentId::new(4)); + cloned.add_write(ComponentId::new(7)); + cloned.add_read(ComponentId::new(4)); cloned.add_archetypal(ComponentId::new(8)); cloned.write_all(); @@ -1534,8 +1411,8 @@ mod tests { let original = create_sample_filtered_access(); let mut cloned = FilteredAccess::default(); - cloned.add_component_write(ComponentId::new(7)); - cloned.add_component_read(ComponentId::new(4)); + cloned.add_write(ComponentId::new(7)); + cloned.add_read(ComponentId::new(4)); cloned.append_or(&FilteredAccess::default()); cloned.clone_from(&original); @@ -1577,8 +1454,8 @@ mod tests { let original = create_sample_filtered_access_set(); let mut cloned = FilteredAccessSet::default(); - cloned.add_unfiltered_resource_read(ComponentId::new(7)); - cloned.add_unfiltered_resource_write(ComponentId::new(9)); + cloned.add_unfiltered_component_read(ComponentId::new(7)); + cloned.add_unfiltered_component_write(ComponentId::new(9)); cloned.write_all(); cloned.clone_from(&original); @@ -1590,7 +1467,7 @@ mod tests { fn read_all_access_conflicts() { // read_all / single write let mut access_a = Access::default(); - access_a.add_component_write(ComponentId::new(0)); + access_a.add_write(ComponentId::new(0)); let mut access_b = Access::default(); access_b.read_all(); @@ -1610,12 +1487,12 @@ mod tests { #[test] fn access_get_conflicts() { let mut access_a = Access::default(); - access_a.add_component_read(ComponentId::new(0)); - access_a.add_component_read(ComponentId::new(1)); + access_a.add_read(ComponentId::new(0)); + access_a.add_read(ComponentId::new(1)); let mut access_b = Access::default(); - access_b.add_component_read(ComponentId::new(0)); - access_b.add_component_write(ComponentId::new(1)); + access_b.add_read(ComponentId::new(0)); + access_b.add_write(ComponentId::new(1)); assert_eq!( access_a.get_conflicts(&access_b), @@ -1623,8 +1500,8 @@ mod tests { ); let mut access_c = Access::default(); - access_c.add_component_write(ComponentId::new(0)); - access_c.add_component_write(ComponentId::new(1)); + access_c.add_write(ComponentId::new(0)); + access_c.add_write(ComponentId::new(1)); assert_eq!( access_a.get_conflicts(&access_c), @@ -1636,7 +1513,7 @@ mod tests { ); let mut access_d = Access::default(); - access_d.add_component_read(ComponentId::new(0)); + access_d.add_read(ComponentId::new(0)); assert_eq!(access_d.get_conflicts(&access_a), AccessConflicts::empty()); assert_eq!(access_d.get_conflicts(&access_b), AccessConflicts::empty()); @@ -1649,10 +1526,10 @@ mod tests { #[test] fn filtered_combined_access() { let mut access_a = FilteredAccessSet::default(); - access_a.add_unfiltered_resource_read(ComponentId::new(1)); + access_a.add_unfiltered_component_read(ComponentId::new(1)); let mut filter_b = FilteredAccess::default(); - filter_b.add_resource_write(ComponentId::new(1)); + filter_b.add_write(ComponentId::new(1)); let conflicts = access_a.get_conflicts_single(&filter_b); assert_eq!( @@ -1665,22 +1542,22 @@ mod tests { #[test] fn filtered_access_extend() { let mut access_a = FilteredAccess::default(); - access_a.add_component_read(ComponentId::new(0)); - access_a.add_component_read(ComponentId::new(1)); + access_a.add_read(ComponentId::new(0)); + access_a.add_read(ComponentId::new(1)); access_a.and_with(ComponentId::new(2)); let mut access_b = FilteredAccess::default(); - access_b.add_component_read(ComponentId::new(0)); - access_b.add_component_write(ComponentId::new(3)); + access_b.add_read(ComponentId::new(0)); + access_b.add_write(ComponentId::new(3)); access_b.and_without(ComponentId::new(4)); access_a.extend(&access_b); let mut expected = FilteredAccess::default(); - expected.add_component_read(ComponentId::new(0)); - expected.add_component_read(ComponentId::new(1)); + expected.add_read(ComponentId::new(0)); + expected.add_read(ComponentId::new(1)); expected.and_with(ComponentId::new(2)); - expected.add_component_write(ComponentId::new(3)); + expected.add_write(ComponentId::new(3)); expected.and_without(ComponentId::new(4)); assert!(access_a.eq(&expected)); @@ -1690,8 +1567,8 @@ mod tests { fn filtered_access_extend_or() { let mut access_a = FilteredAccess::default(); // Exclusive access to `(&mut A, &mut B)`. - access_a.add_component_write(ComponentId::new(0)); - access_a.add_component_write(ComponentId::new(1)); + access_a.add_write(ComponentId::new(0)); + access_a.add_write(ComponentId::new(1)); // Filter by `With`. let mut access_b = FilteredAccess::default(); @@ -1712,8 +1589,8 @@ mod tests { // The intention here is to test that exclusive access implied by `add_write` // forms correct normalized access structs when extended with `Or` filters. let mut expected = FilteredAccess::default(); - expected.add_component_write(ComponentId::new(0)); - expected.add_component_write(ComponentId::new(1)); + expected.add_write(ComponentId::new(0)); + expected.add_write(ComponentId::new(1)); // The resulted access is expected to represent `Or<((With, With, With), (With, With, With, Without))>`. expected.filter_sets = vec![ AccessFilters { @@ -1733,14 +1610,12 @@ mod tests { fn try_iter_component_access_simple() { let mut access = Access::default(); - access.add_component_read(ComponentId::new(1)); - access.add_component_read(ComponentId::new(2)); - access.add_component_write(ComponentId::new(3)); + access.add_read(ComponentId::new(1)); + access.add_read(ComponentId::new(2)); + access.add_write(ComponentId::new(3)); access.add_archetypal(ComponentId::new(5)); - let result = access - .try_iter_component_access() - .map(Iterator::collect::>); + let result = access.try_iter_access().map(Iterator::collect::>); assert_eq!( result, @@ -1757,13 +1632,11 @@ mod tests { fn try_iter_component_access_unbounded_write_all() { let mut access = Access::default(); - access.add_component_read(ComponentId::new(1)); - access.add_component_read(ComponentId::new(2)); + access.add_read(ComponentId::new(1)); + access.add_read(ComponentId::new(2)); access.write_all(); - let result = access - .try_iter_component_access() - .map(Iterator::collect::>); + let result = access.try_iter_access().map(Iterator::collect::>); assert_eq!( result, @@ -1778,13 +1651,11 @@ mod tests { fn try_iter_component_access_unbounded_read_all() { let mut access = Access::default(); - access.add_component_read(ComponentId::new(1)); - access.add_component_read(ComponentId::new(2)); + access.add_read(ComponentId::new(1)); + access.add_read(ComponentId::new(2)); access.read_all(); - let result = access - .try_iter_component_access() - .map(Iterator::collect::>); + let result = access.try_iter_access().map(Iterator::collect::>); assert_eq!( result, diff --git a/crates/bevy_ecs/src/query/access_iter.rs b/crates/bevy_ecs/src/query/access_iter.rs index a671651db3..0e8979e648 100644 --- a/crates/bevy_ecs/src/query/access_iter.rs +++ b/crates/bevy_ecs/src/query/access_iter.rs @@ -72,14 +72,8 @@ fn has_conflicts_large<'a, Q: QueryData>( } EcsAccessType::Component(EcsAccessLevel::ReadAll) | EcsAccessType::Component(EcsAccessLevel::WriteAll) => true, - EcsAccessType::Resource(ResourceAccessLevel::Read(resource_id)) - | EcsAccessType::Resource(ResourceAccessLevel::Write(resource_id)) => { - filter.check_insert(&resource_id.index()) - } EcsAccessType::Access(access) => { - if access.has_read_all_resources() || access.has_write_all_resources() { - true - } else if let Ok(component_iter) = access.try_iter_component_access() { + if let Ok(component_iter) = access.try_iter_access() { let mut needs_check = false; for kind in component_iter { let index = match kind { @@ -91,11 +85,6 @@ fn has_conflicts_large<'a, Q: QueryData>( needs_check = true; } } - for resource_id in access.resource_reads_and_writes() { - if filter.check_insert(&resource_id.index()) { - needs_check = true; - } - } needs_check } else { true @@ -123,8 +112,6 @@ fn has_conflicts_large<'a, Q: QueryData>( pub enum EcsAccessType<'a> { /// Accesses [`Component`](crate::prelude::Component) data Component(EcsAccessLevel), - /// Accesses [`Resource`](crate::prelude::Resource) data - Resource(ResourceAccessLevel), /// borrowed access from [`WorldQuery::State`](crate::query::WorldQuery) Access(&'a Access), /// Does not access any data that can conflict. @@ -146,32 +133,19 @@ impl<'a> EcsAccessType<'a> { (Empty, _) | (_, Empty) - | (Component(_), Resource(_)) - | (Resource(_), Component(_)) // read only access doesn't conflict | (Component(Read(_)), Component(Read(_))) | (Component(ReadAll), Component(Read(_))) | (Component(Read(_)), Component(ReadAll)) | (Component(ReadAll), Component(ReadAll)) - | (Resource(ResourceAccessLevel::Read(_)), Resource(ResourceAccessLevel::Read(_))) => { + => { Ok(()) } (Component(Read(id)), Component(Write(id_other))) | (Component(Write(id)), Component(Read(id_other))) | (Component(Write(id)), Component(Write(id_other))) - | ( - Resource(ResourceAccessLevel::Read(id)), - Resource(ResourceAccessLevel::Write(id_other)), - ) - | ( - Resource(ResourceAccessLevel::Write(id)), - Resource(ResourceAccessLevel::Read(id_other)), - ) - | ( - Resource(ResourceAccessLevel::Write(id)), - Resource(ResourceAccessLevel::Write(id_other)), - ) => if id == id_other { + => if id == id_other { Err(AccessConflictError(*self, other)) } else { Ok(()) @@ -179,41 +153,28 @@ impl<'a> EcsAccessType<'a> { // Borrowed Access (Component(Read(component_id)), Access(access)) - | (Access(access), Component(Read(component_id))) => if access.has_component_write(component_id) { + | (Access(access), Component(Read(component_id))) => if access.has_write(component_id) { Err(AccessConflictError(*self, other)) } else { Ok(()) }, (Component(Write(component_id)), Access(access)) - | (Access(access), Component(Write(component_id))) => if access.has_component_read(component_id) { + | (Access(access), Component(Write(component_id))) => if access.has_read(component_id) { Err(AccessConflictError(*self, other)) } else { Ok(()) }, (Component(ReadAll), Access(access)) - | (Access(access), Component(ReadAll)) => if access.has_any_component_write() { + | (Access(access), Component(ReadAll)) => if access.has_any_write() { Err(AccessConflictError(*self, other)) } else { Ok(()) }, (Component(WriteAll), Access(access)) - | (Access(access), Component(WriteAll))=> if access.has_any_component_read() { - Err(AccessConflictError(*self, other)) - } else { - Ok(()) - }, - - (Resource(ResourceAccessLevel::Read(component_id)), Access(access)) - | (Access(access), Resource(ResourceAccessLevel::Read(component_id))) => if access.has_resource_write(component_id) { - Err(AccessConflictError(*self, other)) - } else { - Ok(()) - }, - (Resource(ResourceAccessLevel::Write(component_id)), Access(access)) - | (Access(access), Resource(ResourceAccessLevel::Write(component_id))) => if access.has_resource_read(component_id) { + | (Access(access), Component(WriteAll))=> if access.has_any_read() { Err(AccessConflictError(*self, other)) } else { Ok(()) @@ -242,15 +203,6 @@ pub enum EcsAccessLevel { WriteAll, } -/// Access level needed by [`QueryData`] fetch to the resource. -#[derive(Copy, Clone, Debug, PartialEq, Hash)] -pub enum ResourceAccessLevel { - /// Reads the resource with [`ComponentId`] - Read(ComponentId), - /// Writes the resource with [`ComponentId`] - Write(ComponentId), -} - /// Error returned from [`EcsAccessType::is_compatible`] pub struct AccessConflictError<'a>(EcsAccessType<'a>, EcsAccessType<'a>); @@ -319,16 +271,6 @@ impl Display for AccessConflictError<'_> { f, "Access has a read that conflicts with component write all" ), - (Access(_), Resource(ResourceAccessLevel::Read(id))) - | (Resource(ResourceAccessLevel::Read(id)), Access(_)) => write!( - f, - "Access has a write that conflicts with resource {id:?} read." - ), - (Access(_), Resource(ResourceAccessLevel::Write(id))) - | (Resource(ResourceAccessLevel::Write(id)), Access(_)) => write!( - f, - "Access has a read that conflicts with resource {id:?} write." - ), (Access(_), Access(_)) => write!(f, "Access conflicts with other Access"), _ => { @@ -355,7 +297,7 @@ impl Display for QueryAccessError { QueryAccessError::ComponentNotRegistered => { write!( f, - "At least one component in Q was not registered in world. + "At least one component in Q was not registered in world. Consider calling `World::register_component`" ) } diff --git a/crates/bevy_ecs/src/query/builder.rs b/crates/bevy_ecs/src/query/builder.rs index 2c4b29e06e..6b2fd6f5dd 100644 --- a/crates/bevy_ecs/src/query/builder.rs +++ b/crates/bevy_ecs/src/query/builder.rs @@ -170,14 +170,14 @@ impl<'w, D: QueryData, F: QueryFilter> QueryBuilder<'w, D, F> { /// Adds `&T` to the [`FilteredAccess`] of self. pub fn ref_id(&mut self, id: ComponentId) -> &mut Self { self.with_id(id); - self.access.add_component_read(id); + self.access.add_read(id); self } /// Adds `&mut T` to the [`FilteredAccess`] of self. pub fn mut_id(&mut self, id: ComponentId) -> &mut Self { self.with_id(id); - self.access.add_component_write(id); + self.access.add_write(id); self } diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 95f0234072..8d0aea7ffd 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -955,10 +955,10 @@ unsafe impl<'a> WorldQuery for EntityRef<'a> { fn update_component_access(_state: &Self::State, access: &mut FilteredAccess) { assert!( - !access.access().has_any_component_write(), + !access.access().has_any_write(), "EntityRef conflicts with a previous access in this query. Shared access cannot coincide with exclusive access.", ); - access.read_all_components(); + access.read_all(); } fn init_state(_world: &mut World) {} @@ -1071,10 +1071,10 @@ unsafe impl<'a> WorldQuery for EntityMut<'a> { fn update_component_access(_state: &Self::State, access: &mut FilteredAccess) { assert!( - !access.access().has_any_component_read(), + !access.access().has_any_read(), "EntityMut conflicts with a previous access in this query. Exclusive access cannot coincide with any other accesses.", ); - access.write_all_components(); + access.write_all(); } fn init_state(_world: &mut World) {} @@ -1449,16 +1449,16 @@ where fn init_state(world: &mut World) -> Self::State { let mut access = Access::new(); - access.read_all_components(); + access.read_all(); for id in B::component_ids(&mut world.components_registrator()) { - access.remove_component_read(id); + access.remove_read(id); } access } fn get_state(components: &Components) -> Option { let mut access = Access::new(); - access.read_all_components(); + access.read_all(); // 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. @@ -1466,7 +1466,7 @@ where // Since the component isn't registered, there are no entities with that // component, and the extra access will usually have no effect. for id in B::get_component_ids(components).flatten() { - access.remove_component_read(id); + access.remove_read(id); } Some(access) } @@ -1572,16 +1572,16 @@ where fn init_state(world: &mut World) -> Self::State { let mut access = Access::new(); - access.write_all_components(); + access.write_all(); for id in B::component_ids(&mut world.components_registrator()) { - access.remove_component_read(id); + access.remove_read(id); } access } fn get_state(components: &Components) -> Option { let mut access = Access::new(); - access.write_all_components(); + access.write_all(); // 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. @@ -1589,7 +1589,7 @@ where // Since the component isn't registered, there are no entities with that // component, and the extra access will usually have no effect. for id in B::get_component_ids(components).flatten() { - access.remove_component_read(id); + access.remove_read(id); } Some(access) } @@ -1841,11 +1841,11 @@ unsafe impl WorldQuery for &T { fn update_component_access(&component_id: &ComponentId, access: &mut FilteredAccess) { assert!( - !access.access().has_component_write(component_id), + !access.access().has_write(component_id), "&{} conflicts with a previous access in this query. Shared access cannot coincide with exclusive access.", DebugName::type_name::(), ); - access.add_component_read(component_id); + access.add_read(component_id); } fn init_state(world: &mut World) -> ComponentId { @@ -2065,11 +2065,11 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> { fn update_component_access(&component_id: &ComponentId, access: &mut FilteredAccess) { assert!( - !access.access().has_component_write(component_id), + !access.access().has_write(component_id), "&{} conflicts with a previous access in this query. Shared access cannot coincide with exclusive access.", DebugName::type_name::(), ); - access.add_component_read(component_id); + access.add_read(component_id); } fn init_state(world: &mut World) -> ComponentId { @@ -2332,11 +2332,11 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { fn update_component_access(&component_id: &ComponentId, access: &mut FilteredAccess) { assert!( - !access.access().has_component_read(component_id), + !access.access().has_read(component_id), "&mut {} conflicts with a previous access in this query. Mutable component access must be unique.", DebugName::type_name::(), ); - access.add_component_write(component_id); + access.add_write(component_id); } fn init_state(world: &mut World) -> ComponentId { @@ -2539,11 +2539,11 @@ unsafe impl<'__w, T: Component> WorldQuery for Mut<'__w, T> { // Update component access here instead of in `<&mut T as WorldQuery>` to avoid erroneously referencing // `&mut T` in error message. assert!( - !access.access().has_component_read(component_id), + !access.access().has_read(component_id), "Mut<{}> conflicts with a previous access in this query. Mutable component access mut be unique.", DebugName::type_name::(), ); - access.add_component_write(component_id); + access.add_write(component_id); } // Forwarded to `&mut T` diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index 9d6d1e1511..84cc7b2edb 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -826,10 +826,10 @@ unsafe impl WorldQuery for Added { #[inline] fn update_component_access(&id: &ComponentId, access: &mut FilteredAccess) { - if access.access().has_component_write(id) { + if access.access().has_write(id) { panic!("$state_name<{}> conflicts with a previous access in this query. Shared access cannot coincide with exclusive access.", DebugName::type_name::()); } - access.add_component_read(id); + access.add_read(id); } fn init_state(world: &mut World) -> ComponentId { @@ -1053,10 +1053,10 @@ unsafe impl WorldQuery for Changed { #[inline] fn update_component_access(&id: &ComponentId, access: &mut FilteredAccess) { - if access.access().has_component_write(id) { + if access.access().has_write(id) { panic!("$state_name<{}> conflicts with a previous access in this query. Shared access cannot coincide with exclusive access.", DebugName::type_name::()); } - access.add_component_read(id); + access.add_read(id); } fn init_state(world: &mut World) -> ComponentId { diff --git a/crates/bevy_ecs/src/query/mod.rs b/crates/bevy_ecs/src/query/mod.rs index 7267e874cc..b7b4223c68 100644 --- a/crates/bevy_ecs/src/query/mod.rs +++ b/crates/bevy_ecs/src/query/mod.rs @@ -113,18 +113,15 @@ impl DebugCheckedUnwrap for Option { #[expect(clippy::print_stdout, reason = "Allowed in tests.")] mod tests { use crate::{ - archetype::Archetype, - change_detection::Tick, - component::{Component, ComponentId, Components}, - prelude::{AnyOf, Changed, Entity, Or, QueryState, Resource, With, Without}, + component::Component, + prelude::{AnyOf, Changed, Entity, Or, QueryState, With, Without}, query::{ - ArchetypeFilter, ArchetypeQueryData, FilteredAccess, Has, IterQueryData, - QueryCombinationIter, QueryData, QueryFilter, ReadOnlyQueryData, WorldQuery, + ArchetypeFilter, ArchetypeQueryData, Has, QueryCombinationIter, QueryData, QueryFilter, + ReadOnlyQueryData, }, schedule::{IntoScheduleConfigs, Schedule}, - storage::{Table, TableRow}, - system::{assert_is_system, IntoSystem, Query, System, SystemState}, - world::{unsafe_world_cell::UnsafeWorldCell, World}, + system::{IntoSystem, Query, System, SystemState}, + world::World, }; use alloc::{vec, vec::Vec}; use core::{any::type_name, fmt::Debug, hash::Hash}; @@ -817,114 +814,4 @@ mod tests { let values = world.query::<&B>().iter(&world).collect::>(); assert_eq!(values, vec![&B(2)]); } - - #[derive(Resource)] - struct R; - - /// `QueryData` that performs read access on R to test that resource access is tracked - struct ReadsRData; - - // SAFETY: - // `update_component_access` adds resource read access for `R`. - unsafe impl WorldQuery for ReadsRData { - type Fetch<'w> = (); - type State = ComponentId; - - fn shrink_fetch<'wlong: 'wshort, 'wshort>(_: Self::Fetch<'wlong>) -> Self::Fetch<'wshort> {} - - unsafe fn init_fetch<'w, 's>( - _world: UnsafeWorldCell<'w>, - _state: &'s Self::State, - _last_run: Tick, - _this_run: Tick, - ) -> Self::Fetch<'w> { - } - - const IS_DENSE: bool = true; - - #[inline] - unsafe fn set_archetype<'w, 's>( - _fetch: &mut Self::Fetch<'w>, - _state: &'s Self::State, - _archetype: &'w Archetype, - _table: &Table, - ) { - } - - #[inline] - unsafe fn set_table<'w, 's>( - _fetch: &mut Self::Fetch<'w>, - _state: &'s Self::State, - _table: &'w Table, - ) { - } - - fn update_component_access(&component_id: &Self::State, access: &mut FilteredAccess) { - assert!( - !access.access().has_resource_write(component_id), - "ReadsRData conflicts with a previous access in this query. Shared access cannot coincide with exclusive access." - ); - access.add_resource_read(component_id); - } - - fn init_state(world: &mut World) -> Self::State { - world.components_registrator().register_component::() - } - - fn get_state(components: &Components) -> Option { - components.component_id::() - } - - fn matches_component_set( - _state: &Self::State, - _set_contains_id: &impl Fn(ComponentId) -> bool, - ) -> bool { - true - } - } - - // SAFETY: `Self` is the same as `Self::ReadOnly` - unsafe impl QueryData for ReadsRData { - const IS_READ_ONLY: bool = true; - const IS_ARCHETYPAL: bool = true; - type ReadOnly = Self; - type Item<'w, 's> = (); - - fn shrink<'wlong: 'wshort, 'wshort, 's>( - _item: Self::Item<'wlong, 's>, - ) -> Self::Item<'wshort, 's> { - } - - #[inline(always)] - unsafe fn fetch<'w, 's>( - _state: &'s Self::State, - _fetch: &mut Self::Fetch<'w>, - _entity: Entity, - _table_row: TableRow, - ) -> Option> { - Some(()) - } - - fn iter_access( - state: &Self::State, - ) -> impl Iterator> { - core::iter::once(super::access_iter::EcsAccessType::Resource( - super::access_iter::ResourceAccessLevel::Read(*state), - )) - } - } - - // SAFETY: access is read only - unsafe impl ReadOnlyQueryData for ReadsRData {} - - /// SAFETY: access is read only - unsafe impl IterQueryData for ReadsRData {} - - impl ArchetypeQueryData for ReadsRData {} - - #[test] - fn read_res_read_res_no_conflict() { - fn system(_q1: Query>, _q2: Query>) {} - assert_is_system(system); - } } diff --git a/crates/bevy_ecs/src/schedule/mod.rs b/crates/bevy_ecs/src/schedule/mod.rs index 11ee228d11..70a261f47b 100644 --- a/crates/bevy_ecs/src/schedule/mod.rs +++ b/crates/bevy_ecs/src/schedule/mod.rs @@ -1012,8 +1012,8 @@ mod tests { let _ = schedule.initialize(&mut world); - // this should fail, since resources are components - assert_eq!(schedule.graph().conflicting_systems().len(), 1); + // this should fail, since resources are components and non_sends also do access with components + assert_eq!(schedule.graph().conflicting_systems().len(), 2); schedule = Schedule::default(); schedule.add_systems(( diff --git a/crates/bevy_ecs/src/system/combinator.rs b/crates/bevy_ecs/src/system/combinator.rs index bbf38cbcc4..73dbb33d83 100644 --- a/crates/bevy_ecs/src/system/combinator.rs +++ b/crates/bevy_ecs/src/system/combinator.rs @@ -252,7 +252,7 @@ where // We might need to read the default error handler after the component // systems have run to report failures. let error_resource = world.register_resource::(); - a_access.add_unfiltered_resource_read(error_resource); + a_access.add_resource_read(error_resource); a_access } diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 3663087815..976ee8afff 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -749,30 +749,24 @@ unsafe impl<'a, T: Resource> SystemParam for Res<'a, T> { &component_id: &Self::State, system_meta: &mut SystemMeta, component_access_set: &mut FilteredAccessSet, - _world: &mut World, + world: &mut World, ) { - let combined_access = component_access_set.combined_access(); - assert!( - !combined_access.has_resource_write(component_id), - "error[B0002]: Res<{}> in system {} conflicts with a previous ResMut<{0}> access. Consider removing the duplicate access. See: https://bevy.org/learn/errors/b0002", - DebugName::type_name::(), - system_meta.name, - ); - let mut filter = FilteredAccess::default(); - filter.add_component_read(component_id); - filter.add_resource_read(component_id); + filter.add_read(component_id); filter.and_with(IS_RESOURCE); - assert!(component_access_set - .get_conflicts_single(&filter) - .is_empty(), - "error[B0002]: Res<{}> in system {} conflicts with a previous query. Consider removing the duplicate access. See: https://bevy.org/learn/errors/b0002", - DebugName::type_name::(), - system_meta.name - ); + let conflicts = component_access_set.get_conflicts_single(&filter); + if conflicts.is_empty() { + component_access_set.add(filter); + return; + } - component_access_set.add(filter); + let mut accesses = conflicts.format_conflict_list(world.as_unsafe_world_cell()); + // Access list may be empty (if access to all components requested) + if !accesses.is_empty() { + accesses.push(' '); + } + panic!("error[B0002]: Res<{}> in system {} conflicts with a previous system parameter. Consider removing the duplicate access using `Without` to create disjoint Queries or merging conflicting Queries into a `ParamSet`. See: https://bevy.org/learn/errors/b0002", DebugName::type_name::(), system_meta.name); } #[inline] @@ -837,33 +831,24 @@ unsafe impl<'a, T: Resource> SystemParam for ResMut<'a, T> { &component_id: &Self::State, system_meta: &mut SystemMeta, component_access_set: &mut FilteredAccessSet, - _world: &mut World, + world: &mut World, ) { - let combined_access = component_access_set.combined_access(); - if combined_access.has_resource_write(component_id) { - panic!( - "error[B0002]: ResMut<{}> in system {} conflicts with a previous ResMut<{0}> access. Consider removing the duplicate access. See: https://bevy.org/learn/errors/b0002", - DebugName::type_name::(), system_meta.name); - } else if combined_access.has_resource_read(component_id) { - panic!( - "error[B0002]: ResMut<{}> in system {} conflicts with a previous Res<{0}> access. Consider removing the duplicate access. See: https://bevy.org/learn/errors/b0002", - DebugName::type_name::(), system_meta.name); - } - let mut filter = FilteredAccess::default(); - filter.add_component_write(component_id); - filter.add_resource_write(component_id); + filter.add_write(component_id); filter.and_with(IS_RESOURCE); - assert!(component_access_set - .get_conflicts_single(&filter) - .is_empty(), - "error[B0002]: ResMut<{}> in system {} conflicts with a previous query. Consider removing the duplicate access. See: https://bevy.org/learn/errors/b0002", - DebugName::type_name::(), - system_meta.name - ); + let conflicts = component_access_set.get_conflicts_single(&filter); + if conflicts.is_empty() { + component_access_set.add(filter); + return; + } - component_access_set.add(filter); + let mut accesses = conflicts.format_conflict_list(world.as_unsafe_world_cell()); + // Access list may be empty (if access to all components requested) + if !accesses.is_empty() { + accesses.push(' '); + } + panic!("error[B0002]: ResMut<{}> in system {} conflicts with a previous system parameter. Consider removing the duplicate access or using `Without` to create disjoint Queries or merging conflicting Queries into a `ParamSet`. See: https://bevy.org/learn/errors/b0002", DebugName::type_name::(), system_meta.name); } #[inline] @@ -1471,12 +1456,12 @@ unsafe impl<'a, T: 'static> SystemParam for NonSend<'a, T> { let combined_access = component_access_set.combined_access(); assert!( - !combined_access.has_resource_write(component_id), + !combined_access.has_write(component_id), "error[B0002]: NonSend<{}> in system {} conflicts with a previous mutable resource access ({0}). Consider removing the duplicate access. See: https://bevy.org/learn/errors/b0002", DebugName::type_name::(), system_meta.name, ); - component_access_set.add_unfiltered_resource_read(component_id); + component_access_set.add_unfiltered_component_read(component_id); } #[inline] @@ -1541,16 +1526,16 @@ unsafe impl<'a, T: 'static> SystemParam for NonSendMut<'a, T> { system_meta.set_non_send(); let combined_access = component_access_set.combined_access(); - if combined_access.has_resource_write(component_id) { + if combined_access.has_write(component_id) { panic!( "error[B0002]: NonSendMut<{}> in system {} conflicts with a previous mutable resource access ({0}). Consider removing the duplicate access. See: https://bevy.org/learn/errors/b0002", DebugName::type_name::(), system_meta.name); - } else if combined_access.has_resource_read(component_id) { + } else if combined_access.has_read(component_id) { panic!( "error[B0002]: NonSendMut<{}> in system {} conflicts with a previous immutable resource access ({0}). Consider removing the duplicate access. See: https://bevy.org/learn/errors/b0002", DebugName::type_name::(), system_meta.name); } - component_access_set.add_unfiltered_resource_write(component_id); + component_access_set.add_unfiltered_component_write(component_id); } #[inline] @@ -2773,13 +2758,10 @@ unsafe impl SystemParam for FilteredResources<'_, '_> { panic!("error[B0002]: FilteredResources in system {system_name} accesses resources(s){accesses} in a way that conflicts with a previous system parameter. Consider removing the duplicate access. See: https://bevy.org/learn/errors/b0002"); } - if access.has_read_all_resources() { - component_access_set.add_unfiltered_read_all_resources(); - } else { - for component_id in access.resource_reads_and_writes() { - component_access_set.add_unfiltered_resource_read(component_id); - } - } + let mut filter = FilteredAccess::matches_everything(); + filter.access_mut().extend(access); + filter.and_with(IS_RESOURCE); + component_access_set.add(filter); } unsafe fn get_param<'world, 'state>( @@ -2822,21 +2804,10 @@ unsafe impl SystemParam for FilteredResourcesMut<'_, '_> { panic!("error[B0002]: FilteredResourcesMut in system {system_name} accesses resources(s){accesses} in a way that conflicts with a previous system parameter. Consider removing the duplicate access. See: https://bevy.org/learn/errors/b0002"); } - if access.has_read_all_resources() { - component_access_set.add_unfiltered_read_all_resources(); - } else { - for component_id in access.resource_reads() { - component_access_set.add_unfiltered_resource_read(component_id); - } - } - - if access.has_write_all_resources() { - component_access_set.add_unfiltered_write_all_resources(); - } else { - for component_id in access.resource_writes() { - component_access_set.add_unfiltered_resource_write(component_id); - } - } + let mut filter = FilteredAccess::matches_everything(); + filter.access_mut().extend(access); + filter.and_with(IS_RESOURCE); + component_access_set.add(filter); } unsafe fn get_param<'world, 'state>( @@ -2941,7 +2912,10 @@ impl Display for SystemParamValidationError { #[cfg(test)] mod tests { use super::*; + use crate::query::Without; + use crate::resource::IsResource; use crate::system::assert_is_system; + use crate::world::EntityMut; use core::cell::RefCell; #[test] @@ -2960,6 +2934,44 @@ mod tests { schedule.run(&mut world); } + #[test] + #[should_panic] + fn non_send_and_entities() { + #[derive(Resource)] + struct A(usize); + fn my_system(mut ns: NonSendMut, _: Query) { + ns.0 += 1; + } + assert_is_system(my_system); + } + + #[test] + #[should_panic] + fn res_and_entities() { + #[derive(Resource)] + struct A(usize); + fn my_system(mut res: ResMut, _: Query) { + res.0 += 1; + } + assert_is_system(my_system); + } + + #[test] + fn res_and_entities_filtered() { + #[derive(Resource)] + struct A(usize); + fn res_system(mut res: ResMut, _: Query>) { + res.0 += 1; + } + assert_is_system(res_system); + + fn non_send_system(mut ns: NonSendMut, _: Query>) { + ns.0 += 1; + } + + assert_is_system(non_send_system); + } + // Compile test for https://github.com/bevyengine/bevy/pull/2838. #[test] fn system_param_generic_bounds() { diff --git a/crates/bevy_ecs/src/world/entity_access/filtered.rs b/crates/bevy_ecs/src/world/entity_access/filtered.rs index de366205f9..d722f27e3e 100644 --- a/crates/bevy_ecs/src/world/entity_access/filtered.rs +++ b/crates/bevy_ecs/src/world/entity_access/filtered.rs @@ -144,7 +144,7 @@ impl<'w, 's> FilteredEntityRef<'w, 's> { .components() .get_valid_id(TypeId::of::())?; self.access - .has_component_read(id) + .has_read(id) // SAFETY: We have read access .then(|| unsafe { self.entity.get() }) .flatten() @@ -162,7 +162,7 @@ impl<'w, 's> FilteredEntityRef<'w, 's> { .components() .get_valid_id(TypeId::of::())?; self.access - .has_component_read(id) + .has_read(id) // SAFETY: We have read access .then(|| unsafe { self.entity.get_ref() }) .flatten() @@ -178,7 +178,7 @@ impl<'w, 's> FilteredEntityRef<'w, 's> { .components() .get_valid_id(TypeId::of::())?; self.access - .has_component_read(id) + .has_read(id) // SAFETY: We have read access .then(|| unsafe { self.entity.get_change_ticks::() }) .flatten() @@ -193,7 +193,7 @@ impl<'w, 's> FilteredEntityRef<'w, 's> { #[inline] pub fn get_change_ticks_by_id(&self, component_id: ComponentId) -> Option { self.access - .has_component_read(component_id) + .has_read(component_id) // SAFETY: We have read access .then(|| unsafe { self.entity.get_change_ticks_by_id(component_id) }) .flatten() @@ -210,7 +210,7 @@ impl<'w, 's> FilteredEntityRef<'w, 's> { #[inline] pub fn get_by_id(&self, component_id: ComponentId) -> Option> { self.access - .has_component_read(component_id) + .has_read(component_id) // SAFETY: We have read access .then(|| unsafe { self.entity.get_by_id(component_id) }) .flatten() @@ -575,7 +575,7 @@ impl<'w, 's> FilteredEntityMut<'w, 's> { .components() .get_valid_id(TypeId::of::())?; self.access - .has_component_write(id) + .has_write(id) // SAFETY: We have permission to access the component mutable // and we promise to not create other references to the same component .then(|| unsafe { self.entity.get_mut() }) @@ -608,7 +608,7 @@ impl<'w, 's> FilteredEntityMut<'w, 's> { .components() .get_valid_id(TypeId::of::())?; self.access - .has_component_write(id) + .has_write(id) // SAFETY: // - We have write access // - Caller ensures `T` is a mutable component @@ -686,7 +686,7 @@ impl<'w, 's> FilteredEntityMut<'w, 's> { component_id: ComponentId, ) -> Option> { self.access - .has_component_write(component_id) + .has_write(component_id) // SAFETY: We have permission to access the component mutable // and we promise to not create other references to the same component .then(|| unsafe { self.entity.get_mut_by_id(component_id).ok() }) diff --git a/crates/bevy_ecs/src/world/filtered_resource.rs b/crates/bevy_ecs/src/world/filtered_resource.rs index 241df00127..7b288fbe8a 100644 --- a/crates/bevy_ecs/src/world/filtered_resource.rs +++ b/crates/bevy_ecs/src/world/filtered_resource.rs @@ -149,7 +149,7 @@ impl<'w, 's> FilteredResources<'w, 's> { /// Note that [`Self::get()`] may still return `Err` if the resource does not exist. pub fn has_read(&self) -> bool { let component_id = self.world.components().component_id::(); - component_id.is_some_and(|component_id| self.access.has_resource_read(component_id)) + component_id.is_some_and(|component_id| self.access.has_read(component_id)) } /// Gets a reference to the resource of the given type if it exists and the `FilteredResources` has access to it. @@ -159,7 +159,7 @@ impl<'w, 's> FilteredResources<'w, 's> { .components() .valid_component_id::() .ok_or(ResourceFetchError::NotRegistered)?; - if !self.access.has_resource_read(component_id) { + if !self.access.has_read(component_id) { return Err(ResourceFetchError::NoResourceAccess(component_id)); } @@ -179,7 +179,7 @@ impl<'w, 's> FilteredResources<'w, 's> { /// Gets a pointer to the resource with the given [`ComponentId`] if it exists and the `FilteredResources` has access to it. pub fn get_by_id(&self, component_id: ComponentId) -> Result, ResourceFetchError> { - if !self.access.has_resource_read(component_id) { + if !self.access.has_read(component_id) { return Err(ResourceFetchError::NoResourceAccess(component_id)); } // SAFETY: We have read access to this resource @@ -220,14 +220,7 @@ impl<'w, 's> From<&'w FilteredResourcesMut<'_, 's>> for FilteredResources<'w, 's impl<'w> From<&'w World> for FilteredResources<'w, 'static> { fn from(value: &'w World) -> Self { - const READ_ALL_RESOURCES: &Access = { - const ACCESS: Access = { - let mut access = Access::new(); - access.read_all_resources(); - access - }; - &ACCESS - }; + const READ_ALL_RESOURCES: &Access = const { &Access::new_read_all() }; let last_run = value.last_change_tick(); let this_run = value.read_change_tick(); @@ -417,14 +410,14 @@ impl<'w, 's> FilteredResourcesMut<'w, 's> { /// Note that [`Self::get()`] may still return `Err` if the resource does not exist. pub fn has_read(&self) -> bool { let component_id = self.world.components().component_id::(); - component_id.is_some_and(|component_id| self.access.has_resource_read(component_id)) + component_id.is_some_and(|component_id| self.access.has_read(component_id)) } /// Returns `true` if the `FilteredResources` has write access to the given resource. /// Note that [`Self::get_mut()`] may still return `Err` if the resource does not exist. pub fn has_write(&self) -> bool { let component_id = self.world.components().component_id::(); - component_id.is_some_and(|component_id| self.access.has_resource_write(component_id)) + component_id.is_some_and(|component_id| self.access.has_write(component_id)) } /// Gets a reference to the resource of the given type if it exists and the `FilteredResources` has access to it. @@ -489,7 +482,7 @@ impl<'w, 's> FilteredResourcesMut<'w, 's> { &mut self, component_id: ComponentId, ) -> Result, ResourceFetchError> { - if !self.access.has_resource_write(component_id) { + if !self.access.has_write(component_id) { return Err(ResourceFetchError::NoResourceAccess(component_id)); } @@ -510,14 +503,7 @@ impl<'w, 's> FilteredResourcesMut<'w, 's> { impl<'w> From<&'w mut World> for FilteredResourcesMut<'w, 'static> { fn from(value: &'w mut World) -> Self { - const WRITE_ALL_RESOURCES: &Access = { - const ACCESS: Access = { - let mut access = Access::new(); - access.write_all_resources(); - access - }; - &ACCESS - }; + const WRITE_ALL_RESOURCES: &Access = const { &Access::new_write_all() }; let last_run = value.last_change_tick(); let this_run = value.change_tick(); @@ -557,10 +543,7 @@ impl<'w> FilteredResourcesBuilder<'w> { /// Add accesses required to read all resources. pub fn add_read_all(&mut self) -> &mut Self { - self.access.read_all_resources(); - for &component_id in self.world.resource_entities().indices() { - self.access.add_component_read(component_id); - } + self.access.read_all(); self } @@ -575,8 +558,7 @@ impl<'w> FilteredResourcesBuilder<'w> { /// Add accesses required to read the resource with the given [`ComponentId`]. pub fn add_read_by_id(&mut self, component_id: ComponentId) -> &mut Self { - self.access.add_resource_read(component_id); - self.access.add_component_read(component_id); + self.access.add_read(component_id); self } @@ -610,10 +592,7 @@ impl<'w> FilteredResourcesMutBuilder<'w> { /// Add accesses required to read all resources. pub fn add_read_all(&mut self) -> &mut Self { - self.access.read_all_resources(); - for &component_id in self.world.resource_entities().indices() { - self.access.add_component_read(component_id); - } + self.access.read_all(); self } @@ -628,17 +607,13 @@ impl<'w> FilteredResourcesMutBuilder<'w> { /// Add accesses required to read the resource with the given [`ComponentId`]. pub fn add_read_by_id(&mut self, component_id: ComponentId) -> &mut Self { - self.access.add_resource_read(component_id); - self.access.add_component_read(component_id); + self.access.add_read(component_id); self } /// Add accesses required to get mutable access to all resources. pub fn add_write_all(&mut self) -> &mut Self { - self.access.write_all_resources(); - for &component_id in self.world.resource_entities().indices() { - self.access.add_component_write(component_id); - } + self.access.write_all(); self } @@ -653,8 +628,7 @@ impl<'w> FilteredResourcesMutBuilder<'w> { /// Add accesses required to get mutable access to the resource with the given [`ComponentId`]. pub fn add_write_by_id(&mut self, component_id: ComponentId) -> &mut Self { - self.access.add_resource_write(component_id); - self.access.add_component_write(component_id); + self.access.add_write(component_id); self } diff --git a/crates/bevy_render/src/renderer/render_context.rs b/crates/bevy_render/src/renderer/render_context.rs index 06adad0bd9..5a21ada6f1 100644 --- a/crates/bevy_render/src/renderer/render_context.rs +++ b/crates/bevy_render/src/renderer/render_context.rs @@ -249,7 +249,7 @@ unsafe impl<'a, D: QueryData + 'static, F: QueryFilter + 'static> SystemParam component_access_set: &mut FilteredAccessSet, world: &mut World, ) { - component_access_set.add_unfiltered_resource_read(state.resource_id); + component_access_set.add_resource_read(state.resource_id); as SystemParam>::init_access( &state.query_state, diff --git a/errors/B0001.md b/errors/B0001.md index 16486cea77..7e3b31af4d 100644 --- a/errors/B0001.md +++ b/errors/B0001.md @@ -89,3 +89,7 @@ fn main() { .run(); } ``` + +## Note + +If you run this error while using resources, check out [B0001](https://bevy.org/learn/errors/b0002) for additional information. diff --git a/errors/B0002.md b/errors/B0002.md index 031c28dfba..21a92cbc15 100644 --- a/errors/B0002.md +++ b/errors/B0002.md @@ -42,3 +42,39 @@ fn main() { .run(); } ``` + +## Resources as Components + +Under the hood, resources *are* components. The consequences of this are usually nothing more than an implementation detail, however, you may run into an issue where the following system throws an error: + +```rust,no_run +# use bevy::prelude::*; +# #[derive(Resource)] +# struct MyResource; +fn system(all_entities: Query, res: Res) {} +``` + +It's not possible to both query all entities and a resource when resources are stored on an entity. +To fix this, you should either constrain the `all_entities` query (you usually don't need to query *every* single entity), or exclude `MyResource` with `Without` or `Without`. +Here, `IsResource`, is a marker component on every single entity that holds a resource, so it's very convenient if you want to exclude resources. + +Queries that might conflict with `Res` or `ResMut` include: + +- `Query<()>` +- `Query` +- `Query` +- `Query` +- `Query` +- `Query` +- `Query>` + +You can run into the same problem with non-send data: + +```rust,no_run +# use bevy::prelude::*; +# #[derive(Resource)] +# struct MyNonSend; +fn system(all_entities: Query, some_non_send: NonSend) {} +``` + +This can be fixed by adding a `Without` filter to the query. diff --git a/examples/ecs/dynamic.rs b/examples/ecs/dynamic.rs index e5dd3c40b5..777e7c6047 100644 --- a/examples/ecs/dynamic.rs +++ b/examples/ecs/dynamic.rs @@ -158,7 +158,7 @@ fn main() { query.iter_mut(&mut world).for_each(|filtered_entity| { let terms = filtered_entity .access() - .try_iter_component_access() + .try_iter_access() .unwrap() .map(|component_access| { let id = *component_access.index(); diff --git a/release-content/migration-guides/resources_as_components.md b/release-content/migration-guides/resources_as_components.md index d1427e0792..a7ea6be9d2 100644 --- a/release-content/migration-guides/resources_as_components.md +++ b/release-content/migration-guides/resources_as_components.md @@ -1,6 +1,6 @@ --- title: Resources as Components -pull_requests: [20934] +pull_requests: [20934, 22910, 22911, 22919, 22930] --- ## `#[derive(Resource)]` implements the `Component` trait @@ -34,6 +34,35 @@ The `ReflectResource` is a ZST (zero-sized type) in 0.19 and only functions to s Instead, `#[reflect(Resource)]` also reflects the `Component` trait, so use `ReflectComponent` instead. This is likely to show up in code that uses reflection, like BRP (Bevy Reflect Protocol) and `bevy_scene`. +## Broad Queries and System Conflicts + +Now that resources are components, they can be queried using 'broad' queries. These are queries that query all entities. Examples include: + +- `Query<()>` +- `Query` +- `Query` +- `Query` +- `Query` +- `Query` +- `Query>` + +These should rarely come up in real games, but if they do, they might conflict with resource access, i.e. + +```rust +fn system(entity_query: Query, some_resource: Res) {} // err! entity_query conflicts with some_resource +``` + +To fix this, you can narrow down the query by using either the `Without` or `Without` filter. +The `IsResource` marker is attached to all resource entities, so it always filters them out. + +The same is true for non-send data: + +```rust +fn system(entity_query: Query, some_non_send: NonSend) {} // err! entity_query conflicts with some_resource +``` + +This can be fixed by adding a `Without` filter to the query. + ## Renaming Non-Send Resources to Non-Send Data Previously there were two types of resources: `Send` resources and `!Send` resources. @@ -75,7 +104,43 @@ The registration process for components and resources is very similar and now th - `ComponentsQueuedRegistrator::queue_register_resource` was deprecated in favor of `ComponentsQueuedRegistrator::queue_register_component`. - `ComponentDescriptor::new_resource` was deprecated in favor of `ComponentDescriptor::new` - `ComponentDescriptor::new_resource` was deprecated in favor of `ComponentDescriptor::new` -- `World::register_resource_with_descriptor was renamed to World::register_non_send_with_descriptor`. +- `World::register_resource_with_descriptor` was renamed to `World::register_non_send_with_descriptor`. + +## Access + +Resources were also removed from `Access`, which keeps track what data any given query / system has access to. + +- `Access::add_component_read` and `Access::add_resource_read` were deprecated in favor of `Access::add_read`. +- `Access::add_component_write` and `Access::add_resource_write` were deprecated in favor of `Access::add_write`. +- `Access::remove_component_read` was deprecated in favor of `Access::remove_read`. +- `Access::remove_component_write` was deprecated in favor of `Access::remove_write`. +- `Access::has_component_read` and `Access::has_resource_read` were deprecated in favor of `Access::has_read`. +- `Access::has_any_component_read` and `Access::has_any_resource_read` were deprecated in favor of `Access::has_any_read`. +- `Access::has_component_write` and `Access::has_resource_write` were deprecated in favor of `Access::has_write`. +- `Access::has_any_component_write` and `Access::has_any_resource_write` were deprecated in favor of `Access::has_any_write`. +- `Access::read_all_components` was deprecated in favor of `Access::read_all`. +- `Access::write_all_components` was deprecated in favor of `Access::write_all`. +- `Access::read_all_resources` and `Access::write_all_resources` were removed. +- `Access::has_read_all_components` was deprecated in favor of `Access::has_read_all`. +- `Access::has_write_all_components` was deprecated in favor of `Access::has_write_all`. +- `Access::has_read_all_resources` and `Access::has_write_all_resources` were removed. +- `Access::is_components_compatible` was deprecated in favor of `Access::is_compatible`. +- `Access::is_resources_compatible` was removed. +- `Access::is_subset_components` was deprecated in favor of `Access::is_subset`. +- `Access::is_subset_resources` was removed. +- `Access::resource_reads_and_writes`, `Access::resource_reads`, `Access::resource_writes` were removed. +- `Access::try_iter_component_access` was deprecated in favor of `Access::try_iter_access`. +- `FilteredAccess::add_component_read` was deprecated in favor of `FilteredAccess::add_read`. +- `FilteredAccess::add_component_write` was deprecated in favor of `FilteredAccess::add_write`. +- `FilteredAccess::add_resource_read` and `FilteredAccess::add_resource_write` were removed. +- `FilteredAccess::read_all_components` was deprecated in favor of `FilteredAccess::read_all`. +- `FilteredAccess::write_all_components` was deprecated in favor of `FilteredAccess::write_all`. +- `FilteredAccessSet::add_unfiltered_resource_read` was deprecated in favor of `FilteredAccessSet::add_resource_read`. +- `FilteredAccessSet::add_unfiltered_resource_write` was deprecated in favor of `FilteredAccessSet::add_resource_write`. + +Due to the split storage it used to be possible to both access an entity and a resource in a `WorldQuery` implementor. +This is no longer valid. In order to access multiple different entities for a `WorldQuery` implementation, use `WorldQuery::init_nested_access`. +See the implementation of `WorldQuery` for `AssetChanged` for an example of how this can be done correctly. ## Miscellaneous