Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misc. docs and renames for niche ECS internals #16786

Merged
merged 4 commits into from
Dec 12, 2024
Merged

Misc. docs and renames for niche ECS internals #16786

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f6c254a
look here for `remove_bundle_from_archetype` diff
JaySpruce Dec 12, 2024
c1c2981
remove function left for diff
JaySpruce Dec 12, 2024
d5ac5b3
`InsertBundleResult` -> `ArchetypeMoveType`
JaySpruce Dec 12, 2024
43d27ca
old docs fix and singular CI semicolon
JaySpruce Dec 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 51 additions & 46 deletions crates/bevy_ecs/src/archetype.rs
View file
Open in desktop
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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: it might be better to name this and other methods ArchetypeAfterInsertBundle and *_insert_bundle() to match the insert_bundle ordering.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went back and forth on this, but I stuck with bundle_insert because I thought insert_bundle might sound too much like an "action", like get_archetype_after_insert_bundle would actually be doing the insert itself. Not married to it though

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really have too strong feelings here either and I don't think it's worth thinking about too hard. The new names are better, so lets just ship it.

/// 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
Loading