bevy icon indicating copy to clipboard operation
bevy copied to clipboard

OnEnter schedule runs later than expected for default states

Open NiklasEi opened this issue 2 years ago • 4 comments

Bevy version

main branch a1e4114ebee5b903a95dc1438b4f0d946ac3c8be

What you did

I am trying to update a Plugin using States to the latest Bevy main. The plugin relies on "on enter" systems to prepare for certain user-configured states.

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_state::<AppState>()
        .add_system_to_schedule(OnEnter(AppState::Start), prepare_state)
        .add_system(relies_on_prepare_state.in_base_Set(CoreSet::PreUpdate).run_if(in_state(AppState::Start)))
       .run();
}

#[derive(Debug, Clone, PartialEq, Eq, Hash, Default, States)]
enum AppState {
    #[default]
    Start,
    Next,
}

What went wrong

If e.g. prepare_state initializes resources that the system relies_on_prepare_state asks for, the app will panic, because these resources will not be in the ECS. It works fine if we change the state to be Next and add a single system switching to the Next state.

The default states onEnter schedule is run in the first CoreSet::StateTransitions. But the State resource will already have the default value from the beginning. This means that systems running beforeCoreSet::StateTransitions, see an unexpected situation: The state resource says that we are currently in state "X" (so systems only running in that state will run), but the OnEnter schedule of state "X" has not been executed yet.

The minimum here would be to very clearly document that any systems relying on states should only run after CoreSet::StateTransitions.

NiklasEi avatar Feb 11 '23 22:02 NiklasEi

https://github.com/bevyengine/bevy/blob/a1e4114ebee5b903a95dc1438b4f0d946ac3c8be/crates/bevy_app/src/app.rs#L333

This is intended to do this, but it must not be working as expected.

alice-i-cecile avatar Feb 11 '23 22:02 alice-i-cecile

This issue showed up, because my plugin was panicing in a PreUpdate system. The OnEnter schedule does run for the default state, but only later in StateTransitions.

NiklasEi avatar Feb 11 '23 23:02 NiklasEi

Why are you adding systems to PreUpdate? Do you really need those systems to be there?

Now, after stageless, could you not achieve what you want by placing your systems in a separate set and adding the needed ordering constraints so they run before Update/Fixed update but after state transitions?

inodentry avatar Feb 12 '23 00:02 inodentry

That's a good point, thank you. I was still thinking in the "old schedules way".

Ordering against StateTransitions (or any base set for that matter) is currently very hard though because of #7632. Any on_update system cannot be ordered against it, because it is part of StateTransitions. I would expect that to be the most common case for user systems.

I still think that this issue is valid and unexpected behaviour. If the App is already in State "X" before StageTransitions then I would expect that the OnEnter schedule already ran. That is also the behaviour for all other state values apart from the default.

NiklasEi avatar Feb 13 '23 19:02 NiklasEi

This is solved in pyri_state. Default states are handled by setting the current state to None and the next state to Some(default_state), and then that transition is handled atomically in the StateFlush schedule.

So your system in PreUpdate would see the current state as None instead of a state that hasn't been entered yet, and systems in Update would see the current state as Some(current_state).

benfrankel avatar May 23 '24 22:05 benfrankel

This seems to be fixed. The following app runs fine on latest main (d7fc20c484f74e1f4b0640f5a65326d632540cb1):

use bevy::prelude::*;

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .init_state::<MyState>()
        .add_systems(OnEnter(MyState::Test), prepare_state)
        .add_systems(First, use_test_resource.run_if(in_state(MyState::Test)))
        .run();

}

#[derive(States, Default, Clone, Eq, Debug, Hash, PartialEq)]
enum MyState {
    #[default]
    Test
}

#[derive(Resource)]
struct TestResource;

fn prepare_state(world: &mut World) {
    println!("inserting.");
    world.insert_resource(TestResource);
}

fn use_test_resource(_test_res: Res<TestResource>) {
    println!("this system runs fine");
}

NiklasEi avatar May 27 '24 19:05 NiklasEi