fix: Handle<T>::from_reflect incorrectly instantiating regardless of T type (#24048)

# Objective

Fixes issue where handles generate no problem regardless of T type via
`from_reflect` due to strong handle's being fully opaque and simply
cloned.


## Solution

Adds a small type id check to the `FromReflect` implemenation which
fails conversion if the type id is not what we expect:

Reference automatically derived implementation:

```rust
impl<A: Asset> bevy_reflect::FromReflect for Handle<A>
where
    Handle<A>: ::core::any::Any + ::core::marker::Send + ::core::marker::Sync,
    A: bevy_reflect::TypePath,
{
    fn from_reflect(__param0: &dyn bevy_reflect::PartialReflect) -> ::core::option::Option<Self> {
        if let bevy_reflect::ReflectRef::Enum(__param0) =
            bevy_reflect::PartialReflect::reflect_ref(__param0)
        {
            match bevy_reflect::enums::Enum::variant_name(__param0) {
                "Strong" => ::core::option::Option::Some(Handle::Strong {
                    0: {
                        let __0 = __param0.field_at(0usize);
                        let __0 = __0?;
                        <Arc<StrongHandle> as bevy_reflect::FromReflect>::from_reflect(__0)?
                    },
                }),
                "Uuid" => ::core::option::Option::Some(Handle::Uuid {
                    0: {
                        let __0 = __param0.field_at(0usize);
                        let __0 = __0?;
                        <Uuid as bevy_reflect::FromReflect>::from_reflect(__0)?
                    },
                    1: ::core::default::Default::default(),
                }),
                name => panic!(
                    "variant with name `{}` does not exist on enum `{}`",
                    name,
                    <Self as bevy_reflect::TypePath>::type_path()
                ),
            }
        } else {
            ::core::option::Option::None
        }
    }
}
```

## Testing

added basic tests

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Greeble <166992735+greeble-dev@users.noreply.github.com>
This commit is contained in:
Maksymilian Mozolewski
2026-05-04 05:22:34 +01:00
committed by Carter Anderson
parent c757497b27
commit ff8f3b5705
2 changed files with 80 additions and 3 deletions
+78 -2
View File
@@ -5,7 +5,7 @@ use crate::{
use alloc::sync::Arc;
use bevy_ecs::template::{FromTemplate, SpecializeFromTemplate, Template, TemplateContext};
use bevy_platform::{collections::Equivalent, sync::Mutex};
use bevy_reflect::{Reflect, TypePath};
use bevy_reflect::{enums::Enum, FromReflect, PartialReflect, Reflect, ReflectRef, TypePath};
use core::{
any::TypeId,
hash::{Hash, Hasher},
@@ -130,7 +130,7 @@ impl core::fmt::Debug for StrongHandle {
///
/// [`Handle::Strong`], via [`StrongHandle`] also provides access to useful [`Asset`] metadata, such as the [`AssetPath`] (if it exists).
#[derive(Reflect)]
#[reflect(Debug, Hash, PartialEq, Clone, Handle)]
#[reflect(Debug, Hash, PartialEq, Clone, Handle, from_reflect = false)]
pub enum Handle<A: Asset> {
/// A "strong" reference to a live (or loading) [`Asset`]. If a [`Handle`] is [`Handle::Strong`], the [`Asset`] will be kept
/// alive until the [`Handle`] is dropped. Strong handles also provide access to additional asset metadata.
@@ -140,6 +140,43 @@ pub enum Handle<A: Asset> {
Uuid(Uuid, #[reflect(ignore, clone)] PhantomData<fn() -> A>),
}
// `Handle` needs a custom `FromReflect` to do extra type checking - see the
// `strong_handle.type_id` check below.
// `Handle` needs a custom `FromReflect` to do extra type checking - see the
// `strong_handle.type_id` check below.
impl<A: Asset> FromReflect for Handle<A>
where
Handle<A>: Send + Sync,
A: TypePath,
{
fn from_reflect(reflect_value: &dyn PartialReflect) -> Option<Self> {
let ReflectRef::Enum(enum_value) = PartialReflect::reflect_ref(reflect_value) else {
return None;
};
match Enum::variant_name(enum_value) {
"Strong" => {
let strong_field = enum_value.field_at(0usize)?;
let strong_handle = Arc::<StrongHandle>::from_reflect(strong_field)?;
// This is necessary as otherwise you could construct Handle<A> via Handle<B>
if strong_handle.type_id != TypeId::of::<A>() {
return None;
}
Some(Handle::Strong(strong_handle))
}
"Uuid" => {
let uuid_field = enum_value.field_at(0usize)?;
let uuid = Uuid::from_reflect(uuid_field)?;
Some(Handle::Uuid(uuid, Default::default()))
}
_ => None,
}
}
}
impl<T: Asset> Clone for Handle<T> {
fn clone(&self) -> Self {
match self {
@@ -864,4 +901,43 @@ mod tests {
_ => panic!("Expected a strong handle"),
}
}
#[test]
fn handle_from_reflect_verifies_type_id() {
use crate::{AssetApp, Assets};
use bevy_reflect::FromReflect;
#[derive(Reflect, Asset)]
struct A;
#[derive(Reflect, Asset)]
struct B;
let mut app = create_app().0;
app.init_asset::<A>().init_asset::<B>();
let mut assets = app.world_mut().resource_mut::<Assets<A>>();
let handle_a = assets.add(A);
let dynamic_handle_a = handle_a.to_dynamic();
let reflected_handle_a = handle_a.as_partial_reflect();
let handle_b_from_reflect_dynamic: Option<Handle<B>> =
FromReflect::from_reflect(&*dynamic_handle_a);
let handle_b_from_reflect: Option<Handle<B>> =
FromReflect::from_reflect(reflected_handle_a);
let handle_a_from_reflect: Option<Handle<A>> =
FromReflect::from_reflect(reflected_handle_a);
assert!(
handle_b_from_reflect.is_none(),
"Handle<B> should not be constructible from reflected Handle<A>"
);
assert!(
handle_b_from_reflect_dynamic.is_none(),
"Handle<B> should not be constructible from dynamic Handle<A>"
);
assert!(
handle_a_from_reflect.is_some(),
"Handle<A> should be constructible from reflected Handle<A>"
);
}
}
+2 -1
View File
@@ -123,7 +123,8 @@ fn match_reflect_impls(ast: DeriveInput, source: ReflectImplSource) -> TokenStre
/// It will automatically generate implementations for `Reflect`, `Typed`, `GetTypeRegistration`, and `FromReflect`.
/// And, depending on the item's structure, will either implement `Struct`, `TupleStruct`, or `Enum`.
///
/// See the [`FromReflect`] derive macro for more information on how to customize the `FromReflect` implementation.
/// See the [`FromReflect`] derive macro for more information on how to customize the [`FromReflect`] implementation.
/// To implement [`FromReflect`] manually while deriving [`Reflect`], [opt out](#reflectfrom_reflect--false) of the default implementation.
///
/// # Container Attributes
///