Ensure consistency between Un/Typed AssetId and Handle

crates/bevy_asset/src/handle.rs

@@ -11,6 +11,7 @@ use std::{
     hash::{Hash, Hasher},
     sync::Arc,
 };
+use thiserror::Error;
 
 /// Provides [`Handle`] and [`UntypedHandle`] _for a specific asset type_.
 /// This should _only_ be used for one specific asset type.
@@ -191,10 +192,7 @@ impl<A: Asset> Handle<A> {
     /// [`Handle::Weak`].  
     #[inline]
     pub fn untyped(self) -> UntypedHandle {
-        match self {
-            Handle::Strong(handle) => UntypedHandle::Strong(handle),
-            Handle::Weak(id) => UntypedHandle::Weak(id.untyped()),
-        }
+        self.into()
     }
 }
 
@@ -224,13 +222,14 @@ impl<A: Asset> std::fmt::Debug for Handle<A> {
 impl<A: Asset> Hash for Handle<A> {
     #[inline]
     fn hash<H: Hasher>(&self, state: &mut H) {
-        Hash::hash(&self.id(), state);
+        self.id().hash(state);
+        TypeId::of::<A>().hash(state);
     }
 }
 
 impl<A: Asset> PartialOrd for Handle<A> {
     fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
-        Some(self.id().cmp(&other.id()))
+        Some(self.cmp(other))
     }
 }
 
@@ -249,6 +248,34 @@ impl<A: Asset> PartialEq for Handle<A> {
 
 impl<A: Asset> Eq for Handle<A> {}
 
+impl<A: Asset> From<Handle<A>> for AssetId<A> {
+    #[inline]
+    fn from(value: Handle<A>) -> Self {
+        value.id()
+    }
+}
+
+impl<A: Asset> From<&Handle<A>> for AssetId<A> {
+    #[inline]
+    fn from(value: &Handle<A>) -> Self {
+        value.id()
+    }
+}
+
+impl<A: Asset> From<Handle<A>> for UntypedAssetId {
+    #[inline]
+    fn from(value: Handle<A>) -> Self {
+        value.id().into()
+    }
+}
+
+impl<A: Asset> From<&Handle<A>> for UntypedAssetId {
+    #[inline]
+    fn from(value: &Handle<A>) -> Self {
+        value.id().into()
+    }
+}
+
 /// An untyped variant of [`Handle`], which internally stores the [`Asset`] type information at runtime
 /// as a [`TypeId`] instead of encoding it in the compile-time type. This allows handles across [`Asset`] types
 /// to be stored together and compared.
@@ -326,12 +353,20 @@ impl UntypedHandle {
     /// Converts to a typed Handle. This will panic if the internal [`TypeId`] does not match the given asset type `A`
     #[inline]
     pub fn typed<A: Asset>(self) -> Handle<A> {
-        assert_eq!(
-            self.type_id(),
-            TypeId::of::<A>(),
-            "The target Handle<A>'s TypeId does not match the TypeId of this UntypedHandle"
-        );
-        self.typed_unchecked()
+        let Ok(handle) = self.try_typed() else {
+            panic!(
+                "The target Handle<{}>'s TypeId does not match the TypeId of this UntypedHandle",
+                std::any::type_name::<A>()
+            )
+        };
+
+        handle
+    }
+
+    /// Converts to a typed Handle. This will panic if the internal [`TypeId`] does not match the given asset type `A`
+    #[inline]
+    pub fn try_typed<A: Asset>(self) -> Result<Handle<A>, UntypedAssetConversionError> {
+        Handle::try_from(self)
     }
 
     /// The "meta transform" for the strong handle. This will only be [`Some`] if the handle is strong and there is a meta transform
@@ -383,3 +418,212 @@ impl std::fmt::Debug for UntypedHandle {
         }
     }
 }
+
+impl PartialOrd for UntypedHandle {
+    fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+        if self.type_id() == other.type_id() {
+            self.id().partial_cmp(&other.id())
+        } else {
+            None
+        }
+    }
+}
+
+impl From<UntypedHandle> for UntypedAssetId {
+    #[inline]
+    fn from(value: UntypedHandle) -> Self {
+        value.id()
+    }
+}
+
+impl From<&UntypedHandle> for UntypedAssetId {
+    #[inline]
+    fn from(value: &UntypedHandle) -> Self {
+        value.id()
+    }
+}
+
+// Cross Operations
+
+impl<A: Asset> PartialEq<UntypedHandle> for Handle<A> {
+    #[inline]
+    fn eq(&self, other: &UntypedHandle) -> bool {
+        TypeId::of::<A>() == other.type_id() && self.id() == other.id()
+    }
+}
+
+impl<A: Asset> PartialEq<Handle<A>> for UntypedHandle {
+    #[inline]
+    fn eq(&self, other: &Handle<A>) -> bool {
+        other.eq(self)
+    }
+}
+
+impl<A: Asset> PartialOrd<UntypedHandle> for Handle<A> {
+    #[inline]
+    fn partial_cmp(&self, other: &UntypedHandle) -> Option<std::cmp::Ordering> {
+        if TypeId::of::<A>() != other.type_id() {
+            None
+        } else {
+            self.id().partial_cmp(&other.id())
+        }
+    }
+}
+
+impl<A: Asset> PartialOrd<Handle<A>> for UntypedHandle {
+    #[inline]
+    fn partial_cmp(&self, other: &Handle<A>) -> Option<std::cmp::Ordering> {
+        Some(other.partial_cmp(self)?.reverse())
+    }
+}
+
+impl<A: Asset> From<Handle<A>> for UntypedHandle {
+    fn from(value: Handle<A>) -> Self {
+        match value {
+            Handle::Strong(handle) => UntypedHandle::Strong(handle),
+            Handle::Weak(id) => UntypedHandle::Weak(id.into()),
+        }
+    }
+}
+
+impl<A: Asset> TryFrom<UntypedHandle> for Handle<A> {
+    type Error = UntypedAssetConversionError;
+
+    fn try_from(value: UntypedHandle) -> Result<Self, Self::Error> {
+        let found = value.type_id();
+        let expected = TypeId::of::<A>();
+
+        if found != expected {
+            return Err(UntypedAssetConversionError::TypeIdMismatch { expected, found });
+        }
+
+        match value {
+            UntypedHandle::Strong(handle) => Ok(Handle::Strong(handle)),
+            UntypedHandle::Weak(id) => {
+                let Ok(id) = id.try_into() else {
+                    return Err(UntypedAssetConversionError::TypeIdMismatch { expected, found });
+                };
+                Ok(Handle::Weak(id))
+            }
+        }
+    }
+}
+
+/// Errors preventing the conversion of to/from an [`UntypedHandle`] and an [`Handle`].
+#[derive(Error, Debug, PartialEq, Clone)]
+#[non_exhaustive]
+pub enum UntypedAssetConversionError {
+    /// Caused when trying to convert an [`UntypedHandle`] into an [`Handle`] of the wrong type.
+    #[error(
+        "This UntypedHandle is for {found:?} and cannot be converted into an Handle<{expected:?}>"
+    )]
+    TypeIdMismatch { expected: TypeId, found: TypeId },
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    type TestAsset = ();
+
+    const UUID_1: Uuid = Uuid::from_u128(123);
+    const UUID_2: Uuid = Uuid::from_u128(456);
+
+    /// Simple utility to directly hash a value using a fixed hasher
+    fn hash<T: Hash>(data: &T) -> u64 {
+        let mut hasher = bevy_utils::AHasher::default();
+        data.hash(&mut hasher);
+        hasher.finish()
+    }
+
+    /// Typed and Untyped `Handles` should be equivalent to each other and themselves
+    #[test]
+    fn equality() {
+        let typed = AssetId::<TestAsset>::Uuid { uuid: UUID_1 };
+        let untyped = UntypedAssetId::Uuid {
+            type_id: TypeId::of::<TestAsset>(),
+            uuid: UUID_1,
+        };
+
+        let typed = Handle::Weak(typed);
+        let untyped = UntypedHandle::Weak(untyped);
+
+        assert_eq!(
+            Ok(typed.clone()),
+            Handle::<TestAsset>::try_from(untyped.clone())
+        );
+        assert_eq!(UntypedHandle::from(typed.clone()), untyped);
+        assert_eq!(typed, untyped);
+    }
+
+    /// Typed and Untyped `Handles` should be orderable amongst each other and themselves
+    #[allow(clippy::cmp_owned)]
+    #[test]
+    fn ordering() {
+        assert!(UUID_1 < UUID_2);
+
+        let typed_1 = AssetId::<TestAsset>::Uuid { uuid: UUID_1 };
+        let typed_2 = AssetId::<TestAsset>::Uuid { uuid: UUID_2 };
+        let untyped_1 = UntypedAssetId::Uuid {
+            type_id: TypeId::of::<TestAsset>(),
+            uuid: UUID_1,
+        };
+        let untyped_2 = UntypedAssetId::Uuid {
+            type_id: TypeId::of::<TestAsset>(),
+            uuid: UUID_2,
+        };
+
+        let typed_1 = Handle::Weak(typed_1);
+        let typed_2 = Handle::Weak(typed_2);
+        let untyped_1 = UntypedHandle::Weak(untyped_1);
+        let untyped_2 = UntypedHandle::Weak(untyped_2);
+
+        assert!(typed_1 < typed_2);
+        assert!(untyped_1 < untyped_2);
+
+        assert!(UntypedHandle::from(typed_1.clone()) < untyped_2);
+        assert!(untyped_1 < UntypedHandle::from(typed_2.clone()));
+
+        assert!(Handle::<TestAsset>::try_from(untyped_1.clone()).unwrap() < typed_2);
+        assert!(typed_1 < Handle::<TestAsset>::try_from(untyped_2.clone()).unwrap());
+
+        assert!(typed_1 < untyped_2);
+        assert!(untyped_1 < typed_2);
+    }
+
+    /// Typed and Untyped `Handles` should be equivalently hashable to each other and themselves
+    #[test]
+    fn hashing() {
+        let typed = AssetId::<TestAsset>::Uuid { uuid: UUID_1 };
+        let untyped = UntypedAssetId::Uuid {
+            type_id: TypeId::of::<TestAsset>(),
+            uuid: UUID_1,
+        };
+
+        let typed = Handle::Weak(typed);
+        let untyped = UntypedHandle::Weak(untyped);
+
+        assert_eq!(
+            hash(&typed),
+            hash(&Handle::<TestAsset>::try_from(untyped.clone()).unwrap())
+        );
+        assert_eq!(hash(&UntypedHandle::from(typed.clone())), hash(&untyped));
+        assert_eq!(hash(&typed), hash(&untyped));
+    }
+
+    /// Typed and Untyped `Handles` should be interchangeable
+    #[test]
+    fn conversion() {
+        let typed = AssetId::<TestAsset>::Uuid { uuid: UUID_1 };
+        let untyped = UntypedAssetId::Uuid {
+            type_id: TypeId::of::<TestAsset>(),
+            uuid: UUID_1,
+        };
+
+        let typed = Handle::Weak(typed);
+        let untyped = UntypedHandle::Weak(untyped);
+
+        assert_eq!(typed, Handle::try_from(untyped.clone()).unwrap());
+        assert_eq!(UntypedHandle::from(typed.clone()), untyped);
+    }
+}