Stop using AssetId in the AssetServer and use AssetIndex instead. (#19927)

# Objective

- This is sort of a step towards #19024.
- This is a step towards #11266.

Context: The AssetServer **never** deals with Uuid assets anyway. We
always allocate asset indices, and asset UUIDs are not handled properly
(dependencies on UUID assets are never loaded, so their dependents won't
ever be notified).

## Solution

- Create `ErasedAssetIndex` as a replacement for `UntypedAssetId`.
- Use `ErasedAssetIndex` or `AssetIndex` where relevant within the
`AssetServer`.

Author: andriyDev

crates/bevy_asset/src/assets.rs

@@ -291,7 +291,7 @@ pub struct Assets<A: Asset> {
     queued_events: Vec<AssetEvent<A>>,
     /// Assets managed by the `Assets` struct with live strong `Handle`s
     /// originating from `get_strong_handle`.
-    duplicate_handles: HashMap<AssetId<A>, u16>,
+    duplicate_handles: HashMap<AssetIndex, u16>,
 }
 
 impl<A: Asset> Default for Assets<A> {
@@ -400,10 +400,7 @@ impl<A: Asset> Assets<A> {
     pub fn add(&mut self, asset: impl Into<A>) -> Handle<A> {
         let index = self.dense_storage.allocator.reserve();
         self.insert_with_index(index, asset.into()).unwrap();
-        Handle::Strong(
-            self.handle_provider
-                .get_handle(index.into(), false, None, None),
-        )
+        Handle::Strong(self.handle_provider.get_handle(index, false, None, None))
     }
 
     /// Upgrade an `AssetId` into a strong `Handle` that will prevent asset drop.
@@ -415,11 +412,12 @@ impl<A: Asset> Assets<A> {
         if !self.contains(id) {
             return None;
         }
-        *self.duplicate_handles.entry(id).or_insert(0) += 1;
         let index = match id {
-            AssetId::Index { index, .. } => index.into(),
-            AssetId::Uuid { uuid } => uuid.into(),
+            AssetId::Index { index, .. } => index,
+            // We don't support strong handles for Uuid assets.
+            AssetId::Uuid { .. } => return None,
         };
+        *self.duplicate_handles.entry(index).or_insert(0) += 1;
         Some(Handle::Strong(
             self.handle_provider.get_handle(index, false, None, None),
         ))
@@ -479,34 +477,35 @@ impl<A: Asset> Assets<A> {
     /// This is the same as [`Assets::remove`] except it doesn't emit [`AssetEvent::Removed`].
     pub fn remove_untracked(&mut self, id: impl Into<AssetId<A>>) -> Option<A> {
         let id: AssetId<A> = id.into();
-        self.duplicate_handles.remove(&id);
         match id {
-            AssetId::Index { index, .. } => self.dense_storage.remove_still_alive(index),
+            AssetId::Index { index, .. } => {
+                self.duplicate_handles.remove(&index);
+                self.dense_storage.remove_still_alive(index)
+            }
             AssetId::Uuid { uuid } => self.hash_map.remove(&uuid),
         }
     }
 
     /// Removes the [`Asset`] with the given `id`.
-    pub(crate) fn remove_dropped(&mut self, id: AssetId<A>) {
-        match self.duplicate_handles.get_mut(&id) {
+    pub(crate) fn remove_dropped(&mut self, index: AssetIndex) {
+        match self.duplicate_handles.get_mut(&index) {
             None => {}
             Some(0) => {
-                self.duplicate_handles.remove(&id);
+                self.duplicate_handles.remove(&index);
             }
             Some(value) => {
                 *value -= 1;
                 return;
             }
         }
 
-        let existed = match id {
-            AssetId::Index { index, .. } => self.dense_storage.remove_dropped(index).is_some(),
-            AssetId::Uuid { uuid } => self.hash_map.remove(&uuid).is_some(),
-        };
+        let existed = self.dense_storage.remove_dropped(index).is_some();
 
-        self.queued_events.push(AssetEvent::Unused { id });
+        self.queued_events
+            .push(AssetEvent::Unused { id: index.into() });
         if existed {
-            self.queued_events.push(AssetEvent::Removed { id });
+            self.queued_events
+                .push(AssetEvent::Removed { id: index.into() });
         }
     }
 
@@ -574,19 +573,15 @@ impl<A: Asset> Assets<A> {
         // to other asset info operations
         let mut infos = asset_server.write_infos();
         while let Ok(drop_event) = assets.handle_provider.drop_receiver.try_recv() {
-            let id = drop_event.id.typed();
-
             if drop_event.asset_server_managed {
-                let untyped_id = id.untyped();
-
                 // the process_handle_drop call checks whether new handles have been created since the drop event was fired, before removing the asset
-                if !infos.process_handle_drop(untyped_id) {
+                if !infos.process_handle_drop(drop_event.index) {
                     // a new handle has been created, or the asset doesn't exist
                     continue;
                 }
             }
 
-            assets.remove_dropped(id);
+            assets.remove_dropped(drop_event.index.index);
         }
     }
 

crates/bevy_asset/src/handle.rs

@@ -1,6 +1,6 @@
 use crate::{
-    meta::MetaTransform, Asset, AssetId, AssetIndexAllocator, AssetPath, InternalAssetId,
-    UntypedAssetId,
+    meta::MetaTransform, Asset, AssetId, AssetIndex, AssetIndexAllocator, AssetPath,
+    ErasedAssetIndex, UntypedAssetId,
 };
 use alloc::sync::Arc;
 use bevy_reflect::{std_traits::ReflectDefault, Reflect, TypePath};
@@ -26,7 +26,7 @@ pub struct AssetHandleProvider {
 
 #[derive(Debug)]
 pub(crate) struct DropEvent {
-    pub(crate) id: InternalAssetId,
+    pub(crate) index: ErasedAssetIndex,
     pub(crate) asset_server_managed: bool,
 }
 
@@ -45,18 +45,19 @@ impl AssetHandleProvider {
     /// [`UntypedHandle`] will match the [`Asset`] [`TypeId`] assigned to this [`AssetHandleProvider`].
     pub fn reserve_handle(&self) -> UntypedHandle {
         let index = self.allocator.reserve();
-        UntypedHandle::Strong(self.get_handle(InternalAssetId::Index(index), false, None, None))
+        UntypedHandle::Strong(self.get_handle(index, false, None, None))
     }
 
     pub(crate) fn get_handle(
         &self,
-        id: InternalAssetId,
+        index: AssetIndex,
         asset_server_managed: bool,
         path: Option<AssetPath<'static>>,
         meta_transform: Option<MetaTransform>,
     ) -> Arc<StrongHandle> {
         Arc::new(StrongHandle {
-            id: id.untyped(self.type_id),
+            index,
+            type_id: self.type_id,
             drop_sender: self.drop_sender.clone(),
             meta_transform,
             path,
@@ -71,20 +72,16 @@ impl AssetHandleProvider {
         meta_transform: Option<MetaTransform>,
     ) -> Arc<StrongHandle> {
         let index = self.allocator.reserve();
-        self.get_handle(
-            InternalAssetId::Index(index),
-            asset_server_managed,
-            path,
-            meta_transform,
-        )
+        self.get_handle(index, asset_server_managed, path, meta_transform)
     }
 }
 
 /// The internal "strong" [`Asset`] handle storage for [`Handle::Strong`] and [`UntypedHandle::Strong`]. When this is dropped,
 /// the [`Asset`] will be freed. It also stores some asset metadata for easy access from handles.
 #[derive(TypePath)]
 pub struct StrongHandle {
-    pub(crate) id: UntypedAssetId,
+    pub(crate) index: AssetIndex,
+    pub(crate) type_id: TypeId,
     pub(crate) asset_server_managed: bool,
     pub(crate) path: Option<AssetPath<'static>>,
     /// Modifies asset meta. This is stored on the handle because it is:
@@ -97,7 +94,7 @@ pub struct StrongHandle {
 impl Drop for StrongHandle {
     fn drop(&mut self) {
         let _ = self.drop_sender.send(DropEvent {
-            id: self.id.internal(),
+            index: ErasedAssetIndex::new(self.index, self.type_id),
             asset_server_managed: self.asset_server_managed,
         });
     }
@@ -106,7 +103,8 @@ impl Drop for StrongHandle {
 impl core::fmt::Debug for StrongHandle {
     fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
         f.debug_struct("StrongHandle")
-            .field("id", &self.id)
+            .field("index", &self.index)
+            .field("type_id", &self.type_id)
             .field("asset_server_managed", &self.asset_server_managed)
             .field("path", &self.path)
             .field("drop_sender", &self.drop_sender)
@@ -154,7 +152,10 @@ impl<A: Asset> Handle<A> {
     #[inline]
     pub fn id(&self) -> AssetId<A> {
         match self {
-            Handle::Strong(handle) => handle.id.typed_unchecked(),
+            Handle::Strong(handle) => AssetId::Index {
+                index: handle.index,
+                marker: PhantomData,
+            },
             Handle::Uuid(uuid, ..) => AssetId::Uuid { uuid: *uuid },
         }
     }
@@ -202,9 +203,8 @@ impl<A: Asset> core::fmt::Debug for Handle<A> {
             Handle::Strong(handle) => {
                 write!(
                     f,
-                    "StrongHandle<{name}>{{ id: {:?}, path: {:?} }}",
-                    handle.id.internal(),
-                    handle.path
+                    "StrongHandle<{name}>{{ index: {:?}, type_id: {:?}, path: {:?} }}",
+                    handle.index, handle.type_id, handle.path
                 )
             }
             Handle::Uuid(uuid, ..) => write!(f, "UuidHandle<{name}>({uuid:?})"),
@@ -298,7 +298,10 @@ impl UntypedHandle {
     #[inline]
     pub fn id(&self) -> UntypedAssetId {
         match self {
-            UntypedHandle::Strong(handle) => handle.id,
+            UntypedHandle::Strong(handle) => UntypedAssetId::Index {
+                type_id: handle.type_id,
+                index: handle.index,
+            },
             UntypedHandle::Uuid { type_id, uuid } => UntypedAssetId::Uuid {
                 uuid: *uuid,
                 type_id: *type_id,
@@ -319,7 +322,7 @@ impl UntypedHandle {
     #[inline]
     pub fn type_id(&self) -> TypeId {
         match self {
-            UntypedHandle::Strong(handle) => handle.id.type_id(),
+            UntypedHandle::Strong(handle) => handle.type_id,
             UntypedHandle::Uuid { type_id, .. } => *type_id,
         }
     }
@@ -400,9 +403,7 @@ impl core::fmt::Debug for UntypedHandle {
                 write!(
                     f,
                     "StrongHandle{{ type_id: {:?}, id: {:?}, path: {:?} }}",
-                    handle.id.type_id(),
-                    handle.id.internal(),
-                    handle.path
+                    handle.type_id, handle.index, handle.path
                 )
             }
             UntypedHandle::Uuid { type_id, uuid } => {

crates/bevy_asset/src/id.rs

@@ -1,4 +1,4 @@
-use crate::{Asset, AssetIndex};
+use crate::{Asset, AssetIndex, Handle, UntypedHandle};
 use bevy_reflect::{std_traits::ReflectDefault, Reflect};
 use serde::{Deserialize, Serialize};
 use uuid::Uuid;
@@ -68,7 +68,7 @@ impl<A: Asset> AssetId<A> {
     }
 
     #[inline]
-    pub(crate) fn internal(self) -> InternalAssetId {
+    fn internal(self) -> InternalAssetId {
         match self {
             AssetId::Index { index, .. } => InternalAssetId::Index(index),
             AssetId::Uuid { uuid } => InternalAssetId::Uuid(uuid),
@@ -255,7 +255,7 @@ impl UntypedAssetId {
     }
 
     #[inline]
-    pub(crate) fn internal(self) -> InternalAssetId {
+    fn internal(self) -> InternalAssetId {
         match self {
             UntypedAssetId::Index { index, .. } => InternalAssetId::Index(index),
             UntypedAssetId::Uuid { uuid, .. } => InternalAssetId::Uuid(uuid),
@@ -314,36 +314,33 @@ impl PartialOrd for UntypedAssetId {
 
 /// An asset id without static or dynamic types associated with it.
 ///
-/// This exist to support efficient type erased id drop tracking. We
-/// could use [`UntypedAssetId`] for this, but the [`TypeId`] is unnecessary.
-///
-/// Do not _ever_ use this across asset types for comparison.
-/// [`InternalAssetId`] contains no type information and will happily collide
-/// with indices across types.
+/// This is provided to make implementing traits easier for the many different asset ID types.
 #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash, PartialOrd, Ord, From)]
-pub(crate) enum InternalAssetId {
+enum InternalAssetId {
     Index(AssetIndex),
     Uuid(Uuid),
 }
 
-impl InternalAssetId {
-    #[inline]
-    pub(crate) fn typed<A: Asset>(self) -> AssetId<A> {
-        match self {
-            InternalAssetId::Index(index) => AssetId::Index {
-                index,
-                marker: PhantomData,
-            },
-            InternalAssetId::Uuid(uuid) => AssetId::Uuid { uuid },
-        }
+/// An asset index bundled with its (dynamic) type.
+#[derive(Debug, Hash, PartialEq, Eq, Clone, Copy)]
+pub(crate) struct ErasedAssetIndex {
+    pub(crate) index: AssetIndex,
+    pub(crate) type_id: TypeId,
+}
+
+impl ErasedAssetIndex {
+    pub(crate) fn new(index: AssetIndex, type_id: TypeId) -> Self {
+        Self { index, type_id }
     }
+}
 
-    #[inline]
-    pub(crate) fn untyped(self, type_id: TypeId) -> UntypedAssetId {
-        match self {
-            InternalAssetId::Index(index) => UntypedAssetId::Index { index, type_id },
-            InternalAssetId::Uuid(uuid) => UntypedAssetId::Uuid { uuid, type_id },
-        }
+impl Display for ErasedAssetIndex {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+        f.debug_struct("ErasedAssetIndex")
+            .field("type_id", &self.type_id)
+            .field("index", &self.index.index)
+            .field("generation", &self.index.generation)
+            .finish()
     }
 }
 
@@ -414,6 +411,52 @@ impl<A: Asset> TryFrom<UntypedAssetId> for AssetId<A> {
     }
 }
 
+impl TryFrom<UntypedAssetId> for ErasedAssetIndex {
+    type Error = UuidNotSupportedError;
+
+    fn try_from(asset_id: UntypedAssetId) -> Result<Self, Self::Error> {
+        match asset_id {
+            UntypedAssetId::Index { type_id, index } => Ok(ErasedAssetIndex { index, type_id }),
+            UntypedAssetId::Uuid { .. } => Err(UuidNotSupportedError),
+        }
+    }
+}
+
+impl<A: Asset> TryFrom<&Handle<A>> for ErasedAssetIndex {
+    type Error = UuidNotSupportedError;
+
+    fn try_from(handle: &Handle<A>) -> Result<Self, Self::Error> {
+        match handle {
+            Handle::Strong(handle) => Ok(Self::new(handle.index, handle.type_id)),
+            Handle::Uuid(..) => Err(UuidNotSupportedError),
+        }
+    }
+}
+
+impl TryFrom<&UntypedHandle> for ErasedAssetIndex {
+    type Error = UuidNotSupportedError;
+
+    fn try_from(handle: &UntypedHandle) -> Result<Self, Self::Error> {
+        match handle {
+            UntypedHandle::Strong(handle) => Ok(Self::new(handle.index, handle.type_id)),
+            UntypedHandle::Uuid { .. } => Err(UuidNotSupportedError),
+        }
+    }
+}
+
+impl From<ErasedAssetIndex> for UntypedAssetId {
+    fn from(value: ErasedAssetIndex) -> Self {
+        Self::Index {
+            type_id: value.type_id,
+            index: value.index,
+        }
+    }
+}
+
+#[derive(Error, Debug)]
+#[error("Attempted to create a TypedAssetIndex from a Uuid")]
+pub(crate) struct UuidNotSupportedError;
+
 /// Errors preventing the conversion of to/from an [`UntypedAssetId`] and an [`AssetId`].
 #[derive(Error, Debug, PartialEq, Clone)]
 #[non_exhaustive]