From 8a8293b2665bef8d6d4661f685d34b54b7e5d714 Mon Sep 17 00:00:00 2001 From: David Sugar Date: Wed, 29 Dec 2021 20:49:00 +0000 Subject: [PATCH] Renamed Entity::new to Entity::from_raw (#3465) # Objective - Rename `Entity::new(id: u32)` to `Entity::from_raw(id: u32)`. - Add further documentation. - fixes #3108 ## Solution - Renamed `Entity::new(id: u32)` to `Entity::from_raw(id: u32)`. - Docs extended. I derived the examples from the discussion of issue #3108 . The [first case](https://github.com/bevyengine/bevy/issues/3108#issuecomment-966669781) mentioned in the linked issue is quite obvious but the [second one](https://github.com/bevyengine/bevy/issues/3108#issuecomment-967093902) probably needs further explanation. Co-authored-by: r4gus --- benches/benches/bevy_ecs/commands.rs | 4 +- benches/benches/bevy_ecs/world_get.rs | 10 ++-- crates/bevy_ecs/src/entity/mod.rs | 47 +++++++++++++++++-- crates/bevy_ecs/src/entity/serde.rs | 2 +- crates/bevy_ecs/src/lib.rs | 8 ++-- crates/bevy_ecs/src/storage/sparse_set.rs | 10 ++-- crates/bevy_ecs/src/storage/table.rs | 2 +- crates/bevy_scene/src/dynamic_scene.rs | 2 +- .../bevy_transform/src/components/parent.rs | 4 +- 9 files changed, 64 insertions(+), 25 deletions(-) diff --git a/benches/benches/bevy_ecs/commands.rs b/benches/benches/bevy_ecs/commands.rs index c4d974238d..3d78a16dec 100644 --- a/benches/benches/bevy_ecs/commands.rs +++ b/benches/benches/bevy_ecs/commands.rs @@ -250,7 +250,7 @@ fn get_or_spawn(criterion: &mut Criterion) { let mut commands = Commands::new(&mut command_queue, &world); for i in 0..10_000 { commands - .get_or_spawn(Entity::new(i)) + .get_or_spawn(Entity::from_raw(i)) .insert_bundle((Matrix::default(), Vec3::default())); } command_queue.apply(&mut world); @@ -265,7 +265,7 @@ fn get_or_spawn(criterion: &mut Criterion) { let mut commands = Commands::new(&mut command_queue, &world); let mut values = Vec::with_capacity(10_000); for i in 0..10_000 { - values.push((Entity::new(i), (Matrix::default(), Vec3::default()))); + values.push((Entity::from_raw(i), (Matrix::default(), Vec3::default()))); } commands.insert_or_spawn_batch(values); command_queue.apply(&mut world); diff --git a/benches/benches/bevy_ecs/world_get.rs b/benches/benches/bevy_ecs/world_get.rs index fc943ede47..0cc420e2b3 100644 --- a/benches/benches/bevy_ecs/world_get.rs +++ b/benches/benches/bevy_ecs/world_get.rs @@ -37,7 +37,7 @@ fn world_entity(criterion: &mut Criterion) { bencher.iter(|| { for i in 0..entity_count { - let entity = Entity::new(i); + let entity = Entity::from_raw(i); black_box(world.entity(entity)); } }); @@ -58,7 +58,7 @@ fn world_get(criterion: &mut Criterion) { bencher.iter(|| { for i in 0..entity_count { - let entity = Entity::new(i); + let entity = Entity::from_raw(i); assert!(world.get::(entity).is_some()); } }); @@ -68,7 +68,7 @@ fn world_get(criterion: &mut Criterion) { bencher.iter(|| { for i in 0..entity_count { - let entity = Entity::new(i); + let entity = Entity::from_raw(i); assert!(world.get::(entity).is_some()); } }); @@ -90,7 +90,7 @@ fn world_query_get(criterion: &mut Criterion) { bencher.iter(|| { for i in 0..entity_count { - let entity = Entity::new(i); + let entity = Entity::from_raw(i); assert!(query.get(&world, entity).is_ok()); } }); @@ -101,7 +101,7 @@ fn world_query_get(criterion: &mut Criterion) { bencher.iter(|| { for i in 0..entity_count { - let entity = Entity::new(i); + let entity = Entity::from_raw(i); assert!(query.get(&world, entity).is_ok()); } }); diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index c74974d7a4..85e2e4a263 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -57,15 +57,54 @@ pub enum AllocAtWithoutReplacement { } impl Entity { - /// Creates a new entity reference with a generation of 0. + /// Creates a new entity reference with the specified `id` and a generation of 0. /// /// # Note /// - /// Spawning a specific `entity` value is rarely the right choice. Most apps should favor + /// Spawning a specific `entity` value is __rarely the right choice__. Most apps should favor /// [`Commands::spawn`](crate::system::Commands::spawn). This method should generally /// only be used for sharing entities across apps, and only when they have a scheme /// worked out to share an ID space (which doesn't happen by default). - pub fn new(id: u32) -> Entity { + /// + /// In general, one should not try to synchronize the ECS by attempting to ensure that + /// `Entity` lines up between instances, but instead insert a secondary identifier as + /// a component. + /// + /// There are still some use cases where it might be appropriate to use this function + /// externally. + /// + /// ## Examples + /// + /// Initializing a collection (e.g. `array` or `Vec`) with a known size: + /// + /// ```no_run + /// # use bevy_ecs::prelude::*; + /// // Create a new array of size 10 and initialize it with (invalid) entities. + /// let mut entities: [Entity; 10] = [Entity::from_raw(0); 10]; + /// + /// // ... replace the entities with valid ones. + /// ``` + /// + /// Deriving `Reflect` for a component that has an `Entity` field: + /// + /// ```no_run + /// # use bevy_ecs::{prelude::*, component::*}; + /// # use bevy_reflect::Reflect; + /// #[derive(Reflect, Component)] + /// #[reflect(Component)] + /// pub struct MyStruct { + /// pub entity: Entity, + /// } + /// + /// impl FromWorld for MyStruct { + /// fn from_world(_world: &mut World) -> Self { + /// Self { + /// entity: Entity::from_raw(u32::MAX), + /// } + /// } + /// } + /// ``` + pub fn from_raw(id: u32) -> Entity { Entity { id, generation: 0 } } @@ -120,7 +159,7 @@ impl SparseSetIndex for Entity { } fn get_sparse_set_index(value: usize) -> Self { - Entity::new(value as u32) + Entity::from_raw(value as u32) } } diff --git a/crates/bevy_ecs/src/entity/serde.rs b/crates/bevy_ecs/src/entity/serde.rs index 5c589bda4d..94b46468c2 100644 --- a/crates/bevy_ecs/src/entity/serde.rs +++ b/crates/bevy_ecs/src/entity/serde.rs @@ -32,6 +32,6 @@ impl<'de> Visitor<'de> for EntityVisitor { where E: serde::de::Error, { - Ok(Entity::new(v)) + Ok(Entity::from_raw(v)) } } diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index dfccb44440..30d70ca17e 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -1307,8 +1307,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::new(0)); - let _ = query.get(&world_b, Entity::new(0)); + let _ = query.get(&world_a, Entity::from_raw(0)); + let _ = query.get(&world_b, Entity::from_raw(0)); } #[test] @@ -1531,7 +1531,7 @@ mod tests { fn insert_or_spawn_batch() { let mut world = World::default(); let e0 = world.spawn().insert(A(0)).id(); - let e1 = Entity::new(1); + let e1 = Entity::from_raw(1); let values = vec![(e0, (B(0), C)), (e1, (B(1), C))]; @@ -1568,7 +1568,7 @@ mod tests { fn insert_or_spawn_batch_invalid() { let mut world = World::default(); let e0 = world.spawn().insert(A(0)).id(); - let e1 = Entity::new(1); + let e1 = Entity::from_raw(1); let e2 = world.spawn().id(); let invalid_e2 = Entity { generation: 1, diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs index ef0ab47550..ed11fdc658 100644 --- a/crates/bevy_ecs/src/storage/sparse_set.rs +++ b/crates/bevy_ecs/src/storage/sparse_set.rs @@ -448,11 +448,11 @@ mod tests { #[test] fn sparse_set() { let mut set = SparseSet::::default(); - let e0 = Entity::new(0); - let e1 = Entity::new(1); - let e2 = Entity::new(2); - let e3 = Entity::new(3); - let e4 = Entity::new(4); + 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); set.insert(e1, Foo(1)); set.insert(e2, Foo(2)); diff --git a/crates/bevy_ecs/src/storage/table.rs b/crates/bevy_ecs/src/storage/table.rs index d4bd5b471f..2d5c39eef0 100644 --- a/crates/bevy_ecs/src/storage/table.rs +++ b/crates/bevy_ecs/src/storage/table.rs @@ -540,7 +540,7 @@ mod tests { let columns = &[component_id]; let mut table = Table::with_capacity(0, columns.len()); table.add_column(components.get_info(component_id).unwrap()); - let entities = (0..200).map(Entity::new).collect::>(); + let entities = (0..200).map(Entity::from_raw).collect::>(); for entity in entities.iter() { // SAFE: we allocate and immediately set data afterwards unsafe { diff --git a/crates/bevy_scene/src/dynamic_scene.rs b/crates/bevy_scene/src/dynamic_scene.rs index 194590fd1c..ab15cbd61d 100644 --- a/crates/bevy_scene/src/dynamic_scene.rs +++ b/crates/bevy_scene/src/dynamic_scene.rs @@ -87,7 +87,7 @@ impl DynamicScene { // or spawn a new entity with a transiently unique id if there is // no corresponding entry. let entity = *entity_map - .entry(bevy_ecs::entity::Entity::new(scene_entity.entity)) + .entry(bevy_ecs::entity::Entity::from_raw(scene_entity.entity)) .or_insert_with(|| world.spawn().id()); // Apply/ add each component to the given entity. diff --git a/crates/bevy_transform/src/components/parent.rs b/crates/bevy_transform/src/components/parent.rs index c594d4c4da..5f03dc0653 100644 --- a/crates/bevy_transform/src/components/parent.rs +++ b/crates/bevy_transform/src/components/parent.rs @@ -17,7 +17,7 @@ pub struct Parent(pub Entity); // better ways to handle cases like this. impl FromWorld for Parent { fn from_world(_world: &mut World) -> Self { - Parent(Entity::new(u32::MAX)) + Parent(Entity::from_raw(u32::MAX)) } } @@ -56,6 +56,6 @@ impl MapEntities for PreviousParent { // TODO: Better handle this case see `impl FromWorld for Parent` impl FromWorld for PreviousParent { fn from_world(_world: &mut World) -> Self { - PreviousParent(Entity::new(u32::MAX)) + PreviousParent(Entity::from_raw(u32::MAX)) } }