Improve error message for missing events (#18683)
# Objective Improve the parameter validation error message for `Event(Reader|Writer|Mutator)`. System parameters defined using `#[derive(SystemParam)]`, including the parameters for events, currently propagate the validation errors from their subparameters. The error includes the type of the failing parameter, so the resulting error includes the type of the failing subparameter instead of the derived parameter. In particular, `EventReader<T>` will report an error from a `Res<Events<T>>`, even though the user has no parameter of that type! This is a follow-up to #18593. ## Solution Have `#[derive]`d system parameters map errors during propagation so that they report the outer parameter type. To continue to provide context, add a field to `SystemParamValidationError` that identifies the subparameter by name, and is empty for non-`#[derive]`d parameters. Allow them to override the failure message for individual parameters. Use this to convert "Resource does not exist" to "Event not initialized" for `Event(Reader|Writer|Mutator)`. ## Showcase The validation error for a `EventReader<SomeEvent>` parameter when `add_event` has not been called changes from: Before: ``` Parameter `Res<Events<SomeEvent>>` failed validation: Resource does not exist ``` After ``` Parameter `EventReader<SomeEvent>::events` failed validation: Event not initialized ```
This commit is contained in:
parent
4b5df0f1ab
commit
ed28b5ccf7
@ -229,19 +229,39 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
|
||||
let path = bevy_ecs_path();
|
||||
|
||||
let mut field_locals = Vec::new();
|
||||
let mut field_names = Vec::new();
|
||||
let mut fields = Vec::new();
|
||||
let mut field_types = Vec::new();
|
||||
let mut field_messages = Vec::new();
|
||||
for (i, field) in field_definitions.iter().enumerate() {
|
||||
field_locals.push(format_ident!("f{i}"));
|
||||
let i = Index::from(i);
|
||||
fields.push(
|
||||
field
|
||||
.ident
|
||||
.as_ref()
|
||||
.map(|f| quote! { #f })
|
||||
.unwrap_or_else(|| quote! { #i }),
|
||||
);
|
||||
let field_value = field
|
||||
.ident
|
||||
.as_ref()
|
||||
.map(|f| quote! { #f })
|
||||
.unwrap_or_else(|| quote! { #i });
|
||||
field_names.push(format!("::{}", field_value));
|
||||
fields.push(field_value);
|
||||
field_types.push(&field.ty);
|
||||
let mut field_message = None;
|
||||
for meta in field
|
||||
.attrs
|
||||
.iter()
|
||||
.filter(|a| a.path().is_ident("system_param"))
|
||||
{
|
||||
if let Err(e) = meta.parse_nested_meta(|nested| {
|
||||
if nested.path.is_ident("validation_message") {
|
||||
field_message = Some(nested.value()?.parse()?);
|
||||
Ok(())
|
||||
} else {
|
||||
Err(nested.error("Unsupported attribute"))
|
||||
}
|
||||
}) {
|
||||
return e.into_compile_error().into();
|
||||
}
|
||||
}
|
||||
field_messages.push(field_message.unwrap_or_else(|| quote! { err.message }));
|
||||
}
|
||||
|
||||
let generics = ast.generics;
|
||||
@ -427,10 +447,15 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
|
||||
#[inline]
|
||||
unsafe fn validate_param<'w, 's>(
|
||||
state: &'s Self::State,
|
||||
system_meta: &#path::system::SystemMeta,
|
||||
world: #path::world::unsafe_world_cell::UnsafeWorldCell<'w>,
|
||||
_system_meta: &#path::system::SystemMeta,
|
||||
_world: #path::world::unsafe_world_cell::UnsafeWorldCell<'w>,
|
||||
) -> Result<(), #path::system::SystemParamValidationError> {
|
||||
<(#(#tuple_types,)*) as #path::system::SystemParam>::validate_param(&state.state, system_meta, world)
|
||||
let #state_struct_name { state: (#(#tuple_patterns,)*) } = state;
|
||||
#(
|
||||
<#field_types as #path::system::SystemParam>::validate_param(#field_locals, _system_meta, _world)
|
||||
.map_err(|err| #path::system::SystemParamValidationError::new::<Self>(err.skipped, #field_messages, #field_names))?;
|
||||
)*
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[inline]
|
||||
|
@ -44,6 +44,7 @@ use bevy_ecs::{
|
||||
#[derive(SystemParam, Debug)]
|
||||
pub struct EventMutator<'w, 's, E: Event> {
|
||||
pub(super) reader: Local<'s, EventCursor<E>>,
|
||||
#[system_param(validation_message = "Event not initialized")]
|
||||
events: ResMut<'w, Events<E>>,
|
||||
}
|
||||
|
||||
|
@ -16,6 +16,7 @@ use bevy_ecs::{
|
||||
#[derive(SystemParam, Debug)]
|
||||
pub struct EventReader<'w, 's, E: Event> {
|
||||
pub(super) reader: Local<'s, EventCursor<E>>,
|
||||
#[system_param(validation_message = "Event not initialized")]
|
||||
events: Res<'w, Events<E>>,
|
||||
}
|
||||
|
||||
|
@ -60,6 +60,7 @@ use bevy_ecs::{
|
||||
/// [`Observer`]: crate::observer::Observer
|
||||
#[derive(SystemParam)]
|
||||
pub struct EventWriter<'w, E: Event> {
|
||||
#[system_param(validation_message = "Event not initialized")]
|
||||
events: ResMut<'w, Events<E>>,
|
||||
}
|
||||
|
||||
|
@ -131,6 +131,29 @@ use variadics_please::{all_tuples, all_tuples_enumerated};
|
||||
/// This will most commonly occur when working with `SystemParam`s generically, as the requirement
|
||||
/// has not been proven to the compiler.
|
||||
///
|
||||
/// ## Custom Validation Messages
|
||||
///
|
||||
/// When using the derive macro, any [`SystemParamValidationError`]s will be propagated from the sub-parameters.
|
||||
/// If you want to override the error message, add a `#[system_param(validation_message = "New message")]` attribute to the parameter.
|
||||
///
|
||||
/// ```
|
||||
/// # use bevy_ecs::prelude::*;
|
||||
/// # #[derive(Resource)]
|
||||
/// # struct SomeResource;
|
||||
/// # use bevy_ecs::system::SystemParam;
|
||||
/// #
|
||||
/// #[derive(SystemParam)]
|
||||
/// struct MyParam<'w> {
|
||||
/// #[system_param(validation_message = "Custom Message")]
|
||||
/// foo: Res<'w, SomeResource>,
|
||||
/// }
|
||||
///
|
||||
/// let mut world = World::new();
|
||||
/// let err = world.run_system_cached(|param: MyParam| {}).unwrap_err();
|
||||
/// let expected = "Parameter `MyParam::foo` failed validation: Custom Message";
|
||||
/// assert!(err.to_string().ends_with(expected));
|
||||
/// ```
|
||||
///
|
||||
/// ## Builders
|
||||
///
|
||||
/// If you want to use a [`SystemParamBuilder`](crate::system::SystemParamBuilder) with a derived [`SystemParam`] implementation,
|
||||
@ -2662,26 +2685,39 @@ pub struct SystemParamValidationError {
|
||||
/// A string identifying the invalid parameter.
|
||||
/// This is usually the type name of the parameter.
|
||||
pub param: Cow<'static, str>,
|
||||
|
||||
/// A string identifying the field within a parameter using `#[derive(SystemParam)]`.
|
||||
/// This will be an empty string for other parameters.
|
||||
///
|
||||
/// This will be printed after `param` in the `Display` impl, and should include a `::` prefix if non-empty.
|
||||
pub field: Cow<'static, str>,
|
||||
}
|
||||
|
||||
impl SystemParamValidationError {
|
||||
/// Constructs a `SystemParamValidationError` that skips the system.
|
||||
/// The parameter name is initialized to the type name of `T`, so a `SystemParam` should usually pass `Self`.
|
||||
pub fn skipped<T>(message: impl Into<Cow<'static, str>>) -> Self {
|
||||
Self {
|
||||
skipped: true,
|
||||
message: message.into(),
|
||||
param: Cow::Borrowed(core::any::type_name::<T>()),
|
||||
}
|
||||
Self::new::<T>(true, message, Cow::Borrowed(""))
|
||||
}
|
||||
|
||||
/// Constructs a `SystemParamValidationError` for an invalid parameter that should be treated as an error.
|
||||
/// The parameter name is initialized to the type name of `T`, so a `SystemParam` should usually pass `Self`.
|
||||
pub fn invalid<T>(message: impl Into<Cow<'static, str>>) -> Self {
|
||||
Self::new::<T>(false, message, Cow::Borrowed(""))
|
||||
}
|
||||
|
||||
/// Constructs a `SystemParamValidationError` for an invalid parameter.
|
||||
/// The parameter name is initialized to the type name of `T`, so a `SystemParam` should usually pass `Self`.
|
||||
pub fn new<T>(
|
||||
skipped: bool,
|
||||
message: impl Into<Cow<'static, str>>,
|
||||
field: impl Into<Cow<'static, str>>,
|
||||
) -> Self {
|
||||
Self {
|
||||
skipped: false,
|
||||
skipped,
|
||||
message: message.into(),
|
||||
param: Cow::Borrowed(core::any::type_name::<T>()),
|
||||
field: field.into(),
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -2690,8 +2726,9 @@ impl Display for SystemParamValidationError {
|
||||
fn fmt(&self, fmt: &mut core::fmt::Formatter<'_>) -> Result<(), core::fmt::Error> {
|
||||
write!(
|
||||
fmt,
|
||||
"Parameter `{}` failed validation: {}",
|
||||
"Parameter `{}{}` failed validation: {}",
|
||||
ShortName(&self.param),
|
||||
self.field,
|
||||
self.message
|
||||
)
|
||||
}
|
||||
@ -2944,4 +2981,20 @@ mod tests {
|
||||
|
||||
fn res_system(_: Res<MissingResource>) {}
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[should_panic = "Encountered an error in system `bevy_ecs::system::system_param::tests::missing_event_error::event_system`: Parameter `EventReader<MissingEvent>::events` failed validation: Event not initialized"]
|
||||
fn missing_event_error() {
|
||||
use crate::prelude::{Event, EventReader};
|
||||
|
||||
#[derive(Event)]
|
||||
pub struct MissingEvent;
|
||||
|
||||
let mut schedule = crate::schedule::Schedule::default();
|
||||
schedule.add_systems(event_system);
|
||||
let mut world = World::new();
|
||||
schedule.run(&mut world);
|
||||
|
||||
fn event_system(_: EventReader<MissingEvent>) {}
|
||||
}
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user