[Merged by Bors] - Refactor ECS to reduce the dependency on a 1-to-1 mapping between components and real rust types #2490
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,3 @@ | ||
| use crate::storage::SparseSetIndex; | ||
| use std::{ | ||
| alloc::Layout, | ||
|
|
@@ -56,15 +52,8 @@ impl Default for StorageType { | |
|
|
||
| #[derive(Debug)] | ||
| pub struct ComponentInfo { | ||
| id: ComponentId, | ||
| descriptor: ComponentDescriptor, | ||
| } | ||
|
|
||
| impl ComponentInfo { | ||
|
|
@@ -75,44 +64,36 @@ impl ComponentInfo { | |
|
|
||
| #[inline] | ||
| pub fn name(&self) -> &str { | ||
| &self.descriptor.name | ||
| } | ||
|
|
||
| #[inline] | ||
| pub fn type_id(&self) -> Option<TypeId> { | ||
| self.descriptor.type_id | ||
| } | ||
|
|
||
| #[inline] | ||
| pub fn layout(&self) -> Layout { | ||
| self.descriptor.layout | ||
| } | ||
|
|
||
| #[inline] | ||
| pub fn drop(&self) -> unsafe fn(*mut u8) { | ||
| self.descriptor.drop | ||
| } | ||
|
|
||
| #[inline] | ||
| pub fn storage_type(&self) -> StorageType { | ||
| self.descriptor.storage_type | ||
| } | ||
|
|
||
| #[inline] | ||
| pub fn is_send_and_sync(&self) -> bool { | ||
| self.descriptor.is_send_and_sync | ||
| } | ||
|
|
||
| fn new(id: ComponentId, descriptor: ComponentDescriptor) -> Self { | ||
| ComponentInfo { id, descriptor } | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -142,6 +123,7 @@ impl SparseSetIndex for ComponentId { | |
| } | ||
| } | ||
|
|
||
| #[derive(Debug)] | ||
| pub struct ComponentDescriptor { | ||
| name: String, | ||
| storage_type: StorageType, | ||
|
|
@@ -154,14 +136,30 @@ pub struct ComponentDescriptor { | |
| } | ||
|
|
||
| impl ComponentDescriptor { | ||
| // SAFETY: The pointer points to a valid value of type `T` and it is safe to drop this value. | ||
| unsafe fn drop_ptr<T>(x: *mut u8) { | ||
| x.cast::<T>().drop_in_place() | ||
| } | ||
|
|
||
| pub fn new<T: Component>(storage_type: StorageType) -> Self { | ||
| Self { | ||
| name: std::any::type_name::<T>().to_string(), | ||
| storage_type, | ||
| is_send_and_sync: true, | ||
| type_id: Some(TypeId::of::<T>()), | ||
| layout: Layout::new::<T>(), | ||
| drop: Self::drop_ptr::<T>, | ||
| } | ||
| } | ||
|
|
||
| fn new_non_send<T: Any>(storage_type: StorageType) -> Self { | ||
| Self { | ||
| name: std::any::type_name::<T>().to_string(), | ||
| storage_type, | ||
| is_send_and_sync: false, | ||
| type_id: Some(TypeId::of::<T>()), | ||
| layout: Layout::new::<T>(), | ||
| drop: Self::drop_ptr::<T>, | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -181,19 +179,6 @@ impl ComponentDescriptor { | |
| } | ||
| } | ||
|
|
||
| #[derive(Debug, Default)] | ||
| pub struct Components { | ||
| components: Vec<ComponentInfo>, | ||
|
|
@@ -231,7 +216,12 @@ impl Components { | |
|
|
||
| #[inline] | ||
| pub fn get_or_insert_id<T: Component>(&mut self) -> ComponentId { | ||
| // SAFE: The [`ComponentDescriptor`] matches the [`TypeId`] | ||
| unsafe { | ||
| self.get_or_insert_with(TypeId::of::<T>(), || { | ||
| ComponentDescriptor::new::<T>(StorageType::default()) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| #[inline] | ||
|
|
@@ -279,42 +269,58 @@ impl Components { | |
|
|
||
| #[inline] | ||
| pub fn get_or_insert_resource_id<T: Component>(&mut self) -> ComponentId { | ||
| // SAFE: The [`ComponentDescriptor`] matches the [`TypeId`] | ||
| unsafe { | ||
| self.get_or_insert_resource_with(TypeId::of::<T>(), || { | ||
| ComponentDescriptor::new::<T>(StorageType::default()) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| #[inline] | ||
| pub fn get_or_insert_non_send_resource_id<T: Any>(&mut self) -> ComponentId { | ||
| // SAFE: The [`ComponentDescriptor`] matches the [`TypeId`] | ||
| unsafe { | ||
| self.get_or_insert_resource_with(TypeId::of::<T>(), || { | ||
| ComponentDescriptor::new_non_send::<T>(StorageType::default()) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| /// # Safety | ||
| /// | ||
| /// The [`ComponentDescriptor`] must match the [`TypeId`] | ||
| #[inline] | ||
| unsafe fn get_or_insert_resource_with( | ||
| &mut self, | ||
| type_id: TypeId, | ||
| func: impl FnOnce() -> ComponentDescriptor, | ||
|
There was a problem hiding this comment. do we know if perf is actually worse without this There was a problem hiding this comment.
There was a problem hiding this comment. type_name returns a There was a problem hiding this comment.
There was a problem hiding this comment. I think personally I'd rather drop the unsafe, use a Cow and then always take a There was a problem hiding this comment.
There was a problem hiding this comment. I dont know if that actually tells us anything about the performance here though? Can we run benches or whatever and see |
||
| ) -> ComponentId { | ||
| let components = &mut self.components; | ||
| let index = self.resource_indices.entry(type_id).or_insert_with(|| { | ||
| let descriptor = func(); | ||
| let index = components.len(); | ||
| components.push(ComponentInfo::new(ComponentId(index), descriptor)); | ||
| index | ||
| }); | ||
|
|
||
| ComponentId(*index) | ||
| } | ||
|
|
||
| /// # Safety | ||
| /// | ||
| /// The [`ComponentDescriptor`] must match the [`TypeId`] | ||
| #[inline] | ||
| pub(crate) unsafe fn get_or_insert_with( | ||
|
There was a problem hiding this comment. This method was already de-facto unsafe. Nobody checked that the |
||
| &mut self, | ||
| type_id: TypeId, | ||
| func: impl FnOnce() -> ComponentDescriptor, | ||
| ) -> ComponentId { | ||
| let components = &mut self.components; | ||
| let index = self.indices.entry(type_id).or_insert_with(|| { | ||
| let descriptor = func(); | ||
| let index = components.len(); | ||
| components.push(ComponentInfo::new(ComponentId(index), descriptor)); | ||
| index | ||
| }); | ||
|
|
||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Making this public may help with #2486, but there are probably other places that need to be changed too for it to work.