-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Create a When
system param wrapper for skipping systems that fail validation
#18765
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
Conversation
I really don't think we should do this, and would prefer to spin this out into a separate PR. These are useful, terse helpers for a common pattern and I would really like to stop changing their semantics in breaking ways. |
Sure, I'll remove that from this PR. I do still think it's worth doing, though! The panicking version of But I will feel really bad for all the folks trying to use |
... the other way to get consistency is to make |
Mhmm :) This is too late in the 0.16 cycle for this to land, so I'm reluctant to change the behavior yet again. Thanks for splitting it out though; this is a much simpler and uncontroversial helper on its own. |
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.
Love it, just a suggestion for documentation. Not blocking at all.
Co-authored-by: Zachary Harrold <zac@harrold.com.au>
Split comment onto multiple lines to avoid horizontal scrolling in docs.
Objective
Create a
When
system param wrapper for skipping systems that fail validation.Currently, the
Single
andPopulated
parameters cause systems to skip when they fail validation, while theRes
family causes systems to error. Generalize this so that any fallible parameter can be used either to skip a system or to raise an error. A parameter used directly will always raise an error, and a parameter wrapped inWhen<P>
will always cause the system to be silently skipped.Note that this changes the behavior forSingle
andPopulated
. The current behavior will be available usingWhen<Single>
andWhen<Populated>
.Fixes #18516
Solution
Create a
When
system param wrapper that wraps an inner parameter and converts all validation errors toskipped
.Change the behavior ofSingle
andPopulated
to fail by default.Replace in-engine use ofSingle
withWhen<Single>
. I updated thefallible_systems
example, but not all of the others. The other examples I looked at appeared to always have one matching entity, and it seemed more clear to use the simpler type in those cases.