diff --git a/benches/benches/bevy_ecs/world/entity_hash.rs b/benches/benches/bevy_ecs/world/entity_hash.rs index 3562510c90..2f92a715b4 100644 --- a/benches/benches/bevy_ecs/world/entity_hash.rs +++ b/benches/benches/bevy_ecs/world/entity_hash.rs @@ -1,4 +1,4 @@ -use bevy_ecs::entity::{Entity, EntityHashSet}; +use bevy_ecs::entity::{Entity, EntityGeneration, EntityHashSet}; use criterion::{BenchmarkId, Criterion, Throughput}; use rand::{Rng, SeedableRng}; use rand_chacha::ChaCha8Rng; @@ -21,7 +21,10 @@ fn make_entity(rng: &mut impl Rng, size: usize) -> Entity { let bits = ((generation as u64) << 32) | id; let e = Entity::from_bits(bits); assert_eq!(e.index(), !(id as u32)); - assert_eq!(e.generation(), generation as u32); + assert_eq!( + e.generation(), + EntityGeneration::FIRST.after_versions(generation as u32) + ); e } diff --git a/crates/bevy_ecs/src/entity/map_entities.rs b/crates/bevy_ecs/src/entity/map_entities.rs index 6e2c11160c..3dac2fa749 100644 --- a/crates/bevy_ecs/src/entity/map_entities.rs +++ b/crates/bevy_ecs/src/entity/map_entities.rs @@ -3,7 +3,6 @@ use indexmap::IndexSet; use crate::{ entity::{hash_map::EntityHashMap, Entity}, - identifier::masks::{IdentifierMask, HIGH_MASK}, world::World, }; @@ -229,11 +228,9 @@ impl EntityMapper for SceneEntityMapper<'_> { // this new entity reference is specifically designed to never represent any living entity let new = Entity::from_raw_and_generation( self.dead_start.row(), - IdentifierMask::inc_masked_high_by(self.dead_start.generation, self.generations), + self.dead_start.generation.after_versions(self.generations), ); - - // Prevent generations counter from being a greater value than HIGH_MASK. - self.generations = (self.generations + 1) & HIGH_MASK; + self.generations = self.generations.wrapping_add(1); self.map.insert(source, new); diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index 02311edf42..830c9919d0 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -76,12 +76,6 @@ pub use unique_vec::{UniqueEntityEquivalentVec, UniqueEntityVec}; use crate::{ archetype::{ArchetypeId, ArchetypeRow}, change_detection::MaybeLocation, - identifier::{ - error::IdentifierError, - kinds::IdKind, - masks::{IdentifierMask, HIGH_MASK}, - Identifier, - }, storage::{SparseSetIndex, TableId, TableRow}, }; use alloc::vec::Vec; @@ -136,7 +130,7 @@ impl EntityRow { self.0.get() } - /// Gets a some bits that represent this value. + /// Gets some bits that represent this value. /// The bits are opaque and should not be regarded as meaningful. #[inline(always)] const fn to_bits(self) -> u32 { @@ -185,6 +179,54 @@ impl SparseSetIndex for EntityRow { } } +/// This tracks different versions or generations of an [`EntityRow`]. +/// Importantly, this can wrap, meaning each generation is not necessarily unique per [`EntityRow`]. +/// +/// This should be treated as a opaque identifier, and it's internal representation may be subject to change. +#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Display)] +#[cfg_attr(feature = "bevy_reflect", derive(Reflect))] +#[cfg_attr(feature = "bevy_reflect", reflect(opaque))] +#[cfg_attr(feature = "bevy_reflect", reflect(Hash, PartialEq, Debug, Clone))] +#[repr(transparent)] +pub struct EntityGeneration(u32); + +impl EntityGeneration { + /// Represents the first generation of an [`EntityRow`]. + pub const FIRST: Self = Self(0); + + /// Gets some bits that represent this value. + /// The bits are opaque and should not be regarded as meaningful. + #[inline(always)] + const fn to_bits(self) -> u32 { + self.0 + } + + /// Reconstruct an [`EntityGeneration`] previously destructured with [`EntityGeneration::to_bits`]. + /// + /// Only useful when applied to results from `to_bits` in the same instance of an application. + #[inline] + const fn from_bits(bits: u32) -> Self { + Self(bits) + } + + /// Returns the [`EntityGeneration`] that would result from this many more `versions` of the corresponding [`EntityRow`] from passing. + #[inline] + pub const fn after_versions(self, versions: u32) -> Self { + Self(self.0.wrapping_add(versions)) + } + + /// Identical to [`after_versions`](Self::after_versions) but also returns a `bool` indicating if, + /// after these `versions`, one such version could conflict with a previous one. + /// + /// If this happens, this will no longer uniquely identify a version of an [`EntityRow`]. + /// This is called entity aliasing. + #[inline] + pub const fn after_versions_and_could_alias(self, versions: u32) -> (Self, bool) { + let raw = self.0.overflowing_add(versions); + (Self(raw.0), raw.1) + } +} + /// Lightweight identifier of an [entity](crate::entity). /// /// The identifier is implemented using a [generational index]: a combination of an index and a generation. @@ -269,7 +311,7 @@ pub struct Entity { // to make this struct equivalent to a u64. #[cfg(target_endian = "little")] row: EntityRow, - generation: NonZero, + generation: EntityGeneration, #[cfg(target_endian = "big")] row: EntityRow, } @@ -331,10 +373,8 @@ impl Entity { #[inline(always)] pub(crate) const fn from_raw_and_generation( row: EntityRow, - generation: NonZero, + generation: EntityGeneration, ) -> Entity { - debug_assert!(generation.get() <= HIGH_MASK); - Self { row, generation } } @@ -388,7 +428,7 @@ impl Entity { /// a component. #[inline(always)] pub const fn from_raw(row: EntityRow) -> Entity { - Self::from_raw_and_generation(row, NonZero::::MIN) + Self::from_raw_and_generation(row, EntityGeneration::FIRST) } /// This is equivalent to [`from_raw`](Self::from_raw) except that it takes a `u32` instead of an [`EntityRow`]. @@ -410,7 +450,7 @@ impl Entity { /// No particular structure is guaranteed for the returned bits. #[inline(always)] pub const fn to_bits(self) -> u64 { - IdentifierMask::pack_into_u64(self.row.to_bits(), self.generation.get()) + self.row.to_bits() as u64 | ((self.generation.to_bits() as u64) << 32) } /// Reconstruct an `Entity` previously destructured with [`Entity::to_bits`]. @@ -422,12 +462,10 @@ impl Entity { /// 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. - let id = Self::try_from_bits(bits); - - match id { - Ok(entity) => entity, - Err(_) => panic!("Attempted to initialize invalid bits as an entity"), + if let Some(id) = Self::try_from_bits(bits) { + id + } else { + panic!("Attempted to initialize invalid bits as an entity") } } @@ -437,21 +475,18 @@ 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 Ok(id) = Identifier::try_from_bits(bits) { - let kind = id.kind() as u8; + pub const fn try_from_bits(bits: u64) -> Option { + let raw_row = bits as u32; + let raw_gen = (bits >> 32) as u32; - if kind == (IdKind::Entity as u8) { - if let Some(row) = EntityRow::try_from_bits(id.low()) { - return Ok(Self { - row, - generation: id.high(), - }); - } - } + if let Some(row) = EntityRow::try_from_bits(raw_row) { + Some(Self { + row, + generation: EntityGeneration::from_bits(raw_gen), + }) + } else { + None } - - Err(IdentifierError::InvalidEntityId(bits)) } /// Return a transiently unique identifier. @@ -475,25 +510,8 @@ impl Entity { /// entity with a given row is despawned. This serves as a "count" of the number of times a /// given row has been reused (row, generation) pairs uniquely identify a given Entity. #[inline] - pub const fn generation(self) -> u32 { - // 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()) + pub const fn generation(self) -> EntityGeneration { + self.generation } } @@ -515,7 +533,8 @@ impl<'de> Deserialize<'de> for Entity { { use serde::de::Error; let id: u64 = Deserialize::deserialize(deserializer)?; - Entity::try_from_bits(id).map_err(D::Error::custom) + Entity::try_from_bits(id) + .ok_or_else(|| D::Error::custom("Attempting to deserialize an invalid entity.")) } } @@ -809,9 +828,9 @@ impl Entities { return None; } - meta.generation = IdentifierMask::inc_masked_high_by(meta.generation, 1); - - if meta.generation == NonZero::::MIN { + let (new_generation, aliased) = meta.generation.after_versions_and_could_alias(1); + meta.generation = new_generation; + if aliased { warn!( "Entity({}) generation wrapped on Entities::free, aliasing may occur", entity.row() @@ -905,7 +924,7 @@ impl Entities { let meta = &mut self.meta[index as usize]; if meta.location.archetype_id == ArchetypeId::INVALID { - meta.generation = IdentifierMask::inc_masked_high_by(meta.generation, generations); + meta.generation = meta.generation.after_versions(generations); true } else { false @@ -1060,7 +1079,7 @@ impl Entities { // Generation is incremented immediately upon despawn (meta.generation == entity.generation) || (meta.location.archetype_id == ArchetypeId::INVALID) - && (meta.generation == IdentifierMask::inc_masked_high_by(entity.generation, 1))) + && (meta.generation == entity.generation.after_versions(1))) .map(|meta| meta.spawned_or_despawned_by) }) .map(Option::flatten) @@ -1121,9 +1140,9 @@ impl fmt::Display for EntityDoesNotExistDetails { #[derive(Copy, Clone, Debug)] struct EntityMeta { - /// The current generation of the [`Entity`]. - pub generation: NonZero, - /// The current location of the [`Entity`] + /// The current [`EntityGeneration`] of the [`EntityRow`]. + pub generation: EntityGeneration, + /// The current location of the [`EntityRow`] pub location: EntityLocation, /// Location of the last spawn or despawn of this entity spawned_or_despawned_by: MaybeLocation>>, @@ -1132,7 +1151,7 @@ struct EntityMeta { impl EntityMeta { /// meta for **pending entity** const EMPTY: EntityMeta = EntityMeta { - generation: NonZero::::MIN, + generation: EntityGeneration::FIRST, location: EntityLocation::INVALID, spawned_or_despawned_by: MaybeLocation::new(None), }; @@ -1190,7 +1209,7 @@ mod tests { // Generation cannot be greater than 0x7FFF_FFFF else it will be an invalid Entity id let e = Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(0xDEADBEEF).unwrap()), - NonZero::::new(0x5AADF00D).unwrap(), + EntityGeneration::from_bits(0x5AADF00D), ); assert_eq!(Entity::from_bits(e.to_bits()), e); } @@ -1226,16 +1245,18 @@ mod tests { fn entity_const() { const C1: Entity = Entity::from_raw(EntityRow::new(NonMaxU32::new(42).unwrap())); assert_eq!(42, C1.index()); - assert_eq!(1, C1.generation()); + assert_eq!(0, C1.generation().to_bits()); const C2: Entity = Entity::from_bits(0x0000_00ff_0000_00cc); assert_eq!(!0x0000_00cc, C2.index()); - assert_eq!(0x0000_00ff, C2.generation()); + assert_eq!(0x0000_00ff, C2.generation().to_bits()); const C3: u32 = Entity::from_raw(EntityRow::new(NonMaxU32::new(33).unwrap())).index(); assert_eq!(33, C3); - const C4: u32 = Entity::from_bits(0x00dd_00ff_1111_1111).generation(); + const C4: u32 = Entity::from_bits(0x00dd_00ff_1111_1111) + .generation() + .to_bits(); assert_eq!(0x00dd_00ff, C4); } @@ -1261,7 +1282,7 @@ mod tests { // The very next entity allocated should be a further generation on the same index let next_entity = entities.alloc(); assert_eq!(next_entity.index(), entity.index()); - assert!(next_entity.generation() > entity.generation() + GENERATIONS); + assert!(next_entity.generation() > entity.generation().after_versions(GENERATIONS)); } #[test] @@ -1273,41 +1294,41 @@ mod tests { assert_eq!( Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(123).unwrap()), - NonZero::::new(456).unwrap() + EntityGeneration::from_bits(456) ), Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(123).unwrap()), - NonZero::::new(456).unwrap() + EntityGeneration::from_bits(456) ) ); assert_ne!( Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(123).unwrap()), - NonZero::::new(789).unwrap() + EntityGeneration::from_bits(789) ), Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(123).unwrap()), - NonZero::::new(456).unwrap() + EntityGeneration::from_bits(456) ) ); assert_ne!( Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(123).unwrap()), - NonZero::::new(456).unwrap() + EntityGeneration::from_bits(456) ), Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(123).unwrap()), - NonZero::::new(789).unwrap() + EntityGeneration::from_bits(789) ) ); assert_ne!( Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(123).unwrap()), - NonZero::::new(456).unwrap() + EntityGeneration::from_bits(456) ), Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(456).unwrap()), - NonZero::::new(123).unwrap() + EntityGeneration::from_bits(123) ) ); @@ -1316,93 +1337,93 @@ mod tests { assert!( Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(123).unwrap()), - NonZero::::new(456).unwrap() + EntityGeneration::from_bits(456) ) >= Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(123).unwrap()), - NonZero::::new(456).unwrap() + EntityGeneration::from_bits(456) ) ); assert!( Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(123).unwrap()), - NonZero::::new(456).unwrap() + EntityGeneration::from_bits(456) ) <= Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(123).unwrap()), - NonZero::::new(456).unwrap() + EntityGeneration::from_bits(456) ) ); assert!( !(Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(123).unwrap()), - NonZero::::new(456).unwrap() + EntityGeneration::from_bits(456) ) < Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(123).unwrap()), - NonZero::::new(456).unwrap() + EntityGeneration::from_bits(456) )) ); assert!( !(Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(123).unwrap()), - NonZero::::new(456).unwrap() + EntityGeneration::from_bits(456) ) > Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(123).unwrap()), - NonZero::::new(456).unwrap() + EntityGeneration::from_bits(456) )) ); assert!( Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(9).unwrap()), - NonZero::::new(1).unwrap() + EntityGeneration::from_bits(1) ) < Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(1).unwrap()), - NonZero::::new(9).unwrap() + EntityGeneration::from_bits(9) ) ); assert!( Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(1).unwrap()), - NonZero::::new(9).unwrap() + EntityGeneration::from_bits(9) ) > Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(9).unwrap()), - NonZero::::new(1).unwrap() + EntityGeneration::from_bits(1) ) ); assert!( Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(1).unwrap()), - NonZero::::new(1).unwrap() + EntityGeneration::from_bits(1) ) > Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(2).unwrap()), - NonZero::::new(1).unwrap() + EntityGeneration::from_bits(1) ) ); assert!( Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(1).unwrap()), - NonZero::::new(1).unwrap() + EntityGeneration::from_bits(1) ) >= Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(2).unwrap()), - NonZero::::new(1).unwrap() + EntityGeneration::from_bits(1) ) ); assert!( Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(2).unwrap()), - NonZero::::new(2).unwrap() + EntityGeneration::from_bits(2) ) < Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(1).unwrap()), - NonZero::::new(2).unwrap() + EntityGeneration::from_bits(2) ) ); assert!( Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(2).unwrap()), - NonZero::::new(2).unwrap() + EntityGeneration::from_bits(2) ) <= Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(1).unwrap()), - NonZero::::new(2).unwrap() + EntityGeneration::from_bits(2) ) ); } @@ -1452,7 +1473,7 @@ mod tests { fn entity_debug() { let entity = Entity::from_raw(EntityRow::new(NonMaxU32::new(42).unwrap())); let string = format!("{:?}", entity); - assert_eq!(string, "42v1#8589934549"); + assert_eq!(string, "42v0#4294967253"); let entity = Entity::PLACEHOLDER; let string = format!("{:?}", entity); @@ -1463,7 +1484,7 @@ mod tests { fn entity_display() { let entity = Entity::from_raw(EntityRow::new(NonMaxU32::new(42).unwrap())); let string = format!("{}", entity); - assert_eq!(string, "42v1"); + assert_eq!(string, "42v0"); let entity = Entity::PLACEHOLDER; let string = format!("{}", entity); diff --git a/crates/bevy_ecs/src/identifier/error.rs b/crates/bevy_ecs/src/identifier/error.rs deleted file mode 100644 index a4679a12c6..0000000000 --- a/crates/bevy_ecs/src/identifier/error.rs +++ /dev/null @@ -1,29 +0,0 @@ -//! 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 core::fmt; - -/// An Error type for [`super::Identifier`], mostly for providing error -/// handling for conversions 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 initializing 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 core::error::Error for IdentifierError {} diff --git a/crates/bevy_ecs/src/identifier/kinds.rs b/crates/bevy_ecs/src/identifier/kinds.rs deleted file mode 100644 index a5f57365fc..0000000000 --- a/crates/bevy_ecs/src/identifier/kinds.rs +++ /dev/null @@ -1,11 +0,0 @@ -/// 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 deleted file mode 100644 index 30ece9d8e3..0000000000 --- a/crates/bevy_ecs/src/identifier/masks.rs +++ /dev/null @@ -1,233 +0,0 @@ -use core::num::NonZero; - -use super::kinds::IdKind; - -/// 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. -pub(crate) 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 { - // This will truncate to the lowest 32 bits - value as u32 - } - - /// Returns the high component from a `u64` value - #[inline(always)] - pub(crate) const fn get_high(value: u64) -> u32 { - // This will discard the lowest 32 bits - (value >> 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: NonZero, rhs: u32) -> NonZero { - 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 offset 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 { NonZero::::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!( - NonZero::::MIN, - IdentifierMask::inc_masked_high_by(NonZero::::MIN, 0) - ); - assert_eq!( - NonZero::::new(2).unwrap(), - IdentifierMask::inc_masked_high_by(NonZero::::MIN, 1) - ); - assert_eq!( - NonZero::::new(3).unwrap(), - IdentifierMask::inc_masked_high_by(NonZero::::MIN, 2) - ); - assert_eq!( - NonZero::::MIN, - IdentifierMask::inc_masked_high_by(NonZero::::MIN, HIGH_MASK) - ); - assert_eq!( - NonZero::::MIN, - IdentifierMask::inc_masked_high_by(NonZero::::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!( - NonZero::::new(HIGH_MASK).unwrap(), - IdentifierMask::inc_masked_high_by(NonZero::::MAX, 0) - ); - assert_eq!( - NonZero::::MIN, - IdentifierMask::inc_masked_high_by(NonZero::::MAX, 1) - ); - assert_eq!( - NonZero::::new(2).unwrap(), - IdentifierMask::inc_masked_high_by(NonZero::::MAX, 2) - ); - assert_eq!( - NonZero::::new(HIGH_MASK).unwrap(), - IdentifierMask::inc_masked_high_by(NonZero::::MAX, HIGH_MASK) - ); - assert_eq!( - NonZero::::new(HIGH_MASK).unwrap(), - IdentifierMask::inc_masked_high_by(NonZero::::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!( - NonZero::::new(HIGH_MASK).unwrap(), - IdentifierMask::inc_masked_high_by(NonZero::::new(HIGH_MASK).unwrap(), 0) - ); - assert_eq!( - NonZero::::MIN, - IdentifierMask::inc_masked_high_by(NonZero::::new(HIGH_MASK).unwrap(), 1) - ); - assert_eq!( - NonZero::::new(2).unwrap(), - IdentifierMask::inc_masked_high_by(NonZero::::new(HIGH_MASK).unwrap(), 2) - ); - assert_eq!( - NonZero::::new(HIGH_MASK).unwrap(), - IdentifierMask::inc_masked_high_by(NonZero::::new(HIGH_MASK).unwrap(), HIGH_MASK) - ); - assert_eq!( - NonZero::::new(HIGH_MASK).unwrap(), - IdentifierMask::inc_masked_high_by(NonZero::::new(HIGH_MASK).unwrap(), u32::MAX) - ); - } -} diff --git a/crates/bevy_ecs/src/identifier/mod.rs b/crates/bevy_ecs/src/identifier/mod.rs deleted file mode 100644 index c08ea7b4aa..0000000000 --- a/crates/bevy_ecs/src/identifier/mod.rs +++ /dev/null @@ -1,249 +0,0 @@ -//! 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. -#[cfg(feature = "bevy_reflect")] -use bevy_reflect::Reflect; - -use self::{error::IdentifierError, kinds::IdKind, masks::IdentifierMask}; -use core::{hash::Hash, num::NonZero}; - -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)] -#[cfg_attr(feature = "bevy_reflect", derive(Reflect))] -#[cfg_attr(feature = "bevy_reflect", reflect(opaque))] -#[cfg_attr(feature = "bevy_reflect", reflect(Debug, Hash, PartialEq, Clone))] -// Alignment repr necessary to allow LLVM to better output -// optimized 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: NonZero, - #[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 initialize 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: NonZero::::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) -> NonZero { - 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 initialize 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 = NonZero::::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 optimized 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 optimized 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 optimize 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 optimized 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 optimize the codegen for Entity -// far beyond what the derive macro can. -// See -impl Ord for Identifier { - #[inline] - fn cmp(&self, other: &Self) -> core::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] - #[expect( - clippy::nonminimal_bool, - reason = "This intentionally tests all possible comparison operators as separate functions; thus, we don't want to rewrite these comparisons to use different operators." - )] - fn id_comparison() { - 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 51bf415400..0a2e1862dd 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -39,7 +39,6 @@ pub mod entity_disabling; pub mod error; pub mod event; pub mod hierarchy; -pub mod identifier; pub mod intern; pub mod label; pub mod name; diff --git a/crates/bevy_ecs/src/name.rs b/crates/bevy_ecs/src/name.rs index dd34f5578a..cd2e946678 100644 --- a/crates/bevy_ecs/src/name.rs +++ b/crates/bevy_ecs/src/name.rs @@ -276,7 +276,7 @@ mod tests { let d1 = query.get(&world, e1).unwrap(); let d2 = query.get(&world, e2).unwrap(); // NameOrEntity Display for entities without a Name should be {index}v{generation} - assert_eq!(d1.to_string(), "0v1"); + assert_eq!(d1.to_string(), "0v0"); // NameOrEntity Display for entities with a Name should be the Name assert_eq!(d2.to_string(), "MyName"); } diff --git a/crates/bevy_scene/src/serde.rs b/crates/bevy_scene/src/serde.rs index 9eff75f901..cb8206d3dd 100644 --- a/crates/bevy_scene/src/serde.rs +++ b/crates/bevy_scene/src/serde.rs @@ -638,20 +638,20 @@ mod tests { ), }, entities: { - 8589934589: ( + 4294967293: ( components: { "bevy_scene::serde::tests::Bar": (345), "bevy_scene::serde::tests::Baz": (789), "bevy_scene::serde::tests::Foo": (123), }, ), - 8589934590: ( + 4294967294: ( components: { "bevy_scene::serde::tests::Bar": (345), "bevy_scene::serde::tests::Foo": (123), }, ), - 8589934591: ( + 4294967295: ( components: { "bevy_scene::serde::tests::Foo": (123), }, @@ -815,7 +815,7 @@ mod tests { assert_eq!( vec![ - 0, 1, 255, 255, 255, 255, 31, 1, 37, 98, 101, 118, 121, 95, 115, 99, 101, 110, 101, + 0, 1, 255, 255, 255, 255, 15, 1, 37, 98, 101, 118, 121, 95, 115, 99, 101, 110, 101, 58, 58, 115, 101, 114, 100, 101, 58, 58, 116, 101, 115, 116, 115, 58, 58, 77, 121, 67, 111, 109, 112, 111, 110, 101, 110, 116, 1, 2, 3, 102, 102, 166, 63, 205, 204, 108, 64, 1, 12, 72, 101, 108, 108, 111, 32, 87, 111, 114, 108, 100, 33 @@ -856,12 +856,11 @@ mod tests { assert_eq!( vec![ - 146, 128, 129, 207, 0, 0, 0, 1, 255, 255, 255, 255, 145, 129, 217, 37, 98, 101, - 118, 121, 95, 115, 99, 101, 110, 101, 58, 58, 115, 101, 114, 100, 101, 58, 58, 116, - 101, 115, 116, 115, 58, 58, 77, 121, 67, 111, 109, 112, 111, 110, 101, 110, 116, - 147, 147, 1, 2, 3, 146, 202, 63, 166, 102, 102, 202, 64, 108, 204, 205, 129, 165, - 84, 117, 112, 108, 101, 172, 72, 101, 108, 108, 111, 32, 87, 111, 114, 108, 100, - 33 + 146, 128, 129, 206, 255, 255, 255, 255, 145, 129, 217, 37, 98, 101, 118, 121, 95, + 115, 99, 101, 110, 101, 58, 58, 115, 101, 114, 100, 101, 58, 58, 116, 101, 115, + 116, 115, 58, 58, 77, 121, 67, 111, 109, 112, 111, 110, 101, 110, 116, 147, 147, 1, + 2, 3, 146, 202, 63, 166, 102, 102, 202, 64, 108, 204, 205, 129, 165, 84, 117, 112, + 108, 101, 172, 72, 101, 108, 108, 111, 32, 87, 111, 114, 108, 100, 33 ], buf ); @@ -900,7 +899,7 @@ mod tests { assert_eq!( vec![ - 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 255, 255, 255, 255, 1, 0, 0, 0, 1, + 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 255, 255, 255, 255, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 37, 0, 0, 0, 0, 0, 0, 0, 98, 101, 118, 121, 95, 115, 99, 101, 110, 101, 58, 58, 115, 101, 114, 100, 101, 58, 58, 116, 101, 115, 116, 115, 58, 58, 77, 121, 67, 111, 109, 112, 111, 110, 101, 110, 116, 1, 0, 0, 0, 0, 0, 0, 0, 2, 0, diff --git a/release-content/migration-guides/entity_representation.md b/release-content/migration-guides/entity_representation.md new file mode 100644 index 0000000000..cf7dd8d3d5 --- /dev/null +++ b/release-content/migration-guides/entity_representation.md @@ -0,0 +1,57 @@ +--- +title: Manual Entity Creation and Representation +pull_requests: [18704, 19121] +--- + +An entity is made of two parts: and index and a generation. Both have changes: + +### Index + +`Entity` no longer stores its index as a plain `u32` but as the new `EntityRow`, which wraps a `NonMaxU32`. +Previously, `Entity::index` could be `u32::MAX`, but that is no longer a valid index. +As a result, `Entity::from_raw` now takes `EntityRow` as a parameter instead of `u32`. `EntityRow` can be constructed via `EntityRow::new`, which takes a `NonMaxU32`. +If you don't want to add [nonmax](https://docs.rs/nonmax/latest/nonmax/) as a dependency, use `Entity::from_raw_u32` which is identical to the previous `Entity::from_raw`, except that it now returns `Option` where the result is `None` if `u32::MAX` is passed. + +Bevy made this change because it puts a niche in the `EntityRow` type which makes `Option` half the size of `Option`. +This is used internally to open up performance improvements to the ECS. + +Although you probably shouldn't be making entities manually, it is sometimes useful to do so for tests. +To migrate tests, use: + +```diff +- let entity = Entity::from_raw(1); ++ let entity = Entity::from_raw_u32(1).unwrap(); +``` + +If you are creating entities manually in production, don't do that! +Use `Entities::alloc` instead. +But if you must create one manually, either reuse a `EntityRow` you know to be valid by using `Entity::from_raw` and `Entity::row`, or handle the error case of `None` returning from `Entity::from_raw_u32(my_index)`. + +### Generation + +An entity's generation is no longer a `NonZeroU32`. +Instead, it is an `EntityGeneration`. +Internally, this stores a `u32`, but that might change later. + +Working with the generation directly has never been recommended, but it is sometimes useful to do so in tests. +To create a generation do `EntityGeneration::FIRST.after_versions(expected_generation)`. +To use this in tests, do `assert_eq!(entity.generation(), EntityGeneration::FIRST.after_versions(expected_generation))`. + +### Removed Interfaces + +The `identifier` module and all its contents have been removed. +These features have been slimmed down and rolled into `Entity`. + +This means that where `Result` was returned, `Option` is now returned. + +### Functionality + +It is well documented that both the bit format, serialization, and `Ord` implementations for `Entity` are subject to change between versions. +Those have all changed in this version. + +For entity ordering, the order still prioretizes an entity's generation, but after that, it now considers higher index entities less than lower index entities. + +The changes to serialization and the bit format are directly related. +Effectively, this means that all serialized and transmuted entities will not work as expected and may crash. +To migrate, invert the lower 32 bits of the 64 representation of the entity, and subtract 1 from the upper bits. +Again, this is still subject to change, and serialized scenes may break between versions. diff --git a/release-content/migration-guides/make_entity_index_non_max.md b/release-content/migration-guides/make_entity_index_non_max.md deleted file mode 100644 index d3ba963854..0000000000 --- a/release-content/migration-guides/make_entity_index_non_max.md +++ /dev/null @@ -1,25 +0,0 @@ ---- -title: Manual Entity Creation -pull_requests: [18704] ---- - -`Entity` no longer stores its index as a plain `u32` but as the new `EntityRow`, which wraps a `NonMaxU32`. -Previously, `Entity::index` could be `u32::MAX`, but that is no longer a valid index. -As a result, `Entity::from_raw` now takes `EntityRow` as a parameter instead of `u32`. `EntityRow` can be constructed via `EntityRow::new`, which takes a `NonMaxU32`. -This also means that the `Ord` implementation of `Entity` has changed: the index order is reversed, so 'newer' entities come before 'older' entities. -If you don't want to add [nonmax](https://docs.rs/nonmax/latest/nonmax/) as a dependency, use `Entity::from_raw_u32` which is identical to the previous `Entity::from_raw`, except that it now returns `Option` where the result is `None` if `u32::MAX` is passed. - -Bevy made this change because it puts a niche in the `EntityRow` type which makes `Option` half the size of `Option`. -This is used internally to open up performance improvements to the ECS. - -Although you probably shouldn't be making entities manually, it is sometimes useful to do so for tests. -To migrate tests, use: - -```diff -- let entity = Entity::from_raw(1); -+ let entity = Entity::from_raw_u32(1).unwrap(); -``` - -If you are creating entities manually in production, don't do that! -Use `Entities::alloc` instead. -But if you must create one manually, either reuse a `EntityRow` you know to be valid by using `Entity::from_raw` and `Entity::row`, or handle the error case of `None` returning from `Entity::from_raw_u32(my_index)`.