Skip to content

Commit

Permalink
Derive Error for more error types (#10240)
Browse files Browse the repository at this point in the history
# 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
  • Loading branch information
Pascal Hertleif authored Oct 28, 2023
1 parent 9d088dd commit 0c2c52a
Show file tree
Hide file tree
Showing 9 changed files with 50 additions and 72 deletions.
4 changes: 3 additions & 1 deletion crates/bevy_app/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -28,7 +28,9 @@ pub use bevy_utils::label::DynEq;
/// A shorthand for `Interned<dyn AppLabel>`.
pub type InternedAppLabel = Interned<dyn AppLabel>;

#[derive(Debug, Error)]
pub(crate) enum AppError {
#[error("duplicate plugin {plugin_name:?}")]
DuplicatePlugin { plugin_name: String },
}

Expand Down
73 changes: 13 additions & 60 deletions crates/bevy_ecs/src/query/error.rs
Original file line number Diff line number Diff line change
@@ -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.
///
Expand Down Expand Up @@ -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.
///
Expand Down Expand Up @@ -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}!")
}
}
}
}
6 changes: 5 additions & 1 deletion crates/bevy_ecs/src/system/system_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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),
}

Expand Down
4 changes: 3 additions & 1 deletion crates/bevy_render/src/render_asset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<E: Send + Sync + 'static> {
#[error("Failed to prepare asset")]
RetryNextUpdate(E),
}

Expand Down
3 changes: 3 additions & 0 deletions crates/bevy_render/src/render_resource/bind_group.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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,
}

Expand Down
1 change: 1 addition & 0 deletions crates/bevy_transform/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
6 changes: 5 additions & 1 deletion crates/bevy_transform/src/helper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use bevy_ecs::{
system::{Query, SystemParam},
};
use bevy_hierarchy::{HierarchyQueryExt, Parent};
use thiserror::Error;

use crate::components::{GlobalTransform, Transform};

Expand Down Expand Up @@ -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),
}

Expand Down
7 changes: 5 additions & 2 deletions crates/bevy_ui/src/layout/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down
18 changes: 12 additions & 6 deletions examples/games/contributors.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -309,9 +312,13 @@ fn move_system(time: Res<Time>, mut query: Query<(&Velocity, &mut Transform)>) {
}
}

#[derive(Debug, thiserror::Error)]
enum LoadContributorsError {
IO(io::Error),
Var(VarError),
#[error("An IO error occurred while reading the git log.")]
Io(#[from] io::Error),
#[error("The CARGO_MANIFEST_DIR environment variable was not set.")]
Var(#[from] VarError),
#[error("The git process did not return a stdout handle.")]
Stdout,
}

Expand All @@ -321,14 +328,13 @@ enum LoadContributorsError {
/// This function only works if `git` is installed and
/// the program is run through `cargo`.
fn contributors() -> Result<Contributors, LoadContributorsError> {
let manifest_dir = std::env::var("CARGO_MANIFEST_DIR").map_err(LoadContributorsError::Var)?;
let manifest_dir = std::env::var("CARGO_MANIFEST_DIR")?;

let mut cmd = std::process::Command::new("git")
.args(["--no-pager", "log", "--pretty=format:%an"])
.current_dir(manifest_dir)
.stdout(Stdio::piped())
.spawn()
.map_err(LoadContributorsError::IO)?;
.spawn()?;

let stdout = cmd.stdout.take().ok_or(LoadContributorsError::Stdout)?;

Expand Down

0 comments on commit 0c2c52a

Please sign in to comment.