Enforce type safe usage of Handle::get (#4794)
# Objective - Sometimes, people might load an asset as one type, then use it with an `Asset`s for a different type. - See e.g. #4784. - This is especially likely with the Gltf types, since users may not have a clear conceptual model of what types the assets will be. - We had an instance of this ourselves, in the `scene_viewer` example ## Solution - Make `Assets::get` require a type safe handle. --- ## Changelog ### Changed - `Assets::<T>::get` and `Assets::<T>::get_mut` now require that the passed handles are `Handle<T>`, improving the type safety of handles. ### Added - `HandleUntyped::typed_weak`, a helper function for creating a weak typed version of an exisitng `HandleUntyped`. ## Migration Guide `Assets::<T>::get` and `Assets::<T>::get_mut` now require that the passed handles are `Handle<T>`, improving the type safety of handles. If you were previously passing in: - a `HandleId`, use `&Handle::weak(id)` instead, to create a weak handle. You may have been able to store a type safe `Handle` instead. - a `HandleUntyped`, use `&handle_untyped.typed_weak()` to create a weak handle of the specified type. This is most likely to be the useful when using [load_folder](https://docs.rs/bevy_asset/latest/bevy_asset/struct.AssetServer.html#method.load_folder) - a `Handle<U>` of of a different type, consider whether this is the correct handle type to store. If it is (i.e. the same handle id is used for multiple different Asset types) use `Handle::weak(handle.id)` to cast to a different type.
This commit is contained in:
parent
a02c5ae819
commit
1bbd5c25c0
@ -781,8 +781,11 @@ mod test {
|
||||
asset_server.get_handle_untyped(id)
|
||||
}
|
||||
|
||||
fn get_asset(id: impl Into<HandleId>, world: &World) -> Option<&PngAsset> {
|
||||
world.resource::<Assets<PngAsset>>().get(id.into())
|
||||
fn get_asset<'world>(
|
||||
id: &Handle<PngAsset>,
|
||||
world: &'world World,
|
||||
) -> Option<&'world PngAsset> {
|
||||
world.resource::<Assets<PngAsset>>().get(id)
|
||||
}
|
||||
|
||||
fn get_load_state(id: impl Into<HandleId>, world: &World) -> LoadState {
|
||||
@ -800,7 +803,7 @@ mod test {
|
||||
);
|
||||
|
||||
// load the asset
|
||||
let handle = load_asset(path.clone(), &app.world);
|
||||
let handle = load_asset(path.clone(), &app.world).typed();
|
||||
let weak_handle = handle.clone_weak();
|
||||
|
||||
// asset is loading
|
||||
@ -826,7 +829,7 @@ mod test {
|
||||
assert!(get_asset(&weak_handle, &app.world).is_none());
|
||||
|
||||
// finally, reload the asset
|
||||
let handle = load_asset(path.clone(), &app.world);
|
||||
let handle = load_asset(path.clone(), &app.world).typed();
|
||||
assert_eq!(LoadState::Loading, get_load_state(&handle, &app.world));
|
||||
app.update();
|
||||
assert_eq!(LoadState::Loaded, get_load_state(&handle, &app.world));
|
||||
|
@ -129,12 +129,12 @@ impl<T: Asset> Assets<T> {
|
||||
///
|
||||
/// This is the main method for accessing asset data from an [Assets] collection. If you need
|
||||
/// mutable access to the asset, use [`get_mut`](Assets::get_mut).
|
||||
pub fn get<H: Into<HandleId>>(&self, handle: H) -> Option<&T> {
|
||||
pub fn get(&self, handle: &Handle<T>) -> Option<&T> {
|
||||
self.assets.get(&handle.into())
|
||||
}
|
||||
|
||||
/// Checks if an asset exists for the given handle
|
||||
pub fn contains<H: Into<HandleId>>(&self, handle: H) -> bool {
|
||||
pub fn contains(&self, handle: &Handle<T>) -> bool {
|
||||
self.assets.contains_key(&handle.into())
|
||||
}
|
||||
|
||||
@ -142,7 +142,7 @@ impl<T: Asset> Assets<T> {
|
||||
///
|
||||
/// This is the main method for mutably accessing asset data from an [Assets] collection. If you
|
||||
/// do not need mutable access to the asset, you may also use [get](Assets::get).
|
||||
pub fn get_mut<H: Into<HandleId>>(&mut self, handle: H) -> Option<&mut T> {
|
||||
pub fn get_mut(&mut self, handle: &Handle<T>) -> Option<&mut T> {
|
||||
let id: HandleId = handle.into();
|
||||
self.events.send(AssetEvent::Modified {
|
||||
handle: Handle::weak(id),
|
||||
@ -398,6 +398,6 @@ mod tests {
|
||||
let handle = assets_before.add(MyAsset);
|
||||
app.add_asset::<MyAsset>(); // Ensure this doesn't overwrite the Asset
|
||||
let assets_after = app.world.resource_mut::<Assets<MyAsset>>();
|
||||
assert!(assets_after.get(handle).is_some());
|
||||
assert!(assets_after.get(&handle).is_some());
|
||||
}
|
||||
}
|
||||
|
@ -169,7 +169,7 @@ impl<T: Asset> Handle<T> {
|
||||
/// Makes this handle Strong if it wasn't already.
|
||||
///
|
||||
/// This method requires the corresponding [Assets](crate::Assets) collection
|
||||
pub fn make_strong(&mut self, assets: &mut Assets<T>) {
|
||||
pub fn make_strong(&mut self, assets: &Assets<T>) {
|
||||
if self.is_strong() {
|
||||
return;
|
||||
}
|
||||
@ -341,6 +341,14 @@ impl HandleUntyped {
|
||||
matches!(self.handle_type, HandleType::Strong(_))
|
||||
}
|
||||
|
||||
/// Create a weak typed [`Handle`] from this handle.
|
||||
///
|
||||
/// If this handle is strong and dropped, there is no guarantee that the asset
|
||||
/// will still be available (if only the returned handle is kept)
|
||||
pub fn typed_weak<T: Asset>(&self) -> Handle<T> {
|
||||
self.clone_weak().typed()
|
||||
}
|
||||
|
||||
/// Convert this handle into a typed [Handle].
|
||||
///
|
||||
/// The new handle will maintain the Strong or Weak status of the current handle.
|
||||
|
@ -66,7 +66,7 @@ impl<ID: Hash + Eq> TextPipeline<ID> {
|
||||
.iter()
|
||||
.map(|section| {
|
||||
let font = fonts
|
||||
.get(section.style.font.id)
|
||||
.get(§ion.style.font)
|
||||
.ok_or(TextError::NoSuchFont)?;
|
||||
let font_id = self.get_or_insert_font_id(§ion.style.font, font);
|
||||
let font_size = scale_value(section.style.font_size, scale_factor);
|
||||
|
@ -98,7 +98,7 @@ pub fn extract_text2d_sprite(
|
||||
.color
|
||||
.as_rgba_linear();
|
||||
let atlas = texture_atlases
|
||||
.get(text_glyph.atlas_info.texture_atlas.clone_weak())
|
||||
.get(&text_glyph.atlas_info.texture_atlas)
|
||||
.unwrap();
|
||||
let handle = atlas.texture.clone_weak();
|
||||
let index = text_glyph.atlas_info.glyph_index as usize;
|
||||
|
@ -13,6 +13,7 @@ keywords = ["bevy"]
|
||||
bevy_app = { path = "../bevy_app", version = "0.8.0-dev" }
|
||||
bevy_asset = { path = "../bevy_asset", version = "0.8.0-dev" }
|
||||
bevy_core_pipeline = { path = "../bevy_core_pipeline", version = "0.8.0-dev" }
|
||||
bevy_derive = { path = "../bevy_derive", version = "0.8.0-dev" }
|
||||
bevy_ecs = { path = "../bevy_ecs", version = "0.8.0-dev" }
|
||||
bevy_hierarchy = { path = "../bevy_hierarchy", version = "0.8.0-dev" }
|
||||
bevy_input = { path = "../bevy_input", version = "0.8.0-dev" }
|
||||
|
@ -146,7 +146,7 @@ pub fn extract_uinodes(
|
||||
}
|
||||
let image = image.0.clone_weak();
|
||||
// Skip loading images
|
||||
if !images.contains(image.clone_weak()) {
|
||||
if !images.contains(&image) {
|
||||
continue;
|
||||
}
|
||||
extracted_uinodes.uinodes.push(ExtractedUiNode {
|
||||
@ -196,7 +196,7 @@ pub fn extract_text_uinodes(
|
||||
for text_glyph in text_glyphs {
|
||||
let color = text.sections[text_glyph.section_index].style.color;
|
||||
let atlas = texture_atlases
|
||||
.get(text_glyph.atlas_info.texture_atlas.clone_weak())
|
||||
.get(&text_glyph.atlas_info.texture_atlas)
|
||||
.unwrap();
|
||||
let texture = atlas.texture.clone_weak();
|
||||
let index = text_glyph.atlas_info.glyph_index as usize;
|
||||
|
@ -1,5 +1,6 @@
|
||||
use crate::{Size, UiRect};
|
||||
use bevy_asset::Handle;
|
||||
use bevy_derive::{Deref, DerefMut};
|
||||
use bevy_ecs::{prelude::Component, reflect::ReflectComponent};
|
||||
use bevy_math::Vec2;
|
||||
use bevy_reflect::prelude::*;
|
||||
@ -369,7 +370,7 @@ impl From<Color> for UiColor {
|
||||
}
|
||||
|
||||
/// The image of the node
|
||||
#[derive(Component, Clone, Debug, Reflect)]
|
||||
#[derive(Component, Clone, Debug, Reflect, Deref, DerefMut)]
|
||||
#[reflect(Component, Default)]
|
||||
pub struct UiImage(pub Handle<Image>);
|
||||
|
||||
|
@ -31,7 +31,7 @@ pub fn image_node_system(
|
||||
mut query: Query<(&mut CalculatedSize, &UiImage), With<ImageMode>>,
|
||||
) {
|
||||
for (mut calculated_size, image) in query.iter_mut() {
|
||||
if let Some(texture) = textures.get(image.0.clone_weak()) {
|
||||
if let Some(texture) = textures.get(image) {
|
||||
let size = Size {
|
||||
width: texture.texture_descriptor.size.width as f32,
|
||||
height: texture.texture_descriptor.size.height as f32,
|
||||
|
@ -50,8 +50,9 @@ fn setup(
|
||||
) {
|
||||
let mut texture_atlas_builder = TextureAtlasBuilder::default();
|
||||
for handle in &rpg_sprite_handles.handles {
|
||||
let texture = textures.get(handle).unwrap();
|
||||
texture_atlas_builder.add_texture(handle.clone_weak().typed::<Image>(), texture);
|
||||
let handle = handle.typed_weak();
|
||||
let texture = textures.get(&handle).expect("Textures folder contained a file which way matched by a loader which did not create an `Image` asset");
|
||||
texture_atlas_builder.add_texture(handle, texture);
|
||||
}
|
||||
|
||||
let texture_atlas = texture_atlas_builder.finish(&mut textures).unwrap();
|
||||
|
@ -67,7 +67,7 @@ Controls:
|
||||
}
|
||||
|
||||
struct SceneHandle {
|
||||
handle: Handle<Scene>,
|
||||
handle: Handle<Gltf>,
|
||||
animations: Vec<Handle<AnimationClip>>,
|
||||
instance_id: Option<InstanceId>,
|
||||
is_loaded: bool,
|
||||
|
Loading…
Reference in New Issue
Block a user