-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Set panic as default fallible system param behavior #16638
base: main
Are you sure you want to change the base?
Conversation
Naive approach comes with some issues, I'm not sure when I'll have time to truly sit down and work on this, so feel free to take over as needed |
Can you say a bit more about the issues you encountered? |
The main issue is stuff that was fixed in the I've had time to think about it since this PR. |
I'm okay with that as the behavior for now :) |
// Combined systems get skipped together. | ||
(|mut commands: Commands| { | ||
commands.insert_resource(R1); | ||
}) | ||
.pipe(|_: In<()>, _: Res<R1>| {}), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As agreed, this is no longer supported in this scope
Objective
Fixes: #16578
Solution
This is a patch fix, proper fix requires a breaking change.
Added
Panic
enum variant and using is as the system meta default.Warn once behavior can be enabled same way disabling panic (originally disabling wans) is.
To fix an issue with the current architecture, where all combinator system params get checked together,
combinator systems only check params of the first system.
This will result in old, panicking behavior on subsequent systems and will be fixed in 0.16.
Testing
Ran unit tests and
fallible_params
example.