Systems set to run on state resume do not respect `.after()` constraints

[Neurrone]

Bevy version

0.8.1

What you did

Describe how you arrived at the problem. If you can, consider providing a code snippet or link.

I have my game menus each have a corresponding AppState. For example, there is a state for options menu, main menu etc.

I have two systems:

• on_previous_menu_event which I need ran before
• on_resume, which gets ran when I resume a state.

I tried to achieve this by doing the following:

  .add_system(on_previous_menu_event.before(on_menu_spawn))
  .add_system_set(SystemSet::on_resume(AppState::MainMenu).with_system(on_resume.after(on_previous_menu_event)))

The ordering is required for the logic to work correctly.

What went wrong

By inserting print statements into both systems, I see that about half the time, the systems are ran in the opposite order in which I declared above.

Additional information

Other information that can be used to further reproduce or isolate the problem.
This commonly includes:

• screenshots
• logs
• theories about what might be going wrong
• workarounds that you used
• links to related bugs, PRs or discussions

[tim-blackbird]

I don't think it's possible to order a system relative to a system in a different set, currently. I think this is supposed to throw a warning.
You can instead order the entire SystemSet to run after on_previous_menu_event.
Both seem to work.

[Neurrone]

You can instead order the entire SystemSet to run after on_previous_menu_event.

I didn't know that was possible, how do I do that?

[Neurrone]

I tried this which doesn't seem to work either.

.add_system_set(SystemSet::on_resume(AppState::MainMenu).after(on_previous_menu_event).with_system(on_resume))

[tim-blackbird]

I tried reproducing with the code below, but it behaves as i would expect.

Spoiler

use std::time::Duration;

use bevy::prelude::*;

#[derive(Clone, Copy, PartialEq, Eq, Debug, Hash)]
enum AppState {
    One,
    Two,
}

fn system_a(mut state: ResMut<State<AppState>>) {
    match state.current() {
        AppState::One => {
            println!("push two");
            let _ = state.push(AppState::Two);
        }
        AppState::Two => {
            println!("resuming one");
            let _ = state.pop();
        }
    }
    println!("a");
    std::thread::sleep(Duration::from_millis(500));
}

fn system_b() {
    println!("b");
    std::thread::sleep(Duration::from_millis(500));
}

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_state(AppState::One)
        .add_system(system_a)
        .add_system_set(SystemSet::on_resume(AppState::One).with_system(system_b.after(system_a)))
        .run();
}

[Neurrone]

I'll try to create a minimal reproduceable example, thanks a lot for that code sample. I'll build off on that to get something that better matches my code.

[Neurrone]

In the process of trying to emonstrate this bug, I believe I may have found another one.

use std::time::Duration;

use bevy::prelude::*;

#[derive(Clone, Copy, PartialEq, Eq, Debug, Hash)]
enum AppState {
    MainMenu,
    Options,
}

fn display_options_menu(mut state: ResMut<State<AppState>>) {
    std::thread::sleep(Duration::from_millis(500));
    println!("Switching to options menu");
    state.push(AppState::Options).unwrap();
}

fn resume_main_menu() {
    println!("Resuming main menu state");
}

struct PreviousMenuEvent;

fn return_to_main_menu(
    mut state: ResMut<State<AppState>>,
    mut prev_menu_evt: EventWriter<PreviousMenuEvent>,
) {
    // std::thread::sleep(Duration::from_millis(500));
    println!("Returning to main menu");
    prev_menu_evt.send(PreviousMenuEvent);
    state.pop().unwrap();
}

fn on_previous_menu_event(mut prev_menu_evt: EventReader<PreviousMenuEvent>) {
    println!("Running on_previous_menu_event system");
    for _ev in prev_menu_evt.iter() {
        println!("Got previous menu event");
    }
}

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_event::<PreviousMenuEvent>()
        .add_system(on_previous_menu_event)
        .add_state(AppState::MainMenu)
        .add_system_set(SystemSet::on_update(AppState::MainMenu).with_system(display_options_menu))
        .add_system_set(
            SystemSet::on_update(AppState::Options)
                .with_system(return_to_main_menu.before(on_previous_menu_event)),
        )
        .add_system_set(
            SystemSet::on_resume(AppState::MainMenu)
                .with_system(resume_main_menu.after(on_previous_menu_event)),
        )
        .run();
}

In this example, the bug I intended to demonstrate was resume_main_menu doesn't run properly after on_previous_menu_event.

However when I run this example, the on_previous_menu_event system is only ever run once and never gets run subsequently, which is a separate bug that I haven't encountered before.

[tim-blackbird]

When a state transition happens it re-runs on_update so the above code is entering an endless loop, it never even reaches the PostUpdate stage.

_{Jeez. The state stuff is super prone to footguns and nothing is documented :/
I'm glad we're going to yeet this implementation}

[Neurrone]

I'm very confused that on_update gets run a second time during a state transition.

I'll refactor it so that it doesn't run during a state update and see if that fixes it.

[Neurrone]

I've reproduced the problem, where the resume code runs before the event is received. Although now I suspect that the event being received only after returning to the main menu is just due to the order that on_previous_menu_event runs.

on_previous_menu_event is probably run before switch_menus, which would explain why I only receive the event when returning to the main menu.

However, this doesn't explain why the .after constraint on on_resume doesn't seem to work.

use bevy::prelude::*;

#[derive(Clone, Copy, PartialEq, Eq, Debug, Hash)]
enum AppState {
    MainMenu,
    Options,
}

fn switch_menus(
    mut state: ResMut<State<AppState>>,
    mut input: ResMut<Input<KeyCode>>,
    mut prev_menu_evt: EventWriter<PreviousMenuEvent>,
) {
    if !input.just_pressed(KeyCode::Return) {
        return;
    }
    input.clear();
    match state.current() {
        AppState::MainMenu => {
            println!("Switching to options menu");
            state.push(AppState::Options);
        }
        AppState::Options => {
            println!("Returning to main menu");
            prev_menu_evt.send(PreviousMenuEvent);
            state.pop().unwrap();
        }
    }
}

fn resume_main_menu() {
    println!("Resuming main menu state");
}

struct PreviousMenuEvent;

fn on_previous_menu_event(mut prev_menu_evt: EventReader<PreviousMenuEvent>) {
    // println!("Running on_previous_menu_event system");
    for _ev in prev_menu_evt.iter() {
        println!("Got previous menu event");
    }
}

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_event::<PreviousMenuEvent>()
        .add_system(on_previous_menu_event)
        .add_state(AppState::MainMenu)
        .add_system_set(SystemSet::on_update(AppState::MainMenu).with_system(switch_menus))
        .add_system_set(SystemSet::on_update(AppState::Options).with_system(switch_menus))
        .add_system_set(
            SystemSet::on_resume(AppState::MainMenu)
                .with_system(resume_main_menu.after(on_previous_menu_event)),
        )
        .run();
}

I can work around this issue, e.g by using .add_system(on_previous_menu_event.after(switch_menus)) to get the desired effect.