Make current state a property of StateStage

[DJMcNab]

Extends #1021
Fixes #1117
This also allows avoiding the Clone bound on state

Possible future work:

• Make state use Eq instead

[cart]

Hmm this changes things so that State transitions are no longer a global thing with a well defined order of execution. Each StateStage has its own "last state", which can cause confusing outcomes:

enum AppState {
    X,
    Y,
    Z,
}

Res<AppState> = AppState::X

ITERATION 1
------------

StateStageA (current None)

1. on_enter(X)
2. system sets Y
3. on_exit(X)
4. on_enter(Y)
5. on_update(Y)

(StateStageA current is now Y)

StateStageB (current None)

1. on_enter(Y)
2. system sets Z
3. on_exit(Y)
4. on_enter(Z)
5. on_update(Z)

(StateStageB current is now Z)

ITERATION 2
------------

StateStageA (current Y)

1. on_exit(Y)
    * notice that on_exit(Y) is called, despite the fact that we are not currently in AppState::Y
    * (we are currently in AppState::Z)
2. on_enter(Z)

[cart]

Lazy illustrative example:

use bevy::prelude::*;

#[derive(Clone)]
pub enum AppState {
    X,
    Y,
    Z,
}

fn main() {
    App::build()
        .add_plugins(DefaultPlugins)
        .add_resource(State::new(AppState::X))
        // uncomment the next line and 'enter_system' is not called
        .add_stage_after(
            stage::UPDATE,
            "STAGE_A",
            StateStage::<AppState>::default()
                .with_enter_stage(
                    AppState::X,
                    SystemStage::parallel()
                        .with_system(a.system())
                        .with_system(a_x_enter.system()),
                )
                .with_update_stage(AppState::X, SystemStage::parallel().with_system(a.system()))
                .with_exit_stage(AppState::X, SystemStage::parallel().with_system(a.system()))
                .with_enter_stage(AppState::Y, SystemStage::parallel().with_system(a.system()))
                .with_update_stage(AppState::Y, SystemStage::parallel().with_system(a.system()))
                .with_exit_stage(AppState::Y, SystemStage::parallel().with_system(a.system()))
                .with_enter_stage(AppState::Z, SystemStage::parallel().with_system(a.system()))
                .with_update_stage(AppState::Z, SystemStage::parallel().with_system(a.system()))
                .with_exit_stage(AppState::Z, SystemStage::parallel().with_system(a.system())),
        )
        .add_stage_after(
            "STAGE_A",
            "STAGE_B",
            StateStage::<AppState>::default()
                .with_enter_stage(AppState::X, SystemStage::parallel().with_system(b.system()))
                .with_update_stage(AppState::X, SystemStage::parallel().with_system(b.system()))
                .with_exit_stage(AppState::X, SystemStage::parallel().with_system(b.system()))
                .with_enter_stage(
                    AppState::Y,
                    SystemStage::parallel()
                        .with_system(b.system())
                        .with_system(b_y_enter.system()),
                )
                .with_update_stage(AppState::Y, SystemStage::parallel().with_system(b.system()))
                .with_exit_stage(AppState::Y, SystemStage::parallel().with_system(b.system()))
                .with_enter_stage(AppState::Z, SystemStage::parallel().with_system(b.system()))
                .with_update_stage(AppState::Z, SystemStage::parallel().with_system(b.system()))
                .with_exit_stage(AppState::Z, SystemStage::parallel().with_system(b.system())),
        )
        .add_system_to_stage(stage::LAST, end.system())
        .run();
}

fn a_x_enter(mut state: ResMut<State<AppState>>) {
    state.set_next(AppState::Y).unwrap();
}
fn b_y_enter(mut state: ResMut<State<AppState>>) {
    state.set_next(AppState::Z).unwrap();
}
fn a() {
    println!("  a")
}
fn b() {
    println!("  b")
}

fn end(mut x: Local<usize>) {
    *x+=1;

    if *x == 2 {
        panic!("end");
    }
}

// modify state.rs to print each state
#[allow(clippy::mem_discriminant_non_enum)]
impl<T: Resource> Stage for StateStage<T> {
    fn run(&mut self, world: &mut World, resources: &mut Resources) {
        let current_stage = loop {
            let next = {
                let mut state = resources
                    .get_mut::<State<T>>()
                    .expect("Missing state resource");
                state.previous = state.apply_next().or_else(|| state.previous.take());
                mem::discriminant(&state.current)
            };
            if self.current_stage == Some(next) {
                break next;
            } else {
                let cur = self.current_stage.map(|i| i);
                if let Some(current_state_stages) =
                    self.current_stage.and_then(|it| self.stages.get_mut(&it))
                {
                    println!("exit {:?}", cur.unwrap());
                    current_state_stages.exit.run(world, resources);
                }
                self.current_stage = Some(next);
                if let Some(next_state_stages) = self.stages.get_mut(&next) {
                    println!("enter {:?}", next);
                    next_state_stages.enter.run(world, resources);
                }
            }
        };

        if let Some(current_state_stages) = self.stages.get_mut(&current_stage) {
            println!("update {:?}", current_stage);
            current_state_stages.update.run(world, resources);
        }
    }
}

[DJMcNab]

My mental model is that each StateStage is standalone, and on_state_enter and on_state_exit's roles are to do setup and teardown for that State's on_update stage, within the single StateStage. In your example, my code makes it so that each on_enter has a corresponding on_exit, and no unecessary on_enter/on_exits are called.

That is, I would say that your unexpected behaviour is exactly what I would expect from StateStage.

[cart]

Hmm yeah thats a reasonable take. I think whatever solution we land on needs to be reconciled with the upcoming SystemSet::with_run_criteria (#1144). I think it can be accomplished with a new OnStateEnter::<T> system/criteria, which could internally store the state info, but we should spec that out. Handling run_criteria correctly for states felt "easier" with a global view of state because we could just expose that information directly on the State datatype.

[Ratysz]

This is probably superseded by #1424.