When combining system conditions `not()` and `and()`, `not()` has no effect

[mamekoro]

Bevy version and features

0.18.1

What you did

I combined system conditions with and() so on_countdown_completed would run when Countdown reached zero. I added not(resource_added::<Countdown>) because I didn't want it to run immediately after the resource was added, but that didn’t work.

use bevy::prelude::*;

#[derive(Copy, Clone, Debug, PartialEq, Eq, Default, Resource)]
struct Countdown(usize);

fn on_countdown_completed(countdown: Res<Countdown>) {
    dbg!(countdown.is_added(), countdown.is_changed(), countdown.0);
    println!("countdown completed")
}

fn main() {
    App::new()
        .add_plugins(MinimalPlugins)
        .init_resource::<Countdown>()
        .add_systems(
            Update,
            on_countdown_completed.run_if(
                // This system should not run if Countdown was just added.
                not(resource_added::<Countdown>)
                    // This system should run if Countdown was decremented to zero.
                    .and(resource_changed::<Countdown>)
                    .and(resource_equals(Countdown(0))),
            ),
        )
        .run();
}

What went wrong

on_countdown_completed runs when countdown.is_added() is true.

[src/main.rs:7:5] countdown.is_added() = true
[src/main.rs:7:5] countdown.is_changed() = true
[src/main.rs:7:5] countdown.0 = 0
countdown completed

Replacing not(resource_added::<Countdown>) with resource_added::<Countdown> produced the same output.
What's going on?

[hxYuki]

Not sure what happens, but you can swap changed and added, which behaves properly.

        .add_systems(
            Update,
            on_countdown_completed.run_if(
                resource_changed::<Countdown>
                    .and(not(resource_added::<Countdown>))
                    .and(resource_equals(Countdown(0))),
            ),
        )

In addition, it always prints [src\main.rs:9:5] countdown.is_added() = true on the first run.
This may be caused by some caching mechanism inside I guess. And I suspect the not() actually works, but read an outdated message.

countdown: 2
countdown: 1
countdown: 0
[src\main.rs:9:5] countdown.is_added() = true
[src\main.rs:9:5] countdown.is_changed() = true
[src\main.rs:9:5] countdown.0 = 0
countdown completed
reset count
countdown: 2
countdown: 1
countdown: 0
[src\main.rs:9:5] countdown.is_added() = false
[src\main.rs:9:5] countdown.is_changed() = true
[src\main.rs:9:5] countdown.0 = 0
countdown completed
reset count

use std::time::Duration;

use bevy::{prelude::*, time::common_conditions::on_timer};

#[derive(Copy, Clone, Debug, PartialEq, Eq, Default, Resource)]
struct Countdown(usize);

fn on_countdown_completed(countdown: ResMut<Countdown>) {
    dbg!(countdown.is_added(), countdown.is_changed(), countdown.0);
    println!("countdown completed");

    reset_count(countdown);
}

fn tick_down(mut countdown: ResMut<Countdown>) {
    if countdown.0 > 0 {
        countdown.0 -= 1;
        println!("countdown: {}", countdown.0);
    }
}

fn reset_count(mut countdown: ResMut<Countdown>) {
    if countdown.0 == 0 {
        countdown.0 = 3;
    }
    println!("reset count");
}

fn main() {
    App::new()
        .add_plugins(MinimalPlugins)
        .insert_resource::<Countdown>(Countdown(3))
        .add_systems(Update, (tick_down).run_if(on_timer(Duration::from_secs(1))))
        .add_systems(
            Update,
            on_countdown_completed.run_if(
                
                // not(resource_added::<Countdown>)
                //     .and(resource_changed::<Countdown>)
                //     .and(resource_equals(Countdown(0))),
                
                resource_changed::<Countdown>
                    .and(not(resource_added::<Countdown>))
                    .and(resource_equals(Countdown(0))),
            ),
        )
        .run();
}

[amtep]

In addition, it always prints [src\main.rs:9:5] countdown.is_added() = true on the first run.

I think this behavior is correct. The documentation for DetectChanges::is_added says "Returns true if this value was added after the system last ran". So it's not about last frame or anything.

[hxYuki]

behavior is correct.

Yhea, I get the point that the behavior is for the systems which don't run every tick to get the update. But this issue is sure caused from this behavior.

It is because that the latter condition is shortcircuited, then naturally get a wrong state when it can run.

Now that and and or are deprecated in #22690 in favor of *_eager to drop outdated state and and_then/or_else with same behavior. We can close this trivially after next release.