Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 15, 2025
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a7ea451
Replace `UntypedHandle` from ReflectAsset with `impl Into<UntypedAsse…
andriyDev Jun 12, 2025
20f36be
Add a migration guide.
andriyDev Jun 13, 2025
2c4d268
Update doc comments to use a reference to an UntypedHandle.
andriyDev Jun 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 43 additions & 34 deletions crates/bevy_asset/src/reflect.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -42,23 +42,27 @@ 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(
unsafe_code,
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())
}
}

Expand All @@ -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
///
Expand All @@ -96,24 +100,33 @@ 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`]
pub fn add(&self, world: &mut World, value: &dyn PartialReflect) -> UntypedHandle {
(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`]
Expand All @@ -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| {
Expand All @@ -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>>();
Expand All @@ -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>)
},
}
Expand Down Expand Up @@ -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:?}");
/// }
/// ```
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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()
Expand All @@ -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);
}
}
25 changes: 25 additions & 0 deletions release-content/migration-guides/reflect_asset_asset_ids.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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();
```