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

[mgi388]

Objective

• Fix issue where SubStates depending on multiple source states would only react when all source states changed simultaneously.
• SubStates should be created/destroyed whenever any of their source states transitions, not only when all change together.

Solution

• Changed the "did parent change" detection logic from AND to OR. We need to check if any of the event readers changed, not if all of them changed.
• See https://github.com/bevyengine/bevy/actions/runs/15610159742/job/43968937544?pr=19595 for failing test proof before I pushed the fix.
• The generated code we want needs ||s not &&s like this:

fn register_sub_state_systems_in_schedule<T: SubStates<SourceStates = Self>>(schedule: &mut Schedule) {
  let apply_state_transition = |(mut ereader0, mut ereader1, mut ereader2): (
      EventReader<StateTransitionEvent<S0::RawState>>,
      EventReader<StateTransitionEvent<S1::RawState>>,
      EventReader<StateTransitionEvent<S2::RawState>>,
  ),
      event: EventWriter<StateTransitionEvent<T>>,
      commands: Commands,
      current_state_res: Option<ResMut<State<T>>>,
      next_state_res: Option<ResMut<NextState<T>>>,
      (s0, s1, s2): (
          Option<Res<State<S0::RawState>>>,
          Option<Res<State<S1::RawState>>>,
          Option<Res<State<S2::RawState>>>,
  )| {
    // With `||` we can correctly count parent changed if any of the sources changed.
    let parent_changed = (ereader0.read().last().is_some()
        || ereader1.read().last().is_some()
        || ereader2.read().last().is_some());
    let next_state = take_next_state(next_state_res);
    if !parent_changed && next_state.is_none() {
        return;
    }
    // ...
  }
}

Testing

• Add new test.
• Check the fix worked in my game.

[mgi388]

@benfrankel @MiniaczQ care to review?

[benfrankel]

This fix seems correct. I'm curious why the computed states version checks event_reader.is_empty() && ... while the sub states version checks !(event_reader.read().last().is_some() || ...), though. It seems like they're trying to do the same thing.

[mgi388]

@benfrankel

This fix seems correct. I'm curious why the computed states version checks event_reader.is_empty() && ... while the sub states version checks !(event_reader.read().last().is_some() || ...), though. It seems like they're trying to do the same thing.

Yeah I am unsure as well. As you say, the docs for ComputedStates say:

This function gets called whenever one of the SourceStates changes.

And the same for SubStates:

This function gets called whenever one of the SourceStates changes.

I added a new similar test ComputedStates, doing the same thing, and it works with the existing implementation.

I'm inclined to not try and change the internals of either though. The two new tests prove the expected behavior and so theoretically they can be refactored in the future as long as the tests still pass.