Skip to content

Commit

Permalink
Misc. docs and renames for niche ECS internals (#16786)
Browse files Browse the repository at this point in the history
## Objective

Some structs and methods in the ECS internals have names that don't
describe their purpose very well, and sometimes don't have docs either.

Also, the function `remove_bundle_from_archetype` is a counterpart to
`BundleInfo::add_bundle_to_archetype`, but isn't a method and is in a
different file.

## Solution

- Renamed the following structs and added docs:

| Before               | After                        |
|----------------------|------------------------------|
| `AddBundle`          | `ArchetypeAfterBundleInsert` |
| `InsertBundleResult` | `ArchetypeMoveType`          |

- Renamed the following methods:

| Before | After |

|---------------------------------------|----------------------------------------------|
| `Edges::get_add_bundle` | `Edges::get_archetype_after_bundle_insert` |
| `Edges::insert_add_bundle` |
`Edges::cache_archetype_after_bundle_insert` |
| `Edges::get_remove_bundle` |
`Edges::get_archetype_after_bundle_remove` |
| `Edges::insert_remove_bundle` |
`Edges::cache_archetype_after_bundle_remove` |
| `Edges::get_take_bundle` | `Edges::get_archetype_after_bundle_take` |
| `Edges::insert_take_bundle` |
`Edges::cache_archetype_after_bundle_take` |

- Moved `remove_bundle_from_archetype` from `world/entity_ref.rs` to
`BundleInfo`. I left the function in entity_ref in the first commit for
comparison, look there for the diff of comments and whatnot.
- Tidied up docs:
  - General grammar and spacing.
  - Made the usage of "insert" and "add" more consistent.
  - Removed references to information that isn't there.
- Renamed `BundleInfo::add_bundle_to_archetype` to
`BundleInfo::insert_bundle_into_archetype` for consistency.
  • Loading branch information
JaySpruce authored Dec 12, 2024
1 parent ced6159 commit d132239
Show file tree
Hide file tree
Showing 3 changed files with 311 additions and 271 deletions.
97 changes: 51 additions & 46 deletions crates/bevy_ecs/src/archetype.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,19 +109,20 @@ impl ArchetypeId {
}
}

/// Used in [`AddBundle`] to track whether components in the bundle are newly
/// Used in [`ArchetypeAfterBundleInsert`] to track whether components in the bundle are newly
/// added or already existed in the entity's archetype.
#[derive(Copy, Clone, Eq, PartialEq)]
pub(crate) enum ComponentStatus {
Added,
Existing,
}

pub(crate) struct AddBundle {
/// The target archetype after the bundle is added to the source archetype
/// Used in [`Edges`] to cache the result of inserting a bundle into the source archetype.
pub(crate) struct ArchetypeAfterBundleInsert {
/// The target archetype after the bundle is inserted into the source archetype.
pub archetype_id: ArchetypeId,
/// For each component iterated in the same order as the source [`Bundle`](crate::bundle::Bundle),
/// indicate if the component is newly added to the target archetype or if it already existed
/// indicate if the component is newly added to the target archetype or if it already existed.
pub bundle_status: Vec<ComponentStatus>,
/// The set of additional required components that must be initialized immediately when adding this Bundle.
///
Expand All @@ -134,7 +135,7 @@ pub(crate) struct AddBundle {
pub existing: Vec<ComponentId>,
}

impl AddBundle {
impl ArchetypeAfterBundleInsert {
pub(crate) fn iter_inserted(&self) -> impl Iterator<Item = ComponentId> + Clone + '_ {
self.added.iter().chain(self.existing.iter()).copied()
}
Expand All @@ -149,18 +150,18 @@ impl AddBundle {
}

/// This trait is used to report the status of [`Bundle`](crate::bundle::Bundle) components
/// being added to a given entity, relative to that entity's original archetype.
/// being inserted into a given entity, relative to that entity's original archetype.
/// See [`crate::bundle::BundleInfo::write_components`] for more info.
pub(crate) trait BundleComponentStatus {
/// Returns the Bundle's component status for the given "bundle index"
/// Returns the Bundle's component status for the given "bundle index".
///
/// # Safety
/// Callers must ensure that index is always a valid bundle index for the
/// Bundle associated with this [`BundleComponentStatus`]
unsafe fn get_status(&self, index: usize) -> ComponentStatus;
}

impl BundleComponentStatus for AddBundle {
impl BundleComponentStatus for ArchetypeAfterBundleInsert {
#[inline]
unsafe fn get_status(&self, index: usize) -> ComponentStatus {
// SAFETY: caller has ensured index is a valid bundle index for this bundle
Expand All @@ -173,7 +174,7 @@ pub(crate) struct SpawnBundleStatus;
impl BundleComponentStatus for SpawnBundleStatus {
#[inline]
unsafe fn get_status(&self, _index: usize) -> ComponentStatus {
// Components added during a spawn call are always treated as added
// Components inserted during a spawn call are always treated as added.
ComponentStatus::Added
}
}
Expand All @@ -194,37 +195,36 @@ impl BundleComponentStatus for SpawnBundleStatus {
/// [`World`]: crate::world::World
#[derive(Default)]
pub struct Edges {
add_bundle: SparseArray<BundleId, AddBundle>,
insert_bundle: SparseArray<BundleId, ArchetypeAfterBundleInsert>,
remove_bundle: SparseArray<BundleId, Option<ArchetypeId>>,
take_bundle: SparseArray<BundleId, Option<ArchetypeId>>,
}

impl Edges {
/// Checks the cache for the target archetype when adding a bundle to the
/// source archetype. For more information, see [`EntityWorldMut::insert`].
/// Checks the cache for the target archetype when inserting a bundle into the
/// source archetype.
///
/// If this returns `None`, it means there has not been a transition from
/// the source archetype via the provided bundle.
///
/// [`EntityWorldMut::insert`]: crate::world::EntityWorldMut::insert
#[inline]
pub fn get_add_bundle(&self, bundle_id: BundleId) -> Option<ArchetypeId> {
self.get_add_bundle_internal(bundle_id)
pub fn get_archetype_after_bundle_insert(&self, bundle_id: BundleId) -> Option<ArchetypeId> {
self.get_archetype_after_bundle_insert_internal(bundle_id)
.map(|bundle| bundle.archetype_id)
}

/// Internal version of `get_add_bundle` that fetches the full `AddBundle`.
/// Internal version of `get_archetype_after_bundle_insert` that
/// fetches the full `ArchetypeAfterBundleInsert`.
#[inline]
pub(crate) fn get_add_bundle_internal(&self, bundle_id: BundleId) -> Option<&AddBundle> {
self.add_bundle.get(bundle_id)
pub(crate) fn get_archetype_after_bundle_insert_internal(
&self,
bundle_id: BundleId,
) -> Option<&ArchetypeAfterBundleInsert> {
self.insert_bundle.get(bundle_id)
}

/// Caches the target archetype when adding a bundle to the source archetype.
/// For more information, see [`EntityWorldMut::insert`].
///
/// [`EntityWorldMut::insert`]: crate::world::EntityWorldMut::insert
/// Caches the target archetype when inserting a bundle into the source archetype.
#[inline]
pub(crate) fn insert_add_bundle(
pub(crate) fn cache_archetype_after_bundle_insert(
&mut self,
bundle_id: BundleId,
archetype_id: ArchetypeId,
Expand All @@ -233,9 +233,9 @@ impl Edges {
added: Vec<ComponentId>,
existing: Vec<ComponentId>,
) {
self.add_bundle.insert(
self.insert_bundle.insert(
bundle_id,
AddBundle {
ArchetypeAfterBundleInsert {
archetype_id,
bundle_status,
required_components,
Expand All @@ -245,52 +245,57 @@ impl Edges {
);
}

/// Checks the cache for the target archetype when removing a bundle to the
/// source archetype. For more information, see [`EntityWorldMut::remove`].
/// Checks the cache for the target archetype when removing a bundle from the
/// source archetype.
///
/// If this returns `None`, it means there has not been a transition from
/// the source archetype via the provided bundle.
///
/// If this returns `Some(None)`, it means that the bundle cannot be removed
/// from the source archetype.
///
/// [`EntityWorldMut::remove`]: crate::world::EntityWorldMut::remove
#[inline]
pub fn get_remove_bundle(&self, bundle_id: BundleId) -> Option<Option<ArchetypeId>> {
pub fn get_archetype_after_bundle_remove(
&self,
bundle_id: BundleId,
) -> Option<Option<ArchetypeId>> {
self.remove_bundle.get(bundle_id).cloned()
}

/// Caches the target archetype when removing a bundle to the source archetype.
/// For more information, see [`EntityWorldMut::remove`].
///
/// [`EntityWorldMut::remove`]: crate::world::EntityWorldMut::remove
/// Caches the target archetype when removing a bundle from the source archetype.
#[inline]
pub(crate) fn insert_remove_bundle(
pub(crate) fn cache_archetype_after_bundle_remove(
&mut self,
bundle_id: BundleId,
archetype_id: Option<ArchetypeId>,
) {
self.remove_bundle.insert(bundle_id, archetype_id);
}

/// Checks the cache for the target archetype when removing a bundle to the
/// source archetype. For more information, see [`EntityWorldMut::remove`].
/// Checks the cache for the target archetype when taking a bundle from the
/// source archetype.
///
/// Unlike `remove`, `take` will only succeed if the source archetype
/// contains all of the components in the bundle.
///
/// If this returns `None`, it means there has not been a transition from
/// the source archetype via the provided bundle.
///
/// [`EntityWorldMut::remove`]: crate::world::EntityWorldMut::remove
/// If this returns `Some(None)`, it means that the bundle cannot be taken
/// from the source archetype.
#[inline]
pub fn get_take_bundle(&self, bundle_id: BundleId) -> Option<Option<ArchetypeId>> {
pub fn get_archetype_after_bundle_take(
&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 [`EntityWorldMut::take`].
/// Caches the target archetype when taking a bundle from the source archetype.
///
/// [`EntityWorldMut::take`]: crate::world::EntityWorldMut::take
/// Unlike `remove`, `take` will only succeed if the source archetype
/// contains all of the components in the bundle.
#[inline]
pub(crate) fn insert_take_bundle(
pub(crate) fn cache_archetype_after_bundle_take(
&mut self,
bundle_id: BundleId,
archetype_id: Option<ArchetypeId>,
Expand Down Expand Up @@ -577,11 +582,11 @@ impl Archetype {
self.entities.reserve(additional);
}

/// Removes the entity at `index` by swapping it out. Returns the table row the entity is stored
/// Removes the entity at `row` by swapping it out. Returns the table row the entity is stored
/// in.
///
/// # Panics
/// This function will panic if `index >= self.len()`
/// This function will panic if `row >= self.entities.len()`
#[inline]
pub(crate) fn swap_remove(&mut self, row: ArchetypeRow) -> ArchetypeSwapRemoveResult {
let is_last = row.index() == self.entities.len() - 1;
Expand Down
Loading

0 comments on commit d132239

Please sign in to comment.