-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
fix: improve semantic clarity for run condition combinators #22690
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?
fix: improve semantic clarity for run condition combinators #22690
Conversation
|
I don't really agree that this is contentious -- even if the exact solution used in this PR isn't the desired path forward, fixing the change detection footgun seems necessary to me. The current behavior is way too subtle and counterintuitive to be the default. |
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.
A short migration guide is required, but I really like the proposed migration strategy, and the names are solid. Once that's done, you'll have my approval. I agree in principal, so I'll mark this as X-Blessed.
Objective
We include several run condition combinators, such as
and,or, etc., which short-circuit depending on the output of the first condition in the combinator.This is incredibly error-prone due to the subtle way that short-circuiting interacts with change detection -- rather than reacting to changes frame-by-frame, the second condition in short-circuiting combinator will react to the last time that the first condition did not short circuit. This can easily lead to confusing bugs if the user does not expect this, and I suspect that most users will not expect this.
For this reason, when combining multiple run conditions added via
.run_if(), all run conditions are intentionally eagerly evaluated.Solution
Add new run condition combinators
and_then,and_eager,or_else,or_eager, etc., for clarity, and deprecate the previous methods, pointing users to the new ones.After the previous combinators have been removed for a few release cycles, we should consider renaming combinators such as
and_eagerto simplyand.Migration Guide
Bevy supports run condition combinators (
and,or,nan,nor), which have historically short-circuited. While familiar, short-circuiting interacts with Bevy’s change detection in a subtle way: when the left-hand condition short-circuits, the right-hand condition is not evaluated and therefore does not observe changes on that frame. Instead, it reacts based on the last frame it ran, which can lead to confusing and non-local bugs.By contrast, Bevy's scheduler combines multiple .run_if(...) conditions using eager evaluation, which avoids this known pitfall.
To make intent explicit and reduce footguns, short-circuiting combinators have been renamed and eagerly-evaluated variants have been added.
Examples
Most users should use eager evaluation, which ensures all conditions participate in change detection every frame:
If you intentionally rely on short-circuiting for correctness, use the explicit short-circuiting variants:
xorandxnorare unchanged, as they cannot short-circuit by nature.Future naming note
The
_eagersuffix exists to ease migration without changing the behavior of existing code that relied on short-circuiting. After the deprecated combinators have been removed for a few release cycles, we expect to revisit naming and likely remove the _eager suffix, keepingand_then/or_elseas the explicit short-circuiting forms.