From 76688e00697b1afe522efda0d74d04c3d755ea7e Mon Sep 17 00:00:00 2001 From: Leonardo Giovanni Scur Date: Sat, 24 Jul 2021 18:25:43 -0300 Subject: [PATCH 1/3] Change HandleUntyped::typed to not panic Fixes #2534 Introduces a new FIXME: Naughty pipeline handle should be typed for a strange pattern encountered in internal uses of `typed`. --- crates/bevy_asset/src/asset_server.rs | 7 +++++-- crates/bevy_asset/src/handle.rs | 8 ++++---- crates/bevy_pbr/src/entity.rs | 2 +- crates/bevy_pbr/src/render_graph/pbr_pipeline/mod.rs | 1 + crates/bevy_render/src/wireframe/mod.rs | 3 ++- crates/bevy_sprite/src/entity.rs | 8 ++++---- crates/bevy_sprite/src/lib.rs | 1 + crates/bevy_sprite/src/render/mod.rs | 2 ++ crates/bevy_text/src/draw.rs | 6 +++--- crates/bevy_ui/src/entity.rs | 12 ++++++------ crates/bevy_ui/src/render/mod.rs | 1 + 11 files changed, 30 insertions(+), 21 deletions(-) diff --git a/crates/bevy_asset/src/asset_server.rs b/crates/bevy_asset/src/asset_server.rs index d5a331909de34..fb75f6c1b589a 100644 --- a/crates/bevy_asset/src/asset_server.rs +++ b/crates/bevy_asset/src/asset_server.rs @@ -229,8 +229,11 @@ impl AssetServer { /// [`AssetServerSettings`](crate::AssetServerSettings) resource. The default name is /// `"assets"`. #[must_use = "not using the returned strong handle may result in the unexpected release of the asset"] - pub fn load<'a, T: Asset, P: Into>>(&self, path: P) -> Handle { - self.load_untyped(path).typed() + pub fn load<'a, T: Asset, P: Into>>(&self, path: P) -> Result, AssetServerError> { + match self.load_untyped(path).typed() { + Ok(handle) => Ok(handle), + Err(_) => Err(AssetServerError::IncorrectHandleType) + } } async fn load_async( diff --git a/crates/bevy_asset/src/handle.rs b/crates/bevy_asset/src/handle.rs index ae2bb45c0837c..2b0e1cf74a926 100644 --- a/crates/bevy_asset/src/handle.rs +++ b/crates/bevy_asset/src/handle.rs @@ -275,10 +275,10 @@ impl HandleUntyped { matches!(self.handle_type, HandleType::Strong(_)) } - pub fn typed(mut self) -> Handle { + pub fn typed(mut self) -> Result, HandleUntyped> { if let HandleId::Id(type_uuid, _) = self.id { if T::TYPE_UUID != type_uuid { - panic!("Attempted to convert handle to invalid type."); + return Err(self) } } let handle_type = match &self.handle_type { @@ -287,11 +287,11 @@ impl HandleUntyped { }; // ensure we don't send the RefChange event when "self" is dropped self.handle_type = HandleType::Weak; - Handle { + Ok(Handle { handle_type, id: self.id, marker: PhantomData::default(), - } + }) } } diff --git a/crates/bevy_pbr/src/entity.rs b/crates/bevy_pbr/src/entity.rs index 9b1d3c6314743..01efc2daf8870 100644 --- a/crates/bevy_pbr/src/entity.rs +++ b/crates/bevy_pbr/src/entity.rs @@ -27,7 +27,7 @@ impl Default for PbrBundle { fn default() -> Self { Self { render_pipelines: RenderPipelines::from_pipelines(vec![RenderPipeline::new( - PBR_PIPELINE_HANDLE.typed(), + PBR_PIPELINE_HANDLE.typed().unwrap(), )]), mesh: Default::default(), visible: Default::default(), diff --git a/crates/bevy_pbr/src/render_graph/pbr_pipeline/mod.rs b/crates/bevy_pbr/src/render_graph/pbr_pipeline/mod.rs index 2653d7f535107..0accdce25f3cd 100644 --- a/crates/bevy_pbr/src/render_graph/pbr_pipeline/mod.rs +++ b/crates/bevy_pbr/src/render_graph/pbr_pipeline/mod.rs @@ -10,6 +10,7 @@ use bevy_render::{ texture::TextureFormat, }; +// FIXME: Naughty pipeline handle should be typed pub const PBR_PIPELINE_HANDLE: HandleUntyped = HandleUntyped::weak_from_u64(PipelineDescriptor::TYPE_UUID, 13148362314012771389); diff --git a/crates/bevy_render/src/wireframe/mod.rs b/crates/bevy_render/src/wireframe/mod.rs index a2862d4ee3c5e..54ba749f62cbc 100644 --- a/crates/bevy_render/src/wireframe/mod.rs +++ b/crates/bevy_render/src/wireframe/mod.rs @@ -18,6 +18,7 @@ use bevy_utils::HashSet; mod pipeline; +// FIXME: Naughty pipeline handle should be typed pub const WIREFRAME_PIPELINE_HANDLE: HandleUntyped = HandleUntyped::weak_from_u64(PipelineDescriptor::TYPE_UUID, 0x137c75ab7e9ad7f5); @@ -84,7 +85,7 @@ pub fn draw_wireframes_system( }; let mut render_pipeline = RenderPipeline::specialized( - WIREFRAME_PIPELINE_HANDLE.typed(), + WIREFRAME_PIPELINE_HANDLE.typed().unwrap(), PipelineSpecialization { sample_count: msaa.samples, strip_index_format: None, diff --git a/crates/bevy_sprite/src/entity.rs b/crates/bevy_sprite/src/entity.rs index 70fd8d5699640..4d3544a925e96 100644 --- a/crates/bevy_sprite/src/entity.rs +++ b/crates/bevy_sprite/src/entity.rs @@ -28,9 +28,9 @@ pub struct SpriteBundle { impl Default for SpriteBundle { fn default() -> Self { Self { - mesh: QUAD_HANDLE.typed(), + mesh: QUAD_HANDLE.typed().unwrap(), render_pipelines: RenderPipelines::from_pipelines(vec![RenderPipeline::new( - SPRITE_PIPELINE_HANDLE.typed(), + SPRITE_PIPELINE_HANDLE.typed().unwrap(), )]), visible: Visible { is_transparent: true, @@ -68,14 +68,14 @@ impl Default for SpriteSheetBundle { fn default() -> Self { Self { render_pipelines: RenderPipelines::from_pipelines(vec![RenderPipeline::new( - SPRITE_SHEET_PIPELINE_HANDLE.typed(), + SPRITE_SHEET_PIPELINE_HANDLE.typed().unwrap(), )]), visible: Visible { is_transparent: true, ..Default::default() }, main_pass: MainPass, - mesh: QUAD_HANDLE.typed(), + mesh: QUAD_HANDLE.typed().unwrap(), draw: Default::default(), sprite: Default::default(), texture_atlas: Default::default(), diff --git a/crates/bevy_sprite/src/lib.rs b/crates/bevy_sprite/src/lib.rs index eebb2bc25215b..57176c4a85053 100644 --- a/crates/bevy_sprite/src/lib.rs +++ b/crates/bevy_sprite/src/lib.rs @@ -63,6 +63,7 @@ impl Default for SpriteSettings { #[derive(Default)] pub struct SpritePlugin; +// FIXME: Naughty pipeline handle should be typed pub const QUAD_HANDLE: HandleUntyped = HandleUntyped::weak_from_u64(Mesh::TYPE_UUID, 14240461981130137526); diff --git a/crates/bevy_sprite/src/render/mod.rs b/crates/bevy_sprite/src/render/mod.rs index a166e5ebe791f..5abcf184a4e5d 100644 --- a/crates/bevy_sprite/src/render/mod.rs +++ b/crates/bevy_sprite/src/render/mod.rs @@ -12,9 +12,11 @@ use bevy_render::{ texture::TextureFormat, }; +// FIXME: Naughty pipeline handle should be typed pub const SPRITE_PIPELINE_HANDLE: HandleUntyped = HandleUntyped::weak_from_u64(PipelineDescriptor::TYPE_UUID, 2785347840338765446); +// FIXME: Naughty pipeline handle should be typed pub const SPRITE_SHEET_PIPELINE_HANDLE: HandleUntyped = HandleUntyped::weak_from_u64(PipelineDescriptor::TYPE_UUID, 9016885805180281612); diff --git a/crates/bevy_text/src/draw.rs b/crates/bevy_text/src/draw.rs index e4dd639d37598..88ed20e0cae2e 100644 --- a/crates/bevy_text/src/draw.rs +++ b/crates/bevy_text/src/draw.rs @@ -28,7 +28,7 @@ impl<'a> Drawable for DrawableText<'a> { fn draw(&mut self, draw: &mut Draw, context: &mut DrawContext) -> Result<(), DrawError> { context.set_pipeline( draw, - &bevy_sprite::SPRITE_SHEET_PIPELINE_HANDLE.typed(), + &bevy_sprite::SPRITE_SHEET_PIPELINE_HANDLE.typed().unwrap(), &PipelineSpecialization { sample_count: self.msaa.samples, vertex_buffer_layout: self.font_quad_vertex_layout.clone(), @@ -40,7 +40,7 @@ impl<'a> Drawable for DrawableText<'a> { if let Some(RenderResourceId::Buffer(vertex_attribute_buffer_id)) = render_resource_context .get_asset_resource( - &bevy_sprite::QUAD_HANDLE.typed::(), + &bevy_sprite::QUAD_HANDLE.typed::().unwrap(), mesh::VERTEX_ATTRIBUTE_BUFFER_ID, ) { @@ -52,7 +52,7 @@ impl<'a> Drawable for DrawableText<'a> { let mut indices = 0..0; if let Some(RenderResourceId::Buffer(quad_index_buffer)) = render_resource_context .get_asset_resource( - &bevy_sprite::QUAD_HANDLE.typed::(), + &bevy_sprite::QUAD_HANDLE.typed::().unwrap(), mesh::INDEX_BUFFER_ASSET_INDEX, ) { diff --git a/crates/bevy_ui/src/entity.rs b/crates/bevy_ui/src/entity.rs index d7312b4baf10a..c4c14033cb5d5 100644 --- a/crates/bevy_ui/src/entity.rs +++ b/crates/bevy_ui/src/entity.rs @@ -33,9 +33,9 @@ pub struct NodeBundle { impl Default for NodeBundle { fn default() -> Self { NodeBundle { - mesh: QUAD_HANDLE.typed(), + mesh: QUAD_HANDLE.typed().unwrap(), render_pipelines: RenderPipelines::from_pipelines(vec![RenderPipeline::new( - UI_PIPELINE_HANDLE.typed(), + UI_PIPELINE_HANDLE.typed().unwrap(), )]), visible: Visible { is_transparent: true, @@ -69,9 +69,9 @@ pub struct ImageBundle { impl Default for ImageBundle { fn default() -> Self { ImageBundle { - mesh: QUAD_HANDLE.typed(), + mesh: QUAD_HANDLE.typed().unwrap(), render_pipelines: RenderPipelines::from_pipelines(vec![RenderPipeline::new( - UI_PIPELINE_HANDLE.typed(), + UI_PIPELINE_HANDLE.typed().unwrap(), )]), node: Default::default(), image: Default::default(), @@ -143,9 +143,9 @@ impl Default for ButtonBundle { fn default() -> Self { ButtonBundle { button: Button, - mesh: QUAD_HANDLE.typed(), + mesh: QUAD_HANDLE.typed().unwrap(), render_pipelines: RenderPipelines::from_pipelines(vec![RenderPipeline::new( - UI_PIPELINE_HANDLE.typed(), + UI_PIPELINE_HANDLE.typed().unwrap(), )]), interaction: Default::default(), focus_policy: Default::default(), diff --git a/crates/bevy_ui/src/render/mod.rs b/crates/bevy_ui/src/render/mod.rs index 789bce7650db5..02e6d57092dc2 100644 --- a/crates/bevy_ui/src/render/mod.rs +++ b/crates/bevy_ui/src/render/mod.rs @@ -17,6 +17,7 @@ use bevy_render::{ texture::TextureFormat, }; +// FIXME: Naughty pipeline handle should be typed pub const UI_PIPELINE_HANDLE: HandleUntyped = HandleUntyped::weak_from_u64(PipelineDescriptor::TYPE_UUID, 3234320022263993878); From 087cdc24da77432aeedca7e73f9a2b69fd4b248b Mon Sep 17 00:00:00 2001 From: Leonardo Giovanni Scur Date: Sat, 24 Jul 2021 20:37:44 -0300 Subject: [PATCH 2/3] Return AssetServer::load back to the previous signature --- crates/bevy_asset/src/asset_server.rs | 9 ++++----- crates/bevy_asset/src/handle.rs | 7 +++++-- examples/2d/texture_atlas.rs | 11 ++++++++--- examples/3d/render_to_texture.rs | 3 ++- 4 files changed, 19 insertions(+), 11 deletions(-) diff --git a/crates/bevy_asset/src/asset_server.rs b/crates/bevy_asset/src/asset_server.rs index fb75f6c1b589a..c23295a19b5da 100644 --- a/crates/bevy_asset/src/asset_server.rs +++ b/crates/bevy_asset/src/asset_server.rs @@ -229,11 +229,10 @@ impl AssetServer { /// [`AssetServerSettings`](crate::AssetServerSettings) resource. The default name is /// `"assets"`. #[must_use = "not using the returned strong handle may result in the unexpected release of the asset"] - pub fn load<'a, T: Asset, P: Into>>(&self, path: P) -> Result, AssetServerError> { - match self.load_untyped(path).typed() { - Ok(handle) => Ok(handle), - Err(_) => Err(AssetServerError::IncorrectHandleType) - } + pub fn load<'a, T: Asset, P: Into>>(&self, path: P) -> Handle { + self.load_untyped(path).typed().expect( + "Failed to convert untyped handle from asset to typed, this should never happen. Report a bug!", + ) } async fn load_async( diff --git a/crates/bevy_asset/src/handle.rs b/crates/bevy_asset/src/handle.rs index 2b0e1cf74a926..e35e8135e1048 100644 --- a/crates/bevy_asset/src/handle.rs +++ b/crates/bevy_asset/src/handle.rs @@ -275,18 +275,21 @@ impl HandleUntyped { matches!(self.handle_type, HandleType::Strong(_)) } - pub fn typed(mut self) -> Result, HandleUntyped> { + pub fn typed(mut self) -> Result, Self> { if let HandleId::Id(type_uuid, _) = self.id { if T::TYPE_UUID != type_uuid { - return Err(self) + return Err(self); } } + let handle_type = match &self.handle_type { HandleType::Strong(sender) => HandleType::Strong(sender.clone()), HandleType::Weak => HandleType::Weak, }; + // ensure we don't send the RefChange event when "self" is dropped self.handle_type = HandleType::Weak; + Ok(Handle { handle_type, id: self.id, diff --git a/examples/2d/texture_atlas.rs b/examples/2d/texture_atlas.rs index 9e9f28ca72174..2210e3b560ade 100644 --- a/examples/2d/texture_atlas.rs +++ b/examples/2d/texture_atlas.rs @@ -21,11 +21,16 @@ enum AppState { #[derive(Default)] struct RpgSpriteHandles { - handles: Vec, + handles: Vec>, } fn load_textures(mut rpg_sprite_handles: ResMut, asset_server: Res) { - rpg_sprite_handles.handles = asset_server.load_folder("textures/rpg").unwrap(); + if let Ok(handles) = asset_server.load_folder("textures/rpg") { + rpg_sprite_handles.handles = handles + .into_iter() + .flat_map(|handle| handle.typed::()) + .collect(); + } } fn check_textures( @@ -51,7 +56,7 @@ fn setup( let mut texture_atlas_builder = TextureAtlasBuilder::default(); for handle in rpg_sprite_handles.handles.iter() { let texture = textures.get(handle).unwrap(); - texture_atlas_builder.add_texture(handle.clone_weak().typed::(), texture); + texture_atlas_builder.add_texture(handle.clone_weak(), texture); } let texture_atlas = texture_atlas_builder.finish(&mut textures).unwrap(); diff --git a/examples/3d/render_to_texture.rs b/examples/3d/render_to_texture.rs index 2884f2c0e15b2..ffe93d850f932 100644 --- a/examples/3d/render_to_texture.rs +++ b/examples/3d/render_to_texture.rs @@ -21,6 +21,7 @@ use bevy::{ pub struct FirstPass; +// FIXME: Naughty pipeline handle should be typed pub const RENDER_TEXTURE_HANDLE: HandleUntyped = HandleUntyped::weak_from_u64(Texture::TYPE_UUID, 13378939762009864029); @@ -186,7 +187,7 @@ fn setup( commands.spawn_bundle(first_pass_camera); - let texture_handle = RENDER_TEXTURE_HANDLE.typed(); + let texture_handle = RENDER_TEXTURE_HANDLE.typed().unwrap(); let cube_size = 4.0; let cube_handle = meshes.add(Mesh::from(shape::Box::new(cube_size, cube_size, cube_size))); From 7eb34134e965ab3bd989d2c6dff817cb785d36af Mon Sep 17 00:00:00 2001 From: Leonardo Giovanni Scur Date: Sun, 25 Jul 2021 15:21:24 -0300 Subject: [PATCH 3/3] Clearer wording for FIXME --- crates/bevy_pbr/src/render_graph/pbr_pipeline/mod.rs | 2 +- crates/bevy_render/src/wireframe/mod.rs | 2 +- crates/bevy_sprite/src/lib.rs | 2 +- crates/bevy_sprite/src/render/mod.rs | 4 ++-- crates/bevy_ui/src/render/mod.rs | 2 +- examples/3d/render_to_texture.rs | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/bevy_pbr/src/render_graph/pbr_pipeline/mod.rs b/crates/bevy_pbr/src/render_graph/pbr_pipeline/mod.rs index 0accdce25f3cd..6c520f23e5346 100644 --- a/crates/bevy_pbr/src/render_graph/pbr_pipeline/mod.rs +++ b/crates/bevy_pbr/src/render_graph/pbr_pipeline/mod.rs @@ -10,7 +10,7 @@ use bevy_render::{ texture::TextureFormat, }; -// FIXME: Naughty pipeline handle should be typed +// FIXME: Handle is always converted to typed, see discussion in PR #2536 pub const PBR_PIPELINE_HANDLE: HandleUntyped = HandleUntyped::weak_from_u64(PipelineDescriptor::TYPE_UUID, 13148362314012771389); diff --git a/crates/bevy_render/src/wireframe/mod.rs b/crates/bevy_render/src/wireframe/mod.rs index 54ba749f62cbc..e873cc78afb59 100644 --- a/crates/bevy_render/src/wireframe/mod.rs +++ b/crates/bevy_render/src/wireframe/mod.rs @@ -18,7 +18,7 @@ use bevy_utils::HashSet; mod pipeline; -// FIXME: Naughty pipeline handle should be typed +// FIXME: Handle is always converted to typed, see discussion in PR #2536 pub const WIREFRAME_PIPELINE_HANDLE: HandleUntyped = HandleUntyped::weak_from_u64(PipelineDescriptor::TYPE_UUID, 0x137c75ab7e9ad7f5); diff --git a/crates/bevy_sprite/src/lib.rs b/crates/bevy_sprite/src/lib.rs index 57176c4a85053..59fc1178383f3 100644 --- a/crates/bevy_sprite/src/lib.rs +++ b/crates/bevy_sprite/src/lib.rs @@ -63,7 +63,7 @@ impl Default for SpriteSettings { #[derive(Default)] pub struct SpritePlugin; -// FIXME: Naughty pipeline handle should be typed +// FIXME: Handle is always converted to typed, see discussion in PR #2536 pub const QUAD_HANDLE: HandleUntyped = HandleUntyped::weak_from_u64(Mesh::TYPE_UUID, 14240461981130137526); diff --git a/crates/bevy_sprite/src/render/mod.rs b/crates/bevy_sprite/src/render/mod.rs index 5abcf184a4e5d..1b474c7e1b52a 100644 --- a/crates/bevy_sprite/src/render/mod.rs +++ b/crates/bevy_sprite/src/render/mod.rs @@ -12,11 +12,11 @@ use bevy_render::{ texture::TextureFormat, }; -// FIXME: Naughty pipeline handle should be typed +// FIXME: Handle is always converted to typed, see discussion in PR #2536 pub const SPRITE_PIPELINE_HANDLE: HandleUntyped = HandleUntyped::weak_from_u64(PipelineDescriptor::TYPE_UUID, 2785347840338765446); -// FIXME: Naughty pipeline handle should be typed +// FIXME: Handle is always converted to typed, see discussion in PR #2536 pub const SPRITE_SHEET_PIPELINE_HANDLE: HandleUntyped = HandleUntyped::weak_from_u64(PipelineDescriptor::TYPE_UUID, 9016885805180281612); diff --git a/crates/bevy_ui/src/render/mod.rs b/crates/bevy_ui/src/render/mod.rs index 02e6d57092dc2..f7e9c262b94c1 100644 --- a/crates/bevy_ui/src/render/mod.rs +++ b/crates/bevy_ui/src/render/mod.rs @@ -17,7 +17,7 @@ use bevy_render::{ texture::TextureFormat, }; -// FIXME: Naughty pipeline handle should be typed +// FIXME: Handle is always converted to typed, see discussion in PR #2536 pub const UI_PIPELINE_HANDLE: HandleUntyped = HandleUntyped::weak_from_u64(PipelineDescriptor::TYPE_UUID, 3234320022263993878); diff --git a/examples/3d/render_to_texture.rs b/examples/3d/render_to_texture.rs index ffe93d850f932..b40b6ca9bcac4 100644 --- a/examples/3d/render_to_texture.rs +++ b/examples/3d/render_to_texture.rs @@ -21,7 +21,7 @@ use bevy::{ pub struct FirstPass; -// FIXME: Naughty pipeline handle should be typed +// FIXME: Handle is always converted to typed, see discussion in PR #2536 pub const RENDER_TEXTURE_HANDLE: HandleUntyped = HandleUntyped::weak_from_u64(Texture::TYPE_UUID, 13378939762009864029);