Return the failed system parameter(s) in SystemParamValidationError #18515
Labels
A-ECS
Entities, components, systems, and events
C-Usability
A targeted quality-of-life change that makes Bevy easier to use
D-Modest
A "normal" level of difficulty; suitable for simple features or challenging fixes
S-Ready-For-Implementation
This issue is ready for an implementation PR. Go for it!
X-Uncontroversial
This work is generally agreed upon
Comments
alice-i-cecile
added
A-ECS
5 tasks
This was referenced Mar 26, 2025
github-merge-queue bot
pushed a commit
that referenced
this issue
Mar 30, 2025
# Objective Fixes #18515 After the recent changes to system param validation, the panic message for a missing resource is currently: ``` Encountered an error in system `missing_resource_error::res_system`: SystemParamValidationError { skipped: false } ``` Add the parameter type name and a descriptive message, improving the panic message to: ``` Encountered an error in system `missing_resource_error::res_system`: SystemParamValidationError { skipped: false, message: "Resource does not exist", param: "bevy_ecs::change_detection::Res<missing_resource_error::MissingResource>" } ``` ## Solution Add fields to `SystemParamValidationError` for error context. Include the `type_name` of the param and a message. Store them as `Cow<'static, str>` and only format them into a friendly string in the `Display` impl. This lets us create errors using a `&'static str` with no allocation or formatting, while still supporting runtime `String` values if necessary. Add a unit test that verifies the panic message. ## Future Work If we change the default error handling to use `Display` instead of `Debug`, and to use `ShortName` for the system name, the panic message could be further improved to: ``` Encountered an error in system `res_system`: Parameter `Res<MissingResource>` failed validation: Resource does not exist ``` However, `BevyError` currently includes the backtrace in `Debug` but not `Display`, and I didn't want to try to change that in this PR.
mockersf
pushed a commit
that referenced
this issue
Mar 30, 2025
# Objective Fixes #18515 After the recent changes to system param validation, the panic message for a missing resource is currently: ``` Encountered an error in system `missing_resource_error::res_system`: SystemParamValidationError { skipped: false } ``` Add the parameter type name and a descriptive message, improving the panic message to: ``` Encountered an error in system `missing_resource_error::res_system`: SystemParamValidationError { skipped: false, message: "Resource does not exist", param: "bevy_ecs::change_detection::Res<missing_resource_error::MissingResource>" } ``` ## Solution Add fields to `SystemParamValidationError` for error context. Include the `type_name` of the param and a message. Store them as `Cow<'static, str>` and only format them into a friendly string in the `Display` impl. This lets us create errors using a `&'static str` with no allocation or formatting, while still supporting runtime `String` values if necessary. Add a unit test that verifies the panic message. ## Future Work If we change the default error handling to use `Display` instead of `Debug`, and to use `ShortName` for the system name, the panic message could be further improved to: ``` Encountered an error in system `res_system`: Parameter `Res<MissingResource>` failed validation: Resource does not exist ``` However, `BevyError` currently includes the backtrace in `Debug` but not `Display`, and I didn't want to try to change that in this PR.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Originally posted by @chescock in #18504 (comment)
The text was updated successfully, but these errors were encountered: