Skip to content

Fix SubStates with multiple source states not reacting to all source changes #19595

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
197 changes: 197 additions & 0 deletions crates/bevy_state/src/state/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,203 @@ mod tests {
}
}

#[derive(PartialEq, Eq, Debug, Hash, Clone)]
enum MultiSourceComputedState {
FromSimpleBTrue,
FromSimple2B2,
FromBoth,
}

impl ComputedStates for MultiSourceComputedState {
type SourceStates = (SimpleState, SimpleState2);

fn compute((simple_state, simple_state2): (SimpleState, SimpleState2)) -> Option<Self> {
match (simple_state, simple_state2) {
// If both are in their special states, prioritize the "both" variant.
(SimpleState::B(true), SimpleState2::B2) => Some(Self::FromBoth),
// If only SimpleState is B(true).
(SimpleState::B(true), _) => Some(Self::FromSimpleBTrue),
// If only SimpleState2 is B2.
(_, SimpleState2::B2) => Some(Self::FromSimple2B2),
// Otherwise, no computed state.
_ => None,
}
}
}

/// This test ensures that [`ComputedStates`] with multiple source states
/// react when any source changes.
#[test]
fn computed_state_with_multiple_sources_should_react_to_any_source_change() {
let mut world = World::new();
EventRegistry::register_event::<StateTransitionEvent<SimpleState>>(&mut world);
EventRegistry::register_event::<StateTransitionEvent<SimpleState2>>(&mut world);
EventRegistry::register_event::<StateTransitionEvent<MultiSourceComputedState>>(&mut world);

world.init_resource::<State<SimpleState>>();
world.init_resource::<State<SimpleState2>>();

let mut schedules = Schedules::new();
let mut apply_changes = Schedule::new(StateTransition);
SimpleState::register_state(&mut apply_changes);
SimpleState2::register_state(&mut apply_changes);
MultiSourceComputedState::register_computed_state_systems(&mut apply_changes);
schedules.insert(apply_changes);

world.insert_resource(schedules);
setup_state_transitions_in_world(&mut world);

// Initial state: SimpleState::A, SimpleState2::A1 and
// MultiSourceComputedState should not exist yet.
world.run_schedule(StateTransition);
assert_eq!(world.resource::<State<SimpleState>>().0, SimpleState::A);
assert_eq!(world.resource::<State<SimpleState2>>().0, SimpleState2::A1);
assert!(!world.contains_resource::<State<MultiSourceComputedState>>());

// Change only SimpleState to B(true) - this should trigger
// MultiSourceComputedState.
world.insert_resource(NextState::Pending(SimpleState::B(true)));
world.run_schedule(StateTransition);
assert_eq!(
world.resource::<State<SimpleState>>().0,
SimpleState::B(true)
);
assert_eq!(world.resource::<State<SimpleState2>>().0, SimpleState2::A1);
// The computed state should exist because SimpleState changed to
// B(true).
assert!(world.contains_resource::<State<MultiSourceComputedState>>());
assert_eq!(
world.resource::<State<MultiSourceComputedState>>().0,
MultiSourceComputedState::FromSimpleBTrue
);

// Reset SimpleState to A - computed state should be removed.
world.insert_resource(NextState::Pending(SimpleState::A));
world.run_schedule(StateTransition);
assert!(!world.contains_resource::<State<MultiSourceComputedState>>());

// Now change only SimpleState2 to B2 - this should also trigger
// MultiSourceComputedState.
world.insert_resource(NextState::Pending(SimpleState2::B2));
world.run_schedule(StateTransition);
assert_eq!(world.resource::<State<SimpleState>>().0, SimpleState::A);
assert_eq!(world.resource::<State<SimpleState2>>().0, SimpleState2::B2);
// The computed state should exist because SimpleState2 changed to B2.
assert!(world.contains_resource::<State<MultiSourceComputedState>>());
assert_eq!(
world.resource::<State<MultiSourceComputedState>>().0,
MultiSourceComputedState::FromSimple2B2
);

// Test that changes to both states work.
world.insert_resource(NextState::Pending(SimpleState::B(true)));
world.insert_resource(NextState::Pending(SimpleState2::A1));
world.run_schedule(StateTransition);
assert_eq!(
world.resource::<State<MultiSourceComputedState>>().0,
MultiSourceComputedState::FromSimpleBTrue
);
}

// Test SubState that depends on multiple source states.
#[derive(PartialEq, Eq, Debug, Default, Hash, Clone)]
enum MultiSourceSubState {
#[default]
Active,
}

impl SubStates for MultiSourceSubState {
type SourceStates = (SimpleState, SimpleState2);

fn should_exist(
(simple_state, simple_state2): (SimpleState, SimpleState2),
) -> Option<Self> {
// SubState should exist when:
// - SimpleState is B(true), OR
// - SimpleState2 is B2
match (simple_state, simple_state2) {
(SimpleState::B(true), _) | (_, SimpleState2::B2) => Some(Self::Active),
_ => None,
}
}
}

impl States for MultiSourceSubState {
const DEPENDENCY_DEPTH: usize = <Self as SubStates>::SourceStates::SET_DEPENDENCY_DEPTH + 1;
}

impl FreelyMutableState for MultiSourceSubState {}

/// This test ensures that [`SubStates`] with multiple source states react
/// when any source changes.
#[test]
fn sub_state_with_multiple_sources_should_react_to_any_source_change() {
let mut world = World::new();
EventRegistry::register_event::<StateTransitionEvent<SimpleState>>(&mut world);
EventRegistry::register_event::<StateTransitionEvent<SimpleState2>>(&mut world);
EventRegistry::register_event::<StateTransitionEvent<MultiSourceSubState>>(&mut world);

world.init_resource::<State<SimpleState>>();
world.init_resource::<State<SimpleState2>>();

let mut schedules = Schedules::new();
let mut apply_changes = Schedule::new(StateTransition);
SimpleState::register_state(&mut apply_changes);
SimpleState2::register_state(&mut apply_changes);
MultiSourceSubState::register_sub_state_systems(&mut apply_changes);
schedules.insert(apply_changes);

world.insert_resource(schedules);
setup_state_transitions_in_world(&mut world);

// Initial state: SimpleState::A, SimpleState2::A1 and
// MultiSourceSubState should not exist yet.
world.run_schedule(StateTransition);
assert_eq!(world.resource::<State<SimpleState>>().0, SimpleState::A);
assert_eq!(world.resource::<State<SimpleState2>>().0, SimpleState2::A1);
assert!(!world.contains_resource::<State<MultiSourceSubState>>());

// Change only SimpleState to B(true) - this should trigger
// MultiSourceSubState.
world.insert_resource(NextState::Pending(SimpleState::B(true)));
world.run_schedule(StateTransition);
assert_eq!(
world.resource::<State<SimpleState>>().0,
SimpleState::B(true)
);
assert_eq!(world.resource::<State<SimpleState2>>().0, SimpleState2::A1);
// The sub state should exist because SimpleState changed to B(true).
assert!(world.contains_resource::<State<MultiSourceSubState>>());

// Reset to initial state.
world.insert_resource(NextState::Pending(SimpleState::A));
world.run_schedule(StateTransition);
assert!(!world.contains_resource::<State<MultiSourceSubState>>());

// Now change only SimpleState2 to B2 - this should also trigger
// MultiSourceSubState creation.
world.insert_resource(NextState::Pending(SimpleState2::B2));
world.run_schedule(StateTransition);
assert_eq!(world.resource::<State<SimpleState>>().0, SimpleState::A);
assert_eq!(world.resource::<State<SimpleState2>>().0, SimpleState2::B2);
// The sub state should exist because SimpleState2 changed to B2.
assert!(world.contains_resource::<State<MultiSourceSubState>>());

// Finally, test that it works when both change simultaneously.
world.insert_resource(NextState::Pending(SimpleState::B(false)));
world.insert_resource(NextState::Pending(SimpleState2::A1));
world.run_schedule(StateTransition);
// After this transition, the state should not exist since SimpleState
// is B(false).
assert!(!world.contains_resource::<State<MultiSourceSubState>>());

// Change both at the same time.
world.insert_resource(NextState::Pending(SimpleState::B(true)));
world.insert_resource(NextState::Pending(SimpleState2::B2));
world.run_schedule(StateTransition);
assert!(world.contains_resource::<State<MultiSourceSubState>>());
}

#[test]
fn check_transition_orders() {
let mut world = World::new();
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_state/src/state/state_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ macro_rules! impl_state_set_sealed_tuples {
current_state_res: Option<ResMut<State<T>>>,
next_state_res: Option<ResMut<NextState<T>>>,
($($val),*,): ($(Option<Res<State<$param::RawState>>>),*,)| {
let parent_changed = ($($evt.read().last().is_some())&&*);
let parent_changed = ($($evt.read().last().is_some())||*);
let next_state = take_next_state(next_state_res);

if !parent_changed && next_state.is_none() {
Expand Down