forestia/bevy
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Include SystemParamValidationError in RunSystemError and RegisteredSystemError (#18666)

Browse Source 
# Objective

Provide more useful errors when `World::run_system` and related methods
fail parameter validation.

Let callers determine whether the validation failure would have skipped
or failed the system.

Follow-up to #18541.

## Solution

Add a `SystemParamValidationError` value to the
`RunSystemError::InvalidParams` and
`RegisteredSystemError::InvalidParams` variants. That includes the
complete context of the parameter validation error, including the
`skipped` flag.
This commit is contained in:
Chris Russell 2025-04-01 15:27:08 -04:00 committed by GitHub
parent 61f3b3e8f5
commit 9daf4e7c8b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 48 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
crates/bevy_ecs/src/system/system.rs
View File

@ -368,37 +368,35 @@ impl RunSystemOnce for &mut World {
{ {
let mut system: T::System = IntoSystem::into_system(system); let mut system: T::System = IntoSystem::into_system(system);
system.initialize(self); system.initialize(self);
match system.validate_param(self) { system
Ok(()) => Ok(system.run(input, self)), .validate_param(self)
// TODO: should we expse the fact that the system was skipped to the user? .map_err(|err| RunSystemError::InvalidParams {
// Should we somehow unify this better with system error handling? system: system.name(),
Err(_) => Err(RunSystemError::InvalidParams(system.name())), err,
} })?;
Ok(system.run(input, self))
} }
} }
/// Running system failed. /// Running system failed.
#[derive(Error)] #[derive(Error, Debug)]
pub enum RunSystemError { pub enum RunSystemError {
/// System could not be run due to parameters that failed validation. /// System could not be run due to parameters that failed validation.
/// /// This should not be considered an error if [`field@SystemParamValidationError::skipped`] is `true`.
/// This can occur because the data required by the system was not present in the world. #[error("System {system} did not run due to failed parameter validation: {err}")]
#[error("The data required by the system {0:?} was not found in the world and the system did not run due to failed parameter validation.")] InvalidParams {
InvalidParams(Cow<'static, str>), /// The identifier of the system that was run.
} system: Cow<'static, str>,
/// The returned parameter validation error.
impl Debug for RunSystemError { err: SystemParamValidationError,
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { },
match self {
Self::InvalidParams(arg0) => f.debug_tuple("InvalidParams").field(arg0).finish(),
}
}
} }
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
use crate::prelude::*; use crate::prelude::*;
use alloc::string::ToString;
#[test] #[test]
fn run_system_once() { fn run_system_once() {
@ -470,6 +468,8 @@ mod tests {
// This fails because `T` has not been added to the world yet. // This fails because `T` has not been added to the world yet.
let result = world.run_system_once(system); let result = world.run_system_once(system);
assert!(matches!(result, Err(RunSystemError::InvalidParams(_)))); assert!(matches!(result, Err(RunSystemError::InvalidParams { .. })));
let expected = "System bevy_ecs::system::system::tests::run_system_once_invalid_params::system did not run due to failed parameter validation: Parameter `Res<T>` failed validation: Resource does not exist";
assert_eq!(expected, result.unwrap_err().to_string());
} }
} }

46
crates/bevy_ecs/src/system/system_registry.rs
View File

@ -3,7 +3,7 @@ use crate::reflect::ReflectComponent;
use crate::{ use crate::{
change_detection::Mut, change_detection::Mut,
entity::Entity, entity::Entity,
system::{input::SystemInput, BoxedSystem, IntoSystem}, system::{input::SystemInput, BoxedSystem, IntoSystem, SystemParamValidationError},
world::World, world::World,
}; };
use alloc::boxed::Box; use alloc::boxed::Box;
@ -351,17 +351,16 @@ impl World {
initialized = true; initialized = true;
} }
let result = if system.validate_param(self).is_ok() { let result = system
// Wait to run the commands until the system is available again. .validate_param(self)
// This is needed so the systems can recursively run themselves. .map_err(|err| RegisteredSystemError::InvalidParams { system: id, err })
let ret = system.run_without_applying_deferred(input, self); .map(|()| {
system.queue_deferred(self.into()); // Wait to run the commands until the system is available again.
Ok(ret) // This is needed so the systems can recursively run themselves.
} else { let ret = system.run_without_applying_deferred(input, self);
// TODO: do we want to differentiate between failed validation and skipped systems? system.queue_deferred(self.into());
// Do we want to better unify this with system error handling? ret
Err(RegisteredSystemError::InvalidParams(id)) });
};
// Return ownership of system trait object (if entity still exists) // Return ownership of system trait object (if entity still exists)
if let Ok(mut entity) = self.get_entity_mut(id.entity) { if let Ok(mut entity) = self.get_entity_mut(id.entity) {
@ -494,10 +493,14 @@ pub enum RegisteredSystemError<I: SystemInput = (), O = ()> {
#[error("System {0:?} tried to remove itself")] #[error("System {0:?} tried to remove itself")]
SelfRemove(SystemId<I, O>), SelfRemove(SystemId<I, O>),
/// System could not be run due to parameters that failed validation. /// System could not be run due to parameters that failed validation.
/// /// This should not be considered an error if [`field@SystemParamValidationError::skipped`] is `true`.
/// This can occur because the data required by the system was not present in the world. #[error("System {system:?} did not run due to failed parameter validation: {err}")]
#[error("The data required by the system {0:?} was not found in the world and the system did not run due to failed parameter validation.")] InvalidParams {
InvalidParams(SystemId<I, O>), /// The identifier of the system that was run.
system: SystemId<I, O>,
/// The returned parameter validation error.
err: SystemParamValidationError,
},
} }
impl<I: SystemInput, O> core::fmt::Debug for RegisteredSystemError<I, O> { impl<I: SystemInput, O> core::fmt::Debug for RegisteredSystemError<I, O> {
@ -509,7 +512,11 @@ impl<I: SystemInput, O> core::fmt::Debug for RegisteredSystemError<I, O> {
Self::SystemNotCached => write!(f, "SystemNotCached"), Self::SystemNotCached => write!(f, "SystemNotCached"),
Self::Recursive(arg0) => f.debug_tuple("Recursive").field(arg0).finish(), Self::Recursive(arg0) => f.debug_tuple("Recursive").field(arg0).finish(),
Self::SelfRemove(arg0) => f.debug_tuple("SelfRemove").field(arg0).finish(), Self::SelfRemove(arg0) => f.debug_tuple("SelfRemove").field(arg0).finish(),
Self::InvalidParams(arg0) => f.debug_tuple("InvalidParams").field(arg0).finish(), Self::InvalidParams { system, err } => f
.debug_struct("InvalidParams")
.field("system", system)
.field("err", err)
.finish(),
} }
} }
} }
@ -858,6 +865,7 @@ mod tests {
#[test] #[test]
fn run_system_invalid_params() { fn run_system_invalid_params() {
use crate::system::RegisteredSystemError; use crate::system::RegisteredSystemError;
use alloc::{format, string::ToString};
struct T; struct T;
impl Resource for T {} impl Resource for T {}
@ -870,8 +878,10 @@ mod tests {
assert!(matches!( assert!(matches!(
result, result,
Err(RegisteredSystemError::InvalidParams(_)) Err(RegisteredSystemError::InvalidParams { .. })
)); ));
let expected = format!("System {id:?} did not run due to failed parameter validation: Parameter `Res<T>` failed validation: Resource does not exist");
assert_eq!(expected, result.unwrap_err().to_string());
} }
#[test] #[test]