Skip to content

Commit 8bf5d99

Browse files
rewin123ayamaev-secart
authored
Add method to remove component and all required components for removed component (#15026)
## Objective The new Required Components feature (#14791) in Bevy allows spawning a fixed set of components with a single method with cool require macro. However, there's currently no corresponding method to remove all those components together. This makes it challenging to keep insertion and removal code in sync, especially for simple using cases. ```rust #[derive(Component)] #[require(Y)] struct X; #[derive(Component, Default)] struct Y; world.entity_mut(e).insert(X); // Spawns both X and Y world.entity_mut(e).remove::<X>(); world.entity_mut(e).remove::<Y>(); // We need to manually remove dependencies without any sync with the `require` macro ``` ## Solution Simplifies component management by providing operations for removal required components. This PR introduces simple 'footgun' methods to removes all components of this bundle and its required components. Two new methods are introduced: For Commands: ```rust commands.entity(e).remove_with_requires::<B>(); ``` For World: ```rust world.entity_mut(e).remove_with_requires::<B>(); ``` For performance I created new field in Bundels struct. This new field "contributed_bundle_ids" contains cached ids for dynamic bundles constructed from bundle_info.cintributed_components() ## Testing The PR includes three test cases: 1. Removing a single component with requirements using World. 2. Removing a bundle with requirements using World. 3. Removing a single component with requirements using Commands. 4. Removing a single component with **runtime** requirements using Commands These tests ensure the feature works as expected across different scenarios. ## Showcase 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)] struct W; let mut world = World::new(); // Spawn an entity with X, Y, Z, and W components let entity = world.spawn((X, W)).id(); assert!(world.entity(entity).contains::<X>()); assert!(world.entity(entity).contains::<Y>()); assert!(world.entity(entity).contains::<Z>()); assert!(world.entity(entity).contains::<W>()); // Remove X and required components Y, Z world.entity_mut(entity).remove_with_requires::<X>(); assert!(!world.entity(entity).contains::<X>()); assert!(!world.entity(entity).contains::<Y>()); assert!(!world.entity(entity).contains::<Z>()); assert!(world.entity(entity).contains::<W>()); ``` ## Motivation for PR #15580 ## Performance I made simple benchmark ```rust let mut world = World::default(); let entity = world.spawn_empty().id(); let steps = 100_000_000; let start = std::time::Instant::now(); for _ in 0..steps { world.entity_mut(entity).insert(X); world.entity_mut(entity).remove::<(X, Y, Z, W)>(); } let end = std::time::Instant::now(); println!("normal remove: {:?} ", (end - start).as_secs_f32()); println!("one remove: {:?} micros", (end - start).as_secs_f64() / steps as f64 * 1_000_000.0); let start = std::time::Instant::now(); for _ in 0..steps { world.entity_mut(entity).insert(X); world.entity_mut(entity).remove_with_requires::<X>(); } let end = std::time::Instant::now(); println!("remove_with_requires: {:?} ", (end - start).as_secs_f32()); println!("one remove_with_requires: {:?} micros", (end - start).as_secs_f64() / steps as f64 * 1_000_000.0); ``` Output: CPU: Amd Ryzen 7 2700x ```bash normal remove: 17.36135 one remove: 0.17361348299999999 micros remove_with_requires: 17.534006 one remove_with_requires: 0.17534005400000002 micros ``` NOTE: I didn't find any tests or mechanism in the repository to update BundleInfo after creating new runtime requirements with an existing BundleInfo. So this PR also does not contain such logic. ## Future work (outside this PR) Create cache system for fast removing components in "safe" mode, where "safe" mode is remove only required components that will be no longer required after removing root component. --------- Co-authored-by: a.yamaev <[email protected]> Co-authored-by: Carter Anderson <[email protected]>
1 parent 0628255 commit 8bf5d99

File tree

4 files changed

+249
-0
lines changed

4 files changed

+249
-0
lines changed

crates/bevy_ecs/src/bundle.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1302,6 +1302,8 @@ pub struct Bundles {
13021302
bundle_infos: Vec<BundleInfo>,
13031303
/// Cache static [`BundleId`]
13041304
bundle_ids: TypeIdMap<BundleId>,
1305+
/// Cache bundles, which contains both explicit and required components of [`Bundle`]
1306+
contributed_bundle_ids: TypeIdMap<BundleId>,
13051307
/// Cache dynamic [`BundleId`] with multiple components
13061308
dynamic_bundle_ids: HashMap<Box<[ComponentId]>, BundleId>,
13071309
dynamic_bundle_storages: HashMap<BundleId, Vec<StorageType>>,
@@ -1351,6 +1353,36 @@ impl Bundles {
13511353
id
13521354
}
13531355

1356+
/// Registers a new [`BundleInfo`], which contains both explicit and required components for a statically known type.
1357+
///
1358+
/// Also registers all the components in the bundle.
1359+
pub(crate) fn register_contributed_bundle_info<T: Bundle>(
1360+
&mut self,
1361+
components: &mut Components,
1362+
storages: &mut Storages,
1363+
) -> BundleId {
1364+
if let Some(id) = self.contributed_bundle_ids.get(&TypeId::of::<T>()).cloned() {
1365+
id
1366+
} else {
1367+
let explicit_bundle_id = self.register_info::<T>(components, storages);
1368+
// SAFETY: reading from `explicit_bundle_id` and creating new bundle in same time. Its valid because bundle hashmap allow this
1369+
let id = unsafe {
1370+
let (ptr, len) = {
1371+
// SAFETY: `explicit_bundle_id` is valid and defined above
1372+
let contributed = self
1373+
.get_unchecked(explicit_bundle_id)
1374+
.contributed_components();
1375+
(contributed.as_ptr(), contributed.len())
1376+
};
1377+
// SAFETY: this is sound because the contributed_components Vec for explicit_bundle_id will not be accessed mutably as
1378+
// part of init_dynamic_info. No mutable references will be created and the allocation will remain valid.
1379+
self.init_dynamic_info(components, core::slice::from_raw_parts(ptr, len))
1380+
};
1381+
self.contributed_bundle_ids.insert(TypeId::of::<T>(), id);
1382+
id
1383+
}
1384+
}
1385+
13541386
/// # Safety
13551387
/// A [`BundleInfo`] with the given [`BundleId`] must have been initialized for this instance of `Bundles`.
13561388
pub(crate) unsafe fn get_unchecked(&self, id: BundleId) -> &BundleInfo {

crates/bevy_ecs/src/lib.rs

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2029,6 +2029,138 @@ mod tests {
20292029
assert!(e.contains::<Y>());
20302030
}
20312031

2032+
#[test]
2033+
fn remove_component_and_his_runtime_required_components() {
2034+
#[derive(Component)]
2035+
struct X;
2036+
2037+
#[derive(Component, Default)]
2038+
struct Y;
2039+
2040+
#[derive(Component, Default)]
2041+
struct Z;
2042+
2043+
#[derive(Component)]
2044+
struct V;
2045+
2046+
let mut world = World::new();
2047+
world.register_required_components::<X, Y>();
2048+
world.register_required_components::<Y, Z>();
2049+
2050+
let e = world.spawn((X, V)).id();
2051+
assert!(world.entity(e).contains::<X>());
2052+
assert!(world.entity(e).contains::<Y>());
2053+
assert!(world.entity(e).contains::<Z>());
2054+
assert!(world.entity(e).contains::<V>());
2055+
2056+
//check that `remove` works as expected
2057+
world.entity_mut(e).remove::<X>();
2058+
assert!(!world.entity(e).contains::<X>());
2059+
assert!(world.entity(e).contains::<Y>());
2060+
assert!(world.entity(e).contains::<Z>());
2061+
assert!(world.entity(e).contains::<V>());
2062+
2063+
world.entity_mut(e).insert(X);
2064+
assert!(world.entity(e).contains::<X>());
2065+
assert!(world.entity(e).contains::<Y>());
2066+
assert!(world.entity(e).contains::<Z>());
2067+
assert!(world.entity(e).contains::<V>());
2068+
2069+
//remove `X` again and ensure that `Y` and `Z` was removed too
2070+
world.entity_mut(e).remove_with_requires::<X>();
2071+
assert!(!world.entity(e).contains::<X>());
2072+
assert!(!world.entity(e).contains::<Y>());
2073+
assert!(!world.entity(e).contains::<Z>());
2074+
assert!(world.entity(e).contains::<V>());
2075+
}
2076+
2077+
#[test]
2078+
fn remove_component_and_his_required_components() {
2079+
#[derive(Component)]
2080+
#[require(Y)]
2081+
struct X;
2082+
2083+
#[derive(Component, Default)]
2084+
#[require(Z)]
2085+
struct Y;
2086+
2087+
#[derive(Component, Default)]
2088+
struct Z;
2089+
2090+
#[derive(Component)]
2091+
struct V;
2092+
2093+
let mut world = World::new();
2094+
2095+
let e = world.spawn((X, V)).id();
2096+
assert!(world.entity(e).contains::<X>());
2097+
assert!(world.entity(e).contains::<Y>());
2098+
assert!(world.entity(e).contains::<Z>());
2099+
assert!(world.entity(e).contains::<V>());
2100+
2101+
//check that `remove` works as expected
2102+
world.entity_mut(e).remove::<X>();
2103+
assert!(!world.entity(e).contains::<X>());
2104+
assert!(world.entity(e).contains::<Y>());
2105+
assert!(world.entity(e).contains::<Z>());
2106+
assert!(world.entity(e).contains::<V>());
2107+
2108+
world.entity_mut(e).insert(X);
2109+
assert!(world.entity(e).contains::<X>());
2110+
assert!(world.entity(e).contains::<Y>());
2111+
assert!(world.entity(e).contains::<Z>());
2112+
assert!(world.entity(e).contains::<V>());
2113+
2114+
//remove `X` again and ensure that `Y` and `Z` was removed too
2115+
world.entity_mut(e).remove_with_requires::<X>();
2116+
assert!(!world.entity(e).contains::<X>());
2117+
assert!(!world.entity(e).contains::<Y>());
2118+
assert!(!world.entity(e).contains::<Z>());
2119+
assert!(world.entity(e).contains::<V>());
2120+
}
2121+
2122+
#[test]
2123+
fn remove_bundle_and_his_required_components() {
2124+
#[derive(Component, Default)]
2125+
#[require(Y)]
2126+
struct X;
2127+
2128+
#[derive(Component, Default)]
2129+
struct Y;
2130+
2131+
#[derive(Component, Default)]
2132+
#[require(W)]
2133+
struct Z;
2134+
2135+
#[derive(Component, Default)]
2136+
struct W;
2137+
2138+
#[derive(Component)]
2139+
struct V;
2140+
2141+
#[derive(Bundle, Default)]
2142+
struct TestBundle {
2143+
x: X,
2144+
z: Z,
2145+
}
2146+
2147+
let mut world = World::new();
2148+
let e = world.spawn((TestBundle::default(), V)).id();
2149+
2150+
assert!(world.entity(e).contains::<X>());
2151+
assert!(world.entity(e).contains::<Y>());
2152+
assert!(world.entity(e).contains::<Z>());
2153+
assert!(world.entity(e).contains::<W>());
2154+
assert!(world.entity(e).contains::<V>());
2155+
2156+
world.entity_mut(e).remove_with_requires::<TestBundle>();
2157+
assert!(!world.entity(e).contains::<X>());
2158+
assert!(!world.entity(e).contains::<Y>());
2159+
assert!(!world.entity(e).contains::<Z>());
2160+
assert!(!world.entity(e).contains::<W>());
2161+
assert!(world.entity(e).contains::<V>());
2162+
}
2163+
20322164
#[test]
20332165
fn runtime_required_components() {
20342166
// Same as `required_components` test but with runtime registration

crates/bevy_ecs/src/system/commands/mod.rs

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1369,6 +1369,34 @@ impl EntityCommands<'_> {
13691369
self.queue(remove::<T>)
13701370
}
13711371

1372+
/// Removes all components in the [`Bundle`] components and remove all required components for each component in the [`Bundle`] from entity.
1373+
///
1374+
/// # Example
1375+
///
1376+
/// ```
1377+
/// use bevy_ecs::prelude::*;
1378+
///
1379+
/// #[derive(Component)]
1380+
/// #[require(B)]
1381+
/// struct A;
1382+
/// #[derive(Component, Default)]
1383+
/// struct B;
1384+
///
1385+
/// #[derive(Resource)]
1386+
/// struct PlayerEntity { entity: Entity }
1387+
///
1388+
/// fn remove_with_requires_system(mut commands: Commands, player: Res<PlayerEntity>) {
1389+
/// commands
1390+
/// .entity(player.entity)
1391+
/// // Remove both A and B components from the entity, because B is required by A
1392+
/// .remove_with_requires::<A>();
1393+
/// }
1394+
/// # bevy_ecs::system::assert_is_system(remove_with_requires_system);
1395+
/// ```
1396+
pub fn remove_with_requires<T: Bundle>(&mut self) -> &mut Self {
1397+
self.queue(remove_with_requires::<T>)
1398+
}
1399+
13721400
/// Removes a component from the entity.
13731401
pub fn remove_by_id(&mut self, component_id: ComponentId) -> &mut Self {
13741402
self.queue(remove_by_id(component_id))
@@ -1826,6 +1854,13 @@ fn remove_by_id(component_id: ComponentId) -> impl EntityCommand {
18261854
}
18271855
}
18281856

1857+
/// An [`EntityCommand`] that remove all components in the bundle and remove all required components for each component in the bundle.
1858+
fn remove_with_requires<T: Bundle>(entity: Entity, world: &mut World) {
1859+
if let Some(mut entity) = world.get_entity_mut(entity) {
1860+
entity.remove_with_requires::<T>();
1861+
}
1862+
}
1863+
18291864
/// An [`EntityCommand`] that removes all components associated with a provided entity.
18301865
fn clear() -> impl EntityCommand {
18311866
move |entity: Entity, world: &mut World| {
@@ -2190,6 +2225,42 @@ mod tests {
21902225
assert!(world.contains_resource::<W<f64>>());
21912226
}
21922227

2228+
#[test]
2229+
fn remove_component_with_required_components() {
2230+
#[derive(Component)]
2231+
#[require(Y)]
2232+
struct X;
2233+
2234+
#[derive(Component, Default)]
2235+
struct Y;
2236+
2237+
#[derive(Component)]
2238+
struct Z;
2239+
2240+
let mut world = World::default();
2241+
let mut queue = CommandQueue::default();
2242+
let e = {
2243+
let mut commands = Commands::new(&mut queue, &world);
2244+
commands.spawn((X, Z)).id()
2245+
};
2246+
queue.apply(&mut world);
2247+
2248+
assert!(world.get::<Y>(e).is_some());
2249+
assert!(world.get::<X>(e).is_some());
2250+
assert!(world.get::<Z>(e).is_some());
2251+
2252+
{
2253+
let mut commands = Commands::new(&mut queue, &world);
2254+
commands.entity(e).remove_with_requires::<X>();
2255+
}
2256+
queue.apply(&mut world);
2257+
2258+
assert!(world.get::<Y>(e).is_none());
2259+
assert!(world.get::<X>(e).is_none());
2260+
2261+
assert!(world.get::<Z>(e).is_some());
2262+
}
2263+
21932264
fn is_send<T: Send>() {}
21942265
fn is_sync<T: Sync>() {}
21952266

crates/bevy_ecs/src/world/entity_ref.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1558,6 +1558,20 @@ impl<'w> EntityWorldMut<'w> {
15581558
self
15591559
}
15601560

1561+
/// Removes all components in the [`Bundle`] and remove all required components for each component in the bundle
1562+
pub fn remove_with_requires<T: Bundle>(&mut self) -> &mut Self {
1563+
let storages = &mut self.world.storages;
1564+
let components = &mut self.world.components;
1565+
let bundles = &mut self.world.bundles;
1566+
1567+
let bundle_id = bundles.register_contributed_bundle_info::<T>(components, storages);
1568+
1569+
// SAFETY: the dynamic `BundleInfo` is initialized above
1570+
self.location = unsafe { self.remove_bundle(bundle_id) };
1571+
1572+
self
1573+
}
1574+
15611575
/// Removes any components except those in the [`Bundle`] (and its Required Components) from the entity.
15621576
///
15631577
/// See [`EntityCommands::retain`](crate::system::EntityCommands::retain) for more details.

0 commit comments

Comments
 (0)