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 <a.yamaev@smartengines.com>
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
This commit is contained in:
rewin 2024-10-03 23:35:08 +03:00 committed by GitHub
parent 0628255c45
commit 8bf5d99d86
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 249 additions and 0 deletions

View file

@ -1302,6 +1302,8 @@ pub struct Bundles {
bundle_infos: Vec<BundleInfo>,
/// Cache static [`BundleId`]
bundle_ids: TypeIdMap<BundleId>,
/// Cache bundles, which contains both explicit and required components of [`Bundle`]
contributed_bundle_ids: TypeIdMap<BundleId>,
/// Cache dynamic [`BundleId`] with multiple components
dynamic_bundle_ids: HashMap<Box<[ComponentId]>, BundleId>,
dynamic_bundle_storages: HashMap<BundleId, Vec<StorageType>>,
@ -1351,6 +1353,36 @@ impl Bundles {
id
}
/// 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<T: Bundle>(
&mut self,
components: &mut Components,
storages: &mut Storages,
) -> BundleId {
if let Some(id) = self.contributed_bundle_ids.get(&TypeId::of::<T>()).cloned() {
id
} else {
let explicit_bundle_id = self.register_info::<T>(components, storages);
// 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
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, core::slice::from_raw_parts(ptr, len))
};
self.contributed_bundle_ids.insert(TypeId::of::<T>(), id);
id
}
}
/// # 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 {

View file

@ -2029,6 +2029,138 @@ mod tests {
assert!(e.contains::<Y>());
}
#[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::<X, Y>();
world.register_required_components::<Y, Z>();
let e = world.spawn((X, V)).id();
assert!(world.entity(e).contains::<X>());
assert!(world.entity(e).contains::<Y>());
assert!(world.entity(e).contains::<Z>());
assert!(world.entity(e).contains::<V>());
//check that `remove` works as expected
world.entity_mut(e).remove::<X>();
assert!(!world.entity(e).contains::<X>());
assert!(world.entity(e).contains::<Y>());
assert!(world.entity(e).contains::<Z>());
assert!(world.entity(e).contains::<V>());
world.entity_mut(e).insert(X);
assert!(world.entity(e).contains::<X>());
assert!(world.entity(e).contains::<Y>());
assert!(world.entity(e).contains::<Z>());
assert!(world.entity(e).contains::<V>());
//remove `X` again and ensure that `Y` and `Z` was removed too
world.entity_mut(e).remove_with_requires::<X>();
assert!(!world.entity(e).contains::<X>());
assert!(!world.entity(e).contains::<Y>());
assert!(!world.entity(e).contains::<Z>());
assert!(world.entity(e).contains::<V>());
}
#[test]
fn remove_component_and_his_required_components() {
#[derive(Component)]
#[require(Y)]
struct X;
#[derive(Component, Default)]
#[require(Z)]
struct Y;
#[derive(Component, Default)]
struct Z;
#[derive(Component)]
struct V;
let mut world = World::new();
let e = world.spawn((X, V)).id();
assert!(world.entity(e).contains::<X>());
assert!(world.entity(e).contains::<Y>());
assert!(world.entity(e).contains::<Z>());
assert!(world.entity(e).contains::<V>());
//check that `remove` works as expected
world.entity_mut(e).remove::<X>();
assert!(!world.entity(e).contains::<X>());
assert!(world.entity(e).contains::<Y>());
assert!(world.entity(e).contains::<Z>());
assert!(world.entity(e).contains::<V>());
world.entity_mut(e).insert(X);
assert!(world.entity(e).contains::<X>());
assert!(world.entity(e).contains::<Y>());
assert!(world.entity(e).contains::<Z>());
assert!(world.entity(e).contains::<V>());
//remove `X` again and ensure that `Y` and `Z` was removed too
world.entity_mut(e).remove_with_requires::<X>();
assert!(!world.entity(e).contains::<X>());
assert!(!world.entity(e).contains::<Y>());
assert!(!world.entity(e).contains::<Z>());
assert!(world.entity(e).contains::<V>());
}
#[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::<X>());
assert!(world.entity(e).contains::<Y>());
assert!(world.entity(e).contains::<Z>());
assert!(world.entity(e).contains::<W>());
assert!(world.entity(e).contains::<V>());
world.entity_mut(e).remove_with_requires::<TestBundle>();
assert!(!world.entity(e).contains::<X>());
assert!(!world.entity(e).contains::<Y>());
assert!(!world.entity(e).contains::<Z>());
assert!(!world.entity(e).contains::<W>());
assert!(world.entity(e).contains::<V>());
}
#[test]
fn runtime_required_components() {
// Same as `required_components` test but with runtime registration

View file

@ -1369,6 +1369,34 @@ impl EntityCommands<'_> {
self.queue(remove::<T>)
}
/// Removes all components in the [`Bundle`] components and remove all required components for each component in the [`Bundle`] from entity.
///
/// # Example
///
/// ```
/// use bevy_ecs::prelude::*;
///
/// #[derive(Component)]
/// #[require(B)]
/// struct A;
/// #[derive(Component, Default)]
/// struct B;
///
/// #[derive(Resource)]
/// struct PlayerEntity { entity: Entity }
///
/// fn remove_with_requires_system(mut commands: Commands, player: Res<PlayerEntity>) {
/// commands
/// .entity(player.entity)
/// // Remove both A and B components from the entity, because B is required by A
/// .remove_with_requires::<A>();
/// }
/// # bevy_ecs::system::assert_is_system(remove_with_requires_system);
/// ```
pub fn remove_with_requires<T: Bundle>(&mut self) -> &mut Self {
self.queue(remove_with_requires::<T>)
}
/// Removes a component from the entity.
pub fn remove_by_id(&mut self, component_id: ComponentId) -> &mut Self {
self.queue(remove_by_id(component_id))
@ -1826,6 +1854,13 @@ fn remove_by_id(component_id: ComponentId) -> impl EntityCommand {
}
}
/// An [`EntityCommand`] that remove all components in the bundle and remove all required components for each component in the bundle.
fn remove_with_requires<T: Bundle>(entity: Entity, world: &mut World) {
if let Some(mut entity) = world.get_entity_mut(entity) {
entity.remove_with_requires::<T>();
}
}
/// An [`EntityCommand`] that removes all components associated with a provided entity.
fn clear() -> impl EntityCommand {
move |entity: Entity, world: &mut World| {
@ -2190,6 +2225,42 @@ mod tests {
assert!(world.contains_resource::<W<f64>>());
}
#[test]
fn remove_component_with_required_components() {
#[derive(Component)]
#[require(Y)]
struct X;
#[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, Z)).id()
};
queue.apply(&mut world);
assert!(world.get::<Y>(e).is_some());
assert!(world.get::<X>(e).is_some());
assert!(world.get::<Z>(e).is_some());
{
let mut commands = Commands::new(&mut queue, &world);
commands.entity(e).remove_with_requires::<X>();
}
queue.apply(&mut world);
assert!(world.get::<Y>(e).is_none());
assert!(world.get::<X>(e).is_none());
assert!(world.get::<Z>(e).is_some());
}
fn is_send<T: Send>() {}
fn is_sync<T: Sync>() {}

View file

@ -1558,6 +1558,20 @@ 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_requires<T: Bundle>(&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.register_contributed_bundle_info::<T>(components, storages);
// SAFETY: the dynamic `BundleInfo` is initialized above
self.location = unsafe { self.remove_bundle(bundle_id) };
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.