From cf4449570bb55f66b7bd689b9390ff156d6f0d42 Mon Sep 17 00:00:00 2001 From: rewin Date: Tue, 3 Sep 2024 07:11:09 +0300 Subject: [PATCH 01/25] Remove with required --- crates/bevy_ecs/src/lib.rs | 81 ++++++++++++++++++++++ crates/bevy_ecs/src/system/commands/mod.rs | 45 ++++++++++++ crates/bevy_ecs/src/world/entity_ref.rs | 17 +++++ 3 files changed, 143 insertions(+) diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 442c25d9a2662..e6e61337efd5c 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -2023,6 +2023,87 @@ mod tests { assert!(e.contains::()); } + #[test] + fn remove_component_and_his_required_components() { + #[derive(Component)] + #[require(Y)] + struct X; + + #[derive(Component, Default)] + struct Y; + + #[derive(Component)] + struct Z; + + let mut world = World::new(); + + let e = world.spawn((X, Z)).id(); + assert!(world.entity(e).contains::()); + assert!(world.entity(e).contains::()); + assert!(world.entity(e).contains::()); + + //check that `remove` works as expected + world.entity_mut(e).remove::(); + assert!(!world.entity(e).contains::()); + assert!(world.entity(e).contains::()); + assert!(world.entity(e).contains::()); + + world.entity_mut(e).insert(X); + assert!(world.entity(e).contains::()); + assert!(world.entity(e).contains::()); + assert!(world.entity(e).contains::()); + + //remove `X` again and ensure that `Y` was removed too + world.entity_mut(e).remove_with_required::(); + assert!(!world.entity(e).contains::()); + assert!(!world.entity(e).contains::()); + assert!(world.entity(e).contains::()); + } + + #[test] + fn remove_bundle_and_his_required_components() { + #[derive(Component, Default)] + #[require(Y)] + struct X; + + #[derive(Component, Default)] + struct Y; + + #[derive(Component, Default)] + #[require(W)] + struct Z; + + #[derive(Component, Default)] + struct W; + + #[derive(Component)] + struct V; + + #[derive(Bundle, Default)] + struct TestBundle { + x: X, + z: Z + } + + let mut world = World::new(); + let e = world.spawn((TestBundle::default(), V)).id(); + + assert!(world.entity(e).contains::()); + assert!(world.entity(e).contains::()); + assert!(world.entity(e).contains::()); + assert!(world.entity(e).contains::()); + assert!(world.entity(e).contains::()); + + world.entity_mut(e).remove_with_required::(); + assert!(!world.entity(e).contains::()); + assert!(!world.entity(e).contains::()); + assert!(!world.entity(e).contains::()); + assert!(!world.entity(e).contains::()); + assert!(world.entity(e).contains::()); + + + } + // These structs are primarily compilation tests to test the derive macros. Because they are // never constructed, we have to manually silence the `dead_code` lint. #[allow(dead_code)] diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 55468cbcad094..c0451606aaff6 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1216,11 +1216,17 @@ impl EntityCommands<'_> { self.add(remove::) } + /// Remove a all components in the bundle and remove all required components for each component in the bundle + pub fn remove_with_required(self) -> Self { + self.add(remove_with_required::) + } + /// Removes a component from the entity. pub fn remove_by_id(self, component_id: ComponentId) -> Self { self.add(remove_by_id(component_id)) } + /// Removes all components associated with the entity. pub fn clear(self) -> Self { self.add(clear()) @@ -1525,6 +1531,13 @@ fn remove_by_id(component_id: ComponentId) -> impl EntityCommand { } } +/// An [`EntityCommand`] that removes components in a [`Bundle`] from an entity and remove all required components for each component in the [`Bundle`] +fn remove_with_required(entity: Entity, world: &mut World) { + if let Some(mut entity) = world.get_entity_mut(entity) { + entity.remove_with_required::(); + } +} + /// An [`EntityCommand`] that removes all components associated with a provided entity. fn clear() -> impl EntityCommand { move |entity: Entity, world: &mut World| { @@ -1807,6 +1820,38 @@ mod tests { assert!(world.contains_resource::>()); } + #[test] + fn remove_bundle_with_required_components() { + + #[derive(Component)] + #[require(Y)] + struct X; + + #[derive(Component, Default)] + struct Y; + + let mut world = World::default(); + let mut queue = CommandQueue::default(); + let e = { + let mut commands = Commands::new(&mut queue, &world); + commands.spawn(X).id() + }; + queue.apply(&mut world); + + assert!(world.get::(e).is_some()); + assert!(world.get::(e).is_some()); + + { + let mut commands = Commands::new(&mut queue, &world); + commands.entity(e) + .remove_with_required::(); + } + queue.apply(&mut world); + + assert!(world.get::(e).is_none()); + assert!(world.get::(e).is_none()); + } + fn is_send() {} fn is_sync() {} diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 5adfb2d302021..f7a5b55b606f7 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -1180,6 +1180,23 @@ impl<'w> EntityWorldMut<'w> { self } + /// Remove all components in the [`Bundle`] and all required components for each component in the [`Bundle`]. + /// + /// Just remove all bundle_info.contributed_components() from the entity + pub fn remove_with_required(&mut self) -> &mut Self { + + let storages = &mut self.world.storages; + let components = &mut self.world.components; + let bundle = self.world.bundles.init_info::(components, storages); + + let bundle_info = unsafe { self.world.bundles.get_unchecked(bundle) }; + let contributed_components = bundle_info.contributed_components().to_vec(); // Make copy to avoid lifetime issues + let extended_bundle = self.world.bundles.init_dynamic_info(components, &contributed_components); + self.location = unsafe { self.remove_bundle(extended_bundle) }; + + self + } + /// Removes any components except those in the [`Bundle`] (and its Required Components) from the entity. /// /// See [`EntityCommands::retain`](crate::system::EntityCommands::retain) for more details. From 8c0dde9dc34d309edb26a0787ed2b966ca259eb3 Mon Sep 17 00:00:00 2001 From: rewin Date: Tue, 3 Sep 2024 07:40:09 +0300 Subject: [PATCH 02/25] fmt --- crates/bevy_ecs/src/lib.rs | 4 +--- crates/bevy_ecs/src/system/commands/mod.rs | 5 +---- crates/bevy_ecs/src/world/entity_ref.rs | 8 +++++--- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index e6e61337efd5c..b098106be5e8b 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -2082,7 +2082,7 @@ mod tests { #[derive(Bundle, Default)] struct TestBundle { x: X, - z: Z + z: Z, } let mut world = World::new(); @@ -2100,8 +2100,6 @@ mod tests { assert!(!world.entity(e).contains::()); assert!(!world.entity(e).contains::()); assert!(world.entity(e).contains::()); - - } // These structs are primarily compilation tests to test the derive macros. Because they are diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index c0451606aaff6..3d581ccb014de 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1226,7 +1226,6 @@ impl EntityCommands<'_> { self.add(remove_by_id(component_id)) } - /// Removes all components associated with the entity. pub fn clear(self) -> Self { self.add(clear()) @@ -1822,7 +1821,6 @@ mod tests { #[test] fn remove_bundle_with_required_components() { - #[derive(Component)] #[require(Y)] struct X; @@ -1843,8 +1841,7 @@ mod tests { { let mut commands = Commands::new(&mut queue, &world); - commands.entity(e) - .remove_with_required::(); + commands.entity(e).remove_with_required::(); } queue.apply(&mut world); diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index f7a5b55b606f7..f3dd4591c90e8 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -1181,17 +1181,19 @@ impl<'w> EntityWorldMut<'w> { } /// Remove all components in the [`Bundle`] and all required components for each component in the [`Bundle`]. - /// + /// /// Just remove all bundle_info.contributed_components() from the entity pub fn remove_with_required(&mut self) -> &mut Self { - let storages = &mut self.world.storages; let components = &mut self.world.components; let bundle = self.world.bundles.init_info::(components, storages); let bundle_info = unsafe { self.world.bundles.get_unchecked(bundle) }; let contributed_components = bundle_info.contributed_components().to_vec(); // Make copy to avoid lifetime issues - let extended_bundle = self.world.bundles.init_dynamic_info(components, &contributed_components); + let extended_bundle = self + .world + .bundles + .init_dynamic_info(components, &contributed_components); self.location = unsafe { self.remove_bundle(extended_bundle) }; self From bcd8c3320abe9971112602dbe9defef63b55d07a Mon Sep 17 00:00:00 2001 From: rewin Date: Tue, 3 Sep 2024 07:45:19 +0300 Subject: [PATCH 03/25] comments --- crates/bevy_ecs/src/world/entity_ref.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index f3dd4591c90e8..b080db4ae4e90 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -1182,12 +1182,13 @@ impl<'w> EntityWorldMut<'w> { /// Remove all components in the [`Bundle`] and all required components for each component in the [`Bundle`]. /// - /// Just remove all bundle_info.contributed_components() from the entity + /// Just remove all `bundle_info.contributed_components()` from the entity pub fn remove_with_required(&mut self) -> &mut Self { let storages = &mut self.world.storages; let components = &mut self.world.components; let bundle = self.world.bundles.init_info::(components, storages); + // SAFETY: the `BundleInfo` is initialized above let bundle_info = unsafe { self.world.bundles.get_unchecked(bundle) }; let contributed_components = bundle_info.contributed_components().to_vec(); // Make copy to avoid lifetime issues let extended_bundle = self From 404ede45c111b3c63e0b472655600033de5bacbf Mon Sep 17 00:00:00 2001 From: rewin Date: Tue, 3 Sep 2024 07:49:42 +0300 Subject: [PATCH 04/25] comments --- crates/bevy_ecs/src/world/entity_ref.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index b080db4ae4e90..dff2309c48614 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -1195,6 +1195,7 @@ impl<'w> EntityWorldMut<'w> { .world .bundles .init_dynamic_info(components, &contributed_components); + // SAFETY: the dynamic `BundleInfo` is initialized above self.location = unsafe { self.remove_bundle(extended_bundle) }; self From 4819ccf60bbfb822effdd332798af9039c580871 Mon Sep 17 00:00:00 2001 From: rewin Date: Wed, 4 Sep 2024 06:17:16 +0300 Subject: [PATCH 05/25] Safe removing with requirements --- crates/bevy_ecs/src/lib.rs | 40 +++++++++++++++++++++--- crates/bevy_ecs/src/world/entity_ref.rs | 41 ++++++++++++++++++++----- 2 files changed, 69 insertions(+), 12 deletions(-) diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index b098106be5e8b..45f24ba2e09a5 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -2025,39 +2025,71 @@ mod tests { #[test] fn remove_component_and_his_required_components() { + + // We create this 'require' tree (down is required by up component) + // X + // \ + // Y + // \ + // Z V + // \ / + // W + + // And after remove X and this requirements we must have this tree (W will keep becuase it's still required by V) + // V + // / + // W + #[derive(Component)] #[require(Y)] struct X; #[derive(Component, Default)] + #[require(Z)] struct Y; - #[derive(Component)] + #[derive(Component, Default)] + #[require(W)] struct Z; + #[derive(Component, Default)] + struct W; + + #[derive(Component, Default)] + #[require(W)] + struct V; + let mut world = World::new(); - let e = world.spawn((X, Z)).id(); + let e = world.spawn((X, V)).id(); assert!(world.entity(e).contains::()); assert!(world.entity(e).contains::()); assert!(world.entity(e).contains::()); + assert!(world.entity(e).contains::()); + assert!(world.entity(e).contains::()); //check that `remove` works as expected world.entity_mut(e).remove::(); assert!(!world.entity(e).contains::()); assert!(world.entity(e).contains::()); assert!(world.entity(e).contains::()); + assert!(world.entity(e).contains::()); + assert!(world.entity(e).contains::()); world.entity_mut(e).insert(X); assert!(world.entity(e).contains::()); assert!(world.entity(e).contains::()); assert!(world.entity(e).contains::()); + assert!(world.entity(e).contains::()); + assert!(world.entity(e).contains::()); - //remove `X` again and ensure that `Y` was removed too + //remove `X` again and ensure that `Y` and `Z` was removed too world.entity_mut(e).remove_with_required::(); assert!(!world.entity(e).contains::()); assert!(!world.entity(e).contains::()); - assert!(world.entity(e).contains::()); + assert!(!world.entity(e).contains::()); + assert!(world.entity(e).contains::()); + assert!(world.entity(e).contains::()); } #[test] diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index dff2309c48614..219cc40ad4a27 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -1180,23 +1180,48 @@ impl<'w> EntityWorldMut<'w> { self } - /// Remove all components in the [`Bundle`] and all required components for each component in the [`Bundle`]. - /// - /// Just remove all `bundle_info.contributed_components()` from the entity + /// Removes all components of this [`Bundle`] and all required components of this [`Bundle`] that are not required by other components of this entity + /// The function can be noticeably slower than remove or retain functions because the function dynamically determines which components are still required by entity components outside of [`Bundle`] + /// pub fn remove_with_required(&mut self) -> &mut Self { let storages = &mut self.world.storages; let components = &mut self.world.components; let bundle = self.world.bundles.init_info::(components, storages); // SAFETY: the `BundleInfo` is initialized above - let bundle_info = unsafe { self.world.bundles.get_unchecked(bundle) }; - let contributed_components = bundle_info.contributed_components().to_vec(); // Make copy to avoid lifetime issues - let extended_bundle = self + // We make clone of components array to not lock immutable access to world + let contributed_components = unsafe { self.world.bundles.get_unchecked(bundle).contributed_components().to_vec() }; + + let old_location = self.location; + let old_archetype = &mut self.world.archetypes[old_location.archetype_id]; + + let remaining_components = old_archetype + .components() + .into_iter() + .filter(|c| !contributed_components.contains(c)) + .collect::>(); + + let remaining_bundle = self + .world + .bundles + .init_dynamic_info(components, &remaining_components); + + let remaining_bundle_info = unsafe { self.world.bundles.get_unchecked(remaining_bundle) }; + + // Remove components required by reamining components from contributed_components of bundle T + let to_delete = contributed_components + .iter() + .filter(|c| !remaining_bundle_info.contributed_components().contains(c)) + .copied() + .collect::>(); + + + let to_delete_bundle = self .world .bundles - .init_dynamic_info(components, &contributed_components); + .init_dynamic_info(components, &to_delete); // SAFETY: the dynamic `BundleInfo` is initialized above - self.location = unsafe { self.remove_bundle(extended_bundle) }; + self.location = unsafe { self.remove_bundle(to_delete_bundle) }; self } From 48065c8494cec2d6b501ca2793438722636b18b3 Mon Sep 17 00:00:00 2001 From: rewin Date: Wed, 4 Sep 2024 06:41:04 +0300 Subject: [PATCH 06/25] docs for entity_ref remove_with_required --- crates/bevy_ecs/src/world/entity_ref.rs | 60 ++++++++++++++++++++++++- 1 file changed, 58 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 219cc40ad4a27..96d2c90c3f309 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -1180,9 +1180,64 @@ impl<'w> EntityWorldMut<'w> { self } - /// Removes all components of this [`Bundle`] and all required components of this [`Bundle`] that are not required by other components of this entity - /// The function can be noticeably slower than remove or retain functions because the function dynamically determines which components are still required by entity components outside of [`Bundle`] + /// Removes all components of this [`Bundle`] and its required components that are not required by other components of this entity. + /// + /// This function can be noticeably slower than simple remove or retain functions because it dynamically determines which components + /// are still required by entity components outside of the [`Bundle`]. + /// + /// # Example + /// + /// This example demonstrates safely removing component `X` and all its requirements that are not required outside `X`'s requirement tree: + /// + /// ```rust /// + /// use bevy_ecs::prelude::*; + /// + /// #[derive(Component)] + /// #[require(Y)] + /// struct X; + /// + /// #[derive(Component, Default)] + /// #[require(Z)] + /// struct Y; + /// + /// #[derive(Component, Default)] + /// struct Z; + /// + /// #[derive(Component)] + /// #[require(Z)] + /// struct W; + /// + /// let mut world = World::new(); + /// + /// // Spawn an entity with X, Y, Z, and W components + /// let entity = world.spawn((X, W)).id(); + /// + /// // Initial component tree: + /// // X + /// // \ + /// // Y W + /// // \ / + /// // Z + /// + /// assert!(world.entity(entity).contains::()); + /// assert!(world.entity(entity).contains::()); + /// assert!(world.entity(entity).contains::()); + /// assert!(world.entity(entity).contains::()); + /// + /// // Remove X and its unused requirements + /// world.entity_mut(entity).remove_with_required::(); + /// + /// // Resulting component tree: + /// // W + /// // / + /// // Z + /// + /// assert!(!world.entity(entity).contains::()); + /// assert!(!world.entity(entity).contains::()); + /// assert!(world.entity(entity).contains::()); + /// assert!(world.entity(entity).contains::()); + /// ``` pub fn remove_with_required(&mut self) -> &mut Self { let storages = &mut self.world.storages; let components = &mut self.world.components; @@ -1206,6 +1261,7 @@ impl<'w> EntityWorldMut<'w> { .bundles .init_dynamic_info(components, &remaining_components); + // SAFETY: the `BundleInfo` is initialized above let remaining_bundle_info = unsafe { self.world.bundles.get_unchecked(remaining_bundle) }; // Remove components required by reamining components from contributed_components of bundle T From 665f0a1d1fdddd61ec53b2692f921f3fb0152fbb Mon Sep 17 00:00:00 2001 From: rewin Date: Wed, 4 Sep 2024 06:51:30 +0300 Subject: [PATCH 07/25] Comments for remove_with_required in EntityCommands --- crates/bevy_ecs/src/lib.rs | 5 +- crates/bevy_ecs/src/system/commands/mod.rs | 59 +++++++++++++++++++++- crates/bevy_ecs/src/world/entity_ref.rs | 22 ++++---- 3 files changed, 71 insertions(+), 15 deletions(-) diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 45f24ba2e09a5..20a81199471e7 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -2025,12 +2025,11 @@ mod tests { #[test] fn remove_component_and_his_required_components() { - // We create this 'require' tree (down is required by up component) // X // \ - // Y - // \ + // Y + // \ // Z V // \ / // W diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 3d581ccb014de..0bdbdc386e483 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1216,7 +1216,58 @@ impl EntityCommands<'_> { self.add(remove::) } - /// Remove a all components in the bundle and remove all required components for each component in the bundle + /// Removes all components in the bundle and remove all required components for each component in the bundle + /// that are not required by other components of this entity. + /// + /// This function can be noticeably slower than simple remove or retain functions because it dynamically determines which components + /// are still required by entity components outside of the [`Bundle`]. + /// + /// # Example + /// + /// ```rust + /// use bevy_ecs::prelude::*; + /// + /// #[derive(Component)] + /// #[require(Y)] + /// struct X; + /// + /// #[derive(Component, Default)] + /// #[require(Z)] + /// struct Y; + /// + /// #[derive(Component, Default)] + /// struct Z; + /// + /// #[derive(Component)] + /// #[require(Z)] + /// struct W; + /// + /// fn remove_x_system(mut commands: Commands, query: Query>) { + /// for entity in &query { + /// // Remove X and its unused requirements + /// commands.entity(entity).remove_with_required::(); + /// } + /// } + /// + /// // Usage in a system: + /// fn setup(mut commands: Commands) { + /// // Spawn an entity with X, Y, Z, and W components + /// commands.spawn((X, W)); + /// + /// // Initial component tree: + /// // X + /// // \ + /// // Y W + /// // \ / + /// // Z + /// } + /// + /// // After calling remove_x_system: + /// // Resulting component tree: + /// // W + /// // / + /// // Z + /// ``` pub fn remove_with_required(self) -> Self { self.add(remove_with_required::) } @@ -1530,7 +1581,11 @@ fn remove_by_id(component_id: ComponentId) -> impl EntityCommand { } } -/// An [`EntityCommand`] that removes components in a [`Bundle`] from an entity and remove all required components for each component in the [`Bundle`] +/// An [`EntityCommand`] tjhat remove all components in the bundle and remove all required components for each component in the bundle +/// that are not required by other components of this entity. +/// +/// This function can be noticeably slower than simple remove or retain functions because it dynamically determines which components +/// are still required by entity components outside of the [`Bundle`]. fn remove_with_required(entity: Entity, world: &mut World) { if let Some(mut entity) = world.get_entity_mut(entity) { entity.remove_with_required::(); diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 96d2c90c3f309..e7520507395e3 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -1182,7 +1182,7 @@ impl<'w> EntityWorldMut<'w> { /// Removes all components of this [`Bundle`] and its required components that are not required by other components of this entity. /// - /// This function can be noticeably slower than simple remove or retain functions because it dynamically determines which components + /// This function can be noticeably slower than simple remove or retain functions because it dynamically determines which components /// are still required by entity components outside of the [`Bundle`]. /// /// # Example @@ -1190,9 +1190,9 @@ impl<'w> EntityWorldMut<'w> { /// This example demonstrates safely removing component `X` and all its requirements that are not required outside `X`'s requirement tree: /// /// ```rust - /// + /// /// use bevy_ecs::prelude::*; - /// + /// /// #[derive(Component)] /// #[require(Y)] /// struct X; @@ -1215,7 +1215,7 @@ impl<'w> EntityWorldMut<'w> { /// /// // Initial component tree: /// // X - /// // \ + /// // \ /// // Y W /// // \ / /// // Z @@ -1245,7 +1245,13 @@ impl<'w> EntityWorldMut<'w> { // SAFETY: the `BundleInfo` is initialized above // We make clone of components array to not lock immutable access to world - let contributed_components = unsafe { self.world.bundles.get_unchecked(bundle).contributed_components().to_vec() }; + let contributed_components = unsafe { + self.world + .bundles + .get_unchecked(bundle) + .contributed_components() + .to_vec() + }; let old_location = self.location; let old_archetype = &mut self.world.archetypes[old_location.archetype_id]; @@ -1271,11 +1277,7 @@ impl<'w> EntityWorldMut<'w> { .copied() .collect::>(); - - let to_delete_bundle = self - .world - .bundles - .init_dynamic_info(components, &to_delete); + let to_delete_bundle = self.world.bundles.init_dynamic_info(components, &to_delete); // SAFETY: the dynamic `BundleInfo` is initialized above self.location = unsafe { self.remove_bundle(to_delete_bundle) }; From 282b46f392dd5e3b619b32787386789879ae5ac3 Mon Sep 17 00:00:00 2001 From: rewin Date: Wed, 4 Sep 2024 06:58:14 +0300 Subject: [PATCH 08/25] Fix typos and clippy --- crates/bevy_ecs/src/world/entity_ref.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index e7520507395e3..fb1c54cb7e3d3 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -1258,7 +1258,6 @@ impl<'w> EntityWorldMut<'w> { let remaining_components = old_archetype .components() - .into_iter() .filter(|c| !contributed_components.contains(c)) .collect::>(); @@ -1270,7 +1269,7 @@ impl<'w> EntityWorldMut<'w> { // SAFETY: the `BundleInfo` is initialized above let remaining_bundle_info = unsafe { self.world.bundles.get_unchecked(remaining_bundle) }; - // Remove components required by reamining components from contributed_components of bundle T + // Remove components required by remaining components from contributed_components of bundle T let to_delete = contributed_components .iter() .filter(|c| !remaining_bundle_info.contributed_components().contains(c)) From 98a8ffc3291781d8d4b9aebea12f0a356bf2592d Mon Sep 17 00:00:00 2001 From: rewin Date: Wed, 4 Sep 2024 07:02:10 +0300 Subject: [PATCH 09/25] fix typos --- 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 20a81199471e7..2cfc70034ef4f 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -2034,7 +2034,7 @@ mod tests { // \ / // W - // And after remove X and this requirements we must have this tree (W will keep becuase it's still required by V) + // And after remove X and this requirements we must have this tree (W will keep because it's still required by V) // V // / // W From febd9715c47924e9745f8f3344103f9c1a61d598 Mon Sep 17 00:00:00 2001 From: rewin Date: Wed, 4 Sep 2024 09:06:16 +0300 Subject: [PATCH 10/25] add recursive and descendal strategy for remove --- crates/bevy_ecs/src/lib.rs | 74 ++++++++++- crates/bevy_ecs/src/system/commands/mod.rs | 103 +++++++++++++- crates/bevy_ecs/src/world/entity_ref.rs | 148 ++++++++++++++++++++- 3 files changed, 315 insertions(+), 10 deletions(-) diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 2cfc70034ef4f..6bd44f6e77027 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -2083,7 +2083,7 @@ mod tests { assert!(world.entity(e).contains::()); //remove `X` again and ensure that `Y` and `Z` was removed too - world.entity_mut(e).remove_with_required::(); + world.entity_mut(e).require_recursive_remove::(); assert!(!world.entity(e).contains::()); assert!(!world.entity(e).contains::()); assert!(!world.entity(e).contains::()); @@ -2125,7 +2125,7 @@ mod tests { assert!(world.entity(e).contains::()); assert!(world.entity(e).contains::()); - world.entity_mut(e).remove_with_required::(); + world.entity_mut(e).require_recursive_remove::(); assert!(!world.entity(e).contains::()); assert!(!world.entity(e).contains::()); assert!(!world.entity(e).contains::()); @@ -2133,6 +2133,76 @@ mod tests { assert!(world.entity(e).contains::()); } + #[test] + fn test_require_descendant_remove_with_shared_dependency() { + #[derive(Component, Default)] + #[require(B)] + struct A; + + #[derive(Component, Default)] + #[require(C)] + struct B; + + #[derive(Component, Default)] + #[require(D)] + struct C; + + #[derive(Component, Default)] + struct D; + + #[derive(Component, Default)] + #[require(D)] + struct E; + + //Require tree: A -> B -> C -> D <- E + + let mut world = World::new(); + let entity = world.spawn((A, B, C, D, E)).id(); + + world.entity_mut(entity).require_descendant_remove::(); + + assert!(!world.entity(entity).contains::()); + assert!(!world.entity(entity).contains::()); + assert!(!world.entity(entity).contains::()); + assert!(world.entity(entity).contains::()); + assert!(world.entity(entity).contains::()); + } + + #[test] + fn test_require_descendant_remove_leaf_component() { + #[derive(Component, Default)] + #[require(B)] + struct A; + + #[derive(Component, Default)] + #[require(C)] + struct B; + + #[derive(Component, Default)] + #[require(D)] + struct C; + + #[derive(Component, Default)] + struct D; + + #[derive(Component, Default)] + #[require(D)] + struct E; + + //Require tree: A -> B -> C -> D <- E + + let mut world = World::new(); + let entity = world.spawn((A, B, C, D, E)).id(); + + world.entity_mut(entity).require_descendant_remove::(); + + assert!(!world.entity(entity).contains::()); + assert!(!world.entity(entity).contains::()); + assert!(!world.entity(entity).contains::()); + assert!(!world.entity(entity).contains::()); + assert!(!world.entity(entity).contains::()); + } + // These structs are primarily compilation tests to test the derive macros. Because they are // never constructed, we have to manually silence the `dead_code` lint. #[allow(dead_code)] diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 0bdbdc386e483..c249970b3a370 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1245,7 +1245,7 @@ impl EntityCommands<'_> { /// fn remove_x_system(mut commands: Commands, query: Query>) { /// for entity in &query { /// // Remove X and its unused requirements - /// commands.entity(entity).remove_with_required::(); + /// commands.entity(entity).require_recursive_remove::(); /// } /// } /// @@ -1268,8 +1268,76 @@ impl EntityCommands<'_> { /// // / /// // Z /// ``` - pub fn remove_with_required(self) -> Self { - self.add(remove_with_required::) + pub fn require_recursive_remove(self) -> Self { + self.add(require_recursive_remove::) + } + + /// Removes the components of this [`Bundle`], components that require them, and their required components, + /// unless they are required by components outside of this removal set. + /// + /// This method performs a top-down removal, starting by collecting all bundle components, all required components for bundle components, all components that require bundle components. + /// Then removing all collected components, unless those components are required outside remove set + /// elsewhere in the entity. + /// + /// This function can be noticeably slower than simple remove or retain functions because it dynamically determines which components + /// are still required by entity components outside of the [`Bundle`]. + /// + /// # Example + /// + /// ```rust + /// use bevy_ecs::prelude::*; + /// + /// #[derive(Component)] + /// #[require(B)] + /// struct A; + /// + /// #[derive(Component, Default)] + /// #[require(C)] + /// struct B; + /// + /// #[derive(Component, Default)] + /// #[require(D)] + /// struct C; + /// + /// #[derive(Component, Default)] + /// struct D; + /// + /// #[derive(Component)] + /// #[require(D)] + /// struct E; + /// + /// fn remove_b_system(mut commands: Commands, query: Query>) { + /// for entity in &query { + /// // Remove B and its requirement tree + /// commands.entity(entity).require_descendant_remove::(); + /// } + /// } + /// + /// // Usage in a system: + /// fn setup(mut commands: Commands) { + /// // Spawn an entity with A, B, C, D, and E components + /// commands.spawn((A, B, C, D, E)); + /// + /// // Initial component structure: + /// // A -> B -> C -> D <- E + /// } + /// + /// // After calling remove_b_system: + /// // Resulting component structure: + /// // D <- E + /// // + /// // A is removed as it requires B + /// // B is removed + /// // C is removed as it's required by B + /// // D is kept as E still requires it + /// // E is kept + /// ``` + /// + /// This method is useful for removing a component along with its entire requirement tree and any + /// components that depend on them, while preserving components that are still required by parts of + /// the entity outside the removal tree. + pub fn require_descendant_remove(self) -> Self { + self.add(require_descendant_remove::) } /// Removes a component from the entity. @@ -1586,9 +1654,15 @@ fn remove_by_id(component_id: ComponentId) -> impl EntityCommand { /// /// This function can be noticeably slower than simple remove or retain functions because it dynamically determines which components /// are still required by entity components outside of the [`Bundle`]. -fn remove_with_required(entity: Entity, world: &mut World) { +fn require_recursive_remove(entity: Entity, world: &mut World) { if let Some(mut entity) = world.get_entity_mut(entity) { - entity.remove_with_required::(); + entity.require_recursive_remove::(); + } +} + +fn require_descendant_remove(entity: Entity, world: &mut World) { + if let Some(mut entity) = world.get_entity_mut(entity) { + entity.require_descendant_remove::(); } } @@ -1896,10 +1970,27 @@ mod tests { { let mut commands = Commands::new(&mut queue, &world); - commands.entity(e).remove_with_required::(); + commands.entity(e).require_recursive_remove::(); + } + queue.apply(&mut world); + + assert!(world.get::(e).is_none()); + assert!(world.get::(e).is_none()); + + { + let mut commands = Commands::new(&mut queue, &world); + commands.entity(e).insert(X); } queue.apply(&mut world); + assert!(world.get::(e).is_some()); + assert!(world.get::(e).is_some()); + + { + let mut commands = Commands::new(&mut queue, &world); + commands.entity(e).require_descendant_remove::(); + } + queue.apply(&mut world); assert!(world.get::(e).is_none()); assert!(world.get::(e).is_none()); } diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index fb1c54cb7e3d3..382ea93f66e84 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -13,6 +13,7 @@ use crate::{ world::{DeferredWorld, Mut, World}, }; use bevy_ptr::{OwningPtr, Ptr}; +use bevy_utils::HashSet; use std::{any::TypeId, marker::PhantomData}; use thiserror::Error; @@ -1226,7 +1227,7 @@ impl<'w> EntityWorldMut<'w> { /// assert!(world.entity(entity).contains::()); /// /// // Remove X and its unused requirements - /// world.entity_mut(entity).remove_with_required::(); + /// world.entity_mut(entity).require_recursive_remove::(); /// /// // Resulting component tree: /// // W @@ -1238,7 +1239,9 @@ impl<'w> EntityWorldMut<'w> { /// assert!(world.entity(entity).contains::()); /// assert!(world.entity(entity).contains::()); /// ``` - pub fn remove_with_required(&mut self) -> &mut Self { + /// Note: It will note delete components in [`Bundle`] if some components require bundle components. Use + /// + pub fn require_recursive_remove(&mut self) -> &mut Self { let storages = &mut self.world.storages; let components = &mut self.world.components; let bundle = self.world.bundles.init_info::(components, storages); @@ -1283,6 +1286,147 @@ impl<'w> EntityWorldMut<'w> { self } + /// Removes the components of this [`Bundle`], components that require them, and their required components, + /// unless they are required by components outside of this removal set. + /// + /// This method performs a top-down removal, starting by collecting all bundle components, all required components for bundle components, all components that require bundle components. + /// Then removing all collected components, unless those components are required outside remove set + /// elsewhere in the entity. + /// + /// # Example + /// + /// ```rust + /// use bevy_ecs::prelude::*; + /// + /// #[derive(Component)] + /// #[require(B)] + /// struct A; + /// + /// #[derive(Component, Default)] + /// #[require(C)] + /// struct B; + /// + /// #[derive(Component, Default)] + /// #[require(D)] + /// struct C; + /// + /// #[derive(Component, Default)] + /// struct D; + /// + /// #[derive(Component)] + /// #[require(D)] + /// struct E; + /// + /// let mut world = World::new(); + /// + /// // Spawn an entity with A, B, C, D, and E components + /// let entity = world.spawn((A, B, C, D, E)).id(); + /// + /// // Initial component structure: + /// // A -> B -> C -> D <- E + /// + /// assert!(world.entity(entity).contains::()); + /// assert!(world.entity(entity).contains::()); + /// assert!(world.entity(entity).contains::()); + /// assert!(world.entity(entity).contains::()); + /// assert!(world.entity(entity).contains::()); + /// + /// // Remove B and its requirement tree + /// world.entity_mut(entity).require_descendant_remove::(); + /// + /// // Resulting component structure: + /// // D <- E + /// + /// assert!(!world.entity(entity).contains::()); // A is removed as it requires B + /// assert!(!world.entity(entity).contains::()); + /// assert!(!world.entity(entity).contains::()); // C is removed as it's required by B + /// assert!(world.entity(entity).contains::()); // D is kept as E still requires it + /// assert!(world.entity(entity).contains::()); + /// ``` + /// + /// This method is useful for removing a component along with its entire requirement tree and any + /// components that depend on them, while preserving components that are still required by parts of + /// the entity outside the removal tree. + pub fn require_descendant_remove(&mut self) -> &mut Self { + // Get all components of the entity + let entity_components = self.archetype().components().collect::>(); + + let storages = &mut self.world.storages; + let components = &mut self.world.components; + let bundle = self.world.bundles.init_info::(components, storages); + + // Get all components in the bundle + // SAFETY: the `BundleInfo` is initialized above + let bundle_components = unsafe { + self.world + .bundles + .get_unchecked(bundle) + .explicit_components() + .to_vec() + }; + + let mut contributed_components = HashSet::::new(); + // SAFETY: the `BundleInfo` is initialized above + contributed_components.extend(unsafe { + self.world + .bundles + .get_unchecked(bundle) + .contributed_components() + .into_iter() + }); + + // Find all components that depend on bundle components + for &component_id in &entity_components { + let bundle = self + .world + .bundles + .init_dynamic_info(components, &[component_id]); + // SAFETY: the `BundleInfo` is initialized above + let bundle_info = unsafe { self.world.bundles.get_unchecked(bundle) }; + for bundle_component_id in &bundle_components { + if bundle_info + .contributed_components() + .contains(bundle_component_id) + { + contributed_components.extend(bundle_info.contributed_components().iter()); + break; + } + } + } + + // Combine bundle components and dependent components + let mut to_remove = contributed_components; + + // Create a bundle with remaining components + let remaining_components: Vec<_> = entity_components + .iter() + .filter(|&&c| !to_remove.contains(&c)) + .copied() + .collect(); + + let remaining_bundle = self + .world + .bundles + .init_dynamic_info(components, &remaining_components); + + // SAFETY: the `BundleInfo` is initialized above + let remaining_bundle_info = unsafe { self.world.bundles.get_unchecked(remaining_bundle) }; + + // Remove components required by remaining components from to_remove + to_remove.retain(|&c| !remaining_bundle_info.contributed_components().contains(&c)); + + // Create a bundle with components to delete + let to_delete_bundle = self + .world + .bundles + .init_dynamic_info(components, &to_remove.into_iter().collect::>()); + + // SAFETY: the dynamic `BundleInfo` is initialized above + self.location = unsafe { self.remove_bundle(to_delete_bundle) }; + + self + } + /// Removes any components except those in the [`Bundle`] (and its Required Components) from the entity. /// /// See [`EntityCommands::retain`](crate::system::EntityCommands::retain) for more details. From aa9ae94186b04990d01b9185eb2a8860e0e120cc Mon Sep 17 00:00:00 2001 From: rewin Date: Wed, 4 Sep 2024 09:14:19 +0300 Subject: [PATCH 11/25] Fix clippy --- crates/bevy_ecs/src/world/entity_ref.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 382ea93f66e84..cde6790a35a9a 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -1372,7 +1372,7 @@ impl<'w> EntityWorldMut<'w> { .bundles .get_unchecked(bundle) .contributed_components() - .into_iter() + .iter() }); // Find all components that depend on bundle components From 4359dbeae695a84b3b42247fc4ab1f0bf5e75287 Mon Sep 17 00:00:00 2001 From: "a.yamaev" Date: Wed, 2 Oct 2024 10:48:00 +0300 Subject: [PATCH 12/25] remove safe remove --- crates/bevy_ecs/src/world/entity_ref.rs | 246 +----------------------- 1 file changed, 1 insertion(+), 245 deletions(-) diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 32d9e6b8ebfff..b1e492397e33b 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -1230,251 +1230,7 @@ impl<'w> EntityWorldMut<'w> { self } - /// Removes all components of this [`Bundle`] and its required components that are not required by other components of this entity. - /// - /// This function can be noticeably slower than simple remove or retain functions because it dynamically determines which components - /// are still required by entity components outside of the [`Bundle`]. - /// - /// # Example - /// - /// This example demonstrates safely removing component `X` and all its requirements that are not required outside `X`'s requirement tree: - /// - /// ```rust - /// - /// use bevy_ecs::prelude::*; - /// - /// #[derive(Component)] - /// #[require(Y)] - /// struct X; - /// - /// #[derive(Component, Default)] - /// #[require(Z)] - /// struct Y; - /// - /// #[derive(Component, Default)] - /// struct Z; - /// - /// #[derive(Component)] - /// #[require(Z)] - /// struct W; - /// - /// let mut world = World::new(); - /// - /// // Spawn an entity with X, Y, Z, and W components - /// let entity = world.spawn((X, W)).id(); - /// - /// // Initial component tree: - /// // X - /// // \ - /// // Y W - /// // \ / - /// // Z - /// - /// assert!(world.entity(entity).contains::()); - /// assert!(world.entity(entity).contains::()); - /// assert!(world.entity(entity).contains::()); - /// assert!(world.entity(entity).contains::()); - /// - /// // Remove X and its unused requirements - /// world.entity_mut(entity).require_recursive_remove::(); - /// - /// // Resulting component tree: - /// // W - /// // / - /// // Z - /// - /// assert!(!world.entity(entity).contains::()); - /// assert!(!world.entity(entity).contains::()); - /// assert!(world.entity(entity).contains::()); - /// assert!(world.entity(entity).contains::()); - /// ``` - /// Note: It will note delete components in [`Bundle`] if some components require bundle components. Use - /// - pub fn require_recursive_remove(&mut self) -> &mut Self { - let storages = &mut self.world.storages; - let components = &mut self.world.components; - let bundle = self.world.bundles.init_info::(components, storages); - - // SAFETY: the `BundleInfo` is initialized above - // We make clone of components array to not lock immutable access to world - let contributed_components = unsafe { - self.world - .bundles - .get_unchecked(bundle) - .contributed_components() - .to_vec() - }; - - let old_location = self.location; - let old_archetype = &mut self.world.archetypes[old_location.archetype_id]; - - let remaining_components = old_archetype - .components() - .filter(|c| !contributed_components.contains(c)) - .collect::>(); - - let remaining_bundle = self - .world - .bundles - .init_dynamic_info(components, &remaining_components); - - // SAFETY: the `BundleInfo` is initialized above - let remaining_bundle_info = unsafe { self.world.bundles.get_unchecked(remaining_bundle) }; - - // Remove components required by remaining components from contributed_components of bundle T - let to_delete = contributed_components - .iter() - .filter(|c| !remaining_bundle_info.contributed_components().contains(c)) - .copied() - .collect::>(); - - let to_delete_bundle = self.world.bundles.init_dynamic_info(components, &to_delete); - // SAFETY: the dynamic `BundleInfo` is initialized above - self.location = unsafe { self.remove_bundle(to_delete_bundle) }; - - self - } - - /// Removes the components of this [`Bundle`], components that require them, and their required components, - /// unless they are required by components outside of this removal set. - /// - /// This method performs a top-down removal, starting by collecting all bundle components, all required components for bundle components, all components that require bundle components. - /// Then removing all collected components, unless those components are required outside remove set - /// elsewhere in the entity. - /// - /// # Example - /// - /// ```rust - /// use bevy_ecs::prelude::*; - /// - /// #[derive(Component)] - /// #[require(B)] - /// struct A; - /// - /// #[derive(Component, Default)] - /// #[require(C)] - /// struct B; - /// - /// #[derive(Component, Default)] - /// #[require(D)] - /// struct C; - /// - /// #[derive(Component, Default)] - /// struct D; - /// - /// #[derive(Component)] - /// #[require(D)] - /// struct E; - /// - /// let mut world = World::new(); - /// - /// // Spawn an entity with A, B, C, D, and E components - /// let entity = world.spawn((A, B, C, D, E)).id(); - /// - /// // Initial component structure: - /// // A -> B -> C -> D <- E - /// - /// assert!(world.entity(entity).contains::()); - /// assert!(world.entity(entity).contains::()); - /// assert!(world.entity(entity).contains::()); - /// assert!(world.entity(entity).contains::()); - /// assert!(world.entity(entity).contains::()); - /// - /// // Remove B and its requirement tree - /// world.entity_mut(entity).require_descendant_remove::(); - /// - /// // Resulting component structure: - /// // D <- E - /// - /// assert!(!world.entity(entity).contains::()); // A is removed as it requires B - /// assert!(!world.entity(entity).contains::()); - /// assert!(!world.entity(entity).contains::()); // C is removed as it's required by B - /// assert!(world.entity(entity).contains::()); // D is kept as E still requires it - /// assert!(world.entity(entity).contains::()); - /// ``` - /// - /// This method is useful for removing a component along with its entire requirement tree and any - /// components that depend on them, while preserving components that are still required by parts of - /// the entity outside the removal tree. - pub fn require_descendant_remove(&mut self) -> &mut Self { - // Get all components of the entity - let entity_components = self.archetype().components().collect::>(); - - let storages = &mut self.world.storages; - let components = &mut self.world.components; - let bundle = self.world.bundles.init_info::(components, storages); - - // Get all components in the bundle - // SAFETY: the `BundleInfo` is initialized above - let bundle_components = unsafe { - self.world - .bundles - .get_unchecked(bundle) - .explicit_components() - .to_vec() - }; - - let mut contributed_components = HashSet::::new(); - // SAFETY: the `BundleInfo` is initialized above - contributed_components.extend(unsafe { - self.world - .bundles - .get_unchecked(bundle) - .contributed_components() - .iter() - }); - - // Find all components that depend on bundle components - for &component_id in &entity_components { - let bundle = self - .world - .bundles - .init_dynamic_info(components, &[component_id]); - // SAFETY: the `BundleInfo` is initialized above - let bundle_info = unsafe { self.world.bundles.get_unchecked(bundle) }; - for bundle_component_id in &bundle_components { - if bundle_info - .contributed_components() - .contains(bundle_component_id) - { - contributed_components.extend(bundle_info.contributed_components().iter()); - break; - } - } - } - - // Combine bundle components and dependent components - let mut to_remove = contributed_components; - - // Create a bundle with remaining components - let remaining_components: Vec<_> = entity_components - .iter() - .filter(|&&c| !to_remove.contains(&c)) - .copied() - .collect(); - - let remaining_bundle = self - .world - .bundles - .init_dynamic_info(components, &remaining_components); - - // SAFETY: the `BundleInfo` is initialized above - let remaining_bundle_info = unsafe { self.world.bundles.get_unchecked(remaining_bundle) }; - - // Remove components required by remaining components from to_remove - to_remove.retain(|&c| !remaining_bundle_info.contributed_components().contains(&c)); - - // Create a bundle with components to delete - let to_delete_bundle = self - .world - .bundles - .init_dynamic_info(components, &to_remove.into_iter().collect::>()); - - // SAFETY: the dynamic `BundleInfo` is initialized above - self.location = unsafe { self.remove_bundle(to_delete_bundle) }; - - self - } + /// Removes any components except those in the [`Bundle`] (and its Required Components) from the entity. /// From 79bbf0a6f2b92b4d9eed4c7416277370035d3961 Mon Sep 17 00:00:00 2001 From: "a.yamaev" Date: Wed, 2 Oct 2024 11:21:58 +0300 Subject: [PATCH 13/25] restore footgun solution --- crates/bevy_ecs/src/lib.rs | 99 +------------ crates/bevy_ecs/src/system/commands/mod.rs | 153 +++------------------ crates/bevy_ecs/src/world/entity_ref.rs | 21 ++- 3 files changed, 42 insertions(+), 231 deletions(-) diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index c7e45d6d41379..4a322978066c9 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -2027,20 +2027,6 @@ mod tests { #[test] fn remove_component_and_his_required_components() { - // We create this 'require' tree (down is required by up component) - // X - // \ - // Y - // \ - // Z V - // \ / - // W - - // And after remove X and this requirements we must have this tree (W will keep because it's still required by V) - // V - // / - // W - #[derive(Component)] #[require(Y)] struct X; @@ -2050,14 +2036,9 @@ mod tests { struct Y; #[derive(Component, Default)] - #[require(W)] struct Z; - #[derive(Component, Default)] - struct W; - - #[derive(Component, Default)] - #[require(W)] + #[derive(Component)] struct V; let mut world = World::new(); @@ -2066,7 +2047,6 @@ mod tests { assert!(world.entity(e).contains::()); assert!(world.entity(e).contains::()); assert!(world.entity(e).contains::()); - assert!(world.entity(e).contains::()); assert!(world.entity(e).contains::()); //check that `remove` works as expected @@ -2074,22 +2054,19 @@ mod tests { assert!(!world.entity(e).contains::()); assert!(world.entity(e).contains::()); assert!(world.entity(e).contains::()); - assert!(world.entity(e).contains::()); assert!(world.entity(e).contains::()); world.entity_mut(e).insert(X); assert!(world.entity(e).contains::()); assert!(world.entity(e).contains::()); assert!(world.entity(e).contains::()); - assert!(world.entity(e).contains::()); assert!(world.entity(e).contains::()); //remove `X` again and ensure that `Y` and `Z` was removed too - world.entity_mut(e).require_recursive_remove::(); + world.entity_mut(e).remove_with_required::(); assert!(!world.entity(e).contains::()); assert!(!world.entity(e).contains::()); assert!(!world.entity(e).contains::()); - assert!(world.entity(e).contains::()); assert!(world.entity(e).contains::()); } @@ -2127,7 +2104,7 @@ mod tests { assert!(world.entity(e).contains::()); assert!(world.entity(e).contains::()); - world.entity_mut(e).require_recursive_remove::(); + world.entity_mut(e).remove_with_required::(); assert!(!world.entity(e).contains::()); assert!(!world.entity(e).contains::()); assert!(!world.entity(e).contains::()); @@ -2135,76 +2112,6 @@ mod tests { assert!(world.entity(e).contains::()); } - #[test] - fn test_require_descendant_remove_with_shared_dependency() { - #[derive(Component, Default)] - #[require(B)] - struct A; - - #[derive(Component, Default)] - #[require(C)] - struct B; - - #[derive(Component, Default)] - #[require(D)] - struct C; - - #[derive(Component, Default)] - struct D; - - #[derive(Component, Default)] - #[require(D)] - struct E; - - //Require tree: A -> B -> C -> D <- E - - let mut world = World::new(); - let entity = world.spawn((A, B, C, D, E)).id(); - - world.entity_mut(entity).require_descendant_remove::(); - - assert!(!world.entity(entity).contains::()); - assert!(!world.entity(entity).contains::()); - assert!(!world.entity(entity).contains::()); - assert!(world.entity(entity).contains::()); - assert!(world.entity(entity).contains::()); - } - - #[test] - fn test_require_descendant_remove_leaf_component() { - #[derive(Component, Default)] - #[require(B)] - struct A; - - #[derive(Component, Default)] - #[require(C)] - struct B; - - #[derive(Component, Default)] - #[require(D)] - struct C; - - #[derive(Component, Default)] - struct D; - - #[derive(Component, Default)] - #[require(D)] - struct E; - - //Require tree: A -> B -> C -> D <- E - - let mut world = World::new(); - let entity = world.spawn((A, B, C, D, E)).id(); - - world.entity_mut(entity).require_descendant_remove::(); - - assert!(!world.entity(entity).contains::()); - assert!(!world.entity(entity).contains::()); - assert!(!world.entity(entity).contains::()); - assert!(!world.entity(entity).contains::()); - assert!(!world.entity(entity).contains::()); - } - // These structs are primarily compilation tests to test the derive macros. Because they are // never constructed, we have to manually silence the `dead_code` lint. #[allow(dead_code)] diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 5ef985c42a01b..4ccfb42e03dad 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1216,128 +1216,32 @@ impl EntityCommands<'_> { self.add(remove::) } - /// Removes all components in the bundle and remove all required components for each component in the bundle - /// that are not required by other components of this entity. - /// - /// This function can be noticeably slower than simple remove or retain functions because it dynamically determines which components - /// are still required by entity components outside of the [`Bundle`]. + /// Removes all components in the [`Bundle`] components and remove all required components for each component in the [`Bundle`] from entity. /// /// # Example /// - /// ```rust - /// use bevy_ecs::prelude::*; - /// - /// #[derive(Component)] - /// #[require(Y)] - /// struct X; - /// - /// #[derive(Component, Default)] - /// #[require(Z)] - /// struct Y; - /// - /// #[derive(Component, Default)] - /// struct Z; - /// - /// #[derive(Component)] - /// #[require(Z)] - /// struct W; - /// - /// fn remove_x_system(mut commands: Commands, query: Query>) { - /// for entity in &query { - /// // Remove X and its unused requirements - /// commands.entity(entity).require_recursive_remove::(); - /// } - /// } - /// - /// // Usage in a system: - /// fn setup(mut commands: Commands) { - /// // Spawn an entity with X, Y, Z, and W components - /// commands.spawn((X, W)); - /// - /// // Initial component tree: - /// // X - /// // \ - /// // Y W - /// // \ / - /// // Z - /// } - /// - /// // After calling remove_x_system: - /// // Resulting component tree: - /// // W - /// // / - /// // Z /// ``` - pub fn require_recursive_remove(self) -> Self { - self.add(require_recursive_remove::) - } - - /// Removes the components of this [`Bundle`], components that require them, and their required components, - /// unless they are required by components outside of this removal set. - /// - /// This method performs a top-down removal, starting by collecting all bundle components, all required components for bundle components, all components that require bundle components. - /// Then removing all collected components, unless those components are required outside remove set - /// elsewhere in the entity. - /// - /// This function can be noticeably slower than simple remove or retain functions because it dynamically determines which components - /// are still required by entity components outside of the [`Bundle`]. - /// - /// # Example - /// - /// ```rust /// use bevy_ecs::prelude::*; /// /// #[derive(Component)] /// #[require(B)] /// struct A; - /// /// #[derive(Component, Default)] - /// #[require(C)] /// struct B; /// - /// #[derive(Component, Default)] - /// #[require(D)] - /// struct C; - /// - /// #[derive(Component, Default)] - /// struct D; - /// - /// #[derive(Component)] - /// #[require(D)] - /// struct E; - /// - /// fn remove_b_system(mut commands: Commands, query: Query>) { - /// for entity in &query { - /// // Remove B and its requirement tree - /// commands.entity(entity).require_descendant_remove::(); - /// } - /// } - /// - /// // Usage in a system: - /// fn setup(mut commands: Commands) { - /// // Spawn an entity with A, B, C, D, and E components - /// commands.spawn((A, B, C, D, E)); + /// #[derive(Resource)] + /// struct PlayerEntity { entity: Entity } /// - /// // Initial component structure: - /// // A -> B -> C -> D <- E + /// fn remove_with_required_system(mut commands: Commands, player: Res) { + /// commands + /// .entity(player.entity) + /// // Remove both A and B components from the entity, because B is required by A + /// .remove_with_required::(); /// } - /// - /// // After calling remove_b_system: - /// // Resulting component structure: - /// // D <- E - /// // - /// // A is removed as it requires B - /// // B is removed - /// // C is removed as it's required by B - /// // D is kept as E still requires it - /// // E is kept + /// # bevy_ecs::system::assert_is_system(remove_with_required_system); /// ``` - /// - /// This method is useful for removing a component along with its entire requirement tree and any - /// components that depend on them, while preserving components that are still required by parts of - /// the entity outside the removal tree. - pub fn require_descendant_remove(self) -> Self { - self.add(require_descendant_remove::) + pub fn remove_with_required(self) -> Self { + self.add(remove_with_required::) } /// Removes a component from the entity. @@ -1654,15 +1558,9 @@ fn remove_by_id(component_id: ComponentId) -> impl EntityCommand { /// /// This function can be noticeably slower than simple remove or retain functions because it dynamically determines which components /// are still required by entity components outside of the [`Bundle`]. -fn require_recursive_remove(entity: Entity, world: &mut World) { +fn remove_with_required(entity: Entity, world: &mut World) { if let Some(mut entity) = world.get_entity_mut(entity) { - entity.require_recursive_remove::(); - } -} - -fn require_descendant_remove(entity: Entity, world: &mut World) { - if let Some(mut entity) = world.get_entity_mut(entity) { - entity.require_descendant_remove::(); + entity.remove_with_required::(); } } @@ -1957,42 +1855,31 @@ mod tests { #[derive(Component, Default)] struct Y; + #[derive(Component)] + struct Z; + let mut world = World::default(); let mut queue = CommandQueue::default(); let e = { let mut commands = Commands::new(&mut queue, &world); - commands.spawn(X).id() + commands.spawn((X, Z)).id() }; queue.apply(&mut world); assert!(world.get::(e).is_some()); assert!(world.get::(e).is_some()); + assert!(world.get::(e).is_some()); { let mut commands = Commands::new(&mut queue, &world); - commands.entity(e).require_recursive_remove::(); + commands.entity(e).remove_with_required::(); } queue.apply(&mut world); assert!(world.get::(e).is_none()); assert!(world.get::(e).is_none()); - { - let mut commands = Commands::new(&mut queue, &world); - commands.entity(e).insert(X); - } - queue.apply(&mut world); - - assert!(world.get::(e).is_some()); - assert!(world.get::(e).is_some()); - - { - let mut commands = Commands::new(&mut queue, &world); - commands.entity(e).require_descendant_remove::(); - } - queue.apply(&mut world); - assert!(world.get::(e).is_none()); - assert!(world.get::(e).is_none()); + assert!(world.get::(e).is_some()); } fn is_send() {} diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index b1e492397e33b..7bb0db18a3d8e 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -13,7 +13,6 @@ use crate::{ world::{DeferredWorld, Mut, World}, }; use bevy_ptr::{OwningPtr, Ptr}; -use bevy_utils::HashSet; use std::{any::TypeId, marker::PhantomData}; use thiserror::Error; @@ -1230,7 +1229,25 @@ impl<'w> EntityWorldMut<'w> { self } - + /// Removes all components in the [`Bundle`] and remove all required components for each component in the bundle + pub fn remove_with_required(&mut self) -> &mut Self { + let storages = &mut self.world.storages; + let components = &mut self.world.components; + let bundles = &mut self.world.bundles; + + let bundle_id = bundles.init_info::(components, storages); + + // SAFETY: the `BundleInfo` is initialized above + let bundle_info = unsafe { bundles.get_unchecked(bundle_id) }; + + let contributed_components = bundle_info.contributed_components().to_vec(); + let extended_bundle = bundles.init_dynamic_info(components, &contributed_components); + + // SAFETY: the dynamic `BundleInfo` is initialized above + self.location = unsafe { self.remove_bundle(extended_bundle) }; + + self + } /// Removes any components except those in the [`Bundle`] (and its Required Components) from the entity. /// From ea4a15272736c3b6b9fd05494c82a5bbff77e9eb Mon Sep 17 00:00:00 2001 From: "a.yamaev" Date: Wed, 2 Oct 2024 11:39:43 +0300 Subject: [PATCH 14/25] Update to latest main api --- crates/bevy_ecs/src/system/commands/mod.rs | 2 +- crates/bevy_ecs/src/world/entity_ref.rs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 52583013ab7ed..8248ee4e177b5 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1392,7 +1392,7 @@ impl EntityCommands<'_> { /// # bevy_ecs::system::assert_is_system(remove_with_required_system); /// ``` pub fn remove_with_required(self) -> Self { - self.add(remove_with_required::) + self.queue(remove_with_required::) } /// Removes a component from the entity. diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index e791037c48353..a605a3029d092 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -1235,11 +1235,12 @@ impl<'w> EntityWorldMut<'w> { let components = &mut self.world.components; let bundles = &mut self.world.bundles; - let bundle_id = bundles.init_info::(components, storages); + let bundle_id = bundles.register_info::(components, storages); // SAFETY: the `BundleInfo` is initialized above let bundle_info = unsafe { bundles.get_unchecked(bundle_id) }; + // We made copy to fix borrowing issue let contributed_components = bundle_info.contributed_components().to_vec(); let extended_bundle = bundles.init_dynamic_info(components, &contributed_components); From 76950163f6acfaf65dc3f8d367044776af3ec373 Mon Sep 17 00:00:00 2001 From: "a.yamaev" Date: Wed, 2 Oct 2024 11:57:43 +0300 Subject: [PATCH 15/25] rename remove_with_required into remove_with_requires --- crates/bevy_ecs/src/lib.rs | 4 ++-- crates/bevy_ecs/src/system/commands/mod.rs | 16 ++++++++-------- crates/bevy_ecs/src/world/entity_ref.rs | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 0fe27750dd333..b4485f181240c 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -2067,7 +2067,7 @@ mod tests { assert!(world.entity(e).contains::()); //remove `X` again and ensure that `Y` and `Z` was removed too - world.entity_mut(e).remove_with_required::(); + world.entity_mut(e).remove_with_requires::(); assert!(!world.entity(e).contains::()); assert!(!world.entity(e).contains::()); assert!(!world.entity(e).contains::()); @@ -2108,7 +2108,7 @@ mod tests { assert!(world.entity(e).contains::()); assert!(world.entity(e).contains::()); - world.entity_mut(e).remove_with_required::(); + world.entity_mut(e).remove_with_requires::(); assert!(!world.entity(e).contains::()); assert!(!world.entity(e).contains::()); assert!(!world.entity(e).contains::()); diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 8248ee4e177b5..04c0f7d006f4e 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1383,16 +1383,16 @@ impl EntityCommands<'_> { /// #[derive(Resource)] /// struct PlayerEntity { entity: Entity } /// - /// fn remove_with_required_system(mut commands: Commands, player: Res) { + /// fn remove_with_requires_system(mut commands: Commands, player: Res) { /// commands /// .entity(player.entity) /// // Remove both A and B components from the entity, because B is required by A - /// .remove_with_required::(); + /// .remove_with_requires::(); /// } - /// # bevy_ecs::system::assert_is_system(remove_with_required_system); + /// # bevy_ecs::system::assert_is_system(remove_with_requires_system); /// ``` - pub fn remove_with_required(self) -> Self { - self.queue(remove_with_required::) + pub fn remove_with_requires(self) -> Self { + self.queue(remove_with_requires::) } /// Removes a component from the entity. @@ -1836,9 +1836,9 @@ fn remove_by_id(component_id: ComponentId) -> impl EntityCommand { /// /// This function can be noticeably slower than simple remove or retain functions because it dynamically determines which components /// are still required by entity components outside of the [`Bundle`]. -fn remove_with_required(entity: Entity, world: &mut World) { +fn remove_with_requires(entity: Entity, world: &mut World) { if let Some(mut entity) = world.get_entity_mut(entity) { - entity.remove_with_required::(); + entity.remove_with_requires::(); } } @@ -2232,7 +2232,7 @@ mod tests { { let mut commands = Commands::new(&mut queue, &world); - commands.entity(e).remove_with_required::(); + commands.entity(e).remove_with_requires::(); } queue.apply(&mut world); diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index a605a3029d092..af949f72ead0c 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -1230,7 +1230,7 @@ impl<'w> EntityWorldMut<'w> { } /// Removes all components in the [`Bundle`] and remove all required components for each component in the bundle - pub fn remove_with_required(&mut self) -> &mut Self { + pub fn remove_with_requires(&mut self) -> &mut Self { let storages = &mut self.world.storages; let components = &mut self.world.components; let bundles = &mut self.world.bundles; From fd2ea81b9957251cac02b94c903121979ccdf086 Mon Sep 17 00:00:00 2001 From: "a.yamaev" Date: Wed, 2 Oct 2024 12:06:09 +0300 Subject: [PATCH 16/25] fix docs and namings --- crates/bevy_ecs/src/system/commands/mod.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 04c0f7d006f4e..a154397b01c85 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1831,11 +1831,7 @@ fn remove_by_id(component_id: ComponentId) -> impl EntityCommand { } } -/// An [`EntityCommand`] tjhat remove all components in the bundle and remove all required components for each component in the bundle -/// that are not required by other components of this entity. -/// -/// This function can be noticeably slower than simple remove or retain functions because it dynamically determines which components -/// are still required by entity components outside of the [`Bundle`]. +/// An [`EntityCommand`] that remove all components in the bundle and remove all required components for each component in the bundle. fn remove_with_requires(entity: Entity, world: &mut World) { if let Some(mut entity) = world.get_entity_mut(entity) { entity.remove_with_requires::(); @@ -2207,7 +2203,7 @@ mod tests { } #[test] - fn remove_bundle_with_required_components() { + fn remove_component_with_required_components() { #[derive(Component)] #[require(Y)] struct X; From b7caaa86d9567166df8efae2e94393356ad563b8 Mon Sep 17 00:00:00 2001 From: "a.yamaev" Date: Wed, 2 Oct 2024 17:02:59 +0300 Subject: [PATCH 17/25] cache system for required components --- crates/bevy_ecs/src/bundle.rs | 27 +++++++++++++++ crates/bevy_ecs/src/lib.rs | 45 +++++++++++++++++++++++++ crates/bevy_ecs/src/world/entity_ref.rs | 11 ++---- 3 files changed, 74 insertions(+), 9 deletions(-) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 7a23ac3f11e06..7b2e860ff14b8 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -1301,6 +1301,8 @@ pub struct Bundles { bundle_infos: Vec, /// Cache static [`BundleId`] bundle_ids: TypeIdMap, + /// Cache bundles, which contains both explicit and required components of [`Bundle`] + contributed_bundle_ids: TypeIdMap, /// Cache dynamic [`BundleId`] with multiple components dynamic_bundle_ids: HashMap, BundleId>, dynamic_bundle_storages: HashMap>, @@ -1334,6 +1336,7 @@ impl Bundles { storages: &mut Storages, ) -> BundleId { let bundle_infos = &mut self.bundle_infos; + let contributed_bundle_ids = &mut self.contributed_bundle_ids; let id = *self.bundle_ids.entry(TypeId::of::()).or_insert_with(|| { let mut component_ids= Vec::new(); T::component_ids(components, storages, &mut |id| component_ids.push(id)); @@ -1345,11 +1348,35 @@ impl Bundles { // - it was created in the same order as the components in T unsafe { BundleInfo::new(core::any::type_name::(), components, component_ids, id) }; bundle_infos.push(bundle_info); + + // Be sure to update component bundle with requires + contributed_bundle_ids.remove(&TypeId::of::()); + id }); id } + pub(crate) fn register_contributed_bundle_info( + &mut self, + components: &mut Components, + storages: &mut Storages, + ) -> BundleId { + if let Some(id) = self.contributed_bundle_ids.get(&TypeId::of::()).cloned() { + id + } else { + let explicit_bundle_id = self.register_info::(components, storages); + // SAFETY: `explicit_bundle_id` is valid and defined above + let contributed_components = unsafe { + self.get_unchecked(explicit_bundle_id) + .contributed_components() + .to_vec() + }; + + self.init_dynamic_info(components, &contributed_components) + } + } + /// # Safety /// A [`BundleInfo`] with the given [`BundleId`] must have been initialized for this instance of `Bundles`. pub(crate) unsafe fn get_unchecked(&self, id: BundleId) -> &BundleInfo { diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index b4485f181240c..0bd6f1b6d9912 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -2029,6 +2029,51 @@ mod tests { assert!(e.contains::()); } + #[test] + fn remove_component_and_his_runtime_required_components() { + #[derive(Component)] + struct X; + + #[derive(Component, Default)] + struct Y; + + #[derive(Component, Default)] + struct Z; + + #[derive(Component)] + struct V; + + let mut world = World::new(); + world.register_required_components::(); + world.register_required_components::(); + + let e = world.spawn((X, V)).id(); + assert!(world.entity(e).contains::()); + assert!(world.entity(e).contains::()); + assert!(world.entity(e).contains::()); + assert!(world.entity(e).contains::()); + + //check that `remove` works as expected + world.entity_mut(e).remove::(); + assert!(!world.entity(e).contains::()); + assert!(world.entity(e).contains::()); + assert!(world.entity(e).contains::()); + assert!(world.entity(e).contains::()); + + world.entity_mut(e).insert(X); + assert!(world.entity(e).contains::()); + assert!(world.entity(e).contains::()); + assert!(world.entity(e).contains::()); + assert!(world.entity(e).contains::()); + + //remove `X` again and ensure that `Y` and `Z` was removed too + world.entity_mut(e).remove_with_requires::(); + assert!(!world.entity(e).contains::()); + assert!(!world.entity(e).contains::()); + assert!(!world.entity(e).contains::()); + assert!(world.entity(e).contains::()); + } + #[test] fn remove_component_and_his_required_components() { #[derive(Component)] diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index af949f72ead0c..02a8f9c4c8db8 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -1235,17 +1235,10 @@ impl<'w> EntityWorldMut<'w> { let components = &mut self.world.components; let bundles = &mut self.world.bundles; - let bundle_id = bundles.register_info::(components, storages); - - // SAFETY: the `BundleInfo` is initialized above - let bundle_info = unsafe { bundles.get_unchecked(bundle_id) }; - - // We made copy to fix borrowing issue - let contributed_components = bundle_info.contributed_components().to_vec(); - let extended_bundle = bundles.init_dynamic_info(components, &contributed_components); + let bundle_id = bundles.register_contributed_bundle_info::(components, storages); // SAFETY: the dynamic `BundleInfo` is initialized above - self.location = unsafe { self.remove_bundle(extended_bundle) }; + self.location = unsafe { self.remove_bundle(bundle_id) }; self } From c6a953cb0129d2bb72c77ae802f511d4f280616e Mon Sep 17 00:00:00 2001 From: "a.yamaev" Date: Wed, 2 Oct 2024 17:18:43 +0300 Subject: [PATCH 18/25] fix caching --- crates/bevy_ecs/src/bundle.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 7b2e860ff14b8..f3e0c87c87b20 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -1372,8 +1372,9 @@ impl Bundles { .contributed_components() .to_vec() }; - - self.init_dynamic_info(components, &contributed_components) + let id = self.init_dynamic_info(components, &contributed_components); + self.contributed_bundle_ids.insert(TypeId::of::(), id); + id } } From 627a4e0698c007545f26798d5754f258b1109775 Mon Sep 17 00:00:00 2001 From: "a.yamaev" Date: Wed, 2 Oct 2024 17:35:11 +0300 Subject: [PATCH 19/25] change code to last api --- crates/bevy_ecs/src/system/commands/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index f96e4440b2b0a..db5a277d676de 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1393,7 +1393,7 @@ impl EntityCommands<'_> { /// } /// # bevy_ecs::system::assert_is_system(remove_with_requires_system); /// ``` - pub fn remove_with_requires(self) -> Self { + pub fn remove_with_requires(&mut self) -> &mut Self { self.queue(remove_with_requires::) } From 1d3a205747a3a7d7a179e7768427943371b3d91d Mon Sep 17 00:00:00 2001 From: "a.yamaev" Date: Wed, 2 Oct 2024 17:47:20 +0300 Subject: [PATCH 20/25] Some more docs --- crates/bevy_ecs/src/bundle.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 5e51a9ae5eaed..142e0afa41d43 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -1358,6 +1358,9 @@ impl Bundles { id } + /// Registers a new [`BundleInfo`], which contains both explicit and required components for a staticly known type. + /// + /// Also registers all the components in the bundle. pub(crate) fn register_contributed_bundle_info( &mut self, components: &mut Components, From e3dcbe2cc833c7c9894a87a28d1899bb96651b65 Mon Sep 17 00:00:00 2001 From: "a.yamaev" Date: Wed, 2 Oct 2024 17:53:41 +0300 Subject: [PATCH 21/25] fix typos --- crates/bevy_ecs/src/bundle.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 142e0afa41d43..28185758b4154 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -1358,7 +1358,7 @@ impl Bundles { id } - /// Registers a new [`BundleInfo`], which contains both explicit and required components for a staticly known type. + /// Registers a new [`BundleInfo`], which contains both explicit and required components for a statically known type. /// /// Also registers all the components in the bundle. pub(crate) fn register_contributed_bundle_info( From 644f8ecc7c1093f3e703e240d817fea6f951ffcc Mon Sep 17 00:00:00 2001 From: rewin Date: Wed, 2 Oct 2024 21:14:14 +0300 Subject: [PATCH 22/25] Optimizing from cart Co-authored-by: Carter Anderson --- crates/bevy_ecs/src/bundle.rs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 28185758b4154..ce5e6b1fc8c86 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -1349,10 +1349,6 @@ impl Bundles { // - it was created in the same order as the components in T unsafe { BundleInfo::new(core::any::type_name::(), components, component_ids, id) }; bundle_infos.push(bundle_info); - - // Be sure to update component bundle with requires - contributed_bundle_ids.remove(&TypeId::of::()); - id }); id @@ -1370,13 +1366,18 @@ impl Bundles { id } else { let explicit_bundle_id = self.register_info::(components, storages); - // SAFETY: `explicit_bundle_id` is valid and defined above - let contributed_components = unsafe { - self.get_unchecked(explicit_bundle_id) - .contributed_components() - .to_vec() + let id = unsafe { + let (ptr, len) = { + // SAFETY: `explicit_bundle_id` is valid and defined above + let contributed = self + .get_unchecked(explicit_bundle_id) + .contributed_components(); + (contributed.as_ptr(), contributed.len()) + }; + // SAFETY: this is sound because the contributed_components Vec for explicit_bundle_id will not be accessed mutably as + // part of init_dynamic_info. No mutable references will be created and the allocation will remain valid. + self.init_dynamic_info(components, std::slice::from_raw_parts(ptr, len)) }; - let id = self.init_dynamic_info(components, &contributed_components); self.contributed_bundle_ids.insert(TypeId::of::(), id); id } From 567c23ede4cf68300f900f58a3ba35f382229d30 Mon Sep 17 00:00:00 2001 From: rewin Date: Wed, 2 Oct 2024 21:24:13 +0300 Subject: [PATCH 23/25] Fix warnings --- crates/bevy_ecs/src/bundle.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index ce5e6b1fc8c86..8883622f78144 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -1337,7 +1337,6 @@ impl Bundles { storages: &mut Storages, ) -> BundleId { let bundle_infos = &mut self.bundle_infos; - let contributed_bundle_ids = &mut self.contributed_bundle_ids; let id = *self.bundle_ids.entry(TypeId::of::()).or_insert_with(|| { let mut component_ids= Vec::new(); T::component_ids(components, storages, &mut |id| component_ids.push(id)); @@ -1366,6 +1365,7 @@ impl Bundles { id } else { let explicit_bundle_id = self.register_info::(components, storages); + // SAFETY: reading from `explicit_bundle_id` and creating new bundle in same time. Its valid because bundle hashmap alllow this let id = unsafe { let (ptr, len) = { // SAFETY: `explicit_bundle_id` is valid and defined above From 73b73c26420616fb9f98aac66fed3802e844d3fe Mon Sep 17 00:00:00 2001 From: rewin Date: Wed, 2 Oct 2024 21:26:14 +0300 Subject: [PATCH 24/25] fix typos --- crates/bevy_ecs/src/bundle.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 8883622f78144..acd61222a1587 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -1365,7 +1365,7 @@ impl Bundles { id } else { let explicit_bundle_id = self.register_info::(components, storages); - // SAFETY: reading from `explicit_bundle_id` and creating new bundle in same time. Its valid because bundle hashmap alllow this + // SAFETY: reading from `explicit_bundle_id` and creating new bundle in same time. Its valid because bundle hashmap allow this let id = unsafe { let (ptr, len) = { // SAFETY: `explicit_bundle_id` is valid and defined above From 829e849e7e6d0c8f60bdba65d3ffab097d904ccd Mon Sep 17 00:00:00 2001 From: rewin Date: Wed, 2 Oct 2024 21:31:15 +0300 Subject: [PATCH 25/25] fix weird std to core replacement error --- crates/bevy_ecs/src/bundle.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index acd61222a1587..56e11e854e13e 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -1376,7 +1376,7 @@ impl Bundles { }; // SAFETY: this is sound because the contributed_components Vec for explicit_bundle_id will not be accessed mutably as // part of init_dynamic_info. No mutable references will be created and the allocation will remain valid. - self.init_dynamic_info(components, std::slice::from_raw_parts(ptr, len)) + self.init_dynamic_info(components, core::slice::from_raw_parts(ptr, len)) }; self.contributed_bundle_ids.insert(TypeId::of::(), id); id