Skip to content

Commit

Permalink
Add more granular system sets for state transition schedule ordering (#…
Browse files Browse the repository at this point in the history
…13763)

# Objective

Fixes #13711 

## Solution

Introduce smaller, generic system sets for each schedule variant, which
are ordered against other generic variants:
- `ExitSchedules<S>` - For `OnExit` schedules, runs from leaf states to
root states.
- `TransitionSchedules<S>` - For `OnTransition` schedules, runs in
arbitrary order.
- `EnterSchedules<S>` - For `OnEnter` schedules, runs from root states
to leaf states.

Also unified `ApplyStateTransition<S>` schedule which works in basically
the same way, just for internals.

## Testing

- One test that tests schedule execution order

---------

Co-authored-by: Lee-Orr <lee-orr@users.noreply.github.com>
  • Loading branch information
MiniaczQ and lee-orr committed Jun 10, 2024
1 parent 50ee483 commit 6d0b750
Show file tree
Hide file tree
Showing 7 changed files with 265 additions and 88 deletions.
5 changes: 3 additions & 2 deletions crates/bevy_state/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,9 @@ pub mod prelude {
pub use crate::condition::*;
#[doc(hidden)]
pub use crate::state::{
last_transition, ComputedStates, NextState, OnEnter, OnExit, OnTransition, State, StateSet,
StateTransition, StateTransitionEvent, StateTransitionSteps, States, SubStates,
last_transition, ComputedStates, EnterSchedules, ExitSchedules, NextState, OnEnter, OnExit,
OnTransition, State, StateSet, StateTransition, StateTransitionEvent, States, SubStates,
TransitionSchedules,
};
#[doc(hidden)]
pub use crate::state_scoped::StateScoped;
Expand Down
26 changes: 16 additions & 10 deletions crates/bevy_state/src/state/freely_mutable_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,27 +17,33 @@ use super::{take_next_state, transitions::*};
pub trait FreelyMutableState: States {
/// This function registers all the necessary systems to apply state changes and run transition schedules
fn register_state(schedule: &mut Schedule) {
schedule.configure_sets((
ApplyStateTransition::<Self>::default()
.in_set(StateTransitionSteps::DependentTransitions),
ExitSchedules::<Self>::default().in_set(StateTransitionSteps::ExitSchedules),
TransitionSchedules::<Self>::default()
.in_set(StateTransitionSteps::TransitionSchedules),
EnterSchedules::<Self>::default().in_set(StateTransitionSteps::EnterSchedules),
));

schedule
.add_systems(
apply_state_transition::<Self>.in_set(ApplyStateTransition::<Self>::apply()),
)
.add_systems(
last_transition::<Self>
.pipe(run_enter::<Self>)
.in_set(StateTransitionSteps::EnterSchedules),
apply_state_transition::<Self>.in_set(ApplyStateTransition::<Self>::default()),
)
.add_systems(
last_transition::<Self>
.pipe(run_exit::<Self>)
.in_set(StateTransitionSteps::ExitSchedules),
.in_set(ExitSchedules::<Self>::default()),
)
.add_systems(
last_transition::<Self>
.pipe(run_transition::<Self>)
.in_set(StateTransitionSteps::TransitionSchedules),
.in_set(TransitionSchedules::<Self>::default()),
)
.configure_sets(
ApplyStateTransition::<Self>::apply().in_set(StateTransitionSteps::RootTransitions),
.add_systems(
last_transition::<Self>
.pipe(run_enter::<Self>)
.in_set(EnterSchedules::<Self>::default()),
);
}
}
Expand Down
129 changes: 120 additions & 9 deletions crates/bevy_state/src/state/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -508,14 +508,13 @@ mod tests {
#[test]
fn same_state_transition_should_emit_event_and_not_run_schedules() {
let mut world = World::new();
setup_state_transitions_in_world(&mut world, None);
EventRegistry::register_event::<StateTransitionEvent<SimpleState>>(&mut world);
world.init_resource::<State<SimpleState>>();
let mut schedules = Schedules::new();
let mut apply_changes = Schedule::new(StateTransition);
SimpleState::register_state(&mut apply_changes);
schedules.insert(apply_changes);
let mut schedules = world.resource_mut::<Schedules>();
let apply_changes = schedules.get_mut(StateTransition).unwrap();
SimpleState::register_state(apply_changes);

world.insert_resource(TransitionCounter::default());
let mut on_exit = Schedule::new(OnExit(SimpleState::A));
on_exit.add_systems(|mut c: ResMut<TransitionCounter>| c.exit += 1);
schedules.insert(on_exit);
Expand All @@ -528,9 +527,7 @@ mod tests {
let mut on_enter = Schedule::new(OnEnter(SimpleState::A));
on_enter.add_systems(|mut c: ResMut<TransitionCounter>| c.enter += 1);
schedules.insert(on_enter);

world.insert_resource(schedules);
setup_state_transitions_in_world(&mut world, None);
world.insert_resource(TransitionCounter::default());

world.run_schedule(StateTransition);
assert_eq!(world.resource::<State<SimpleState>>().0, SimpleState::A);
Expand All @@ -546,7 +543,7 @@ mod tests {
*world.resource::<TransitionCounter>(),
TransitionCounter {
exit: 0,
transition: 0,
transition: 1, // Same state transitions are allowed
enter: 0
}
);
Expand Down Expand Up @@ -619,4 +616,118 @@ mod tests {
1
);
}

#[derive(Resource, Default, Debug)]
struct TransitionTracker(Vec<&'static str>);

#[derive(PartialEq, Eq, Debug, Hash, Clone)]
enum TransitionTestingComputedState {
IsA,
IsBAndEven,
IsBAndOdd,
}

impl ComputedStates for TransitionTestingComputedState {
type SourceStates = (Option<SimpleState>, Option<SubState>);

fn compute(sources: (Option<SimpleState>, Option<SubState>)) -> Option<Self> {
match sources {
(Some(simple), sub) => {
if simple == SimpleState::A {
Some(Self::IsA)
} else if sub == Some(SubState::One) {
Some(Self::IsBAndOdd)
} else if sub == Some(SubState::Two) {
Some(Self::IsBAndEven)
} else {
None
}
}
_ => None,
}
}
}

#[test]
fn check_transition_orders() {
let mut world = World::new();
setup_state_transitions_in_world(&mut world, None);
EventRegistry::register_event::<StateTransitionEvent<SimpleState>>(&mut world);
EventRegistry::register_event::<StateTransitionEvent<SubState>>(&mut world);
EventRegistry::register_event::<StateTransitionEvent<TransitionTestingComputedState>>(
&mut world,
);
world.insert_resource(State(SimpleState::B(true)));
world.init_resource::<State<SubState>>();
world.insert_resource(State(TransitionTestingComputedState::IsA));
let mut schedules = world.remove_resource::<Schedules>().unwrap();
let apply_changes = schedules.get_mut(StateTransition).unwrap();
SimpleState::register_state(apply_changes);
SubState::register_sub_state_systems(apply_changes);
TransitionTestingComputedState::register_computed_state_systems(apply_changes);

world.init_resource::<TransitionTracker>();
fn register_transition(string: &'static str) -> impl Fn(ResMut<TransitionTracker>) {
move |mut transitions: ResMut<TransitionTracker>| transitions.0.push(string)
}

schedules.add_systems(
StateTransition,
register_transition("simple exit").in_set(ExitSchedules::<SimpleState>::default()),
);
schedules.add_systems(
StateTransition,
register_transition("simple transition")
.in_set(TransitionSchedules::<SimpleState>::default()),
);
schedules.add_systems(
StateTransition,
register_transition("simple enter").in_set(EnterSchedules::<SimpleState>::default()),
);

schedules.add_systems(
StateTransition,
register_transition("sub exit").in_set(ExitSchedules::<SubState>::default()),
);
schedules.add_systems(
StateTransition,
register_transition("sub transition")
.in_set(TransitionSchedules::<SubState>::default()),
);
schedules.add_systems(
StateTransition,
register_transition("sub enter").in_set(EnterSchedules::<SubState>::default()),
);

schedules.add_systems(
StateTransition,
register_transition("computed exit")
.in_set(ExitSchedules::<TransitionTestingComputedState>::default()),
);
schedules.add_systems(
StateTransition,
register_transition("computed transition")
.in_set(TransitionSchedules::<TransitionTestingComputedState>::default()),
);
schedules.add_systems(
StateTransition,
register_transition("computed enter")
.in_set(EnterSchedules::<TransitionTestingComputedState>::default()),
);

world.insert_resource(schedules);

world.run_schedule(StateTransition);

let transitions = &world.resource::<TransitionTracker>().0;

assert_eq!(transitions.len(), 9);
assert_eq!(transitions[0], "computed exit");
assert_eq!(transitions[1], "sub exit");
assert_eq!(transitions[2], "simple exit");
// Transition order is arbitrary and doesn't need testing.
assert_eq!(transitions[6], "simple enter");
assert_eq!(transitions[7], "sub enter");
assert_eq!(transitions[8], "computed enter");
}
}
121 changes: 78 additions & 43 deletions crates/bevy_state/src/state/state_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ use self::sealed::StateSetSealed;
use super::{
computed_states::ComputedStates, internal_apply_state_transition, last_transition, run_enter,
run_exit, run_transition, sub_states::SubStates, take_next_state, ApplyStateTransition,
NextState, State, StateTransitionEvent, StateTransitionSteps, States,
EnterSchedules, ExitSchedules, NextState, State, StateTransitionEvent, StateTransitionSteps,
States, TransitionSchedules,
};

mod sealed {
Expand Down Expand Up @@ -114,27 +115,35 @@ impl<S: InnerStateSet> StateSet for S {
internal_apply_state_transition(event, commands, current_state, new_state);
};

schedule.configure_sets((
ApplyStateTransition::<T>::default()
.in_set(StateTransitionSteps::DependentTransitions)
.after(ApplyStateTransition::<S::RawState>::default()),
ExitSchedules::<T>::default()
.in_set(StateTransitionSteps::ExitSchedules)
.before(ExitSchedules::<S::RawState>::default()),
TransitionSchedules::<T>::default().in_set(StateTransitionSteps::TransitionSchedules),
EnterSchedules::<T>::default()
.in_set(StateTransitionSteps::EnterSchedules)
.after(EnterSchedules::<S::RawState>::default()),
));

schedule
.add_systems(apply_state_transition.in_set(ApplyStateTransition::<T>::apply()))
.add_systems(
last_transition::<T>
.pipe(run_enter::<T>)
.in_set(StateTransitionSteps::EnterSchedules),
)
.add_systems(apply_state_transition.in_set(ApplyStateTransition::<T>::default()))
.add_systems(
last_transition::<T>
.pipe(run_exit::<T>)
.in_set(StateTransitionSteps::ExitSchedules),
.in_set(ExitSchedules::<T>::default()),
)
.add_systems(
last_transition::<T>
.pipe(run_transition::<T>)
.in_set(StateTransitionSteps::TransitionSchedules),
.in_set(TransitionSchedules::<T>::default()),
)
.configure_sets(
ApplyStateTransition::<T>::apply()
.in_set(StateTransitionSteps::DependentTransitions)
.after(ApplyStateTransition::<S::RawState>::apply()),
.add_systems(
last_transition::<T>
.pipe(run_enter::<T>)
.in_set(EnterSchedules::<T>::default()),
);
}

Expand Down Expand Up @@ -186,27 +195,35 @@ impl<S: InnerStateSet> StateSet for S {
internal_apply_state_transition(event, commands, current_state_res, new_state);
};

schedule.configure_sets((
ApplyStateTransition::<T>::default()
.in_set(StateTransitionSteps::DependentTransitions)
.after(ApplyStateTransition::<S::RawState>::default()),
ExitSchedules::<T>::default()
.in_set(StateTransitionSteps::ExitSchedules)
.before(ExitSchedules::<S::RawState>::default()),
TransitionSchedules::<T>::default().in_set(StateTransitionSteps::TransitionSchedules),
EnterSchedules::<T>::default()
.in_set(StateTransitionSteps::EnterSchedules)
.after(EnterSchedules::<S::RawState>::default()),
));

schedule
.add_systems(apply_state_transition.in_set(ApplyStateTransition::<T>::apply()))
.add_systems(
last_transition::<T>
.pipe(run_enter::<T>)
.in_set(StateTransitionSteps::EnterSchedules),
)
.add_systems(apply_state_transition.in_set(ApplyStateTransition::<T>::default()))
.add_systems(
last_transition::<T>
.pipe(run_exit::<T>)
.in_set(StateTransitionSteps::ExitSchedules),
.in_set(ExitSchedules::<T>::default()),
)
.add_systems(
last_transition::<T>
.pipe(run_transition::<T>)
.in_set(StateTransitionSteps::TransitionSchedules),
.in_set(TransitionSchedules::<T>::default()),
)
.configure_sets(
ApplyStateTransition::<T>::apply()
.in_set(StateTransitionSteps::DependentTransitions)
.after(ApplyStateTransition::<S::RawState>::apply()),
.add_systems(
last_transition::<T>
.pipe(run_enter::<T>)
.in_set(EnterSchedules::<T>::default()),
);
}
}
Expand Down Expand Up @@ -243,16 +260,25 @@ macro_rules! impl_state_set_sealed_tuples {
internal_apply_state_transition(event, commands, current_state, new_state);
};

schedule
.add_systems(apply_state_transition.in_set(ApplyStateTransition::<T>::apply()))
.add_systems(last_transition::<T>.pipe(run_enter::<T>).in_set(StateTransitionSteps::EnterSchedules))
.add_systems(last_transition::<T>.pipe(run_exit::<T>).in_set(StateTransitionSteps::ExitSchedules))
.add_systems(last_transition::<T>.pipe(run_transition::<T>).in_set(StateTransitionSteps::TransitionSchedules))
.configure_sets(
ApplyStateTransition::<T>::apply()
schedule.configure_sets((
ApplyStateTransition::<T>::default()
.in_set(StateTransitionSteps::DependentTransitions)
$(.after(ApplyStateTransition::<$param::RawState>::apply()))*
);
$(.after(ApplyStateTransition::<$param::RawState>::default()))*,
ExitSchedules::<T>::default()
.in_set(StateTransitionSteps::ExitSchedules)
$(.before(ExitSchedules::<$param::RawState>::default()))*,
TransitionSchedules::<T>::default()
.in_set(StateTransitionSteps::TransitionSchedules),
EnterSchedules::<T>::default()
.in_set(StateTransitionSteps::EnterSchedules)
$(.after(EnterSchedules::<$param::RawState>::default()))*,
));

schedule
.add_systems(apply_state_transition.in_set(ApplyStateTransition::<T>::default()))
.add_systems(last_transition::<T>.pipe(run_exit::<T>).in_set(ExitSchedules::<T>::default()))
.add_systems(last_transition::<T>.pipe(run_transition::<T>).in_set(TransitionSchedules::<T>::default()))
.add_systems(last_transition::<T>.pipe(run_enter::<T>).in_set(EnterSchedules::<T>::default()));
}

fn register_sub_state_systems_in_schedule<T: SubStates<SourceStates = Self>>(
Expand Down Expand Up @@ -288,16 +314,25 @@ macro_rules! impl_state_set_sealed_tuples {
internal_apply_state_transition(event, commands, current_state_res, new_state);
};

schedule
.add_systems(apply_state_transition.in_set(ApplyStateTransition::<T>::apply()))
.add_systems(last_transition::<T>.pipe(run_enter::<T>).in_set(StateTransitionSteps::EnterSchedules))
.add_systems(last_transition::<T>.pipe(run_exit::<T>).in_set(StateTransitionSteps::ExitSchedules))
.add_systems(last_transition::<T>.pipe(run_transition::<T>).in_set(StateTransitionSteps::TransitionSchedules))
.configure_sets(
ApplyStateTransition::<T>::apply()
schedule.configure_sets((
ApplyStateTransition::<T>::default()
.in_set(StateTransitionSteps::DependentTransitions)
$(.after(ApplyStateTransition::<$param::RawState>::apply()))*
);
$(.after(ApplyStateTransition::<$param::RawState>::default()))*,
ExitSchedules::<T>::default()
.in_set(StateTransitionSteps::ExitSchedules)
$(.before(ExitSchedules::<$param::RawState>::default()))*,
TransitionSchedules::<T>::default()
.in_set(StateTransitionSteps::TransitionSchedules),
EnterSchedules::<T>::default()
.in_set(StateTransitionSteps::EnterSchedules)
$(.after(EnterSchedules::<$param::RawState>::default()))*,
));

schedule
.add_systems(apply_state_transition.in_set(ApplyStateTransition::<T>::default()))
.add_systems(last_transition::<T>.pipe(run_exit::<T>).in_set(ExitSchedules::<T>::default()))
.add_systems(last_transition::<T>.pipe(run_transition::<T>).in_set(TransitionSchedules::<T>::default()))
.add_systems(last_transition::<T>.pipe(run_enter::<T>).in_set(EnterSchedules::<T>::default()));
}
}
};
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_state/src/state/sub_states.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ pub trait SubStates: States + FreelyMutableState {
///
/// This can either be a single type that implements [`States`], or a tuple
/// containing multiple types that implement [`States`], or any combination of
/// types implementing [`States`] and Options of types implementing [`States`]
/// types implementing [`States`] and Options of types implementing [`States`].
type SourceStates: StateSet;

/// This function gets called whenever one of the [`SourceStates`](Self::SourceStates) changes.
Expand Down
Loading

0 comments on commit 6d0b750

Please sign in to comment.