From 7afcc276fb11377619dd86965c87b1643e78ac3a Mon Sep 17 00:00:00 2001 From: Rose Peck Date: Mon, 20 Jun 2022 21:31:43 -0400 Subject: [PATCH 01/59] Add basic types for archetype invariants --- .../src/world/archetype_invariants.rs | 80 +++++++++++++++++++ crates/bevy_ecs/src/world/mod.rs | 1 + 2 files changed, 81 insertions(+) create mode 100644 crates/bevy_ecs/src/world/archetype_invariants.rs diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs new file mode 100644 index 0000000000000..c4cfae5151baa --- /dev/null +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -0,0 +1,80 @@ +use bevy_utils::HashSet; + +use crate::{prelude::{Bundle, Component}, world::World, component::{ComponentId}}; + +/// A rule about the [`Component`]s that can coexist on entities +/// +/// These rules must be true at all times for all entities in the [`World`]. +/// +/// When added to the [`World`], archetype invariants behave like [`assert!`]; +/// all archetype invariants must be true for every entity in the [`World`]. +/// Archetype invariants are checked each time [`Archetypes`] is modified; +/// this can occur on component addition, component removal, and entity spawning. +/// +/// Archetypes are only modified when a novel archetype (set of components) is seen for the first time; +/// swapping between existing archetypes will not trigger these checks. +#[derive(Clone, Debug, PartialEq)] +pub struct ArchetypeInvariant { + /// For all entities where the predicate is true + pub predicate: ArchetypeStatement, + /// The consequence must also be true + pub consequence: ArchetypeStatement, +} + +/// A statement about the presence or absence of some subset of components in the given [`Bundle`] +/// +/// This type is used as part of an [`ArchetypeInvariant`]. +/// For the single-component equivalent, see [`ComponentStatement`]. +/// +/// When used as a predicate, the archetype invariant matches all entities which satisfy the statement. +/// When used as a consquence, then the statment must be true for all entities that were matched by the predicate. +#[derive(Clone, Debug, PartialEq)] +pub enum ArchetypeStatement { + /// Evaluates to true if and only if the entity has the component of type `C` + Has(ComponentId), + /// Evaluates to true if and only if the entity does not have the component of type `C` + DoesNotHave(ComponentId), + /// Evaluates to true if and only if the entity has all of the components present in Bundle `B` + AllOf(HashSet), + /// The entity has at least one component in the bundle, and may have all of them + AtLeastOneOf(HashSet), + /// The entity has none of the components in the bundle + NoneOf(HashSet), +} + +impl ArchetypeStatement { + /// Constructs a new [`ArchetypeStatement::Has`] variant for a component of type `C` + pub fn has(world: &mut World) -> Self { + let component_id = world.init_component::(); + ArchetypeStatement::Has(component_id) + } + + /// Constructs a new [`ArchetypeStatement::DoesNotHave`] variant for a component of type `C` + pub fn does_not_have(world: &mut World) -> Self { + let component_id = world.init_component::(); + ArchetypeStatement::DoesNotHave(component_id) + } + + /// Constructs a new [`ArchetypeStatement::AllOf`] variant for all components stored in the bundle `B` + pub fn all_of(world: &mut World) -> Self { + let component_ids = B::component_ids(&mut world.components, &mut world.storages); + ArchetypeStatement::AllOf(component_ids.into_iter().collect()) + } + + /// Constructs a new [`ArchetypeStatement::AtLeastOneOf`] variant for all components stored in the bundle `B` + pub fn at_least_one_of(world: &mut World) -> Self { + let component_ids = B::component_ids(&mut world.components, &mut world.storages); + ArchetypeStatement::AtLeastOneOf(component_ids.into_iter().collect()) + } + + /// Constructs a new [`ArchetypeStatement::NoneOf`] variant for all components stored in the bundle `B` + pub fn none_of(world: &mut World) -> Self { + let component_ids = B::component_ids(&mut world.components, &mut world.storages); + ArchetypeStatement::NoneOf(component_ids.into_iter().collect()) + } +} + +pub struct ArchetypeInvariants { + raw_list: Vec, + last_checked_archetype_index: u32, +} diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index d220495a82e1f..e16c25cd7f8d1 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -1,3 +1,4 @@ +pub mod archetype_invariants; mod entity_ref; mod spawn_batch; mod world_cell; From c4dc9e20c416288c497cd5d9c954c6efd550ab01 Mon Sep 17 00:00:00 2001 From: Rose Peck Date: Mon, 20 Jun 2022 21:32:14 -0400 Subject: [PATCH 02/59] Add docs for Components init methods --- crates/bevy_ecs/src/component.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index f4e2b61d14784..f320a756564f7 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -370,6 +370,10 @@ pub struct Components { } impl Components { + + /// Adds a new component type to [`Components`]. + /// + /// If the component type is already present, then simply return its [`ComponentId`]. #[inline] pub fn init_component(&mut self, storages: &mut Storages) -> ComponentId { let type_id = TypeId::of::(); @@ -385,6 +389,7 @@ impl Components { ComponentId(*index) } + /// Adds a new component with the provided [`ComponentDescriptor`] to [`Components`]. pub fn init_component_with_descriptor( &mut self, storages: &mut Storages, From 22b7698a3b5c18db796d5643daed42f67465be71 Mon Sep 17 00:00:00 2001 From: Rose Peck Date: Tue, 21 Jun 2022 08:19:33 -0400 Subject: [PATCH 03/59] Add archetype invariants to World --- .../src/world/archetype_invariants.rs | 9 +++++++++ crates/bevy_ecs/src/world/mod.rs | 20 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index c4cfae5151baa..6c37f42996549 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -74,7 +74,16 @@ impl ArchetypeStatement { } } +#[derive(Default)] pub struct ArchetypeInvariants { raw_list: Vec, last_checked_archetype_index: u32, } + +impl ArchetypeInvariants { + + /// Adds a new [`ArchetypeInvariant`] to this set of archetype invariants. + pub fn add(&mut self, archetype_invariant: ArchetypeInvariant) { + self.raw_list.push(archetype_invariant); + } +} \ No newline at end of file diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index e16c25cd7f8d1..fa12718b3bf0b 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -8,6 +8,8 @@ pub use entity_ref::*; pub use spawn_batch::*; pub use world_cell::*; +use self::archetype_invariants::{ArchetypeInvariants, ArchetypeInvariant}; + use crate::{ archetype::{ArchetypeComponentId, ArchetypeComponentInfo, ArchetypeId, Archetypes}, bundle::{Bundle, BundleInserter, BundleSpawner, Bundles}, @@ -84,6 +86,7 @@ pub struct World { pub(crate) entities: Entities, pub(crate) components: Components, pub(crate) archetypes: Archetypes, + pub(crate) archetype_invariants: ArchetypeInvariants, pub(crate) storages: Storages, pub(crate) bundles: Bundles, pub(crate) removed_components: SparseSet>, @@ -101,6 +104,7 @@ impl Default for World { entities: Default::default(), components: Default::default(), archetypes: Default::default(), + archetype_invariants: Default::default(), storages: Default::default(), bundles: Default::default(), removed_components: Default::default(), @@ -154,6 +158,12 @@ impl World { &self.archetypes } + /// Retrieves this world's [ArchetypeInvariants] collection + #[inline] + pub fn archetype_invariants(&self) -> &ArchetypeInvariants { + &self.archetype_invariants + } + /// Retrieves this world's [Components] collection #[inline] pub fn components(&self) -> &Components { @@ -666,6 +676,16 @@ impl World { } } + /// Inserts a new [`ArchetypeInvariant`] to the world. + /// This should only be done at world initialization. + /// Doing this after entities are spawned make it impossible to tell which archetypes are still valid. + pub fn add_archetype_invariant( + &mut self, + archetype_invariant: ArchetypeInvariant + ) { + self.archetype_invariants.add(archetype_invariant); + } + /// Inserts a new resource with standard starting values. /// /// If the resource already exists, nothing happens. From fbb2d54636b22871477ec830927a5aa8580daf4b Mon Sep 17 00:00:00 2001 From: Rose Peck Date: Tue, 21 Jun 2022 08:19:40 -0400 Subject: [PATCH 04/59] Add basic archetype invariant helper --- .../src/world/archetype_invariants.rs | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index 6c37f42996549..4431162a54def 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -21,6 +21,18 @@ pub struct ArchetypeInvariant { pub consequence: ArchetypeStatement, } +impl ArchetypeInvariant { + /// This is a helper function for constructing common invariants. + /// All components of the provided bundle require each other. + /// In other words, if any one component of this bundle is present, then all of them must be. + pub fn full_bundle(world: &mut World) -> Self { + Self { + predicate: ArchetypeStatement::at_least_one_of::(world), + consequence: ArchetypeStatement::all_of::(world) + } + } +} + /// A statement about the presence or absence of some subset of components in the given [`Bundle`] /// /// This type is used as part of an [`ArchetypeInvariant`]. @@ -86,4 +98,33 @@ impl ArchetypeInvariants { pub fn add(&mut self, archetype_invariant: ArchetypeInvariant) { self.raw_list.push(archetype_invariant); } +} + +#[cfg(test)] +mod tests { + use crate::{ + self as bevy_ecs, + component::Component, + world::World, + world::archetype_invariants::ArchetypeInvariant + }; + + #[derive(Component)] + struct A; + + #[derive(Component)] + struct B; + + #[derive(Component)] + struct C; + + #[test] + fn full_bundle() { + let mut world = World::new(); + + let bundle_invariant = ArchetypeInvariant::full_bundle::<(A, B, C)>(&mut world); + world.add_archetype_invariant(bundle_invariant); + + todo!(); + } } \ No newline at end of file From df94ac39f31b251870f489f61335f4c146dc0a41 Mon Sep 17 00:00:00 2001 From: Rose Peck Date: Wed, 22 Jun 2022 13:49:40 -0400 Subject: [PATCH 05/59] Recheck archetype invariants when a new invariant is added --- crates/bevy_ecs/src/world/archetype_invariants.rs | 4 ++++ crates/bevy_ecs/src/world/mod.rs | 5 +++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index 4431162a54def..ed0dc028978c0 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -95,7 +95,11 @@ pub struct ArchetypeInvariants { impl ArchetypeInvariants { /// Adds a new [`ArchetypeInvariant`] to this set of archetype invariants. + /// + /// Whenever a new archetype invariant is added, all existing archetypes are re-checked. + /// This may include empty archetypes- archetypes that contain no entities. pub fn add(&mut self, archetype_invariant: ArchetypeInvariant) { + self.last_checked_archetype_index = 0; self.raw_list.push(archetype_invariant); } } diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index fa12718b3bf0b..5cc60b73579dc 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -677,8 +677,9 @@ impl World { } /// Inserts a new [`ArchetypeInvariant`] to the world. - /// This should only be done at world initialization. - /// Doing this after entities are spawned make it impossible to tell which archetypes are still valid. + /// + /// Whenever a new archetype invariant is added, all existing archetypes are re-checked. + /// This may include empty archetypes- archetypes that contain no entities. pub fn add_archetype_invariant( &mut self, archetype_invariant: ArchetypeInvariant From 7dde3e877df45c18a15c729c19cac83a48877da7 Mon Sep 17 00:00:00 2001 From: Rose Peck Date: Wed, 22 Jun 2022 14:31:19 -0400 Subject: [PATCH 06/59] Refactor to use both typed and untyped archetype invariants // WIP --- .../src/world/archetype_invariants.rs | 117 +++++++++++++----- crates/bevy_ecs/src/world/mod.rs | 18 ++- 2 files changed, 104 insertions(+), 31 deletions(-) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index ed0dc028978c0..2b51cc62268fd 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -1,8 +1,10 @@ +use std::any::TypeId; + use bevy_utils::HashSet; -use crate::{prelude::{Bundle, Component}, world::World, component::{ComponentId}}; +use crate::{prelude::{Bundle, Component}, world::World, component::ComponentId}; -/// A rule about the [`Component`]s that can coexist on entities +/// A rule about which [`Component`]s can coexist on entities /// /// These rules must be true at all times for all entities in the [`World`]. /// @@ -13,6 +15,9 @@ use crate::{prelude::{Bundle, Component}, world::World, component::{ComponentId} /// /// Archetypes are only modified when a novel archetype (set of components) is seen for the first time; /// swapping between existing archetypes will not trigger these checks. +/// +/// Note that this is converted to an [`UntypedArchetypeInvariant`] when added to a [`World`]. +/// This is to ensure compatibility between different invariants. #[derive(Clone, Debug, PartialEq)] pub struct ArchetypeInvariant { /// For all entities where the predicate is true @@ -25,10 +30,22 @@ impl ArchetypeInvariant { /// This is a helper function for constructing common invariants. /// All components of the provided bundle require each other. /// In other words, if any one component of this bundle is present, then all of them must be. - pub fn full_bundle(world: &mut World) -> Self { + #[inline] + pub fn full_bundle() -> Self { Self { - predicate: ArchetypeStatement::at_least_one_of::(world), - consequence: ArchetypeStatement::all_of::(world) + predicate: ArchetypeStatement::at_least_one_of::(), + consequence: ArchetypeStatement::all_of::() + } + } + + /// Erases the type information of this archetype invariant. + /// + /// Requires mutable world access, since the components might not have been added to the world yet. + #[inline] + pub fn into_untyped(self, world: &mut World) -> UntypedArchetypeInvariant { + UntypedArchetypeInvariant { + predicate: self.predicate.into_untyped(world), + consequence: self.consequence.into_untyped(world) } } } @@ -40,55 +57,96 @@ impl ArchetypeInvariant { /// /// When used as a predicate, the archetype invariant matches all entities which satisfy the statement. /// When used as a consquence, then the statment must be true for all entities that were matched by the predicate. +/// +/// Note that this is converted to an [`UntypedArchetypeStatment`] when added to a [`World`]. +/// This is to ensure compatibility between different invariants. #[derive(Clone, Debug, PartialEq)] pub enum ArchetypeStatement { /// Evaluates to true if and only if the entity has the component of type `C` - Has(ComponentId), + Has(TypeId), /// Evaluates to true if and only if the entity does not have the component of type `C` - DoesNotHave(ComponentId), + DoesNotHave(TypeId), /// Evaluates to true if and only if the entity has all of the components present in Bundle `B` - AllOf(HashSet), + AllOf(HashSet), /// The entity has at least one component in the bundle, and may have all of them - AtLeastOneOf(HashSet), + AtLeastOneOf(HashSet), /// The entity has none of the components in the bundle - NoneOf(HashSet), + NoneOf(HashSet), } impl ArchetypeStatement { + /// Erases the type information of this archetype statment. + /// + /// Requires mutable world access, since the components might not have been added to the world yet. + pub fn into_untyped(self, _world: &mut World) -> UntypedArchetypeStatement { + match self { + ArchetypeStatement::Has(_) => todo!(), + ArchetypeStatement::DoesNotHave(_) => todo!(), + ArchetypeStatement::AllOf(_) => todo!(), + ArchetypeStatement::AtLeastOneOf(_) => todo!(), + ArchetypeStatement::NoneOf(_) => todo!(), + } + } + /// Constructs a new [`ArchetypeStatement::Has`] variant for a component of type `C` - pub fn has(world: &mut World) -> Self { - let component_id = world.init_component::(); - ArchetypeStatement::Has(component_id) + pub fn has() -> Self { + let type_id = TypeId::of::(); + ArchetypeStatement::Has(type_id) } /// Constructs a new [`ArchetypeStatement::DoesNotHave`] variant for a component of type `C` - pub fn does_not_have(world: &mut World) -> Self { - let component_id = world.init_component::(); - ArchetypeStatement::DoesNotHave(component_id) + pub fn does_not_have() -> Self { + let type_id = TypeId::of::(); + ArchetypeStatement::DoesNotHave(type_id) } /// Constructs a new [`ArchetypeStatement::AllOf`] variant for all components stored in the bundle `B` - pub fn all_of(world: &mut World) -> Self { - let component_ids = B::component_ids(&mut world.components, &mut world.storages); - ArchetypeStatement::AllOf(component_ids.into_iter().collect()) + pub fn all_of() -> Self { + todo!() } /// Constructs a new [`ArchetypeStatement::AtLeastOneOf`] variant for all components stored in the bundle `B` - pub fn at_least_one_of(world: &mut World) -> Self { - let component_ids = B::component_ids(&mut world.components, &mut world.storages); - ArchetypeStatement::AtLeastOneOf(component_ids.into_iter().collect()) + pub fn at_least_one_of() -> Self { + todo!() } /// Constructs a new [`ArchetypeStatement::NoneOf`] variant for all components stored in the bundle `B` - pub fn none_of(world: &mut World) -> Self { - let component_ids = B::component_ids(&mut world.components, &mut world.storages); - ArchetypeStatement::NoneOf(component_ids.into_iter().collect()) + pub fn none_of() -> Self { + todo!() } } +/// A type-erased version of [`ArchetypeInvariant`]. +/// Intended to be used with dynamic components that cannot be represented with Rust types. +/// Prefer [`ArchetypeInvariant`] when possible. +#[derive(Clone, Debug, PartialEq)] +pub struct UntypedArchetypeInvariant { + /// For all entities where the predicate is true + pub predicate: UntypedArchetypeStatement, + /// The consequence must also be true + pub consequence: UntypedArchetypeStatement, +} + +/// A type-erased version of [`ArchetypeStatment`] +/// Intended to be used with dynamic components that cannot be represented with Rust types. +/// Prefer [`ArchetypeStatment`] when possible. +#[derive(Clone, Debug, PartialEq)] +pub enum UntypedArchetypeStatement { + /// Evaluates to true if and only if the entity has the component of type `C` + Has(ComponentId), + /// Evaluates to true if and only if the entity does not have the component of type `C` + DoesNotHave(ComponentId), + /// Evaluates to true if and only if the entity has all of the components present in Bundle `B` + AllOf(HashSet), + /// The entity has at least one component in the bundle, and may have all of them + AtLeastOneOf(HashSet), + /// The entity has none of the components in the bundle + NoneOf(HashSet), +} + #[derive(Default)] pub struct ArchetypeInvariants { - raw_list: Vec, + raw_list: Vec, last_checked_archetype_index: u32, } @@ -98,7 +156,7 @@ impl ArchetypeInvariants { /// /// Whenever a new archetype invariant is added, all existing archetypes are re-checked. /// This may include empty archetypes- archetypes that contain no entities. - pub fn add(&mut self, archetype_invariant: ArchetypeInvariant) { + pub fn add(&mut self, archetype_invariant: UntypedArchetypeInvariant) { self.last_checked_archetype_index = 0; self.raw_list.push(archetype_invariant); } @@ -126,8 +184,9 @@ mod tests { fn full_bundle() { let mut world = World::new(); - let bundle_invariant = ArchetypeInvariant::full_bundle::<(A, B, C)>(&mut world); - world.add_archetype_invariant(bundle_invariant); + world.add_archetype_invariant( + ArchetypeInvariant::full_bundle::<(A, B, C)>() + ); todo!(); } diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 5cc60b73579dc..f075f7c974aa6 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -8,7 +8,7 @@ pub use entity_ref::*; pub use spawn_batch::*; pub use world_cell::*; -use self::archetype_invariants::{ArchetypeInvariants, ArchetypeInvariant}; +use self::archetype_invariants::{ArchetypeInvariants, ArchetypeInvariant, UntypedArchetypeInvariant}; use crate::{ archetype::{ArchetypeComponentId, ArchetypeComponentInfo, ArchetypeId, Archetypes}, @@ -680,11 +680,25 @@ impl World { /// /// Whenever a new archetype invariant is added, all existing archetypes are re-checked. /// This may include empty archetypes- archetypes that contain no entities. + #[inline] pub fn add_archetype_invariant( &mut self, archetype_invariant: ArchetypeInvariant ) { - self.archetype_invariants.add(archetype_invariant); + let internal_invariant = archetype_invariant.into_untyped(self); + self.archetype_invariants.add(internal_invariant); + } + + /// Inserts a new [`UntypedArchetypeInvariant`] to the world + /// + /// Whenever a new archetype invariant is added, all existing archetypes are re-checked. + /// This may include empty archetypes- archetypes that contain no entities. + /// Prefer [`add_archetype_invariant`](World::add_archertype_invariant) where possible. + pub fn add_untyped_archetype_invariant( + &mut self, + archetype_invariant: UntypedArchetypeInvariant + ) { + self.archetype_invariants.add(archetype_invariant) } /// Inserts a new resource with standard starting values. From ff169fee366e9a42c98a16504092351e0aa5cc2a Mon Sep 17 00:00:00 2001 From: Rose Peck Date: Fri, 24 Jun 2022 14:32:02 -0400 Subject: [PATCH 07/59] Rework API to use bundles and raw types --- crates/bevy_ecs/src/component.rs | 3 +- .../src/world/archetype_invariants.rs | 161 +++++++++--------- crates/bevy_ecs/src/world/mod.rs | 21 ++- 3 files changed, 89 insertions(+), 96 deletions(-) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index f320a756564f7..90d376b34bdf5 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -370,9 +370,8 @@ pub struct Components { } impl Components { - /// Adds a new component type to [`Components`]. - /// + /// /// If the component type is already present, then simply return its [`ComponentId`]. #[inline] pub fn init_component(&mut self, storages: &mut Storages) -> ComponentId { diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index 2b51cc62268fd..46c266adb4d29 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -1,8 +1,8 @@ -use std::any::TypeId; +use std::marker::PhantomData; -use bevy_utils::HashSet; +use bevy_utils::{tracing::warn, HashSet}; -use crate::{prelude::{Bundle, Component}, world::World, component::ComponentId}; +use crate::{component::ComponentId, prelude::Bundle, world::World}; /// A rule about which [`Component`]s can coexist on entities /// @@ -12,107 +12,106 @@ use crate::{prelude::{Bundle, Component}, world::World, component::ComponentId}; /// all archetype invariants must be true for every entity in the [`World`]. /// Archetype invariants are checked each time [`Archetypes`] is modified; /// this can occur on component addition, component removal, and entity spawning. -/// +/// /// Archetypes are only modified when a novel archetype (set of components) is seen for the first time; /// swapping between existing archetypes will not trigger these checks. -/// +/// /// Note that this is converted to an [`UntypedArchetypeInvariant`] when added to a [`World`]. /// This is to ensure compatibility between different invariants. #[derive(Clone, Debug, PartialEq)] -pub struct ArchetypeInvariant { +pub struct ArchetypeInvariant { /// For all entities where the predicate is true - pub predicate: ArchetypeStatement, + pub predicate: ArchetypeStatement, /// The consequence must also be true - pub consequence: ArchetypeStatement, + pub consequence: ArchetypeStatement, } -impl ArchetypeInvariant { - /// This is a helper function for constructing common invariants. - /// All components of the provided bundle require each other. - /// In other words, if any one component of this bundle is present, then all of them must be. - #[inline] - pub fn full_bundle() -> Self { - Self { - predicate: ArchetypeStatement::at_least_one_of::(), - consequence: ArchetypeStatement::all_of::() - } - } - +impl ArchetypeInvariant { /// Erases the type information of this archetype invariant. - /// + /// /// Requires mutable world access, since the components might not have been added to the world yet. #[inline] pub fn into_untyped(self, world: &mut World) -> UntypedArchetypeInvariant { - UntypedArchetypeInvariant { + UntypedArchetypeInvariant { predicate: self.predicate.into_untyped(world), - consequence: self.consequence.into_untyped(world) + consequence: self.consequence.into_untyped(world), + } + } +} + +impl ArchetypeInvariant { + /// This is a helper function for constructing common invariants. + /// All components of the provided bundle require each other. + /// In other words, if any one component of this bundle is present, then all of them must be. + #[inline] + pub fn full_bundle() -> Self { + Self { + predicate: ArchetypeStatement::::at_least_one_of(), + consequence: ArchetypeStatement::::all_of(), } } } /// A statement about the presence or absence of some subset of components in the given [`Bundle`] /// -/// This type is used as part of an [`ArchetypeInvariant`]. -/// For the single-component equivalent, see [`ComponentStatement`]. +/// This type is used as part of an [`ArchetypeInvariant`]. /// -/// When used as a predicate, the archetype invariant matches all entities which satisfy the statement. +/// When used as a predicate, the archetype invariant matches all entities which satisfy the statement. /// When used as a consquence, then the statment must be true for all entities that were matched by the predicate. -/// +/// +/// For the statements about a single component `C`, wrap it in a single-component bundle `(C,)`. +/// For single component bundles, `AllOf` and `AtLeastOneOf` are equivalent. +/// Prefer `ArchetypeStatement::<(C,)>::all_of` over `ArchetypeStatement::<(C,)>::at_least_one_of` for consistency and clarity. +/// /// Note that this is converted to an [`UntypedArchetypeStatment`] when added to a [`World`]. /// This is to ensure compatibility between different invariants. #[derive(Clone, Debug, PartialEq)] -pub enum ArchetypeStatement { - /// Evaluates to true if and only if the entity has the component of type `C` - Has(TypeId), - /// Evaluates to true if and only if the entity does not have the component of type `C` - DoesNotHave(TypeId), - /// Evaluates to true if and only if the entity has all of the components present in Bundle `B` - AllOf(HashSet), - /// The entity has at least one component in the bundle, and may have all of them - AtLeastOneOf(HashSet), - /// The entity has none of the components in the bundle - NoneOf(HashSet), +pub enum ArchetypeStatement { + /// Evaluates to true if and only if the entity has all of the components present in the bundle `B` + AllOf(PhantomData), + /// The entity has at least one component in the bundle `B`, and may have all of them. + /// When using a single-component bundle, `AllOf` is preferred. + AtLeastOneOf(PhantomData), + /// The entity has none of the components in the bundle `B` + NoneOf(PhantomData), } -impl ArchetypeStatement { +impl ArchetypeStatement { /// Erases the type information of this archetype statment. - /// + /// /// Requires mutable world access, since the components might not have been added to the world yet. - pub fn into_untyped(self, _world: &mut World) -> UntypedArchetypeStatement { + pub fn into_untyped(self, world: &mut World) -> UntypedArchetypeStatement { + let component_ids = B::component_ids(&mut world.components, &mut world.storages); + let component_ids: HashSet = component_ids.into_iter().collect(); + match self { - ArchetypeStatement::Has(_) => todo!(), - ArchetypeStatement::DoesNotHave(_) => todo!(), - ArchetypeStatement::AllOf(_) => todo!(), - ArchetypeStatement::AtLeastOneOf(_) => todo!(), - ArchetypeStatement::NoneOf(_) => todo!(), + ArchetypeStatement::AllOf(_) => UntypedArchetypeStatement::AllOf(component_ids), + ArchetypeStatement::AtLeastOneOf(_) => { + if component_ids.len() == 1 { + warn!("An `ArchetypeStatement::AtLeastOneOf` was constructed for a bundle with only one component. Prefer the equivalent `ArchetypeStatment:AllOf` for consistency and clarity."); + } + UntypedArchetypeStatement::AtLeastOneOf(component_ids) + } + ArchetypeStatement::NoneOf(_) => UntypedArchetypeStatement::NoneOf(component_ids), } } - - /// Constructs a new [`ArchetypeStatement::Has`] variant for a component of type `C` - pub fn has() -> Self { - let type_id = TypeId::of::(); - ArchetypeStatement::Has(type_id) - } - /// Constructs a new [`ArchetypeStatement::DoesNotHave`] variant for a component of type `C` - pub fn does_not_have() -> Self { - let type_id = TypeId::of::(); - ArchetypeStatement::DoesNotHave(type_id) + /// Constructs a new [`ArchetypeStatement::AllOf`] variant for all components stored in the bundle `B` + #[inline] + pub const fn all_of() -> Self { + ArchetypeStatement::AllOf(PhantomData) } - /// Constructs a new [`ArchetypeStatement::AllOf`] variant for all components stored in the bundle `B` - pub fn all_of() -> Self { - todo!() - } - - /// Constructs a new [`ArchetypeStatement::AtLeastOneOf`] variant for all components stored in the bundle `B` - pub fn at_least_one_of() -> Self { - todo!() + /// Constructs a new [`ArchetypeStatement::AtLeastOneOf`] variant for all components stored in the bundle `B` + #[inline] + pub const fn at_least_one_of() -> Self { + ArchetypeStatement::AtLeastOneOf(PhantomData) } - + /// Constructs a new [`ArchetypeStatement::NoneOf`] variant for all components stored in the bundle `B` - pub fn none_of() -> Self { - todo!() + #[inline] + pub const fn none_of() -> Self { + ArchetypeStatement::NoneOf(PhantomData) } } @@ -132,15 +131,12 @@ pub struct UntypedArchetypeInvariant { /// Prefer [`ArchetypeStatment`] when possible. #[derive(Clone, Debug, PartialEq)] pub enum UntypedArchetypeStatement { - /// Evaluates to true if and only if the entity has the component of type `C` - Has(ComponentId), - /// Evaluates to true if and only if the entity does not have the component of type `C` - DoesNotHave(ComponentId), - /// Evaluates to true if and only if the entity has all of the components present in Bundle `B` + /// Evaluates to true if and only if the entity has all of the components present in the set AllOf(HashSet), - /// The entity has at least one component in the bundle, and may have all of them + /// The entity has at least one component in the set, and may have all of them. + /// When using a single-component bundle, `AllOf` is preferred AtLeastOneOf(HashSet), - /// The entity has none of the components in the bundle + /// The entity has none of the components in the set NoneOf(HashSet), } @@ -151,9 +147,8 @@ pub struct ArchetypeInvariants { } impl ArchetypeInvariants { - /// Adds a new [`ArchetypeInvariant`] to this set of archetype invariants. - /// + /// /// Whenever a new archetype invariant is added, all existing archetypes are re-checked. /// This may include empty archetypes- archetypes that contain no entities. pub fn add(&mut self, archetype_invariant: UntypedArchetypeInvariant) { @@ -165,18 +160,16 @@ impl ArchetypeInvariants { #[cfg(test)] mod tests { use crate::{ - self as bevy_ecs, - component::Component, + self as bevy_ecs, component::Component, world::archetype_invariants::ArchetypeInvariant, world::World, - world::archetype_invariants::ArchetypeInvariant }; #[derive(Component)] struct A; - + #[derive(Component)] struct B; - + #[derive(Component)] struct C; @@ -184,10 +177,8 @@ mod tests { fn full_bundle() { let mut world = World::new(); - world.add_archetype_invariant( - ArchetypeInvariant::full_bundle::<(A, B, C)>() - ); + world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::full_bundle()); todo!(); } -} \ No newline at end of file +} diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index f075f7c974aa6..3e9965512b641 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -8,7 +8,9 @@ pub use entity_ref::*; pub use spawn_batch::*; pub use world_cell::*; -use self::archetype_invariants::{ArchetypeInvariants, ArchetypeInvariant, UntypedArchetypeInvariant}; +use self::archetype_invariants::{ + ArchetypeInvariant, ArchetypeInvariants, UntypedArchetypeInvariant, +}; use crate::{ archetype::{ArchetypeComponentId, ArchetypeComponentInfo, ArchetypeId, Archetypes}, @@ -158,7 +160,7 @@ impl World { &self.archetypes } - /// Retrieves this world's [ArchetypeInvariants] collection + /// Retrieves this world's [`ArchetypeInvariants`] collection #[inline] pub fn archetype_invariants(&self) -> &ArchetypeInvariants { &self.archetype_invariants @@ -676,29 +678,30 @@ impl World { } } - /// Inserts a new [`ArchetypeInvariant`] to the world. - /// + /// Inserts a new [`ArchetypeInvariant`] to the world. + /// /// Whenever a new archetype invariant is added, all existing archetypes are re-checked. /// This may include empty archetypes- archetypes that contain no entities. #[inline] - pub fn add_archetype_invariant( + pub fn add_archetype_invariant( &mut self, - archetype_invariant: ArchetypeInvariant + archetype_invariant: ArchetypeInvariant, ) { let internal_invariant = archetype_invariant.into_untyped(self); self.archetype_invariants.add(internal_invariant); } /// Inserts a new [`UntypedArchetypeInvariant`] to the world - /// + /// /// Whenever a new archetype invariant is added, all existing archetypes are re-checked. /// This may include empty archetypes- archetypes that contain no entities. /// Prefer [`add_archetype_invariant`](World::add_archertype_invariant) where possible. + #[inline] pub fn add_untyped_archetype_invariant( &mut self, - archetype_invariant: UntypedArchetypeInvariant + archetype_invariant: UntypedArchetypeInvariant, ) { - self.archetype_invariants.add(archetype_invariant) + self.archetype_invariants.add(archetype_invariant); } /// Inserts a new resource with standard starting values. From 177afe68bc8fa8ce6b0de7e64f35775f96bea02e Mon Sep 17 00:00:00 2001 From: Rose Peck Date: Fri, 24 Jun 2022 15:00:25 -0400 Subject: [PATCH 08/59] Add `archetype_invariants` to `Archetype::new` --- crates/bevy_ecs/src/archetype.rs | 8 ++++++-- crates/bevy_ecs/src/bundle.rs | 11 +++++++---- crates/bevy_ecs/src/world/entity_ref.rs | 9 +++++++++ crates/bevy_ecs/src/world/mod.rs | 3 +++ crates/bevy_ecs/src/world/spawn_batch.rs | 1 + 5 files changed, 26 insertions(+), 6 deletions(-) diff --git a/crates/bevy_ecs/src/archetype.rs b/crates/bevy_ecs/src/archetype.rs index 2c01f8c1bb4b2..1b0baeb2b69a2 100644 --- a/crates/bevy_ecs/src/archetype.rs +++ b/crates/bevy_ecs/src/archetype.rs @@ -5,7 +5,7 @@ use crate::{ bundle::BundleId, component::{ComponentId, StorageType}, entity::{Entity, EntityLocation}, - storage::{Column, SparseArray, SparseSet, SparseSetIndex, TableId}, + storage::{Column, SparseArray, SparseSet, SparseSetIndex, TableId}, world::archetype_invariants::{ArchetypeInvariants}, }; use std::{ collections::HashMap, @@ -152,6 +152,7 @@ impl Archetype { sparse_set_components: Box<[ComponentId]>, table_archetype_components: Vec, sparse_set_archetype_components: Vec, + archetype_invariants: &ArchetypeInvariants ) -> Self { let mut components = SparseSet::with_capacity(table_components.len() + sparse_set_components.len()); @@ -391,7 +392,7 @@ impl Default for Archetypes { archetype_ids: Default::default(), archetype_component_count: 0, }; - archetypes.get_id_or_insert(TableId::empty(), Vec::new(), Vec::new()); + archetypes.get_id_or_insert(TableId::empty(), Vec::new(), Vec::new(), &ArchetypeInvariants::default()); // adds the resource archetype. it is "special" in that it is inaccessible via a "hash", // which prevents entities from being added to it @@ -402,6 +403,7 @@ impl Default for Archetypes { Box::new([]), Vec::new(), Vec::new(), + &ArchetypeInvariants::default(), )); archetypes } @@ -488,6 +490,7 @@ impl Archetypes { table_id: TableId, table_components: Vec, sparse_set_components: Vec, + archetype_invariants: &ArchetypeInvariants, ) -> ArchetypeId { let table_components = table_components.into_boxed_slice(); let sparse_set_components = sparse_set_components.into_boxed_slice(); @@ -521,6 +524,7 @@ impl Archetypes { sparse_set_components, table_archetype_components, sparse_set_archetype_components, + archetype_invariants )); id }) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 6b81435fc8b4b..829f561a73acd 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -8,7 +8,7 @@ use crate::{ archetype::{AddBundle, Archetype, ArchetypeId, Archetypes, ComponentStatus}, component::{Component, ComponentId, ComponentTicks, Components, StorageType}, entity::{Entities, Entity, EntityLocation}, - storage::{SparseSetIndex, SparseSets, Storages, Table}, + storage::{SparseSetIndex, SparseSets, Storages, Table}, world::archetype_invariants::ArchetypeInvariants, }; use bevy_ecs_macros::all_tuples; use bevy_ptr::OwningPtr; @@ -180,9 +180,10 @@ impl BundleInfo { storages: &'a mut Storages, archetype_id: ArchetypeId, change_tick: u32, + archetype_invariants: &ArchetypeInvariants, ) -> BundleInserter<'a, 'b> { let new_archetype_id = - self.add_bundle_to_archetype(archetypes, storages, components, archetype_id); + self.add_bundle_to_archetype(archetypes, storages, components, archetype_id, archetype_invariants); let archetypes_ptr = archetypes.archetypes.as_mut_ptr(); if new_archetype_id == archetype_id { let archetype = &mut archetypes[archetype_id]; @@ -239,9 +240,10 @@ impl BundleInfo { components: &mut Components, storages: &'a mut Storages, change_tick: u32, + archetype_invariants: &ArchetypeInvariants, ) -> BundleSpawner<'a, 'b> { let new_archetype_id = - self.add_bundle_to_archetype(archetypes, storages, components, ArchetypeId::EMPTY); + self.add_bundle_to_archetype(archetypes, storages, components, ArchetypeId::EMPTY, archetype_invariants); let (empty_archetype, archetype) = archetypes.get_2_mut(ArchetypeId::EMPTY, new_archetype_id); let table = &mut storages.tables[archetype.table_id()]; @@ -311,6 +313,7 @@ impl BundleInfo { storages: &mut Storages, components: &mut Components, archetype_id: ArchetypeId, + archetype_invariants: &ArchetypeInvariants, ) -> ArchetypeId { if let Some(add_bundle) = archetypes[archetype_id].edges().get_add_bundle(self.id) { return add_bundle.archetype_id; @@ -374,7 +377,7 @@ impl BundleInfo { }; }; let new_archetype_id = - archetypes.get_id_or_insert(table_id, table_components, sparse_set_components); + archetypes.get_id_or_insert(table_id, table_components, sparse_set_components, archetype_invariants); // add an edge from the old archetype to the new archetype archetypes[archetype_id].edges_mut().insert_add_bundle( self.id, diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index bf2701714190d..2cb0a26396a79 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -10,6 +10,8 @@ use crate::{ use bevy_ptr::{OwningPtr, Ptr, UnsafeCellDeref}; use std::{any::TypeId, cell::UnsafeCell}; +use super::archetype_invariants::ArchetypeInvariants; + /// A read-only reference to a particular [`Entity`] and all of its components pub struct EntityRef<'w> { world: &'w World, @@ -244,6 +246,7 @@ impl<'w> EntityMut<'w> { &mut self.world.storages, self.location.archetype_id, change_tick, + &self.world.archetype_invariants, ); // SAFE: location matches current entity. `T` matches `bundle_info` unsafe { @@ -260,6 +263,7 @@ impl<'w> EntityMut<'w> { let components = &mut self.world.components; let entities = &mut self.world.entities; let removed_components = &mut self.world.removed_components; + let archetype_invariants = &self.world.archetype_invariants; let bundle_info = self.world.bundles.init_info::(components, storages); let old_location = self.location; @@ -271,6 +275,7 @@ impl<'w> EntityMut<'w> { old_location.archetype_id, bundle_info, false, + archetype_invariants, )? }; @@ -382,6 +387,7 @@ impl<'w> EntityMut<'w> { let components = &mut self.world.components; let entities = &mut self.world.entities; let removed_components = &mut self.world.removed_components; + let archetype_invariants = &self.world.archetype_invariants; let bundle_info = self.world.bundles.init_info::(components, storages); let old_location = self.location; @@ -393,6 +399,7 @@ impl<'w> EntityMut<'w> { old_location.archetype_id, bundle_info, true, + archetype_invariants, ) .expect("intersections should always return a result") }; @@ -742,6 +749,7 @@ unsafe fn remove_bundle_from_archetype( archetype_id: ArchetypeId, bundle_info: &BundleInfo, intersection: bool, + archetype_invariants: &ArchetypeInvariants, ) -> Option { // check the archetype graph to see if the Bundle has been removed from this archetype in the // past @@ -811,6 +819,7 @@ unsafe fn remove_bundle_from_archetype( next_table_id, next_table_components, next_sparse_set_components, + archetype_invariants, ); Some(new_archetype_id) }; diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 3e9965512b641..d540dfb292404 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -1059,6 +1059,7 @@ impl World { &mut self.components, &mut self.storages, change_tick, + &self.archetype_invariants, )); let mut invalid_entities = Vec::new(); @@ -1083,6 +1084,7 @@ impl World { &mut self.storages, location.archetype_id, change_tick, + &self.archetype_invariants, ); // SAFE: `entity` is valid, `location` matches entity, bundle matches inserter unsafe { inserter.insert(entity, location.index, bundle) }; @@ -1102,6 +1104,7 @@ impl World { &mut self.components, &mut self.storages, change_tick, + &self.archetype_invariants, ); // SAFE: `entity` is valid, `location` matches entity, bundle matches inserter unsafe { spawner.spawn_non_existent(entity, bundle) }; diff --git a/crates/bevy_ecs/src/world/spawn_batch.rs b/crates/bevy_ecs/src/world/spawn_batch.rs index 002e30a2b3f3a..b71d0f1ad5ee1 100644 --- a/crates/bevy_ecs/src/world/spawn_batch.rs +++ b/crates/bevy_ecs/src/world/spawn_batch.rs @@ -38,6 +38,7 @@ where &mut world.components, &mut world.storages, *world.change_tick.get_mut(), + &world.archetype_invariants, ); spawner.reserve_storage(length); From cc1f9c1a4867b23a23afedd691b6541700cfd1ee Mon Sep 17 00:00:00 2001 From: Rose Peck Date: Fri, 24 Jun 2022 16:07:32 -0400 Subject: [PATCH 09/59] Add archetype checking skeleton --- crates/bevy_ecs/src/archetype.rs | 3 + .../src/world/archetype_invariants.rs | 60 ++++++++++++++++--- crates/bevy_ecs/src/world/mod.rs | 6 +- 3 files changed, 60 insertions(+), 9 deletions(-) diff --git a/crates/bevy_ecs/src/archetype.rs b/crates/bevy_ecs/src/archetype.rs index 1b0baeb2b69a2..ba9cbb7b34a7a 100644 --- a/crates/bevy_ecs/src/archetype.rs +++ b/crates/bevy_ecs/src/archetype.rs @@ -180,6 +180,9 @@ impl Archetype { }, ); } + + archetype_invariants.test_archetype(components.indices()); + Self { id, table_info: TableInfo { diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index 46c266adb4d29..5d739e73fd084 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -1,6 +1,8 @@ use std::marker::PhantomData; -use bevy_utils::{tracing::warn, HashSet}; +use bevy_utils::{tracing::warn}; +use fixedbitset::FixedBitSet; +use std::collections::BTreeSet; use crate::{component::ComponentId, prelude::Bundle, world::World}; @@ -82,7 +84,7 @@ impl ArchetypeStatement { /// Requires mutable world access, since the components might not have been added to the world yet. pub fn into_untyped(self, world: &mut World) -> UntypedArchetypeStatement { let component_ids = B::component_ids(&mut world.components, &mut world.storages); - let component_ids: HashSet = component_ids.into_iter().collect(); + let component_ids: BTreeSet = component_ids.into_iter().collect(); match self { ArchetypeStatement::AllOf(_) => UntypedArchetypeStatement::AllOf(component_ids), @@ -132,18 +134,31 @@ pub struct UntypedArchetypeInvariant { #[derive(Clone, Debug, PartialEq)] pub enum UntypedArchetypeStatement { /// Evaluates to true if and only if the entity has all of the components present in the set - AllOf(HashSet), + AllOf(BTreeSet), /// The entity has at least one component in the set, and may have all of them. /// When using a single-component bundle, `AllOf` is preferred - AtLeastOneOf(HashSet), + AtLeastOneOf(BTreeSet), /// The entity has none of the components in the set - NoneOf(HashSet), + NoneOf(BTreeSet), +} + +impl UntypedArchetypeStatement { + /// Get the set of [`ComponentId`]s affected by this statement + pub fn component_ids(&self) -> &BTreeSet { + match self { + UntypedArchetypeStatement::AllOf(set) => &set, + UntypedArchetypeStatement::AtLeastOneOf(set) => &set, + UntypedArchetypeStatement::NoneOf(set) => &set, + } + } } #[derive(Default)] pub struct ArchetypeInvariants { + /// The list of invariants that must be upheld raw_list: Vec, - last_checked_archetype_index: u32, + /// The set of all components that are used across all invariants + sorted_unique_component_ids: BTreeSet } impl ArchetypeInvariants { @@ -151,10 +166,41 @@ impl ArchetypeInvariants { /// /// Whenever a new archetype invariant is added, all existing archetypes are re-checked. /// This may include empty archetypes- archetypes that contain no entities. + #[inline] pub fn add(&mut self, archetype_invariant: UntypedArchetypeInvariant) { - self.last_checked_archetype_index = 0; + let predicate_ids = archetype_invariant.predicate.component_ids(); + let consequence_ids = archetype_invariant.consequence.component_ids(); + + for id in predicate_ids.iter() { + self.sorted_unique_component_ids.insert(*id); + } + for id in consequence_ids.iter() { + self.sorted_unique_component_ids.insert(*id); + } + self.raw_list.push(archetype_invariant); } + + /// Assert that the provided iterator of [`ComponentId`]s obeys all archetype invariants + /// + /// `component_ids` is generally provided via the `components` field on [`Archetype`]. + /// + /// # Panics + /// Panics if any archetype invariant is violated + pub(crate) fn test_archetype(&self, component_ids_of_archetype: impl Iterator) { + // Bit array of which components are contained in the archetype given + let mut contained_components = FixedBitSet::with_capacity(self.sorted_unique_component_ids.len()); + let sorted_component_ids_of_archetype: BTreeSet = component_ids_of_archetype.collect(); + + for (i, invariant_component_id) in self.sorted_unique_component_ids.iter().enumerate() { + if sorted_component_ids_of_archetype.contains(invariant_component_id) { + contained_components.put(i); + } + } + + for invariant in self.raw_list.iter() { + } + } } #[cfg(test)] diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index d540dfb292404..eda885203cc95 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -687,8 +687,10 @@ impl World { &mut self, archetype_invariant: ArchetypeInvariant, ) { - let internal_invariant = archetype_invariant.into_untyped(self); - self.archetype_invariants.add(internal_invariant); + let untyped_invariant = archetype_invariant.into_untyped(self); + self.archetype_invariants.add(untyped_invariant); + + todo!("Check all archetypes against the new invariant"); } /// Inserts a new [`UntypedArchetypeInvariant`] to the world From 3a88bb807f031c5f910150753f60b2647e3c2c54 Mon Sep 17 00:00:00 2001 From: Rose Peck Date: Fri, 24 Jun 2022 16:44:33 -0400 Subject: [PATCH 10/59] Add naieve archetype checking --- .../src/world/archetype_invariants.rs | 81 ++++++++++++------- 1 file changed, 54 insertions(+), 27 deletions(-) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index 5d739e73fd084..d00748487ad79 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -1,7 +1,6 @@ use std::marker::PhantomData; -use bevy_utils::{tracing::warn}; -use fixedbitset::FixedBitSet; +use bevy_utils::{tracing::warn, HashSet}; use std::collections::BTreeSet; use crate::{component::ComponentId, prelude::Bundle, world::World}; @@ -9,6 +8,9 @@ use crate::{component::ComponentId, prelude::Bundle, world::World}; /// A rule about which [`Component`]s can coexist on entities /// /// These rules must be true at all times for all entities in the [`World`]. +/// The generic [`Bundle`] type `B1` is always used in the `predicate`, +/// while `B2` is used in the `consequence`. +/// If only a single generic is provided, these types are the same. /// /// When added to the [`World`], archetype invariants behave like [`assert!`]; /// all archetype invariants must be true for every entity in the [`World`]. @@ -151,14 +153,42 @@ impl UntypedArchetypeStatement { UntypedArchetypeStatement::NoneOf(set) => &set, } } + + /// Test if this statement is true for the provided set of [`ComponentId`]s + pub fn test(&self, component_ids: &HashSet) -> bool{ + match self { + UntypedArchetypeStatement::AllOf(required_ids) => { + for required_id in required_ids { + if !component_ids.contains(required_id) { + return false; + } + } + true + }, + UntypedArchetypeStatement::AtLeastOneOf(desired_ids) => { + for desired_id in desired_ids { + if component_ids.contains(desired_id) { + return true; + } + } + false + }, + UntypedArchetypeStatement::NoneOf(forbidden_ids) => { + for forbidden_id in forbidden_ids { + if component_ids.contains(forbidden_id) { + return false; + } + } + true + }, + } + } } #[derive(Default)] pub struct ArchetypeInvariants { /// The list of invariants that must be upheld raw_list: Vec, - /// The set of all components that are used across all invariants - sorted_unique_component_ids: BTreeSet } impl ArchetypeInvariants { @@ -167,17 +197,7 @@ impl ArchetypeInvariants { /// Whenever a new archetype invariant is added, all existing archetypes are re-checked. /// This may include empty archetypes- archetypes that contain no entities. #[inline] - pub fn add(&mut self, archetype_invariant: UntypedArchetypeInvariant) { - let predicate_ids = archetype_invariant.predicate.component_ids(); - let consequence_ids = archetype_invariant.consequence.component_ids(); - - for id in predicate_ids.iter() { - self.sorted_unique_component_ids.insert(*id); - } - for id in consequence_ids.iter() { - self.sorted_unique_component_ids.insert(*id); - } - + pub fn add(&mut self, archetype_invariant: UntypedArchetypeInvariant) { self.raw_list.push(archetype_invariant); } @@ -188,17 +208,15 @@ impl ArchetypeInvariants { /// # Panics /// Panics if any archetype invariant is violated pub(crate) fn test_archetype(&self, component_ids_of_archetype: impl Iterator) { - // Bit array of which components are contained in the archetype given - let mut contained_components = FixedBitSet::with_capacity(self.sorted_unique_component_ids.len()); - let sorted_component_ids_of_archetype: BTreeSet = component_ids_of_archetype.collect(); - - for (i, invariant_component_id) in self.sorted_unique_component_ids.iter().enumerate() { - if sorted_component_ids_of_archetype.contains(invariant_component_id) { - contained_components.put(i); - } - } - + let component_ids_of_archetype: HashSet = component_ids_of_archetype.collect(); + for invariant in self.raw_list.iter() { + if + invariant.predicate.test(&component_ids_of_archetype) && + !invariant.consequence.test(&component_ids_of_archetype) + { + panic!("Archetype invariant violated!") + } } } } @@ -220,11 +238,20 @@ mod tests { struct C; #[test] - fn full_bundle() { + fn full_bundle_happy() { let mut world = World::new(); world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::full_bundle()); + world.spawn().insert_bundle((A, B, C)); + } - todo!(); + #[test] + #[should_panic] + fn full_bundle_sad() { + let mut world = World::new(); + + world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::full_bundle()); + world.spawn().insert_bundle((A, B)); + // The archetype invariant should catch this invalid arrangement and panic } } From dc3d834d1e28d637d1b3eb8ac9133590373678fa Mon Sep 17 00:00:00 2001 From: Rose Peck Date: Fri, 24 Jun 2022 16:47:04 -0400 Subject: [PATCH 11/59] Remove superfluous comment --- crates/bevy_ecs/src/world/archetype_invariants.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index d00748487ad79..1d9b0380c6132 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -252,6 +252,5 @@ mod tests { world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::full_bundle()); world.spawn().insert_bundle((A, B)); - // The archetype invariant should catch this invalid arrangement and panic } } From 842f8964ad6fe8016d9412fd5eb6aed0e6be4a39 Mon Sep 17 00:00:00 2001 From: plof27 Date: Sun, 26 Jun 2022 10:21:27 -0400 Subject: [PATCH 12/59] Run cargo fmt and clippy --- crates/bevy_ecs/src/archetype.rs | 14 ++++++--- crates/bevy_ecs/src/bundle.rs | 30 ++++++++++++++----- .../src/world/archetype_invariants.rs | 30 ++++++++++--------- 3 files changed, 49 insertions(+), 25 deletions(-) diff --git a/crates/bevy_ecs/src/archetype.rs b/crates/bevy_ecs/src/archetype.rs index ba9cbb7b34a7a..8bcff71bf0f01 100644 --- a/crates/bevy_ecs/src/archetype.rs +++ b/crates/bevy_ecs/src/archetype.rs @@ -5,7 +5,8 @@ use crate::{ bundle::BundleId, component::{ComponentId, StorageType}, entity::{Entity, EntityLocation}, - storage::{Column, SparseArray, SparseSet, SparseSetIndex, TableId}, world::archetype_invariants::{ArchetypeInvariants}, + storage::{Column, SparseArray, SparseSet, SparseSetIndex, TableId}, + world::archetype_invariants::ArchetypeInvariants, }; use std::{ collections::HashMap, @@ -152,7 +153,7 @@ impl Archetype { sparse_set_components: Box<[ComponentId]>, table_archetype_components: Vec, sparse_set_archetype_components: Vec, - archetype_invariants: &ArchetypeInvariants + archetype_invariants: &ArchetypeInvariants, ) -> Self { let mut components = SparseSet::with_capacity(table_components.len() + sparse_set_components.len()); @@ -395,7 +396,12 @@ impl Default for Archetypes { archetype_ids: Default::default(), archetype_component_count: 0, }; - archetypes.get_id_or_insert(TableId::empty(), Vec::new(), Vec::new(), &ArchetypeInvariants::default()); + archetypes.get_id_or_insert( + TableId::empty(), + Vec::new(), + Vec::new(), + &ArchetypeInvariants::default(), + ); // adds the resource archetype. it is "special" in that it is inaccessible via a "hash", // which prevents entities from being added to it @@ -527,7 +533,7 @@ impl Archetypes { sparse_set_components, table_archetype_components, sparse_set_archetype_components, - archetype_invariants + archetype_invariants, )); id }) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 829f561a73acd..993122e6c56e6 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -8,7 +8,8 @@ use crate::{ archetype::{AddBundle, Archetype, ArchetypeId, Archetypes, ComponentStatus}, component::{Component, ComponentId, ComponentTicks, Components, StorageType}, entity::{Entities, Entity, EntityLocation}, - storage::{SparseSetIndex, SparseSets, Storages, Table}, world::archetype_invariants::ArchetypeInvariants, + storage::{SparseSetIndex, SparseSets, Storages, Table}, + world::archetype_invariants::ArchetypeInvariants, }; use bevy_ecs_macros::all_tuples; use bevy_ptr::OwningPtr; @@ -172,6 +173,7 @@ impl BundleInfo { &self.storage_types } + #[allow(clippy::too_many_arguments)] pub(crate) fn get_bundle_inserter<'a, 'b>( &'b self, entities: &'a mut Entities, @@ -182,8 +184,13 @@ impl BundleInfo { change_tick: u32, archetype_invariants: &ArchetypeInvariants, ) -> BundleInserter<'a, 'b> { - let new_archetype_id = - self.add_bundle_to_archetype(archetypes, storages, components, archetype_id, archetype_invariants); + let new_archetype_id = self.add_bundle_to_archetype( + archetypes, + storages, + components, + archetype_id, + archetype_invariants, + ); let archetypes_ptr = archetypes.archetypes.as_mut_ptr(); if new_archetype_id == archetype_id { let archetype = &mut archetypes[archetype_id]; @@ -242,8 +249,13 @@ impl BundleInfo { change_tick: u32, archetype_invariants: &ArchetypeInvariants, ) -> BundleSpawner<'a, 'b> { - let new_archetype_id = - self.add_bundle_to_archetype(archetypes, storages, components, ArchetypeId::EMPTY, archetype_invariants); + let new_archetype_id = self.add_bundle_to_archetype( + archetypes, + storages, + components, + ArchetypeId::EMPTY, + archetype_invariants, + ); let (empty_archetype, archetype) = archetypes.get_2_mut(ArchetypeId::EMPTY, new_archetype_id); let table = &mut storages.tables[archetype.table_id()]; @@ -376,8 +388,12 @@ impl BundleInfo { new_sparse_set_components }; }; - let new_archetype_id = - archetypes.get_id_or_insert(table_id, table_components, sparse_set_components, archetype_invariants); + let new_archetype_id = archetypes.get_id_or_insert( + table_id, + table_components, + sparse_set_components, + archetype_invariants, + ); // add an edge from the old archetype to the new archetype archetypes[archetype_id].edges_mut().insert_add_bundle( self.id, diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index 1d9b0380c6132..e882494e40723 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -148,14 +148,14 @@ impl UntypedArchetypeStatement { /// Get the set of [`ComponentId`]s affected by this statement pub fn component_ids(&self) -> &BTreeSet { match self { - UntypedArchetypeStatement::AllOf(set) => &set, - UntypedArchetypeStatement::AtLeastOneOf(set) => &set, - UntypedArchetypeStatement::NoneOf(set) => &set, + UntypedArchetypeStatement::AllOf(set) + | UntypedArchetypeStatement::AtLeastOneOf(set) + | UntypedArchetypeStatement::NoneOf(set) => set, } } /// Test if this statement is true for the provided set of [`ComponentId`]s - pub fn test(&self, component_ids: &HashSet) -> bool{ + pub fn test(&self, component_ids: &HashSet) -> bool { match self { UntypedArchetypeStatement::AllOf(required_ids) => { for required_id in required_ids { @@ -164,7 +164,7 @@ impl UntypedArchetypeStatement { } } true - }, + } UntypedArchetypeStatement::AtLeastOneOf(desired_ids) => { for desired_id in desired_ids { if component_ids.contains(desired_id) { @@ -172,7 +172,7 @@ impl UntypedArchetypeStatement { } } false - }, + } UntypedArchetypeStatement::NoneOf(forbidden_ids) => { for forbidden_id in forbidden_ids { if component_ids.contains(forbidden_id) { @@ -180,7 +180,7 @@ impl UntypedArchetypeStatement { } } true - }, + } } } } @@ -197,7 +197,7 @@ impl ArchetypeInvariants { /// Whenever a new archetype invariant is added, all existing archetypes are re-checked. /// This may include empty archetypes- archetypes that contain no entities. #[inline] - pub fn add(&mut self, archetype_invariant: UntypedArchetypeInvariant) { + pub fn add(&mut self, archetype_invariant: UntypedArchetypeInvariant) { self.raw_list.push(archetype_invariant); } @@ -207,13 +207,15 @@ impl ArchetypeInvariants { /// /// # Panics /// Panics if any archetype invariant is violated - pub(crate) fn test_archetype(&self, component_ids_of_archetype: impl Iterator) { + pub(crate) fn test_archetype( + &self, + component_ids_of_archetype: impl Iterator, + ) { let component_ids_of_archetype: HashSet = component_ids_of_archetype.collect(); - - for invariant in self.raw_list.iter() { - if - invariant.predicate.test(&component_ids_of_archetype) && - !invariant.consequence.test(&component_ids_of_archetype) + + for invariant in &self.raw_list { + if invariant.predicate.test(&component_ids_of_archetype) + && !invariant.consequence.test(&component_ids_of_archetype) { panic!("Archetype invariant violated!") } From 2f647849e600f02b43f853359c39ed19b11fe026 Mon Sep 17 00:00:00 2001 From: plof27 Date: Sun, 26 Jun 2022 10:37:01 -0400 Subject: [PATCH 13/59] Check all archetypes whenever a new invariant is added --- .../src/world/archetype_invariants.rs | 21 +++++++++++++++++++ crates/bevy_ecs/src/world/mod.rs | 8 +++++-- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index e882494e40723..4b2287ab2a874 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -130,6 +130,27 @@ pub struct UntypedArchetypeInvariant { pub consequence: UntypedArchetypeStatement, } +impl UntypedArchetypeInvariant { + + /// Assert that the provided iterator of [`ComponentId`]s obeys this archetype invariant + /// + /// `component_ids` is generally provided via the `components` field on [`Archetype`]. + /// When testing against multiple archetypes, [`ArchetypeInvariants::test_archetype`] is preferred, + /// as it can more efficiently cache checks between archetypes. + /// + /// # Panics + /// Panics if the archetype invariant is violated + pub fn test_archetype(&self, component_ids_of_archetype: impl Iterator) { + let component_ids_of_archetype: HashSet = component_ids_of_archetype.collect(); + + if self.predicate.test(&component_ids_of_archetype) + && !self.consequence.test(&component_ids_of_archetype) + { + panic!("Archetype invariant violated!") + } + } +} + /// A type-erased version of [`ArchetypeStatment`] /// Intended to be used with dynamic components that cannot be represented with Rust types. /// Prefer [`ArchetypeStatment`] when possible. diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index eda885203cc95..8044448bab252 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -688,9 +688,13 @@ impl World { archetype_invariant: ArchetypeInvariant, ) { let untyped_invariant = archetype_invariant.into_untyped(self); - self.archetype_invariants.add(untyped_invariant); + + for archetype in &self.archetypes.archetypes { + let components = archetype.components.indices(); + untyped_invariant.test_archetype(components); + } - todo!("Check all archetypes against the new invariant"); + self.archetype_invariants.add(untyped_invariant); } /// Inserts a new [`UntypedArchetypeInvariant`] to the world From d9a38c8cf6452d18f8a38d770e5a904099585758 Mon Sep 17 00:00:00 2001 From: plof27 Date: Sun, 26 Jun 2022 11:07:32 -0400 Subject: [PATCH 14/59] Improve error messages on archetype invariant failure There's still more work to do improving these. In particular, it would be good to output the Rust type names (where available) --- .../src/world/archetype_invariants.rs | 41 +++++++++++++++++-- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index 4b2287ab2a874..4be837aafed1b 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -131,11 +131,10 @@ pub struct UntypedArchetypeInvariant { } impl UntypedArchetypeInvariant { - /// Assert that the provided iterator of [`ComponentId`]s obeys this archetype invariant /// /// `component_ids` is generally provided via the `components` field on [`Archetype`]. - /// When testing against multiple archetypes, [`ArchetypeInvariants::test_archetype`] is preferred, + /// When testing against multiple archetypes, [`ArchetypeInvariants::test_archetype`] is preferred, /// as it can more efficiently cache checks between archetypes. /// /// # Panics @@ -146,7 +145,10 @@ impl UntypedArchetypeInvariant { if self.predicate.test(&component_ids_of_archetype) && !self.consequence.test(&component_ids_of_archetype) { - panic!("Archetype invariant violated!") + panic!( + "Archetype invariant violated! The invariant {:?} failed for archetype {:?}", + self, component_ids_of_archetype + ); } } } @@ -238,7 +240,21 @@ impl ArchetypeInvariants { if invariant.predicate.test(&component_ids_of_archetype) && !invariant.consequence.test(&component_ids_of_archetype) { - panic!("Archetype invariant violated!") + let mut failed_invariants = vec![]; + + for invariant in &self.raw_list { + if invariant.predicate.test(&component_ids_of_archetype) + && !invariant.consequence.test(&component_ids_of_archetype) + { + failed_invariants.push(invariant.clone()); + } + } + + panic!( + "Archetype invariant violated! The following invariants were violated for archetype {:?}:\n{:?}", + component_ids_of_archetype, + failed_invariants, + ) } } } @@ -268,6 +284,14 @@ mod tests { world.spawn().insert_bundle((A, B, C)); } + #[test] + fn full_bundle_on_insert_happy() { + let mut world = World::new(); + + world.spawn().insert_bundle((A, B, C)); + world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::full_bundle()); + } + #[test] #[should_panic] fn full_bundle_sad() { @@ -276,4 +300,13 @@ mod tests { world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::full_bundle()); world.spawn().insert_bundle((A, B)); } + + #[test] + #[should_panic] + fn full_bundle_on_insert_sad() { + let mut world = World::new(); + + world.spawn().insert_bundle((A, B)); + world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::full_bundle()); + } } From 6f75a66d9aa8579399a0b5d4a992d1cc3bb20b20 Mon Sep 17 00:00:00 2001 From: plof27 Date: Sun, 26 Jun 2022 11:57:50 -0400 Subject: [PATCH 15/59] Change `BTreeSet`s to use `HashSet` instead There is no need for a sorted set here. --- crates/bevy_ecs/src/world/archetype_invariants.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index 4be837aafed1b..53bc5ca7b7e91 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -1,7 +1,6 @@ use std::marker::PhantomData; use bevy_utils::{tracing::warn, HashSet}; -use std::collections::BTreeSet; use crate::{component::ComponentId, prelude::Bundle, world::World}; @@ -86,7 +85,7 @@ impl ArchetypeStatement { /// Requires mutable world access, since the components might not have been added to the world yet. pub fn into_untyped(self, world: &mut World) -> UntypedArchetypeStatement { let component_ids = B::component_ids(&mut world.components, &mut world.storages); - let component_ids: BTreeSet = component_ids.into_iter().collect(); + let component_ids: HashSet = component_ids.into_iter().collect(); match self { ArchetypeStatement::AllOf(_) => UntypedArchetypeStatement::AllOf(component_ids), @@ -159,17 +158,17 @@ impl UntypedArchetypeInvariant { #[derive(Clone, Debug, PartialEq)] pub enum UntypedArchetypeStatement { /// Evaluates to true if and only if the entity has all of the components present in the set - AllOf(BTreeSet), + AllOf(HashSet), /// The entity has at least one component in the set, and may have all of them. /// When using a single-component bundle, `AllOf` is preferred - AtLeastOneOf(BTreeSet), + AtLeastOneOf(HashSet), /// The entity has none of the components in the set - NoneOf(BTreeSet), + NoneOf(HashSet), } impl UntypedArchetypeStatement { /// Get the set of [`ComponentId`]s affected by this statement - pub fn component_ids(&self) -> &BTreeSet { + pub fn component_ids(&self) -> &HashSet { match self { UntypedArchetypeStatement::AllOf(set) | UntypedArchetypeStatement::AtLeastOneOf(set) From 541111f263c3e11832bc40cbe297eedd3524a388 Mon Sep 17 00:00:00 2001 From: plof27 Date: Mon, 27 Jun 2022 16:39:21 -0400 Subject: [PATCH 16/59] Add `AtMostOneOf` variant This makes mutually exclusive bundles much easier to specify --- .../src/world/archetype_invariants.rs | 30 ++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index 53bc5ca7b7e91..b5f49fe639352 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -75,6 +75,9 @@ pub enum ArchetypeStatement { /// The entity has at least one component in the bundle `B`, and may have all of them. /// When using a single-component bundle, `AllOf` is preferred. AtLeastOneOf(PhantomData), + /// The entity has zero or one of the components in the bundle `B`, and may have all of them. + /// When using a single-component bundle, this is a tautology. + AtMostOneOf(PhantomData), /// The entity has none of the components in the bundle `B` NoneOf(PhantomData), } @@ -95,6 +98,7 @@ impl ArchetypeStatement { } UntypedArchetypeStatement::AtLeastOneOf(component_ids) } + ArchetypeStatement::AtMostOneOf(_) => UntypedArchetypeStatement::AtMostOneOf(component_ids), ArchetypeStatement::NoneOf(_) => UntypedArchetypeStatement::NoneOf(component_ids), } } @@ -111,6 +115,12 @@ impl ArchetypeStatement { ArchetypeStatement::AtLeastOneOf(PhantomData) } + /// Constructs a new [`ArchetypeStatement::AtMostOneOf`] variant for all components stored in the bundle `B` + #[inline] + pub const fn at_most_one_of() -> Self { + ArchetypeStatement::AtMostOneOf(PhantomData) + } + /// Constructs a new [`ArchetypeStatement::NoneOf`] variant for all components stored in the bundle `B` #[inline] pub const fn none_of() -> Self { @@ -119,6 +129,7 @@ impl ArchetypeStatement { } /// A type-erased version of [`ArchetypeInvariant`]. +/// /// Intended to be used with dynamic components that cannot be represented with Rust types. /// Prefer [`ArchetypeInvariant`] when possible. #[derive(Clone, Debug, PartialEq)] @@ -160,8 +171,11 @@ pub enum UntypedArchetypeStatement { /// Evaluates to true if and only if the entity has all of the components present in the set AllOf(HashSet), /// The entity has at least one component in the set, and may have all of them. - /// When using a single-component bundle, `AllOf` is preferred + /// When using a single-component set, `AllOf` is preferred AtLeastOneOf(HashSet), + /// The entity has zero or one of the components in the set, and may have all of them. + /// When using a single-component set, this is a tautology. + AtMostOneOf(HashSet), /// The entity has none of the components in the set NoneOf(HashSet), } @@ -172,6 +186,7 @@ impl UntypedArchetypeStatement { match self { UntypedArchetypeStatement::AllOf(set) | UntypedArchetypeStatement::AtLeastOneOf(set) + | UntypedArchetypeStatement::AtMostOneOf(set) | UntypedArchetypeStatement::NoneOf(set) => set, } } @@ -195,6 +210,19 @@ impl UntypedArchetypeStatement { } false } + UntypedArchetypeStatement::AtMostOneOf(exclusive_ids) => { + let mut found_previous = false; + for exclusive_id in exclusive_ids { + if component_ids.contains(exclusive_id) { + if found_previous { + return false + } else { + found_previous = true; + } + } + } + true + } UntypedArchetypeStatement::NoneOf(forbidden_ids) => { for forbidden_id in forbidden_ids { if component_ids.contains(forbidden_id) { From 6914e064f803319b3e8468b68aef4072fc1e9ba7 Mon Sep 17 00:00:00 2001 From: plof27 Date: Mon, 27 Jun 2022 18:15:48 -0400 Subject: [PATCH 17/59] Run cargo fmt and cargo clippy --- crates/bevy_ecs/src/world/archetype_invariants.rs | 11 ++++++----- crates/bevy_ecs/src/world/mod.rs | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index b5f49fe639352..b2f88b256dd32 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -98,7 +98,9 @@ impl ArchetypeStatement { } UntypedArchetypeStatement::AtLeastOneOf(component_ids) } - ArchetypeStatement::AtMostOneOf(_) => UntypedArchetypeStatement::AtMostOneOf(component_ids), + ArchetypeStatement::AtMostOneOf(_) => { + UntypedArchetypeStatement::AtMostOneOf(component_ids) + } ArchetypeStatement::NoneOf(_) => UntypedArchetypeStatement::NoneOf(component_ids), } } @@ -129,7 +131,7 @@ impl ArchetypeStatement { } /// A type-erased version of [`ArchetypeInvariant`]. -/// +/// /// Intended to be used with dynamic components that cannot be represented with Rust types. /// Prefer [`ArchetypeInvariant`] when possible. #[derive(Clone, Debug, PartialEq)] @@ -215,10 +217,9 @@ impl UntypedArchetypeStatement { for exclusive_id in exclusive_ids { if component_ids.contains(exclusive_id) { if found_previous { - return false - } else { - found_previous = true; + return false; } + found_previous = true; } } true diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 8044448bab252..3e971356ba038 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -688,7 +688,7 @@ impl World { archetype_invariant: ArchetypeInvariant, ) { let untyped_invariant = archetype_invariant.into_untyped(self); - + for archetype in &self.archetypes.archetypes { let components = archetype.components.indices(); untyped_invariant.test_archetype(components); From 5b82d092ea531bcfb9d45c139f680127ee0c6309 Mon Sep 17 00:00:00 2001 From: plof27 Date: Mon, 27 Jun 2022 22:10:46 -0400 Subject: [PATCH 18/59] Fix docstring type references --- .../src/world/archetype_invariants.rs | 23 ++++++++----------- crates/bevy_ecs/src/world/mod.rs | 2 +- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index b2f88b256dd32..a8c582857dc35 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -4,7 +4,7 @@ use bevy_utils::{tracing::warn, HashSet}; use crate::{component::ComponentId, prelude::Bundle, world::World}; -/// A rule about which [`Component`]s can coexist on entities +/// A rule about which [`Component`](crate::component::Component)s can coexist on entities /// /// These rules must be true at all times for all entities in the [`World`]. /// The generic [`Bundle`] type `B1` is always used in the `predicate`, @@ -13,7 +13,7 @@ use crate::{component::ComponentId, prelude::Bundle, world::World}; /// /// When added to the [`World`], archetype invariants behave like [`assert!`]; /// all archetype invariants must be true for every entity in the [`World`]. -/// Archetype invariants are checked each time [`Archetypes`] is modified; +/// Archetype invariants are checked each time [`Archetypes`](crate::archetype::Archetypes) is modified; /// this can occur on component addition, component removal, and entity spawning. /// /// Archetypes are only modified when a novel archetype (set of components) is seen for the first time; @@ -66,7 +66,7 @@ impl ArchetypeInvariant { /// For single component bundles, `AllOf` and `AtLeastOneOf` are equivalent. /// Prefer `ArchetypeStatement::<(C,)>::all_of` over `ArchetypeStatement::<(C,)>::at_least_one_of` for consistency and clarity. /// -/// Note that this is converted to an [`UntypedArchetypeStatment`] when added to a [`World`]. +/// Note that this is converted to an [`UntypedArchetypeStatement`] when added to a [`World`]. /// This is to ensure compatibility between different invariants. #[derive(Clone, Debug, PartialEq)] pub enum ArchetypeStatement { @@ -75,7 +75,7 @@ pub enum ArchetypeStatement { /// The entity has at least one component in the bundle `B`, and may have all of them. /// When using a single-component bundle, `AllOf` is preferred. AtLeastOneOf(PhantomData), - /// The entity has zero or one of the components in the bundle `B`, and may have all of them. + /// The entity has zero or one of the components in the bundle `B`, but no more. /// When using a single-component bundle, this is a tautology. AtMostOneOf(PhantomData), /// The entity has none of the components in the bundle `B` @@ -145,7 +145,7 @@ pub struct UntypedArchetypeInvariant { impl UntypedArchetypeInvariant { /// Assert that the provided iterator of [`ComponentId`]s obeys this archetype invariant /// - /// `component_ids` is generally provided via the `components` field on [`Archetype`]. + /// `component_ids` is generally provided via the `components` field on [`Archetype`](crate::archetype::Archetype). /// When testing against multiple archetypes, [`ArchetypeInvariants::test_archetype`] is preferred, /// as it can more efficiently cache checks between archetypes. /// @@ -165,9 +165,9 @@ impl UntypedArchetypeInvariant { } } -/// A type-erased version of [`ArchetypeStatment`] +/// A type-erased version of [`ArchetypeStatement`] /// Intended to be used with dynamic components that cannot be represented with Rust types. -/// Prefer [`ArchetypeStatment`] when possible. +/// Prefer [`ArchetypeStatement`] when possible. #[derive(Clone, Debug, PartialEq)] pub enum UntypedArchetypeStatement { /// Evaluates to true if and only if the entity has all of the components present in the set @@ -175,7 +175,7 @@ pub enum UntypedArchetypeStatement { /// The entity has at least one component in the set, and may have all of them. /// When using a single-component set, `AllOf` is preferred AtLeastOneOf(HashSet), - /// The entity has zero or one of the components in the set, and may have all of them. + /// The entity has zero or one of the components in the set, but no more. /// When using a single-component set, this is a tautology. AtMostOneOf(HashSet), /// The entity has none of the components in the set @@ -254,14 +254,11 @@ impl ArchetypeInvariants { /// Assert that the provided iterator of [`ComponentId`]s obeys all archetype invariants /// - /// `component_ids` is generally provided via the `components` field on [`Archetype`]. + /// `component_ids` is generally provided via the `components` field on [`Archetype`](crate::archetype::Archetype). /// /// # Panics /// Panics if any archetype invariant is violated - pub(crate) fn test_archetype( - &self, - component_ids_of_archetype: impl Iterator, - ) { + pub fn test_archetype(&self, component_ids_of_archetype: impl Iterator) { let component_ids_of_archetype: HashSet = component_ids_of_archetype.collect(); for invariant in &self.raw_list { diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 3e971356ba038..aee5640cbad03 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -701,7 +701,7 @@ impl World { /// /// Whenever a new archetype invariant is added, all existing archetypes are re-checked. /// This may include empty archetypes- archetypes that contain no entities. - /// Prefer [`add_archetype_invariant`](World::add_archertype_invariant) where possible. + /// Prefer [`add_archetype_invariant`](World::add_archetype_invariant) where possible. #[inline] pub fn add_untyped_archetype_invariant( &mut self, From 2776b484287dfce8e2e792a7a9b42cb35ab78b7c Mon Sep 17 00:00:00 2001 From: Rose Peck Date: Thu, 30 Jun 2022 17:56:21 -0700 Subject: [PATCH 19/59] Apply documentation suggestions from code review Co-authored-by: Federico Rinaldi --- crates/bevy_ecs/src/component.rs | 2 +- .../src/world/archetype_invariants.rs | 53 +++++++++---------- crates/bevy_ecs/src/world/mod.rs | 11 ++-- 3 files changed, 31 insertions(+), 35 deletions(-) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 90d376b34bdf5..b26c932a6cc74 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -372,7 +372,7 @@ pub struct Components { impl Components { /// Adds a new component type to [`Components`]. /// - /// If the component type is already present, then simply return its [`ComponentId`]. + /// If the component type is already present, it simply returns its [`ComponentId`]. #[inline] pub fn init_component(&mut self, storages: &mut Storages) -> ComponentId { let type_id = TypeId::of::(); diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index a8c582857dc35..263731929755a 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -4,7 +4,7 @@ use bevy_utils::{tracing::warn, HashSet}; use crate::{component::ComponentId, prelude::Bundle, world::World}; -/// A rule about which [`Component`](crate::component::Component)s can coexist on entities +/// A rule about which [`Component`](crate::component::Component)s can coexist on entities. /// /// These rules must be true at all times for all entities in the [`World`]. /// The generic [`Bundle`] type `B1` is always used in the `predicate`, @@ -18,9 +18,6 @@ use crate::{component::ComponentId, prelude::Bundle, world::World}; /// /// Archetypes are only modified when a novel archetype (set of components) is seen for the first time; /// swapping between existing archetypes will not trigger these checks. -/// -/// Note that this is converted to an [`UntypedArchetypeInvariant`] when added to a [`World`]. -/// This is to ensure compatibility between different invariants. #[derive(Clone, Debug, PartialEq)] pub struct ArchetypeInvariant { /// For all entities where the predicate is true @@ -43,9 +40,9 @@ impl ArchetypeInvariant { } impl ArchetypeInvariant { - /// This is a helper function for constructing common invariants. - /// All components of the provided bundle require each other. - /// In other words, if any one component of this bundle is present, then all of them must be. + /// Creates an archetype invariant where all components of `B` require each other. + /// + /// In other words, if any component of this bundle is present, then all of them must be. #[inline] pub fn full_bundle() -> Self { Self { @@ -70,20 +67,20 @@ impl ArchetypeInvariant { /// This is to ensure compatibility between different invariants. #[derive(Clone, Debug, PartialEq)] pub enum ArchetypeStatement { - /// Evaluates to true if and only if the entity has all of the components present in the bundle `B` + /// Evaluates to true if and only if the entity has all of the components present in the bundle `B`. AllOf(PhantomData), - /// The entity has at least one component in the bundle `B`, and may have all of them. + /// The entity has at least one component in the bundle `B`. /// When using a single-component bundle, `AllOf` is preferred. AtLeastOneOf(PhantomData), /// The entity has zero or one of the components in the bundle `B`, but no more. - /// When using a single-component bundle, this is a tautology. + /// When using a single-component bundle, this will always be true. AtMostOneOf(PhantomData), - /// The entity has none of the components in the bundle `B` + /// The entity has none of the components in the bundle `B`. NoneOf(PhantomData), } impl ArchetypeStatement { - /// Erases the type information of this archetype statment. + /// Erases the type information of this archetype statement. /// /// Requires mutable world access, since the components might not have been added to the world yet. pub fn into_untyped(self, world: &mut World) -> UntypedArchetypeStatement { @@ -105,25 +102,25 @@ impl ArchetypeStatement { } } - /// Constructs a new [`ArchetypeStatement::AllOf`] variant for all components stored in the bundle `B` + /// Constructs a new [`ArchetypeStatement::AllOf`] variant for all components stored in the bundle `B`. #[inline] pub const fn all_of() -> Self { ArchetypeStatement::AllOf(PhantomData) } - /// Constructs a new [`ArchetypeStatement::AtLeastOneOf`] variant for all components stored in the bundle `B` + /// Constructs a new [`ArchetypeStatement::AtLeastOneOf`] variant for all components stored in the bundle `B`. #[inline] pub const fn at_least_one_of() -> Self { ArchetypeStatement::AtLeastOneOf(PhantomData) } - /// Constructs a new [`ArchetypeStatement::AtMostOneOf`] variant for all components stored in the bundle `B` + /// Constructs a new [`ArchetypeStatement::AtMostOneOf`] variant for all components stored in the bundle `B`. #[inline] pub const fn at_most_one_of() -> Self { ArchetypeStatement::AtMostOneOf(PhantomData) } - /// Constructs a new [`ArchetypeStatement::NoneOf`] variant for all components stored in the bundle `B` + /// Constructs a new [`ArchetypeStatement::NoneOf`] variant for all components stored in the bundle `B`. #[inline] pub const fn none_of() -> Self { ArchetypeStatement::NoneOf(PhantomData) @@ -143,14 +140,14 @@ pub struct UntypedArchetypeInvariant { } impl UntypedArchetypeInvariant { - /// Assert that the provided iterator of [`ComponentId`]s obeys this archetype invariant + /// Asserts that the provided iterator of [`ComponentId`]s obeys this archetype invariant. /// /// `component_ids` is generally provided via the `components` field on [`Archetype`](crate::archetype::Archetype). /// When testing against multiple archetypes, [`ArchetypeInvariants::test_archetype`] is preferred, /// as it can more efficiently cache checks between archetypes. /// /// # Panics - /// Panics if the archetype invariant is violated + /// Panics if the archetype invariant is violated. pub fn test_archetype(&self, component_ids_of_archetype: impl Iterator) { let component_ids_of_archetype: HashSet = component_ids_of_archetype.collect(); @@ -165,20 +162,21 @@ impl UntypedArchetypeInvariant { } } -/// A type-erased version of [`ArchetypeStatement`] +/// A type-erased version of [`ArchetypeStatement`]. +/// /// Intended to be used with dynamic components that cannot be represented with Rust types. /// Prefer [`ArchetypeStatement`] when possible. #[derive(Clone, Debug, PartialEq)] pub enum UntypedArchetypeStatement { - /// Evaluates to true if and only if the entity has all of the components present in the set + /// Evaluates to true if and only if the entity has all of the components present in the set. AllOf(HashSet), /// The entity has at least one component in the set, and may have all of them. - /// When using a single-component set, `AllOf` is preferred + /// When using a single-component set, `AllOf` is preferred. AtLeastOneOf(HashSet), /// The entity has zero or one of the components in the set, but no more. /// When using a single-component set, this is a tautology. AtMostOneOf(HashSet), - /// The entity has none of the components in the set + /// The entity has none of the components in the set. NoneOf(HashSet), } @@ -193,7 +191,7 @@ impl UntypedArchetypeStatement { } } - /// Test if this statement is true for the provided set of [`ComponentId`]s + /// Test if this statement is true for the provided set of [`ComponentId`]s. pub fn test(&self, component_ids: &HashSet) -> bool { match self { UntypedArchetypeStatement::AllOf(required_ids) => { @@ -238,7 +236,7 @@ impl UntypedArchetypeStatement { #[derive(Default)] pub struct ArchetypeInvariants { - /// The list of invariants that must be upheld + /// The list of invariants that must be upheld. raw_list: Vec, } @@ -246,18 +244,19 @@ impl ArchetypeInvariants { /// Adds a new [`ArchetypeInvariant`] to this set of archetype invariants. /// /// Whenever a new archetype invariant is added, all existing archetypes are re-checked. - /// This may include empty archetypes- archetypes that contain no entities. + /// This may include empty archetypes: archetypes that contain no entities. #[inline] pub fn add(&mut self, archetype_invariant: UntypedArchetypeInvariant) { self.raw_list.push(archetype_invariant); } - /// Assert that the provided iterator of [`ComponentId`]s obeys all archetype invariants + /// Asserts that the provided iterator of [`ComponentId`]s obeys all archetype invariants. /// /// `component_ids` is generally provided via the `components` field on [`Archetype`](crate::archetype::Archetype). /// /// # Panics - /// Panics if any archetype invariant is violated + /// + /// Panics if any archetype invariant is violated. pub fn test_archetype(&self, component_ids_of_archetype: impl Iterator) { let component_ids_of_archetype: HashSet = component_ids_of_archetype.collect(); diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index aee5640cbad03..e75e243917070 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -1,4 +1,4 @@ -pub mod archetype_invariants; +mod archetype_invariants; mod entity_ref; mod spawn_batch; mod world_cell; @@ -7,10 +7,7 @@ pub use crate::change_detection::Mut; pub use entity_ref::*; pub use spawn_batch::*; pub use world_cell::*; - -use self::archetype_invariants::{ - ArchetypeInvariant, ArchetypeInvariants, UntypedArchetypeInvariant, -}; +pub use archetype_invariants::*; use crate::{ archetype::{ArchetypeComponentId, ArchetypeComponentInfo, ArchetypeId, Archetypes}, @@ -160,7 +157,7 @@ impl World { &self.archetypes } - /// Retrieves this world's [`ArchetypeInvariants`] collection + /// Retrieves this world's [`ArchetypeInvariants`] collection. #[inline] pub fn archetype_invariants(&self) -> &ArchetypeInvariants { &self.archetype_invariants @@ -697,7 +694,7 @@ impl World { self.archetype_invariants.add(untyped_invariant); } - /// Inserts a new [`UntypedArchetypeInvariant`] to the world + /// Inserts a new [`UntypedArchetypeInvariant`] to the world. /// /// Whenever a new archetype invariant is added, all existing archetypes are re-checked. /// This may include empty archetypes- archetypes that contain no entities. From 347db392ef889e9ac5bd89580daec6693c621f5d Mon Sep 17 00:00:00 2001 From: plof27 Date: Thu, 30 Jun 2022 21:20:41 -0400 Subject: [PATCH 20/59] Improve docstrings and names Thanks Nilirad! --- .../src/world/archetype_invariants.rs | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index 263731929755a..cfd91957a8f2b 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -11,8 +11,7 @@ use crate::{component::ComponentId, prelude::Bundle, world::World}; /// while `B2` is used in the `consequence`. /// If only a single generic is provided, these types are the same. /// -/// When added to the [`World`], archetype invariants behave like [`assert!`]; -/// all archetype invariants must be true for every entity in the [`World`]. +/// When added to the [`World`], archetype invariants behave like [`assert!`]. /// Archetype invariants are checked each time [`Archetypes`](crate::archetype::Archetypes) is modified; /// this can occur on component addition, component removal, and entity spawning. /// @@ -20,9 +19,11 @@ use crate::{component::ComponentId, prelude::Bundle, world::World}; /// swapping between existing archetypes will not trigger these checks. #[derive(Clone, Debug, PartialEq)] pub struct ArchetypeInvariant { - /// For all entities where the predicate is true + /// Defines which entities this invariant applies to. + /// This is the "if" of the if/then clause. pub predicate: ArchetypeStatement, - /// The consequence must also be true + /// Defines what must be true for the entities that this invariant applies to. + /// This is the "then" of the if/then clause. pub consequence: ArchetypeStatement, } @@ -44,7 +45,7 @@ impl ArchetypeInvariant { /// /// In other words, if any component of this bundle is present, then all of them must be. #[inline] - pub fn full_bundle() -> Self { + pub fn atomic_bundle() -> Self { Self { predicate: ArchetypeStatement::::at_least_one_of(), consequence: ArchetypeStatement::::all_of(), @@ -234,6 +235,7 @@ impl UntypedArchetypeStatement { } } +/// A list of [`ArchetypeInvariant`]s to be stored on a [`World`]. #[derive(Default)] pub struct ArchetypeInvariants { /// The list of invariants that must be upheld. @@ -304,7 +306,7 @@ mod tests { fn full_bundle_happy() { let mut world = World::new(); - world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::full_bundle()); + world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::atomic_bundle()); world.spawn().insert_bundle((A, B, C)); } @@ -313,7 +315,7 @@ mod tests { let mut world = World::new(); world.spawn().insert_bundle((A, B, C)); - world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::full_bundle()); + world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::atomic_bundle()); } #[test] @@ -321,7 +323,7 @@ mod tests { fn full_bundle_sad() { let mut world = World::new(); - world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::full_bundle()); + world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::atomic_bundle()); world.spawn().insert_bundle((A, B)); } @@ -331,6 +333,6 @@ mod tests { let mut world = World::new(); world.spawn().insert_bundle((A, B)); - world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::full_bundle()); + world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::atomic_bundle()); } } From 5eceedb0dcbc209fb7cb6243d66bfbaf7a9bcc65 Mon Sep 17 00:00:00 2001 From: plof27 Date: Thu, 30 Jun 2022 21:31:21 -0400 Subject: [PATCH 21/59] Add `Only` variant to `ArchetypeStatement` --- .../src/world/archetype_invariants.rs | 24 +++++++++++++++++-- crates/bevy_ecs/src/world/mod.rs | 2 +- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index cfd91957a8f2b..631228db6d19d 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -19,7 +19,7 @@ use crate::{component::ComponentId, prelude::Bundle, world::World}; /// swapping between existing archetypes will not trigger these checks. #[derive(Clone, Debug, PartialEq)] pub struct ArchetypeInvariant { - /// Defines which entities this invariant applies to. + /// Defines which entities this invariant applies to. /// This is the "if" of the if/then clause. pub predicate: ArchetypeStatement, /// Defines what must be true for the entities that this invariant applies to. @@ -78,6 +78,8 @@ pub enum ArchetypeStatement { AtMostOneOf(PhantomData), /// The entity has none of the components in the bundle `B`. NoneOf(PhantomData), + /// The entity contains only components from the bundle `B`, and no others. + Only(PhantomData), } impl ArchetypeStatement { @@ -100,6 +102,7 @@ impl ArchetypeStatement { UntypedArchetypeStatement::AtMostOneOf(component_ids) } ArchetypeStatement::NoneOf(_) => UntypedArchetypeStatement::NoneOf(component_ids), + ArchetypeStatement::Only(_) => UntypedArchetypeStatement::Only(component_ids), } } @@ -126,6 +129,12 @@ impl ArchetypeStatement { pub const fn none_of() -> Self { ArchetypeStatement::NoneOf(PhantomData) } + + /// Constructs a new [`ArchetypeStatement::Only`] variant for all components stored in the bundle `B`. + #[inline] + pub const fn only() -> Self { + ArchetypeStatement::Only(PhantomData) + } } /// A type-erased version of [`ArchetypeInvariant`]. @@ -179,6 +188,8 @@ pub enum UntypedArchetypeStatement { AtMostOneOf(HashSet), /// The entity has none of the components in the set. NoneOf(HashSet), + /// The entity contains only components from the bundle `B`, and no others. + Only(HashSet), } impl UntypedArchetypeStatement { @@ -188,7 +199,8 @@ impl UntypedArchetypeStatement { UntypedArchetypeStatement::AllOf(set) | UntypedArchetypeStatement::AtLeastOneOf(set) | UntypedArchetypeStatement::AtMostOneOf(set) - | UntypedArchetypeStatement::NoneOf(set) => set, + | UntypedArchetypeStatement::NoneOf(set) + | UntypedArchetypeStatement::Only(set) => set, } } @@ -231,6 +243,14 @@ impl UntypedArchetypeStatement { } true } + UntypedArchetypeStatement::Only(only_ids) => { + for component_id in component_ids { + if !only_ids.contains(component_id) { + return false; + } + } + true + } } } } diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index e75e243917070..8a912a3b8e600 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -4,10 +4,10 @@ mod spawn_batch; mod world_cell; pub use crate::change_detection::Mut; +pub use archetype_invariants::*; pub use entity_ref::*; pub use spawn_batch::*; pub use world_cell::*; -pub use archetype_invariants::*; use crate::{ archetype::{ArchetypeComponentId, ArchetypeComponentInfo, ArchetypeId, Archetypes}, From 67ff12ede5d1b64bd4b9ea930a0a7af9b2cf6d55 Mon Sep 17 00:00:00 2001 From: plof27 Date: Thu, 30 Jun 2022 21:41:13 -0400 Subject: [PATCH 22/59] Fix module imports --- crates/bevy_ecs/src/archetype.rs | 2 +- crates/bevy_ecs/src/bundle.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/archetype.rs b/crates/bevy_ecs/src/archetype.rs index 8bcff71bf0f01..a2bd4d4332ba1 100644 --- a/crates/bevy_ecs/src/archetype.rs +++ b/crates/bevy_ecs/src/archetype.rs @@ -6,7 +6,7 @@ use crate::{ component::{ComponentId, StorageType}, entity::{Entity, EntityLocation}, storage::{Column, SparseArray, SparseSet, SparseSetIndex, TableId}, - world::archetype_invariants::ArchetypeInvariants, + world::ArchetypeInvariants, }; use std::{ collections::HashMap, diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 993122e6c56e6..96422b810117a 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -9,7 +9,7 @@ use crate::{ component::{Component, ComponentId, ComponentTicks, Components, StorageType}, entity::{Entities, Entity, EntityLocation}, storage::{SparseSetIndex, SparseSets, Storages, Table}, - world::archetype_invariants::ArchetypeInvariants, + world::ArchetypeInvariants, }; use bevy_ecs_macros::all_tuples; use bevy_ptr::OwningPtr; From 7a5d757a08ddcb6f93b1c15e6d3da573633de279 Mon Sep 17 00:00:00 2001 From: Rose Peck Date: Sat, 2 Jul 2022 04:57:38 -0700 Subject: [PATCH 23/59] Fix test names Co-authored-by: Federico Rinaldi --- crates/bevy_ecs/src/world/archetype_invariants.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index 631228db6d19d..efb9e59e7ad74 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -323,7 +323,7 @@ mod tests { struct C; #[test] - fn full_bundle_happy() { + fn atomic_bundle_happy() { let mut world = World::new(); world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::atomic_bundle()); @@ -331,7 +331,7 @@ mod tests { } #[test] - fn full_bundle_on_insert_happy() { + fn atomic_bundle_on_insert_happy() { let mut world = World::new(); world.spawn().insert_bundle((A, B, C)); @@ -340,7 +340,7 @@ mod tests { #[test] #[should_panic] - fn full_bundle_sad() { + fn atomic_bundle_sad() { let mut world = World::new(); world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::atomic_bundle()); @@ -349,7 +349,7 @@ mod tests { #[test] #[should_panic] - fn full_bundle_on_insert_sad() { + fn atomic_bundle_on_insert_sad() { let mut world = World::new(); world.spawn().insert_bundle((A, B)); From b10e954f70f06176e1c4479845bd21ba6c28ee93 Mon Sep 17 00:00:00 2001 From: plof27 Date: Sat, 2 Jul 2022 08:07:57 -0400 Subject: [PATCH 24/59] Fix docstring for `ArchetypeInvariants.add` --- crates/bevy_ecs/src/world/archetype_invariants.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index efb9e59e7ad74..66360984b6a1d 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -264,9 +264,6 @@ pub struct ArchetypeInvariants { impl ArchetypeInvariants { /// Adds a new [`ArchetypeInvariant`] to this set of archetype invariants. - /// - /// Whenever a new archetype invariant is added, all existing archetypes are re-checked. - /// This may include empty archetypes: archetypes that contain no entities. #[inline] pub fn add(&mut self, archetype_invariant: UntypedArchetypeInvariant) { self.raw_list.push(archetype_invariant); From 2bc2cfae17880b03cc9f93b2a9d88c45dc10d7dd Mon Sep 17 00:00:00 2001 From: plof27 Date: Sat, 2 Jul 2022 08:18:03 -0400 Subject: [PATCH 25/59] Change `predicate` to `premise` --- .../src/world/archetype_invariants.rs | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index 66360984b6a1d..801c47de68293 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -7,7 +7,7 @@ use crate::{component::ComponentId, prelude::Bundle, world::World}; /// A rule about which [`Component`](crate::component::Component)s can coexist on entities. /// /// These rules must be true at all times for all entities in the [`World`]. -/// The generic [`Bundle`] type `B1` is always used in the `predicate`, +/// The generic [`Bundle`] type `B1` is always used in the `premise`, /// while `B2` is used in the `consequence`. /// If only a single generic is provided, these types are the same. /// @@ -21,7 +21,7 @@ use crate::{component::ComponentId, prelude::Bundle, world::World}; pub struct ArchetypeInvariant { /// Defines which entities this invariant applies to. /// This is the "if" of the if/then clause. - pub predicate: ArchetypeStatement, + pub premise: ArchetypeStatement, /// Defines what must be true for the entities that this invariant applies to. /// This is the "then" of the if/then clause. pub consequence: ArchetypeStatement, @@ -34,7 +34,7 @@ impl ArchetypeInvariant { #[inline] pub fn into_untyped(self, world: &mut World) -> UntypedArchetypeInvariant { UntypedArchetypeInvariant { - predicate: self.predicate.into_untyped(world), + premise: self.premise.into_untyped(world), consequence: self.consequence.into_untyped(world), } } @@ -47,7 +47,7 @@ impl ArchetypeInvariant { #[inline] pub fn atomic_bundle() -> Self { Self { - predicate: ArchetypeStatement::::at_least_one_of(), + premise: ArchetypeStatement::::at_least_one_of(), consequence: ArchetypeStatement::::all_of(), } } @@ -57,8 +57,8 @@ impl ArchetypeInvariant { /// /// This type is used as part of an [`ArchetypeInvariant`]. /// -/// When used as a predicate, the archetype invariant matches all entities which satisfy the statement. -/// When used as a consquence, then the statment must be true for all entities that were matched by the predicate. +/// When used as a premise, the archetype invariant matches all entities which satisfy the statement. +/// When used as a consquence, then the statment must be true for all entities that were matched by the premise. /// /// For the statements about a single component `C`, wrap it in a single-component bundle `(C,)`. /// For single component bundles, `AllOf` and `AtLeastOneOf` are equivalent. @@ -143,9 +143,11 @@ impl ArchetypeStatement { /// Prefer [`ArchetypeInvariant`] when possible. #[derive(Clone, Debug, PartialEq)] pub struct UntypedArchetypeInvariant { - /// For all entities where the predicate is true - pub predicate: UntypedArchetypeStatement, - /// The consequence must also be true + /// Defines which entities this invariant applies to. + /// This is the "if" of the if/then clause. + pub premise: UntypedArchetypeStatement, + /// Defines what must be true for the entities that this invariant applies to. + /// This is the "then" of the if/then clause. pub consequence: UntypedArchetypeStatement, } @@ -161,7 +163,7 @@ impl UntypedArchetypeInvariant { pub fn test_archetype(&self, component_ids_of_archetype: impl Iterator) { let component_ids_of_archetype: HashSet = component_ids_of_archetype.collect(); - if self.predicate.test(&component_ids_of_archetype) + if self.premise.test(&component_ids_of_archetype) && !self.consequence.test(&component_ids_of_archetype) { panic!( @@ -280,13 +282,13 @@ impl ArchetypeInvariants { let component_ids_of_archetype: HashSet = component_ids_of_archetype.collect(); for invariant in &self.raw_list { - if invariant.predicate.test(&component_ids_of_archetype) + if invariant.premise.test(&component_ids_of_archetype) && !invariant.consequence.test(&component_ids_of_archetype) { let mut failed_invariants = vec![]; for invariant in &self.raw_list { - if invariant.predicate.test(&component_ids_of_archetype) + if invariant.premise.test(&component_ids_of_archetype) && !invariant.consequence.test(&component_ids_of_archetype) { failed_invariants.push(invariant.clone()); From a8716cc3bea1a37b73a7cfd52cb5f238466334f1 Mon Sep 17 00:00:00 2001 From: plof27 Date: Sat, 2 Jul 2022 08:21:46 -0400 Subject: [PATCH 26/59] Rename `AtLeastOneOf` to `AnyOf` --- .../src/world/archetype_invariants.rs | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index 801c47de68293..331ec78cd11f4 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -47,7 +47,7 @@ impl ArchetypeInvariant { #[inline] pub fn atomic_bundle() -> Self { Self { - premise: ArchetypeStatement::::at_least_one_of(), + premise: ArchetypeStatement::::any_of(), consequence: ArchetypeStatement::::all_of(), } } @@ -61,7 +61,7 @@ impl ArchetypeInvariant { /// When used as a consquence, then the statment must be true for all entities that were matched by the premise. /// /// For the statements about a single component `C`, wrap it in a single-component bundle `(C,)`. -/// For single component bundles, `AllOf` and `AtLeastOneOf` are equivalent. +/// For single component bundles, `AllOf` and `AnyOf` are equivalent. /// Prefer `ArchetypeStatement::<(C,)>::all_of` over `ArchetypeStatement::<(C,)>::at_least_one_of` for consistency and clarity. /// /// Note that this is converted to an [`UntypedArchetypeStatement`] when added to a [`World`]. @@ -72,7 +72,7 @@ pub enum ArchetypeStatement { AllOf(PhantomData), /// The entity has at least one component in the bundle `B`. /// When using a single-component bundle, `AllOf` is preferred. - AtLeastOneOf(PhantomData), + AnyOf(PhantomData), /// The entity has zero or one of the components in the bundle `B`, but no more. /// When using a single-component bundle, this will always be true. AtMostOneOf(PhantomData), @@ -92,11 +92,11 @@ impl ArchetypeStatement { match self { ArchetypeStatement::AllOf(_) => UntypedArchetypeStatement::AllOf(component_ids), - ArchetypeStatement::AtLeastOneOf(_) => { + ArchetypeStatement::AnyOf(_) => { if component_ids.len() == 1 { - warn!("An `ArchetypeStatement::AtLeastOneOf` was constructed for a bundle with only one component. Prefer the equivalent `ArchetypeStatment:AllOf` for consistency and clarity."); + warn!("An `ArchetypeStatement::AnyOf` was constructed for a bundle with only one component. Prefer the equivalent `ArchetypeStatment:AllOf` for consistency and clarity."); } - UntypedArchetypeStatement::AtLeastOneOf(component_ids) + UntypedArchetypeStatement::AnyOf(component_ids) } ArchetypeStatement::AtMostOneOf(_) => { UntypedArchetypeStatement::AtMostOneOf(component_ids) @@ -112,10 +112,10 @@ impl ArchetypeStatement { ArchetypeStatement::AllOf(PhantomData) } - /// Constructs a new [`ArchetypeStatement::AtLeastOneOf`] variant for all components stored in the bundle `B`. + /// Constructs a new [`ArchetypeStatement::AnyOf`] variant for all components stored in the bundle `B`. #[inline] - pub const fn at_least_one_of() -> Self { - ArchetypeStatement::AtLeastOneOf(PhantomData) + pub const fn any_of() -> Self { + ArchetypeStatement::AnyOf(PhantomData) } /// Constructs a new [`ArchetypeStatement::AtMostOneOf`] variant for all components stored in the bundle `B`. @@ -184,7 +184,7 @@ pub enum UntypedArchetypeStatement { AllOf(HashSet), /// The entity has at least one component in the set, and may have all of them. /// When using a single-component set, `AllOf` is preferred. - AtLeastOneOf(HashSet), + AnyOf(HashSet), /// The entity has zero or one of the components in the set, but no more. /// When using a single-component set, this is a tautology. AtMostOneOf(HashSet), @@ -199,7 +199,7 @@ impl UntypedArchetypeStatement { pub fn component_ids(&self) -> &HashSet { match self { UntypedArchetypeStatement::AllOf(set) - | UntypedArchetypeStatement::AtLeastOneOf(set) + | UntypedArchetypeStatement::AnyOf(set) | UntypedArchetypeStatement::AtMostOneOf(set) | UntypedArchetypeStatement::NoneOf(set) | UntypedArchetypeStatement::Only(set) => set, @@ -217,7 +217,7 @@ impl UntypedArchetypeStatement { } true } - UntypedArchetypeStatement::AtLeastOneOf(desired_ids) => { + UntypedArchetypeStatement::AnyOf(desired_ids) => { for desired_id in desired_ids { if component_ids.contains(desired_id) { return true; From 3e063854b293a5a77a3755a37e46e27cbf74cff1 Mon Sep 17 00:00:00 2001 From: plof27 Date: Sat, 2 Jul 2022 09:17:42 -0400 Subject: [PATCH 27/59] Add helper methods and tests for common invariants --- .../src/world/archetype_invariants.rs | 204 +++++++++++++++++- 1 file changed, 194 insertions(+), 10 deletions(-) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index 331ec78cd11f4..0832b908425fb 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -15,6 +15,12 @@ use crate::{component::ComponentId, prelude::Bundle, world::World}; /// Archetype invariants are checked each time [`Archetypes`](crate::archetype::Archetypes) is modified; /// this can occur on component addition, component removal, and entity spawning. /// +/// Note that archetype invariants are not symmetric by default. +/// For example, `ArchetypeInvariant::::requires_one()` means that `B1` requires `B2`, +/// but not that `B2` requires `B1`. +/// In this case, an entity with just `B2` is completely valid, but an entity with just `B1` is not. +/// If symmetry is desired, repeat the invariant with the order of the types switched. +/// /// Archetypes are only modified when a novel archetype (set of components) is seen for the first time; /// swapping between existing archetypes will not trigger these checks. #[derive(Clone, Debug, PartialEq)] @@ -38,6 +44,41 @@ impl ArchetypeInvariant { consequence: self.consequence.into_untyped(world), } } + + /// Creates an archetype invariant where any component of `B1` forbids every comonent from `B2`, and vice versa. + /// + /// In other words, if any component from `B1` is present, then none of the components from `B2` can be present. + /// Although this appears assymetric, it actually implies its own converse. + /// This is particularly useful for avoiding query conflicts. + #[inline] + pub fn forbids() -> Self { + Self { + premise: ArchetypeStatement::::any_of(), + consequence: ArchetypeStatement::::none_of(), + } + } + + /// Creates an archetype invariant where components of `B1` require all the components of `B2`. + /// + /// In other words, if any component from `B1` is present, then all of the components from `B2` must be. + #[inline] + pub fn requires_all() -> Self { + Self { + premise: ArchetypeStatement::::any_of(), + consequence: ArchetypeStatement::::all_of(), + } + } + + /// Creates an archetype invariant where any components of `B1` must appear with some components of `B2`. + /// + /// In other words, if any component from `B1` is present, then at least one component from `B2` must be. + #[inline] + pub fn requires_one() -> Self { + Self { + premise: ArchetypeStatement::::any_of(), + consequence: ArchetypeStatement::::any_of(), + } + } } impl ArchetypeInvariant { @@ -45,12 +86,35 @@ impl ArchetypeInvariant { /// /// In other words, if any component of this bundle is present, then all of them must be. #[inline] - pub fn atomic_bundle() -> Self { + pub fn atomic() -> Self { Self { premise: ArchetypeStatement::::any_of(), consequence: ArchetypeStatement::::all_of(), } } + + /// Creates an archetype where components of `B` cannot appear with each other. + /// + /// In other words, if any component of this bundle is present, then no others can be. + /// This is particularly useful for creating enum-like groups of components, such as `Dead` and `Ailve`. + #[inline] + pub fn disjoint() -> Self { + Self { + premise: ArchetypeStatement::::any_of(), + consequence: ArchetypeStatement::::at_most_one_of(), + } + } + + /// Creates an archetype invariant where components of `B` can only appear with each other. + /// + /// In other words, if any component of this bundle is present, then _only_ components from this bundle can be present. + #[inline] + pub fn exhaustive() -> Self { + Self { + premise: ArchetypeStatement::::any_of(), + consequence: ArchetypeStatement::::only(), + } + } } /// A statement about the presence or absence of some subset of components in the given [`Bundle`] @@ -62,7 +126,7 @@ impl ArchetypeInvariant { /// /// For the statements about a single component `C`, wrap it in a single-component bundle `(C,)`. /// For single component bundles, `AllOf` and `AnyOf` are equivalent. -/// Prefer `ArchetypeStatement::<(C,)>::all_of` over `ArchetypeStatement::<(C,)>::at_least_one_of` for consistency and clarity. +/// Prefer `ArchetypeStatement::<(C,)>::all_of` over `ArchetypeStatement::<(C,)>::any_of` for consistency and clarity. /// /// Note that this is converted to an [`UntypedArchetypeStatement`] when added to a [`World`]. /// This is to ensure compatibility between different invariants. @@ -322,36 +386,156 @@ mod tests { struct C; #[test] - fn atomic_bundle_happy() { + fn on_insert_happy() { let mut world = World::new(); - world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::atomic_bundle()); world.spawn().insert_bundle((A, B, C)); + world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::atomic()); } #[test] - fn atomic_bundle_on_insert_happy() { + #[should_panic] + fn on_insert_sad() { let mut world = World::new(); + world.spawn().insert_bundle((A, B)); + world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::atomic()); + } + + #[test] + fn forbids_happy() { + let mut world = World::new(); + + world.add_archetype_invariant(ArchetypeInvariant::<(A,), (B, C)>::forbids()); + world.spawn().insert(A); + world.spawn().insert_bundle((B, C)); + } + + #[test] + #[should_panic] + fn forbids_sad() { + let mut world = World::new(); + + world.add_archetype_invariant(ArchetypeInvariant::<(A,), (B, C)>::forbids()); + world.spawn().insert_bundle((A, B)); + } + + #[test] + fn requires_all_happy() { + let mut world = World::new(); + + world.add_archetype_invariant(ArchetypeInvariant::<(A,), (B, C)>::requires_all()); world.spawn().insert_bundle((A, B, C)); - world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::atomic_bundle()); + world.spawn().insert_bundle((B, C)); } #[test] #[should_panic] - fn atomic_bundle_sad() { + fn requires_all_sad_partial() { let mut world = World::new(); - world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::atomic_bundle()); + world.add_archetype_invariant(ArchetypeInvariant::<(A,), (B, C)>::requires_all()); world.spawn().insert_bundle((A, B)); } #[test] #[should_panic] - fn atomic_bundle_on_insert_sad() { + fn requires_all_sad_none() { let mut world = World::new(); + world.add_archetype_invariant(ArchetypeInvariant::<(A,), (B, C)>::requires_all()); + world.spawn().insert(A); + } + + #[test] + fn requires_one_happy() { + let mut world = World::new(); + + world.add_archetype_invariant(ArchetypeInvariant::<(A,), (B, C)>::requires_one()); + world.spawn().insert_bundle((A, B, C)); world.spawn().insert_bundle((A, B)); - world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::atomic_bundle()); + world.spawn().insert_bundle((B, C)); + } + + #[test] + #[should_panic] + fn requires_one_sad() { + let mut world = World::new(); + + world.add_archetype_invariant(ArchetypeInvariant::<(A,), (B, C)>::requires_one()); + world.spawn().insert(A); + } + + #[test] + fn atomic_happy() { + let mut world = World::new(); + + world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::atomic()); + world.spawn().insert_bundle((A, B, C)); + } + + #[test] + #[should_panic] + fn atomic_sad() { + let mut world = World::new(); + + world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::atomic()); + world.spawn().insert_bundle((A, B)); + } + + #[test] + fn disjoint_happy() { + let mut world = World::new(); + + world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::disjoint()); + world.spawn().insert(A); + world.spawn().insert(B); + world.spawn().insert(C); + } + + #[test] + #[should_panic] + fn disjoint_sad_partial() { + let mut world = World::new(); + + world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::disjoint()); + world.spawn().insert_bundle((A, B)); + } + + #[test] + #[should_panic] + fn disjoint_sad_all() { + let mut world = World::new(); + + world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::disjoint()); + world.spawn().insert_bundle((A, B, C)); + } + + #[test] + fn exhaustive_happy() { + let mut world = World::new(); + + world.add_archetype_invariant(ArchetypeInvariant::<(A, B)>::exhaustive()); + world.spawn().insert_bundle((A, B)); + world.spawn().insert(A); + world.spawn().insert(C); + } + + #[test] + #[should_panic] + fn exhaustive_sad_partial() { + let mut world = World::new(); + + world.add_archetype_invariant(ArchetypeInvariant::<(A, B)>::exhaustive()); + world.spawn().insert_bundle((A, C)); + } + + #[test] + #[should_panic] + fn exhaustive_sad_all() { + let mut world = World::new(); + + world.add_archetype_invariant(ArchetypeInvariant::<(A, B)>::exhaustive()); + world.spawn().insert_bundle((A, B, C)); } } From 702ef4e2fb10b4b45972909774dbbc5915783346 Mon Sep 17 00:00:00 2001 From: plof27 Date: Sat, 2 Jul 2022 09:25:36 -0400 Subject: [PATCH 28/59] Add `add_archetype_invariant` methods to `App` --- crates/bevy_app/src/app.rs | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index cdf1984c8b01e..e956c3bb04089 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -2,13 +2,13 @@ use crate::{CoreStage, Plugin, PluginGroup, PluginGroupBuilder, StartupSchedule, pub use bevy_derive::AppLabel; use bevy_ecs::{ event::{Event, Events}, - prelude::{FromWorld, IntoExclusiveSystem}, + prelude::{Bundle, FromWorld, IntoExclusiveSystem}, schedule::{ IntoSystemDescriptor, Schedule, ShouldRun, Stage, StageLabel, State, StateData, SystemSet, SystemStage, }, system::Resource, - world::World, + world::{ArchetypeInvariant, UntypedArchetypeInvariant, World}, }; use bevy_utils::{tracing::debug, HashMap}; use std::fmt::Debug; @@ -130,6 +130,32 @@ impl App { (runner)(app); } + /// Adds a new [`ArchetypeInvariant`] to this app's [`World`] + /// + /// Whenever a new archetype invariant is added to a world, all existing archetypes are re-checked. + /// This may include empty archetypes- archetypes that contain no entities. + pub fn add_archetype_invariant( + &mut self, + archetype_invariant: ArchetypeInvariant, + ) -> &mut Self { + self.world.add_archetype_invariant(archetype_invariant); + self + } + + /// Inserts a new [`UntypedArchetypeInvariant`] to this app's [`World`]. + /// + /// Whenever a new archetype invariant is added to a world, all existing archetypes are re-checked. + /// This may include empty archetypes- archetypes that contain no entities. + /// Prefer [`add_archetype_invariant`](App::add_archetype_invariant) where possible. + pub fn add_untyped_archetype_invariant( + &mut self, + archetype_invariant: UntypedArchetypeInvariant, + ) -> &mut Self { + self.world + .add_untyped_archetype_invariant(archetype_invariant); + self + } + /// Adds a [`Stage`] with the given `label` to the last position of the app's /// [`Schedule`]. /// From 1ff129972c8fa05b6d040b82cbfc821b45c8d24d Mon Sep 17 00:00:00 2001 From: plof27 Date: Sat, 2 Jul 2022 09:31:17 -0400 Subject: [PATCH 29/59] Add archetype rechecking when adding an untyped archetype invariant --- .../src/world/archetype_invariants.rs | 21 +++++++++++++++++++ crates/bevy_ecs/src/world/mod.rs | 5 +++++ 2 files changed, 26 insertions(+) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index 0832b908425fb..5d6336a263325 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -402,6 +402,27 @@ mod tests { world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::atomic()); } + #[test] + fn on_insert_untyped_happy() { + let mut world = World::new(); + + world.spawn().insert_bundle((A, B, C)); + let archetype_invariant = + ArchetypeInvariant::<(A, B, C)>::atomic().into_untyped(&mut world); + world.add_untyped_archetype_invariant(archetype_invariant); + } + + #[test] + #[should_panic] + fn on_insert_untyped_sad() { + let mut world = World::new(); + + world.spawn().insert_bundle((A, B)); + let archetype_invariant = + ArchetypeInvariant::<(A, B, C)>::atomic().into_untyped(&mut world); + world.add_untyped_archetype_invariant(archetype_invariant); + } + #[test] fn forbids_happy() { let mut world = World::new(); diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 8a912a3b8e600..85cc61a7467a3 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -704,6 +704,11 @@ impl World { &mut self, archetype_invariant: UntypedArchetypeInvariant, ) { + for archetype in &self.archetypes.archetypes { + let components = archetype.components.indices(); + archetype_invariant.test_archetype(components); + } + self.archetype_invariants.add(archetype_invariant); } From 5d80c9fe3d22a6a46888559629c120bd990058b4 Mon Sep 17 00:00:00 2001 From: plof27 Date: Sat, 2 Jul 2022 10:42:15 -0400 Subject: [PATCH 30/59] Improve error messages when archetypes fail Now, the Rust type names are fetched and nicely formatted, where possible. It would be good to shorten the type names, but there is a separate PR for this. --- crates/bevy_ecs/src/archetype.rs | 17 +++-- crates/bevy_ecs/src/bundle.rs | 1 + .../src/world/archetype_invariants.rs | 66 +++++++++++++++++-- crates/bevy_ecs/src/world/entity_ref.rs | 1 + 4 files changed, 74 insertions(+), 11 deletions(-) diff --git a/crates/bevy_ecs/src/archetype.rs b/crates/bevy_ecs/src/archetype.rs index a2bd4d4332ba1..d08ed711ad7cf 100644 --- a/crates/bevy_ecs/src/archetype.rs +++ b/crates/bevy_ecs/src/archetype.rs @@ -3,7 +3,7 @@ use crate::{ bundle::BundleId, - component::{ComponentId, StorageType}, + component::{ComponentId, Components, StorageType}, entity::{Entity, EntityLocation}, storage::{Column, SparseArray, SparseSet, SparseSetIndex, TableId}, world::ArchetypeInvariants, @@ -154,13 +154,14 @@ impl Archetype { table_archetype_components: Vec, sparse_set_archetype_components: Vec, archetype_invariants: &ArchetypeInvariants, + components: &Components, ) -> Self { - let mut components = + let mut component_set = SparseSet::with_capacity(table_components.len() + sparse_set_components.len()); for (component_id, archetype_component_id) in table_components.iter().zip(table_archetype_components) { - components.insert( + component_set.insert( *component_id, ArchetypeComponentInfo { storage_type: StorageType::Table, @@ -173,7 +174,7 @@ impl Archetype { .iter() .zip(sparse_set_archetype_components) { - components.insert( + component_set.insert( *component_id, ArchetypeComponentInfo { storage_type: StorageType::SparseSet, @@ -182,7 +183,7 @@ impl Archetype { ); } - archetype_invariants.test_archetype(components.indices()); + archetype_invariants.test_archetype(component_set.indices(), components); Self { id, @@ -190,7 +191,7 @@ impl Archetype { id: table_id, entity_rows: Default::default(), }, - components, + components: component_set, table_components, sparse_set_components, unique_components: SparseSet::new(), @@ -401,6 +402,7 @@ impl Default for Archetypes { Vec::new(), Vec::new(), &ArchetypeInvariants::default(), + &Components::default(), ); // adds the resource archetype. it is "special" in that it is inaccessible via a "hash", @@ -413,6 +415,7 @@ impl Default for Archetypes { Vec::new(), Vec::new(), &ArchetypeInvariants::default(), + &Components::default(), )); archetypes } @@ -500,6 +503,7 @@ impl Archetypes { table_components: Vec, sparse_set_components: Vec, archetype_invariants: &ArchetypeInvariants, + components: &Components, ) -> ArchetypeId { let table_components = table_components.into_boxed_slice(); let sparse_set_components = sparse_set_components.into_boxed_slice(); @@ -534,6 +538,7 @@ impl Archetypes { table_archetype_components, sparse_set_archetype_components, archetype_invariants, + components, )); id }) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 96422b810117a..02b187060201b 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -393,6 +393,7 @@ impl BundleInfo { table_components, sparse_set_components, archetype_invariants, + components, ); // add an edge from the old archetype to the new archetype archetypes[archetype_id].edges_mut().insert_add_bundle( diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index 5d6336a263325..e77a8540aa803 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -2,7 +2,11 @@ use std::marker::PhantomData; use bevy_utils::{tracing::warn, HashSet}; -use crate::{component::ComponentId, prelude::Bundle, world::World}; +use crate::{ + component::{ComponentId, Components}, + prelude::Bundle, + world::World, +}; /// A rule about which [`Component`](crate::component::Component)s can coexist on entities. /// @@ -236,6 +240,15 @@ impl UntypedArchetypeInvariant { ); } } + + /// Returns formatted string describing this archetype invariant + pub fn display(&self, components: &Components) -> String { + format!( + "{{Premise: {}, Consequence: {}}}", + self.premise.display(components), + self.consequence.display(components) + ) + } } /// A type-erased version of [`ArchetypeStatement`]. @@ -270,6 +283,31 @@ impl UntypedArchetypeStatement { } } + /// Returns formatted string describing this archetype invariant + /// + /// For Rust types, the names should match the type name. + /// If any [`ComponentId`]s in the invariant have not been registered in the world, + /// then the raw component id will be returned instead. + pub fn display(&self, components: &Components) -> String { + let component_names: String = self + .component_ids() + .iter() + .map(|id| match components.get_info(*id) { + Some(info) => info.name().to_owned(), + None => format!("{:?}", id), + }) + .reduce(|acc, s| format!("{}, {}", acc, s)) + .unwrap_or("".to_owned()); + + match self { + UntypedArchetypeStatement::AllOf(_) => format!("AllOf({component_names})"), + UntypedArchetypeStatement::AnyOf(_) => format!("AnyOf({component_names})"), + UntypedArchetypeStatement::AtMostOneOf(_) => format!("AtMostOneOf({component_names})"), + UntypedArchetypeStatement::NoneOf(_) => format!("NoneOf({component_names})"), + UntypedArchetypeStatement::Only(_) => format!("Only({component_names})"), + } + } + /// Test if this statement is true for the provided set of [`ComponentId`]s. pub fn test(&self, component_ids: &HashSet) -> bool { match self { @@ -342,7 +380,11 @@ impl ArchetypeInvariants { /// # Panics /// /// Panics if any archetype invariant is violated. - pub fn test_archetype(&self, component_ids_of_archetype: impl Iterator) { + pub fn test_archetype( + &self, + component_ids_of_archetype: impl Iterator, + components: &Components, + ) { let component_ids_of_archetype: HashSet = component_ids_of_archetype.collect(); for invariant in &self.raw_list { @@ -359,10 +401,24 @@ impl ArchetypeInvariants { } } + let archetype_component_names: Vec = component_ids_of_archetype + .into_iter() + .map(|id| match components.get_info(id) { + Some(info) => info.name().to_owned(), + None => format!("{:?}", id), + }) + .collect(); + + let failed_invariant_names: String = failed_invariants + .into_iter() + .map(|invariant| invariant.display(components)) + .reduce(|acc, s| format!("{}\n{}", acc, s)) + .unwrap(); + panic!( - "Archetype invariant violated! The following invariants were violated for archetype {:?}:\n{:?}", - component_ids_of_archetype, - failed_invariants, + "Archetype invariant violated! The following invariants were violated for archetype {:?}:\n{}", + archetype_component_names, + failed_invariant_names, ) } } diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 2cb0a26396a79..d31f2e9bd222e 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -820,6 +820,7 @@ unsafe fn remove_bundle_from_archetype( next_table_components, next_sparse_set_components, archetype_invariants, + components, ); Some(new_archetype_id) }; From db0927d673d695cc5fad69a445bfc4facdb34481 Mon Sep 17 00:00:00 2001 From: plof27 Date: Sat, 2 Jul 2022 10:52:58 -0400 Subject: [PATCH 31/59] Add archetype invariant types to the prelude --- crates/bevy_ecs/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 30a373a9d5ae0..efe0d3b894f83 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -42,7 +42,7 @@ pub mod prelude { NonSendMut, ParallelCommands, ParamSet, Query, RemovedComponents, Res, ResMut, System, SystemParamFunction, }, - world::{FromWorld, Mut, World}, + world::{FromWorld, Mut, World, ArchetypeInvariant, ArchetypeStatement}, }; } From 01d50f943bd5bd8c988e3e8fa3661e9b752421c2 Mon Sep 17 00:00:00 2001 From: plof27 Date: Sat, 2 Jul 2022 11:06:40 -0400 Subject: [PATCH 32/59] Fix format and clippy errors --- crates/bevy_ecs/src/archetype.rs | 1 + crates/bevy_ecs/src/lib.rs | 2 +- crates/bevy_ecs/src/world/archetype_invariants.rs | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/archetype.rs b/crates/bevy_ecs/src/archetype.rs index d08ed711ad7cf..7af49b5dd1eb1 100644 --- a/crates/bevy_ecs/src/archetype.rs +++ b/crates/bevy_ecs/src/archetype.rs @@ -146,6 +146,7 @@ pub struct Archetype { } impl Archetype { + #[allow(clippy::too_many_arguments)] pub fn new( id: ArchetypeId, table_id: TableId, diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index efe0d3b894f83..07d18a10fbc64 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -42,7 +42,7 @@ pub mod prelude { NonSendMut, ParallelCommands, ParamSet, Query, RemovedComponents, Res, ResMut, System, SystemParamFunction, }, - world::{FromWorld, Mut, World, ArchetypeInvariant, ArchetypeStatement}, + world::{ArchetypeInvariant, ArchetypeStatement, FromWorld, Mut, World}, }; } diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index e77a8540aa803..6cff9458106f6 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -297,7 +297,7 @@ impl UntypedArchetypeStatement { None => format!("{:?}", id), }) .reduce(|acc, s| format!("{}, {}", acc, s)) - .unwrap_or("".to_owned()); + .unwrap_or_default(); match self { UntypedArchetypeStatement::AllOf(_) => format!("AllOf({component_names})"), From f8a23fc274f084e335c34e40e65cd15669cd607c Mon Sep 17 00:00:00 2001 From: plof27 Date: Sat, 2 Jul 2022 15:05:56 -0400 Subject: [PATCH 33/59] Use `get_short_name` to further improve error messages --- crates/bevy_ecs/src/world/archetype_invariants.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index 6cff9458106f6..a2e1dc071b954 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -1,6 +1,6 @@ use std::marker::PhantomData; -use bevy_utils::{tracing::warn, HashSet}; +use bevy_utils::{get_short_name, tracing::warn, HashSet}; use crate::{ component::{ComponentId, Components}, @@ -293,7 +293,7 @@ impl UntypedArchetypeStatement { .component_ids() .iter() .map(|id| match components.get_info(*id) { - Some(info) => info.name().to_owned(), + Some(info) => get_short_name(info.name()), None => format!("{:?}", id), }) .reduce(|acc, s| format!("{}, {}", acc, s)) @@ -404,7 +404,7 @@ impl ArchetypeInvariants { let archetype_component_names: Vec = component_ids_of_archetype .into_iter() .map(|id| match components.get_info(id) { - Some(info) => info.name().to_owned(), + Some(info) => get_short_name(info.name()), None => format!("{:?}", id), }) .collect(); From 9d6364857a31d3af0d887054e4bc12d65e68aefc Mon Sep 17 00:00:00 2001 From: Rose Peck Date: Sat, 2 Jul 2022 12:25:23 -0700 Subject: [PATCH 34/59] Apply suggestions from code review - fix docstrings Co-authored-by: Federico Rinaldi --- crates/bevy_app/src/app.rs | 2 +- crates/bevy_ecs/src/world/archetype_invariants.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index e956c3bb04089..c3c225fd85afc 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -130,7 +130,7 @@ impl App { (runner)(app); } - /// Adds a new [`ArchetypeInvariant`] to this app's [`World`] + /// Adds a new [`ArchetypeInvariant`] to this app's [`World`]. /// /// Whenever a new archetype invariant is added to a world, all existing archetypes are re-checked. /// This may include empty archetypes- archetypes that contain no entities. diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index a2e1dc071b954..a40afd784e95c 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -52,7 +52,7 @@ impl ArchetypeInvariant { /// Creates an archetype invariant where any component of `B1` forbids every comonent from `B2`, and vice versa. /// /// In other words, if any component from `B1` is present, then none of the components from `B2` can be present. - /// Although this appears assymetric, it actually implies its own converse. + /// Although this appears asymetric, it actually implies its own converse. /// This is particularly useful for avoiding query conflicts. #[inline] pub fn forbids() -> Self { @@ -100,7 +100,7 @@ impl ArchetypeInvariant { /// Creates an archetype where components of `B` cannot appear with each other. /// /// In other words, if any component of this bundle is present, then no others can be. - /// This is particularly useful for creating enum-like groups of components, such as `Dead` and `Ailve`. + /// This is particularly useful for creating enum-like groups of components, such as `Dead` and `Alive`. #[inline] pub fn disjoint() -> Self { Self { @@ -126,7 +126,7 @@ impl ArchetypeInvariant { /// This type is used as part of an [`ArchetypeInvariant`]. /// /// When used as a premise, the archetype invariant matches all entities which satisfy the statement. -/// When used as a consquence, then the statment must be true for all entities that were matched by the premise. +/// When used as a consequence, then the statment must be true for all entities that were matched by the premise. /// /// For the statements about a single component `C`, wrap it in a single-component bundle `(C,)`. /// For single component bundles, `AllOf` and `AnyOf` are equivalent. From b293063cba63b46061786e569aad209fced1baa5 Mon Sep 17 00:00:00 2001 From: plof27 Date: Sat, 2 Jul 2022 15:26:51 -0400 Subject: [PATCH 35/59] Rename exhaustive to exclusive --- crates/bevy_ecs/src/world/archetype_invariants.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index a40afd784e95c..9dfb2882cf913 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -113,7 +113,7 @@ impl ArchetypeInvariant { /// /// In other words, if any component of this bundle is present, then _only_ components from this bundle can be present. #[inline] - pub fn exhaustive() -> Self { + pub fn exclusive() -> Self { Self { premise: ArchetypeStatement::::any_of(), consequence: ArchetypeStatement::::only(), @@ -589,10 +589,10 @@ mod tests { } #[test] - fn exhaustive_happy() { + fn exclusive_happy() { let mut world = World::new(); - world.add_archetype_invariant(ArchetypeInvariant::<(A, B)>::exhaustive()); + world.add_archetype_invariant(ArchetypeInvariant::<(A, B)>::exclusive()); world.spawn().insert_bundle((A, B)); world.spawn().insert(A); world.spawn().insert(C); @@ -600,19 +600,19 @@ mod tests { #[test] #[should_panic] - fn exhaustive_sad_partial() { + fn exclusive_sad_partial() { let mut world = World::new(); - world.add_archetype_invariant(ArchetypeInvariant::<(A, B)>::exhaustive()); + world.add_archetype_invariant(ArchetypeInvariant::<(A, B)>::exclusive()); world.spawn().insert_bundle((A, C)); } #[test] #[should_panic] - fn exhaustive_sad_all() { + fn exclusive_sad_all() { let mut world = World::new(); - world.add_archetype_invariant(ArchetypeInvariant::<(A, B)>::exhaustive()); + world.add_archetype_invariant(ArchetypeInvariant::<(A, B)>::exclusive()); world.spawn().insert_bundle((A, B, C)); } } From 4c1759afa88a104d5b3cb56537150abef8e61476 Mon Sep 17 00:00:00 2001 From: Rose Peck Date: Mon, 4 Jul 2022 11:29:51 -0700 Subject: [PATCH 36/59] Apply suggestions from code review - fix docstrings Co-authored-by: Federico Rinaldi Co-authored-by: Alice Cecile --- crates/bevy_ecs/src/world/archetype_invariants.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index 9dfb2882cf913..0ffa26c98ccea 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -52,7 +52,7 @@ impl ArchetypeInvariant { /// Creates an archetype invariant where any component of `B1` forbids every comonent from `B2`, and vice versa. /// /// In other words, if any component from `B1` is present, then none of the components from `B2` can be present. - /// Although this appears asymetric, it actually implies its own converse. + /// Although this appears asymmetric, it actually implies its own converse. /// This is particularly useful for avoiding query conflicts. #[inline] pub fn forbids() -> Self { @@ -73,7 +73,7 @@ impl ArchetypeInvariant { } } - /// Creates an archetype invariant where any components of `B1` must appear with some components of `B2`. + /// Creates an archetype invariant where components of `B1` require at least one component of `B2`. /// /// In other words, if any component from `B1` is present, then at least one component from `B2` must be. #[inline] @@ -121,7 +121,7 @@ impl ArchetypeInvariant { } } -/// A statement about the presence or absence of some subset of components in the given [`Bundle`] +/// A statement about the presence or absence of some subset of components in the given [`Bundle`]. /// /// This type is used as part of an [`ArchetypeInvariant`]. /// @@ -272,7 +272,7 @@ pub enum UntypedArchetypeStatement { } impl UntypedArchetypeStatement { - /// Get the set of [`ComponentId`]s affected by this statement + /// Returns the set of [`ComponentId`]s affected by this statement. pub fn component_ids(&self) -> &HashSet { match self { UntypedArchetypeStatement::AllOf(set) @@ -283,7 +283,7 @@ impl UntypedArchetypeStatement { } } - /// Returns formatted string describing this archetype invariant + /// Returns formatted string describing this archetype invariant. /// /// For Rust types, the names should match the type name. /// If any [`ComponentId`]s in the invariant have not been registered in the world, From 22b6af1e36a4ea5a54982c3ddc949212cd18eb2f Mon Sep 17 00:00:00 2001 From: plof27 Date: Mon, 4 Jul 2022 14:33:08 -0400 Subject: [PATCH 37/59] Remove warning when constructing `ArchetypeStatement::AnyOf` with a single-component bundle --- crates/bevy_ecs/src/world/archetype_invariants.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index 0ffa26c98ccea..7e0ca16e815cd 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -1,6 +1,6 @@ use std::marker::PhantomData; -use bevy_utils::{get_short_name, tracing::warn, HashSet}; +use bevy_utils::{get_short_name, HashSet}; use crate::{ component::{ComponentId, Components}, @@ -160,12 +160,7 @@ impl ArchetypeStatement { match self { ArchetypeStatement::AllOf(_) => UntypedArchetypeStatement::AllOf(component_ids), - ArchetypeStatement::AnyOf(_) => { - if component_ids.len() == 1 { - warn!("An `ArchetypeStatement::AnyOf` was constructed for a bundle with only one component. Prefer the equivalent `ArchetypeStatment:AllOf` for consistency and clarity."); - } - UntypedArchetypeStatement::AnyOf(component_ids) - } + ArchetypeStatement::AnyOf(_) => UntypedArchetypeStatement::AnyOf(component_ids), ArchetypeStatement::AtMostOneOf(_) => { UntypedArchetypeStatement::AtMostOneOf(component_ids) } From 5c0b8063bb1d78fb798ddc35e64e968d5b7f924d Mon Sep 17 00:00:00 2001 From: plof27 Date: Mon, 4 Jul 2022 16:22:22 -0400 Subject: [PATCH 38/59] Add basic example // WIP --- Cargo.toml | 5 +++ examples/README.md | 1 + examples/ecs/archetype_invariants.rs | 58 ++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+) create mode 100644 examples/ecs/archetype_invariants.rs diff --git a/Cargo.toml b/Cargo.toml index 3a094ff9f3722..94fb06979c533 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -718,6 +718,11 @@ description = "Full guide to Bevy's ECS" category = "ECS (Entity Component System)" wasm = false +[[example]] +name = "archetype_invariants" +path = "examples/ecs/archetype_invariants.rs" +description = "Assertions about valid combinations of components" + [[example]] name = "component_change_detection" path = "examples/ecs/component_change_detection.rs" diff --git a/examples/README.md b/examples/README.md index 5b4ac1fc95811..bed95d251421e 100644 --- a/examples/README.md +++ b/examples/README.md @@ -184,6 +184,7 @@ Example | Description Example | Description --- | --- +[Archetype Invariants](../examples/ecs/archetype_invariants.rs) | Assertions about valid combinations of components [Component Change Detection](../examples/ecs/component_change_detection.rs) | Change detection on components [Custom Query Parameters](../examples/ecs/custom_query_param.rs) | Groups commonly used compound queries and query filters into a single type [ECS Guide](../examples/ecs/ecs_guide.rs) | Full guide to Bevy's ECS diff --git a/examples/ecs/archetype_invariants.rs b/examples/ecs/archetype_invariants.rs new file mode 100644 index 0000000000000..3a1036f075897 --- /dev/null +++ b/examples/ecs/archetype_invariants.rs @@ -0,0 +1,58 @@ +//! Archetype invariants are rules about which combinations of components can coexist. +//! +//! An archetype (in the sense that Bevy uses it) is the "unique set of components" that belong to an entity. +//! These are useful to codify your assumptions about the composition of your entities. +//! For example, an entity can never have a `Player` component with a `Camera` together, +//! or a `GlobalTransform` may only be valid in association with a `Transform`. +//! By constructing `ArchetypeInvariant`s out of `ArchetypeStatement`s, +//! we can encode this logic into our app. +//! +//! Archetype invariants are guaranteed to hold at *all* points during the app's lifecycle; +//! this is automtically checked on component insertion and removal, including when entities are spawned. +//! Make sure to test thoroughly when using archetype invariants in production though; +//! any violations will result in a panic! +use bevy::prelude::*; + +fn main(){ + App::new() + .add_plugins(DefaultPlugins) + // Archetype invariants are constructed in terms of bundles; + // use (MyComponent, ) to construct a bundle with a single item. + // This invariant ensures that Player and Camera can never be found together. + .add_archetype_invariant(ArchetypeInvariant::<(Player,), (Camera,)>::forbids()) + // This invariant ensures that the presence of a `GlobalTransform` component implies the existence of a `Transform` component + .add_archetype_invariant(ArchetypeInvariant::<(GlobalTransform,), (Transform,)>::requires_one()) + // Note that the converse statement isn't automatically true! + // With only the above invariant, a entity with only `Transform` is valid. + // To fix this, swap the order of the generic types and add a new invariant. + .add_archetype_invariant(ArchetypeInvariant::<(Transform,), (GlobalTransform,)>::requires_one()) + // The `disjoint` invariant ensures that at most one component from the bundle is present on a given entity. + // This way, an entity can never be an animal and a vegetable at once. + // This is useful for creating groups of components that behave conceptually similar to an enum + .add_archetype_invariant(ArchetypeInvariant::<(Animal, Vegetable, Mineral)>::disjoint()) + + // You can also specify custom invariants by constructing `ArchetypeInvariant` directly. + // This invariant ensures that all entities have at least one component from the bundle given. + // Combined with the above invariant, this means that every entity has exactly one of (Animal, Vegetable, Mineral). + .add_archetype_invariant(ArchetypeInvariant { + // This statement is always true, and so matches all entities regardless of their archetype + // TODO: replace this with ArchetypeStatement::True (once it's added) + premise: ArchetypeStatement::<()>::all_of(), + // ArchetypeStatement::AnyOf evaluates to true when at least one of the components exists + consequence: ArchetypeStatement::<(Animal, Vegetable, Mineral)>::any_of() + }) + .run(); +} + +#[derive(Component)] +struct Player; + +#[derive(Component)] +struct Animal; + +#[derive(Component)] +struct Vegetable; + +#[derive(Component)] +struct Mineral; + From 7e762757021cf0016f4ea0e348163d68c20a5452 Mon Sep 17 00:00:00 2001 From: plof27 Date: Mon, 4 Jul 2022 16:55:19 -0400 Subject: [PATCH 39/59] Add static true and false variants to `ArchetypeStatement` --- .../src/world/archetype_invariants.rs | 84 ++++++++++++++++++- examples/ecs/archetype_invariants.rs | 49 +++++------ 2 files changed, 105 insertions(+), 28 deletions(-) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index 7e0ca16e815cd..338889d004b65 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -148,6 +148,12 @@ pub enum ArchetypeStatement { NoneOf(PhantomData), /// The entity contains only components from the bundle `B`, and no others. Only(PhantomData), + /// This statement is always true. + /// Useful for constructing universal invariants. + True, + /// This statement is always false. + /// Useful for constructing universal invariants. + False, } impl ArchetypeStatement { @@ -166,6 +172,8 @@ impl ArchetypeStatement { } ArchetypeStatement::NoneOf(_) => UntypedArchetypeStatement::NoneOf(component_ids), ArchetypeStatement::Only(_) => UntypedArchetypeStatement::Only(component_ids), + ArchetypeStatement::True => UntypedArchetypeStatement::True, + ArchetypeStatement::False => UntypedArchetypeStatement::False, } } @@ -200,6 +208,23 @@ impl ArchetypeStatement { } } +// We must pass in a generic type to all archetype statements; +// we use the empty bundle `()` by convention. +// These helper methods are useful because they improve type inference and consistency in user code. +impl ArchetypeStatement<()> { + /// Constructs a new [`ArchetypeStatement::True`] variant. + #[inline] + pub const fn always_true() -> Self { + ArchetypeStatement::<()>::True + } + + /// Constructs a new [`ArchetypeStatement::False`] variant. + #[inline] + pub const fn always_false() -> Self { + ArchetypeStatement::<()>::False + } +} + /// A type-erased version of [`ArchetypeInvariant`]. /// /// Intended to be used with dynamic components that cannot be represented with Rust types. @@ -264,17 +289,26 @@ pub enum UntypedArchetypeStatement { NoneOf(HashSet), /// The entity contains only components from the bundle `B`, and no others. Only(HashSet), + /// This statement is always true. + /// Useful for constructing universal invariants. + True, + /// This statement is always false. + /// Useful for constructing universal invariants. + False, } impl UntypedArchetypeStatement { /// Returns the set of [`ComponentId`]s affected by this statement. - pub fn component_ids(&self) -> &HashSet { + /// + /// Returns `Some` for all variants other than the static `True` and `False`. + pub fn component_ids(&self) -> Option<&HashSet> { match self { UntypedArchetypeStatement::AllOf(set) | UntypedArchetypeStatement::AnyOf(set) | UntypedArchetypeStatement::AtMostOneOf(set) | UntypedArchetypeStatement::NoneOf(set) - | UntypedArchetypeStatement::Only(set) => set, + | UntypedArchetypeStatement::Only(set) => Some(set), + UntypedArchetypeStatement::True | UntypedArchetypeStatement::False => None, } } @@ -286,6 +320,7 @@ impl UntypedArchetypeStatement { pub fn display(&self, components: &Components) -> String { let component_names: String = self .component_ids() + .unwrap_or(&HashSet::new()) .iter() .map(|id| match components.get_info(*id) { Some(info) => get_short_name(info.name()), @@ -300,6 +335,8 @@ impl UntypedArchetypeStatement { UntypedArchetypeStatement::AtMostOneOf(_) => format!("AtMostOneOf({component_names})"), UntypedArchetypeStatement::NoneOf(_) => format!("NoneOf({component_names})"), UntypedArchetypeStatement::Only(_) => format!("Only({component_names})"), + UntypedArchetypeStatement::True => "True".to_owned(), + UntypedArchetypeStatement::False => "False".to_owned(), } } @@ -350,6 +387,8 @@ impl UntypedArchetypeStatement { } true } + UntypedArchetypeStatement::True => true, + UntypedArchetypeStatement::False => false, } } } @@ -422,9 +461,10 @@ impl ArchetypeInvariants { #[cfg(test)] mod tests { + use crate as bevy_ecs; use crate::{ - self as bevy_ecs, component::Component, world::archetype_invariants::ArchetypeInvariant, - world::World, + component::Component, world::archetype_invariants::ArchetypeInvariant, + world::archetype_invariants::ArchetypeStatement, world::World, }; #[derive(Component)] @@ -474,6 +514,42 @@ mod tests { world.add_untyped_archetype_invariant(archetype_invariant); } + #[test] + fn tautology() { + let mut world = World::new(); + + // This invariant is a tautology. + world.add_archetype_invariant(ArchetypeInvariant { + premise: ArchetypeStatement::always_true(), + consequence: ArchetypeStatement::always_true(), + }); + // This invariant is also a tautology. + world.add_archetype_invariant(ArchetypeInvariant { + premise: ArchetypeStatement::always_false(), + consequence: ArchetypeStatement::always_false(), + }); + + // Since invariants are only checked when archetypes are created, + // we must add something to trigger the check. + world.spawn().insert(A); + } + + #[test] + #[should_panic] + fn contradiction() { + let mut world = World::new(); + + // This invariant is a contradiction. + world.add_archetype_invariant(ArchetypeInvariant { + premise: ArchetypeStatement::always_true(), + consequence: ArchetypeStatement::always_false(), + }); + + // Since invariants are only checked when archetypes are created, + // we must add something to trigger the check. + world.spawn().insert(A); + } + #[test] fn forbids_happy() { let mut world = World::new(); diff --git a/examples/ecs/archetype_invariants.rs b/examples/ecs/archetype_invariants.rs index 3a1036f075897..a06ad9d8b9453 100644 --- a/examples/ecs/archetype_invariants.rs +++ b/examples/ecs/archetype_invariants.rs @@ -1,47 +1,49 @@ //! Archetype invariants are rules about which combinations of components can coexist. -//! +//! //! An archetype (in the sense that Bevy uses it) is the "unique set of components" that belong to an entity. //! These are useful to codify your assumptions about the composition of your entities. //! For example, an entity can never have a `Player` component with a `Camera` together, //! or a `GlobalTransform` may only be valid in association with a `Transform`. //! By constructing `ArchetypeInvariant`s out of `ArchetypeStatement`s, //! we can encode this logic into our app. -//! +//! //! Archetype invariants are guaranteed to hold at *all* points during the app's lifecycle; //! this is automtically checked on component insertion and removal, including when entities are spawned. //! Make sure to test thoroughly when using archetype invariants in production though; //! any violations will result in a panic! use bevy::prelude::*; -fn main(){ - App::new() +fn main() { + App::new() .add_plugins(DefaultPlugins) - // Archetype invariants are constructed in terms of bundles; - // use (MyComponent, ) to construct a bundle with a single item. - // This invariant ensures that Player and Camera can never be found together. - .add_archetype_invariant(ArchetypeInvariant::<(Player,), (Camera,)>::forbids()) - // This invariant ensures that the presence of a `GlobalTransform` component implies the existence of a `Transform` component - .add_archetype_invariant(ArchetypeInvariant::<(GlobalTransform,), (Transform,)>::requires_one()) - // Note that the converse statement isn't automatically true! - // With only the above invariant, a entity with only `Transform` is valid. - // To fix this, swap the order of the generic types and add a new invariant. - .add_archetype_invariant(ArchetypeInvariant::<(Transform,), (GlobalTransform,)>::requires_one()) + // Archetype invariants are constructed in terms of bundles; + // use (MyComponent, ) to construct a bundle with a single item. + // This invariant ensures that Player and Camera can never be found together. + .add_archetype_invariant(ArchetypeInvariant::<(Player,), (Camera,)>::forbids()) + // This invariant ensures that the presence of a `GlobalTransform` component implies the existence of a `Transform` component + .add_archetype_invariant( + ArchetypeInvariant::<(GlobalTransform,), (Transform,)>::requires_one(), + ) + // Note that the converse statement isn't automatically true! + // With only the above invariant, a entity with only `Transform` is valid. + // To fix this, swap the order of the generic types and add a new invariant. + .add_archetype_invariant( + ArchetypeInvariant::<(Transform,), (GlobalTransform,)>::requires_one(), + ) // The `disjoint` invariant ensures that at most one component from the bundle is present on a given entity. // This way, an entity can never be an animal and a vegetable at once. - // This is useful for creating groups of components that behave conceptually similar to an enum - .add_archetype_invariant(ArchetypeInvariant::<(Animal, Vegetable, Mineral)>::disjoint()) - + // This is useful for creating groups of components that behave conceptually similar to an enum + .add_archetype_invariant(ArchetypeInvariant::<(Animal, Vegetable, Mineral)>::disjoint()) // You can also specify custom invariants by constructing `ArchetypeInvariant` directly. // This invariant ensures that all entities have at least one component from the bundle given. // Combined with the above invariant, this means that every entity has exactly one of (Animal, Vegetable, Mineral). .add_archetype_invariant(ArchetypeInvariant { - // This statement is always true, and so matches all entities regardless of their archetype - // TODO: replace this with ArchetypeStatement::True (once it's added) - premise: ArchetypeStatement::<()>::all_of(), - // ArchetypeStatement::AnyOf evaluates to true when at least one of the components exists - consequence: ArchetypeStatement::<(Animal, Vegetable, Mineral)>::any_of() + // This statement is always true, and so matches all entities regardless of their archetype + premise: ArchetypeStatement::always_true(), + // ArchetypeStatement::AnyOf evaluates to true when at least one of the components exists + consequence: ArchetypeStatement::<(Animal, Vegetable, Mineral)>::any_of(), }) - .run(); + .run(); } #[derive(Component)] @@ -55,4 +57,3 @@ struct Vegetable; #[derive(Component)] struct Mineral; - From 5ec538d400021e337925cbc3534fd1a2ce728af6 Mon Sep 17 00:00:00 2001 From: plof27 Date: Mon, 4 Jul 2022 17:00:20 -0400 Subject: [PATCH 40/59] Rename `requires_all` to `requires` This is the more common case, and this is a more ergonomic name. --- crates/bevy_ecs/src/world/archetype_invariants.rs | 14 +++++++------- examples/ecs/archetype_invariants.rs | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index 338889d004b65..b7a37d8a269ef 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -66,7 +66,7 @@ impl ArchetypeInvariant { /// /// In other words, if any component from `B1` is present, then all of the components from `B2` must be. #[inline] - pub fn requires_all() -> Self { + pub fn requires() -> Self { Self { premise: ArchetypeStatement::::any_of(), consequence: ArchetypeStatement::::all_of(), @@ -569,29 +569,29 @@ mod tests { } #[test] - fn requires_all_happy() { + fn requires_happy() { let mut world = World::new(); - world.add_archetype_invariant(ArchetypeInvariant::<(A,), (B, C)>::requires_all()); + world.add_archetype_invariant(ArchetypeInvariant::<(A,), (B, C)>::requires()); world.spawn().insert_bundle((A, B, C)); world.spawn().insert_bundle((B, C)); } #[test] #[should_panic] - fn requires_all_sad_partial() { + fn requires_sad_partial() { let mut world = World::new(); - world.add_archetype_invariant(ArchetypeInvariant::<(A,), (B, C)>::requires_all()); + world.add_archetype_invariant(ArchetypeInvariant::<(A,), (B, C)>::requires()); world.spawn().insert_bundle((A, B)); } #[test] #[should_panic] - fn requires_all_sad_none() { + fn requires_sad_none() { let mut world = World::new(); - world.add_archetype_invariant(ArchetypeInvariant::<(A,), (B, C)>::requires_all()); + world.add_archetype_invariant(ArchetypeInvariant::<(A,), (B, C)>::requires()); world.spawn().insert(A); } diff --git a/examples/ecs/archetype_invariants.rs b/examples/ecs/archetype_invariants.rs index a06ad9d8b9453..097b23c1143d9 100644 --- a/examples/ecs/archetype_invariants.rs +++ b/examples/ecs/archetype_invariants.rs @@ -22,13 +22,13 @@ fn main() { .add_archetype_invariant(ArchetypeInvariant::<(Player,), (Camera,)>::forbids()) // This invariant ensures that the presence of a `GlobalTransform` component implies the existence of a `Transform` component .add_archetype_invariant( - ArchetypeInvariant::<(GlobalTransform,), (Transform,)>::requires_one(), + ArchetypeInvariant::<(GlobalTransform,), (Transform,)>::requires(), ) // Note that the converse statement isn't automatically true! // With only the above invariant, a entity with only `Transform` is valid. // To fix this, swap the order of the generic types and add a new invariant. .add_archetype_invariant( - ArchetypeInvariant::<(Transform,), (GlobalTransform,)>::requires_one(), + ArchetypeInvariant::<(Transform,), (GlobalTransform,)>::requires(), ) // The `disjoint` invariant ensures that at most one component from the bundle is present on a given entity. // This way, an entity can never be an animal and a vegetable at once. From 52cd3310ef6e5e09417ac2abbb699e20b169ac9e Mon Sep 17 00:00:00 2001 From: plof27 Date: Mon, 4 Jul 2022 17:11:38 -0400 Subject: [PATCH 41/59] Clean up example --- examples/ecs/archetype_invariants.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/examples/ecs/archetype_invariants.rs b/examples/ecs/archetype_invariants.rs index 097b23c1143d9..555cdf0453905 100644 --- a/examples/ecs/archetype_invariants.rs +++ b/examples/ecs/archetype_invariants.rs @@ -11,6 +11,10 @@ //! this is automtically checked on component insertion and removal, including when entities are spawned. //! Make sure to test thoroughly when using archetype invariants in production though; //! any violations will result in a panic! +//! +//! There are many helper methods provided on `ArchetypeInvariant` to help easily construct common invariant patterns, +//! but we will only be showcasing some of them here. +//! For a full list, see the docs for `ArchetypeInvariant`. use bevy::prelude::*; fn main() { @@ -20,27 +24,23 @@ fn main() { // use (MyComponent, ) to construct a bundle with a single item. // This invariant ensures that Player and Camera can never be found together. .add_archetype_invariant(ArchetypeInvariant::<(Player,), (Camera,)>::forbids()) - // This invariant ensures that the presence of a `GlobalTransform` component implies the existence of a `Transform` component - .add_archetype_invariant( - ArchetypeInvariant::<(GlobalTransform,), (Transform,)>::requires(), - ) + // This invariant ensures that the presence of a `GlobalTransform` component implies the existence of a `Transform` component. + .add_archetype_invariant(ArchetypeInvariant::<(GlobalTransform,), (Transform,)>::requires()) // Note that the converse statement isn't automatically true! // With only the above invariant, a entity with only `Transform` is valid. // To fix this, swap the order of the generic types and add a new invariant. - .add_archetype_invariant( - ArchetypeInvariant::<(Transform,), (GlobalTransform,)>::requires(), - ) + .add_archetype_invariant(ArchetypeInvariant::<(Transform,), (GlobalTransform,)>::requires()) // The `disjoint` invariant ensures that at most one component from the bundle is present on a given entity. // This way, an entity can never be an animal and a vegetable at once. - // This is useful for creating groups of components that behave conceptually similar to an enum + // This is useful for creating groups of components that behave conceptually similarly to an enum. .add_archetype_invariant(ArchetypeInvariant::<(Animal, Vegetable, Mineral)>::disjoint()) // You can also specify custom invariants by constructing `ArchetypeInvariant` directly. // This invariant ensures that all entities have at least one component from the bundle given. // Combined with the above invariant, this means that every entity has exactly one of (Animal, Vegetable, Mineral). .add_archetype_invariant(ArchetypeInvariant { - // This statement is always true, and so matches all entities regardless of their archetype + // This statement is always true, and so matches all entities regardless of their archetype. premise: ArchetypeStatement::always_true(), - // ArchetypeStatement::AnyOf evaluates to true when at least one of the components exists + // ArchetypeStatement::AnyOf evaluates to true when at least one of the components is present on the entity. consequence: ArchetypeStatement::<(Animal, Vegetable, Mineral)>::any_of(), }) .run(); From 5b200795dc5fc3ce38665c6db54e55ae1f43a828 Mon Sep 17 00:00:00 2001 From: plof27 Date: Mon, 4 Jul 2022 17:34:03 -0400 Subject: [PATCH 42/59] Add example metadata and transitivity information --- Cargo.toml | 5 +++++ examples/ecs/archetype_invariants.rs | 27 ++++++++++++++++++++++++++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 94fb06979c533..b48f13320e94b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -721,7 +721,12 @@ wasm = false [[example]] name = "archetype_invariants" path = "examples/ecs/archetype_invariants.rs" + +[package.metadata.example.archetype_invariants] +name = "archetype_invariants" description = "Assertions about valid combinations of components" +category = "ECS (Entity Component System)" +wasm = false [[example]] name = "component_change_detection" diff --git a/examples/ecs/archetype_invariants.rs b/examples/ecs/archetype_invariants.rs index 555cdf0453905..ff9de89ebacfe 100644 --- a/examples/ecs/archetype_invariants.rs +++ b/examples/ecs/archetype_invariants.rs @@ -19,7 +19,6 @@ use bevy::prelude::*; fn main() { App::new() - .add_plugins(DefaultPlugins) // Archetype invariants are constructed in terms of bundles; // use (MyComponent, ) to construct a bundle with a single item. // This invariant ensures that Player and Camera can never be found together. @@ -43,6 +42,8 @@ fn main() { // ArchetypeStatement::AnyOf evaluates to true when at least one of the components is present on the entity. consequence: ArchetypeStatement::<(Animal, Vegetable, Mineral)>::any_of(), }) + .add_startup_system(spawn_vegetable) + .add_system(position_vegetable) .run(); } @@ -57,3 +58,27 @@ struct Vegetable; #[derive(Component)] struct Mineral; + +fn spawn_vegetable(mut commands: Commands) { + commands.spawn().insert(Vegetable); +} + +fn position_vegetable(mut commands: Commands, query: Query>) { + let vegetable_entity = query.single(); + + // Because of our invariants, these components need to be added together. + // Adding them separately (as in the broken code below) will cause the entity to briefly enter an invalid state, + // where it has only one of the two components. + commands + .entity(vegetable_entity) + .insert_bundle((GlobalTransform::default(), Transform::default())); + + // Adding the components one at a time panics + // Track this limitation at https://github.com/bevyengine/bevy/issues/5074 + /* + commands + .entity(vegetable_entity) + .insert(GlobalTransform::default()) + .insert(Transform::default()); + */ +} From 38123ca53f14237edbd32e040e4dea38e918ad5f Mon Sep 17 00:00:00 2001 From: plof27 Date: Mon, 4 Jul 2022 17:35:05 -0400 Subject: [PATCH 43/59] Clean up docs for archetype statements --- .../bevy_ecs/src/world/archetype_invariants.rs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index b7a37d8a269ef..8c51a6d12c58d 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -139,19 +139,24 @@ pub enum ArchetypeStatement { /// Evaluates to true if and only if the entity has all of the components present in the bundle `B`. AllOf(PhantomData), /// The entity has at least one component in the bundle `B`. - /// When using a single-component bundle, `AllOf` is preferred. + /// + /// When using a single-component bundle, `AllOf` is preferred by convention. AnyOf(PhantomData), /// The entity has zero or one of the components in the bundle `B`, but no more. - /// When using a single-component bundle, this will always be true. + /// + /// When using a single-component bundle this is tautologically true. + /// Prefer the much clearer `True` variant. AtMostOneOf(PhantomData), /// The entity has none of the components in the bundle `B`. NoneOf(PhantomData), /// The entity contains only components from the bundle `B`, and no others. Only(PhantomData), /// This statement is always true. + /// /// Useful for constructing universal invariants. True, /// This statement is always false. + /// /// Useful for constructing universal invariants. False, } @@ -280,19 +285,24 @@ pub enum UntypedArchetypeStatement { /// Evaluates to true if and only if the entity has all of the components present in the set. AllOf(HashSet), /// The entity has at least one component in the set, and may have all of them. + /// /// When using a single-component set, `AllOf` is preferred. AnyOf(HashSet), /// The entity has zero or one of the components in the set, but no more. - /// When using a single-component set, this is a tautology. + /// + /// When using a single-component bundle this is tautologically true. + /// Prefer the much clearer `True` variant. AtMostOneOf(HashSet), /// The entity has none of the components in the set. NoneOf(HashSet), /// The entity contains only components from the bundle `B`, and no others. Only(HashSet), /// This statement is always true. + /// /// Useful for constructing universal invariants. True, /// This statement is always false. + /// /// Useful for constructing universal invariants. False, } From 4109138d23ee099dfaab683c61b9c2be117c074b Mon Sep 17 00:00:00 2001 From: plof27 Date: Mon, 4 Jul 2022 17:47:41 -0400 Subject: [PATCH 44/59] Clean up docs around `UntypedArchetypeInvariant` --- .../src/world/archetype_invariants.rs | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index 8c51a6d12c58d..8755eb04d46a4 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -19,14 +19,17 @@ use crate::{ /// Archetype invariants are checked each time [`Archetypes`](crate::archetype::Archetypes) is modified; /// this can occur on component addition, component removal, and entity spawning. /// +/// Archetypes are only modified when a novel archetype (set of components) is seen for the first time; +/// swapping between existing archetypes will not trigger these checks. +/// /// Note that archetype invariants are not symmetric by default. /// For example, `ArchetypeInvariant::::requires_one()` means that `B1` requires `B2`, /// but not that `B2` requires `B1`. /// In this case, an entity with just `B2` is completely valid, but an entity with just `B1` is not. /// If symmetry is desired, repeat the invariant with the order of the types switched. -/// -/// Archetypes are only modified when a novel archetype (set of components) is seen for the first time; -/// swapping between existing archetypes will not trigger these checks. +/// +/// When working with dynamic component types (for non-Rust components), +/// use the untyped equivalents [`UntypedArchetypeInvariant`] and [`UntypedArchetypeStatement`] directly. #[derive(Clone, Debug, PartialEq)] pub struct ArchetypeInvariant { /// Defines which entities this invariant applies to. @@ -131,9 +134,6 @@ impl ArchetypeInvariant { /// For the statements about a single component `C`, wrap it in a single-component bundle `(C,)`. /// For single component bundles, `AllOf` and `AnyOf` are equivalent. /// Prefer `ArchetypeStatement::<(C,)>::all_of` over `ArchetypeStatement::<(C,)>::any_of` for consistency and clarity. -/// -/// Note that this is converted to an [`UntypedArchetypeStatement`] when added to a [`World`]. -/// This is to ensure compatibility between different invariants. #[derive(Clone, Debug, PartialEq)] pub enum ArchetypeStatement { /// Evaluates to true if and only if the entity has all of the components present in the bundle `B`. @@ -403,7 +403,10 @@ impl UntypedArchetypeStatement { } } -/// A list of [`ArchetypeInvariant`]s to be stored on a [`World`]. +/// A list of [`ArchetypeInvariant`]s, stored on a [`World`]. +/// +/// These store [`UntypedArchetypeInvariant`]s to ensure fast computation +/// and compatiblity with dynamic (non-Rust) component types. #[derive(Default)] pub struct ArchetypeInvariants { /// The list of invariants that must be upheld. @@ -417,6 +420,11 @@ impl ArchetypeInvariants { self.raw_list.push(archetype_invariant); } + /// Returns the raw list of [`UntypedArchetypeInvariant`]s + pub fn raw_list(&self) -> &Vec { + &self.raw_list + } + /// Asserts that the provided iterator of [`ComponentId`]s obeys all archetype invariants. /// /// `component_ids` is generally provided via the `components` field on [`Archetype`](crate::archetype::Archetype). From 95428ebe5390f9e1dad5c8c816d72f24ed5287d3 Mon Sep 17 00:00:00 2001 From: plof27 Date: Mon, 4 Jul 2022 17:53:56 -0400 Subject: [PATCH 45/59] Run cargo fmt --- crates/bevy_ecs/src/world/archetype_invariants.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index 8755eb04d46a4..e6edd3bc0810c 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -27,7 +27,7 @@ use crate::{ /// but not that `B2` requires `B1`. /// In this case, an entity with just `B2` is completely valid, but an entity with just `B1` is not. /// If symmetry is desired, repeat the invariant with the order of the types switched. -/// +/// /// When working with dynamic component types (for non-Rust components), /// use the untyped equivalents [`UntypedArchetypeInvariant`] and [`UntypedArchetypeStatement`] directly. #[derive(Clone, Debug, PartialEq)] @@ -404,7 +404,7 @@ impl UntypedArchetypeStatement { } /// A list of [`ArchetypeInvariant`]s, stored on a [`World`]. -/// +/// /// These store [`UntypedArchetypeInvariant`]s to ensure fast computation /// and compatiblity with dynamic (non-Rust) component types. #[derive(Default)] From 011bd2e1a5b098dfc6fff7871f289c928a7459b4 Mon Sep 17 00:00:00 2001 From: plof27 Date: Mon, 4 Jul 2022 18:07:33 -0400 Subject: [PATCH 46/59] Update example name --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index b48f13320e94b..822862d3cdcc5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -723,7 +723,7 @@ name = "archetype_invariants" path = "examples/ecs/archetype_invariants.rs" [package.metadata.example.archetype_invariants] -name = "archetype_invariants" +name = "Archetype Invariants" description = "Assertions about valid combinations of components" category = "ECS (Entity Component System)" wasm = false From 9bb0cf42cf768ccca735e1a95a18638daf754223 Mon Sep 17 00:00:00 2001 From: plof27 Date: Wed, 6 Jul 2022 11:19:18 -0400 Subject: [PATCH 47/59] Clean up example --- examples/ecs/archetype_invariants.rs | 63 +++++++++++++++------------- 1 file changed, 35 insertions(+), 28 deletions(-) diff --git a/examples/ecs/archetype_invariants.rs b/examples/ecs/archetype_invariants.rs index ff9de89ebacfe..f1ab7833be6f6 100644 --- a/examples/ecs/archetype_invariants.rs +++ b/examples/ecs/archetype_invariants.rs @@ -23,27 +23,31 @@ fn main() { // use (MyComponent, ) to construct a bundle with a single item. // This invariant ensures that Player and Camera can never be found together. .add_archetype_invariant(ArchetypeInvariant::<(Player,), (Camera,)>::forbids()) - // This invariant ensures that the presence of a `GlobalTransform` component implies the existence of a `Transform` component. - .add_archetype_invariant(ArchetypeInvariant::<(GlobalTransform,), (Transform,)>::requires()) - // Note that the converse statement isn't automatically true! - // With only the above invariant, a entity with only `Transform` is valid. - // To fix this, swap the order of the generic types and add a new invariant. - .add_archetype_invariant(ArchetypeInvariant::<(Transform,), (GlobalTransform,)>::requires()) + // This invariant ensures that the `GlobalTransform` component is always found with the `Transform` component, and vice versa. + .add_archetype_invariant(ArchetypeInvariant::<(GlobalTransform, Transform)>::atomic()) + // This invariant ensures that the `Player` compoenent is always found with the `Life` component. + // This requirement is only in one direction: it is possible to have entities which have `Life`, but not `Player` (like enemies). + .add_archetype_invariant(ArchetypeInvariant::<(Player,), (Life,)>::requires()) // The `disjoint` invariant ensures that at most one component from the bundle is present on a given entity. - // This way, an entity can never be an animal and a vegetable at once. + // This way, an entity never belongs to more than one RPG class at once. // This is useful for creating groups of components that behave conceptually similarly to an enum. - .add_archetype_invariant(ArchetypeInvariant::<(Animal, Vegetable, Mineral)>::disjoint()) - // You can also specify custom invariants by constructing `ArchetypeInvariant` directly. - // This invariant ensures that all entities have at least one component from the bundle given. - // Combined with the above invariant, this means that every entity has exactly one of (Animal, Vegetable, Mineral). + .add_archetype_invariant(ArchetypeInvariant::<(Archer, Swordsman, Mage)>::disjoint()) + // This invariant indicates that any entity with the `Player` component always has + // at least one component in the `(Archer, Swordsman, Mage)` bundle. + // We could use a type alias to improve clarity and avoid errors caused by duplication: + // type Class = (Archer, Swordsman, Mage); + .add_archetype_invariant( + ArchetypeInvariant::<(Player,), (Archer, Swordsman, Mage)>::requires_one(), + ) + // You can also specify custom invariants by constructing `ArchetypeInvariant direcly. + // This invariant specifies that the `Node` component cannot appear on any entity in our world. + // We're not using bevy_ui in our App, so this component should never show up. .add_archetype_invariant(ArchetypeInvariant { - // This statement is always true, and so matches all entities regardless of their archetype. - premise: ArchetypeStatement::always_true(), - // ArchetypeStatement::AnyOf evaluates to true when at least one of the components is present on the entity. - consequence: ArchetypeStatement::<(Animal, Vegetable, Mineral)>::any_of(), + premise: ArchetypeStatement::<(Node,)>::all_of(), + consequence: ArchetypeStatement::always_false(), }) - .add_startup_system(spawn_vegetable) - .add_system(position_vegetable) + .add_startup_system(spawn_player) + .add_system(position_player) .run(); } @@ -51,33 +55,36 @@ fn main() { struct Player; #[derive(Component)] -struct Animal; +struct Life; #[derive(Component)] -struct Vegetable; +struct Archer; #[derive(Component)] -struct Mineral; +struct Swordsman; -fn spawn_vegetable(mut commands: Commands) { - commands.spawn().insert(Vegetable); +#[derive(Component)] +struct Mage; + +fn spawn_player(mut commands: Commands) { + commands.spawn().insert_bundle((Player, Mage)); } -fn position_vegetable(mut commands: Commands, query: Query>) { - let vegetable_entity = query.single(); +fn position_player(mut commands: Commands, query: Query, Added)>) { + let player_entity = query.single(); // Because of our invariants, these components need to be added together. // Adding them separately (as in the broken code below) will cause the entity to briefly enter an invalid state, // where it has only one of the two components. commands - .entity(vegetable_entity) + .entity(player_entity) .insert_bundle((GlobalTransform::default(), Transform::default())); - // Adding the components one at a time panics - // Track this limitation at https://github.com/bevyengine/bevy/issues/5074 + // Adding the components one at a time panics. + // Track this limitation at https://github.com/bevyengine/bevy/issues/5074. /* commands - .entity(vegetable_entity) + .entity(player_entity) .insert(GlobalTransform::default()) .insert(Transform::default()); */ From 0a920cb4428289c8d47e6a93cd76df892e7c059a Mon Sep 17 00:00:00 2001 From: plof27 Date: Wed, 6 Jul 2022 11:29:16 -0400 Subject: [PATCH 48/59] Remove use of "tautologically true", replace with "always true" --- crates/bevy_ecs/src/world/archetype_invariants.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index e6edd3bc0810c..3b9d6d2869cbe 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -144,7 +144,7 @@ pub enum ArchetypeStatement { AnyOf(PhantomData), /// The entity has zero or one of the components in the bundle `B`, but no more. /// - /// When using a single-component bundle this is tautologically true. + /// When using a single-component bundle this is always true. /// Prefer the much clearer `True` variant. AtMostOneOf(PhantomData), /// The entity has none of the components in the bundle `B`. @@ -290,7 +290,7 @@ pub enum UntypedArchetypeStatement { AnyOf(HashSet), /// The entity has zero or one of the components in the set, but no more. /// - /// When using a single-component bundle this is tautologically true. + /// When using a single-component bundle this is always true. /// Prefer the much clearer `True` variant. AtMostOneOf(HashSet), /// The entity has none of the components in the set. From 97a33e6d991583d2ea5a03754b9bfe8c41957e11 Mon Sep 17 00:00:00 2001 From: plof27 Date: Wed, 6 Jul 2022 12:23:48 -0400 Subject: [PATCH 49/59] Fix error message for on_insert failures, clean up error display logic --- crates/bevy_ecs/src/component.rs | 17 ++++++ .../src/world/archetype_invariants.rs | 52 +++++++++---------- crates/bevy_ecs/src/world/mod.rs | 8 +-- 3 files changed, 45 insertions(+), 32 deletions(-) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index b26c932a6cc74..3e296bad519e2 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -7,6 +7,7 @@ use crate::{ }; pub use bevy_ecs_macros::Component; use bevy_ptr::OwningPtr; +use bevy_utils::get_short_name; use std::{ alloc::Layout, any::{Any, TypeId}, @@ -251,6 +252,22 @@ impl SparseSetIndex for ComponentId { } } +/// Prints the shortened type names of the provided [`ComponentId`]s +/// +/// Uses [`get_short_name`] to strip the module paths of the items, resulting in cleaner lists. +pub fn display_component_id_types<'a, I: Iterator>( + component_ids: I, + components: &Components, +) -> String { + component_ids + .map(|id| match components.get_info(*id) { + Some(info) => get_short_name(info.name()), + None => format!("{:?}", id), + }) + .reduce(|acc, s| format!("{}, {}", acc, s)) + .unwrap_or_default() +} + pub struct ComponentDescriptor { name: Cow<'static, str>, // SAFETY: This must remain private. It must match the statically known StorageType of the diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index 3b9d6d2869cbe..4bb163d8ef7f2 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -1,9 +1,9 @@ use std::marker::PhantomData; -use bevy_utils::{get_short_name, HashSet}; +use bevy_utils::HashSet; use crate::{ - component::{ComponentId, Components}, + component::{display_component_id_types, ComponentId, Components}, prelude::Bundle, world::World, }; @@ -253,15 +253,23 @@ impl UntypedArchetypeInvariant { /// /// # Panics /// Panics if the archetype invariant is violated. - pub fn test_archetype(&self, component_ids_of_archetype: impl Iterator) { + pub fn test_archetype( + &self, + component_ids_of_archetype: impl Iterator, + components: &Components, + ) { let component_ids_of_archetype: HashSet = component_ids_of_archetype.collect(); if self.premise.test(&component_ids_of_archetype) && !self.consequence.test(&component_ids_of_archetype) { + let archetype_component_names = + display_component_id_types(component_ids_of_archetype.iter(), components); + panic!( - "Archetype invariant violated! The invariant {:?} failed for archetype {:?}", - self, component_ids_of_archetype + "Archetype invariant {} failed for archetype [{}]", + self.display(components), + archetype_component_names ); } } @@ -328,16 +336,10 @@ impl UntypedArchetypeStatement { /// If any [`ComponentId`]s in the invariant have not been registered in the world, /// then the raw component id will be returned instead. pub fn display(&self, components: &Components) -> String { - let component_names: String = self - .component_ids() - .unwrap_or(&HashSet::new()) - .iter() - .map(|id| match components.get_info(*id) { - Some(info) => get_short_name(info.name()), - None => format!("{:?}", id), - }) - .reduce(|acc, s| format!("{}, {}", acc, s)) - .unwrap_or_default(); + let component_names = display_component_id_types( + self.component_ids().unwrap_or(&HashSet::new()).iter(), + components, + ); match self { UntypedArchetypeStatement::AllOf(_) => format!("AllOf({component_names})"), @@ -453,24 +455,18 @@ impl ArchetypeInvariants { } } - let archetype_component_names: Vec = component_ids_of_archetype - .into_iter() - .map(|id| match components.get_info(id) { - Some(info) => get_short_name(info.name()), - None => format!("{:?}", id), - }) - .collect(); + let archetype_component_names = + display_component_id_types(component_ids_of_archetype.iter(), components); - let failed_invariant_names: String = failed_invariants - .into_iter() + let failed_invariant_names = failed_invariants + .iter() .map(|invariant| invariant.display(components)) .reduce(|acc, s| format!("{}\n{}", acc, s)) - .unwrap(); + .unwrap_or_default(); panic!( - "Archetype invariant violated! The following invariants were violated for archetype {:?}:\n{}", - archetype_component_names, - failed_invariant_names, + "At least one archetype invariant was violated for the archetype [{archetype_component_names}]: \ + \n{failed_invariant_names}" ) } } diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 85cc61a7467a3..f9ed677fc97f2 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -687,8 +687,8 @@ impl World { let untyped_invariant = archetype_invariant.into_untyped(self); for archetype in &self.archetypes.archetypes { - let components = archetype.components.indices(); - untyped_invariant.test_archetype(components); + let archetype_components = archetype.components.indices(); + untyped_invariant.test_archetype(archetype_components, self.components()); } self.archetype_invariants.add(untyped_invariant); @@ -705,8 +705,8 @@ impl World { archetype_invariant: UntypedArchetypeInvariant, ) { for archetype in &self.archetypes.archetypes { - let components = archetype.components.indices(); - archetype_invariant.test_archetype(components); + let archetype_components = archetype.components.indices(); + archetype_invariant.test_archetype(archetype_components, self.components()); } self.archetype_invariants.add(archetype_invariant); From 07dce545ae56c7bb40d19d5952cc774f4983f048 Mon Sep 17 00:00:00 2001 From: Rose Peck Date: Thu, 7 Jul 2022 18:23:07 -0700 Subject: [PATCH 50/59] Apply suggestions from code review - clean up docstrings and comments Co-authored-by: Alice Cecile Co-authored-by: Federico Rinaldi --- crates/bevy_ecs/src/component.rs | 2 +- crates/bevy_ecs/src/world/archetype_invariants.rs | 2 +- examples/ecs/archetype_invariants.rs | 12 ++++++------ 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 3e296bad519e2..66a7808466d21 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -252,7 +252,7 @@ impl SparseSetIndex for ComponentId { } } -/// Prints the shortened type names of the provided [`ComponentId`]s +/// Returns the shortened type names of the provided [`ComponentId`]s /// /// Uses [`get_short_name`] to strip the module paths of the items, resulting in cleaner lists. pub fn display_component_id_types<'a, I: Iterator>( diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index 4bb163d8ef7f2..c2667807e1f07 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -29,7 +29,7 @@ use crate::{ /// If symmetry is desired, repeat the invariant with the order of the types switched. /// /// When working with dynamic component types (for non-Rust components), -/// use the untyped equivalents [`UntypedArchetypeInvariant`] and [`UntypedArchetypeStatement`] directly. +/// use [`UntypedArchetypeInvariant`] and [`UntypedArchetypeStatement`] instead. #[derive(Clone, Debug, PartialEq)] pub struct ArchetypeInvariant { /// Defines which entities this invariant applies to. diff --git a/examples/ecs/archetype_invariants.rs b/examples/ecs/archetype_invariants.rs index f1ab7833be6f6..d7a9902f50fee 100644 --- a/examples/ecs/archetype_invariants.rs +++ b/examples/ecs/archetype_invariants.rs @@ -14,23 +14,23 @@ //! //! There are many helper methods provided on `ArchetypeInvariant` to help easily construct common invariant patterns, //! but we will only be showcasing some of them here. -//! For a full list, see the docs for `ArchetypeInvariant`. +//! For a full list, see the docs for [`ArchetypeInvariant`]. use bevy::prelude::*; fn main() { App::new() // Archetype invariants are constructed in terms of bundles; // use (MyComponent, ) to construct a bundle with a single item. - // This invariant ensures that Player and Camera can never be found together. + // This invariant ensures that Player and Camera can never be found together on the same entity. .add_archetype_invariant(ArchetypeInvariant::<(Player,), (Camera,)>::forbids()) // This invariant ensures that the `GlobalTransform` component is always found with the `Transform` component, and vice versa. .add_archetype_invariant(ArchetypeInvariant::<(GlobalTransform, Transform)>::atomic()) - // This invariant ensures that the `Player` compoenent is always found with the `Life` component. + // This invariant ensures that the `Player` component is always found with the `Life` component. // This requirement is only in one direction: it is possible to have entities which have `Life`, but not `Player` (like enemies). .add_archetype_invariant(ArchetypeInvariant::<(Player,), (Life,)>::requires()) // The `disjoint` invariant ensures that at most one component from the bundle is present on a given entity. // This way, an entity never belongs to more than one RPG class at once. - // This is useful for creating groups of components that behave conceptually similarly to an enum. + // This is useful for creating groups of components that behave similarly to an enum. .add_archetype_invariant(ArchetypeInvariant::<(Archer, Swordsman, Mage)>::disjoint()) // This invariant indicates that any entity with the `Player` component always has // at least one component in the `(Archer, Swordsman, Mage)` bundle. @@ -39,7 +39,7 @@ fn main() { .add_archetype_invariant( ArchetypeInvariant::<(Player,), (Archer, Swordsman, Mage)>::requires_one(), ) - // You can also specify custom invariants by constructing `ArchetypeInvariant direcly. + // You can also specify custom invariants by constructing `ArchetypeInvariant directly. // This invariant specifies that the `Node` component cannot appear on any entity in our world. // We're not using bevy_ui in our App, so this component should never show up. .add_archetype_invariant(ArchetypeInvariant { @@ -67,7 +67,7 @@ struct Swordsman; struct Mage; fn spawn_player(mut commands: Commands) { - commands.spawn().insert_bundle((Player, Mage)); + commands.spawn().insert_bundle((Player, Mage, Life)); } fn position_player(mut commands: Commands, query: Query, Added)>) { From 16bc2452240e593c696d67de58d53912067c78c5 Mon Sep 17 00:00:00 2001 From: plof27 Date: Thu, 7 Jul 2022 21:27:50 -0400 Subject: [PATCH 51/59] Remove `With` parameter from example This is totally unnecessary. Thanks Nilirad! --- examples/ecs/archetype_invariants.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/ecs/archetype_invariants.rs b/examples/ecs/archetype_invariants.rs index d7a9902f50fee..021989fa66b4b 100644 --- a/examples/ecs/archetype_invariants.rs +++ b/examples/ecs/archetype_invariants.rs @@ -70,7 +70,7 @@ fn spawn_player(mut commands: Commands) { commands.spawn().insert_bundle((Player, Mage, Life)); } -fn position_player(mut commands: Commands, query: Query, Added)>) { +fn position_player(mut commands: Commands, query: Query>) { let player_entity = query.single(); // Because of our invariants, these components need to be added together. From 5ded872dd44fc629bcd7ee58366827e1c0f8b136 Mon Sep 17 00:00:00 2001 From: Rose Peck Date: Mon, 11 Jul 2022 11:48:06 -0700 Subject: [PATCH 52/59] Add trailing period to docstring Co-authored-by: Federico Rinaldi --- crates/bevy_ecs/src/component.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 66a7808466d21..21b526722dfec 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -252,7 +252,7 @@ impl SparseSetIndex for ComponentId { } } -/// Returns the shortened type names of the provided [`ComponentId`]s +/// Returns the shortened type names of the provided [`ComponentId`]s. /// /// Uses [`get_short_name`] to strip the module paths of the items, resulting in cleaner lists. pub fn display_component_id_types<'a, I: Iterator>( From 66910814304cf2209fdcbf6acd35491d4f288dc8 Mon Sep 17 00:00:00 2001 From: plof27 Date: Mon, 11 Jul 2022 14:55:39 -0400 Subject: [PATCH 53/59] Make archetype testing functions pub(crate) instead of pub If users want to test archetypes, they can do it through a world. This is probably the more natural pattern for end users anyway. --- crates/bevy_ecs/src/world/archetype_invariants.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index c2667807e1f07..4e4814bc34280 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -253,7 +253,7 @@ impl UntypedArchetypeInvariant { /// /// # Panics /// Panics if the archetype invariant is violated. - pub fn test_archetype( + pub(crate) fn test_archetype( &self, component_ids_of_archetype: impl Iterator, components: &Components, @@ -434,7 +434,7 @@ impl ArchetypeInvariants { /// # Panics /// /// Panics if any archetype invariant is violated. - pub fn test_archetype( + pub(crate) fn test_archetype( &self, component_ids_of_archetype: impl Iterator, components: &Components, From 835f7cdbbd1128d8c208ca60a003de222b8983ac Mon Sep 17 00:00:00 2001 From: plof27 Date: Wed, 27 Jul 2022 18:15:38 -0400 Subject: [PATCH 54/59] Remove references to how to get `component_ids` Nilirad is right. This doesn't belong here, it belongs on the `Archetype` docs --- crates/bevy_ecs/src/world/archetype_invariants.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index 4e4814bc34280..1d02c9fd24c59 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -247,8 +247,7 @@ pub struct UntypedArchetypeInvariant { impl UntypedArchetypeInvariant { /// Asserts that the provided iterator of [`ComponentId`]s obeys this archetype invariant. /// - /// `component_ids` is generally provided via the `components` field on [`Archetype`](crate::archetype::Archetype). - /// When testing against multiple archetypes, [`ArchetypeInvariants::test_archetype`] is preferred, + /// When testing against multiple archetype invariants, [`ArchetypeInvariants::test_archetype`] is preferred, /// as it can more efficiently cache checks between archetypes. /// /// # Panics @@ -429,8 +428,6 @@ impl ArchetypeInvariants { /// Asserts that the provided iterator of [`ComponentId`]s obeys all archetype invariants. /// - /// `component_ids` is generally provided via the `components` field on [`Archetype`](crate::archetype::Archetype). - /// /// # Panics /// /// Panics if any archetype invariant is violated. From 3649696875e7d8af17b3a2aa06a9de7a983cfe84 Mon Sep 17 00:00:00 2001 From: plof27 Date: Fri, 29 Jul 2022 08:26:37 -0400 Subject: [PATCH 55/59] Fix statement docs --- crates/bevy_ecs/src/world/archetype_invariants.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index 1d02c9fd24c59..a479aadf6f96e 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -329,10 +329,10 @@ impl UntypedArchetypeStatement { } } - /// Returns formatted string describing this archetype invariant. + /// Returns formatted string describing this archetype statement. /// /// For Rust types, the names should match the type name. - /// If any [`ComponentId`]s in the invariant have not been registered in the world, + /// If any [`ComponentId`]s in the statement have not been registered in the world, /// then the raw component id will be returned instead. pub fn display(&self, components: &Components) -> String { let component_names = display_component_id_types( From 21a1a2350d08982d5e86293f4d43db02ed8146a2 Mon Sep 17 00:00:00 2001 From: plof27 Date: Fri, 29 Jul 2022 10:03:22 -0400 Subject: [PATCH 56/59] Rework helper method API to be defined on Bundles The new API is equivalent, but it has a nicer syntax, especially for binary invariants. It may not seem like much of an improvement now, but it will get much cleaner once "components as bundles" is merged. --- crates/bevy_ecs/src/lib.rs | 5 +- .../src/world/archetype_invariants.rs | 212 ++++++++++-------- examples/ecs/archetype_invariants.rs | 14 +- 3 files changed, 130 insertions(+), 101 deletions(-) diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 07d18a10fbc64..f418c9e0cf537 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -42,7 +42,10 @@ pub mod prelude { NonSendMut, ParallelCommands, ParamSet, Query, RemovedComponents, Res, ResMut, System, SystemParamFunction, }, - world::{ArchetypeInvariant, ArchetypeStatement, FromWorld, Mut, World}, + world::{ + ArchetypeInvariant, ArchetypeInvariantHelpers, ArchetypeStatement, FromWorld, Mut, + World, + }, }; } diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index a479aadf6f96e..a45d5eed460ea 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -51,77 +51,6 @@ impl ArchetypeInvariant { consequence: self.consequence.into_untyped(world), } } - - /// Creates an archetype invariant where any component of `B1` forbids every comonent from `B2`, and vice versa. - /// - /// In other words, if any component from `B1` is present, then none of the components from `B2` can be present. - /// Although this appears asymmetric, it actually implies its own converse. - /// This is particularly useful for avoiding query conflicts. - #[inline] - pub fn forbids() -> Self { - Self { - premise: ArchetypeStatement::::any_of(), - consequence: ArchetypeStatement::::none_of(), - } - } - - /// Creates an archetype invariant where components of `B1` require all the components of `B2`. - /// - /// In other words, if any component from `B1` is present, then all of the components from `B2` must be. - #[inline] - pub fn requires() -> Self { - Self { - premise: ArchetypeStatement::::any_of(), - consequence: ArchetypeStatement::::all_of(), - } - } - - /// Creates an archetype invariant where components of `B1` require at least one component of `B2`. - /// - /// In other words, if any component from `B1` is present, then at least one component from `B2` must be. - #[inline] - pub fn requires_one() -> Self { - Self { - premise: ArchetypeStatement::::any_of(), - consequence: ArchetypeStatement::::any_of(), - } - } -} - -impl ArchetypeInvariant { - /// Creates an archetype invariant where all components of `B` require each other. - /// - /// In other words, if any component of this bundle is present, then all of them must be. - #[inline] - pub fn atomic() -> Self { - Self { - premise: ArchetypeStatement::::any_of(), - consequence: ArchetypeStatement::::all_of(), - } - } - - /// Creates an archetype where components of `B` cannot appear with each other. - /// - /// In other words, if any component of this bundle is present, then no others can be. - /// This is particularly useful for creating enum-like groups of components, such as `Dead` and `Alive`. - #[inline] - pub fn disjoint() -> Self { - Self { - premise: ArchetypeStatement::::any_of(), - consequence: ArchetypeStatement::::at_most_one_of(), - } - } - - /// Creates an archetype invariant where components of `B` can only appear with each other. - /// - /// In other words, if any component of this bundle is present, then _only_ components from this bundle can be present. - #[inline] - pub fn exclusive() -> Self { - Self { - premise: ArchetypeStatement::::any_of(), - consequence: ArchetypeStatement::::only(), - } - } } /// A statement about the presence or absence of some subset of components in the given [`Bundle`]. @@ -230,6 +159,106 @@ impl ArchetypeStatement<()> { } } +/// Defines helper methods to eaily contruct common archetype invariants. +/// +/// For more details on each method, see the implementation docs. +/// This trait is sealed: it should never be implemented by dependencies. +pub trait ArchetypeInvariantHelpers: private::Sealed { + fn forbids() -> ArchetypeInvariant; + + fn requires() -> ArchetypeInvariant; + + fn requires_one() -> ArchetypeInvariant; + + fn atomic() -> ArchetypeInvariant; + + fn disjoint() -> ArchetypeInvariant; + + fn exclusive() -> ArchetypeInvariant; +} + +impl ArchetypeInvariantHelpers for B { + /// Creates an archetype invariant where any component of `B` forbids every comonent from `B2`, and vice versa. + /// + /// In other words, if any component from `B` is present, then none of the components from `B2` can be present. + /// Although this appears asymmetric, it actually implies its own converse. + /// This is particularly useful for avoiding query conflicts. + #[inline] + fn forbids() -> ArchetypeInvariant { + ArchetypeInvariant { + premise: ArchetypeStatement::::any_of(), + consequence: ArchetypeStatement::::none_of(), + } + } + + /// Creates an archetype invariant where components of `B` require all the components of `B2`. + /// + /// In other words, if any component from `B` is present, then all of the components from `B2` must be. + #[inline] + fn requires() -> ArchetypeInvariant { + ArchetypeInvariant { + premise: ArchetypeStatement::::any_of(), + consequence: ArchetypeStatement::::all_of(), + } + } + + /// Creates an archetype invariant where components of `B` require at least one component of `B2`. + /// + /// In other words, if any component from `B` is present, then at least one component from `B2` must be. + #[inline] + fn requires_one() -> ArchetypeInvariant { + ArchetypeInvariant { + premise: ArchetypeStatement::::any_of(), + consequence: ArchetypeStatement::::any_of(), + } + } + + /// Creates an archetype invariant where all components of `B` require each other. + /// + /// In other words, if any component of this bundle is present, then all of them must be. + #[inline] + fn atomic() -> ArchetypeInvariant { + ArchetypeInvariant { + premise: ArchetypeStatement::::any_of(), + consequence: ArchetypeStatement::::all_of(), + } + } + + /// Creates an archetype where components of `B` cannot appear with each other. + /// + /// In other words, if any component of this bundle is present, then no others can be. + /// This is particularly useful for creating enum-like groups of components, such as `Dead` and `Alive`. + #[inline] + fn disjoint() -> ArchetypeInvariant { + ArchetypeInvariant { + premise: ArchetypeStatement::::any_of(), + consequence: ArchetypeStatement::::at_most_one_of(), + } + } + + /// Creates an archetype invariant where components of `B` can only appear with each other. + /// + /// In other words, if any component of this bundle is present, then _only_ components from this bundle can be present. + #[inline] + fn exclusive() -> ArchetypeInvariant { + ArchetypeInvariant { + premise: ArchetypeStatement::::any_of(), + consequence: ArchetypeStatement::::only(), + } + } +} + +/// A special module used to prevent [`ArchetypeInvariantHelpers`] from being implemented by any other module. +/// +/// For more information, see: . +mod private { + use crate::prelude::Bundle; + + pub trait Sealed {} + + impl Sealed for B {} +} + /// A type-erased version of [`ArchetypeInvariant`]. /// /// Intended to be used with dynamic components that cannot be represented with Rust types. @@ -475,6 +504,7 @@ mod tests { use crate as bevy_ecs; use crate::{ component::Component, world::archetype_invariants::ArchetypeInvariant, + world::archetype_invariants::ArchetypeInvariantHelpers, world::archetype_invariants::ArchetypeStatement, world::World, }; @@ -492,7 +522,7 @@ mod tests { let mut world = World::new(); world.spawn().insert_bundle((A, B, C)); - world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::atomic()); + world.add_archetype_invariant(<(A, B, C)>::atomic()); } #[test] @@ -501,7 +531,7 @@ mod tests { let mut world = World::new(); world.spawn().insert_bundle((A, B)); - world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::atomic()); + world.add_archetype_invariant(<(A, B, C)>::atomic()); } #[test] @@ -509,8 +539,7 @@ mod tests { let mut world = World::new(); world.spawn().insert_bundle((A, B, C)); - let archetype_invariant = - ArchetypeInvariant::<(A, B, C)>::atomic().into_untyped(&mut world); + let archetype_invariant = <(A, B, C)>::atomic().into_untyped(&mut world); world.add_untyped_archetype_invariant(archetype_invariant); } @@ -520,8 +549,7 @@ mod tests { let mut world = World::new(); world.spawn().insert_bundle((A, B)); - let archetype_invariant = - ArchetypeInvariant::<(A, B, C)>::atomic().into_untyped(&mut world); + let archetype_invariant = <(A, B, C)>::atomic().into_untyped(&mut world); world.add_untyped_archetype_invariant(archetype_invariant); } @@ -565,7 +593,7 @@ mod tests { fn forbids_happy() { let mut world = World::new(); - world.add_archetype_invariant(ArchetypeInvariant::<(A,), (B, C)>::forbids()); + world.add_archetype_invariant(<(A,)>::forbids::<(B, C)>()); world.spawn().insert(A); world.spawn().insert_bundle((B, C)); } @@ -575,7 +603,7 @@ mod tests { fn forbids_sad() { let mut world = World::new(); - world.add_archetype_invariant(ArchetypeInvariant::<(A,), (B, C)>::forbids()); + world.add_archetype_invariant(<(A,)>::forbids::<(B, C)>()); world.spawn().insert_bundle((A, B)); } @@ -583,7 +611,7 @@ mod tests { fn requires_happy() { let mut world = World::new(); - world.add_archetype_invariant(ArchetypeInvariant::<(A,), (B, C)>::requires()); + world.add_archetype_invariant(<(A,)>::requires::<(B, C)>()); world.spawn().insert_bundle((A, B, C)); world.spawn().insert_bundle((B, C)); } @@ -593,7 +621,7 @@ mod tests { fn requires_sad_partial() { let mut world = World::new(); - world.add_archetype_invariant(ArchetypeInvariant::<(A,), (B, C)>::requires()); + world.add_archetype_invariant(<(A,)>::requires::<(B, C)>()); world.spawn().insert_bundle((A, B)); } @@ -602,7 +630,7 @@ mod tests { fn requires_sad_none() { let mut world = World::new(); - world.add_archetype_invariant(ArchetypeInvariant::<(A,), (B, C)>::requires()); + world.add_archetype_invariant(<(A,)>::requires::<(B, C)>()); world.spawn().insert(A); } @@ -610,7 +638,7 @@ mod tests { fn requires_one_happy() { let mut world = World::new(); - world.add_archetype_invariant(ArchetypeInvariant::<(A,), (B, C)>::requires_one()); + world.add_archetype_invariant(<(A,)>::requires_one::<(B, C)>()); world.spawn().insert_bundle((A, B, C)); world.spawn().insert_bundle((A, B)); world.spawn().insert_bundle((B, C)); @@ -621,7 +649,7 @@ mod tests { fn requires_one_sad() { let mut world = World::new(); - world.add_archetype_invariant(ArchetypeInvariant::<(A,), (B, C)>::requires_one()); + world.add_archetype_invariant(<(A,)>::requires_one::<(B, C)>()); world.spawn().insert(A); } @@ -629,7 +657,7 @@ mod tests { fn atomic_happy() { let mut world = World::new(); - world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::atomic()); + world.add_archetype_invariant(<(A, B, C)>::atomic()); world.spawn().insert_bundle((A, B, C)); } @@ -638,7 +666,7 @@ mod tests { fn atomic_sad() { let mut world = World::new(); - world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::atomic()); + world.add_archetype_invariant(<(A, B, C)>::atomic()); world.spawn().insert_bundle((A, B)); } @@ -646,7 +674,7 @@ mod tests { fn disjoint_happy() { let mut world = World::new(); - world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::disjoint()); + world.add_archetype_invariant(<(A, B, C)>::disjoint()); world.spawn().insert(A); world.spawn().insert(B); world.spawn().insert(C); @@ -657,7 +685,7 @@ mod tests { fn disjoint_sad_partial() { let mut world = World::new(); - world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::disjoint()); + world.add_archetype_invariant(<(A, B, C)>::disjoint()); world.spawn().insert_bundle((A, B)); } @@ -666,7 +694,7 @@ mod tests { fn disjoint_sad_all() { let mut world = World::new(); - world.add_archetype_invariant(ArchetypeInvariant::<(A, B, C)>::disjoint()); + world.add_archetype_invariant(<(A, B, C)>::disjoint()); world.spawn().insert_bundle((A, B, C)); } @@ -674,7 +702,7 @@ mod tests { fn exclusive_happy() { let mut world = World::new(); - world.add_archetype_invariant(ArchetypeInvariant::<(A, B)>::exclusive()); + world.add_archetype_invariant(<(A, B)>::exclusive()); world.spawn().insert_bundle((A, B)); world.spawn().insert(A); world.spawn().insert(C); @@ -685,7 +713,7 @@ mod tests { fn exclusive_sad_partial() { let mut world = World::new(); - world.add_archetype_invariant(ArchetypeInvariant::<(A, B)>::exclusive()); + world.add_archetype_invariant(<(A, B)>::exclusive()); world.spawn().insert_bundle((A, C)); } @@ -694,7 +722,7 @@ mod tests { fn exclusive_sad_all() { let mut world = World::new(); - world.add_archetype_invariant(ArchetypeInvariant::<(A, B)>::exclusive()); + world.add_archetype_invariant(<(A, B)>::exclusive()); world.spawn().insert_bundle((A, B, C)); } } diff --git a/examples/ecs/archetype_invariants.rs b/examples/ecs/archetype_invariants.rs index 021989fa66b4b..869b7e417e7d7 100644 --- a/examples/ecs/archetype_invariants.rs +++ b/examples/ecs/archetype_invariants.rs @@ -22,24 +22,22 @@ fn main() { // Archetype invariants are constructed in terms of bundles; // use (MyComponent, ) to construct a bundle with a single item. // This invariant ensures that Player and Camera can never be found together on the same entity. - .add_archetype_invariant(ArchetypeInvariant::<(Player,), (Camera,)>::forbids()) + .add_archetype_invariant(<(Player,)>::forbids::<(Camera,)>()) // This invariant ensures that the `GlobalTransform` component is always found with the `Transform` component, and vice versa. - .add_archetype_invariant(ArchetypeInvariant::<(GlobalTransform, Transform)>::atomic()) + .add_archetype_invariant(<(GlobalTransform, Transform)>::atomic()) // This invariant ensures that the `Player` component is always found with the `Life` component. // This requirement is only in one direction: it is possible to have entities which have `Life`, but not `Player` (like enemies). - .add_archetype_invariant(ArchetypeInvariant::<(Player,), (Life,)>::requires()) + .add_archetype_invariant(<(Player,)>::requires::<(Life,)>()) // The `disjoint` invariant ensures that at most one component from the bundle is present on a given entity. // This way, an entity never belongs to more than one RPG class at once. // This is useful for creating groups of components that behave similarly to an enum. - .add_archetype_invariant(ArchetypeInvariant::<(Archer, Swordsman, Mage)>::disjoint()) + .add_archetype_invariant(<(Archer, Swordsman, Mage)>::disjoint()) // This invariant indicates that any entity with the `Player` component always has // at least one component in the `(Archer, Swordsman, Mage)` bundle. // We could use a type alias to improve clarity and avoid errors caused by duplication: // type Class = (Archer, Swordsman, Mage); - .add_archetype_invariant( - ArchetypeInvariant::<(Player,), (Archer, Swordsman, Mage)>::requires_one(), - ) - // You can also specify custom invariants by constructing `ArchetypeInvariant directly. + .add_archetype_invariant(<(Player,)>::requires_one::<(Archer, Swordsman, Mage)>()) + // You can also specify custom invariants by constructing `ArchetypeInvariant` directly. // This invariant specifies that the `Node` component cannot appear on any entity in our world. // We're not using bevy_ui in our App, so this component should never show up. .add_archetype_invariant(ArchetypeInvariant { From 1acce114c273681e5769b2a97954397b3b7edffb Mon Sep 17 00:00:00 2001 From: plof27 Date: Sat, 8 Oct 2022 18:59:21 -0400 Subject: [PATCH 57/59] Fix merge issues --- crates/bevy_app/src/app.rs | 2 +- .../src/world/archetype_invariants.rs | 71 ++++++++++--------- crates/bevy_ecs/src/world/mod.rs | 1 + examples/ecs/archetype_invariants.rs | 4 +- 4 files changed, 41 insertions(+), 37 deletions(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index a74720d8528a3..111b5eda6726f 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -3,7 +3,7 @@ pub use bevy_derive::AppLabel; use bevy_derive::{Deref, DerefMut}; use bevy_ecs::{ event::{Event, Events}, - prelude::{Bundle, FromWorld, IntoExclusiveSystem}, + prelude::{Bundle, FromWorld}, schedule::{ IntoSystemDescriptor, Schedule, ShouldRun, Stage, StageLabel, State, StateData, SystemSet, SystemStage, diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index a45d5eed460ea..96ce622618a8f 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -30,7 +30,7 @@ use crate::{ /// /// When working with dynamic component types (for non-Rust components), /// use [`UntypedArchetypeInvariant`] and [`UntypedArchetypeStatement`] instead. -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq, Eq)] pub struct ArchetypeInvariant { /// Defines which entities this invariant applies to. /// This is the "if" of the if/then clause. @@ -63,7 +63,7 @@ impl ArchetypeInvariant { /// For the statements about a single component `C`, wrap it in a single-component bundle `(C,)`. /// For single component bundles, `AllOf` and `AnyOf` are equivalent. /// Prefer `ArchetypeStatement::<(C,)>::all_of` over `ArchetypeStatement::<(C,)>::any_of` for consistency and clarity. -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq, Eq)] pub enum ArchetypeStatement { /// Evaluates to true if and only if the entity has all of the components present in the bundle `B`. AllOf(PhantomData), @@ -95,7 +95,10 @@ impl ArchetypeStatement { /// /// Requires mutable world access, since the components might not have been added to the world yet. pub fn into_untyped(self, world: &mut World) -> UntypedArchetypeStatement { - let component_ids = B::component_ids(&mut world.components, &mut world.storages); + let mut component_ids = Vec::new(); + B::component_ids(&mut world.components, &mut world.storages, &mut |id| { + component_ids.push(id); + }); let component_ids: HashSet = component_ids.into_iter().collect(); match self { @@ -263,7 +266,7 @@ mod private { /// /// Intended to be used with dynamic components that cannot be represented with Rust types. /// Prefer [`ArchetypeInvariant`] when possible. -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq, Eq)] pub struct UntypedArchetypeInvariant { /// Defines which entities this invariant applies to. /// This is the "if" of the if/then clause. @@ -316,7 +319,7 @@ impl UntypedArchetypeInvariant { /// /// Intended to be used with dynamic components that cannot be represented with Rust types. /// Prefer [`ArchetypeStatement`] when possible. -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq, Eq)] pub enum UntypedArchetypeStatement { /// Evaluates to true if and only if the entity has all of the components present in the set. AllOf(HashSet), @@ -521,7 +524,7 @@ mod tests { fn on_insert_happy() { let mut world = World::new(); - world.spawn().insert_bundle((A, B, C)); + world.spawn((A, B, C)); world.add_archetype_invariant(<(A, B, C)>::atomic()); } @@ -530,7 +533,7 @@ mod tests { fn on_insert_sad() { let mut world = World::new(); - world.spawn().insert_bundle((A, B)); + world.spawn((A, B)); world.add_archetype_invariant(<(A, B, C)>::atomic()); } @@ -538,7 +541,7 @@ mod tests { fn on_insert_untyped_happy() { let mut world = World::new(); - world.spawn().insert_bundle((A, B, C)); + world.spawn((A, B, C)); let archetype_invariant = <(A, B, C)>::atomic().into_untyped(&mut world); world.add_untyped_archetype_invariant(archetype_invariant); } @@ -548,7 +551,7 @@ mod tests { fn on_insert_untyped_sad() { let mut world = World::new(); - world.spawn().insert_bundle((A, B)); + world.spawn((A, B)); let archetype_invariant = <(A, B, C)>::atomic().into_untyped(&mut world); world.add_untyped_archetype_invariant(archetype_invariant); } @@ -570,7 +573,7 @@ mod tests { // Since invariants are only checked when archetypes are created, // we must add something to trigger the check. - world.spawn().insert(A); + world.spawn(A); } #[test] @@ -586,7 +589,7 @@ mod tests { // Since invariants are only checked when archetypes are created, // we must add something to trigger the check. - world.spawn().insert(A); + world.spawn(A); } #[test] @@ -594,8 +597,8 @@ mod tests { let mut world = World::new(); world.add_archetype_invariant(<(A,)>::forbids::<(B, C)>()); - world.spawn().insert(A); - world.spawn().insert_bundle((B, C)); + world.spawn(A); + world.spawn((B, C)); } #[test] @@ -604,7 +607,7 @@ mod tests { let mut world = World::new(); world.add_archetype_invariant(<(A,)>::forbids::<(B, C)>()); - world.spawn().insert_bundle((A, B)); + world.spawn((A, B)); } #[test] @@ -612,8 +615,8 @@ mod tests { let mut world = World::new(); world.add_archetype_invariant(<(A,)>::requires::<(B, C)>()); - world.spawn().insert_bundle((A, B, C)); - world.spawn().insert_bundle((B, C)); + world.spawn((A, B, C)); + world.spawn((B, C)); } #[test] @@ -622,7 +625,7 @@ mod tests { let mut world = World::new(); world.add_archetype_invariant(<(A,)>::requires::<(B, C)>()); - world.spawn().insert_bundle((A, B)); + world.spawn((A, B)); } #[test] @@ -631,7 +634,7 @@ mod tests { let mut world = World::new(); world.add_archetype_invariant(<(A,)>::requires::<(B, C)>()); - world.spawn().insert(A); + world.spawn(A); } #[test] @@ -639,9 +642,9 @@ mod tests { let mut world = World::new(); world.add_archetype_invariant(<(A,)>::requires_one::<(B, C)>()); - world.spawn().insert_bundle((A, B, C)); - world.spawn().insert_bundle((A, B)); - world.spawn().insert_bundle((B, C)); + world.spawn((A, B, C)); + world.spawn((A, B)); + world.spawn((B, C)); } #[test] @@ -650,7 +653,7 @@ mod tests { let mut world = World::new(); world.add_archetype_invariant(<(A,)>::requires_one::<(B, C)>()); - world.spawn().insert(A); + world.spawn(A); } #[test] @@ -658,7 +661,7 @@ mod tests { let mut world = World::new(); world.add_archetype_invariant(<(A, B, C)>::atomic()); - world.spawn().insert_bundle((A, B, C)); + world.spawn((A, B, C)); } #[test] @@ -667,7 +670,7 @@ mod tests { let mut world = World::new(); world.add_archetype_invariant(<(A, B, C)>::atomic()); - world.spawn().insert_bundle((A, B)); + world.spawn((A, B)); } #[test] @@ -675,9 +678,9 @@ mod tests { let mut world = World::new(); world.add_archetype_invariant(<(A, B, C)>::disjoint()); - world.spawn().insert(A); - world.spawn().insert(B); - world.spawn().insert(C); + world.spawn(A); + world.spawn(B); + world.spawn(C); } #[test] @@ -686,7 +689,7 @@ mod tests { let mut world = World::new(); world.add_archetype_invariant(<(A, B, C)>::disjoint()); - world.spawn().insert_bundle((A, B)); + world.spawn((A, B)); } #[test] @@ -695,7 +698,7 @@ mod tests { let mut world = World::new(); world.add_archetype_invariant(<(A, B, C)>::disjoint()); - world.spawn().insert_bundle((A, B, C)); + world.spawn((A, B, C)); } #[test] @@ -703,9 +706,9 @@ mod tests { let mut world = World::new(); world.add_archetype_invariant(<(A, B)>::exclusive()); - world.spawn().insert_bundle((A, B)); - world.spawn().insert(A); - world.spawn().insert(C); + world.spawn((A, B)); + world.spawn(A); + world.spawn(C); } #[test] @@ -714,7 +717,7 @@ mod tests { let mut world = World::new(); world.add_archetype_invariant(<(A, B)>::exclusive()); - world.spawn().insert_bundle((A, C)); + world.spawn((A, C)); } #[test] @@ -723,6 +726,6 @@ mod tests { let mut world = World::new(); world.add_archetype_invariant(<(A, B)>::exclusive()); - world.spawn().insert_bundle((A, B, C)); + world.spawn((A, B, C)); } } diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index d7e6c6ed4344c..bb38b212a9bb8 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -460,6 +460,7 @@ impl World { &mut self.components, &mut self.storages, *self.change_tick.get_mut(), + &self.archetype_invariants, ); // SAFETY: bundle's type matches `bundle_info`, entity is allocated but non-existent diff --git a/examples/ecs/archetype_invariants.rs b/examples/ecs/archetype_invariants.rs index 869b7e417e7d7..08fce450c7cf7 100644 --- a/examples/ecs/archetype_invariants.rs +++ b/examples/ecs/archetype_invariants.rs @@ -65,7 +65,7 @@ struct Swordsman; struct Mage; fn spawn_player(mut commands: Commands) { - commands.spawn().insert_bundle((Player, Mage, Life)); + commands.spawn((Player, Mage, Life)); } fn position_player(mut commands: Commands, query: Query>) { @@ -76,7 +76,7 @@ fn position_player(mut commands: Commands, query: Query>) // where it has only one of the two components. commands .entity(player_entity) - .insert_bundle((GlobalTransform::default(), Transform::default())); + .insert((GlobalTransform::default(), Transform::default())); // Adding the components one at a time panics. // Track this limitation at https://github.com/bevyengine/bevy/issues/5074. From ed16ff796d7208947c54b7194c04c7f3212cb4eb Mon Sep 17 00:00:00 2001 From: plof27 Date: Sat, 8 Oct 2022 19:09:55 -0400 Subject: [PATCH 58/59] =?UTF-8?q?=F0=9F=A5=BA?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/world/archetype_invariants.rs | 21 +++++++++---------- examples/ecs/archetype_invariants.rs | 8 +++---- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index 96ce622618a8f..7be2853edc8af 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -95,11 +95,10 @@ impl ArchetypeStatement { /// /// Requires mutable world access, since the components might not have been added to the world yet. pub fn into_untyped(self, world: &mut World) -> UntypedArchetypeStatement { - let mut component_ids = Vec::new(); + let mut component_ids = HashSet::new(); B::component_ids(&mut world.components, &mut world.storages, &mut |id| { - component_ids.push(id); + component_ids.insert(id); }); - let component_ids: HashSet = component_ids.into_iter().collect(); match self { ArchetypeStatement::AllOf(_) => UntypedArchetypeStatement::AllOf(component_ids), @@ -181,7 +180,7 @@ pub trait ArchetypeInvariantHelpers: private::Sealed { } impl ArchetypeInvariantHelpers for B { - /// Creates an archetype invariant where any component of `B` forbids every comonent from `B2`, and vice versa. + /// Creates an archetype invariant where any component of `B` forbids every component from `B2`, and vice versa. /// /// In other words, if any component from `B` is present, then none of the components from `B2` can be present. /// Although this appears asymmetric, it actually implies its own converse. @@ -596,7 +595,7 @@ mod tests { fn forbids_happy() { let mut world = World::new(); - world.add_archetype_invariant(<(A,)>::forbids::<(B, C)>()); + world.add_archetype_invariant(A::forbids::<(B, C)>()); world.spawn(A); world.spawn((B, C)); } @@ -606,7 +605,7 @@ mod tests { fn forbids_sad() { let mut world = World::new(); - world.add_archetype_invariant(<(A,)>::forbids::<(B, C)>()); + world.add_archetype_invariant(A::forbids::<(B, C)>()); world.spawn((A, B)); } @@ -614,7 +613,7 @@ mod tests { fn requires_happy() { let mut world = World::new(); - world.add_archetype_invariant(<(A,)>::requires::<(B, C)>()); + world.add_archetype_invariant(A::requires::<(B, C)>()); world.spawn((A, B, C)); world.spawn((B, C)); } @@ -624,7 +623,7 @@ mod tests { fn requires_sad_partial() { let mut world = World::new(); - world.add_archetype_invariant(<(A,)>::requires::<(B, C)>()); + world.add_archetype_invariant(A::requires::<(B, C)>()); world.spawn((A, B)); } @@ -633,7 +632,7 @@ mod tests { fn requires_sad_none() { let mut world = World::new(); - world.add_archetype_invariant(<(A,)>::requires::<(B, C)>()); + world.add_archetype_invariant(A::requires::<(B, C)>()); world.spawn(A); } @@ -641,7 +640,7 @@ mod tests { fn requires_one_happy() { let mut world = World::new(); - world.add_archetype_invariant(<(A,)>::requires_one::<(B, C)>()); + world.add_archetype_invariant(A::requires_one::<(B, C)>()); world.spawn((A, B, C)); world.spawn((A, B)); world.spawn((B, C)); @@ -652,7 +651,7 @@ mod tests { fn requires_one_sad() { let mut world = World::new(); - world.add_archetype_invariant(<(A,)>::requires_one::<(B, C)>()); + world.add_archetype_invariant(A::requires_one::<(B, C)>()); world.spawn(A); } diff --git a/examples/ecs/archetype_invariants.rs b/examples/ecs/archetype_invariants.rs index 08fce450c7cf7..9b32e788f04a3 100644 --- a/examples/ecs/archetype_invariants.rs +++ b/examples/ecs/archetype_invariants.rs @@ -22,12 +22,12 @@ fn main() { // Archetype invariants are constructed in terms of bundles; // use (MyComponent, ) to construct a bundle with a single item. // This invariant ensures that Player and Camera can never be found together on the same entity. - .add_archetype_invariant(<(Player,)>::forbids::<(Camera,)>()) + .add_archetype_invariant(Player::forbids::()) // This invariant ensures that the `GlobalTransform` component is always found with the `Transform` component, and vice versa. .add_archetype_invariant(<(GlobalTransform, Transform)>::atomic()) // This invariant ensures that the `Player` component is always found with the `Life` component. // This requirement is only in one direction: it is possible to have entities which have `Life`, but not `Player` (like enemies). - .add_archetype_invariant(<(Player,)>::requires::<(Life,)>()) + .add_archetype_invariant(Player::requires::()) // The `disjoint` invariant ensures that at most one component from the bundle is present on a given entity. // This way, an entity never belongs to more than one RPG class at once. // This is useful for creating groups of components that behave similarly to an enum. @@ -36,12 +36,12 @@ fn main() { // at least one component in the `(Archer, Swordsman, Mage)` bundle. // We could use a type alias to improve clarity and avoid errors caused by duplication: // type Class = (Archer, Swordsman, Mage); - .add_archetype_invariant(<(Player,)>::requires_one::<(Archer, Swordsman, Mage)>()) + .add_archetype_invariant(Player::requires_one::<(Archer, Swordsman, Mage)>()) // You can also specify custom invariants by constructing `ArchetypeInvariant` directly. // This invariant specifies that the `Node` component cannot appear on any entity in our world. // We're not using bevy_ui in our App, so this component should never show up. .add_archetype_invariant(ArchetypeInvariant { - premise: ArchetypeStatement::<(Node,)>::all_of(), + premise: ArchetypeStatement::::all_of(), consequence: ArchetypeStatement::always_false(), }) .add_startup_system(spawn_player) From b7b7d47bfbd6e7bad055b7ae52e4882e95aa59f8 Mon Sep 17 00:00:00 2001 From: plof27 Date: Sat, 8 Oct 2022 19:54:37 -0400 Subject: [PATCH 59/59] =?UTF-8?q?Update=20docstrings=20to=20match=20?= =?UTF-8?q?=F0=9F=A5=BA?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/bevy_ecs/src/world/archetype_invariants.rs | 6 +++--- examples/ecs/archetype_invariants.rs | 3 +-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index 7be2853edc8af..8dfea1f8d2835 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -60,9 +60,9 @@ impl ArchetypeInvariant { /// When used as a premise, the archetype invariant matches all entities which satisfy the statement. /// When used as a consequence, then the statment must be true for all entities that were matched by the premise. /// -/// For the statements about a single component `C`, wrap it in a single-component bundle `(C,)`. -/// For single component bundles, `AllOf` and `AnyOf` are equivalent. -/// Prefer `ArchetypeStatement::<(C,)>::all_of` over `ArchetypeStatement::<(C,)>::any_of` for consistency and clarity. +/// Statements can also be defined on single components, like so: `ArchetypeStatement::::all_of`. +/// For single component statements, `AllOf` and `AnyOf` are equivalent. +/// Prefer `ArchetypeStatement::::all_of` over `ArchetypeStatement::::any_of` for consistency and clarity. #[derive(Clone, Debug, PartialEq, Eq)] pub enum ArchetypeStatement { /// Evaluates to true if and only if the entity has all of the components present in the bundle `B`. diff --git a/examples/ecs/archetype_invariants.rs b/examples/ecs/archetype_invariants.rs index 9b32e788f04a3..e1ff64ce5c502 100644 --- a/examples/ecs/archetype_invariants.rs +++ b/examples/ecs/archetype_invariants.rs @@ -19,8 +19,7 @@ use bevy::prelude::*; fn main() { App::new() - // Archetype invariants are constructed in terms of bundles; - // use (MyComponent, ) to construct a bundle with a single item. + // Archetype invariants are constructed in terms of either bundles or single components. // This invariant ensures that Player and Camera can never be found together on the same entity. .add_archetype_invariant(Player::forbids::()) // This invariant ensures that the `GlobalTransform` component is always found with the `Transform` component, and vice versa.