From 0bb175ed75e78578fcee64508c32c5ca8147a03a Mon Sep 17 00:00:00 2001 From: Anders429 Date: Wed, 9 Aug 2023 16:31:22 -0700 Subject: [PATCH 1/5] Check foreign_identifier_lookup when inserting with an entity type. --- src/archetypes/mod.rs | 70 ++++++++++++++++++++--------------------- src/world/impl_serde.rs | 20 ++++++++++++ 2 files changed, 55 insertions(+), 35 deletions(-) diff --git a/src/archetypes/mod.rs b/src/archetypes/mod.rs index 8ed6bea9..ce33530b 100644 --- a/src/archetypes/mod.rs +++ b/src/archetypes/mod.rs @@ -186,9 +186,6 @@ where } } - /// # Safety - /// `component_map` must contain an entry for each component in the entity `E`. Each entry must - /// correspond to its component's location in the registry `R`. pub(crate) unsafe fn get_mut_or_insert_new_for_entity(&mut self) -> &mut Archetype where E: Entity, @@ -209,48 +206,51 @@ where None => unsafe { unreachable_unchecked() }, } } else { - let identifier = R::create_archetype_identifier(); - - let hash = Self::make_hash( - // SAFETY: The `IdentifierRef` obtained here does not live longer than the - // `identifier_buffer`. - unsafe { identifier.as_ref() }, - &self.hash_builder, - ); + // Although type id lookup failed, that doesn't mean the archetype does not exist. We + // instead look up by the raw slice using `foreign_identifier_lookup`. + let identifier_buffer = R::create_archetype_identifier(); - if let Some(archetype_bucket) = self.raw_archetypes.find( - hash, - Self::equivalent_identifier( - // SAFETY: The `IdentifierRef` obtained here does not live longer than the - // `identifier_buffer`. - unsafe { identifier.as_ref() }, - ), + let archetype = if let Some(&identifier) = self.foreign_identifier_lookup.get( + // SAFETY: The slice created here does not outlive the `identifier_buffer`. + unsafe { identifier_buffer.as_slice() }, ) { - // SAFETY: This reference to the archetype contained in this bucket is unique. - unsafe { archetype_bucket.as_mut() } + if let Some(archetype) = self.raw_archetypes.get_mut( + Self::make_hash(identifier, &self.hash_builder), + Self::equivalent_identifier(identifier), + ) { + archetype + } else { + // SAFETY: Since the identifier was present in `foreign_identifier_lookup`, it + // is guaranteed to have an associated `archetype`. + unsafe { unreachable_unchecked() } + } } else { - self.type_id_lookup.insert( - TypeId::of::(), - // SAFETY: The `IdentifierRef` obtained here does not live longer than the - // `identifier_buffer`. - unsafe { identifier.as_ref() }, - ); - // SAFETY: Since the archetype is not contained anywhere in this container, it is - // invariantly guaranteed that the identifier is not contained in - // `foreign_identifier_lookup` either. Additionally, both the slice and - // `IdentifierRef` created here do not outlive `identifier`. + // SAFETY: This identifier has already been verified to not be contained in + // `foreign_identifier_lookup`. Additionally, the slice and `IdentifierRef` created + // here will not outlive the `identifier_buffer`. unsafe { self.foreign_identifier_lookup.insert_unique_unchecked( - &*(identifier.as_slice() as *const [u8]), - identifier.as_ref(), + &*(identifier_buffer.as_slice() as *const [u8]), + identifier_buffer.as_ref(), ); } self.raw_archetypes.insert_entry( - hash, - Archetype::new(identifier), + // SAFETY: The `IdentifierRef` created here does not outlive the + // `identifier_buffer`. + Self::make_hash(unsafe { identifier_buffer.as_ref() }, &self.hash_builder), + Archetype::new(identifier_buffer), Self::make_hasher(&self.hash_builder), ) - } + }; + + self.type_id_lookup.insert( + TypeId::of::(), + // SAFETY: The `IdentifierRef` obtained here does not live longer than the + // `identifier_buffer`. + unsafe { archetype.identifier() }, + ); + + archetype } } diff --git a/src/world/impl_serde.rs b/src/world/impl_serde.rs index 1965712e..9b0ead32 100644 --- a/src/world/impl_serde.rs +++ b/src/world/impl_serde.rs @@ -112,6 +112,7 @@ mod tests { use alloc::vec; use claims::{ assert_err_eq, + assert_ok, assert_ok_eq, }; use serde::{ @@ -493,4 +494,23 @@ mod tests { Error::invalid_length(2, &"serialized World") ); } + + #[test] + fn deserialize_then_mutate() { + let mut world = World::::new(); + world.insert(entity!(A(0))); + + let serializer = Serializer::builder().build(); + let tokens = assert_ok!(world.serialize(&serializer)); + + let mut deserializer = Deserializer::builder().tokens(tokens).build(); + let mut deserialized_world = assert_ok!(World::::deserialize( + &mut deserializer + )); + + world.insert(entity!(A(1))); + deserialized_world.insert(entity!(A(1))); + + assert_eq!(world, deserialized_world); + } } From 5ad96c2db68697f0557cd5ca7c0cdf335e8495bc Mon Sep 17 00:00:00 2001 From: Anders429 Date: Wed, 9 Aug 2023 16:40:16 -0700 Subject: [PATCH 2/5] Fix incorrect clone implementations. --- src/archetype/identifier/mod.rs | 6 +----- src/entity/allocator/location.rs | 5 +---- src/query/mod.rs | 2 +- 3 files changed, 3 insertions(+), 10 deletions(-) diff --git a/src/archetype/identifier/mod.rs b/src/archetype/identifier/mod.rs index 2f57492f..e8805f60 100644 --- a/src/archetype/identifier/mod.rs +++ b/src/archetype/identifier/mod.rs @@ -318,11 +318,7 @@ where R: Registry, { fn clone(&self) -> Self { - Self { - registry: PhantomData, - - pointer: self.pointer, - } + *self } } diff --git a/src/entity/allocator/location.rs b/src/entity/allocator/location.rs index e310137c..23aea8db 100644 --- a/src/entity/allocator/location.rs +++ b/src/entity/allocator/location.rs @@ -59,10 +59,7 @@ where R: Registry, { fn clone(&self) -> Self { - Self { - identifier: self.identifier, - index: self.index, - } + *self } } diff --git a/src/query/mod.rs b/src/query/mod.rs index 96241d66..9b330a8f 100644 --- a/src/query/mod.rs +++ b/src/query/mod.rs @@ -145,7 +145,7 @@ impl Clone for Query { fn clone(&self) -> Self { - Self::new() + *self } } From d82e2f51fdfd94712ec87fedfa354e0466a1e81e Mon Sep 17 00:00:00 2001 From: Anders429 Date: Wed, 9 Aug 2023 16:41:26 -0700 Subject: [PATCH 3/5] Look over reference to type id lookup container. --- src/archetypes/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/archetypes/mod.rs b/src/archetypes/mod.rs index ce33530b..14ce7899 100644 --- a/src/archetypes/mod.rs +++ b/src/archetypes/mod.rs @@ -439,7 +439,7 @@ where } } - for (&type_id, identifier) in self.type_id_lookup.iter() { + for (&type_id, identifier) in &self.type_id_lookup { cloned_archetypes.type_id_lookup.insert( type_id, // SAFETY: Each identifier in `self.type_id_lookup` is guaranteed to be found in @@ -515,7 +515,7 @@ where // // Note that no type id entries are removed here. New ones are just added, since the old // archetypes were just cleared, not removed entirely. - for (&type_id, identifier) in source.type_id_lookup.iter() { + for (&type_id, identifier) in &source.type_id_lookup { self.type_id_lookup.insert( type_id, // SAFETY: Each identifier in `source.type_id_lookup` is guaranteed to be found in From 1297cc2374bfd696a1eeaeb5a1c456abbcca3ce3 Mon Sep 17 00:00:00 2001 From: Anders429 Date: Wed, 9 Aug 2023 16:42:43 -0700 Subject: [PATCH 4/5] Update changelog. --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f38f1db..d1cb2582 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Unreleased ### Fixed - Dynamic scheduling now respects both `EntryViews` and `ResourceViews`. +- Mutating a deserialized `World` no longer creates duplicate `Archetype`s, instead correctly looking up the existing deserialized `Archetype`. ## 0.9.0 - 2023-04-22 ### Changed From 208618b4860f06d2db8cfea4dd96db217e9963e8 Mon Sep 17 00:00:00 2001 From: Anders429 Date: Wed, 9 Aug 2023 17:21:32 -0700 Subject: [PATCH 5/5] Fix stray lint. --- src/system/schedule/sendable.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/system/schedule/sendable.rs b/src/system/schedule/sendable.rs index d12ff83d..b7c1c1a1 100644 --- a/src/system/schedule/sendable.rs +++ b/src/system/schedule/sendable.rs @@ -30,7 +30,7 @@ where R: Registry, { fn clone(&self) -> Self { - Self(self.0) + *self } }