-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
When
& WhenMut
: skip system if Res
is missing.
#18927
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
base: main
Are you sure you want to change the base?
Conversation
It looks like your PR has been selected for a highlight in the next release blog post, but you didn't provide a release note. Please review the instructions for writing release notes, then expand or revise the content in the release notes directory to showcase your changes. |
I like the idea, but
|
Good point, I think the name was chosen so that the same convention can be implemented for more types beyond |
The cools thing about making So I think Alternatively, we could do the inverse if skipping systems is a more commonly-desired scenario. Maybe we make |
@alice-i-cecile May need to make a call on I'm really not sure what the right usage of
Sort of don't want to break |
Strongly agree here.
Yes, worth having.
All system params please! |
Note that there is also an existing PR for a generic |
Fair but I think it's going to benefit readability if skipping (or not skipping) is explicit. And I do think there's benefit to having a fail fast variant for these sorts of parameters. If I expect a system to run, it might not be helpful for it to silently skip (of course, you can panic manually with This could also be confusing for new users who see that So imo adding a generic If it's even desired behavior haha I could be alone in this thought process. |
I agree, this feels like one thats worth thinking deeply about and getting right. In terms of design it sounds like there are two paths forward: Option A: Single and friends Panic
fn system(
res: When<Res<Foo>>,
single: When<Single<Bar>>,
) Option B: Only Resources Panic
In this case we'd need to make some implementation decisions: // b.1 - No wrapper for Res & ResMut
fn system(
res: When<Foo>,
res_mut: WhenMut<Foo>,
reader: WhenMut<EventReader<Bar>>,
)
// b.2 - Blanket Wrapper
fn system(
res: When<Res<Foo>>,
res_mut: When<ResMut<Foo>>,
reader: When<EventReader<Bar>>,
)
// b.3 Required Single panics
fn system(
query: Required<Single<Foo>>,
) In my opinion Option B better reflects the nature of Components vs Resources. Looking at my codebase I'd like most As for the implementation I think we should close this pr in favor of the blanket wrapper in #18765, and |
Personally, I'm in favour of I also like that this opens the door for a form of per-parameter run conditions as a general API, expanding on |
Objective
ParamWarnPolicy
has been removed in0.16
, deprecatingnever_param_warn()
. LikeSingle
andPopulated
, we need aRes
type that will skip the system if the resource is not present.Solution
Implement two new system params:
When
&WhenMut
, identical toRes
&ResMut
but with avalidate_param
that will skip instead of panic.These types can also be extended to 'wrap' other types beyond
Res
&ResMut
.Testing
Tested both
should skip
andshould not skip
cases, verify by runningcargo test -p bevy_ecs system::when
Showcase
Res
&ResMut
now have two alternativesWhen
andWhenMut
for when a system should skip instead of panic.