Replace UntypedHandle from ReflectAsset with impl Into<UntypedAssetId>. (#19606)

# Objective

- A step towards #19024.
- Allow `ReflectAsset` to work with any `AssetId` not just `Handle`.
- `ReflectAsset::ids()` returns an iterator of `AssetId`s, but then
there's no way to use these ids, since all the other APIs in
`ReflectAsset` require a handle (and we don't have a reflect way to get
the handle).

## Solution

- Replace the `UntypedHandle` argument in `ReflectAsset` methods with
`impl Into<UntypedAssetId>`.
- This matches the regular asset API.
- This allows `ReflectAsset::ids()` to be more useful.

## Testing

- None.
This commit is contained in:
andriyDev 2025-06-15 09:42:54 -07:00 committed by GitHub
parent 8bce3e46f2
commit 98c14e5917
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 68 additions and 34 deletions

View File

@ -18,16 +18,16 @@ pub struct ReflectAsset {
handle_type_id: TypeId,
assets_resource_type_id: TypeId,
get: fn(&World, UntypedHandle) -> Option<&dyn Reflect>,
get: fn(&World, UntypedAssetId) -> Option<&dyn Reflect>,
// SAFETY:
// - may only be called with an [`UnsafeWorldCell`] which can be used to access the corresponding `Assets<T>` resource mutably
// - may only be used to access **at most one** access at once
get_unchecked_mut: unsafe fn(UnsafeWorldCell<'_>, UntypedHandle) -> Option<&mut dyn Reflect>,
get_unchecked_mut: unsafe fn(UnsafeWorldCell<'_>, UntypedAssetId) -> Option<&mut dyn Reflect>,
add: fn(&mut World, &dyn PartialReflect) -> UntypedHandle,
insert: fn(&mut World, UntypedHandle, &dyn PartialReflect),
insert: fn(&mut World, UntypedAssetId, &dyn PartialReflect),
len: fn(&World) -> usize,
ids: for<'w> fn(&'w World) -> Box<dyn Iterator<Item = UntypedAssetId> + 'w>,
remove: fn(&mut World, UntypedHandle) -> Option<Box<dyn Reflect>>,
remove: fn(&mut World, UntypedAssetId) -> Option<Box<dyn Reflect>>,
}
impl ReflectAsset {
@ -42,15 +42,19 @@ impl ReflectAsset {
}
/// Equivalent of [`Assets::get`]
pub fn get<'w>(&self, world: &'w World, handle: UntypedHandle) -> Option<&'w dyn Reflect> {
(self.get)(world, handle)
pub fn get<'w>(
&self,
world: &'w World,
asset_id: impl Into<UntypedAssetId>,
) -> Option<&'w dyn Reflect> {
(self.get)(world, asset_id.into())
}
/// Equivalent of [`Assets::get_mut`]
pub fn get_mut<'w>(
&self,
world: &'w mut World,
handle: UntypedHandle,
asset_id: impl Into<UntypedAssetId>,
) -> Option<&'w mut dyn Reflect> {
// SAFETY: unique world access
#[expect(
@ -58,7 +62,7 @@ impl ReflectAsset {
reason = "Use of unsafe `Self::get_unchecked_mut()` function."
)]
unsafe {
(self.get_unchecked_mut)(world.as_unsafe_world_cell(), handle)
(self.get_unchecked_mut)(world.as_unsafe_world_cell(), asset_id.into())
}
}
@ -76,8 +80,8 @@ impl ReflectAsset {
/// # let handle_1: UntypedHandle = unimplemented!();
/// # let handle_2: UntypedHandle = unimplemented!();
/// let unsafe_world_cell = world.as_unsafe_world_cell();
/// let a = unsafe { reflect_asset.get_unchecked_mut(unsafe_world_cell, handle_1).unwrap() };
/// let b = unsafe { reflect_asset.get_unchecked_mut(unsafe_world_cell, handle_2).unwrap() };
/// let a = unsafe { reflect_asset.get_unchecked_mut(unsafe_world_cell, &handle_1).unwrap() };
/// let b = unsafe { reflect_asset.get_unchecked_mut(unsafe_world_cell, &handle_2).unwrap() };
/// // ^ not allowed, two mutable references through the same asset resource, even though the
/// // handles are distinct
///
@ -96,10 +100,10 @@ impl ReflectAsset {
pub unsafe fn get_unchecked_mut<'w>(
&self,
world: UnsafeWorldCell<'w>,
handle: UntypedHandle,
asset_id: impl Into<UntypedAssetId>,
) -> Option<&'w mut dyn Reflect> {
// SAFETY: requirements are deferred to the caller
unsafe { (self.get_unchecked_mut)(world, handle) }
unsafe { (self.get_unchecked_mut)(world, asset_id.into()) }
}
/// Equivalent of [`Assets::add`]
@ -107,13 +111,22 @@ impl ReflectAsset {
(self.add)(world, value)
}
/// Equivalent of [`Assets::insert`]
pub fn insert(&self, world: &mut World, handle: UntypedHandle, value: &dyn PartialReflect) {
(self.insert)(world, handle, value);
pub fn insert(
&self,
world: &mut World,
asset_id: impl Into<UntypedAssetId>,
value: &dyn PartialReflect,
) {
(self.insert)(world, asset_id.into(), value);
}
/// Equivalent of [`Assets::remove`]
pub fn remove(&self, world: &mut World, handle: UntypedHandle) -> Option<Box<dyn Reflect>> {
(self.remove)(world, handle)
pub fn remove(
&self,
world: &mut World,
asset_id: impl Into<UntypedAssetId>,
) -> Option<Box<dyn Reflect>> {
(self.remove)(world, asset_id.into())
}
/// Equivalent of [`Assets::len`]
@ -137,17 +150,17 @@ impl<A: Asset + FromReflect> FromType<A> for ReflectAsset {
ReflectAsset {
handle_type_id: TypeId::of::<Handle<A>>(),
assets_resource_type_id: TypeId::of::<Assets<A>>(),
get: |world, handle| {
get: |world, asset_id| {
let assets = world.resource::<Assets<A>>();
let asset = assets.get(&handle.typed_debug_checked());
let asset = assets.get(asset_id.typed_debug_checked());
asset.map(|asset| asset as &dyn Reflect)
},
get_unchecked_mut: |world, handle| {
get_unchecked_mut: |world, asset_id| {
// SAFETY: `get_unchecked_mut` must be called with `UnsafeWorldCell` having access to `Assets<A>`,
// and must ensure to only have at most one reference to it live at all times.
#[expect(unsafe_code, reason = "Uses `UnsafeWorldCell::get_resource_mut()`.")]
let assets = unsafe { world.get_resource_mut::<Assets<A>>().unwrap().into_inner() };
let asset = assets.get_mut(&handle.typed_debug_checked());
let asset = assets.get_mut(asset_id.typed_debug_checked());
asset.map(|asset| asset as &mut dyn Reflect)
},
add: |world, value| {
@ -156,11 +169,11 @@ impl<A: Asset + FromReflect> FromType<A> for ReflectAsset {
.expect("could not call `FromReflect::from_reflect` in `ReflectAsset::add`");
assets.add(value).untyped()
},
insert: |world, handle, value| {
insert: |world, asset_id, value| {
let mut assets = world.resource_mut::<Assets<A>>();
let value: A = FromReflect::from_reflect(value)
.expect("could not call `FromReflect::from_reflect` in `ReflectAsset::set`");
assets.insert(&handle.typed_debug_checked(), value);
assets.insert(asset_id.typed_debug_checked(), value);
},
len: |world| {
let assets = world.resource::<Assets<A>>();
@ -170,9 +183,9 @@ impl<A: Asset + FromReflect> FromType<A> for ReflectAsset {
let assets = world.resource::<Assets<A>>();
Box::new(assets.ids().map(AssetId::untyped))
},
remove: |world, handle| {
remove: |world, asset_id| {
let mut assets = world.resource_mut::<Assets<A>>();
let value = assets.remove(&handle.typed_debug_checked());
let value = assets.remove(asset_id.typed_debug_checked());
value.map(|value| Box::new(value) as Box<dyn Reflect>)
},
}
@ -200,7 +213,7 @@ impl<A: Asset + FromReflect> FromType<A> for ReflectAsset {
/// let reflect_asset = type_registry.get_type_data::<ReflectAsset>(reflect_handle.asset_type_id()).unwrap();
///
/// let handle = reflect_handle.downcast_handle_untyped(handle.as_any()).unwrap();
/// let value = reflect_asset.get(world, handle).unwrap();
/// let value = reflect_asset.get(world, &handle).unwrap();
/// println!("{value:?}");
/// }
/// ```
@ -247,7 +260,7 @@ mod tests {
use alloc::{string::String, vec::Vec};
use core::any::TypeId;
use crate::{Asset, AssetApp, AssetPlugin, ReflectAsset, UntypedHandle};
use crate::{Asset, AssetApp, AssetPlugin, ReflectAsset};
use bevy_app::App;
use bevy_ecs::reflect::AppTypeRegistry;
use bevy_reflect::Reflect;
@ -281,7 +294,7 @@ mod tests {
let handle = reflect_asset.add(app.world_mut(), &value);
// struct is a reserved keyword, so we can't use it here
let strukt = reflect_asset
.get_mut(app.world_mut(), handle)
.get_mut(app.world_mut(), &handle)
.unwrap()
.reflect_mut()
.as_struct()
@ -294,16 +307,12 @@ mod tests {
assert_eq!(reflect_asset.len(app.world()), 1);
let ids: Vec<_> = reflect_asset.ids(app.world()).collect();
assert_eq!(ids.len(), 1);
let id = ids[0];
let fetched_handle = UntypedHandle::Weak(ids[0]);
let asset = reflect_asset
.get(app.world(), fetched_handle.clone_weak())
.unwrap();
let asset = reflect_asset.get(app.world(), id).unwrap();
assert_eq!(asset.downcast_ref::<AssetType>().unwrap().field, "edited");
reflect_asset
.remove(app.world_mut(), fetched_handle)
.unwrap();
reflect_asset.remove(app.world_mut(), id).unwrap();
assert_eq!(reflect_asset.len(app.world()), 0);
}
}

View File

@ -0,0 +1,25 @@
---
title: `ReflectAsset` now uses `UntypedAssetId` instead of `UntypedHandle`.
pull_requests: [19606]
---
Previously, `ReflectAsset` methods all required having `UntypedHandle`. The only way to get an
`UntypedHandle` through this API was with `ReflectAsset::add`. `ReflectAsset::ids` was not very
useful in this regard.
Now, all methods have been changed to accept `impl Into<UntypedAssetId>`, which matches our regular
`Assets<T>` API. This means you may need to change how you are calling these methods.
For example, if your code previously looked like:
```rust
let my_handle: UntypedHandle;
let my_asset = reflect_asset.get_mut(world, my_handle).unwrap();
```
You can migrate it to:
```rust
let my_handle: UntypedHandle;
let my_asset = reflect_asset.get_mut(world, &my_handle).unwrap();
```