Remove ComponentStorage and associated types (#12311)

# Objective When doing a final pass for #3362, it appeared that `ComponentStorage` as a trait, the two types implementing it, and the associated type on `Component` aren't really necessary anymore. This likely was due to an earlier constraint on the use of consts in traits, but that definitely doesn't seem to be a problem in Rust 1.76. ## Solution Remove them. --- ## Changelog Changed: `Component::Storage` has been replaced with `Component::STORAGE_TYPE` as a const. Removed: `bevy::ecs::component::ComponentStorage` trait Removed: `bevy::ecs::component::TableStorage` struct Removed: `bevy::ecs::component::SparseSetStorage` struct ## Migration Guide If you were manually implementing `Component` instead of using the derive macro, replace the associated `Storage` associated type with the `STORAGE_TYPE` const: ```rust // in Bevy 0.13 impl Component for MyComponent { type Storage = TableStorage; } // in Bevy 0.14 impl Component for MyComponent { const STORAGE_TYPE: StorageType = StorageType::Table; } ``` Component is no longer object safe. If you were relying on `&dyn Component`, `Box<dyn Component>`, etc. please [file an issue ](https://github.com/bevyengine/bevy/issues) to get [this change](#12311) reverted. --------- Co-authored-by: Alice Cecile <[email protected]>
bevyengine · Mar 5, 2024 · dc40cd1 · dc40cd1
1 parent 70b70cd
commit dc40cd1
Show file tree

Hide file tree

Showing 7 changed files with 44 additions and 75 deletions.
diff --git a/crates/bevy_ecs/macros/src/component.rs b/crates/bevy_ecs/macros/src/component.rs
@@ -60,7 +60,7 @@ pub fn derive_component(input: TokenStream) -> TokenStream {
 
     TokenStream::from(quote! {
         impl #impl_generics #bevy_ecs_path::component::Component for #struct_name #type_generics #where_clause {
-            type Storage = #storage;
+            const STORAGE_TYPE: #bevy_ecs_path::component::StorageType = #storage;
         }
     })
 }
@@ -110,10 +110,10 @@ fn parse_component_attr(ast: &DeriveInput) -> Result<Attrs> {
 }
 
 fn storage_path(bevy_ecs_path: &Path, ty: StorageTy) -> TokenStream2 {
-    let typename = match ty {
-        StorageTy::Table => Ident::new("TableStorage", Span::call_site()),
-        StorageTy::SparseSet => Ident::new("SparseStorage", Span::call_site()),
+    let storage_type = match ty {
+        StorageTy::Table => Ident::new("Table", Span::call_site()),
+        StorageTy::SparseSet => Ident::new("SparseSet", Span::call_site()),
     };
 
-    quote! { #bevy_ecs_path::component::#typename }
+    quote! { #bevy_ecs_path::component::StorageType::#storage_type }
 }
diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs
@@ -10,7 +10,7 @@ use crate::{
         AddBundle, Archetype, ArchetypeId, Archetypes, BundleComponentStatus, ComponentStatus,
         SpawnBundleStatus,
     },
-    component::{Component, ComponentId, ComponentStorage, Components, StorageType, Tick},
+    component::{Component, ComponentId, Components, StorageType, Tick},
     entity::{Entities, Entity, EntityLocation},
     prelude::World,
     query::DebugCheckedUnwrap,
@@ -203,7 +203,7 @@ unsafe impl<C: Component> Bundle for C {
 impl<C: Component> DynamicBundle for C {
     #[inline]
     fn get_components(self, func: &mut impl FnMut(StorageType, OwningPtr<'_>)) {
-        OwningPtr::make(self, |ptr| func(C::Storage::STORAGE_TYPE, ptr));
+        OwningPtr::make(self, |ptr| func(C::STORAGE_TYPE, ptr));
     }
 }
 

diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs
@@ -151,42 +151,13 @@ use std::{
 /// [`SyncCell`]: bevy_utils::synccell::SyncCell
 /// [`Exclusive`]: https://doc.rust-lang.org/nightly/std/sync/struct.Exclusive.html
 pub trait Component: Send + Sync + 'static {
-    /// A marker type indicating the storage type used for this component.
-    /// This must be either [`TableStorage`] or [`SparseStorage`].
-    type Storage: ComponentStorage;
+    /// A constant indicating the storage type used for this component.
+    const STORAGE_TYPE: StorageType;
 
     /// Called when registering this component, allowing mutable access to it's [`ComponentHooks`].
     fn register_component_hooks(_hooks: &mut ComponentHooks) {}
 }
 
-/// Marker type for components stored in a [`Table`](crate::storage::Table).
-pub struct TableStorage;
-
-/// Marker type for components stored in a [`ComponentSparseSet`](crate::storage::ComponentSparseSet).
-pub struct SparseStorage;
-
-/// Types used to specify the storage strategy for a component.
-///
-/// This trait is implemented for [`TableStorage`] and [`SparseStorage`].
-/// Custom implementations are forbidden.
-pub trait ComponentStorage: sealed::Sealed {
-    /// A value indicating the storage strategy specified by this type.
-    const STORAGE_TYPE: StorageType;
-}
-
-impl ComponentStorage for TableStorage {
-    const STORAGE_TYPE: StorageType = StorageType::Table;
-}
-impl ComponentStorage for SparseStorage {
-    const STORAGE_TYPE: StorageType = StorageType::SparseSet;
-}
-
-mod sealed {
-    pub trait Sealed {}
-    impl Sealed for super::TableStorage {}
-    impl Sealed for super::SparseStorage {}
-}
-
 /// The storage used for a specific component type.
 ///
 /// # Examples
@@ -472,7 +443,7 @@ impl ComponentDescriptor {
     pub fn new<T: Component>() -> Self {
         Self {
             name: Cow::Borrowed(std::any::type_name::<T>()),
-            storage_type: T::Storage::STORAGE_TYPE,
+            storage_type: T::STORAGE_TYPE,
             is_send_and_sync: true,
             type_id: Some(TypeId::of::<T>()),
             layout: Layout::new::<T>(),
@@ -503,7 +474,7 @@ impl ComponentDescriptor {
 
     /// Create a new `ComponentDescriptor` for a resource.
     ///
-    /// The [`StorageType`] for resources is always [`TableStorage`].
+    /// The [`StorageType`] for resources is always [`StorageType::Table`].
     pub fn new_resource<T: Resource>() -> Self {
         Self {
             name: Cow::Borrowed(std::any::type_name::<T>()),

diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs
@@ -1,7 +1,7 @@
 use crate::{
     archetype::Archetype,
     change_detection::{Ticks, TicksMut},
-    component::{Component, ComponentId, ComponentStorage, StorageType, Tick},
+    component::{Component, ComponentId, StorageType, Tick},
     entity::Entity,
     query::{Access, DebugCheckedUnwrap, FilteredAccess, WorldQuery},
     storage::{ComponentSparseSet, Table, TableRow},
@@ -711,9 +711,9 @@ unsafe impl<'a> QueryData for FilteredEntityMut<'a> {
 
 #[doc(hidden)]
 pub struct ReadFetch<'w, T> {
-    // T::Storage = TableStorage
+    // T::STORAGE_TYPE = StorageType::Table
     table_components: Option<ThinSlicePtr<'w, UnsafeCell<T>>>,
-    // T::Storage = SparseStorage
+    // T::STORAGE_TYPE = StorageType::SparseSet
     sparse_set: Option<&'w ComponentSparseSet>,
 }
 
@@ -747,7 +747,7 @@ unsafe impl<T: Component> WorldQuery for &T {
     ) -> ReadFetch<'w, T> {
         ReadFetch {
             table_components: None,
-            sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet).then(|| {
+            sparse_set: (T::STORAGE_TYPE == StorageType::SparseSet).then(|| {
                 // SAFETY: The underlying type associated with `component_id` is `T`,
                 // which we are allowed to access since we registered it in `update_archetype_component_access`.
                 // Note that we do not actually access any components in this function, we just get a shared
@@ -764,7 +764,7 @@ unsafe impl<T: Component> WorldQuery for &T {
     }
 
     const IS_DENSE: bool = {
-        match T::Storage::STORAGE_TYPE {
+        match T::STORAGE_TYPE {
             StorageType::Table => true,
             StorageType::SparseSet => false,
         }
@@ -806,7 +806,7 @@ unsafe impl<T: Component> WorldQuery for &T {
         entity: Entity,
         table_row: TableRow,
     ) -> Self::Item<'w> {
-        match T::Storage::STORAGE_TYPE {
+        match T::STORAGE_TYPE {
             StorageType::Table => {
                 // SAFETY: STORAGE_TYPE = Table
                 let table = unsafe { fetch.table_components.debug_checked_unwrap() };
@@ -862,13 +862,13 @@ unsafe impl<T: Component> ReadOnlyQueryData for &T {}
 
 #[doc(hidden)]
 pub struct RefFetch<'w, T> {
-    // T::Storage = TableStorage
+    // T::STORAGE_TYPE = StorageType::Table
     table_data: Option<(
         ThinSlicePtr<'w, UnsafeCell<T>>,
         ThinSlicePtr<'w, UnsafeCell<Tick>>,
         ThinSlicePtr<'w, UnsafeCell<Tick>>,
     )>,
-    // T::Storage = SparseStorage
+    // T::STORAGE_TYPE = StorageType::SparseSet
     sparse_set: Option<&'w ComponentSparseSet>,
 
     last_run: Tick,
@@ -905,7 +905,7 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> {
     ) -> RefFetch<'w, T> {
         RefFetch {
             table_data: None,
-            sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet).then(|| {
+            sparse_set: (T::STORAGE_TYPE == StorageType::SparseSet).then(|| {
                 // SAFETY: The underlying type associated with `component_id` is `T`,
                 // which we are allowed to access since we registered it in `update_archetype_component_access`.
                 // Note that we do not actually access any components in this function, we just get a shared
@@ -924,7 +924,7 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> {
     }
 
     const IS_DENSE: bool = {
-        match T::Storage::STORAGE_TYPE {
+        match T::STORAGE_TYPE {
             StorageType::Table => true,
             StorageType::SparseSet => false,
         }
@@ -965,7 +965,7 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> {
         entity: Entity,
         table_row: TableRow,
     ) -> Self::Item<'w> {
-        match T::Storage::STORAGE_TYPE {
+        match T::STORAGE_TYPE {
             StorageType::Table => {
                 // SAFETY: STORAGE_TYPE = Table
                 let (table_components, added_ticks, changed_ticks) =
@@ -1045,13 +1045,13 @@ unsafe impl<'__w, T: Component> ReadOnlyQueryData for Ref<'__w, T> {}
 
 #[doc(hidden)]
 pub struct WriteFetch<'w, T> {
-    // T::Storage = TableStorage
+    // T::STORAGE_TYPE = StorageType::Table
     table_data: Option<(
         ThinSlicePtr<'w, UnsafeCell<T>>,
         ThinSlicePtr<'w, UnsafeCell<Tick>>,
         ThinSlicePtr<'w, UnsafeCell<Tick>>,
     )>,
-    // T::Storage = SparseStorage
+    // T::STORAGE_TYPE = StorageType::SparseSet
     sparse_set: Option<&'w ComponentSparseSet>,
 
     last_run: Tick,
@@ -1088,7 +1088,7 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T {
     ) -> WriteFetch<'w, T> {
         WriteFetch {
             table_data: None,
-            sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet).then(|| {
+            sparse_set: (T::STORAGE_TYPE == StorageType::SparseSet).then(|| {
                 // SAFETY: The underlying type associated with `component_id` is `T`,
                 // which we are allowed to access since we registered it in `update_archetype_component_access`.
                 // Note that we do not actually access any components in this function, we just get a shared
@@ -1107,7 +1107,7 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T {
     }
 
     const IS_DENSE: bool = {
-        match T::Storage::STORAGE_TYPE {
+        match T::STORAGE_TYPE {
             StorageType::Table => true,
             StorageType::SparseSet => false,
         }
@@ -1148,7 +1148,7 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T {
         entity: Entity,
         table_row: TableRow,
     ) -> Self::Item<'w> {
-        match T::Storage::STORAGE_TYPE {
+        match T::STORAGE_TYPE {
             StorageType::Table => {
                 // SAFETY: STORAGE_TYPE = Table
                 let (table_components, added_ticks, changed_ticks) =
@@ -1433,7 +1433,7 @@ unsafe impl<T: Component> WorldQuery for Has<T> {
     }
 
     const IS_DENSE: bool = {
-        match T::Storage::STORAGE_TYPE {
+        match T::STORAGE_TYPE {
             StorageType::Table => true,
             StorageType::SparseSet => false,
         }

diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs
@@ -1,6 +1,6 @@
 use crate::{
     archetype::Archetype,
-    component::{Component, ComponentId, ComponentStorage, StorageType, Tick},
+    component::{Component, ComponentId, StorageType, Tick},
     entity::Entity,
     query::{DebugCheckedUnwrap, FilteredAccess, WorldQuery},
     storage::{Column, ComponentSparseSet, Table, TableRow},
@@ -142,7 +142,7 @@ unsafe impl<T: Component> WorldQuery for With<T> {
     }
 
     const IS_DENSE: bool = {
-        match T::Storage::STORAGE_TYPE {
+        match T::STORAGE_TYPE {
             StorageType::Table => true,
             StorageType::SparseSet => false,
         }
@@ -250,7 +250,7 @@ unsafe impl<T: Component> WorldQuery for Without<T> {
     }
 
     const IS_DENSE: bool = {
-        match T::Storage::STORAGE_TYPE {
+        match T::STORAGE_TYPE {
             StorageType::Table => true,
             StorageType::SparseSet => false,
         }
@@ -604,15 +604,15 @@ unsafe impl<T: Component> WorldQuery for Added<T> {
     ) -> Self::Fetch<'w> {
         Self::Fetch::<'w> {
             table_ticks: None,
-            sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet)
+            sparse_set: (T::STORAGE_TYPE == StorageType::SparseSet)
                 .then(|| world.storages().sparse_sets.get(id).debug_checked_unwrap()),
             last_run,
             this_run,
         }
     }
 
     const IS_DENSE: bool = {
-        match T::Storage::STORAGE_TYPE {
+        match T::STORAGE_TYPE {
             StorageType::Table => true,
             StorageType::SparseSet => false,
         }
@@ -651,7 +651,7 @@ unsafe impl<T: Component> WorldQuery for Added<T> {
         entity: Entity,
         table_row: TableRow,
     ) -> Self::Item<'w> {
-        match T::Storage::STORAGE_TYPE {
+        match T::STORAGE_TYPE {
             StorageType::Table => {
                 // SAFETY: STORAGE_TYPE = Table
                 let table = unsafe { fetch.table_ticks.debug_checked_unwrap() };
@@ -813,15 +813,15 @@ unsafe impl<T: Component> WorldQuery for Changed<T> {
     ) -> Self::Fetch<'w> {
         Self::Fetch::<'w> {
             table_ticks: None,
-            sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet)
+            sparse_set: (T::STORAGE_TYPE == StorageType::SparseSet)
                 .then(|| world.storages().sparse_sets.get(id).debug_checked_unwrap()),
             last_run,
             this_run,
         }
     }
 
     const IS_DENSE: bool = {
-        match T::Storage::STORAGE_TYPE {
+        match T::STORAGE_TYPE {
             StorageType::Table => true,
             StorageType::SparseSet => false,
         }
@@ -860,7 +860,7 @@ unsafe impl<T: Component> WorldQuery for Changed<T> {
         entity: Entity,
         table_row: TableRow,
     ) -> Self::Item<'w> {
-        match T::Storage::STORAGE_TYPE {
+        match T::STORAGE_TYPE {
             StorageType::Table => {
                 // SAFETY: STORAGE_TYPE = Table
                 let table = unsafe { fetch.table_ticks.debug_checked_unwrap() };

diff --git a/crates/bevy_ecs/src/world/unsafe_world_cell.rs b/crates/bevy_ecs/src/world/unsafe_world_cell.rs
@@ -7,9 +7,7 @@ use crate::{
     archetype::{Archetype, ArchetypeComponentId, Archetypes},
     bundle::Bundles,
     change_detection::{MutUntyped, Ticks, TicksMut},
-    component::{
-        ComponentId, ComponentStorage, ComponentTicks, Components, StorageType, Tick, TickCells,
-    },
+    component::{ComponentId, ComponentTicks, Components, StorageType, Tick, TickCells},
     entity::{Entities, Entity, EntityLocation},
     prelude::Component,
     removal_detection::RemovedComponentEvents,
@@ -713,7 +711,7 @@ impl<'w> UnsafeEntityCell<'w> {
             get_component(
                 self.world,
                 component_id,
-                T::Storage::STORAGE_TYPE,
+                T::STORAGE_TYPE,
                 self.entity,
                 self.location,
             )
@@ -740,7 +738,7 @@ impl<'w> UnsafeEntityCell<'w> {
             get_component_and_ticks(
                 self.world,
                 component_id,
-                T::Storage::STORAGE_TYPE,
+                T::STORAGE_TYPE,
                 self.entity,
                 self.location,
             )
@@ -770,7 +768,7 @@ impl<'w> UnsafeEntityCell<'w> {
             get_ticks(
                 self.world,
                 component_id,
-                T::Storage::STORAGE_TYPE,
+                T::STORAGE_TYPE,
                 self.entity,
                 self.location,
             )
@@ -839,7 +837,7 @@ impl<'w> UnsafeEntityCell<'w> {
             get_component_and_ticks(
                 self.world,
                 component_id,
-                T::Storage::STORAGE_TYPE,
+                T::STORAGE_TYPE,
                 self.entity,
                 self.location,
             )

diff --git a/examples/ecs/component_hooks.rs b/examples/ecs/component_hooks.rs
@@ -13,15 +13,15 @@
 //! - Enforcing structural rules: When you have systems that depend on specific relationships
 //! between components (like hierarchies or parent-child links) and need to maintain correctness.
 
-use bevy::ecs::component::{ComponentHooks, TableStorage};
+use bevy::ecs::component::{ComponentHooks, StorageType};
 use bevy::prelude::*;
 use std::collections::HashMap;
 
 #[derive(Debug)]
 struct MyComponent(KeyCode);
 
 impl Component for MyComponent {
-    type Storage = TableStorage;
+    const STORAGE_TYPE: StorageType = StorageType::Table;
 
     /// Hooks can also be registered during component initialisation by
     /// implementing `register_component_hooks`