Set SCOPED_ENTITIES_ENABLED to true for ComputedStates

[nezuo]

Objective

• There is no way to enable scoped entities for ComputedStates.
• There is a blanket impl of States for ComputedStates, making it impossible to configure.

Solution

• Set SCOPED_ENTITIES_ENABLED to true for ComputedStates by default (solution suggested by @alice-i-cecile).

Testing

• I tested the computed_states example which relies on state scoped computed states.
• Added integration test.

[github-actions]

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

[mgi388]

If this is a regression, is there any unit test we can add that proves it now works?

Re:

I tested the computed_states example which relies on state scoped computed states.

Does this mean that if you run the example prior to this PR it's visibly broken and after this PR you can see it's fixed? Can you elaborate?

[nezuo]

Does this mean that if you run the example prior to this PR it's visibly broken and after this PR you can see it's fixed? Can you elaborate?

Yes, running the example on main is broken. If you click Play and then escape back to the menu, the sprite added for the game state is not despawned. After my change, it is properly despawned.

[nezuo]

I wrote this test in src/state/computed_state.rs if we want to use it:

#[cfg(test)]
mod tests {
    use crate::{
        app::{AppExtStates, StatesPlugin},
        prelude::DespawnOnEnterState,
        state::{ComputedStates, StateTransition},
    };
    use bevy_app::App;
    use bevy_ecs::component::Component;
    use bevy_state_macros::States;

    #[derive(Component)]
    struct TestComponent;

    #[derive(States, Default, PartialEq, Eq, Hash, Debug, Clone)]
    struct TestState;

    #[derive(PartialEq, Eq, Hash, Debug, Clone)]
    struct TestComputedState;

    impl ComputedStates for TestComputedState {
        type SourceStates = TestState;

        fn compute(_: Self::SourceStates) -> Option<Self> {
            Some(TestComputedState)
        }
    }

    #[test]
    fn computed_states_are_state_scoped_by_default() {
        let mut app = App::new();
        app.add_plugins(StatesPlugin);
        app.insert_state(TestState);
        app.add_computed_state::<TestComputedState>();

        let world = app.world_mut();
        world.spawn((DespawnOnEnterState(TestComputedState), TestComponent));

        world.query::<&TestComponent>().single(&world).unwrap();

        let world = app.world_mut();
        world.run_schedule(StateTransition);

        assert!(world.query::<&TestComponent>().single(&world).is_err());
    }
}

Let me know if I should include it or if the test should be placed somewhere else.

[alice-i-cecile]

Please do add that integration test :)

[hymm]

wouldn't it be better to add a associated const to ComputedState and then set the one on states equal to that?

impl<S: ComputedStates> States for S {
    const DEPENDENCY_DEPTH: usize = S::SourceStates::SET_DEPENDENCY_DEPTH + 1;

    const SCOPED_ENTITIES_ENABLED: bool = S::SCOPED_ENTITIES_ENABLED;
}

[alice-i-cecile]

wouldn't it be better to add a associated const to ComputedState and then set the one on states equal to that?

I'm in favor of removing this flag completely in the future, so I'd prefer the simplest approach for now.

[alice-i-cecile]

@nezuo my suggestion doesn't compile; could you correct it for me 😅

[alice-i-cecile]

Thanks :) I'm not a very good Rust compiler!