Early return on run condition evaluation.

[Pieresqi]

What problem does this solve or what need does it fill?

If you have nested run conditions then "inner" or "lower level" will be evaluated even if "outermost" or "higher level" results to false.

This leads to wasting of resources and useless evaluation of run condition even if they don't need to be.

(I am not sure how to describe it but code snippet lower demonstrates actual vs desired behaviour)

What solution would you like?

If run condition in "higher" hierarchy resolves to false, then return early.

What alternative(s) have you considered?

Intended behaviour ?

Additional context

Let's have this bevy app:

use bevy::prelude::*;

fn main() {
    App::new()
        .add_plugins(MinimalPlugins)
        .add_systems(
            Update,
            (
                (
                    system_a.run_if(move || {
                        println!("inner condition");
                        true
                    }),
                    system_b,
                )
                    .run_if(move || {
                        println!("outer condition");
                        false
                    }),
                end,
            )
                .chain(),
        )
        .run();
}

fn system_a() {
    println!("system_a system");
}

fn system_b() {
    println!("system_b system");
}

fn end() {
    println!("end system")
}

This will print:

outer condition
inner condition
end system

Ideally this should only print:

outer condition
end system

[hymm]

This is expected. There's some weird interactions with change detection if a run condition uses it and you support early out. You can use and_then and or_else on a run condition if you want early out behavior.

[Pieresqi]

I think to trigger change detection you need mutable access which run condition doesn't let you have ? Or are there some interior mutability shenanigans ?

Or you mean run condition with change detection like resource_changed ? That would still be governed by "higher" run condition.

[hymm]

Or you mean run condition with change detection like resource_changed?

This. Each run condition has internal state on when it last ran, which leads to weirdness between what different run conditions observe and also what a system would observe. Quoting myself from a pr that was investigating this when early returns were allowed between different run conditions:

tests being talked about: https://github.com/maniwani/bevy/pull/65/commits/6fb33a5e8b994f82034539900e843af0695777fd

I realized that what is happening is that system sets are only evaluated once and it's evaluation is separate from the systems. So in the case of the test, the system set already ran on changed in the earlier frame, so it doesn't have it's change trackers reset, but the system condition would only evaluate to true on the next frame. So the behavior is correct in a sense, but I'm also not sure how intuitive it is. If we reverse the order of the conditions then it does run the system. So there's a non-commutative property for conditions on system sets -> systems. We could make it so that change trackers are set for everything, conditions and system, even if the system doesn't run. That would force multiple changed conditions to all be true on the same frame. It would be consistent, but we no longer can wait for all these things to be changed before running the system.

I think it might be worse with queries, since the changed entities that a run criteria observes can be different from what the system finally sees when it does run.

edit: adding some more details I just remembered

The schedule v3 pr tried to fix these issues by reseting a systems run_conditions change ticks to be the same as the system. Unfortunately you can't do the same for system_sets run_conditions, since they don't have change ticks or a set run time. And even if you tried to pick a specific change tick that would lead to inconsistency with the system run_conditions that have a different change tick. Overall it was a hard problem to figure out how to make it feel consistent and was adding a lot of complexity to the scheduler, so it was decided to just have run_conditions be evaluated always and not allow early out.

[UkoeHB]

Posting solution I recommended in #11464:

This could be solved with an optional run_if_fallthrough() condition that only executes if all parent conditions are true.