Skip to content

Commit

Permalink
Add method to remove component and all required components for remove…
Browse files Browse the repository at this point in the history
…d component (bevyengine#15026)

## Objective
The new Required Components feature (bevyengine#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
bevyengine#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]>
  • Loading branch information
3 people authored and ItsDoot committed Oct 4, 2024
1 parent 8add4cd commit a9785ae
Show file tree
Hide file tree
Showing 4 changed files with 249 additions and 0 deletions.
32 changes: 32 additions & 0 deletions crates/bevy_ecs/src/bundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>>,
Expand Down Expand Up @@ -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 {
Expand Down
132 changes: 132 additions & 0 deletions crates/bevy_ecs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
71 changes: 71 additions & 0 deletions crates/bevy_ecs/src/system/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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| {
Expand Down Expand Up @@ -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>() {}

Expand Down
14 changes: 14 additions & 0 deletions crates/bevy_ecs/src/world/entity_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit a9785ae

Please sign in to comment.