Skip to content

Commit f654350

Browse files
Add BundleRemover (#18521)
# Objective It has long been a todo item in the ecs to create a `BundleRemover` alongside the inserter, spawner, etc. This is an uncontroversial first step of #18514. ## Solution Move existing code from complex helper functions to one generalized `BundleRemover`. ## Testing Existing tests.
1 parent bea0a0a commit f654350

File tree

2 files changed

+423
-379
lines changed

2 files changed

+423
-379
lines changed

crates/bevy_ecs/src/bundle.rs

+272-1
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,10 @@ use crate::{
2020
query::DebugCheckedUnwrap,
2121
relationship::RelationshipHookMode,
2222
storage::{SparseSetIndex, SparseSets, Storages, Table, TableRow},
23-
world::{unsafe_world_cell::UnsafeWorldCell, EntityWorldMut, ON_ADD, ON_INSERT, ON_REPLACE},
23+
world::{
24+
unsafe_world_cell::UnsafeWorldCell, EntityWorldMut, ON_ADD, ON_INSERT, ON_REMOVE,
25+
ON_REPLACE,
26+
},
2427
};
2528
use alloc::{boxed::Box, vec, vec::Vec};
2629
use bevy_platform::collections::{HashMap, HashSet};
@@ -1371,6 +1374,274 @@ impl<'w> BundleInserter<'w> {
13711374
}
13721375
}
13731376

1377+
// SAFETY: We have exclusive world access so our pointers can't be invalidated externally
1378+
pub(crate) struct BundleRemover<'w> {
1379+
world: UnsafeWorldCell<'w>,
1380+
bundle_info: ConstNonNull<BundleInfo>,
1381+
old_and_new_table: Option<(NonNull<Table>, NonNull<Table>)>,
1382+
old_archetype: NonNull<Archetype>,
1383+
new_archetype: NonNull<Archetype>,
1384+
}
1385+
1386+
impl<'w> BundleRemover<'w> {
1387+
/// Creates a new [`BundleRemover`], if such a remover would do anything.
1388+
///
1389+
/// If `require_all` is true, the [`BundleRemover`] is only created if the entire bundle is present on the archetype.
1390+
///
1391+
/// # Safety
1392+
/// Caller must ensure that `archetype_id` is valid
1393+
#[inline]
1394+
pub(crate) unsafe fn new<T: Bundle>(
1395+
world: &'w mut World,
1396+
archetype_id: ArchetypeId,
1397+
require_all: bool,
1398+
) -> Option<Self> {
1399+
// SAFETY: These come from the same world. `world.components_registrator` can't be used since we borrow other fields too.
1400+
let mut registrator =
1401+
unsafe { ComponentsRegistrator::new(&mut world.components, &mut world.component_ids) };
1402+
let bundle_id = world
1403+
.bundles
1404+
.register_info::<T>(&mut registrator, &mut world.storages);
1405+
// SAFETY: we initialized this bundle_id in `init_info`, and caller ensures archetype is valid.
1406+
unsafe { Self::new_with_id(world, archetype_id, bundle_id, require_all) }
1407+
}
1408+
1409+
/// Creates a new [`BundleRemover`], if such a remover would do anything.
1410+
///
1411+
/// If `require_all` is true, the [`BundleRemover`] is only created if the entire bundle is present on the archetype.
1412+
///
1413+
/// # Safety
1414+
/// Caller must ensure that `bundle_id` exists in `world.bundles` and `archetype_id` is valid.
1415+
#[inline]
1416+
pub(crate) unsafe fn new_with_id(
1417+
world: &'w mut World,
1418+
archetype_id: ArchetypeId,
1419+
bundle_id: BundleId,
1420+
require_all: bool,
1421+
) -> Option<Self> {
1422+
let bundle_info = world.bundles.get_unchecked(bundle_id);
1423+
// SAFETY: Caller ensures archetype and bundle ids are correct.
1424+
let new_archetype_id = unsafe {
1425+
bundle_info.remove_bundle_from_archetype(
1426+
&mut world.archetypes,
1427+
&mut world.storages,
1428+
&world.components,
1429+
&world.observers,
1430+
archetype_id,
1431+
!require_all,
1432+
)?
1433+
};
1434+
if new_archetype_id == archetype_id {
1435+
return None;
1436+
}
1437+
let (old_archetype, new_archetype) =
1438+
world.archetypes.get_2_mut(archetype_id, new_archetype_id);
1439+
1440+
let tables = if old_archetype.table_id() == new_archetype.table_id() {
1441+
None
1442+
} else {
1443+
let (old, new) = world
1444+
.storages
1445+
.tables
1446+
.get_2_mut(old_archetype.table_id(), new_archetype.table_id());
1447+
Some((old.into(), new.into()))
1448+
};
1449+
1450+
Some(Self {
1451+
bundle_info: bundle_info.into(),
1452+
new_archetype: new_archetype.into(),
1453+
old_archetype: old_archetype.into(),
1454+
old_and_new_table: tables,
1455+
world: world.as_unsafe_world_cell(),
1456+
})
1457+
}
1458+
1459+
/// This can be passed to [`remove`](Self::remove) as the `pre_remove` function if you don't want to do anything before removing.
1460+
pub fn empty_pre_remove(
1461+
_: &mut SparseSets,
1462+
_: Option<&mut Table>,
1463+
_: &Components,
1464+
_: &[ComponentId],
1465+
) -> (bool, ()) {
1466+
(true, ())
1467+
}
1468+
1469+
/// Performs the removal.
1470+
///
1471+
/// `pre_remove` should return a bool for if the components still need to be dropped.
1472+
///
1473+
/// # Safety
1474+
/// The `location` must have the same archetype as the remover.
1475+
#[inline]
1476+
pub(crate) unsafe fn remove<T: 'static>(
1477+
&mut self,
1478+
entity: Entity,
1479+
location: EntityLocation,
1480+
caller: MaybeLocation,
1481+
pre_remove: impl FnOnce(
1482+
&mut SparseSets,
1483+
Option<&mut Table>,
1484+
&Components,
1485+
&[ComponentId],
1486+
) -> (bool, T),
1487+
) -> (EntityLocation, T) {
1488+
// Hooks
1489+
// SAFETY: all bundle components exist in World
1490+
unsafe {
1491+
// SAFETY: We only keep access to archetype/bundle data.
1492+
let mut deferred_world = self.world.into_deferred();
1493+
let bundle_components_in_archetype = || {
1494+
self.bundle_info
1495+
.as_ref()
1496+
.iter_explicit_components()
1497+
.filter(|component_id| self.old_archetype.as_ref().contains(*component_id))
1498+
};
1499+
if self.old_archetype.as_ref().has_replace_observer() {
1500+
deferred_world.trigger_observers(
1501+
ON_REPLACE,
1502+
entity,
1503+
bundle_components_in_archetype(),
1504+
caller,
1505+
);
1506+
}
1507+
deferred_world.trigger_on_replace(
1508+
self.old_archetype.as_ref(),
1509+
entity,
1510+
bundle_components_in_archetype(),
1511+
caller,
1512+
RelationshipHookMode::Run,
1513+
);
1514+
if self.old_archetype.as_ref().has_remove_observer() {
1515+
deferred_world.trigger_observers(
1516+
ON_REMOVE,
1517+
entity,
1518+
bundle_components_in_archetype(),
1519+
caller,
1520+
);
1521+
}
1522+
deferred_world.trigger_on_remove(
1523+
self.old_archetype.as_ref(),
1524+
entity,
1525+
bundle_components_in_archetype(),
1526+
caller,
1527+
);
1528+
}
1529+
1530+
// SAFETY: We still have the cell, so this is unique, it doesn't conflict with other references, and we drop it shortly.
1531+
let world = unsafe { self.world.world_mut() };
1532+
1533+
let (needs_drop, pre_remove_result) = pre_remove(
1534+
&mut world.storages.sparse_sets,
1535+
self.old_and_new_table
1536+
.as_ref()
1537+
// SAFETY: There is no conflicting access for this scope.
1538+
.map(|(old, _)| unsafe { &mut *old.as_ptr() }),
1539+
&world.components,
1540+
self.bundle_info.as_ref().explicit_components(),
1541+
);
1542+
1543+
// Handle sparse set removes
1544+
for component_id in self.bundle_info.as_ref().iter_explicit_components() {
1545+
if self.old_archetype.as_ref().contains(component_id) {
1546+
world.removed_components.send(component_id, entity);
1547+
1548+
// Make sure to drop components stored in sparse sets.
1549+
// Dense components are dropped later in `move_to_and_drop_missing_unchecked`.
1550+
if let Some(StorageType::SparseSet) =
1551+
self.old_archetype.as_ref().get_storage_type(component_id)
1552+
{
1553+
world
1554+
.storages
1555+
.sparse_sets
1556+
.get_mut(component_id)
1557+
// Set exists because the component existed on the entity
1558+
.unwrap()
1559+
// If it was already forgotten, it would not be in the set.
1560+
.remove(entity);
1561+
}
1562+
}
1563+
}
1564+
1565+
// Handle archetype change
1566+
let remove_result = self
1567+
.old_archetype
1568+
.as_mut()
1569+
.swap_remove(location.archetype_row);
1570+
// if an entity was moved into this entity's archetype row, update its archetype row
1571+
if let Some(swapped_entity) = remove_result.swapped_entity {
1572+
let swapped_location = world.entities.get(swapped_entity).unwrap();
1573+
1574+
world.entities.set(
1575+
swapped_entity.index(),
1576+
EntityLocation {
1577+
archetype_id: swapped_location.archetype_id,
1578+
archetype_row: location.archetype_row,
1579+
table_id: swapped_location.table_id,
1580+
table_row: swapped_location.table_row,
1581+
},
1582+
);
1583+
}
1584+
1585+
// Handle table change
1586+
let new_location = if let Some((mut old_table, mut new_table)) = self.old_and_new_table {
1587+
let move_result = if needs_drop {
1588+
// SAFETY: old_table_row exists
1589+
unsafe {
1590+
old_table
1591+
.as_mut()
1592+
.move_to_and_drop_missing_unchecked(location.table_row, new_table.as_mut())
1593+
}
1594+
} else {
1595+
// SAFETY: old_table_row exists
1596+
unsafe {
1597+
old_table.as_mut().move_to_and_forget_missing_unchecked(
1598+
location.table_row,
1599+
new_table.as_mut(),
1600+
)
1601+
}
1602+
};
1603+
1604+
// SAFETY: move_result.new_row is a valid position in new_archetype's table
1605+
let new_location = unsafe {
1606+
self.new_archetype
1607+
.as_mut()
1608+
.allocate(entity, move_result.new_row)
1609+
};
1610+
1611+
// if an entity was moved into this entity's table row, update its table row
1612+
if let Some(swapped_entity) = move_result.swapped_entity {
1613+
let swapped_location = world.entities.get(swapped_entity).unwrap();
1614+
1615+
world.entities.set(
1616+
swapped_entity.index(),
1617+
EntityLocation {
1618+
archetype_id: swapped_location.archetype_id,
1619+
archetype_row: swapped_location.archetype_row,
1620+
table_id: swapped_location.table_id,
1621+
table_row: location.table_row,
1622+
},
1623+
);
1624+
world.archetypes[swapped_location.archetype_id]
1625+
.set_entity_table_row(swapped_location.archetype_row, location.table_row);
1626+
}
1627+
1628+
new_location
1629+
} else {
1630+
// The tables are the same
1631+
self.new_archetype
1632+
.as_mut()
1633+
.allocate(entity, location.table_row)
1634+
};
1635+
1636+
// SAFETY: The entity is valid and has been moved to the new location already.
1637+
unsafe {
1638+
world.entities.set(entity.index(), new_location);
1639+
}
1640+
1641+
(new_location, pre_remove_result)
1642+
}
1643+
}
1644+
13741645
// SAFETY: We have exclusive world access so our pointers can't be invalidated externally
13751646
pub(crate) struct BundleSpawner<'w> {
13761647
world: UnsafeWorldCell<'w>,

0 commit comments

Comments
 (0)