From 613561f40c42761172b72d73a2a1cee1b3130d4c Mon Sep 17 00:00:00 2001 From: Boxy Date: Fri, 24 Feb 2023 12:09:17 +0000 Subject: [PATCH 1/4] update archetype and table row in remove op --- crates/bevy_ecs/src/world/entity_ref.rs | 59 ++++++++++++++++++++++++- 1 file changed, 57 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 5a6f179ceee6b..12e47420fe7b0 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -369,8 +369,19 @@ impl<'w> EntityMut<'w> { ) { let old_archetype = &mut archetypes[old_archetype_id]; let remove_result = old_archetype.swap_remove(old_location.archetype_row); + // if an entity was moved into this entity's archetype row, update its archetype row if let Some(swapped_entity) = remove_result.swapped_entity { - entities.set(swapped_entity.index(), old_location); + let swapped_location = entities.get(swapped_entity).unwrap(); + + entities.set( + swapped_entity.index(), + EntityLocation { + archetype_id: swapped_location.archetype_id, + archetype_row: old_location.archetype_row, + table_id: swapped_location.table_id, + table_row: swapped_location.table_row, + }, + ); } let old_table_row = remove_result.table_row; let old_table_id = old_archetype.table_id(); @@ -393,9 +404,19 @@ impl<'w> EntityMut<'w> { // SAFETY: move_result.new_row is a valid position in new_archetype's table let new_location = new_archetype.allocate(entity, move_result.new_row); - // if an entity was moved into this entity's table spot, update its table row + // if an entity was moved into this entity's table row, update its table row if let Some(swapped_entity) = move_result.swapped_entity { let swapped_location = entities.get(swapped_entity).unwrap(); + + entities.set( + swapped_entity.index(), + EntityLocation { + archetype_id: swapped_location.archetype_id, + archetype_row: swapped_location.archetype_row, + table_id: swapped_location.table_id, + table_row: old_location.table_row, + }, + ); archetypes[swapped_location.archetype_id] .set_entity_table_row(swapped_location.archetype_row, old_table_row); } @@ -905,4 +926,38 @@ mod tests { // Ensure that the location has been properly updated. assert!(entity.location() != old_location); } + + #[test] + fn removing_sparse_updates_archetype_row() { + #[derive(Component, PartialEq, Debug)] + struct Dense(u8); + + #[derive(Component)] + #[component(storage = "SparseSet")] + struct Sparse; + + let mut world = World::new(); + let e1 = world.spawn((Dense(0), Sparse)).id(); + let e2 = world.spawn((Dense(1), Sparse)).id(); + + world.entity_mut(e1).remove::(); + assert_eq!(world.entity(e2).get::().unwrap(), &Dense(1)); + } + + #[test] + fn removing_dense_updates_table_row() { + #[derive(Component, PartialEq, Debug)] + struct Dense(u8); + + #[derive(Component)] + #[component(storage = "SparseSet")] + struct Sparse; + + let mut world = World::new(); + let e1 = world.spawn((Dense(0), Sparse)).id(); + let e2 = world.spawn((Dense(1), Sparse)).id(); + + world.entity_mut(e1).remove::(); + assert_eq!(world.entity(e2).get::().unwrap(), &Dense(1)); + } } From 9bba4680b773fc3f19fa64ed62f304653ae206b1 Mon Sep 17 00:00:00 2001 From: Boxy Date: Fri, 24 Feb 2023 17:20:10 +0000 Subject: [PATCH 2/4] update archetype and table row in insert op --- crates/bevy_ecs/src/bundle.rs | 31 +++++++++- crates/bevy_ecs/src/world/entity_ref.rs | 75 +++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 162fe07c0beda..ff4ceb03af2f7 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -555,7 +555,16 @@ impl<'a, 'b> BundleInserter<'a, 'b> { InsertBundleResult::NewArchetypeSameTable { new_archetype } => { let result = self.archetype.swap_remove(location.archetype_row); if let Some(swapped_entity) = result.swapped_entity { - self.entities.set(swapped_entity.index(), location); + let swapped_location = self.entities.get(swapped_entity).unwrap(); + self.entities.set( + swapped_entity.index(), + EntityLocation { + archetype_id: swapped_location.archetype_id, + archetype_row: location.archetype_row, + table_id: swapped_location.table_id, + table_row: swapped_location.table_row, + }, + ); } let new_location = new_archetype.allocate(entity, result.table_row); self.entities.set(entity.index(), new_location); @@ -583,7 +592,16 @@ impl<'a, 'b> BundleInserter<'a, 'b> { } => { let result = self.archetype.swap_remove(location.archetype_row); if let Some(swapped_entity) = result.swapped_entity { - self.entities.set(swapped_entity.index(), location); + let swapped_location = self.entities.get(swapped_entity).unwrap(); + self.entities.set( + swapped_entity.index(), + EntityLocation { + archetype_id: swapped_location.archetype_id, + archetype_row: location.archetype_row, + table_id: swapped_location.table_id, + table_row: swapped_location.table_row, + }, + ); } // PERF: store "non bundle" components in edge, then just move those to avoid // redundant copies @@ -608,6 +626,15 @@ impl<'a, 'b> BundleInserter<'a, 'b> { .add(swapped_location.archetype_id.index()) }; + self.entities.set( + swapped_entity.index(), + EntityLocation { + archetype_id: swapped_location.archetype_id, + archetype_row: swapped_location.archetype_row, + table_id: swapped_location.table_id, + table_row: result.table_row, + }, + ); swapped_archetype .set_entity_table_row(swapped_location.archetype_row, result.table_row); } diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 12e47420fe7b0..da6ab6e5b0c4c 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -960,4 +960,79 @@ mod tests { world.entity_mut(e1).remove::(); assert_eq!(world.entity(e2).get::().unwrap(), &Dense(1)); } + + #[test] + fn inserting_sparse_updates_archetype_row() { + use bevy_ecs::prelude::*; + + #[derive(Component, PartialEq, Debug)] + struct Dense(u8); + + #[derive(Component)] + #[component(storage = "SparseSet")] + struct Sparse; + + let mut world = World::new(); + let e1 = world.spawn(Dense(0)).id(); + let e2 = world.spawn(Dense(1)).id(); + + world.entity_mut(e1).insert(Sparse); + assert_eq!(world.entity(e2).get::().unwrap(), &Dense(1)); + } + + #[test] + fn inserting_dense_updates_archetype_row() { + use bevy_ecs::prelude::*; + + #[derive(Component, PartialEq, Debug)] + struct Dense(u8); + + #[derive(Component)] + struct Dense2; + + #[derive(Component)] + #[component(storage = "SparseSet")] + struct Sparse; + + let mut world = World::new(); + let e1 = world.spawn(Dense(0)).id(); + let e2 = world.spawn(Dense(1)).id(); + + world.entity_mut(e1).insert(Sparse).remove::(); + + // archetype with [e2, e1] + // table with [e1, e2] + + world.entity_mut(e2).insert(Dense2); + + assert_eq!(world.entity(e1).get::().unwrap(), &Dense(0)); + } + + #[test] + fn inserting_dense_updates_table_row() { + use bevy_ecs::prelude::*; + + #[derive(Component, PartialEq, Debug)] + struct Dense(u8); + + #[derive(Component)] + struct Dense2; + + #[derive(Component)] + #[component(storage = "SparseSet")] + struct Sparse; + + let mut world = World::new(); + let e1 = world.spawn(Dense(0)).id(); + let e2 = world.spawn(Dense(1)).id(); + + world.entity_mut(e1).insert(Sparse).remove::(); + + // archetype with [e2, e1] + // table with [e1, e2] + + world.entity_mut(e1).insert(Dense2); + + assert_eq!(world.entity(e2).get::().unwrap(), &Dense(1)); + } } From 9e76c86b3d3637716ea9ec8773bbe2d412ff7b2f Mon Sep 17 00:00:00 2001 From: Boxy Date: Fri, 24 Feb 2023 17:20:49 +0000 Subject: [PATCH 3/4] link PR from tests --- crates/bevy_ecs/src/world/entity_ref.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index da6ab6e5b0c4c..12ec7385c2059 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -927,6 +927,7 @@ mod tests { assert!(entity.location() != old_location); } + // regression test for https://github.com/bevyengine/bevy/pull/7805 #[test] fn removing_sparse_updates_archetype_row() { #[derive(Component, PartialEq, Debug)] @@ -944,6 +945,7 @@ mod tests { assert_eq!(world.entity(e2).get::().unwrap(), &Dense(1)); } + // regression test for https://github.com/bevyengine/bevy/pull/7805 #[test] fn removing_dense_updates_table_row() { #[derive(Component, PartialEq, Debug)] @@ -961,6 +963,7 @@ mod tests { assert_eq!(world.entity(e2).get::().unwrap(), &Dense(1)); } + // regression test for https://github.com/bevyengine/bevy/pull/7805 #[test] fn inserting_sparse_updates_archetype_row() { use bevy_ecs::prelude::*; @@ -980,6 +983,7 @@ mod tests { assert_eq!(world.entity(e2).get::().unwrap(), &Dense(1)); } + // regression test for https://github.com/bevyengine/bevy/pull/7805 #[test] fn inserting_dense_updates_archetype_row() { use bevy_ecs::prelude::*; @@ -1008,6 +1012,7 @@ mod tests { assert_eq!(world.entity(e1).get::().unwrap(), &Dense(0)); } + // regression test for https://github.com/bevyengine/bevy/pull/7805 #[test] fn inserting_dense_updates_table_row() { use bevy_ecs::prelude::*; From 884b4d40380c959f7ea7c1b00ff4abd4cdad4388 Mon Sep 17 00:00:00 2001 From: Boxy Date: Fri, 24 Feb 2023 17:47:48 +0000 Subject: [PATCH 4/4] update archetype and table row in despawn op --- crates/bevy_ecs/src/world/entity_ref.rs | 79 ++++++++++++++++++++++--- 1 file changed, 71 insertions(+), 8 deletions(-) diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 12ec7385c2059..0e6341d566e80 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -510,10 +510,19 @@ impl<'w> EntityMut<'w> { } let remove_result = archetype.swap_remove(location.archetype_row); if let Some(swapped_entity) = remove_result.swapped_entity { + let swapped_location = world.entities.get(swapped_entity).unwrap(); // SAFETY: swapped_entity is valid and the swapped entity's components are // moved to the new location immediately after. unsafe { - world.entities.set(swapped_entity.index(), location); + world.entities.set( + swapped_entity.index(), + EntityLocation { + archetype_id: swapped_location.archetype_id, + archetype_row: location.archetype_row, + table_id: swapped_location.table_id, + table_row: swapped_location.table_row, + }, + ); } } table_row = remove_result.table_row; @@ -530,6 +539,19 @@ impl<'w> EntityMut<'w> { if let Some(moved_entity) = moved_entity { let moved_location = world.entities.get(moved_entity).unwrap(); + // SAFETY: `moved_entity` is valid and the provided `EntityLocation` accurately reflects + // the current location of the entity and its component data. + unsafe { + world.entities.set( + moved_entity.index(), + EntityLocation { + archetype_id: moved_location.archetype_id, + archetype_row: moved_location.archetype_row, + table_id: moved_location.table_id, + table_row, + }, + ); + } world.archetypes[moved_location.archetype_id] .set_entity_table_row(moved_location.archetype_row, table_row); } @@ -966,8 +988,6 @@ mod tests { // regression test for https://github.com/bevyengine/bevy/pull/7805 #[test] fn inserting_sparse_updates_archetype_row() { - use bevy_ecs::prelude::*; - #[derive(Component, PartialEq, Debug)] struct Dense(u8); @@ -986,8 +1006,6 @@ mod tests { // regression test for https://github.com/bevyengine/bevy/pull/7805 #[test] fn inserting_dense_updates_archetype_row() { - use bevy_ecs::prelude::*; - #[derive(Component, PartialEq, Debug)] struct Dense(u8); @@ -1012,11 +1030,8 @@ mod tests { assert_eq!(world.entity(e1).get::().unwrap(), &Dense(0)); } - // regression test for https://github.com/bevyengine/bevy/pull/7805 #[test] fn inserting_dense_updates_table_row() { - use bevy_ecs::prelude::*; - #[derive(Component, PartialEq, Debug)] struct Dense(u8); @@ -1040,4 +1055,52 @@ mod tests { assert_eq!(world.entity(e2).get::().unwrap(), &Dense(1)); } + + // regression test for https://github.com/bevyengine/bevy/pull/7805 + #[test] + fn despawning_entity_updates_archetype_row() { + #[derive(Component, PartialEq, Debug)] + struct Dense(u8); + + #[derive(Component)] + #[component(storage = "SparseSet")] + struct Sparse; + + let mut world = World::new(); + let e1 = world.spawn(Dense(0)).id(); + let e2 = world.spawn(Dense(1)).id(); + + world.entity_mut(e1).insert(Sparse).remove::(); + + // archetype with [e2, e1] + // table with [e1, e2] + + world.entity_mut(e2).despawn(); + + assert_eq!(world.entity(e1).get::().unwrap(), &Dense(0)); + } + + // regression test for https://github.com/bevyengine/bevy/pull/7805 + #[test] + fn despawning_entity_updates_table_row() { + #[derive(Component, PartialEq, Debug)] + struct Dense(u8); + + #[derive(Component)] + #[component(storage = "SparseSet")] + struct Sparse; + + let mut world = World::new(); + let e1 = world.spawn(Dense(0)).id(); + let e2 = world.spawn(Dense(1)).id(); + + world.entity_mut(e1).insert(Sparse).remove::(); + + // archetype with [e2, e1] + // table with [e1, e2] + + world.entity_mut(e1).despawn(); + + assert_eq!(world.entity(e2).get::().unwrap(), &Dense(1)); + } }