Skip to content

Improve error message for missing events #18683

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 2, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 35 additions & 10 deletions crates/bevy_ecs/macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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]
Expand Down
1 change: 1 addition & 0 deletions crates/bevy_ecs/src/event/mutator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>>,
}

Expand Down
1 change: 1 addition & 0 deletions crates/bevy_ecs/src/event/reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>>,
}

Expand Down
1 change: 1 addition & 0 deletions crates/bevy_ecs/src/event/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>>,
}

Expand Down
67 changes: 60 additions & 7 deletions crates/bevy_ecs/src/system/system_param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -2689,26 +2712,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(),
}
}
}
Expand All @@ -2717,8 +2753,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
)
}
Expand Down Expand Up @@ -2971,4 +3008,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>) {}
}
}