diff --git a/crates/bevy_ecs/src/entity/map_entities.rs b/crates/bevy_ecs/src/entity/map_entities.rs index 9b8844e6e0ebd..7994f9683dcc2 100644 --- a/crates/bevy_ecs/src/entity/map_entities.rs +++ b/crates/bevy_ecs/src/entity/map_entities.rs @@ -1,8 +1,6 @@ -use crate::{entity::Entity, world::World}; +use crate::{entity::Entity, identifier::masks::IdentifierMask, world::World}; use bevy_utils::EntityHashMap; -use super::inc_generation_by; - /// Operation to map all contained [`Entity`] fields in a type to new values. /// /// As entity IDs are valid only for the [`World`] they're sourced from, using [`Entity`] @@ -70,10 +68,10 @@ impl<'m> EntityMapper<'m> { } // this new entity reference is specifically designed to never represent any living entity - let new = Entity { - generation: inc_generation_by(self.dead_start.generation, self.generations), - index: self.dead_start.index, - }; + let new = Entity::new( + self.dead_start.index(), + IdentifierMask::inc_masked_high_by(self.dead_start.generation, self.generations), + ); self.generations += 1; self.map.insert(entity, new); @@ -109,7 +107,7 @@ impl<'m> EntityMapper<'m> { // SAFETY: Entities data is kept in a valid state via `EntityMap::world_scope` let entities = unsafe { world.entities_mut() }; assert!(entities.free(self.dead_start).is_some()); - assert!(entities.reserve_generations(self.dead_start.index, self.generations)); + assert!(entities.reserve_generations(self.dead_start.index(), self.generations)); } /// Creates an [`EntityMapper`] from a provided [`World`] and [`EntityHashMap`], then calls the diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index 129f88c516123..513990b41d5da 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -42,6 +42,7 @@ pub use map_entities::*; use crate::{ archetype::{ArchetypeId, ArchetypeRow}, + identifier::{error::IdentifierError, kinds::IdKind, masks::IdentifierMask, Identifier}, storage::{SparseSetIndex, TableId, TableRow}, }; use serde::{Deserialize, Serialize}; @@ -188,13 +189,9 @@ pub(crate) enum AllocAtWithoutReplacement { } impl Entity { - #[cfg(test)] - pub(crate) const fn new(index: u32, generation: u32) -> Result { - if let Some(generation) = NonZeroU32::new(generation) { - Ok(Entity { index, generation }) - } else { - Err("Failed to construct Entity, check that generation > 0") - } + #[inline(always)] + pub(crate) const fn new(index: u32, generation: NonZeroU32) -> Entity { + Self { index, generation } } /// An entity ID with a placeholder value. This may or may not correspond to an actual entity, @@ -245,12 +242,9 @@ impl Entity { /// In general, one should not try to synchronize the ECS by attempting to ensure that /// `Entity` lines up between instances, but instead insert a secondary identifier as /// a component. - #[inline] + #[inline(always)] pub const fn from_raw(index: u32) -> Entity { - Entity { - index, - generation: NonZeroU32::MIN, - } + Self::new(index, NonZeroU32::MIN) } /// Convert to a form convenient for passing outside of rust. @@ -261,7 +255,7 @@ impl Entity { /// No particular structure is guaranteed for the returned bits. #[inline(always)] pub const fn to_bits(self) -> u64 { - (self.generation.get() as u64) << 32 | self.index as u64 + IdentifierMask::pack_into_u64(self.index, self.generation.get()) } /// Reconstruct an `Entity` previously destructured with [`Entity::to_bits`]. @@ -270,7 +264,7 @@ impl Entity { /// /// # Panics /// - /// This method will likely panic if given `u64` values that did not come from [`Entity::to_bits`] + /// This method will likely panic if given `u64` values that did not come from [`Entity::to_bits`]. #[inline] pub const fn from_bits(bits: u64) -> Self { // Construct an Identifier initially to extract the kind from. @@ -288,15 +282,19 @@ impl Entity { /// /// This method is the fallible counterpart to [`Entity::from_bits`]. #[inline(always)] - pub const fn try_from_bits(bits: u64) -> Result { - if let Some(generation) = NonZeroU32::new((bits >> 32) as u32) { - Ok(Self { - generation, - index: bits as u32, - }) - } else { - Err("Invalid generation bits") + pub const fn try_from_bits(bits: u64) -> Result { + if let Ok(id) = Identifier::try_from_bits(bits) { + let kind = id.kind() as u8; + + if kind == (IdKind::Entity as u8) { + return Ok(Self { + index: id.low(), + generation: id.high(), + }); + } } + + Err(IdentifierError::InvalidEntityId(bits)) } /// Return a transiently unique identifier. @@ -314,7 +312,24 @@ impl Entity { /// given index has been reused (index, generation) pairs uniquely identify a given Entity. #[inline] pub const fn generation(self) -> u32 { - self.generation.get() + // Mask so not to expose any flags + IdentifierMask::extract_value_from_high(self.generation.get()) + } +} + +impl TryFrom for Entity { + type Error = IdentifierError; + + #[inline] + fn try_from(value: Identifier) -> Result { + Self::try_from_bits(value.to_bits()) + } +} + +impl From for Identifier { + #[inline] + fn from(value: Entity) -> Self { + Identifier::from_bits(value.to_bits()) } } @@ -340,7 +355,7 @@ impl<'de> Deserialize<'de> for Entity { impl fmt::Debug for Entity { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}v{}", self.index, self.generation) + write!(f, "{}v{}", self.index(), self.generation()) } } @@ -374,16 +389,8 @@ impl<'a> Iterator for ReserveEntitiesIterator<'a> { fn next(&mut self) -> Option { self.index_iter .next() - .map(|&index| Entity { - generation: self.meta[index as usize].generation, - index, - }) - .or_else(|| { - self.index_range.next().map(|index| Entity { - generation: NonZeroU32::MIN, - index, - }) - }) + .map(|&index| Entity::new(index, self.meta[index as usize].generation)) + .or_else(|| self.index_range.next().map(Entity::from_raw)) } fn size_hint(&self) -> (usize, Option) { @@ -518,20 +525,16 @@ impl Entities { if n > 0 { // Allocate from the freelist. let index = self.pending[(n - 1) as usize]; - Entity { - generation: self.meta[index as usize].generation, - index, - } + Entity::new(index, self.meta[index as usize].generation) } else { // Grab a new ID, outside the range of `meta.len()`. `flush()` must // eventually be called to make it valid. // // As `self.free_cursor` goes more and more negative, we return IDs farther // and farther beyond `meta.len()`. - Entity { - generation: NonZeroU32::MIN, - index: u32::try_from(self.meta.len() as IdCursor - n).expect("too many entities"), - } + Entity::from_raw( + u32::try_from(self.meta.len() as IdCursor - n).expect("too many entities"), + ) } } @@ -550,17 +553,11 @@ impl Entities { if let Some(index) = self.pending.pop() { let new_free_cursor = self.pending.len() as IdCursor; *self.free_cursor.get_mut() = new_free_cursor; - Entity { - generation: self.meta[index as usize].generation, - index, - } + Entity::new(index, self.meta[index as usize].generation) } else { let index = u32::try_from(self.meta.len()).expect("too many entities"); self.meta.push(EntityMeta::EMPTY); - Entity { - generation: NonZeroU32::MIN, - index, - } + Entity::from_raw(index) } } @@ -571,15 +568,16 @@ impl Entities { pub fn alloc_at(&mut self, entity: Entity) -> Option { self.verify_flushed(); - let loc = if entity.index as usize >= self.meta.len() { - self.pending.extend((self.meta.len() as u32)..entity.index); + let loc = if entity.index() as usize >= self.meta.len() { + self.pending + .extend((self.meta.len() as u32)..entity.index()); let new_free_cursor = self.pending.len() as IdCursor; *self.free_cursor.get_mut() = new_free_cursor; self.meta - .resize(entity.index as usize + 1, EntityMeta::EMPTY); + .resize(entity.index() as usize + 1, EntityMeta::EMPTY); self.len += 1; None - } else if let Some(index) = self.pending.iter().position(|item| *item == entity.index) { + } else if let Some(index) = self.pending.iter().position(|item| *item == entity.index()) { self.pending.swap_remove(index); let new_free_cursor = self.pending.len() as IdCursor; *self.free_cursor.get_mut() = new_free_cursor; @@ -587,12 +585,12 @@ impl Entities { None } else { Some(mem::replace( - &mut self.meta[entity.index as usize].location, + &mut self.meta[entity.index() as usize].location, EntityMeta::EMPTY.location, )) }; - self.meta[entity.index as usize].generation = entity.generation; + self.meta[entity.index() as usize].generation = entity.generation; loc } @@ -606,22 +604,23 @@ impl Entities { ) -> AllocAtWithoutReplacement { self.verify_flushed(); - let result = if entity.index as usize >= self.meta.len() { - self.pending.extend((self.meta.len() as u32)..entity.index); + let result = if entity.index() as usize >= self.meta.len() { + self.pending + .extend((self.meta.len() as u32)..entity.index()); let new_free_cursor = self.pending.len() as IdCursor; *self.free_cursor.get_mut() = new_free_cursor; self.meta - .resize(entity.index as usize + 1, EntityMeta::EMPTY); + .resize(entity.index() as usize + 1, EntityMeta::EMPTY); self.len += 1; AllocAtWithoutReplacement::DidNotExist - } else if let Some(index) = self.pending.iter().position(|item| *item == entity.index) { + } else if let Some(index) = self.pending.iter().position(|item| *item == entity.index()) { self.pending.swap_remove(index); let new_free_cursor = self.pending.len() as IdCursor; *self.free_cursor.get_mut() = new_free_cursor; self.len += 1; AllocAtWithoutReplacement::DidNotExist } else { - let current_meta = &self.meta[entity.index as usize]; + let current_meta = &self.meta[entity.index() as usize]; if current_meta.location.archetype_id == ArchetypeId::INVALID { AllocAtWithoutReplacement::DidNotExist } else if current_meta.generation == entity.generation { @@ -631,7 +630,7 @@ impl Entities { } }; - self.meta[entity.index as usize].generation = entity.generation; + self.meta[entity.index() as usize].generation = entity.generation; result } @@ -641,12 +640,12 @@ impl Entities { pub fn free(&mut self, entity: Entity) -> Option { self.verify_flushed(); - let meta = &mut self.meta[entity.index as usize]; + let meta = &mut self.meta[entity.index() as usize]; if meta.generation != entity.generation { return None; } - meta.generation = inc_generation_by(meta.generation, 1); + meta.generation = IdentifierMask::inc_masked_high_by(meta.generation, 1); if meta.generation == NonZeroU32::MIN { warn!( @@ -657,7 +656,7 @@ impl Entities { let loc = mem::replace(&mut meta.location, EntityMeta::EMPTY.location); - self.pending.push(entity.index); + self.pending.push(entity.index()); let new_free_cursor = self.pending.len() as IdCursor; *self.free_cursor.get_mut() = new_free_cursor; @@ -699,7 +698,7 @@ impl Entities { /// Note: for pending entities, returns `Some(EntityLocation::INVALID)`. #[inline] pub fn get(&self, entity: Entity) -> Option { - if let Some(meta) = self.meta.get(entity.index as usize) { + if let Some(meta) = self.meta.get(entity.index() as usize) { if meta.generation != entity.generation || meta.location.archetype_id == ArchetypeId::INVALID { @@ -736,7 +735,7 @@ impl Entities { let meta = &mut self.meta[index as usize]; if meta.location.archetype_id == ArchetypeId::INVALID { - meta.generation = inc_generation_by(meta.generation, generations); + meta.generation = IdentifierMask::inc_masked_high_by(meta.generation, generations); true } else { false @@ -752,17 +751,14 @@ impl Entities { pub fn resolve_from_id(&self, index: u32) -> Option { let idu = index as usize; if let Some(&EntityMeta { generation, .. }) = self.meta.get(idu) { - Some(Entity { generation, index }) + Some(Entity::new(index, generation)) } else { // `id` is outside of the meta list - check whether it is reserved but not yet flushed. let free_cursor = self.free_cursor.load(Ordering::Relaxed); // If this entity was manually created, then free_cursor might be positive // Returning None handles that case correctly let num_pending = usize::try_from(-free_cursor).ok()?; - (idu < self.meta.len() + num_pending).then_some(Entity { - generation: NonZeroU32::MIN, - index, - }) + (idu < self.meta.len() + num_pending).then_some(Entity::from_raw(index)) } } @@ -793,10 +789,7 @@ impl Entities { self.len += -current_free_cursor as u32; for (index, meta) in self.meta.iter_mut().enumerate().skip(old_meta_len) { init( - Entity { - index: index as u32, - generation: meta.generation, - }, + Entity::new(index as u32, meta.generation), &mut meta.location, ); } @@ -808,13 +801,7 @@ impl Entities { self.len += (self.pending.len() - new_free_cursor) as u32; for index in self.pending.drain(new_free_cursor..) { let meta = &mut self.meta[index as usize]; - init( - Entity { - index, - generation: meta.generation, - }, - &mut meta.location, - ); + init(Entity::new(index, meta.generation), &mut meta.location); } } @@ -930,49 +917,10 @@ impl EntityLocation { }; } -/// Offsets a generation value by the specified amount, wrapping to 1 instead of 0 -pub(crate) const fn inc_generation_by(lhs: NonZeroU32, rhs: u32) -> NonZeroU32 { - let (lo, hi) = lhs.get().overflowing_add(rhs); - let ret = lo + hi as u32; - // SAFETY: - // - Adding the overflow flag will offet overflows to start at 1 instead of 0 - // - The sum of `NonZeroU32::MAX` + `u32::MAX` + 1 (overflow) == `NonZeroU32::MAX` - // - If the operation doesn't overflow, no offsetting takes place - unsafe { NonZeroU32::new_unchecked(ret) } -} - #[cfg(test)] mod tests { use super::*; - #[test] - fn inc_generation_by_is_safe() { - // Correct offsets without overflow - assert_eq!( - inc_generation_by(NonZeroU32::MIN, 1), - NonZeroU32::new(2).unwrap() - ); - - assert_eq!( - inc_generation_by(NonZeroU32::MIN, 10), - NonZeroU32::new(11).unwrap() - ); - - // Check that overflows start at 1 correctly - assert_eq!(inc_generation_by(NonZeroU32::MAX, 1), NonZeroU32::MIN); - - assert_eq!( - inc_generation_by(NonZeroU32::MAX, 2), - NonZeroU32::new(2).unwrap() - ); - - // Ensure we can't overflow twice - assert_eq!( - inc_generation_by(NonZeroU32::MAX, u32::MAX), - NonZeroU32::MAX - ); - } - #[test] fn entity_niche_optimization() { assert_eq!( @@ -983,10 +931,8 @@ mod tests { #[test] fn entity_bits_roundtrip() { - let e = Entity { - generation: NonZeroU32::new(0xDEADBEEF).unwrap(), - index: 0xBAADF00D, - }; + // Generation cannot be greater than 0x7FFF_FFFF else it will be an invalid Entity id + let e = Entity::new(0xDEADBEEF, NonZeroU32::new(0x5AADF00D).unwrap()); assert_eq!(Entity::from_bits(e.to_bits()), e); } @@ -1040,7 +986,7 @@ mod tests { let entity = entities.alloc(); entities.free(entity); - assert!(entities.reserve_generations(entity.index, 1)); + assert!(entities.reserve_generations(entity.index(), 1)); } #[test] @@ -1051,7 +997,7 @@ mod tests { let entity = entities.alloc(); entities.free(entity); - assert!(entities.reserve_generations(entity.index, GENERATIONS)); + assert!(entities.reserve_generations(entity.index(), GENERATIONS)); // The very next entity allocated should be a further generation on the same index let next_entity = entities.alloc(); @@ -1059,30 +1005,31 @@ mod tests { assert!(next_entity.generation() > entity.generation() + GENERATIONS); } + #[rustfmt::skip] #[test] fn entity_comparison() { // This is intentionally testing `lt` and `ge` as separate functions. #![allow(clippy::nonminimal_bool)] - assert_eq!(Entity::new(123, 456), Entity::new(123, 456)); - assert_ne!(Entity::new(123, 789), Entity::new(123, 456)); - assert_ne!(Entity::new(123, 456), Entity::new(123, 789)); - assert_ne!(Entity::new(123, 456), Entity::new(456, 123)); + assert_eq!( Entity::new(123, NonZeroU32::new(456).unwrap()), Entity::new(123, NonZeroU32::new(456).unwrap()) ); + assert_ne!( Entity::new(123, NonZeroU32::new(789).unwrap()), Entity::new(123, NonZeroU32::new(456).unwrap()) ); + assert_ne!(Entity::new(123, NonZeroU32::new(456).unwrap()), Entity::new(123, NonZeroU32::new(789).unwrap())); + assert_ne!(Entity::new(123, NonZeroU32::new(456).unwrap()), Entity::new(456, NonZeroU32::new(123).unwrap())); // ordering is by generation then by index - assert!(Entity::new(123, 456) >= Entity::new(123, 456)); - assert!(Entity::new(123, 456) <= Entity::new(123, 456)); - assert!(!(Entity::new(123, 456) < Entity::new(123, 456))); - assert!(!(Entity::new(123, 456) > Entity::new(123, 456))); + assert!(Entity::new(123, NonZeroU32::new(456).unwrap()) >= Entity::new(123, NonZeroU32::new(456).unwrap())); + assert!(Entity::new(123, NonZeroU32::new(456).unwrap()) <= Entity::new(123, NonZeroU32::new(456).unwrap())); + assert!(!(Entity::new(123, NonZeroU32::new(456).unwrap()) < Entity::new(123, NonZeroU32::new(456).unwrap()))); + assert!(!(Entity::new(123, NonZeroU32::new(456).unwrap()) > Entity::new(123, NonZeroU32::new(456).unwrap()))); - assert!(Entity::new(9, 1) < Entity::new(1, 9)); - assert!(Entity::new(1, 9) > Entity::new(9, 1)); + assert!(Entity::new(9, NonZeroU32::new(1).unwrap()) < Entity::new(1, NonZeroU32::new(9).unwrap())); + assert!(Entity::new(1, NonZeroU32::new(9).unwrap()) > Entity::new(9, NonZeroU32::new(1).unwrap())); - assert!(Entity::new(1, 1) < Entity::new(2, 1)); - assert!(Entity::new(1, 1) <= Entity::new(2, 1)); - assert!(Entity::new(2, 2) > Entity::new(1, 2)); - assert!(Entity::new(2, 2) >= Entity::new(1, 2)); + assert!(Entity::new(1, NonZeroU32::new(1).unwrap()) < Entity::new(2, NonZeroU32::new(1).unwrap())); + assert!(Entity::new(1, NonZeroU32::new(1).unwrap()) <= Entity::new(2, NonZeroU32::new(1).unwrap())); + assert!(Entity::new(2, NonZeroU32::new(2).unwrap()) > Entity::new(1, NonZeroU32::new(2).unwrap())); + assert!(Entity::new(2, NonZeroU32::new(2).unwrap()) >= Entity::new(1, NonZeroU32::new(2).unwrap())); } // Feel free to change this test if needed, but it seemed like an important diff --git a/crates/bevy_ecs/src/identifier/error.rs b/crates/bevy_ecs/src/identifier/error.rs new file mode 100644 index 0000000000000..8974ce954b2d9 --- /dev/null +++ b/crates/bevy_ecs/src/identifier/error.rs @@ -0,0 +1,29 @@ +//! Error types for [`super::Identifier`] conversions. An ID can be converted +//! to various kinds, but these can fail if they are not valid forms of those +//! kinds. The error type in this module encapsulates the various failure modes. +use std::fmt; + +/// An Error type for [`super::Identifier`], mostly for providing error +/// handling for convertions of an ID to a type abstracting over the ID bits. +#[derive(Debug, PartialEq, Eq, Clone, Copy)] +#[non_exhaustive] +pub enum IdentifierError { + /// A given ID has an invalid value for initialising to a [`crate::identifier::Identifier`]. + InvalidIdentifier, + /// A given ID has an invalid configuration of bits for converting to an [`crate::entity::Entity`]. + InvalidEntityId(u64), +} + +impl fmt::Display for IdentifierError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::InvalidIdentifier => write!( + f, + "The given id contains a zero value high component, which is invalid" + ), + Self::InvalidEntityId(_) => write!(f, "The given id is not a valid entity."), + } + } +} + +impl std::error::Error for IdentifierError {} diff --git a/crates/bevy_ecs/src/identifier/kinds.rs b/crates/bevy_ecs/src/identifier/kinds.rs new file mode 100644 index 0000000000000..a5f57365fc1f0 --- /dev/null +++ b/crates/bevy_ecs/src/identifier/kinds.rs @@ -0,0 +1,11 @@ +/// The kinds of ID that [`super::Identifier`] can represent. Each +/// variant imposes different usages of the low/high segments +/// of the ID. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[repr(u8)] +pub enum IdKind { + /// An ID variant that is compatible with [`crate::entity::Entity`]. + Entity = 0, + /// A future ID variant. + Placeholder = 0b1000_0000, +} diff --git a/crates/bevy_ecs/src/identifier/masks.rs b/crates/bevy_ecs/src/identifier/masks.rs new file mode 100644 index 0000000000000..67d820b44b2bb --- /dev/null +++ b/crates/bevy_ecs/src/identifier/masks.rs @@ -0,0 +1,234 @@ +use std::num::NonZeroU32; + +use super::kinds::IdKind; + +/// Mask for extracting the lower 32-bit segment of a `u64` value. Can be +/// negated to extract the higher 32-bit segment. +const LOW_MASK: u64 = 0x0000_0000_FFFF_FFFF; +/// Mask for extracting the value portion of a 32-bit high segment. This +/// yields 31-bits of total value, as the final bit (the most significant) +/// is reserved as a flag bit. Can be negated to extract the flag bit. +const HIGH_MASK: u32 = 0x7FFF_FFFF; + +/// Abstraction over masks needed to extract values/components of an [`super::Identifier`]. +pub(crate) struct IdentifierMask; + +impl IdentifierMask { + /// Returns the low component from a `u64` value + #[inline(always)] + pub(crate) const fn get_low(value: u64) -> u32 { + (value & LOW_MASK) as u32 + } + + /// Returns the high component from a `u64` value + #[inline(always)] + pub(crate) const fn get_high(value: u64) -> u32 { + ((value & !LOW_MASK) >> u32::BITS) as u32 + } + + /// Pack a low and high `u32` values into a single `u64` value. + #[inline(always)] + pub(crate) const fn pack_into_u64(low: u32, high: u32) -> u64 { + ((high as u64) << u32::BITS) | (low as u64) + } + + /// Pack the [`IdKind`] bits into a high segment. + #[inline(always)] + pub(crate) const fn pack_kind_into_high(value: u32, kind: IdKind) -> u32 { + value | ((kind as u32) << 24) + } + + /// Extract the value component from a high segment of an [`super::Identifier`]. + #[inline(always)] + pub(crate) const fn extract_value_from_high(value: u32) -> u32 { + value & HIGH_MASK + } + + /// Extract the ID kind component from a high segment of an [`super::Identifier`]. + #[inline(always)] + pub(crate) const fn extract_kind_from_high(value: u32) -> IdKind { + // The negated HIGH_MASK will extract just the bit we need for kind. + let kind_mask = !HIGH_MASK; + let bit = value & kind_mask; + + if bit == kind_mask { + IdKind::Placeholder + } else { + IdKind::Entity + } + } + + /// Offsets a masked generation value by the specified amount, wrapping to 1 instead of 0. + /// Will never be greater than [`HIGH_MASK`] or less than `1`, and increments are masked to + /// never be greater than [`HIGH_MASK`]. + #[inline(always)] + pub(crate) const fn inc_masked_high_by(lhs: NonZeroU32, rhs: u32) -> NonZeroU32 { + let lo = (lhs.get() & HIGH_MASK).wrapping_add(rhs & HIGH_MASK); + // Checks high 32 bit for whether we have overflowed 31 bits. + let overflowed = lo >> 31; + + // SAFETY: + // - Adding the overflow flag will offet overflows to start at 1 instead of 0 + // - The sum of `0x7FFF_FFFF` + `u32::MAX` + 1 (overflow) == `0x7FFF_FFFF` + // - If the operation doesn't overflow at 31 bits, no offsetting takes place + unsafe { NonZeroU32::new_unchecked(lo.wrapping_add(overflowed) & HIGH_MASK) } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn get_u64_parts() { + // Two distinct bit patterns per low/high component + let value: u64 = 0x7FFF_FFFF_0000_000C; + + assert_eq!(IdentifierMask::get_low(value), 0x0000_000C); + assert_eq!(IdentifierMask::get_high(value), 0x7FFF_FFFF); + } + + #[test] + fn extract_kind() { + // All bits are ones. + let high: u32 = 0xFFFF_FFFF; + + assert_eq!( + IdentifierMask::extract_kind_from_high(high), + IdKind::Placeholder + ); + + // Second and second to last bits are ones. + let high: u32 = 0x4000_0002; + + assert_eq!(IdentifierMask::extract_kind_from_high(high), IdKind::Entity); + } + + #[test] + fn extract_high_value() { + // All bits are ones. + let high: u32 = 0xFFFF_FFFF; + + // Excludes the most significant bit as that is a flag bit. + assert_eq!(IdentifierMask::extract_value_from_high(high), 0x7FFF_FFFF); + + // Start bit and end bit are ones. + let high: u32 = 0x8000_0001; + + assert_eq!(IdentifierMask::extract_value_from_high(high), 0x0000_0001); + + // Classic bit pattern. + let high: u32 = 0xDEAD_BEEF; + + assert_eq!(IdentifierMask::extract_value_from_high(high), 0x5EAD_BEEF); + } + + #[test] + fn pack_kind_bits() { + // All bits are ones expect the most significant bit, which is zero + let high: u32 = 0x7FFF_FFFF; + + assert_eq!( + IdentifierMask::pack_kind_into_high(high, IdKind::Placeholder), + 0xFFFF_FFFF + ); + + // Arbitrary bit pattern + let high: u32 = 0x00FF_FF00; + + assert_eq!( + IdentifierMask::pack_kind_into_high(high, IdKind::Entity), + // Remains unchanged as before + 0x00FF_FF00 + ); + + // Bit pattern that almost spells a word + let high: u32 = 0x40FF_EEEE; + + assert_eq!( + IdentifierMask::pack_kind_into_high(high, IdKind::Placeholder), + 0xC0FF_EEEE // Milk and no sugar, please. + ); + } + + #[test] + fn pack_into_u64() { + let high: u32 = 0x7FFF_FFFF; + let low: u32 = 0x0000_00CC; + + assert_eq!( + IdentifierMask::pack_into_u64(low, high), + 0x7FFF_FFFF_0000_00CC + ); + } + + #[test] + fn incrementing_masked_nonzero_high_is_safe() { + // Adding from lowest value with lowest to highest increment + // No result should ever be greater than 0x7FFF_FFFF or HIGH_MASK + assert_eq!( + NonZeroU32::MIN, + IdentifierMask::inc_masked_high_by(NonZeroU32::MIN, 0) + ); + assert_eq!( + NonZeroU32::new(2).unwrap(), + IdentifierMask::inc_masked_high_by(NonZeroU32::MIN, 1) + ); + assert_eq!( + NonZeroU32::new(3).unwrap(), + IdentifierMask::inc_masked_high_by(NonZeroU32::MIN, 2) + ); + assert_eq!( + NonZeroU32::MIN, + IdentifierMask::inc_masked_high_by(NonZeroU32::MIN, HIGH_MASK) + ); + assert_eq!( + NonZeroU32::MIN, + IdentifierMask::inc_masked_high_by(NonZeroU32::MIN, u32::MAX) + ); + // Adding from absolute highest value with lowest to highest increment + // No result should ever be greater than 0x7FFF_FFFF or HIGH_MASK + assert_eq!( + NonZeroU32::new(HIGH_MASK).unwrap(), + IdentifierMask::inc_masked_high_by(NonZeroU32::MAX, 0) + ); + assert_eq!( + NonZeroU32::MIN, + IdentifierMask::inc_masked_high_by(NonZeroU32::MAX, 1) + ); + assert_eq!( + NonZeroU32::new(2).unwrap(), + IdentifierMask::inc_masked_high_by(NonZeroU32::MAX, 2) + ); + assert_eq!( + NonZeroU32::new(HIGH_MASK).unwrap(), + IdentifierMask::inc_masked_high_by(NonZeroU32::MAX, HIGH_MASK) + ); + assert_eq!( + NonZeroU32::new(HIGH_MASK).unwrap(), + IdentifierMask::inc_masked_high_by(NonZeroU32::MAX, u32::MAX) + ); + // Adding from actual highest value with lowest to highest increment + // No result should ever be greater than 0x7FFF_FFFF or HIGH_MASK + assert_eq!( + NonZeroU32::new(HIGH_MASK).unwrap(), + IdentifierMask::inc_masked_high_by(NonZeroU32::new(HIGH_MASK).unwrap(), 0) + ); + assert_eq!( + NonZeroU32::MIN, + IdentifierMask::inc_masked_high_by(NonZeroU32::new(HIGH_MASK).unwrap(), 1) + ); + assert_eq!( + NonZeroU32::new(2).unwrap(), + IdentifierMask::inc_masked_high_by(NonZeroU32::new(HIGH_MASK).unwrap(), 2) + ); + assert_eq!( + NonZeroU32::new(HIGH_MASK).unwrap(), + IdentifierMask::inc_masked_high_by(NonZeroU32::new(HIGH_MASK).unwrap(), HIGH_MASK) + ); + assert_eq!( + NonZeroU32::new(HIGH_MASK).unwrap(), + IdentifierMask::inc_masked_high_by(NonZeroU32::new(HIGH_MASK).unwrap(), u32::MAX) + ); + } +} diff --git a/crates/bevy_ecs/src/identifier/mod.rs b/crates/bevy_ecs/src/identifier/mod.rs new file mode 100644 index 0000000000000..20111b591b3a5 --- /dev/null +++ b/crates/bevy_ecs/src/identifier/mod.rs @@ -0,0 +1,241 @@ +//! A module for the unified [`Identifier`] ID struct, for use as a representation +//! of multiple types of IDs in a single, packed type. Allows for describing an [`crate::entity::Entity`], +//! or other IDs that can be packed and expressed within a `u64` sized type. +//! [`Identifier`]s cannot be created directly, only able to be converted from other +//! compatible IDs. +use self::{error::IdentifierError, kinds::IdKind, masks::IdentifierMask}; +use std::{hash::Hash, num::NonZeroU32}; + +pub mod error; +pub(crate) mod kinds; +pub(crate) mod masks; + +/// A unified identifier for all entity and similar IDs. +/// Has the same size as a `u64` integer, but the layout is split between a 32-bit low +/// segment, a 31-bit high segment, and the significant bit reserved as type flags to denote +/// entity kinds. +#[derive(Debug, Clone, Copy)] +// Alignment repr necessary to allow LLVM to better output +// optimised codegen for `to_bits`, `PartialEq` and `Ord`. +#[repr(C, align(8))] +pub struct Identifier { + // Do not reorder the fields here. The ordering is explicitly used by repr(C) + // to make this struct equivalent to a u64. + #[cfg(target_endian = "little")] + low: u32, + high: NonZeroU32, + #[cfg(target_endian = "big")] + low: u32, +} + +impl Identifier { + /// Construct a new [`Identifier`]. The `high` parameter is masked with the + /// `kind` so to pack the high value and bit flags into the same field. + #[inline(always)] + pub const fn new(low: u32, high: u32, kind: IdKind) -> Result { + // the high bits are masked to cut off the most significant bit + // as these are used for the type flags. This means that the high + // portion is only 31 bits, but this still provides 2^31 + // values/kinds/ids that can be stored in this segment. + let masked_value = IdentifierMask::extract_value_from_high(high); + + let packed_high = IdentifierMask::pack_kind_into_high(masked_value, kind); + + // If the packed high component ends up being zero, that means that we tried + // to initialise an Identifier into an invalid state. + if packed_high == 0 { + Err(IdentifierError::InvalidIdentifier) + } else { + // SAFETY: The high value has been checked to ensure it is never + // zero. + unsafe { + Ok(Self { + low, + high: NonZeroU32::new_unchecked(packed_high), + }) + } + } + } + + /// Returns the value of the low segment of the [`Identifier`]. + #[inline(always)] + pub const fn low(self) -> u32 { + self.low + } + + /// Returns the value of the high segment of the [`Identifier`]. This + /// does not apply any masking. + #[inline(always)] + pub const fn high(self) -> NonZeroU32 { + self.high + } + + /// Returns the masked value of the high segment of the [`Identifier`]. + /// Does not include the flag bits. + #[inline(always)] + pub const fn masked_high(self) -> u32 { + IdentifierMask::extract_value_from_high(self.high.get()) + } + + /// Returns the kind of [`Identifier`] from the high segment. + #[inline(always)] + pub const fn kind(self) -> IdKind { + IdentifierMask::extract_kind_from_high(self.high.get()) + } + + /// Convert the [`Identifier`] into a `u64`. + #[inline(always)] + pub const fn to_bits(self) -> u64 { + IdentifierMask::pack_into_u64(self.low, self.high.get()) + } + + /// Convert a `u64` into an [`Identifier`]. + /// + /// # Panics + /// + /// This method will likely panic if given `u64` values that did not come from [`Identifier::to_bits`]. + #[inline(always)] + pub const fn from_bits(value: u64) -> Self { + let id = Self::try_from_bits(value); + + match id { + Ok(id) => id, + Err(_) => panic!("Attempted to initialise invalid bits as an id"), + } + } + + /// Convert a `u64` into an [`Identifier`]. + /// + /// This method is the fallible counterpart to [`Identifier::from_bits`]. + #[inline(always)] + pub const fn try_from_bits(value: u64) -> Result { + let high = NonZeroU32::new(IdentifierMask::get_high(value)); + + match high { + Some(high) => Ok(Self { + low: IdentifierMask::get_low(value), + high, + }), + None => Err(IdentifierError::InvalidIdentifier), + } + } +} + +// By not short-circuiting in comparisons, we get better codegen. +// See +impl PartialEq for Identifier { + #[inline] + fn eq(&self, other: &Self) -> bool { + // By using `to_bits`, the codegen can be optimised out even + // further potentially. Relies on the correct alignment/field + // order of `Entity`. + self.to_bits() == other.to_bits() + } +} + +impl Eq for Identifier {} + +// The derive macro codegen output is not optimal and can't be optimised as well +// by the compiler. This impl resolves the issue of non-optimal codegen by relying +// on comparing against the bit representation of `Entity` instead of comparing +// the fields. The result is then LLVM is able to optimise the codegen for Entity +// far beyond what the derive macro can. +// See +impl PartialOrd for Identifier { + #[inline] + fn partial_cmp(&self, other: &Self) -> Option { + // Make use of our `Ord` impl to ensure optimal codegen output + Some(self.cmp(other)) + } +} + +// The derive macro codegen output is not optimal and can't be optimised as well +// by the compiler. This impl resolves the issue of non-optimal codegen by relying +// on comparing against the bit representation of `Entity` instead of comparing +// the fields. The result is then LLVM is able to optimise the codegen for Entity +// far beyond what the derive macro can. +// See +impl Ord for Identifier { + #[inline] + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + // This will result in better codegen for ordering comparisons, plus + // avoids pitfalls with regards to macro codegen relying on property + // position when we want to compare against the bit representation. + self.to_bits().cmp(&other.to_bits()) + } +} + +impl Hash for Identifier { + #[inline] + fn hash(&self, state: &mut H) { + self.to_bits().hash(state); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn id_construction() { + let id = Identifier::new(12, 55, IdKind::Entity).unwrap(); + + assert_eq!(id.low(), 12); + assert_eq!(id.high().get(), 55); + assert_eq!( + IdentifierMask::extract_kind_from_high(id.high().get()), + IdKind::Entity + ); + } + + #[test] + fn from_bits() { + // This high value should correspond to the max high() value + // and also Entity flag. + let high = 0x7FFFFFFF; + let low = 0xC; + let bits: u64 = high << u32::BITS | low; + + let id = Identifier::try_from_bits(bits).unwrap(); + + assert_eq!(id.to_bits(), 0x7FFFFFFF0000000C); + assert_eq!(id.low(), low as u32); + assert_eq!(id.high().get(), 0x7FFFFFFF); + assert_eq!( + IdentifierMask::extract_kind_from_high(id.high().get()), + IdKind::Entity + ); + } + + #[rustfmt::skip] + #[test] + fn id_comparison() { + // This is intentionally testing `lt` and `ge` as separate functions. + #![allow(clippy::nonminimal_bool)] + + assert!(Identifier::new(123, 456, IdKind::Entity).unwrap() == Identifier::new(123, 456, IdKind::Entity).unwrap()); + assert!(Identifier::new(123, 456, IdKind::Placeholder).unwrap() == Identifier::new(123, 456, IdKind::Placeholder).unwrap()); + assert!(Identifier::new(123, 789, IdKind::Entity).unwrap() != Identifier::new(123, 456, IdKind::Entity).unwrap()); + assert!(Identifier::new(123, 456, IdKind::Entity).unwrap() != Identifier::new(123, 789, IdKind::Entity).unwrap()); + assert!(Identifier::new(123, 456, IdKind::Entity).unwrap() != Identifier::new(456, 123, IdKind::Entity).unwrap()); + assert!(Identifier::new(123, 456, IdKind::Entity).unwrap()!= Identifier::new(123, 456, IdKind::Placeholder).unwrap()); + + // ordering is by flag then high then by low + + assert!(Identifier::new(123, 456, IdKind::Entity).unwrap() >= Identifier::new(123, 456, IdKind::Entity).unwrap()); + assert!(Identifier::new(123, 456, IdKind::Entity).unwrap() <= Identifier::new(123, 456, IdKind::Entity).unwrap()); + assert!(!(Identifier::new(123, 456, IdKind::Entity).unwrap() < Identifier::new(123, 456, IdKind::Entity).unwrap())); + assert!(!(Identifier::new(123, 456, IdKind::Entity).unwrap() > Identifier::new(123, 456, IdKind::Entity).unwrap())); + + assert!(Identifier::new(9, 1, IdKind::Entity).unwrap() < Identifier::new(1, 9, IdKind::Entity).unwrap()); + assert!(Identifier::new(1, 9, IdKind::Entity).unwrap() > Identifier::new(9, 1, IdKind::Entity).unwrap()); + + assert!(Identifier::new(9, 1, IdKind::Entity).unwrap() < Identifier::new(9, 1, IdKind::Placeholder).unwrap()); + assert!(Identifier::new(1, 9, IdKind::Placeholder).unwrap() > Identifier::new(1, 9, IdKind::Entity).unwrap()); + + assert!(Identifier::new(1, 1, IdKind::Entity).unwrap() < Identifier::new(2, 1, IdKind::Entity).unwrap()); + assert!(Identifier::new(1, 1, IdKind::Entity).unwrap() <= Identifier::new(2, 1, IdKind::Entity).unwrap()); + assert!(Identifier::new(2, 2, IdKind::Entity).unwrap()> Identifier::new(1, 2, IdKind::Entity).unwrap()); + assert!(Identifier::new(2, 2, IdKind::Entity).unwrap() >= Identifier::new(1, 2, IdKind::Entity).unwrap()); + } +} diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 21c81e846d10e..180ffabe2fa75 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -10,6 +10,7 @@ pub mod change_detection; pub mod component; pub mod entity; pub mod event; +pub mod identifier; pub mod query; #[cfg(feature = "bevy_reflect")] pub mod reflect; @@ -69,6 +70,7 @@ mod tests { world::{EntityRef, Mut, World}, }; use bevy_tasks::{ComputeTaskPool, TaskPool}; + use std::num::NonZeroU32; use std::{ any::TypeId, marker::PhantomData, @@ -1590,7 +1592,7 @@ mod tests { "spawning into existing `world_b` entities works" ); - let e4_mismatched_generation = Entity::new(3, 2).unwrap(); + let e4_mismatched_generation = Entity::new(3, NonZeroU32::new(2).unwrap()); assert!( world_b.get_or_spawn(e4_mismatched_generation).is_none(), "attempting to spawn on top of an entity with a mismatched entity generation fails" @@ -1685,7 +1687,7 @@ mod tests { let e0 = world.spawn(A(0)).id(); let e1 = Entity::from_raw(1); let e2 = world.spawn_empty().id(); - let invalid_e2 = Entity::new(e2.index(), 2).unwrap(); + let invalid_e2 = Entity::new(e2.index(), NonZeroU32::new(2).unwrap()); let values = vec![(e0, (B(0), C)), (e1, (B(1), C)), (invalid_e2, (B(2), C))];