From 7b03fca99c0e3b0a7d1c3534f2874385c793765f Mon Sep 17 00:00:00 2001 From: Imbris Date: Tue, 6 Jun 2023 00:57:22 -0400 Subject: [PATCH 1/5] Test that attempting to delete an entity with the wrong generation doesn't clear components --- tests/tests.rs | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/tests.rs b/tests/tests.rs index ded6c1cb5..4cbd17ca5 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -53,6 +53,35 @@ fn task_panics() { .dispatch(&mut world); } +#[test] +fn delete_wrong_gen() { + let mut world = create_world(); + + // create + let entity_a = world.create_entity().with(CompInt(7)).build(); + assert_eq!( + world.read_component::().get(entity_a), + Some(&CompInt(7)) + ); + // delete + assert!(world.delete_entity(entity_a).is_ok()); + assert_eq!(world.read_component::().get(entity_a), None); + // create + let entity_b = world.create_entity().with(CompInt(6)).build(); + assert_eq!( + world.read_component::().get(entity_b), + Some(&CompInt(6)) + ); + assert_eq!(world.read_component::().get(entity_a), None); + // delete stale + assert!(world.delete_entity(entity_a).is_err()); + assert_eq!( + world.read_component::().get(entity_b), + Some(&CompInt(6)) + ); + assert_eq!(world.read_component::().get(entity_a), None); +} + #[test] fn dynamic_create() { struct Sys; From 635ee215aec4db3674cc18977607177e31efdc98 Mon Sep 17 00:00:00 2001 From: Imbris Date: Tue, 6 Jun 2023 00:42:17 -0400 Subject: [PATCH 2/5] Delete components after killing entities to avoid clearing components on an entity with the wrong generation. Also removed some spots that redudantly try to ensure that `MetaTable>` is available since it is always inserted in `WorldExt::new`. --- src/world/entity.rs | 18 +++++++++++------- src/world/world_ext.rs | 24 +++++++++++++++--------- 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/src/world/entity.rs b/src/world/entity.rs index 2c18cc09f..03ea8d388 100644 --- a/src/world/entity.rs +++ b/src/world/entity.rs @@ -60,12 +60,16 @@ pub(crate) struct Allocator { impl Allocator { /// Kills a list of entities immediately. - pub fn kill(&mut self, delete: &[Entity]) -> Result<(), WrongGeneration> { - for &entity in delete { + /// + /// If an entity with an outdated generation is encountered, the index of + /// that entity within the provided slice is returned (entities after this + /// index are not killed). + pub fn kill(&mut self, delete: &[Entity]) -> Result<(), (WrongGeneration, usize)> { + for (index, &entity) in delete.iter().enumerate() { let id = entity.id() as usize; if !self.is_alive(entity) { - return self.del_err(entity); + return Err((self.del_err(entity), index)); } self.alive.remove(entity.id()); @@ -89,7 +93,7 @@ impl Allocator { /// maintained). pub fn kill_atomic(&self, e: Entity) -> Result<(), WrongGeneration> { if !self.is_alive(e) { - return self.del_err(e); + return Err(self.del_err(e)); } self.killed.add_atomic(e.id()); @@ -97,14 +101,14 @@ impl Allocator { Ok(()) } - pub(crate) fn del_err(&self, e: Entity) -> Result<(), WrongGeneration> { - Err(WrongGeneration { + pub(crate) fn del_err(&self, e: Entity) -> WrongGeneration { + WrongGeneration { action: "delete", actual_gen: self.generations[e.id() as usize] .0 .unwrap_or_else(Generation::one), entity: e, - }) + } } /// Return `true` if the entity is alive. diff --git a/src/world/world_ext.rs b/src/world/world_ext.rs index 49f667734..2d11ac73e 100644 --- a/src/world/world_ext.rs +++ b/src/world/world_ext.rs @@ -259,7 +259,11 @@ pub trait WorldExt { fn delete_entity(&mut self, entity: Entity) -> Result<(), WrongGeneration>; /// Deletes the specified entities and their components. - fn delete_entities(&mut self, delete: &[Entity]) -> Result<(), WrongGeneration>; + /// + /// If an entity with an outdated generation is encountered, the index of + /// that entity within the provided slice is returned (entities after this + /// index are not deleted). + fn delete_entities(&mut self, delete: &[Entity]) -> Result<(), (WrongGeneration, usize)>; /// Deletes all entities and their components. fn delete_all(&mut self); @@ -315,8 +319,6 @@ impl WorldExt for World { { self.entry() .or_insert_with(move || MaskedStorage::::new(storage())); - self.entry::>() - .or_insert_with(Default::default); self.fetch_mut::>() .register(&*self.fetch::>()); } @@ -369,12 +371,18 @@ impl WorldExt for World { fn delete_entity(&mut self, entity: Entity) -> Result<(), WrongGeneration> { self.delete_entities(&[entity]) + .map_err(|(wrong_gen, _)| wrong_gen) } - fn delete_entities(&mut self, delete: &[Entity]) -> Result<(), WrongGeneration> { - self.delete_components(delete); - - self.entities_mut().alloc.kill(delete) + fn delete_entities(&mut self, delete: &[Entity]) -> Result<(), (WrongGeneration, usize)> { + let res = self.entities_mut().alloc.kill(delete); + if let Err((wrong_gen, failed_index)) = res { + self.delete_components(&delete[..failed_index]); + Err((wrong_gen, failed_index)) + } else { + self.delete_components(delete); + Ok(()) + } } fn delete_all(&mut self) { @@ -406,8 +414,6 @@ impl WorldExt for World { } fn delete_components(&mut self, delete: &[Entity]) { - self.entry::>() - .or_insert_with(Default::default); for storage in self.fetch_mut::>().iter_mut(self) { storage.drop(delete); } From e2a730083efa2a7d783354da02cad87dce594989 Mon Sep 17 00:00:00 2001 From: Imbris Date: Tue, 6 Jun 2023 01:28:38 -0400 Subject: [PATCH 3/5] Update changelog, use action to install `cargo-hack` so that the MSRV doesn't need to follow it and it should be faster, add cargo-deny exception for Unicode-DFS-2016 license in unicode-ident --- .github/workflows/ci.yml | 3 ++- CHANGELOG.md | 3 +++ deny.toml | 4 ++++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d1d180b7f..abfd05d63 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -56,7 +56,8 @@ jobs: # if: matrix.toolchain == 'stable' && matrix.os == 'ubuntu-latest' # run tests - - run: cargo install cargo-hack + - name: install cargo-hack + uses: taiki-e/install-action@cargo-hack - run: cargo hack test --workspace --each-feature if: matrix.toolchain == 'nightly' - run: cargo hack test --workspace --each-feature --skip nightly diff --git a/CHANGELOG.md b/CHANGELOG.md index 7eadb362a..8bc49782e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,11 @@ # 0.19.0 (2022-07-15) +* Added index where entity deletion stopped to the error returned from `WorldExt::delete_entities` ([#766]) +* Fix bug where deleting an entity with the wrong generation could clear the components of an existing entity. ([#766]) * Bump shred to version `0.14.1`, MSRV to 1.60.0 ([shred changelog][shred-changelog], [#756]) [#756]: https://github.com/amethyst/specs/pull/756 +[#766]: https://github.com/amethyst/specs/pull/766 # 0.18.0 (2022-07-02) diff --git a/deny.toml b/deny.toml index d6bcf3a73..a11550548 100644 --- a/deny.toml +++ b/deny.toml @@ -28,3 +28,7 @@ allow = ["MIT", "Apache-2.0", "Unlicense", "BSD-3-Clause"] # We want really high confidence when inferring licenses from text confidence-threshold = 0.93 + +exceptions = [ + { allow = ["Unicode-DFS-2016"], name = "unicode-ident" }, +] From 420cb7d9f59852e9c7d210e41e3d57a9a636047a Mon Sep 17 00:00:00 2001 From: Imbris Date: Tue, 6 Jun 2023 21:44:34 -0400 Subject: [PATCH 4/5] Bump MSRV to 1.65.0 for nalgebra in the CI (even though it is in dev-dependencies, since we will need to bump the MSRV for other reasons soon enough) --- .github/workflows/ci.yml | 2 +- CHANGELOG.md | 1 + Cargo.toml | 2 +- README.md | 2 +- 4 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index abfd05d63..115296d26 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -25,7 +25,7 @@ jobs: fail-fast: true matrix: os: [macos-latest, windows-latest, ubuntu-latest] - toolchain: [stable, beta, nightly, 1.60.0] + toolchain: [stable, beta, nightly, 1.65.0] steps: - uses: actions/checkout@v2 diff --git a/CHANGELOG.md b/CHANGELOG.md index 8bc49782e..ac22cbbf6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # 0.19.0 (2022-07-15) +* Bump MSRV to 1.65.0 ([#766]) * Added index where entity deletion stopped to the error returned from `WorldExt::delete_entities` ([#766]) * Fix bug where deleting an entity with the wrong generation could clear the components of an existing entity. ([#766]) * Bump shred to version `0.14.1`, MSRV to 1.60.0 ([shred changelog][shred-changelog], [#756]) diff --git a/Cargo.toml b/Cargo.toml index eb9ea77e4..af12fd4f7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,7 +14,7 @@ license = "MIT OR Apache-2.0" authors = ["slide-rs hackers"] include = ["src", "README.md", "LICENSE-MIT", "LICENSE-APACHE"] edition = "2021" -rust-version = "1.60.0" +rust-version = "1.65.0" # the `storage_cmp` and `storage_sparse` benches are called from `benches_main` autobenches = false diff --git a/README.md b/README.md index b3f4b1e5c..a61bf18b9 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,7 @@ Unlike most other ECS libraries out there, it provides other and you can use barriers to force several stages in system execution * high performance for real-world applications -Minimum Rust version: 1.40 +Minimum Rust version: 1.65 ## [Link to the book][book] From 096218b83bd0f089ef81bb07c75606639233288a Mon Sep 17 00:00:00 2001 From: Imbris Date: Sat, 10 Jun 2023 12:30:44 -0400 Subject: [PATCH 5/5] Adjust date on release in changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ac22cbbf6..3a560457c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -# 0.19.0 (2022-07-15) +# 0.19.0 (2023-06-10) * Bump MSRV to 1.65.0 ([#766]) * Added index where entity deletion stopped to the error returned from `WorldExt::delete_entities` ([#766])