When & WhenMut: skip system if Res is missing.#18927
When & WhenMut: skip system if Res is missing.#18927mrchantey wants to merge 5 commits intobevyengine:mainfrom
When & WhenMut: skip system if Res is missing.#18927Conversation
|
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 |
You're not alone! I also think we'll eventually want I'd vote for first getting some version of this or #18765 merged first, and then making a follow-up PR to debate changing the behavior of |
|
closing in favor of #18765 which is a more maintainable implementation. |
Objective
ParamWarnPolicyhas been removed in0.16, deprecatingnever_param_warn(). LikeSingleandPopulated, we need aRestype that will skip the system if the resource is not present.Solution
Implement two new system params:
When&WhenMut, identical toRes&ResMutbut with avalidate_paramthat will skip instead of panic.These types can also be extended to 'wrap' other types beyond
Res&ResMut.Testing
Tested both
should skipandshould not skipcases, verify by runningcargo test -p bevy_ecs system::whenShowcase
Res&ResMutnow have two alternativesWhenandWhenMutfor when a system should skip instead of panic.