From e6fc08a05879680d019b90bbb72f19bcecbd0037 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Thu, 3 Apr 2025 11:40:04 -0400 Subject: [PATCH 01/38] added EntityRow --- crates/bevy_ecs/src/entity/mod.rs | 135 ++++++++++++++++++++++++++---- 1 file changed, 117 insertions(+), 18 deletions(-) diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index faa97083ba8f9..8e4f0edf603a4 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -67,6 +67,7 @@ pub mod unique_array; pub mod unique_slice; pub mod unique_vec; +use nonmax::NonMaxU32; pub use unique_array::{UniqueEntityArray, UniqueEntityEquivalentArray}; pub use unique_slice::{UniqueEntityEquivalentSlice, UniqueEntitySlice}; pub use unique_vec::{UniqueEntityEquivalentVec, UniqueEntityVec}; @@ -103,6 +104,84 @@ use bevy_platform_support::sync::atomic::AtomicIsize as AtomicIdCursor; #[cfg(not(target_has_atomic = "64"))] type IdCursor = isize; +/// This represents the row or "index" of an [`Entity`] within the [`Entities`] table. +/// This is a lighter weight version of [`Entity`]. +/// +/// This is a unique identifier for an entity in the world. +/// This differs from [`Entity`] in that [`Entity`] is unique for all entities total (unless the [`Entity::generation`] wraps), +/// but this is only unique for entities that are active. +/// +/// This can be used over [`Entity`] to improve performance in some cases, +/// but improper use can cause this to identify a different entity than intended. +/// Use with caution. +#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)] +#[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 EntityRow(NonMaxU32); + +impl EntityRow { + const PLACEHOLDER: Self = Self(NonMaxU32::MAX); + + /// Constructs a new [`EntityRow`] from its index. + const fn new(index: NonMaxU32) -> Self { + Self(index) + } + + /// Gets the index of the entity. + #[inline(always)] + pub const fn index(&self) -> u32 { + self.0.get() + } + + /// Gets a 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 { + // SAFETY: NonMax is repr transparent. + let underlying: NonZero = unsafe { mem::transmute(self.0) }; + underlying.get() + } + + /// Reconstruct an [`EntityRow`] previously destructured with [`EntityRow::to_bits`]. + /// + /// Only useful when applied to results from `to_bits` in the same instance of an application. + /// + /// # Panics + /// + /// This method will likely panic if given `u32` values that did not come from [`EntityRow::to_bits`]. + #[inline] + pub const fn from_bits(bits: u32) -> Self { + Self::try_from_bits(bits).expect("Attempted to initialize invalid bits as an entity row") + } + + /// Reconstruct an [`EntityRow`] previously destructured with [`EntityRow::to_bits`]. + /// + /// Only useful when applied to results from `to_bits` in the same instance of an application. + /// + /// This method is the fallible counterpart to [`EntityRow::from_bits`]. + #[inline(always)] + pub const fn try_from_bits(bits: u32) -> Option { + match NonMaxU32::new(bits) { + Some(valid) => Some(Self(valid)), + None => None, + } + } +} + +impl SparseSetIndex for EntityRow { + #[inline] + fn sparse_set_index(&self) -> usize { + self.index() as usize + } + + #[inline] + fn get_sparse_set_index(value: usize) -> Self { + Self::from_bits(value as u32) + } +} + /// Lightweight identifier of an [entity](crate::entity). /// /// The identifier is implemented using a [generational index]: a combination of an index and a generation. @@ -186,10 +265,10 @@ pub struct Entity { // 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")] - index: u32, + index: EntityRow, generation: NonZero, #[cfg(target_endian = "big")] - index: u32, + index: EntityRow, } // By not short-circuiting in comparisons, we get better codegen. @@ -256,7 +335,10 @@ impl Entity { /// Construct an [`Entity`] from a raw `index` value and a non-zero `generation` value. /// Ensure that the generation value is never greater than `0x7FFF_FFFF`. #[inline(always)] - pub(crate) const fn from_raw_and_generation(index: u32, generation: NonZero) -> Entity { + pub(crate) const fn from_raw_and_generation( + index: EntityRow, + generation: NonZero, + ) -> Entity { debug_assert!(generation.get() <= HIGH_MASK); Self { index, generation } @@ -296,9 +378,9 @@ impl Entity { /// } /// } /// ``` - pub const PLACEHOLDER: Self = Self::from_raw(u32::MAX); + pub const PLACEHOLDER: Self = Self::from_raw(EntityRow::PLACEHOLDER); - /// Creates a new entity ID with the specified `index` and a generation of 1. + /// Creates a new entity ID with the specified `row` and a generation of 1. /// /// # Note /// @@ -311,8 +393,8 @@ impl Entity { /// `Entity` lines up between instances, but instead insert a secondary identifier as /// a component. #[inline(always)] - pub const fn from_raw(index: u32) -> Entity { - Self::from_raw_and_generation(index, NonZero::::MIN) + pub const fn from_raw(row: EntityRow) -> Entity { + Self::from_raw_and_generation(row, NonZero::::MIN) } /// Convert to a form convenient for passing outside of rust. @@ -323,7 +405,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.index, self.generation.get()) + IdentifierMask::pack_into_u64(self.index.to_bits(), self.generation.get()) } /// Reconstruct an `Entity` previously destructured with [`Entity::to_bits`]. @@ -355,10 +437,12 @@ impl Entity { let kind = id.kind() as u8; if kind == (IdKind::Entity as u8) { - return Ok(Self { - index: id.low(), - generation: id.high(), - }); + if let Some(row) = EntityRow::try_from_bits(id.low()) { + return Ok(Self { + index: row, + generation: id.high(), + }); + } } } @@ -366,15 +450,22 @@ impl Entity { } /// Return a transiently unique identifier. + /// See also [`EntityRow`]. /// - /// No two simultaneously-live entities share the same index, but dead entities' indices may collide + /// No two simultaneously-live entities share the same row, but dead entities' indices may collide /// with both live and dead entities. Useful for compactly representing entities within a /// specific snapshot of the world, such as when serializing. #[inline] - pub const fn index(self) -> u32 { + pub const fn row(self) -> EntityRow { self.index } + /// Equivalent to `self.row().index()`. See [`Self::row`] for details. + #[inline] + pub const fn index(self) -> u32 { + self.index.index() + } + /// Returns the generation of this Entity's index. The generation is incremented each time an /// entity with a given index is despawned. This serves as a "count" of the number of times a /// given index has been reused (index, generation) pairs uniquely identify a given Entity. @@ -480,12 +571,12 @@ impl fmt::Display for Entity { impl SparseSetIndex for Entity { #[inline] fn sparse_set_index(&self) -> usize { - self.index() as usize + self.row().sparse_set_index() } #[inline] fn get_sparse_set_index(value: usize) -> Self { - Entity::from_raw(value as u32) + Entity::from_raw(EntityRow::get_sparse_set_index(value)) } } @@ -508,9 +599,17 @@ impl<'a> Iterator for ReserveEntitiesIterator<'a> { self.freelist_indices .next() .map(|&index| { - Entity::from_raw_and_generation(index, self.meta[index as usize].generation) + // SAFETY: This came from the free list so it must be valid. + let row = unsafe { EntityRow::new(NonMaxU32::new_unchecked(index)) }; + Entity::from_raw_and_generation(row, self.meta[index as usize].generation) + }) + .or_else(|| { + self.new_indices.next().map(|index| { + // SAFETY: This came from an exclusive range so the max can't be hit. + let row = unsafe { EntityRow::new(NonMaxU32::new_unchecked(index)) }; + Entity::from_raw(row) + }) }) - .or_else(|| self.new_indices.next().map(Entity::from_raw)) } fn size_hint(&self) -> (usize, Option) { From 6d67b52aeef45678dabf081e881333829535d801 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Thu, 3 Apr 2025 12:02:43 -0400 Subject: [PATCH 02/38] fixed remaining entity file --- crates/bevy_ecs/src/entity/mod.rs | 240 +++++++++++++++++++++--------- 1 file changed, 169 insertions(+), 71 deletions(-) diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index 8e4f0edf603a4..2910a248ce03f 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -45,6 +45,7 @@ use bevy_reflect::Reflect; use bevy_reflect::{ReflectDeserialize, ReflectSerialize}; pub use clone_entities::*; +use derive_more::derive::Display; pub use entity_set::*; pub use map_entities::*; @@ -114,7 +115,7 @@ type IdCursor = isize; /// This can be used over [`Entity`] to improve performance in some cases, /// but improper use can cause this to identify a different entity than intended. /// Use with caution. -#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)] +#[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))] @@ -586,7 +587,7 @@ pub struct ReserveEntitiesIterator<'a> { meta: &'a [EntityMeta], // Reserved indices formerly in the freelist to hand out. - freelist_indices: core::slice::Iter<'a, u32>, + freelist_indices: core::slice::Iter<'a, EntityRow>, // New Entity indices to hand out, outside the range of meta.len(). new_indices: core::ops::Range, @@ -598,10 +599,8 @@ impl<'a> Iterator for ReserveEntitiesIterator<'a> { fn next(&mut self) -> Option { self.freelist_indices .next() - .map(|&index| { - // SAFETY: This came from the free list so it must be valid. - let row = unsafe { EntityRow::new(NonMaxU32::new_unchecked(index)) }; - Entity::from_raw_and_generation(row, self.meta[index as usize].generation) + .map(|&row| { + Entity::from_raw_and_generation(row, self.meta[row.index() as usize].generation) }) .or_else(|| { self.new_indices.next().map(|index| { @@ -676,7 +675,7 @@ pub struct Entities { /// [`reserve_entity`]: Entities::reserve_entity /// [`reserve_entities`]: Entities::reserve_entities /// [`flush`]: Entities::flush - pending: Vec, + pending: Vec, free_cursor: AtomicIdCursor, } @@ -752,17 +751,21 @@ impl Entities { let n = self.free_cursor.fetch_sub(1, Ordering::Relaxed); if n > 0 { // Allocate from the freelist. - let index = self.pending[(n - 1) as usize]; - Entity::from_raw_and_generation(index, self.meta[index as usize].generation) + let row = self.pending[(n - 1) as usize]; + Entity::from_raw_and_generation(row, self.meta[row.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::from_raw( - u32::try_from(self.meta.len() as IdCursor - n).expect("too many entities"), - ) + let raw = self.meta.len() as IdCursor - n; + if raw >= u32::MAX as IdCursor { + panic!("too many entities"); + } + // SAFETY: We just checked the bounds + let row = unsafe { EntityRow::new(NonMaxU32::new_unchecked(raw as u32)) }; + Entity::from_raw(row) } } @@ -777,14 +780,17 @@ impl Entities { /// Allocate an entity ID directly. pub fn alloc(&mut self) -> Entity { self.verify_flushed(); - if let Some(index) = self.pending.pop() { + if let Some(row) = self.pending.pop() { let new_free_cursor = self.pending.len() as IdCursor; *self.free_cursor.get_mut() = new_free_cursor; - Entity::from_raw_and_generation(index, self.meta[index as usize].generation) + Entity::from_raw_and_generation(row, self.meta[row.index() as usize].generation) } else { - let index = u32::try_from(self.meta.len()).expect("too many entities"); + let index = u32::try_from(self.meta.len()) + .ok() + .and_then(NonMaxU32::new) + .expect("too many entities"); self.meta.push(EntityMeta::EMPTY); - Entity::from_raw(index) + Entity::from_raw(EntityRow::new(index)) } } @@ -799,14 +805,17 @@ impl Entities { self.verify_flushed(); let loc = if entity.index() as usize >= self.meta.len() { - self.pending - .extend((self.meta.len() as u32)..entity.index()); + self.pending.extend( + ((self.meta.len() as u32)..entity.index()) + // SAFETY: The upper bound is a valid index + .map(|index| EntityRow::new(unsafe { NonMaxU32::new_unchecked(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); 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.row()) { self.pending.swap_remove(index); let new_free_cursor = self.pending.len() as IdCursor; *self.free_cursor.get_mut() = new_free_cursor; @@ -840,14 +849,17 @@ impl Entities { self.verify_flushed(); let result = if entity.index() as usize >= self.meta.len() { - self.pending - .extend((self.meta.len() as u32)..entity.index()); + self.pending.extend( + ((self.meta.len() as u32)..entity.index()) + // SAFETY: The upper bound is a valid index + .map(|index| EntityRow::new(unsafe { NonMaxU32::new_unchecked(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); 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.row()) { self.pending.swap_remove(index); let new_free_cursor = self.pending.len() as IdCursor; *self.free_cursor.get_mut() = new_free_cursor; @@ -883,13 +895,13 @@ impl Entities { if meta.generation == NonZero::::MIN { warn!( "Entity({}) generation wrapped on Entities::free, aliasing may occur", - entity.index + entity.row() ); } let loc = mem::replace(&mut meta.location, EntityMeta::EMPTY.location); - self.pending.push(entity.index()); + self.pending.push(entity.row()); let new_free_cursor = self.pending.len() as IdCursor; *self.free_cursor.get_mut() = new_free_cursor; @@ -921,7 +933,7 @@ impl Entities { // This will return false for entities which have been freed, even if // not reallocated since the generation is incremented in `free` pub fn contains(&self, entity: Entity) -> bool { - self.resolve_from_id(entity.index()) + self.resolve_from_id(entity.row()) .is_some_and(|e| e.generation() == entity.generation()) } @@ -987,17 +999,17 @@ impl Entities { /// Note: This method may return [`Entities`](Entity) which are currently free /// Note that [`contains`](Entities::contains) will correctly return false for freed /// entities, since it checks the generation - pub fn resolve_from_id(&self, index: u32) -> Option { - let idu = index as usize; + pub fn resolve_from_id(&self, row: EntityRow) -> Option { + let idu = row.index() as usize; if let Some(&EntityMeta { generation, .. }) = self.meta.get(idu) { - Some(Entity::from_raw_and_generation(index, generation)) + Some(Entity::from_raw_and_generation(row, 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::from_raw(index)) + (idu < self.meta.len() + num_pending).then_some(Entity::from_raw(row)) } } @@ -1026,8 +1038,10 @@ impl Entities { let new_meta_len = old_meta_len + -current_free_cursor as usize; self.meta.resize(new_meta_len, EntityMeta::EMPTY); for (index, meta) in self.meta.iter_mut().enumerate().skip(old_meta_len) { + // SAFETY: the index is less than the meta length, which can not exceeded u32::MAX + let row = EntityRow::new(unsafe { NonMaxU32::new_unchecked(index as u32) }); init( - Entity::from_raw_and_generation(index as u32, meta.generation), + Entity::from_raw_and_generation(row, meta.generation), &mut meta.location, ); } @@ -1036,10 +1050,10 @@ impl Entities { 0 }; - for index in self.pending.drain(new_free_cursor..) { - let meta = &mut self.meta[index as usize]; + for row in self.pending.drain(new_free_cursor..) { + let meta = &mut self.meta[row.index() as usize]; init( - Entity::from_raw_and_generation(index, meta.generation), + Entity::from_raw_and_generation(row, meta.generation), &mut meta.location, ); } @@ -1252,8 +1266,10 @@ mod tests { #[test] fn entity_bits_roundtrip() { // Generation cannot be greater than 0x7FFF_FFFF else it will be an invalid Entity id - let e = - Entity::from_raw_and_generation(0xDEADBEEF, NonZero::::new(0x5AADF00D).unwrap()); + let e = Entity::from_raw_and_generation( + EntityRow::new(NonMaxU32::new(0xDEADBEEF).unwrap()), + NonZero::::new(0x5AADF00D).unwrap(), + ); assert_eq!(Entity::from_bits(e.to_bits()), e); } @@ -1286,7 +1302,7 @@ mod tests { #[test] fn entity_const() { - const C1: Entity = Entity::from_raw(42); + const C1: Entity = Entity::from_raw(EntityRow::new(NonMaxU32::new(42).unwrap())); assert_eq!(42, C1.index()); assert_eq!(1, C1.generation()); @@ -1294,7 +1310,7 @@ mod tests { assert_eq!(0x0000_00cc, C2.index()); assert_eq!(0x0000_00ff, C2.generation()); - const C3: u32 = Entity::from_raw(33).index(); + 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_0000_0000).generation(); @@ -1333,65 +1349,139 @@ mod tests { )] fn entity_comparison() { assert_eq!( - Entity::from_raw_and_generation(123, NonZero::::new(456).unwrap()), - Entity::from_raw_and_generation(123, NonZero::::new(456).unwrap()) + Entity::from_raw_and_generation( + EntityRow::new(NonMaxU32::new(123).unwrap()), + NonZero::::new(456).unwrap() + ), + Entity::from_raw_and_generation( + EntityRow::new(NonMaxU32::new(123).unwrap()), + NonZero::::new(456).unwrap() + ) ); assert_ne!( - Entity::from_raw_and_generation(123, NonZero::::new(789).unwrap()), - Entity::from_raw_and_generation(123, NonZero::::new(456).unwrap()) + Entity::from_raw_and_generation( + EntityRow::new(NonMaxU32::new(123).unwrap()), + NonZero::::new(789).unwrap() + ), + Entity::from_raw_and_generation( + EntityRow::new(NonMaxU32::new(123).unwrap()), + NonZero::::new(456).unwrap() + ) ); assert_ne!( - Entity::from_raw_and_generation(123, NonZero::::new(456).unwrap()), - Entity::from_raw_and_generation(123, NonZero::::new(789).unwrap()) + Entity::from_raw_and_generation( + EntityRow::new(NonMaxU32::new(123).unwrap()), + NonZero::::new(456).unwrap() + ), + Entity::from_raw_and_generation( + EntityRow::new(NonMaxU32::new(123).unwrap()), + NonZero::::new(789).unwrap() + ) ); assert_ne!( - Entity::from_raw_and_generation(123, NonZero::::new(456).unwrap()), - Entity::from_raw_and_generation(456, NonZero::::new(123).unwrap()) + Entity::from_raw_and_generation( + EntityRow::new(NonMaxU32::new(123).unwrap()), + NonZero::::new(456).unwrap() + ), + Entity::from_raw_and_generation( + EntityRow::new(NonMaxU32::new(456).unwrap()), + NonZero::::new(123).unwrap() + ) ); // ordering is by generation then by index assert!( - Entity::from_raw_and_generation(123, NonZero::::new(456).unwrap()) - >= Entity::from_raw_and_generation(123, NonZero::::new(456).unwrap()) + Entity::from_raw_and_generation( + EntityRow::new(NonMaxU32::new(123).unwrap()), + NonZero::::new(456).unwrap() + ) >= Entity::from_raw_and_generation( + EntityRow::new(NonMaxU32::new(123).unwrap()), + NonZero::::new(456).unwrap() + ) ); assert!( - Entity::from_raw_and_generation(123, NonZero::::new(456).unwrap()) - <= Entity::from_raw_and_generation(123, NonZero::::new(456).unwrap()) + Entity::from_raw_and_generation( + EntityRow::new(NonMaxU32::new(123).unwrap()), + NonZero::::new(456).unwrap() + ) <= Entity::from_raw_and_generation( + EntityRow::new(NonMaxU32::new(123).unwrap()), + NonZero::::new(456).unwrap() + ) ); assert!( - !(Entity::from_raw_and_generation(123, NonZero::::new(456).unwrap()) - < Entity::from_raw_and_generation(123, NonZero::::new(456).unwrap())) + !(Entity::from_raw_and_generation( + EntityRow::new(NonMaxU32::new(123).unwrap()), + NonZero::::new(456).unwrap() + ) < Entity::from_raw_and_generation( + EntityRow::new(NonMaxU32::new(123).unwrap()), + NonZero::::new(456).unwrap() + )) ); assert!( - !(Entity::from_raw_and_generation(123, NonZero::::new(456).unwrap()) - > Entity::from_raw_and_generation(123, NonZero::::new(456).unwrap())) + !(Entity::from_raw_and_generation( + EntityRow::new(NonMaxU32::new(123).unwrap()), + NonZero::::new(456).unwrap() + ) > Entity::from_raw_and_generation( + EntityRow::new(NonMaxU32::new(123).unwrap()), + NonZero::::new(456).unwrap() + )) ); assert!( - Entity::from_raw_and_generation(9, NonZero::::new(1).unwrap()) - < Entity::from_raw_and_generation(1, NonZero::::new(9).unwrap()) + Entity::from_raw_and_generation( + EntityRow::new(NonMaxU32::new(9).unwrap()), + NonZero::::new(1).unwrap() + ) < Entity::from_raw_and_generation( + EntityRow::new(NonMaxU32::new(1).unwrap()), + NonZero::::new(9).unwrap() + ) ); assert!( - Entity::from_raw_and_generation(1, NonZero::::new(9).unwrap()) - > Entity::from_raw_and_generation(9, NonZero::::new(1).unwrap()) + Entity::from_raw_and_generation( + EntityRow::new(NonMaxU32::new(1).unwrap()), + NonZero::::new(9).unwrap() + ) > Entity::from_raw_and_generation( + EntityRow::new(NonMaxU32::new(9).unwrap()), + NonZero::::new(1).unwrap() + ) ); assert!( - Entity::from_raw_and_generation(1, NonZero::::new(1).unwrap()) - < Entity::from_raw_and_generation(2, NonZero::::new(1).unwrap()) + Entity::from_raw_and_generation( + EntityRow::new(NonMaxU32::new(1).unwrap()), + NonZero::::new(1).unwrap() + ) < Entity::from_raw_and_generation( + EntityRow::new(NonMaxU32::new(2).unwrap()), + NonZero::::new(1).unwrap() + ) ); assert!( - Entity::from_raw_and_generation(1, NonZero::::new(1).unwrap()) - <= Entity::from_raw_and_generation(2, NonZero::::new(1).unwrap()) + Entity::from_raw_and_generation( + EntityRow::new(NonMaxU32::new(1).unwrap()), + NonZero::::new(1).unwrap() + ) <= Entity::from_raw_and_generation( + EntityRow::new(NonMaxU32::new(2).unwrap()), + NonZero::::new(1).unwrap() + ) ); assert!( - Entity::from_raw_and_generation(2, NonZero::::new(2).unwrap()) - > Entity::from_raw_and_generation(1, NonZero::::new(2).unwrap()) + Entity::from_raw_and_generation( + EntityRow::new(NonMaxU32::new(2).unwrap()), + NonZero::::new(2).unwrap() + ) > Entity::from_raw_and_generation( + EntityRow::new(NonMaxU32::new(1).unwrap()), + NonZero::::new(2).unwrap() + ) ); assert!( - Entity::from_raw_and_generation(2, NonZero::::new(2).unwrap()) - >= Entity::from_raw_and_generation(1, NonZero::::new(2).unwrap()) + Entity::from_raw_and_generation( + EntityRow::new(NonMaxU32::new(2).unwrap()), + NonZero::::new(2).unwrap() + ) >= Entity::from_raw_and_generation( + EntityRow::new(NonMaxU32::new(1).unwrap()), + NonZero::::new(2).unwrap() + ) ); } @@ -1403,11 +1493,15 @@ mod tests { let hash = EntityHash; let first_id = 0xC0FFEE << 8; - let first_hash = hash.hash_one(Entity::from_raw(first_id)); + let first_hash = hash.hash_one(Entity::from_raw(EntityRow::new( + NonMaxU32::new(first_id).unwrap(), + ))); for i in 1..=255 { let id = first_id + i; - let hash = hash.hash_one(Entity::from_raw(id)); + let hash = hash.hash_one(Entity::from_raw(EntityRow::new( + NonMaxU32::new(id).unwrap(), + ))); assert_eq!(hash.wrapping_sub(first_hash) as u32, i); } } @@ -1419,18 +1513,22 @@ mod tests { let hash = EntityHash; let first_id = 0xC0FFEE; - let first_hash = hash.hash_one(Entity::from_raw(first_id)) >> 57; + let first_hash = hash.hash_one(Entity::from_raw(EntityRow::new( + NonMaxU32::new(first_id).unwrap(), + ))) >> 57; for bit in 0..u32::BITS { let id = first_id ^ (1 << bit); - let hash = hash.hash_one(Entity::from_raw(id)) >> 57; + let hash = hash.hash_one(Entity::from_raw(EntityRow::new( + NonMaxU32::new(id).unwrap(), + ))) >> 57; assert_ne!(hash, first_hash); } } #[test] fn entity_debug() { - let entity = Entity::from_raw(42); + let entity = Entity::from_raw(EntityRow::new(NonMaxU32::new(42).unwrap())); let string = format!("{:?}", entity); assert_eq!(string, "42v1#4294967338"); @@ -1441,7 +1539,7 @@ mod tests { #[test] fn entity_display() { - let entity = Entity::from_raw(42); + let entity = Entity::from_raw(EntityRow::new(NonMaxU32::new(42).unwrap())); let string = format!("{}", entity); assert_eq!(string, "42v1"); From ef11fb45e56b5bb5b347a4b8f4caf9c3a34d7183 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Thu, 3 Apr 2025 12:10:50 -0400 Subject: [PATCH 03/38] fixed more tests --- crates/bevy_ecs/src/entity/map_entities.rs | 14 ++++++++------ crates/bevy_ecs/src/entity/mod.rs | 6 +++--- crates/bevy_ecs/src/lib.rs | 18 +++++++++--------- 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/crates/bevy_ecs/src/entity/map_entities.rs b/crates/bevy_ecs/src/entity/map_entities.rs index 10da6cc559bfc..946a204d399f3 100644 --- a/crates/bevy_ecs/src/entity/map_entities.rs +++ b/crates/bevy_ecs/src/entity/map_entities.rs @@ -120,7 +120,7 @@ impl> MapEntities for SmallVec { /// fn get_mapped(&mut self, entity: Entity) -> Entity { /// self.map.get(&entity).copied().unwrap_or(entity) /// } -/// +/// /// fn set_mapped(&mut self, source: Entity, target: Entity) { /// self.map.insert(source, target); /// } @@ -177,7 +177,7 @@ 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.index(), + self.dead_start.row(), IdentifierMask::inc_masked_high_by(self.dead_start.generation, self.generations), ); @@ -280,15 +280,17 @@ impl<'m> SceneEntityMapper<'m> { #[cfg(test)] mod tests { + use nonmax::NonMaxU32; + use crate::{ - entity::{Entity, EntityHashMap, EntityMapper, SceneEntityMapper}, + entity::{Entity, EntityHashMap, EntityMapper, EntityRow, SceneEntityMapper}, world::World, }; #[test] fn entity_mapper() { - const FIRST_IDX: u32 = 1; - const SECOND_IDX: u32 = 2; + const FIRST_IDX: EntityRow = EntityRow::new(NonMaxU32::new(1).unwrap()); + const SECOND_IDX: EntityRow = EntityRow::new(NonMaxU32::new(2).unwrap()); let mut map = EntityHashMap::default(); let mut world = World::new(); @@ -321,7 +323,7 @@ mod tests { let mut world = World::new(); let dead_ref = SceneEntityMapper::world_scope(&mut map, &mut world, |_, mapper| { - mapper.get_mapped(Entity::from_raw(0)) + mapper.get_mapped(Entity::from_raw(EntityRow::new(NonMaxU32::ZERO))) }); // Next allocated entity should be a further generation on the same index diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index 2910a248ce03f..db86475556c46 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -126,20 +126,20 @@ impl EntityRow { const PLACEHOLDER: Self = Self(NonMaxU32::MAX); /// Constructs a new [`EntityRow`] from its index. - const fn new(index: NonMaxU32) -> Self { + pub(crate) const fn new(index: NonMaxU32) -> Self { Self(index) } /// Gets the index of the entity. #[inline(always)] - pub const fn index(&self) -> u32 { + pub const fn index(self) -> u32 { self.0.get() } /// Gets a 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 { + const fn to_bits(self) -> u32 { // SAFETY: NonMax is repr transparent. let underlying: NonZero = unsafe { mem::transmute(self.0) }; underlying.get() diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 79ba938f4b749..c2c202dde682c 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -133,7 +133,7 @@ mod tests { bundle::Bundle, change_detection::Ref, component::{Component, ComponentId, RequiredComponents, RequiredComponentsError}, - entity::{Entity, EntityMapper}, + entity::{Entity, EntityMapper, EntityRow}, entity_disabling::DefaultQueryFilters, prelude::Or, query::{Added, Changed, FilteredAccess, QueryFilter, With, Without}, @@ -154,6 +154,7 @@ mod tests { num::NonZero, sync::atomic::{AtomicUsize, Ordering}, }; + use nonmax::NonMaxU32; use std::sync::Mutex; #[derive(Component, Resource, Debug, PartialEq, Eq, Hash, Clone, Copy)] @@ -1542,8 +1543,8 @@ mod tests { let mut world_a = World::new(); let world_b = World::new(); let mut query = world_a.query::<&A>(); - let _ = query.get(&world_a, Entity::from_raw(0)); - let _ = query.get(&world_b, Entity::from_raw(0)); + let _ = query.get(&world_a, Entity::from_raw(EntityRow::new(NonMaxU32::ZERO))); + let _ = query.get(&world_b, Entity::from_raw(EntityRow::new(NonMaxU32::ZERO))); } #[test] @@ -1700,7 +1701,7 @@ mod tests { fn insert_or_spawn_batch() { let mut world = World::default(); let e0 = world.spawn(A(0)).id(); - let e1 = Entity::from_raw(1); + let e1 = Entity::from_raw(EntityRow::new(NonMaxU32::ONE)); let values = vec![(e0, (B(0), C)), (e1, (B(1), C))]; @@ -1741,10 +1742,9 @@ mod tests { fn insert_or_spawn_batch_invalid() { let mut world = World::default(); let e0 = world.spawn(A(0)).id(); - let e1 = Entity::from_raw(1); + let e1 = Entity::from_raw(EntityRow::new(NonMaxU32::ONE)); let e2 = world.spawn_empty().id(); - let invalid_e2 = - Entity::from_raw_and_generation(e2.index(), NonZero::::new(2).unwrap()); + let invalid_e2 = Entity::from_raw_and_generation(e2.row(), NonZero::::new(2).unwrap()); let values = vec![(e0, (B(0), C)), (e1, (B(1), C)), (invalid_e2, (B(2), C))]; @@ -1875,7 +1875,7 @@ mod tests { fn try_insert_batch() { let mut world = World::default(); let e0 = world.spawn(A(0)).id(); - let e1 = Entity::from_raw(1); + let e1 = Entity::from_raw(EntityRow::new(NonMaxU32::ONE)); let values = vec![(e0, (A(1), B(0))), (e1, (A(0), B(1)))]; @@ -1899,7 +1899,7 @@ mod tests { fn try_insert_batch_if_new() { let mut world = World::default(); let e0 = world.spawn(A(0)).id(); - let e1 = Entity::from_raw(1); + let e1 = Entity::from_raw(EntityRow::new(NonMaxU32::ONE)); let values = vec![(e0, (A(1), B(0))), (e1, (A(0), B(1)))]; From 88aae534818f3f4fd15f77369ea8a464a032aea1 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Thu, 3 Apr 2025 12:28:53 -0400 Subject: [PATCH 04/38] fixed remaining tests --- benches/Cargo.toml | 1 + benches/benches/bevy_ecs/world/world_get.rs | 32 +++++++++++++++------ crates/bevy_ecs/src/entity/map_entities.rs | 2 +- crates/bevy_ecs/src/entity/mod.rs | 11 ++++++- crates/bevy_ecs/src/lib.rs | 13 ++++----- crates/bevy_ecs/src/query/state.rs | 5 ++-- crates/bevy_ecs/src/storage/sparse_set.rs | 13 +++++---- crates/bevy_ecs/src/storage/table/mod.rs | 7 +++-- crates/bevy_remote/src/builtin_methods.rs | 6 ++-- crates/bevy_transform/src/systems.rs | 8 +++--- crates/bevy_ui/src/layout/mod.rs | 3 +- crates/bevy_ui/src/layout/ui_surface.rs | 15 +++++----- 12 files changed, 75 insertions(+), 41 deletions(-) diff --git a/benches/Cargo.toml b/benches/Cargo.toml index f5faeec237fc3..6a41ea9cb00dd 100644 --- a/benches/Cargo.toml +++ b/benches/Cargo.toml @@ -32,6 +32,7 @@ bevy_platform_support = { path = "../crates/bevy_platform_support", default-feat glam = "0.29" rand = "0.8" rand_chacha = "0.3" +nonmax = { version = "0.5", default-features = false } # Make `bevy_render` compile on Linux with x11 windowing. x11 vs. Wayland does not matter here # because the benches do not actually open any windows. diff --git a/benches/benches/bevy_ecs/world/world_get.rs b/benches/benches/bevy_ecs/world/world_get.rs index 283b984186150..e6e2a0bb903ef 100644 --- a/benches/benches/bevy_ecs/world/world_get.rs +++ b/benches/benches/bevy_ecs/world/world_get.rs @@ -1,9 +1,10 @@ use core::hint::black_box; +use nonmax::NonMaxU32; use bevy_ecs::{ bundle::{Bundle, NoBundleEffect}, component::Component, - entity::Entity, + entity::{Entity, EntityRow}, system::{Query, SystemState}, world::World, }; @@ -53,7 +54,9 @@ pub fn world_entity(criterion: &mut Criterion) { bencher.iter(|| { for i in 0..entity_count { - let entity = Entity::from_raw(i); + let entity = + // SAFETY: Range is exclusive. + Entity::from_raw(EntityRow::new(unsafe { NonMaxU32::new_unchecked(i) })); black_box(world.entity(entity)); } }); @@ -74,7 +77,9 @@ pub fn world_get(criterion: &mut Criterion) { bencher.iter(|| { for i in 0..entity_count { - let entity = Entity::from_raw(i); + let entity = + // SAFETY: Range is exclusive. + Entity::from_raw(EntityRow::new(unsafe { NonMaxU32::new_unchecked(i) })); assert!(world.get::(entity).is_some()); } }); @@ -84,7 +89,9 @@ pub fn world_get(criterion: &mut Criterion) { bencher.iter(|| { for i in 0..entity_count { - let entity = Entity::from_raw(i); + let entity = + // SAFETY: Range is exclusive. + Entity::from_raw(EntityRow::new(unsafe { NonMaxU32::new_unchecked(i) })); assert!(world.get::(entity).is_some()); } }); @@ -106,7 +113,9 @@ pub fn world_query_get(criterion: &mut Criterion) { bencher.iter(|| { for i in 0..entity_count { - let entity = Entity::from_raw(i); + let entity = + // SAFETY: Range is exclusive. + Entity::from_raw(EntityRow::new(unsafe { NonMaxU32::new_unchecked(i) })); assert!(query.get(&world, entity).is_ok()); } }); @@ -131,7 +140,9 @@ pub fn world_query_get(criterion: &mut Criterion) { bencher.iter(|| { for i in 0..entity_count { - let entity = Entity::from_raw(i); + let entity = + // SAFETY: Range is exclusive. + Entity::from_raw(EntityRow::new(unsafe { NonMaxU32::new_unchecked(i) })); assert!(query.get(&world, entity).is_ok()); } }); @@ -142,7 +153,9 @@ pub fn world_query_get(criterion: &mut Criterion) { bencher.iter(|| { for i in 0..entity_count { - let entity = Entity::from_raw(i); + let entity = + // SAFETY: Range is exclusive. + Entity::from_raw(EntityRow::new(unsafe { NonMaxU32::new_unchecked(i) })); assert!(query.get(&world, entity).is_ok()); } }); @@ -169,7 +182,10 @@ pub fn world_query_get(criterion: &mut Criterion) { bencher.iter(|| { for i in 0..entity_count { - let entity = Entity::from_raw(i); + // SAFETY: Range is exclusive. + let entity = Entity::from_raw(EntityRow::new(unsafe { + NonMaxU32::new_unchecked(i) + })); assert!(query.get(&world, entity).is_ok()); } }); diff --git a/crates/bevy_ecs/src/entity/map_entities.rs b/crates/bevy_ecs/src/entity/map_entities.rs index 946a204d399f3..2ef6a78caf182 100644 --- a/crates/bevy_ecs/src/entity/map_entities.rs +++ b/crates/bevy_ecs/src/entity/map_entities.rs @@ -323,7 +323,7 @@ mod tests { let mut world = World::new(); let dead_ref = SceneEntityMapper::world_scope(&mut map, &mut world, |_, mapper| { - mapper.get_mapped(Entity::from_raw(EntityRow::new(NonMaxU32::ZERO))) + mapper.get_mapped(Entity::from_raw(EntityRow::INDEX_ZERO)) }); // Next allocated entity should be a further generation on the same index diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index db86475556c46..8c46110166828 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -125,8 +125,17 @@ pub struct EntityRow(NonMaxU32); impl EntityRow { const PLACEHOLDER: Self = Self(NonMaxU32::MAX); + /// The row at index zero. This exists to make tests easier, but should never be used in an practice. + pub const INDEX_ZERO: Self = Self(NonMaxU32::ZERO); + + /// The row at index one. This exists to make tests easier, but should never be used in an practice. + pub const INDEX_ONE: Self = Self(NonMaxU32::ONE); + + /// The row at index one. This exists to make tests easier, but should never be used in an practice. + pub const INDEX_TWO: Self = Self(NonMaxU32::new(2).unwrap()); + /// Constructs a new [`EntityRow`] from its index. - pub(crate) const fn new(index: NonMaxU32) -> Self { + pub const fn new(index: NonMaxU32) -> Self { Self(index) } diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index c2c202dde682c..33b6981c3101c 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -154,7 +154,6 @@ mod tests { num::NonZero, sync::atomic::{AtomicUsize, Ordering}, }; - use nonmax::NonMaxU32; use std::sync::Mutex; #[derive(Component, Resource, Debug, PartialEq, Eq, Hash, Clone, Copy)] @@ -1543,8 +1542,8 @@ mod tests { let mut world_a = World::new(); let world_b = World::new(); let mut query = world_a.query::<&A>(); - let _ = query.get(&world_a, Entity::from_raw(EntityRow::new(NonMaxU32::ZERO))); - let _ = query.get(&world_b, Entity::from_raw(EntityRow::new(NonMaxU32::ZERO))); + let _ = query.get(&world_a, Entity::from_raw(EntityRow::INDEX_ZERO)); + let _ = query.get(&world_b, Entity::from_raw(EntityRow::INDEX_ZERO)); } #[test] @@ -1701,7 +1700,7 @@ mod tests { fn insert_or_spawn_batch() { let mut world = World::default(); let e0 = world.spawn(A(0)).id(); - let e1 = Entity::from_raw(EntityRow::new(NonMaxU32::ONE)); + let e1 = Entity::from_raw(EntityRow::INDEX_ONE); let values = vec![(e0, (B(0), C)), (e1, (B(1), C))]; @@ -1742,7 +1741,7 @@ mod tests { fn insert_or_spawn_batch_invalid() { let mut world = World::default(); let e0 = world.spawn(A(0)).id(); - let e1 = Entity::from_raw(EntityRow::new(NonMaxU32::ONE)); + let e1 = Entity::from_raw(EntityRow::INDEX_ONE); let e2 = world.spawn_empty().id(); let invalid_e2 = Entity::from_raw_and_generation(e2.row(), NonZero::::new(2).unwrap()); @@ -1875,7 +1874,7 @@ mod tests { fn try_insert_batch() { let mut world = World::default(); let e0 = world.spawn(A(0)).id(); - let e1 = Entity::from_raw(EntityRow::new(NonMaxU32::ONE)); + let e1 = Entity::from_raw(EntityRow::INDEX_ONE); let values = vec![(e0, (A(1), B(0))), (e1, (A(0), B(1)))]; @@ -1899,7 +1898,7 @@ mod tests { fn try_insert_batch_if_new() { let mut world = World::default(); let e0 = world.spawn(A(0)).id(); - let e1 = Entity::from_raw(EntityRow::new(NonMaxU32::ONE)); + let e1 = Entity::from_raw(EntityRow::INDEX_ONE); let values = vec![(e0, (A(1), B(0))), (e1, (A(0), B(1)))]; diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index e9a00f4646e1f..ce4015db9599e 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -1773,7 +1773,7 @@ impl QueryState { /// /// fn my_system(query: Query<&A>) -> Result { /// let a = query.single()?; - /// + /// /// // Do something with `a` /// Ok(()) /// } @@ -1881,6 +1881,7 @@ impl From> for QueryState(); - let _panics = query_state.get(&world_2, Entity::from_raw(0)); + let _panics = query_state.get(&world_2, Entity::from_raw(EntityRow::INDEX_ZERO)); } #[test] diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs index bb79382e06a8d..31ccb2dcbb05f 100644 --- a/crates/bevy_ecs/src/storage/sparse_set.rs +++ b/crates/bevy_ecs/src/storage/sparse_set.rs @@ -652,10 +652,11 @@ mod tests { use super::SparseSets; use crate::{ component::{Component, ComponentDescriptor, ComponentId, ComponentInfo}, - entity::Entity, + entity::{Entity, EntityRow}, storage::SparseSet, }; use alloc::{vec, vec::Vec}; + use nonmax::NonMaxU32; #[derive(Debug, Eq, PartialEq)] struct Foo(usize); @@ -663,11 +664,11 @@ mod tests { #[test] fn sparse_set() { let mut set = SparseSet::::default(); - let e0 = Entity::from_raw(0); - let e1 = Entity::from_raw(1); - let e2 = Entity::from_raw(2); - let e3 = Entity::from_raw(3); - let e4 = Entity::from_raw(4); + let e0 = Entity::from_raw(EntityRow::new(NonMaxU32::new(0).unwrap())); + let e1 = Entity::from_raw(EntityRow::new(NonMaxU32::new(1).unwrap())); + let e2 = Entity::from_raw(EntityRow::new(NonMaxU32::new(2).unwrap())); + let e3 = Entity::from_raw(EntityRow::new(NonMaxU32::new(3).unwrap())); + let e4 = Entity::from_raw(EntityRow::new(NonMaxU32::new(4).unwrap())); set.insert(e1, Foo(1)); set.insert(e2, Foo(2)); diff --git a/crates/bevy_ecs/src/storage/table/mod.rs b/crates/bevy_ecs/src/storage/table/mod.rs index d211bc10dae68..3c2c1f987d15c 100644 --- a/crates/bevy_ecs/src/storage/table/mod.rs +++ b/crates/bevy_ecs/src/storage/table/mod.rs @@ -823,11 +823,12 @@ mod tests { use crate::{ change_detection::MaybeLocation, component::{Component, ComponentIds, Components, ComponentsRegistrator, Tick}, - entity::Entity, + entity::{Entity, EntityRow}, ptr::OwningPtr, storage::{TableBuilder, TableId, TableRow, Tables}, }; use alloc::vec::Vec; + use nonmax::NonMaxU32; #[derive(Component)] struct W(T); @@ -856,7 +857,9 @@ mod tests { let mut table = TableBuilder::with_capacity(0, columns.len()) .add_column(components.get_info(component_id).unwrap()) .build(); - let entities = (0..200).map(Entity::from_raw).collect::>(); + let entities = (0..200) + .map(|index| Entity::from_raw(EntityRow::new(NonMaxU32::new(index).unwrap()))) + .collect::>(); for entity in &entities { // SAFETY: we allocate and immediately set data afterwards unsafe { diff --git a/crates/bevy_remote/src/builtin_methods.rs b/crates/bevy_remote/src/builtin_methods.rs index e8b6856623cff..043f6d283a3df 100644 --- a/crates/bevy_remote/src/builtin_methods.rs +++ b/crates/bevy_remote/src/builtin_methods.rs @@ -1490,13 +1490,15 @@ mod tests { "Deserialized value does not match original" ); } + use bevy_ecs::entity::EntityRow; + use super::*; #[test] fn serialization_tests() { test_serialize_deserialize(BrpQueryRow { components: Default::default(), - entity: Entity::from_raw(0), + entity: Entity::from_raw(EntityRow::INDEX_ZERO), has: Default::default(), }); test_serialize_deserialize(BrpListWatchingResponse::default()); @@ -1510,7 +1512,7 @@ mod tests { ..Default::default() }); test_serialize_deserialize(BrpListParams { - entity: Entity::from_raw(0), + entity: Entity::from_raw(EntityRow::INDEX_ZERO), }); } } diff --git a/crates/bevy_transform/src/systems.rs b/crates/bevy_transform/src/systems.rs index ecf66512715e9..3a122c6cc2b0e 100644 --- a/crates/bevy_transform/src/systems.rs +++ b/crates/bevy_transform/src/systems.rs @@ -557,7 +557,7 @@ mod parallel { mod test { use alloc::{vec, vec::Vec}; use bevy_app::prelude::*; - use bevy_ecs::{prelude::*, world::CommandQueue}; + use bevy_ecs::{entity::EntityRow, prelude::*, world::CommandQueue}; use bevy_math::{vec3, Vec3}; use bevy_tasks::{ComputeTaskPool, TaskPool}; @@ -798,8 +798,8 @@ mod test { let translation = vec3(1.0, 0.0, 0.0); // These will be overwritten. - let mut child = Entity::from_raw(0); - let mut grandchild = Entity::from_raw(1); + let mut child = Entity::from_raw(EntityRow::INDEX_ZERO); + let mut grandchild = Entity::from_raw(EntityRow::INDEX_ONE); let parent = app .world_mut() .spawn(Transform::from_translation(translation)) @@ -849,7 +849,7 @@ mod test { ); fn setup_world(world: &mut World) -> (Entity, Entity) { - let mut grandchild = Entity::from_raw(0); + let mut grandchild = Entity::from_raw(EntityRow::INDEX_ZERO); let child = world .spawn(Transform::IDENTITY) .with_children(|builder| { diff --git a/crates/bevy_ui/src/layout/mod.rs b/crates/bevy_ui/src/layout/mod.rs index cf1f5acd335e8..6d79246e78383 100644 --- a/crates/bevy_ui/src/layout/mod.rs +++ b/crates/bevy_ui/src/layout/mod.rs @@ -344,6 +344,7 @@ with UI components as a child of an entity without UI components, your UI layout #[cfg(test)] mod tests { + use bevy_ecs::entity::EntityRow; use taffy::TraversePartialTree; use bevy_asset::{AssetEvent, Assets}; @@ -1039,7 +1040,7 @@ mod tests { let (mut world, ..) = setup_ui_test_world(); - let root_node_entity = Entity::from_raw(1); + let root_node_entity = Entity::from_raw(EntityRow::INDEX_ONE); struct TestSystemParam { root_node_entity: Entity, diff --git a/crates/bevy_ui/src/layout/ui_surface.rs b/crates/bevy_ui/src/layout/ui_surface.rs index a9322de53e87e..62a9492fe24f4 100644 --- a/crates/bevy_ui/src/layout/ui_surface.rs +++ b/crates/bevy_ui/src/layout/ui_surface.rs @@ -305,6 +305,7 @@ pub fn get_text_buffer<'a>( mod tests { use super::*; use crate::{ContentSize, FixedMeasure}; + use bevy_ecs::entity::EntityRow; use bevy_math::Vec2; use taffy::TraversePartialTree; @@ -318,7 +319,7 @@ mod tests { #[test] fn test_upsert() { let mut ui_surface = UiSurface::default(); - let root_node_entity = Entity::from_raw(1); + let root_node_entity = Entity::from_raw(EntityRow::INDEX_ONE); let node = Node::default(); // standard upsert @@ -350,7 +351,7 @@ mod tests { #[test] fn test_remove_entities() { let mut ui_surface = UiSurface::default(); - let root_node_entity = Entity::from_raw(1); + let root_node_entity = Entity::from_raw(EntityRow::INDEX_ONE); let node = Node::default(); ui_surface.upsert_node(&LayoutContext::TEST_CONTEXT, root_node_entity, &node, None); @@ -366,7 +367,7 @@ mod tests { #[test] fn test_try_update_measure() { let mut ui_surface = UiSurface::default(); - let root_node_entity = Entity::from_raw(1); + let root_node_entity = Entity::from_raw(EntityRow::INDEX_ONE); let node = Node::default(); ui_surface.upsert_node(&LayoutContext::TEST_CONTEXT, root_node_entity, &node, None); @@ -381,8 +382,8 @@ mod tests { #[test] fn test_update_children() { let mut ui_surface = UiSurface::default(); - let root_node_entity = Entity::from_raw(1); - let child_entity = Entity::from_raw(2); + let root_node_entity = Entity::from_raw(EntityRow::INDEX_ONE); + let child_entity = Entity::from_raw(EntityRow::INDEX_TWO); let node = Node::default(); ui_surface.upsert_node(&LayoutContext::TEST_CONTEXT, root_node_entity, &node, None); @@ -402,8 +403,8 @@ mod tests { #[test] fn test_set_camera_children() { let mut ui_surface = UiSurface::default(); - let root_node_entity = Entity::from_raw(1); - let child_entity = Entity::from_raw(2); + let root_node_entity = Entity::from_raw(EntityRow::INDEX_ONE); + let child_entity = Entity::from_raw(EntityRow::INDEX_TWO); let node = Node::default(); ui_surface.upsert_node(&LayoutContext::TEST_CONTEXT, root_node_entity, &node, None); From d7e955bd4d1aa8d0b84b8f01c2a5b35dd199141d Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Thu, 3 Apr 2025 13:02:29 -0400 Subject: [PATCH 05/38] fix wrong tests --- crates/bevy_ecs/src/entity/mod.rs | 28 +++++++++++++++++----------- crates/bevy_ecs/src/lib.rs | 14 ++++++-------- crates/bevy_ecs/src/observer/mod.rs | 2 +- 3 files changed, 24 insertions(+), 20 deletions(-) diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index 8c46110166828..fbeb43bce0d93 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -150,7 +150,7 @@ impl EntityRow { #[inline(always)] const fn to_bits(self) -> u32 { // SAFETY: NonMax is repr transparent. - let underlying: NonZero = unsafe { mem::transmute(self.0) }; + let underlying: NonZero = unsafe { mem::transmute::>(self.0) }; underlying.get() } @@ -173,8 +173,11 @@ impl EntityRow { /// This method is the fallible counterpart to [`EntityRow::from_bits`]. #[inline(always)] pub const fn try_from_bits(bits: u32) -> Option { - match NonMaxU32::new(bits) { - Some(valid) => Some(Self(valid)), + match NonZero::::new(bits) { + // SAFETY: NonMax is repr transparent. + Some(underlying) => Some(Self(unsafe { + mem::transmute::, NonMaxU32>(underlying) + })), None => None, } } @@ -1274,6 +1277,9 @@ mod tests { #[test] fn entity_bits_roundtrip() { + let r = EntityRow::new(NonMaxU32::new(0xDEADBEEF).unwrap()); + assert_eq!(EntityRow::from_bits(r.to_bits()), r); + // 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()), @@ -1316,13 +1322,13 @@ mod tests { assert_eq!(1, C1.generation()); const C2: Entity = Entity::from_bits(0x0000_00ff_0000_00cc); - assert_eq!(0x0000_00cc, C2.index()); + assert_eq!(!0x0000_00cc, C2.index()); assert_eq!(0x0000_00ff, C2.generation()); 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_0000_0000).generation(); + const C4: u32 = Entity::from_bits(0x00dd_00ff_1111_1111).generation(); assert_eq!(0x00dd_00ff, C4); } @@ -1460,7 +1466,7 @@ mod tests { Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(1).unwrap()), NonZero::::new(1).unwrap() - ) < Entity::from_raw_and_generation( + ) > Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(2).unwrap()), NonZero::::new(1).unwrap() ) @@ -1469,7 +1475,7 @@ mod tests { Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(1).unwrap()), NonZero::::new(1).unwrap() - ) <= Entity::from_raw_and_generation( + ) >= Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(2).unwrap()), NonZero::::new(1).unwrap() ) @@ -1478,7 +1484,7 @@ mod tests { Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(2).unwrap()), NonZero::::new(2).unwrap() - ) > Entity::from_raw_and_generation( + ) < Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(1).unwrap()), NonZero::::new(2).unwrap() ) @@ -1487,7 +1493,7 @@ mod tests { Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(2).unwrap()), NonZero::::new(2).unwrap() - ) >= Entity::from_raw_and_generation( + ) <= Entity::from_raw_and_generation( EntityRow::new(NonMaxU32::new(1).unwrap()), NonZero::::new(2).unwrap() ) @@ -1511,7 +1517,7 @@ mod tests { let hash = hash.hash_one(Entity::from_raw(EntityRow::new( NonMaxU32::new(id).unwrap(), ))); - assert_eq!(hash.wrapping_sub(first_hash) as u32, i); + assert_eq!(first_hash.wrapping_sub(hash) as u32, i); } } @@ -1539,7 +1545,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#4294967338"); + assert_eq!(string, "42v1#8589934549"); let entity = Entity::PLACEHOLDER; let string = format!("{:?}", entity); diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 33b6981c3101c..dcaa0361935ff 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -488,10 +488,9 @@ mod tests { results.lock().unwrap().push((e, i)); }); results.lock().unwrap().sort(); - assert_eq!( - &*results.lock().unwrap(), - &[(e1, 1), (e2, 2), (e3, 3), (e4, 4), (e5, 5)] - ); + let mut expected = [(e1, 1), (e2, 2), (e3, 3), (e4, 4), (e5, 5)]; + expected.sort(); + assert_eq!(&*results.lock().unwrap(), &expected); } #[test] @@ -509,10 +508,9 @@ mod tests { .par_iter(&world) .for_each(|(e, &SparseStored(i))| results.lock().unwrap().push((e, i))); results.lock().unwrap().sort(); - assert_eq!( - &*results.lock().unwrap(), - &[(e1, 1), (e2, 2), (e3, 3), (e4, 4), (e5, 5)] - ); + let mut expected = [(e1, 1), (e2, 2), (e3, 3), (e4, 4), (e5, 5)]; + expected.sort(); + assert_eq!(&*results.lock().unwrap(), &expected); } #[test] diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs index 53e499c6525d2..2671850958c32 100644 --- a/crates/bevy_ecs/src/observer/mod.rs +++ b/crates/bevy_ecs/src/observer/mod.rs @@ -1079,7 +1079,7 @@ mod tests { world.add_observer(|_: Trigger, mut res: ResMut| res.observed("add_2")); world.spawn(A).flush(); - assert_eq!(vec!["add_1", "add_2"], world.resource::().0); + assert_eq!(vec!["add_2", "add_1"], world.resource::().0); // Our A entity plus our two observers assert_eq!(world.entities().len(), 3); } From 28584d74be7aafb3a211a3f5357bcd0d172e9815 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Thu, 3 Apr 2025 13:10:35 -0400 Subject: [PATCH 06/38] fixed doc tests --- crates/bevy_ecs/src/entity/mod.rs | 11 +++++++++++ crates/bevy_ecs/src/query/state.rs | 10 +++++----- crates/bevy_ecs/src/system/query.rs | 12 ++++++------ 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index fbeb43bce0d93..fc6f61dd9d1f0 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -410,6 +410,17 @@ impl Entity { Self::from_raw_and_generation(row, NonZero::::MIN) } + /// This is equivalent to [`from_raw`](Self::from_raw) except that it uses an index instead of an [`EntityRow`]. + /// + /// Returns `None` if the index is `u32::MAX`. + #[inline(always)] + pub const fn fresh_from_index(index: u32) -> Option { + match NonMaxU32::new(index) { + Some(index) => Some(Self::from_raw(EntityRow::new(index))), + None => None, + } + } + /// Convert to a form convenient for passing outside of rust. /// /// Only useful for identifying entities within the same instance of an application. Do not use diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index ce4015db9599e..66c63f89555c8 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -984,7 +984,7 @@ impl QueryState { /// /// assert_eq!(component_values, [&A(0), &A(1), &A(2)]); /// - /// let wrong_entity = Entity::from_raw(365); + /// let wrong_entity = Entity::fresh_from_index(365).unwrap(); /// /// assert_eq!(match query_state.get_many(&mut world, [wrong_entity]).unwrap_err() {QueryEntityError::EntityDoesNotExist(error) => error.entity, _ => panic!()}, wrong_entity); /// ``` @@ -1022,7 +1022,7 @@ impl QueryState { /// /// assert_eq!(component_values, [&A(0), &A(1), &A(2)]); /// - /// let wrong_entity = Entity::from_raw(365); + /// let wrong_entity = Entity::fresh_from_index(365).unwrap(); /// /// assert_eq!(match query_state.get_many_unique(&mut world, UniqueEntityArray::from([wrong_entity])).unwrap_err() {QueryEntityError::EntityDoesNotExist(error) => error.entity, _ => panic!()}, wrong_entity); /// ``` @@ -1078,7 +1078,7 @@ impl QueryState { /// /// assert_eq!(component_values, [&A(5), &A(6), &A(7)]); /// - /// let wrong_entity = Entity::from_raw(57); + /// let wrong_entity = Entity::fresh_from_index(57).unwrap(); /// let invalid_entity = world.spawn_empty().id(); /// /// assert_eq!(match query_state.get_many(&mut world, [wrong_entity]).unwrap_err() {QueryEntityError::EntityDoesNotExist(error) => error.entity, _ => panic!()}, wrong_entity); @@ -1124,7 +1124,7 @@ impl QueryState { /// /// assert_eq!(component_values, [&A(5), &A(6), &A(7)]); /// - /// let wrong_entity = Entity::from_raw(57); + /// let wrong_entity = Entity::fresh_from_index(57).unwrap(); /// let invalid_entity = world.spawn_empty().id(); /// /// assert_eq!(match query_state.get_many_unique(&mut world, UniqueEntityArray::from([wrong_entity])).unwrap_err() {QueryEntityError::EntityDoesNotExist(error) => error.entity, _ => panic!()}, wrong_entity); @@ -1452,7 +1452,7 @@ impl QueryState { /// /// # assert_eq!(component_values, [&A(5), &A(6), &A(7)]); /// - /// # let wrong_entity = Entity::from_raw(57); + /// # let wrong_entity = Entity::fresh_from_index(57).unwrap(); /// # let invalid_entity = world.spawn_empty().id(); /// /// # assert_eq!(match query_state.get_many(&mut world, [wrong_entity]).unwrap_err() {QueryEntityError::EntityDoesNotExist(error) => error.entity, _ => panic!()}, wrong_entity); diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index aa28903c8eee9..b48b5106770b9 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -933,7 +933,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// /// type Item = &'a Entity; /// type IntoIter = UniqueEntityIter>; - /// + /// /// fn into_iter(self) -> Self::IntoIter { /// // SAFETY: `Friends` ensures that it unique_list contains only unique entities. /// unsafe { UniqueEntityIter::from_iterator_unchecked(self.unique_list.iter()) } @@ -1414,7 +1414,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// /// assert_eq!(component_values, [&A(0), &A(1), &A(2)]); /// - /// let wrong_entity = Entity::from_raw(365); + /// let wrong_entity = Entity::fresh_from_index(365).unwrap(); /// /// assert_eq!( /// match query.get_many([wrong_entity]).unwrap_err() { @@ -1466,7 +1466,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// /// assert_eq!(component_values, [&A(0), &A(1), &A(2)]); /// - /// let wrong_entity = Entity::from_raw(365); + /// let wrong_entity = Entity::fresh_from_index(365).unwrap(); /// /// assert_eq!( /// match query.get_many_unique(UniqueEntityArray::from([wrong_entity])).unwrap_err() { @@ -1662,7 +1662,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// let entities: [Entity; 3] = entities.try_into().unwrap(); /// /// world.spawn(A(73)); - /// let wrong_entity = Entity::from_raw(57); + /// let wrong_entity = Entity::fresh_from_index(57).unwrap(); /// let invalid_entity = world.spawn_empty().id(); /// /// @@ -1737,7 +1737,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// let entity_set: UniqueEntityArray<3> = entity_set.try_into().unwrap(); /// /// world.spawn(A(73)); - /// let wrong_entity = Entity::from_raw(57); + /// let wrong_entity = Entity::fresh_from_index(57).unwrap(); /// let invalid_entity = world.spawn_empty().id(); /// /// @@ -2357,7 +2357,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// * `&mut T` -> `&T` /// * `&mut T` -> `Ref` /// * [`EntityMut`](crate::world::EntityMut) -> [`EntityRef`](crate::world::EntityRef) - /// + /// /// [`EntityLocation`]: crate::entity::EntityLocation /// [`&Archetype`]: crate::archetype::Archetype /// From 20d68df3c30323b9c01f4fb2df963cee737fd6a2 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Thu, 3 Apr 2025 14:15:57 -0400 Subject: [PATCH 07/38] fix doc --- crates/bevy_ecs/src/entity/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index fc6f61dd9d1f0..547e16bf51517 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -162,7 +162,7 @@ impl EntityRow { /// /// This method will likely panic if given `u32` values that did not come from [`EntityRow::to_bits`]. #[inline] - pub const fn from_bits(bits: u32) -> Self { + const fn from_bits(bits: u32) -> Self { Self::try_from_bits(bits).expect("Attempted to initialize invalid bits as an entity row") } @@ -172,7 +172,7 @@ impl EntityRow { /// /// This method is the fallible counterpart to [`EntityRow::from_bits`]. #[inline(always)] - pub const fn try_from_bits(bits: u32) -> Option { + const fn try_from_bits(bits: u32) -> Option { match NonZero::::new(bits) { // SAFETY: NonMax is repr transparent. Some(underlying) => Some(Self(unsafe { From 3fc85064dca312000ed55605b75bcaa4b4f76ef5 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Thu, 3 Apr 2025 14:27:04 -0400 Subject: [PATCH 08/38] fix example --- examples/ecs/error_handling.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/ecs/error_handling.rs b/examples/ecs/error_handling.rs index b13a018530fc0..16dbf04b4c516 100644 --- a/examples/ecs/error_handling.rs +++ b/examples/ecs/error_handling.rs @@ -169,7 +169,7 @@ fn failing_system(world: &mut World) -> Result { fn failing_commands(mut commands: Commands) { commands // This entity doesn't exist! - .entity(Entity::from_raw(12345678)) + .entity(Entity::fresh_from_index(12345678).unwrap()) // Normally, this failed command would panic, // but since we've set the global error handler to `warn` // it will log a warning instead. From da0268d1de172337812c8e8238a9706685ac76d4 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Thu, 3 Apr 2025 14:27:12 -0400 Subject: [PATCH 09/38] fix benches --- benches/benches/bevy_ecs/world/entity_hash.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/benches/benches/bevy_ecs/world/entity_hash.rs b/benches/benches/bevy_ecs/world/entity_hash.rs index 7e8dfb4a21f2f..3562510c90fda 100644 --- a/benches/benches/bevy_ecs/world/entity_hash.rs +++ b/benches/benches/bevy_ecs/world/entity_hash.rs @@ -17,9 +17,10 @@ fn make_entity(rng: &mut impl Rng, size: usize) -> Entity { let generation = 1.0 + -(1.0 - x).log2() * 2.0; // this is not reliable, but we're internal so a hack is ok - let bits = ((generation as u64) << 32) | (id as u64); + let id = id as u64 + 1; + let bits = ((generation as u64) << 32) | id; let e = Entity::from_bits(bits); - assert_eq!(e.index(), id as u32); + assert_eq!(e.index(), !(id as u32)); assert_eq!(e.generation(), generation as u32); e } From fbd9306623230706cb779b5f3f64757bd55b513d Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Thu, 3 Apr 2025 14:53:06 -0400 Subject: [PATCH 10/38] fix scene example --- assets/scenes/load_scene_example.scn.ron | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/assets/scenes/load_scene_example.scn.ron b/assets/scenes/load_scene_example.scn.ron index e768a7b149c41..477bee30545df 100644 --- a/assets/scenes/load_scene_example.scn.ron +++ b/assets/scenes/load_scene_example.scn.ron @@ -5,7 +5,7 @@ ), }, entities: { - 4294967296: ( + 4294967297: ( components: { "bevy_ecs::name::Name": "joe", "bevy_transform::components::global_transform::GlobalTransform": ((1.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0)), @@ -23,7 +23,7 @@ ), }, ), - 4294967297: ( + 4294967298: ( components: { "scene::ComponentA": ( x: 3.0, @@ -32,4 +32,4 @@ }, ), }, -) \ No newline at end of file +) From 3164bf25b1049bf100d8c890e8d9aa994fd874dc Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Thu, 3 Apr 2025 15:21:13 -0400 Subject: [PATCH 11/38] fixed should serialize --- crates/bevy_scene/src/serde.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/bevy_scene/src/serde.rs b/crates/bevy_scene/src/serde.rs index 526069b5ab0ac..845c2a750c968 100644 --- a/crates/bevy_scene/src/serde.rs +++ b/crates/bevy_scene/src/serde.rs @@ -638,21 +638,21 @@ mod tests { ), }, entities: { - 4294967296: ( + 8589934589: ( components: { + "bevy_scene::serde::tests::Bar": (345), + "bevy_scene::serde::tests::Baz": (789), "bevy_scene::serde::tests::Foo": (123), }, ), - 4294967297: ( + 8589934590: ( components: { "bevy_scene::serde::tests::Bar": (345), "bevy_scene::serde::tests::Foo": (123), }, ), - 4294967298: ( + 8589934591: ( components: { - "bevy_scene::serde::tests::Bar": (345), - "bevy_scene::serde::tests::Baz": (789), "bevy_scene::serde::tests::Foo": (123), }, ), From de0b7b7494aaa0b1b35aadb8c7f39c874970ac40 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Thu, 3 Apr 2025 15:23:15 -0400 Subject: [PATCH 12/38] fixed postcard --- crates/bevy_scene/src/serde.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_scene/src/serde.rs b/crates/bevy_scene/src/serde.rs index 845c2a750c968..d9760d36793b1 100644 --- a/crates/bevy_scene/src/serde.rs +++ b/crates/bevy_scene/src/serde.rs @@ -815,7 +815,7 @@ mod tests { assert_eq!( vec![ - 0, 1, 128, 128, 128, 128, 16, 1, 37, 98, 101, 118, 121, 95, 115, 99, 101, 110, 101, + 0, 1, 255, 255, 255, 255, 31, 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 From 5930ada85147a4347b7e58acc7035ce15f508628 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Thu, 3 Apr 2025 15:24:25 -0400 Subject: [PATCH 13/38] message pack --- crates/bevy_scene/src/serde.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/crates/bevy_scene/src/serde.rs b/crates/bevy_scene/src/serde.rs index d9760d36793b1..5c72ad616f15c 100644 --- a/crates/bevy_scene/src/serde.rs +++ b/crates/bevy_scene/src/serde.rs @@ -856,11 +856,12 @@ mod tests { assert_eq!( vec![ - 146, 128, 129, 207, 0, 0, 0, 1, 0, 0, 0, 0, 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, 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 ], buf ); From bf2875225827ee08e0a3f02500923a028fccf242 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Thu, 3 Apr 2025 15:25:11 -0400 Subject: [PATCH 14/38] fixed bincode --- crates/bevy_scene/src/serde.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/crates/bevy_scene/src/serde.rs b/crates/bevy_scene/src/serde.rs index 5c72ad616f15c..988e4cafe1845 100644 --- a/crates/bevy_scene/src/serde.rs +++ b/crates/bevy_scene/src/serde.rs @@ -900,12 +900,13 @@ mod tests { assert_eq!( vec![ - 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 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, 0, 0, 0, - 0, 0, 0, 3, 0, 0, 0, 0, 0, 0, 0, 102, 102, 166, 63, 205, 204, 108, 64, 1, 0, 0, 0, - 12, 0, 0, 0, 0, 0, 0, 0, 72, 101, 108, 108, 111, 32, 87, 111, 114, 108, 100, 33 + 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, 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, + 0, 0, 0, 0, 0, 0, 3, 0, 0, 0, 0, 0, 0, 0, 102, 102, 166, 63, 205, 204, 108, 64, 1, + 0, 0, 0, 12, 0, 0, 0, 0, 0, 0, 0, 72, 101, 108, 108, 111, 32, 87, 111, 114, 108, + 100, 33 ], serialized_scene ); From 10a39c20234a3b492674c8211e70c0e0e4cd7602 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Thu, 3 Apr 2025 15:26:41 -0400 Subject: [PATCH 15/38] fix should deserialize --- crates/bevy_scene/src/serde.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/bevy_scene/src/serde.rs b/crates/bevy_scene/src/serde.rs index 988e4cafe1845..562a4873917ba 100644 --- a/crates/bevy_scene/src/serde.rs +++ b/crates/bevy_scene/src/serde.rs @@ -675,18 +675,18 @@ mod tests { ), }, entities: { - 4294967296: ( + 8589934591: ( components: { "bevy_scene::serde::tests::Foo": (123), }, ), - 4294967297: ( + 8589934590: ( components: { "bevy_scene::serde::tests::Foo": (123), "bevy_scene::serde::tests::Bar": (345), }, ), - 4294967298: ( + 8589934589: ( components: { "bevy_scene::serde::tests::Foo": (123), "bevy_scene::serde::tests::Bar": (345), From afaea980d67614acf12372e2882ab441e0867837 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Thu, 3 Apr 2025 15:30:10 -0400 Subject: [PATCH 16/38] fix scene_child... --- crates/bevy_scene/src/scene_spawner.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_scene/src/scene_spawner.rs b/crates/bevy_scene/src/scene_spawner.rs index e9f32ae9e0a1a..c78a248eb317e 100644 --- a/crates/bevy_scene/src/scene_spawner.rs +++ b/crates/bevy_scene/src/scene_spawner.rs @@ -915,7 +915,7 @@ mod tests { app.update(); let world = app.world_mut(); - let spawned_root = world.entity(spawned).get::().unwrap()[0]; + let spawned_root = world.entity(spawned).get::().unwrap()[1]; let children = world.entity(spawned_root).get::().unwrap(); assert_eq!(children.len(), 3); assert!(world.entity(children[0]).get::().is_some()); From 190f43b5282ecde0fd74d7c00a43ddb56b67748d Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Thu, 3 Apr 2025 15:34:27 -0400 Subject: [PATCH 17/38] fix extraction order --- crates/bevy_scene/src/dynamic_scene_builder.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/bevy_scene/src/dynamic_scene_builder.rs b/crates/bevy_scene/src/dynamic_scene_builder.rs index 5deac09aaa073..edd61dbba5951 100644 --- a/crates/bevy_scene/src/dynamic_scene_builder.rs +++ b/crates/bevy_scene/src/dynamic_scene_builder.rs @@ -509,10 +509,10 @@ mod tests { let mut entities = builder.build().entities.into_iter(); // Assert entities are ordered - assert_eq!(entity_a, entities.next().map(|e| e.entity).unwrap()); - assert_eq!(entity_b, entities.next().map(|e| e.entity).unwrap()); - assert_eq!(entity_c, entities.next().map(|e| e.entity).unwrap()); assert_eq!(entity_d, entities.next().map(|e| e.entity).unwrap()); + assert_eq!(entity_c, entities.next().map(|e| e.entity).unwrap()); + assert_eq!(entity_b, entities.next().map(|e| e.entity).unwrap()); + assert_eq!(entity_a, entities.next().map(|e| e.entity).unwrap()); } #[test] From 8ba283981c5b9691c5fd7d6542aff9aeb7d96e4f Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Thu, 3 Apr 2025 15:35:18 -0400 Subject: [PATCH 18/38] fix extraction query --- crates/bevy_scene/src/dynamic_scene_builder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_scene/src/dynamic_scene_builder.rs b/crates/bevy_scene/src/dynamic_scene_builder.rs index edd61dbba5951..9bb4780afee75 100644 --- a/crates/bevy_scene/src/dynamic_scene_builder.rs +++ b/crates/bevy_scene/src/dynamic_scene_builder.rs @@ -539,7 +539,7 @@ mod tests { assert_eq!(scene.entities.len(), 2); let mut scene_entities = vec![scene.entities[0].entity, scene.entities[1].entity]; scene_entities.sort(); - assert_eq!(scene_entities, [entity_a_b, entity_a]); + assert_eq!(scene_entities, [entity_a, entity_a_b]); } #[test] From 2ba14d87d692f36700f800a06dd10b1263e11439 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Thu, 3 Apr 2025 15:36:19 -0400 Subject: [PATCH 19/38] fix allowed_components --- crates/bevy_scene/src/dynamic_scene_builder.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_scene/src/dynamic_scene_builder.rs b/crates/bevy_scene/src/dynamic_scene_builder.rs index 9bb4780afee75..c9e594107ec66 100644 --- a/crates/bevy_scene/src/dynamic_scene_builder.rs +++ b/crates/bevy_scene/src/dynamic_scene_builder.rs @@ -621,9 +621,9 @@ mod tests { .build(); assert_eq!(scene.entities.len(), 3); - assert!(scene.entities[0].components[0].represents::()); + assert!(scene.entities[2].components[0].represents::()); assert!(scene.entities[1].components[0].represents::()); - assert_eq!(scene.entities[2].components.len(), 0); + assert_eq!(scene.entities[0].components.len(), 0); } #[test] From f91b96dcdfc232f52ed2850b78ef684710ab02d6 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Thu, 3 Apr 2025 21:49:34 -0400 Subject: [PATCH 20/38] remove custom index constants --- crates/bevy_ecs/src/entity/map_entities.rs | 14 ++++++-------- crates/bevy_ecs/src/entity/mod.rs | 9 --------- crates/bevy_ecs/src/lib.rs | 14 +++++++------- crates/bevy_ecs/src/query/state.rs | 3 +-- crates/bevy_remote/src/builtin_methods.rs | 5 ++--- crates/bevy_transform/src/systems.rs | 8 ++++---- crates/bevy_ui/src/layout/mod.rs | 3 +-- crates/bevy_ui/src/layout/ui_surface.rs | 15 +++++++-------- 8 files changed, 28 insertions(+), 43 deletions(-) diff --git a/crates/bevy_ecs/src/entity/map_entities.rs b/crates/bevy_ecs/src/entity/map_entities.rs index 2ef6a78caf182..d50ed6ccb2545 100644 --- a/crates/bevy_ecs/src/entity/map_entities.rs +++ b/crates/bevy_ecs/src/entity/map_entities.rs @@ -280,23 +280,19 @@ impl<'m> SceneEntityMapper<'m> { #[cfg(test)] mod tests { - use nonmax::NonMaxU32; use crate::{ - entity::{Entity, EntityHashMap, EntityMapper, EntityRow, SceneEntityMapper}, + entity::{Entity, EntityHashMap, EntityMapper, SceneEntityMapper}, world::World, }; #[test] fn entity_mapper() { - const FIRST_IDX: EntityRow = EntityRow::new(NonMaxU32::new(1).unwrap()); - const SECOND_IDX: EntityRow = EntityRow::new(NonMaxU32::new(2).unwrap()); - let mut map = EntityHashMap::default(); let mut world = World::new(); let mut mapper = SceneEntityMapper::new(&mut map, &mut world); - let mapped_ent = Entity::from_raw(FIRST_IDX); + let mapped_ent = Entity::fresh_from_index(1).unwrap(); let dead_ref = mapper.get_mapped(mapped_ent); assert_eq!( @@ -305,7 +301,9 @@ mod tests { "should persist the allocated mapping from the previous line" ); assert_eq!( - mapper.get_mapped(Entity::from_raw(SECOND_IDX)).index(), + mapper + .get_mapped(Entity::fresh_from_index(2).unwrap()) + .index(), dead_ref.index(), "should re-use the same index for further dead refs" ); @@ -323,7 +321,7 @@ mod tests { let mut world = World::new(); let dead_ref = SceneEntityMapper::world_scope(&mut map, &mut world, |_, mapper| { - mapper.get_mapped(Entity::from_raw(EntityRow::INDEX_ZERO)) + mapper.get_mapped(Entity::fresh_from_index(0).unwrap()) }); // Next allocated entity should be a further generation on the same index diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index ea07104093e05..6405395fab189 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -125,15 +125,6 @@ pub struct EntityRow(NonMaxU32); impl EntityRow { const PLACEHOLDER: Self = Self(NonMaxU32::MAX); - /// The row at index zero. This exists to make tests easier, but should never be used in an practice. - pub const INDEX_ZERO: Self = Self(NonMaxU32::ZERO); - - /// The row at index one. This exists to make tests easier, but should never be used in an practice. - pub const INDEX_ONE: Self = Self(NonMaxU32::ONE); - - /// The row at index one. This exists to make tests easier, but should never be used in an practice. - pub const INDEX_TWO: Self = Self(NonMaxU32::new(2).unwrap()); - /// Constructs a new [`EntityRow`] from its index. pub const fn new(index: NonMaxU32) -> Self { Self(index) diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index dcaa0361935ff..1939f3b99ebb4 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -133,7 +133,7 @@ mod tests { bundle::Bundle, change_detection::Ref, component::{Component, ComponentId, RequiredComponents, RequiredComponentsError}, - entity::{Entity, EntityMapper, EntityRow}, + entity::{Entity, EntityMapper}, entity_disabling::DefaultQueryFilters, prelude::Or, query::{Added, Changed, FilteredAccess, QueryFilter, With, Without}, @@ -1540,8 +1540,8 @@ mod tests { let mut world_a = World::new(); let world_b = World::new(); let mut query = world_a.query::<&A>(); - let _ = query.get(&world_a, Entity::from_raw(EntityRow::INDEX_ZERO)); - let _ = query.get(&world_b, Entity::from_raw(EntityRow::INDEX_ZERO)); + let _ = query.get(&world_a, Entity::fresh_from_index(0).unwrap()); + let _ = query.get(&world_b, Entity::fresh_from_index(0).unwrap()); } #[test] @@ -1698,7 +1698,7 @@ mod tests { fn insert_or_spawn_batch() { let mut world = World::default(); let e0 = world.spawn(A(0)).id(); - let e1 = Entity::from_raw(EntityRow::INDEX_ONE); + let e1 = Entity::fresh_from_index(1).unwrap(); let values = vec![(e0, (B(0), C)), (e1, (B(1), C))]; @@ -1739,7 +1739,7 @@ mod tests { fn insert_or_spawn_batch_invalid() { let mut world = World::default(); let e0 = world.spawn(A(0)).id(); - let e1 = Entity::from_raw(EntityRow::INDEX_ONE); + let e1 = Entity::fresh_from_index(1).unwrap(); let e2 = world.spawn_empty().id(); let invalid_e2 = Entity::from_raw_and_generation(e2.row(), NonZero::::new(2).unwrap()); @@ -1872,7 +1872,7 @@ mod tests { fn try_insert_batch() { let mut world = World::default(); let e0 = world.spawn(A(0)).id(); - let e1 = Entity::from_raw(EntityRow::INDEX_ONE); + let e1 = Entity::fresh_from_index(1).unwrap(); let values = vec![(e0, (A(1), B(0))), (e1, (A(0), B(1)))]; @@ -1896,7 +1896,7 @@ mod tests { fn try_insert_batch_if_new() { let mut world = World::default(); let e0 = world.spawn(A(0)).id(); - let e1 = Entity::from_raw(EntityRow::INDEX_ONE); + let e1 = Entity::fresh_from_index(1).unwrap(); let values = vec![(e0, (A(1), B(0))), (e1, (A(0), B(1)))]; diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 66c63f89555c8..dcc12371ad2ec 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -1881,7 +1881,6 @@ impl From> for QueryState(); - let _panics = query_state.get(&world_2, Entity::from_raw(EntityRow::INDEX_ZERO)); + let _panics = query_state.get(&world_2, Entity::fresh_from_index(0).unwrap()); } #[test] diff --git a/crates/bevy_remote/src/builtin_methods.rs b/crates/bevy_remote/src/builtin_methods.rs index 043f6d283a3df..fcd3de7081b4d 100644 --- a/crates/bevy_remote/src/builtin_methods.rs +++ b/crates/bevy_remote/src/builtin_methods.rs @@ -1490,7 +1490,6 @@ mod tests { "Deserialized value does not match original" ); } - use bevy_ecs::entity::EntityRow; use super::*; @@ -1498,7 +1497,7 @@ mod tests { fn serialization_tests() { test_serialize_deserialize(BrpQueryRow { components: Default::default(), - entity: Entity::from_raw(EntityRow::INDEX_ZERO), + entity: Entity::fresh_from_index(0).unwrap(), has: Default::default(), }); test_serialize_deserialize(BrpListWatchingResponse::default()); @@ -1512,7 +1511,7 @@ mod tests { ..Default::default() }); test_serialize_deserialize(BrpListParams { - entity: Entity::from_raw(EntityRow::INDEX_ZERO), + entity: Entity::fresh_from_index(0).unwrap(), }); } } diff --git a/crates/bevy_transform/src/systems.rs b/crates/bevy_transform/src/systems.rs index 3a122c6cc2b0e..8cfd97877ac29 100644 --- a/crates/bevy_transform/src/systems.rs +++ b/crates/bevy_transform/src/systems.rs @@ -557,7 +557,7 @@ mod parallel { mod test { use alloc::{vec, vec::Vec}; use bevy_app::prelude::*; - use bevy_ecs::{entity::EntityRow, prelude::*, world::CommandQueue}; + use bevy_ecs::{prelude::*, world::CommandQueue}; use bevy_math::{vec3, Vec3}; use bevy_tasks::{ComputeTaskPool, TaskPool}; @@ -798,8 +798,8 @@ mod test { let translation = vec3(1.0, 0.0, 0.0); // These will be overwritten. - let mut child = Entity::from_raw(EntityRow::INDEX_ZERO); - let mut grandchild = Entity::from_raw(EntityRow::INDEX_ONE); + let mut child = Entity::fresh_from_index(0).unwrap(); + let mut grandchild = Entity::fresh_from_index(1).unwrap(); let parent = app .world_mut() .spawn(Transform::from_translation(translation)) @@ -849,7 +849,7 @@ mod test { ); fn setup_world(world: &mut World) -> (Entity, Entity) { - let mut grandchild = Entity::from_raw(EntityRow::INDEX_ZERO); + let mut grandchild = Entity::fresh_from_index(0).unwrap(); let child = world .spawn(Transform::IDENTITY) .with_children(|builder| { diff --git a/crates/bevy_ui/src/layout/mod.rs b/crates/bevy_ui/src/layout/mod.rs index 6d79246e78383..eefdf33c5130c 100644 --- a/crates/bevy_ui/src/layout/mod.rs +++ b/crates/bevy_ui/src/layout/mod.rs @@ -344,7 +344,6 @@ with UI components as a child of an entity without UI components, your UI layout #[cfg(test)] mod tests { - use bevy_ecs::entity::EntityRow; use taffy::TraversePartialTree; use bevy_asset::{AssetEvent, Assets}; @@ -1040,7 +1039,7 @@ mod tests { let (mut world, ..) = setup_ui_test_world(); - let root_node_entity = Entity::from_raw(EntityRow::INDEX_ONE); + let root_node_entity = Entity::fresh_from_index(1).unwrap(); struct TestSystemParam { root_node_entity: Entity, diff --git a/crates/bevy_ui/src/layout/ui_surface.rs b/crates/bevy_ui/src/layout/ui_surface.rs index 62a9492fe24f4..3c8b16213a567 100644 --- a/crates/bevy_ui/src/layout/ui_surface.rs +++ b/crates/bevy_ui/src/layout/ui_surface.rs @@ -305,7 +305,6 @@ pub fn get_text_buffer<'a>( mod tests { use super::*; use crate::{ContentSize, FixedMeasure}; - use bevy_ecs::entity::EntityRow; use bevy_math::Vec2; use taffy::TraversePartialTree; @@ -319,7 +318,7 @@ mod tests { #[test] fn test_upsert() { let mut ui_surface = UiSurface::default(); - let root_node_entity = Entity::from_raw(EntityRow::INDEX_ONE); + let root_node_entity = Entity::fresh_from_index(1).unwrap(); let node = Node::default(); // standard upsert @@ -351,7 +350,7 @@ mod tests { #[test] fn test_remove_entities() { let mut ui_surface = UiSurface::default(); - let root_node_entity = Entity::from_raw(EntityRow::INDEX_ONE); + let root_node_entity = Entity::fresh_from_index(1).unwrap(); let node = Node::default(); ui_surface.upsert_node(&LayoutContext::TEST_CONTEXT, root_node_entity, &node, None); @@ -367,7 +366,7 @@ mod tests { #[test] fn test_try_update_measure() { let mut ui_surface = UiSurface::default(); - let root_node_entity = Entity::from_raw(EntityRow::INDEX_ONE); + let root_node_entity = Entity::fresh_from_index(1).unwrap(); let node = Node::default(); ui_surface.upsert_node(&LayoutContext::TEST_CONTEXT, root_node_entity, &node, None); @@ -382,8 +381,8 @@ mod tests { #[test] fn test_update_children() { let mut ui_surface = UiSurface::default(); - let root_node_entity = Entity::from_raw(EntityRow::INDEX_ONE); - let child_entity = Entity::from_raw(EntityRow::INDEX_TWO); + let root_node_entity = Entity::fresh_from_index(1).unwrap(); + let child_entity = Entity::fresh_from_index(2).unwrap(); let node = Node::default(); ui_surface.upsert_node(&LayoutContext::TEST_CONTEXT, root_node_entity, &node, None); @@ -403,8 +402,8 @@ mod tests { #[test] fn test_set_camera_children() { let mut ui_surface = UiSurface::default(); - let root_node_entity = Entity::from_raw(EntityRow::INDEX_ONE); - let child_entity = Entity::from_raw(EntityRow::INDEX_TWO); + let root_node_entity = Entity::fresh_from_index(1).unwrap(); + let child_entity = Entity::fresh_from_index(2).unwrap(); let node = Node::default(); ui_surface.upsert_node(&LayoutContext::TEST_CONTEXT, root_node_entity, &node, None); From 9b2dc56c2143bc47474cb85e836a86aaaa65e173 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Fri, 4 Apr 2025 10:37:33 -0400 Subject: [PATCH 21/38] add migration guide --- .../18704_Make_entity::index_non_max.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 release-content/migration-guides/18704_Make_entity::index_non_max.md diff --git a/release-content/migration-guides/18704_Make_entity::index_non_max.md b/release-content/migration-guides/18704_Make_entity::index_non_max.md new file mode 100644 index 0000000000000..64438b8585b65 --- /dev/null +++ b/release-content/migration-guides/18704_Make_entity::index_non_max.md @@ -0,0 +1,15 @@ +--- +title: Manual Entity Creation +pull_requests: [18704] +--- + +`Entity` no longer stores its incex 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::fresh_from_index` which is identical to the previous `Entity::from_raw`, except that it now returns `Option` where the results 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::fresh_from_index(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::fresh_from_index(my_idnex)`. From 73062cb67ac61cda9f489b56dd2b2cde8cbf520d Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Fri, 4 Apr 2025 10:42:41 -0400 Subject: [PATCH 22/38] change name to not have "::" --- ...ntity::index_non_max.md => 18704_Make_entity_index_non_max.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename release-content/migration-guides/{18704_Make_entity::index_non_max.md => 18704_Make_entity_index_non_max.md} (100%) diff --git a/release-content/migration-guides/18704_Make_entity::index_non_max.md b/release-content/migration-guides/18704_Make_entity_index_non_max.md similarity index 100% rename from release-content/migration-guides/18704_Make_entity::index_non_max.md rename to release-content/migration-guides/18704_Make_entity_index_non_max.md From 74349dc640aec5c0f0c4db9081cf564eac30c772 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Fri, 4 Apr 2025 11:04:44 -0400 Subject: [PATCH 23/38] fix markdown lint --- .../migration-guides/18704_Make_entity_index_non_max.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/release-content/migration-guides/18704_Make_entity_index_non_max.md b/release-content/migration-guides/18704_Make_entity_index_non_max.md index 64438b8585b65..bf513b578022c 100644 --- a/release-content/migration-guides/18704_Make_entity_index_non_max.md +++ b/release-content/migration-guides/18704_Make_entity_index_non_max.md @@ -8,8 +8,10 @@ pull_requests: [18704] 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::fresh_from_index(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::fresh_from_index(my_idnex)`. From 40e1b3d9142e8491baefa1aace494d8f06de6a79 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Sat, 5 Apr 2025 09:56:41 -0400 Subject: [PATCH 24/38] fix typo --- .../migration-guides/18704_Make_entity_index_non_max.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/release-content/migration-guides/18704_Make_entity_index_non_max.md b/release-content/migration-guides/18704_Make_entity_index_non_max.md index bf513b578022c..ee7dacdbc4753 100644 --- a/release-content/migration-guides/18704_Make_entity_index_non_max.md +++ b/release-content/migration-guides/18704_Make_entity_index_non_max.md @@ -14,4 +14,4 @@ Although you probably shouldn't be making entities manually, it is sometimes use + let entity = Entity::fresh_from_index(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::fresh_from_index(my_idnex)`. +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::fresh_from_index(my_index)`. From a855e132462606d0147f998b2a2af9319a4aec7a Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Sat, 5 Apr 2025 19:56:56 -0400 Subject: [PATCH 25/38] make to_bits one transmute --- crates/bevy_ecs/src/entity/mod.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index 6405395fab189..e9b2d009ba052 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -140,9 +140,8 @@ impl EntityRow { /// The bits are opaque and should not be regarded as meaningful. #[inline(always)] const fn to_bits(self) -> u32 { - // SAFETY: NonMax is repr transparent. - let underlying: NonZero = unsafe { mem::transmute::>(self.0) }; - underlying.get() + // SAFETY: NonMax and NonZero are repr transparent. + unsafe { mem::transmute::(self.0) } } /// Reconstruct an [`EntityRow`] previously destructured with [`EntityRow::to_bits`]. From fd628edcc2c9ebf3d058cd16154dd447540c43f2 Mon Sep 17 00:00:00 2001 From: Eagster <79881080+ElliottjPierce@users.noreply.github.com> Date: Sun, 20 Apr 2025 11:54:44 -0400 Subject: [PATCH 26/38] fix safety comment Co-authored-by: atlv --- crates/bevy_ecs/src/entity/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index e9b2d009ba052..1ba0bea3e8f22 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -140,7 +140,7 @@ impl EntityRow { /// The bits are opaque and should not be regarded as meaningful. #[inline(always)] const fn to_bits(self) -> u32 { - // SAFETY: NonMax and NonZero are repr transparent. + // SAFETY: NonMax is repr transparent. unsafe { mem::transmute::(self.0) } } From 30ee5890bb08f5ffc83b31365058bfe00ee8b5bd Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Sun, 20 Apr 2025 11:58:45 -0400 Subject: [PATCH 27/38] use row everywhere --- crates/bevy_ecs/src/entity/mod.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index 1ba0bea3e8f22..c74ef6f35158c 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -268,10 +268,10 @@ pub struct Entity { // 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")] - index: EntityRow, + row: EntityRow, generation: NonZero, #[cfg(target_endian = "big")] - index: EntityRow, + row: EntityRow, } // By not short-circuiting in comparisons, we get better codegen. @@ -345,7 +345,10 @@ impl Entity { ) -> Entity { debug_assert!(generation.get() <= HIGH_MASK); - Self { index, generation } + Self { + row: index, + generation, + } } /// An entity ID with a placeholder value. This may or may not correspond to an actual entity, @@ -420,7 +423,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.index.to_bits(), self.generation.get()) + IdentifierMask::pack_into_u64(self.row.to_bits(), self.generation.get()) } /// Reconstruct an `Entity` previously destructured with [`Entity::to_bits`]. @@ -454,7 +457,7 @@ impl Entity { if kind == (IdKind::Entity as u8) { if let Some(row) = EntityRow::try_from_bits(id.low()) { return Ok(Self { - index: row, + row, generation: id.high(), }); } @@ -472,13 +475,13 @@ impl Entity { /// specific snapshot of the world, such as when serializing. #[inline] pub const fn row(self) -> EntityRow { - self.index + self.row } /// Equivalent to `self.row().index()`. See [`Self::row`] for details. #[inline] pub const fn index(self) -> u32 { - self.index.index() + self.row.index() } /// Returns the generation of this Entity's index. The generation is incremented each time an From 43645585adf41fcaf5db32c63495839e9caee88f Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Sun, 20 Apr 2025 12:01:03 -0400 Subject: [PATCH 28/38] change migration guide name --- ..._Make_entity_index_non_max.md => make_entity_index_non_max.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename release-content/migration-guides/{18704_Make_entity_index_non_max.md => make_entity_index_non_max.md} (100%) diff --git a/release-content/migration-guides/18704_Make_entity_index_non_max.md b/release-content/migration-guides/make_entity_index_non_max.md similarity index 100% rename from release-content/migration-guides/18704_Make_entity_index_non_max.md rename to release-content/migration-guides/make_entity_index_non_max.md From a32912c62adccd516fdb7ab5bda5a5604ab884c2 Mon Sep 17 00:00:00 2001 From: Eagster <79881080+ElliottjPierce@users.noreply.github.com> Date: Sun, 20 Apr 2025 20:39:55 -0400 Subject: [PATCH 29/38] Fix safety comment Co-authored-by: atlv --- crates/bevy_ecs/src/entity/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index c74ef6f35158c..cd0c3a9497a7c 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -164,7 +164,7 @@ impl EntityRow { #[inline(always)] const fn try_from_bits(bits: u32) -> Option { match NonZero::::new(bits) { - // SAFETY: NonMax is repr transparent. + // SAFETY: NonMax and NonZero are repr transparent. Some(underlying) => Some(Self(unsafe { mem::transmute::, NonMaxU32>(underlying) })), From 5bd4982c02f7391e8b7c3587396e08b5ff0dbf48 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Sun, 20 Apr 2025 20:49:36 -0400 Subject: [PATCH 30/38] use row naming everywhere --- crates/bevy_ecs/src/entity/map_entities.rs | 6 +++--- crates/bevy_ecs/src/entity/mod.rs | 25 ++++++++++------------ crates/bevy_ecs/src/lib.rs | 12 +++++------ crates/bevy_ecs/src/query/state.rs | 2 +- crates/bevy_remote/src/builtin_methods.rs | 4 ++-- crates/bevy_transform/src/systems.rs | 6 +++--- crates/bevy_ui/src/layout/mod.rs | 2 +- crates/bevy_ui/src/layout/ui_surface.rs | 14 ++++++------ examples/ecs/error_handling.rs | 2 +- 9 files changed, 35 insertions(+), 38 deletions(-) diff --git a/crates/bevy_ecs/src/entity/map_entities.rs b/crates/bevy_ecs/src/entity/map_entities.rs index d50ed6ccb2545..e68ed95f5fda2 100644 --- a/crates/bevy_ecs/src/entity/map_entities.rs +++ b/crates/bevy_ecs/src/entity/map_entities.rs @@ -292,7 +292,7 @@ mod tests { let mut world = World::new(); let mut mapper = SceneEntityMapper::new(&mut map, &mut world); - let mapped_ent = Entity::fresh_from_index(1).unwrap(); + let mapped_ent = Entity::fresh_from_row(1).unwrap(); let dead_ref = mapper.get_mapped(mapped_ent); assert_eq!( @@ -302,7 +302,7 @@ mod tests { ); assert_eq!( mapper - .get_mapped(Entity::fresh_from_index(2).unwrap()) + .get_mapped(Entity::fresh_from_row(2).unwrap()) .index(), dead_ref.index(), "should re-use the same index for further dead refs" @@ -321,7 +321,7 @@ mod tests { let mut world = World::new(); let dead_ref = SceneEntityMapper::world_scope(&mut map, &mut world, |_, mapper| { - mapper.get_mapped(Entity::fresh_from_index(0).unwrap()) + mapper.get_mapped(Entity::fresh_from_row(0).unwrap()) }); // Next allocated entity should be a further generation on the same index diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index cd0c3a9497a7c..d8ceebf2730d8 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -336,19 +336,16 @@ pub(crate) enum AllocAtWithoutReplacement { } impl Entity { - /// Construct an [`Entity`] from a raw `index` value and a non-zero `generation` value. + /// Construct an [`Entity`] from a raw `row` value and a non-zero `generation` value. /// Ensure that the generation value is never greater than `0x7FFF_FFFF`. #[inline(always)] pub(crate) const fn from_raw_and_generation( - index: EntityRow, + row: EntityRow, generation: NonZero, ) -> Entity { debug_assert!(generation.get() <= HIGH_MASK); - Self { - row: index, - generation, - } + Self { row, generation } } /// An entity ID with a placeholder value. This may or may not correspond to an actual entity, @@ -404,13 +401,13 @@ impl Entity { Self::from_raw_and_generation(row, NonZero::::MIN) } - /// This is equivalent to [`from_raw`](Self::from_raw) except that it uses an index instead of an [`EntityRow`]. + /// This is equivalent to [`from_raw`](Self::from_raw) except that it uses an row instead of an [`EntityRow`]. /// - /// Returns `None` if the index is `u32::MAX`. + /// Returns `None` if the row is `u32::MAX`. #[inline(always)] - pub const fn fresh_from_index(index: u32) -> Option { - match NonMaxU32::new(index) { - Some(index) => Some(Self::from_raw(EntityRow::new(index))), + pub const fn fresh_from_row(row: u32) -> Option { + match NonMaxU32::new(row) { + Some(row) => Some(Self::from_raw(EntityRow::new(row))), None => None, } } @@ -484,9 +481,9 @@ impl Entity { self.row.index() } - /// Returns the generation of this Entity's index. The generation is incremented each time an - /// entity with a given index is despawned. This serves as a "count" of the number of times a - /// given index has been reused (index, generation) pairs uniquely identify a given Entity. + /// Returns the generation of this Entity's row. The generation is incremented each time an + /// 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 diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 1939f3b99ebb4..79c1d17d3928c 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -1540,8 +1540,8 @@ mod tests { let mut world_a = World::new(); let world_b = World::new(); let mut query = world_a.query::<&A>(); - let _ = query.get(&world_a, Entity::fresh_from_index(0).unwrap()); - let _ = query.get(&world_b, Entity::fresh_from_index(0).unwrap()); + let _ = query.get(&world_a, Entity::fresh_from_row(0).unwrap()); + let _ = query.get(&world_b, Entity::fresh_from_row(0).unwrap()); } #[test] @@ -1698,7 +1698,7 @@ mod tests { fn insert_or_spawn_batch() { let mut world = World::default(); let e0 = world.spawn(A(0)).id(); - let e1 = Entity::fresh_from_index(1).unwrap(); + let e1 = Entity::fresh_from_row(1).unwrap(); let values = vec![(e0, (B(0), C)), (e1, (B(1), C))]; @@ -1739,7 +1739,7 @@ mod tests { fn insert_or_spawn_batch_invalid() { let mut world = World::default(); let e0 = world.spawn(A(0)).id(); - let e1 = Entity::fresh_from_index(1).unwrap(); + let e1 = Entity::fresh_from_row(1).unwrap(); let e2 = world.spawn_empty().id(); let invalid_e2 = Entity::from_raw_and_generation(e2.row(), NonZero::::new(2).unwrap()); @@ -1872,7 +1872,7 @@ mod tests { fn try_insert_batch() { let mut world = World::default(); let e0 = world.spawn(A(0)).id(); - let e1 = Entity::fresh_from_index(1).unwrap(); + let e1 = Entity::fresh_from_row(1).unwrap(); let values = vec![(e0, (A(1), B(0))), (e1, (A(0), B(1)))]; @@ -1896,7 +1896,7 @@ mod tests { fn try_insert_batch_if_new() { let mut world = World::default(); let e0 = world.spawn(A(0)).id(); - let e1 = Entity::fresh_from_index(1).unwrap(); + let e1 = Entity::fresh_from_row(1).unwrap(); let values = vec![(e0, (A(1), B(0))), (e1, (A(0), B(1)))]; diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index dcc12371ad2ec..eb12aee123379 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -1894,7 +1894,7 @@ mod tests { let world_2 = World::new(); let mut query_state = world_1.query::(); - let _panics = query_state.get(&world_2, Entity::fresh_from_index(0).unwrap()); + let _panics = query_state.get(&world_2, Entity::fresh_from_row(0).unwrap()); } #[test] diff --git a/crates/bevy_remote/src/builtin_methods.rs b/crates/bevy_remote/src/builtin_methods.rs index fcd3de7081b4d..1edee9e51421b 100644 --- a/crates/bevy_remote/src/builtin_methods.rs +++ b/crates/bevy_remote/src/builtin_methods.rs @@ -1497,7 +1497,7 @@ mod tests { fn serialization_tests() { test_serialize_deserialize(BrpQueryRow { components: Default::default(), - entity: Entity::fresh_from_index(0).unwrap(), + entity: Entity::fresh_from_row(0).unwrap(), has: Default::default(), }); test_serialize_deserialize(BrpListWatchingResponse::default()); @@ -1511,7 +1511,7 @@ mod tests { ..Default::default() }); test_serialize_deserialize(BrpListParams { - entity: Entity::fresh_from_index(0).unwrap(), + entity: Entity::fresh_from_row(0).unwrap(), }); } } diff --git a/crates/bevy_transform/src/systems.rs b/crates/bevy_transform/src/systems.rs index 8cfd97877ac29..cf2bd2f9b85a6 100644 --- a/crates/bevy_transform/src/systems.rs +++ b/crates/bevy_transform/src/systems.rs @@ -798,8 +798,8 @@ mod test { let translation = vec3(1.0, 0.0, 0.0); // These will be overwritten. - let mut child = Entity::fresh_from_index(0).unwrap(); - let mut grandchild = Entity::fresh_from_index(1).unwrap(); + let mut child = Entity::fresh_from_row(0).unwrap(); + let mut grandchild = Entity::fresh_from_row(1).unwrap(); let parent = app .world_mut() .spawn(Transform::from_translation(translation)) @@ -849,7 +849,7 @@ mod test { ); fn setup_world(world: &mut World) -> (Entity, Entity) { - let mut grandchild = Entity::fresh_from_index(0).unwrap(); + let mut grandchild = Entity::fresh_from_row(0).unwrap(); let child = world .spawn(Transform::IDENTITY) .with_children(|builder| { diff --git a/crates/bevy_ui/src/layout/mod.rs b/crates/bevy_ui/src/layout/mod.rs index eefdf33c5130c..ddbe4e353997e 100644 --- a/crates/bevy_ui/src/layout/mod.rs +++ b/crates/bevy_ui/src/layout/mod.rs @@ -1039,7 +1039,7 @@ mod tests { let (mut world, ..) = setup_ui_test_world(); - let root_node_entity = Entity::fresh_from_index(1).unwrap(); + let root_node_entity = Entity::fresh_from_row(1).unwrap(); struct TestSystemParam { root_node_entity: Entity, diff --git a/crates/bevy_ui/src/layout/ui_surface.rs b/crates/bevy_ui/src/layout/ui_surface.rs index 3c8b16213a567..659696eef2726 100644 --- a/crates/bevy_ui/src/layout/ui_surface.rs +++ b/crates/bevy_ui/src/layout/ui_surface.rs @@ -318,7 +318,7 @@ mod tests { #[test] fn test_upsert() { let mut ui_surface = UiSurface::default(); - let root_node_entity = Entity::fresh_from_index(1).unwrap(); + let root_node_entity = Entity::fresh_from_row(1).unwrap(); let node = Node::default(); // standard upsert @@ -350,7 +350,7 @@ mod tests { #[test] fn test_remove_entities() { let mut ui_surface = UiSurface::default(); - let root_node_entity = Entity::fresh_from_index(1).unwrap(); + let root_node_entity = Entity::fresh_from_row(1).unwrap(); let node = Node::default(); ui_surface.upsert_node(&LayoutContext::TEST_CONTEXT, root_node_entity, &node, None); @@ -366,7 +366,7 @@ mod tests { #[test] fn test_try_update_measure() { let mut ui_surface = UiSurface::default(); - let root_node_entity = Entity::fresh_from_index(1).unwrap(); + let root_node_entity = Entity::fresh_from_row(1).unwrap(); let node = Node::default(); ui_surface.upsert_node(&LayoutContext::TEST_CONTEXT, root_node_entity, &node, None); @@ -381,8 +381,8 @@ mod tests { #[test] fn test_update_children() { let mut ui_surface = UiSurface::default(); - let root_node_entity = Entity::fresh_from_index(1).unwrap(); - let child_entity = Entity::fresh_from_index(2).unwrap(); + let root_node_entity = Entity::fresh_from_row(1).unwrap(); + let child_entity = Entity::fresh_from_row(2).unwrap(); let node = Node::default(); ui_surface.upsert_node(&LayoutContext::TEST_CONTEXT, root_node_entity, &node, None); @@ -402,8 +402,8 @@ mod tests { #[test] fn test_set_camera_children() { let mut ui_surface = UiSurface::default(); - let root_node_entity = Entity::fresh_from_index(1).unwrap(); - let child_entity = Entity::fresh_from_index(2).unwrap(); + let root_node_entity = Entity::fresh_from_row(1).unwrap(); + let child_entity = Entity::fresh_from_row(2).unwrap(); let node = Node::default(); ui_surface.upsert_node(&LayoutContext::TEST_CONTEXT, root_node_entity, &node, None); diff --git a/examples/ecs/error_handling.rs b/examples/ecs/error_handling.rs index 16dbf04b4c516..9f98092cca06d 100644 --- a/examples/ecs/error_handling.rs +++ b/examples/ecs/error_handling.rs @@ -169,7 +169,7 @@ fn failing_system(world: &mut World) -> Result { fn failing_commands(mut commands: Commands) { commands // This entity doesn't exist! - .entity(Entity::fresh_from_index(12345678).unwrap()) + .entity(Entity::fresh_from_row(12345678).unwrap()) // Normally, this failed command would panic, // but since we've set the global error handler to `warn` // it will log a warning instead. From f66efff1855f83aeb5d300366c84067c04eefdaa Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Sun, 20 Apr 2025 20:56:56 -0400 Subject: [PATCH 31/38] fix docs --- crates/bevy_ecs/src/query/state.rs | 10 +++++----- crates/bevy_ecs/src/system/query.rs | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index eb12aee123379..fd99641f9e76e 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -984,7 +984,7 @@ impl QueryState { /// /// assert_eq!(component_values, [&A(0), &A(1), &A(2)]); /// - /// let wrong_entity = Entity::fresh_from_index(365).unwrap(); + /// let wrong_entity = Entity::fresh_from_row(365).unwrap(); /// /// assert_eq!(match query_state.get_many(&mut world, [wrong_entity]).unwrap_err() {QueryEntityError::EntityDoesNotExist(error) => error.entity, _ => panic!()}, wrong_entity); /// ``` @@ -1022,7 +1022,7 @@ impl QueryState { /// /// assert_eq!(component_values, [&A(0), &A(1), &A(2)]); /// - /// let wrong_entity = Entity::fresh_from_index(365).unwrap(); + /// let wrong_entity = Entity::fresh_from_row(365).unwrap(); /// /// assert_eq!(match query_state.get_many_unique(&mut world, UniqueEntityArray::from([wrong_entity])).unwrap_err() {QueryEntityError::EntityDoesNotExist(error) => error.entity, _ => panic!()}, wrong_entity); /// ``` @@ -1078,7 +1078,7 @@ impl QueryState { /// /// assert_eq!(component_values, [&A(5), &A(6), &A(7)]); /// - /// let wrong_entity = Entity::fresh_from_index(57).unwrap(); + /// let wrong_entity = Entity::fresh_from_row(57).unwrap(); /// let invalid_entity = world.spawn_empty().id(); /// /// assert_eq!(match query_state.get_many(&mut world, [wrong_entity]).unwrap_err() {QueryEntityError::EntityDoesNotExist(error) => error.entity, _ => panic!()}, wrong_entity); @@ -1124,7 +1124,7 @@ impl QueryState { /// /// assert_eq!(component_values, [&A(5), &A(6), &A(7)]); /// - /// let wrong_entity = Entity::fresh_from_index(57).unwrap(); + /// let wrong_entity = Entity::fresh_from_row(57).unwrap(); /// let invalid_entity = world.spawn_empty().id(); /// /// assert_eq!(match query_state.get_many_unique(&mut world, UniqueEntityArray::from([wrong_entity])).unwrap_err() {QueryEntityError::EntityDoesNotExist(error) => error.entity, _ => panic!()}, wrong_entity); @@ -1452,7 +1452,7 @@ impl QueryState { /// /// # assert_eq!(component_values, [&A(5), &A(6), &A(7)]); /// - /// # let wrong_entity = Entity::fresh_from_index(57).unwrap(); + /// # let wrong_entity = Entity::fresh_from_row(57).unwrap(); /// # let invalid_entity = world.spawn_empty().id(); /// /// # assert_eq!(match query_state.get_many(&mut world, [wrong_entity]).unwrap_err() {QueryEntityError::EntityDoesNotExist(error) => error.entity, _ => panic!()}, wrong_entity); diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 74e2405d2ddb1..6db7d37f924e2 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -1414,7 +1414,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// /// assert_eq!(component_values, [&A(0), &A(1), &A(2)]); /// - /// let wrong_entity = Entity::fresh_from_index(365).unwrap(); + /// let wrong_entity = Entity::fresh_from_row(365).unwrap(); /// /// assert_eq!( /// match query.get_many([wrong_entity]).unwrap_err() { @@ -1466,7 +1466,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// /// assert_eq!(component_values, [&A(0), &A(1), &A(2)]); /// - /// let wrong_entity = Entity::fresh_from_index(365).unwrap(); + /// let wrong_entity = Entity::fresh_from_row(365).unwrap(); /// /// assert_eq!( /// match query.get_many_unique(UniqueEntityArray::from([wrong_entity])).unwrap_err() { @@ -1665,7 +1665,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// let entities: [Entity; 3] = entities.try_into().unwrap(); /// /// world.spawn(A(73)); - /// let wrong_entity = Entity::fresh_from_index(57).unwrap(); + /// let wrong_entity = Entity::fresh_from_row(57).unwrap(); /// let invalid_entity = world.spawn_empty().id(); /// /// @@ -1740,7 +1740,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// let entity_set: UniqueEntityArray<3> = entity_set.try_into().unwrap(); /// /// world.spawn(A(73)); - /// let wrong_entity = Entity::fresh_from_index(57).unwrap(); + /// let wrong_entity = Entity::fresh_from_row(57).unwrap(); /// let invalid_entity = world.spawn_empty().id(); /// /// From 3cf5c7dd3d5d35c75a969c2be5c9b26e53653698 Mon Sep 17 00:00:00 2001 From: Eagster <79881080+ElliottjPierce@users.noreply.github.com> Date: Tue, 22 Apr 2025 10:40:23 -0400 Subject: [PATCH 32/38] Use better comment Co-authored-by: atlv --- crates/bevy_ecs/src/entity/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index d8ceebf2730d8..5e9b043e513f8 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -401,7 +401,7 @@ impl Entity { Self::from_raw_and_generation(row, NonZero::::MIN) } - /// This is equivalent to [`from_raw`](Self::from_raw) except that it uses an row instead of an [`EntityRow`]. + /// This is equivalent to [`from_raw`](Self::from_raw) except that it takes a `u32` instead of an [`EntityRow`]. /// /// Returns `None` if the row is `u32::MAX`. #[inline(always)] From 79a297eb1779ed9a9b91a565bc36858f205c1dad Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Tue, 6 May 2025 17:44:08 -0400 Subject: [PATCH 33/38] fix new test --- .../src/relationship/relationship_source_collection.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/relationship/relationship_source_collection.rs b/crates/bevy_ecs/src/relationship/relationship_source_collection.rs index 49e3725f6a6b8..91c7ac3a9d0e0 100644 --- a/crates/bevy_ecs/src/relationship/relationship_source_collection.rs +++ b/crates/bevy_ecs/src/relationship/relationship_source_collection.rs @@ -709,7 +709,7 @@ mod tests { let collection = rel_target.collection(); // Insertions should maintain ordering - assert!(collection.iter().eq(&[b, c, d])); + assert!(collection.iter().eq(&[d, c, b])); world.entity_mut(c).despawn(); @@ -717,7 +717,7 @@ mod tests { let collection = rel_target.collection(); // Removals should maintain ordering - assert!(collection.iter().eq(&[b, d])); + assert!(collection.iter().eq(&[d, b])); } #[test] From 28c08f0da41bf908ea2509c4d6ed6fb7f572259d Mon Sep 17 00:00:00 2001 From: Eagster <79881080+ElliottjPierce@users.noreply.github.com> Date: Tue, 6 May 2025 21:39:41 -0400 Subject: [PATCH 34/38] Improve migration guide Co-authored-by: Zachary Harrold --- .../migration-guides/make_entity_index_non_max.md | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/release-content/migration-guides/make_entity_index_non_max.md b/release-content/migration-guides/make_entity_index_non_max.md index ee7dacdbc4753..b8b2e8c07db40 100644 --- a/release-content/migration-guides/make_entity_index_non_max.md +++ b/release-content/migration-guides/make_entity_index_non_max.md @@ -3,15 +3,19 @@ title: Manual Entity Creation pull_requests: [18704] --- -`Entity` no longer stores its incex 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::fresh_from_index` which is identical to the previous `Entity::from_raw`, except that it now returns `Option` where the results is `None` if `u32::MAX` is passed. +`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::fresh_from_index` 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. +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: +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::fresh_from_index(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::fresh_from_index(my_index)`. +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::fresh_from_index(my_index)`. From 2cbed0b3f6ec30e9ed69d08c355ffaab37c73a58 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Tue, 6 May 2025 21:46:16 -0400 Subject: [PATCH 35/38] rename from_raw_u32 --- crates/bevy_ecs/src/entity/map_entities.rs | 8 +++----- crates/bevy_ecs/src/entity/mod.rs | 2 +- crates/bevy_ecs/src/lib.rs | 8 ++++---- crates/bevy_ecs/src/query/state.rs | 2 +- crates/bevy_remote/src/builtin_methods.rs | 4 ++-- crates/bevy_transform/src/systems.rs | 6 +++--- crates/bevy_ui/src/layout/mod.rs | 2 +- crates/bevy_ui/src/layout/ui_surface.rs | 14 +++++++------- examples/ecs/error_handling.rs | 2 +- .../migration-guides/make_entity_index_non_max.md | 6 +++--- 10 files changed, 26 insertions(+), 28 deletions(-) diff --git a/crates/bevy_ecs/src/entity/map_entities.rs b/crates/bevy_ecs/src/entity/map_entities.rs index 3fc952bd0eb3b..6e2c11160c052 100644 --- a/crates/bevy_ecs/src/entity/map_entities.rs +++ b/crates/bevy_ecs/src/entity/map_entities.rs @@ -343,7 +343,7 @@ mod tests { let mut world = World::new(); let mut mapper = SceneEntityMapper::new(&mut map, &mut world); - let mapped_ent = Entity::fresh_from_row(1).unwrap(); + let mapped_ent = Entity::from_raw_u32(1).unwrap(); let dead_ref = mapper.get_mapped(mapped_ent); assert_eq!( @@ -352,9 +352,7 @@ mod tests { "should persist the allocated mapping from the previous line" ); assert_eq!( - mapper - .get_mapped(Entity::fresh_from_row(2).unwrap()) - .index(), + mapper.get_mapped(Entity::from_raw_u32(2).unwrap()).index(), dead_ref.index(), "should re-use the same index for further dead refs" ); @@ -372,7 +370,7 @@ mod tests { let mut world = World::new(); let dead_ref = SceneEntityMapper::world_scope(&mut map, &mut world, |_, mapper| { - mapper.get_mapped(Entity::fresh_from_row(0).unwrap()) + mapper.get_mapped(Entity::from_raw_u32(0).unwrap()) }); // Next allocated entity should be a further generation on the same index diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index 7911073e7e189..02311edf426dd 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -395,7 +395,7 @@ impl Entity { /// /// Returns `None` if the row is `u32::MAX`. #[inline(always)] - pub const fn fresh_from_row(row: u32) -> Option { + pub const fn from_raw_u32(row: u32) -> Option { match NonMaxU32::new(row) { Some(row) => Some(Self::from_raw(EntityRow::new(row))), None => None, diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index e325be1d336ec..15bfb49f6795f 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -1540,8 +1540,8 @@ mod tests { let mut world_a = World::new(); let world_b = World::new(); let mut query = world_a.query::<&A>(); - let _ = query.get(&world_a, Entity::fresh_from_row(0).unwrap()); - let _ = query.get(&world_b, Entity::fresh_from_row(0).unwrap()); + let _ = query.get(&world_a, Entity::from_raw_u32(0).unwrap()); + let _ = query.get(&world_b, Entity::from_raw_u32(0).unwrap()); } #[test] @@ -1782,7 +1782,7 @@ mod tests { fn try_insert_batch() { let mut world = World::default(); let e0 = world.spawn(A(0)).id(); - let e1 = Entity::fresh_from_row(1).unwrap(); + let e1 = Entity::from_raw_u32(1).unwrap(); let values = vec![(e0, (A(1), B(0))), (e1, (A(0), B(1)))]; @@ -1806,7 +1806,7 @@ mod tests { fn try_insert_batch_if_new() { let mut world = World::default(); let e0 = world.spawn(A(0)).id(); - let e1 = Entity::fresh_from_row(1).unwrap(); + let e1 = Entity::from_raw_u32(1).unwrap(); let values = vec![(e0, (A(1), B(0))), (e1, (A(0), B(1)))]; diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 4ef3ec5cb4f0a..8654fc8e69adb 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -1903,7 +1903,7 @@ mod tests { let world_2 = World::new(); let mut query_state = world_1.query::(); - let _panics = query_state.get(&world_2, Entity::fresh_from_row(0).unwrap()); + let _panics = query_state.get(&world_2, Entity::from_raw_u32(0).unwrap()); } #[test] diff --git a/crates/bevy_remote/src/builtin_methods.rs b/crates/bevy_remote/src/builtin_methods.rs index 229369da5c7aa..b59f08604ba7b 100644 --- a/crates/bevy_remote/src/builtin_methods.rs +++ b/crates/bevy_remote/src/builtin_methods.rs @@ -1497,7 +1497,7 @@ mod tests { fn serialization_tests() { test_serialize_deserialize(BrpQueryRow { components: Default::default(), - entity: Entity::fresh_from_row(0).unwrap(), + entity: Entity::from_raw_u32(0).unwrap(), has: Default::default(), }); test_serialize_deserialize(BrpListWatchingResponse::default()); @@ -1511,7 +1511,7 @@ mod tests { ..Default::default() }); test_serialize_deserialize(BrpListParams { - entity: Entity::fresh_from_row(0).unwrap(), + entity: Entity::from_raw_u32(0).unwrap(), }); } } diff --git a/crates/bevy_transform/src/systems.rs b/crates/bevy_transform/src/systems.rs index cf2bd2f9b85a6..62038b37ed90f 100644 --- a/crates/bevy_transform/src/systems.rs +++ b/crates/bevy_transform/src/systems.rs @@ -798,8 +798,8 @@ mod test { let translation = vec3(1.0, 0.0, 0.0); // These will be overwritten. - let mut child = Entity::fresh_from_row(0).unwrap(); - let mut grandchild = Entity::fresh_from_row(1).unwrap(); + let mut child = Entity::from_raw_u32(0).unwrap(); + let mut grandchild = Entity::from_raw_u32(1).unwrap(); let parent = app .world_mut() .spawn(Transform::from_translation(translation)) @@ -849,7 +849,7 @@ mod test { ); fn setup_world(world: &mut World) -> (Entity, Entity) { - let mut grandchild = Entity::fresh_from_row(0).unwrap(); + let mut grandchild = Entity::from_raw_u32(0).unwrap(); let child = world .spawn(Transform::IDENTITY) .with_children(|builder| { diff --git a/crates/bevy_ui/src/layout/mod.rs b/crates/bevy_ui/src/layout/mod.rs index 2b0814b8150e3..9e6906b1f7f69 100644 --- a/crates/bevy_ui/src/layout/mod.rs +++ b/crates/bevy_ui/src/layout/mod.rs @@ -1039,7 +1039,7 @@ mod tests { let (mut world, ..) = setup_ui_test_world(); - let root_node_entity = Entity::fresh_from_row(1).unwrap(); + let root_node_entity = Entity::from_raw_u32(1).unwrap(); struct TestSystemParam { root_node_entity: Entity, diff --git a/crates/bevy_ui/src/layout/ui_surface.rs b/crates/bevy_ui/src/layout/ui_surface.rs index d99f281ad7790..2df6afa947dac 100644 --- a/crates/bevy_ui/src/layout/ui_surface.rs +++ b/crates/bevy_ui/src/layout/ui_surface.rs @@ -318,7 +318,7 @@ mod tests { #[test] fn test_upsert() { let mut ui_surface = UiSurface::default(); - let root_node_entity = Entity::fresh_from_row(1).unwrap(); + let root_node_entity = Entity::from_raw_u32(1).unwrap(); let node = Node::default(); // standard upsert @@ -350,7 +350,7 @@ mod tests { #[test] fn test_remove_entities() { let mut ui_surface = UiSurface::default(); - let root_node_entity = Entity::fresh_from_row(1).unwrap(); + let root_node_entity = Entity::from_raw_u32(1).unwrap(); let node = Node::default(); ui_surface.upsert_node(&LayoutContext::TEST_CONTEXT, root_node_entity, &node, None); @@ -366,7 +366,7 @@ mod tests { #[test] fn test_try_update_measure() { let mut ui_surface = UiSurface::default(); - let root_node_entity = Entity::fresh_from_row(1).unwrap(); + let root_node_entity = Entity::from_raw_u32(1).unwrap(); let node = Node::default(); ui_surface.upsert_node(&LayoutContext::TEST_CONTEXT, root_node_entity, &node, None); @@ -381,8 +381,8 @@ mod tests { #[test] fn test_update_children() { let mut ui_surface = UiSurface::default(); - let root_node_entity = Entity::fresh_from_row(1).unwrap(); - let child_entity = Entity::fresh_from_row(2).unwrap(); + let root_node_entity = Entity::from_raw_u32(1).unwrap(); + let child_entity = Entity::from_raw_u32(2).unwrap(); let node = Node::default(); ui_surface.upsert_node(&LayoutContext::TEST_CONTEXT, root_node_entity, &node, None); @@ -402,8 +402,8 @@ mod tests { #[test] fn test_set_camera_children() { let mut ui_surface = UiSurface::default(); - let root_node_entity = Entity::fresh_from_row(1).unwrap(); - let child_entity = Entity::fresh_from_row(2).unwrap(); + let root_node_entity = Entity::from_raw_u32(1).unwrap(); + let child_entity = Entity::from_raw_u32(2).unwrap(); let node = Node::default(); ui_surface.upsert_node(&LayoutContext::TEST_CONTEXT, root_node_entity, &node, None); diff --git a/examples/ecs/error_handling.rs b/examples/ecs/error_handling.rs index 9f98092cca06d..d326d7d4aae81 100644 --- a/examples/ecs/error_handling.rs +++ b/examples/ecs/error_handling.rs @@ -169,7 +169,7 @@ fn failing_system(world: &mut World) -> Result { fn failing_commands(mut commands: Commands) { commands // This entity doesn't exist! - .entity(Entity::fresh_from_row(12345678).unwrap()) + .entity(Entity::from_raw_u32(12345678).unwrap()) // Normally, this failed command would panic, // but since we've set the global error handler to `warn` // it will log a warning instead. diff --git a/release-content/migration-guides/make_entity_index_non_max.md b/release-content/migration-guides/make_entity_index_non_max.md index b8b2e8c07db40..8d167180b9243 100644 --- a/release-content/migration-guides/make_entity_index_non_max.md +++ b/release-content/migration-guides/make_entity_index_non_max.md @@ -3,7 +3,7 @@ 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`. If you don't want to add [nonmax](https://docs.rs/nonmax/latest/nonmax/) as a dependency, use `Entity::fresh_from_index` 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. +`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. @@ -13,9 +13,9 @@ To migrate tests, use: ```diff - let entity = Entity::from_raw(1); -+ let entity = Entity::fresh_from_index(1).unwrap(); ++ 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::fresh_from_index(my_index)`. +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)`. From f3735974226dbbd2cc5941b90c89dd70c1c485d3 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Tue, 6 May 2025 21:46:46 -0400 Subject: [PATCH 36/38] new line for migration guide --- .../migration-guides/make_entity_index_non_max.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/release-content/migration-guides/make_entity_index_non_max.md b/release-content/migration-guides/make_entity_index_non_max.md index 8d167180b9243..050673c2c94ed 100644 --- a/release-content/migration-guides/make_entity_index_non_max.md +++ b/release-content/migration-guides/make_entity_index_non_max.md @@ -3,7 +3,10 @@ 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`. 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. +`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. From 50fe2b25b0028552c265cbcd385fd29a5853f44e Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Tue, 6 May 2025 21:59:27 -0400 Subject: [PATCH 37/38] fix docs --- crates/bevy_ecs/src/query/state.rs | 10 +++++----- crates/bevy_ecs/src/system/query.rs | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 8654fc8e69adb..8f3eb1b3fb09c 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -993,7 +993,7 @@ impl QueryState { /// /// assert_eq!(component_values, [&A(0), &A(1), &A(2)]); /// - /// let wrong_entity = Entity::fresh_from_row(365).unwrap(); + /// let wrong_entity = Entity::from_raw_u32(365).unwrap(); /// /// assert_eq!(match query_state.get_many(&mut world, [wrong_entity]).unwrap_err() {QueryEntityError::EntityDoesNotExist(error) => error.entity, _ => panic!()}, wrong_entity); /// ``` @@ -1031,7 +1031,7 @@ impl QueryState { /// /// assert_eq!(component_values, [&A(0), &A(1), &A(2)]); /// - /// let wrong_entity = Entity::fresh_from_row(365).unwrap(); + /// let wrong_entity = Entity::from_raw_u32(365).unwrap(); /// /// assert_eq!(match query_state.get_many_unique(&mut world, UniqueEntityArray::from([wrong_entity])).unwrap_err() {QueryEntityError::EntityDoesNotExist(error) => error.entity, _ => panic!()}, wrong_entity); /// ``` @@ -1087,7 +1087,7 @@ impl QueryState { /// /// assert_eq!(component_values, [&A(5), &A(6), &A(7)]); /// - /// let wrong_entity = Entity::fresh_from_row(57).unwrap(); + /// let wrong_entity = Entity::from_raw_u32(57).unwrap(); /// let invalid_entity = world.spawn_empty().id(); /// /// assert_eq!(match query_state.get_many(&mut world, [wrong_entity]).unwrap_err() {QueryEntityError::EntityDoesNotExist(error) => error.entity, _ => panic!()}, wrong_entity); @@ -1133,7 +1133,7 @@ impl QueryState { /// /// assert_eq!(component_values, [&A(5), &A(6), &A(7)]); /// - /// let wrong_entity = Entity::fresh_from_row(57).unwrap(); + /// let wrong_entity = Entity::from_raw_u32(57).unwrap(); /// let invalid_entity = world.spawn_empty().id(); /// /// assert_eq!(match query_state.get_many_unique(&mut world, UniqueEntityArray::from([wrong_entity])).unwrap_err() {QueryEntityError::EntityDoesNotExist(error) => error.entity, _ => panic!()}, wrong_entity); @@ -1461,7 +1461,7 @@ impl QueryState { /// /// # assert_eq!(component_values, [&A(5), &A(6), &A(7)]); /// - /// # let wrong_entity = Entity::fresh_from_row(57).unwrap(); + /// # let wrong_entity = Entity::from_raw_u32(57).unwrap(); /// # let invalid_entity = world.spawn_empty().id(); /// /// # assert_eq!(match query_state.get_many(&mut world, [wrong_entity]).unwrap_err() {QueryEntityError::EntityDoesNotExist(error) => error.entity, _ => panic!()}, wrong_entity); diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 80d013ff34cb2..b097e229177f7 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -1414,7 +1414,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// /// assert_eq!(component_values, [&A(0), &A(1), &A(2)]); /// - /// let wrong_entity = Entity::fresh_from_row(365).unwrap(); + /// let wrong_entity = Entity::from_raw_u32(365).unwrap(); /// /// assert_eq!( /// match query.get_many([wrong_entity]).unwrap_err() { @@ -1466,7 +1466,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// /// assert_eq!(component_values, [&A(0), &A(1), &A(2)]); /// - /// let wrong_entity = Entity::fresh_from_row(365).unwrap(); + /// let wrong_entity = Entity::from_raw_u32(365).unwrap(); /// /// assert_eq!( /// match query.get_many_unique(UniqueEntityArray::from([wrong_entity])).unwrap_err() { @@ -1665,7 +1665,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// let entities: [Entity; 3] = entities.try_into().unwrap(); /// /// world.spawn(A(73)); - /// let wrong_entity = Entity::fresh_from_row(57).unwrap(); + /// let wrong_entity = Entity::from_raw_u32(57).unwrap(); /// let invalid_entity = world.spawn_empty().id(); /// /// @@ -1740,7 +1740,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// let entity_set: UniqueEntityArray<3> = entity_set.try_into().unwrap(); /// /// world.spawn(A(73)); - /// let wrong_entity = Entity::fresh_from_row(57).unwrap(); + /// let wrong_entity = Entity::from_raw_u32(57).unwrap(); /// let invalid_entity = world.spawn_empty().id(); /// /// From f3602943262119349cc2e370f11ffc610c638f24 Mon Sep 17 00:00:00 2001 From: Eagster <79881080+ElliottjPierce@users.noreply.github.com> Date: Wed, 7 May 2025 11:01:28 -0400 Subject: [PATCH 38/38] Include ord in migration guide Co-authored-by: atlv --- release-content/migration-guides/make_entity_index_non_max.md | 1 + 1 file changed, 1 insertion(+) diff --git a/release-content/migration-guides/make_entity_index_non_max.md b/release-content/migration-guides/make_entity_index_non_max.md index 050673c2c94ed..d3ba963854254 100644 --- a/release-content/migration-guides/make_entity_index_non_max.md +++ b/release-content/migration-guides/make_entity_index_non_max.md @@ -6,6 +6,7 @@ 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`.