EntityMut: rename remove_intersection to remove and remove to take (bevyengine#7810)

# Objective

- A more intuitive distinction between the two. `remove_intersection` is verbose and unclear.
- `EntityMut::remove` and `Commands::remove` should match.


## Solution

- What the title says.

---

## Migration Guide

Before
```rust
fn clear_children(parent: Entity, world: &mut World) {
    if let Some(children) = world.entity_mut(parent).remove::<Children>() {
        for &child in &children.0 {
            world.entity_mut(child).remove_intersection::<Parent>();
        }
    }
}
```

After
```rust
fn clear_children(parent: Entity, world: &mut World) {
    if let Some(children) = world.entity_mut(parent).take::<Children>() {
        for &child in &children.0 {
            world.entity_mut(child).remove::<Parent>();
        }
    }
}
```

Author: maniwani

crates/bevy_ecs/src/archetype.rs

@@ -150,7 +150,7 @@ impl BundleComponentStatus for SpawnBundleStatus {
 pub struct Edges {
     add_bundle: SparseArray<BundleId, AddBundle>,
     remove_bundle: SparseArray<BundleId, Option<ArchetypeId>>,
-    remove_bundle_intersection: SparseArray<BundleId, Option<ArchetypeId>>,
+    take_bundle: SparseArray<BundleId, Option<ArchetypeId>>,
 }
 
 impl Edges {
@@ -222,32 +222,28 @@ impl Edges {
     }
 
     /// Checks the cache for the target archetype when removing a bundle to the
-    /// source archetype. For more information, see [`EntityMut::remove_intersection`].
+    /// source archetype. For more information, see [`EntityMut::remove`].
     ///
     /// If this returns `None`, it means there has not been a transition from
     /// the source archetype via the provided bundle.
     ///
-    /// [`EntityMut::remove_intersection`]: crate::world::EntityMut::remove_intersection
+    /// [`EntityMut::remove`]: crate::world::EntityMut::remove
     #[inline]
-    pub fn get_remove_bundle_intersection(
-        &self,
-        bundle_id: BundleId,
-    ) -> Option<Option<ArchetypeId>> {
-        self.remove_bundle_intersection.get(bundle_id).cloned()
+    pub fn get_take_bundle(&self, bundle_id: BundleId) -> Option<Option<ArchetypeId>> {
+        self.take_bundle.get(bundle_id).cloned()
     }
 
     /// Caches the target archetype when removing a bundle to the source archetype.
-    /// For more information, see [`EntityMut::remove_intersection`].
+    /// For more information, see [`EntityMut::take`].
     ///
-    /// [`EntityMut::remove_intersection`]: crate::world::EntityMut::remove_intersection
+    /// [`EntityMut::take`]: crate::world::EntityMut::take
     #[inline]
-    pub(crate) fn insert_remove_bundle_intersection(
+    pub(crate) fn insert_take_bundle(
         &mut self,
         bundle_id: BundleId,
         archetype_id: Option<ArchetypeId>,
     ) {
-        self.remove_bundle_intersection
-            .insert(bundle_id, archetype_id);
+        self.take_bundle.insert(bundle_id, archetype_id);
     }
 }
 

crates/bevy_ecs/src/lib.rs

@@ -191,7 +191,7 @@ mod tests {
         assert_eq!(world.get::<SparseStored>(e2).unwrap().0, 42);
 
         assert_eq!(
-            world.entity_mut(e1).remove::<FooBundle>().unwrap(),
+            world.entity_mut(e1).take::<FooBundle>().unwrap(),
             FooBundle {
                 x: TableStored("xyz"),
                 y: SparseStored(123),
@@ -240,7 +240,7 @@ mod tests {
         assert_eq!(world.get::<A>(e3).unwrap().0, 1);
         assert_eq!(world.get::<B>(e3).unwrap().0, 2);
         assert_eq!(
-            world.entity_mut(e3).remove::<NestedBundle>().unwrap(),
+            world.entity_mut(e3).take::<NestedBundle>().unwrap(),
             NestedBundle {
                 a: A(1),
                 foo: FooBundle {
@@ -283,7 +283,7 @@ mod tests {
         assert_eq!(world.get::<Ignored>(e4), None);
 
         assert_eq!(
-            world.entity_mut(e4).remove::<BundleWithIgnored>().unwrap(),
+            world.entity_mut(e4).take::<BundleWithIgnored>().unwrap(),
             BundleWithIgnored {
                 c: C,
                 ignored: Ignored,
@@ -596,7 +596,7 @@ mod tests {
             &[(e1, A(1), B(3)), (e2, A(2), B(4))]
         );
 
-        assert_eq!(world.entity_mut(e1).remove::<A>(), Some(A(1)));
+        assert_eq!(world.entity_mut(e1).take::<A>(), Some(A(1)));
         assert_eq!(
             world
                 .query::<(Entity, &A, &B)>()
@@ -656,7 +656,7 @@ mod tests {
         }
 
         for (i, entity) in entities.iter().cloned().enumerate() {
-            assert_eq!(world.entity_mut(entity).remove::<A>(), Some(A(i)));
+            assert_eq!(world.entity_mut(entity).take::<A>(), Some(A(i)));
         }
     }
 
@@ -675,7 +675,7 @@ mod tests {
 
         for (i, entity) in entities.iter().cloned().enumerate() {
             assert_eq!(
-                world.entity_mut(entity).remove::<SparseStored>(),
+                world.entity_mut(entity).take::<SparseStored>(),
                 Some(SparseStored(i as u32))
             );
         }
@@ -685,7 +685,7 @@ mod tests {
     fn remove_missing() {
         let mut world = World::new();
         let e = world.spawn((TableStored("abc"), A(123))).id();
-        assert!(world.entity_mut(e).remove::<B>().is_none());
+        assert!(world.entity_mut(e).take::<B>().is_none());
     }
 
     #[test]
@@ -1187,7 +1187,7 @@ mod tests {
     }
 
     #[test]
-    fn remove_intersection() {
+    fn remove() {
         let mut world = World::default();
         let e1 = world.spawn((A(1), B(1), TableStored("a"))).id();
 
@@ -1201,7 +1201,7 @@ mod tests {
             "C is not in the entity, so it should not exist"
         );
 
-        e.remove_intersection::<(A, B, C)>();
+        e.remove::<(A, B, C)>();
         assert_eq!(
             e.get::<TableStored>(),
             Some(&TableStored("a")),
@@ -1225,7 +1225,7 @@ mod tests {
     }
 
     #[test]
-    fn remove() {
+    fn take() {
         let mut world = World::default();
         world.spawn((A(1), B(1), TableStored("1")));
         let e2 = world.spawn((A(2), B(2), TableStored("2"))).id();
@@ -1238,7 +1238,7 @@ mod tests {
             .collect::<Vec<_>>();
         assert_eq!(results, vec![(1, "1"), (2, "2"), (3, "3"),]);
 
-        let removed_bundle = world.entity_mut(e2).remove::<(B, TableStored)>().unwrap();
+        let removed_bundle = world.entity_mut(e2).take::<(B, TableStored)>().unwrap();
         assert_eq!(removed_bundle, (B(2), TableStored("2")));
 
         let results = query

crates/bevy_ecs/src/reflect.rs

@@ -99,10 +99,6 @@ impl ReflectComponent {
     }
 
     /// Removes this [`Component`] type from the entity. Does nothing if it doesn't exist.
-    ///
-    /// # Panics
-    ///
-    /// Panics if there is no [`Component`] of the given type.
     pub fn remove(&self, entity: &mut EntityMut) {
         (self.0.remove)(entity);
     }

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

@@ -951,9 +951,7 @@ where
 {
     fn write(self, world: &mut World) {
         if let Some(mut entity_mut) = world.get_entity_mut(self.entity) {
-            // remove intersection to gracefully handle components that were removed before running
-            // this command
-            entity_mut.remove_intersection::<T>();
+            entity_mut.remove::<T>();
         }
     }
 }

crates/bevy_ecs/src/world/entity_ref.rs

@@ -279,11 +279,13 @@ impl<'w> EntityMut<'w> {
         self
     }
 
-    // TODO: move to BundleInfo
-    /// Removes a [`Bundle`] of components from the entity and returns the bundle.
+    /// Removes all components in the [`Bundle`] from the entity and returns their previous values.
     ///
-    /// Returns `None` if the entity does not contain the bundle.
-    pub fn remove<T: Bundle>(&mut self) -> Option<T> {
+    /// **Note:** If the entity does not have every component in the bundle, this method will not
+    /// remove any of them.
+    // TODO: BundleRemover?
+    #[must_use]
+    pub fn take<T: Bundle>(&mut self) -> Option<T> {
         let archetypes = &mut self.world.archetypes;
         let storages = &mut self.world.storages;
         let components = &mut self.world.components;
@@ -408,9 +410,9 @@ impl<'w> EntityMut<'w> {
         entities.set(entity.index(), new_location);
     }
 
-    // TODO: move to BundleInfo
-    /// Remove any components in the bundle that the entity has.
-    pub fn remove_intersection<T: Bundle>(&mut self) {
+    /// Removes any components in the [`Bundle`] from the entity.
+    // TODO: BundleRemover?
+    pub fn remove<T: Bundle>(&mut self) -> &mut Self {
         let archetypes = &mut self.world.archetypes;
         let storages = &mut self.world.storages;
         let components = &mut self.world.components;
@@ -435,7 +437,7 @@ impl<'w> EntityMut<'w> {
         };
 
         if new_archetype_id == old_location.archetype_id {
-            return;
+            return self;
         }
 
         let old_archetype = &mut archetypes[old_location.archetype_id];
@@ -469,6 +471,8 @@ impl<'w> EntityMut<'w> {
                 new_archetype_id,
             );
         }
+
+        self
     }
 
     pub fn despawn(self) {
@@ -647,11 +651,9 @@ unsafe fn remove_bundle_from_archetype(
     let remove_bundle_result = {
         let current_archetype = &mut archetypes[archetype_id];
         if intersection {
-            current_archetype
-                .edges()
-                .get_remove_bundle_intersection(bundle_info.id)
-        } else {
             current_archetype.edges().get_remove_bundle(bundle_info.id)
+        } else {
+            current_archetype.edges().get_take_bundle(bundle_info.id)
         }
     };
     let result = if let Some(result) = remove_bundle_result {
@@ -679,7 +681,7 @@ unsafe fn remove_bundle_from_archetype(
                     // graph
                     current_archetype
                         .edges_mut()
-                        .insert_remove_bundle(bundle_info.id, None);
+                        .insert_take_bundle(bundle_info.id, None);
                     return None;
                 }
             }
@@ -718,11 +720,11 @@ unsafe fn remove_bundle_from_archetype(
     if intersection {
         current_archetype
             .edges_mut()
-            .insert_remove_bundle_intersection(bundle_info.id, result);
+            .insert_remove_bundle(bundle_info.id, result);
     } else {
         current_archetype
             .edges_mut()
-            .insert_remove_bundle(bundle_info.id, result);
+            .insert_take_bundle(bundle_info.id, result);
     }
     result
 }

crates/bevy_ecs_compile_fail_tests/tests/ui/entity_ref_mut_lifetime_safety.rs

@@ -14,15 +14,15 @@ fn main() {
 
     {
         let gotten: &A = e_mut.get::<A>().unwrap();
-        let gotten2: A = e_mut.remove::<A>().unwrap();
+        let gotten2: A = e_mut.take::<A>().unwrap();
         assert_eq!(gotten, &gotten2); // oops UB
     }
 
     e_mut.insert(A(Box::new(12_usize)));
 
     {
         let mut gotten: Mut<A> = e_mut.get_mut::<A>().unwrap();
-        let mut gotten2: A = e_mut.remove::<A>().unwrap();
+        let mut gotten2: A = e_mut.take::<A>().unwrap();
         assert_eq!(&mut *gotten, &mut gotten2); // oops UB
     }
 

crates/bevy_ecs_compile_fail_tests/tests/ui/entity_ref_mut_lifetime_safety.stderr

@@ -3,8 +3,8 @@ error[E0502]: cannot borrow `e_mut` as mutable because it is also borrowed as im
    |
 16 |         let gotten: &A = e_mut.get::<A>().unwrap();
    |                          ---------------- immutable borrow occurs here
-17 |         let gotten2: A = e_mut.remove::<A>().unwrap();
-   |                          ^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here
+17 |         let gotten2: A = e_mut.take::<A>().unwrap();
+   |                          ^^^^^^^^^^^^^^^^^ mutable borrow occurs here
 18 |         assert_eq!(gotten, &gotten2); // oops UB
    |         ---------------------------- immutable borrow later used here
 
@@ -13,8 +13,8 @@ error[E0499]: cannot borrow `e_mut` as mutable more than once at a time
    |
 24 |         let mut gotten: Mut<A> = e_mut.get_mut::<A>().unwrap();
    |                                  -------------------- first mutable borrow occurs here
-25 |         let mut gotten2: A = e_mut.remove::<A>().unwrap();
-   |                              ^^^^^^^^^^^^^^^^^^^ second mutable borrow occurs here
+25 |         let mut gotten2: A = e_mut.take::<A>().unwrap();
+   |                              ^^^^^^^^^^^^^^^^^ second mutable borrow occurs here
 26 |         assert_eq!(&mut *gotten, &mut gotten2); // oops UB
    |                          ------ first borrow later used here
 

crates/bevy_hierarchy/src/child_builder.rs

@@ -141,7 +141,7 @@ fn remove_children(parent: Entity, children: &[Entity], world: &mut World) {
 }
 
 fn clear_children(parent: Entity, world: &mut World) {
-    if let Some(children) = world.entity_mut(parent).remove::<Children>() {
+    if let Some(children) = world.entity_mut(parent).take::<Children>() {
         for &child in &children.0 {
             world.entity_mut(child).remove::<Parent>();
         }
@@ -532,7 +532,7 @@ impl<'w> BuildWorldChildren for EntityMut<'w> {
 
     fn remove_parent(&mut self) -> &mut Self {
         let child = self.id();
-        if let Some(parent) = self.remove::<Parent>().map(|p| p.get()) {
+        if let Some(parent) = self.take::<Parent>().map(|p| p.get()) {
             self.world_scope(|world| {
                 remove_from_children(world, parent, child);
                 push_events(world, [HierarchyEvent::ChildRemoved { child, parent }]);