From 5091ff73f6a2b8da57d487f469b7811be1e3a19c Mon Sep 17 00:00:00 2001 From: Pascal Hertleif Date: Sun, 29 Oct 2023 00:20:37 +0200 Subject: [PATCH] Derive Error for more error types (#10240) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Objective Align all error-like types to implement `Error`. Fixes #10176 ## Solution - Derive `Error` on more types - Refactor instances of manual implementations that could be derived This adds thiserror as a dependency to bevy_transform, which might increase compilation time -- but I don't know of any situation where you might only use that but not any other crate that pulls in bevy_utils. The `contributors` example has a `LoadContributorsError` type, but as it's an example I have not updated it. Doing that would mean either having a `use bevy_internal::utils::thiserror::Error;` in an example file, or adding `thiserror` as a dev-dependency to the main `bevy` crate. --- ## Changelog - All `…Error` types now implement the `Error` trait --- crates/bevy_app/src/app.rs | 4 +- crates/bevy_ecs/src/query/error.rs | 73 ++++--------------- crates/bevy_ecs/src/system/system_registry.rs | 6 +- crates/bevy_render/src/render_asset.rs | 4 +- .../src/render_resource/bind_group.rs | 3 + crates/bevy_transform/Cargo.toml | 1 + crates/bevy_transform/src/helper.rs | 6 +- crates/bevy_ui/src/layout/mod.rs | 7 +- examples/games/contributors.rs | 18 +++-- 9 files changed, 50 insertions(+), 72 deletions(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index 72f7e1a2bf7f0..821612a23214e 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -8,7 +8,7 @@ use bevy_ecs::{ ScheduleBuildSettings, ScheduleLabel, }, }; -use bevy_utils::{intern::Interned, tracing::debug, HashMap, HashSet}; +use bevy_utils::{intern::Interned, thiserror::Error, tracing::debug, HashMap, HashSet}; use std::{ fmt::Debug, panic::{catch_unwind, resume_unwind, AssertUnwindSafe}, @@ -28,7 +28,9 @@ pub use bevy_utils::label::DynEq; /// A shorthand for `Interned`. pub type InternedAppLabel = Interned; +#[derive(Debug, Error)] pub(crate) enum AppError { + #[error("duplicate plugin {plugin_name:?}")] DuplicatePlugin { plugin_name: String }, } diff --git a/crates/bevy_ecs/src/query/error.rs b/crates/bevy_ecs/src/query/error.rs index eb2769aecb5c2..1df1417910359 100644 --- a/crates/bevy_ecs/src/query/error.rs +++ b/crates/bevy_ecs/src/query/error.rs @@ -1,41 +1,28 @@ -use std::fmt; +use thiserror::Error; use crate::entity::Entity; /// An error that occurs when retrieving a specific [`Entity`]'s query result from [`Query`](crate::system::Query) or [`QueryState`](crate::query::QueryState). // TODO: return the type_name as part of this error -#[derive(Debug, PartialEq, Eq, Clone, Copy)] +#[derive(Debug, PartialEq, Eq, Clone, Copy, Error)] pub enum QueryEntityError { /// The given [`Entity`]'s components do not match the query. /// /// Either it does not have a requested component, or it has a component which the query filters out. + #[error("The components of entity {0:?} do not match the query")] QueryDoesNotMatch(Entity), /// The given [`Entity`] does not exist. + #[error("The entity {0:?} does not exist")] NoSuchEntity(Entity), /// The [`Entity`] was requested mutably more than once. /// /// See [`QueryState::get_many_mut`](crate::query::QueryState::get_many_mut) for an example. + #[error("The entity {0:?} was requested mutably more than once")] AliasedMutability(Entity), } -impl std::error::Error for QueryEntityError {} - -impl fmt::Display for QueryEntityError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match self { - QueryEntityError::QueryDoesNotMatch(_) => { - write!(f, "The given entity's components do not match the query.") - } - QueryEntityError::NoSuchEntity(_) => write!(f, "The requested entity does not exist."), - QueryEntityError::AliasedMutability(_) => { - write!(f, "The entity was requested mutably more than once.") - } - } - } -} - /// An error that occurs when retrieving a specific [`Entity`]'s component from a [`Query`](crate::system::Query). -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug, PartialEq, Eq, Error)] pub enum QueryComponentError { /// The [`Query`](crate::system::Query) does not have read access to the requested component. /// @@ -66,6 +53,7 @@ pub enum QueryComponentError { /// } /// # bevy_ecs::system::assert_is_system(get_missing_read_access_error); /// ``` + #[error("This query does not have read access to the requested component")] MissingReadAccess, /// The [`Query`](crate::system::Query) does not have write access to the requested component. /// @@ -93,59 +81,24 @@ pub enum QueryComponentError { /// } /// # bevy_ecs::system::assert_is_system(get_missing_write_access_error); /// ``` + #[error("This query does not have write access to the requested component")] MissingWriteAccess, /// The given [`Entity`] does not have the requested component. + #[error("The given entity does not have the requested component")] MissingComponent, /// The requested [`Entity`] does not exist. + #[error("The requested entity does not exist")] NoSuchEntity, } -impl std::error::Error for QueryComponentError {} - -impl std::fmt::Display for QueryComponentError { - fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - match self { - QueryComponentError::MissingReadAccess => { - write!( - f, - "This query does not have read access to the requested component." - ) - } - QueryComponentError::MissingWriteAccess => { - write!( - f, - "This query does not have write access to the requested component." - ) - } - QueryComponentError::MissingComponent => { - write!(f, "The given entity does not have the requested component.") - } - QueryComponentError::NoSuchEntity => { - write!(f, "The requested entity does not exist.") - } - } - } -} - /// An error that occurs when evaluating a [`Query`](crate::system::Query) or [`QueryState`](crate::query::QueryState) as a single expected result via /// [`get_single`](crate::system::Query::get_single) or [`get_single_mut`](crate::system::Query::get_single_mut). -#[derive(Debug)] +#[derive(Debug, Error)] pub enum QuerySingleError { /// No entity fits the query. + #[error("No entities fit the query {0}")] NoEntities(&'static str), /// Multiple entities fit the query. + #[error("Multiple entities fit the query {0}")] MultipleEntities(&'static str), } - -impl std::error::Error for QuerySingleError {} - -impl std::fmt::Display for QuerySingleError { - fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - match self { - QuerySingleError::NoEntities(query) => write!(f, "No entities fit the query {query}"), - QuerySingleError::MultipleEntities(query) => { - write!(f, "Multiple entities fit the query {query}!") - } - } - } -} diff --git a/crates/bevy_ecs/src/system/system_registry.rs b/crates/bevy_ecs/src/system/system_registry.rs index 3d6320d301593..40fdc9eeeffe8 100644 --- a/crates/bevy_ecs/src/system/system_registry.rs +++ b/crates/bevy_ecs/src/system/system_registry.rs @@ -3,6 +3,7 @@ use crate::system::{BoxedSystem, Command, IntoSystem}; use crate::world::World; use crate::{self as bevy_ecs}; use bevy_ecs_macros::Component; +use thiserror::Error; /// A small wrapper for [`BoxedSystem`] that also keeps track whether or not the system has been initialized. #[derive(Component)] @@ -197,15 +198,18 @@ impl Command for RunSystem { } /// An operation with stored systems failed. -#[derive(Debug)] +#[derive(Debug, Error)] pub enum RegisteredSystemError { /// A system was run by id, but no system with that id was found. /// /// Did you forget to register it? + #[error("System {0:?} was not registered")] SystemIdNotRegistered(SystemId), /// A system tried to run itself recursively. + #[error("System {0:?} tried to run itself recursively")] Recursive(SystemId), /// A system tried to remove itself. + #[error("System {0:?} tried to remove itself")] SelfRemove(SystemId), } diff --git a/crates/bevy_render/src/render_asset.rs b/crates/bevy_render/src/render_asset.rs index 5d759e0374283..e97874544a630 100644 --- a/crates/bevy_render/src/render_asset.rs +++ b/crates/bevy_render/src/render_asset.rs @@ -6,10 +6,12 @@ use bevy_ecs::{ schedule::SystemConfigs, system::{StaticSystemParam, SystemParam, SystemParamItem}, }; -use bevy_utils::{HashMap, HashSet}; +use bevy_utils::{thiserror::Error, HashMap, HashSet}; use std::marker::PhantomData; +#[derive(Debug, Error)] pub enum PrepareAssetError { + #[error("Failed to prepare asset")] RetryNextUpdate(E), } diff --git a/crates/bevy_render/src/render_resource/bind_group.rs b/crates/bevy_render/src/render_resource/bind_group.rs index 8ee876b9c5208..b2907c64fd661 100644 --- a/crates/bevy_render/src/render_resource/bind_group.rs +++ b/crates/bevy_render/src/render_resource/bind_group.rs @@ -7,6 +7,7 @@ use crate::{ texture::FallbackImage, }; pub use bevy_render_macros::AsBindGroup; +use bevy_utils::thiserror::Error; use encase::ShaderType; use std::ops::Deref; use wgpu::{BindGroupEntry, BindGroupLayoutDescriptor, BindGroupLayoutEntry, BindingResource}; @@ -325,8 +326,10 @@ pub trait AsBindGroup { } /// An error that occurs during [`AsBindGroup::as_bind_group`] calls. +#[derive(Debug, Error)] pub enum AsBindGroupError { /// The bind group could not be generated. Try again next frame. + #[error("The bind group could not be generated")] RetryNextUpdate, } diff --git a/crates/bevy_transform/Cargo.toml b/crates/bevy_transform/Cargo.toml index 7d413423e043c..26e1746085819 100644 --- a/crates/bevy_transform/Cargo.toml +++ b/crates/bevy_transform/Cargo.toml @@ -16,6 +16,7 @@ bevy_hierarchy = { path = "../bevy_hierarchy", version = "0.12.0-dev" } bevy_math = { path = "../bevy_math", version = "0.12.0-dev" } bevy_reflect = { path = "../bevy_reflect", version = "0.12.0-dev", features = ["bevy"] } serde = { version = "1", features = ["derive"], optional = true } +thiserror = "1.0" [dev-dependencies] bevy_tasks = { path = "../bevy_tasks", version = "0.12.0-dev" } diff --git a/crates/bevy_transform/src/helper.rs b/crates/bevy_transform/src/helper.rs index 67a92db7328b3..d1eca1d1df160 100644 --- a/crates/bevy_transform/src/helper.rs +++ b/crates/bevy_transform/src/helper.rs @@ -6,6 +6,7 @@ use bevy_ecs::{ system::{Query, SystemParam}, }; use bevy_hierarchy::{HierarchyQueryExt, Parent}; +use thiserror::Error; use crate::components::{GlobalTransform, Transform}; @@ -63,14 +64,17 @@ fn map_error(err: QueryEntityError, ancestor: bool) -> ComputeGlobalTransformErr } /// Error returned by [`TransformHelper::compute_global_transform`]. -#[derive(Debug)] +#[derive(Debug, Error)] pub enum ComputeGlobalTransformError { /// The entity or one of its ancestors is missing the [`Transform`] component. + #[error("The entity {0:?} or one of its ancestors is missing the `Transform` component")] MissingTransform(Entity), /// The entity does not exist. + #[error("The entity {0:?} does not exist")] NoSuchEntity(Entity), /// An ancestor is missing. /// This probably means that your hierarchy has been improperly maintained. + #[error("The ancestor {0:?} is missing")] MalformedHierarchy(Entity), } diff --git a/crates/bevy_ui/src/layout/mod.rs b/crates/bevy_ui/src/layout/mod.rs index e94dae3fa3482..d404431b1bf26 100644 --- a/crates/bevy_ui/src/layout/mod.rs +++ b/crates/bevy_ui/src/layout/mod.rs @@ -19,6 +19,7 @@ use bevy_utils::{default, HashMap}; use bevy_window::{PrimaryWindow, Window, WindowResolution, WindowScaleFactorChanged}; use std::fmt; use taffy::Taffy; +use thiserror::Error; pub struct LayoutContext { pub scale_factor: f64, @@ -228,10 +229,12 @@ with UI components as a child of an entity without UI components, results may be } } -#[derive(Debug)] +#[derive(Debug, Error)] pub enum LayoutError { + #[error("Invalid hierarchy")] InvalidHierarchy, - TaffyError(taffy::error::TaffyError), + #[error("Taffy error: {0}")] + TaffyError(#[from] taffy::error::TaffyError), } /// Updates the UI's layout tree, computes the new layout geometry and then updates the sizes and transforms of all the UI nodes. diff --git a/examples/games/contributors.rs b/examples/games/contributors.rs index 1ca223c35d0b7..4494a241d83b3 100644 --- a/examples/games/contributors.rs +++ b/examples/games/contributors.rs @@ -1,6 +1,9 @@ //! This example displays each contributor to the bevy source code as a bouncing bevy-ball. -use bevy::{prelude::*, utils::HashSet}; +use bevy::{ + prelude::*, + utils::{thiserror, HashSet}, +}; use rand::{prelude::SliceRandom, Rng}; use std::{ env::VarError, @@ -309,9 +312,13 @@ fn move_system(time: Res