More flexible computed states

[Azorlogh]

Objective

• Currently, if a plugin author wants to define states that are meant to be computed from a user's own state, it will be impossible, because of the orphan rule.

Solution

• This PR changes the computed state mechanism to use free functions that we register on the app, instead of a trait.

#[derive(Debug, Clone, Copy, Eq, PartialEq, Hash, States)]
#[computed]
struct InGame;

fn compute_in_game(sources: AppState) -> Option<InGame> {
    match sources {
        AppState::InGame { .. } => Some(InGame),
        _ => None,
    }
}

App::new()
    .add_state_computation(compute_in_game)

Testing

• I tested these changes using the computed states test suite that I adapted to the changes.

TODO

• [ ] I couldn't find SET_DEPENDENCY_DEPTH used anywhere 🤔. Its docs say it helps order transitions & prevent cyclical dependencies. For testing I tried manually creating a cyclical dependency and the only error I got was the ECS system scheduler Error when initializing schedule StateTransition: System dependencies contain cycle(s).

~~- [ ] I didn't figure out how to make the derive attribute #[states(computed)] instead of just #[computed]~~ https://github.com/bevyengine/bevy/pull/13948#issuecomment-2182424741

---

Changelog

Changed

• Changed the computed states feature to use a free function for computation instead of a method on the ComputedStates trait.

Migration Guide

• Computed states no longer require implementing the ComputedStates manually. Instead, you should use the #[computed] macro attribute and directly pass the computation function to the App.

// 0.14

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

impl ComputedStates for InGame {
    type SourceStates = AppState;

    fn compute(sources: AppState) -> Option<Self> {
        match sources {
            AppState::InGame { .. } => Some(InGame),
            _ => None
        }
    }
}

App::new()
    .init_state::<AppState>()
    .add_computed_state::<InGame>();

// 0.15

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

fn compute_in_game(sources: AppState) {
    match sources {
        AppState::InGame { .. } => Some(InGame),
        _ => None
    }
}

App::new()
    .init_state::<AppState>()
    .add_state_computation(compute_in_game)

[alice-i-cecile]

FYI @lee-orr @MiniaczQ @benfrankel :)

[benfrankel]

You can check out how the derive macro in pyri_state works for reference: https://github.com/benfrankel/pyri_state/blob/main/derive/src/lib.rs (namely fn derive_state and fn parse_state_attrs).

[benfrankel]

Some preliminary questions:

1. Is the ComputedStates trait still needed after this change?
2. Could this approach be applied for SubStates as well?

[Azorlogh]

Some preliminary questions:

1. Is the `ComputedStates` trait still needed after this change?

2. Could this approach be applied for `SubStates` as well?

1. I think the value is in preventing the user from calling AppExtStates::add_state_computation with a non-computed state (that implements FreelyMutableState). There's nothing actually preventing someone from implementing FreelyMutableState and ComputedState at the same time, but at least the derive macro will steer users toward not doing that.

2. I think that could work too. But I'm not sure how to preserve the nice macro attribute syntax? Would it create a regular method on the state, which you then pass into AppExtStates::add_sub_state? I think that might work, but I'm not sure if there's downsides.

#[derive(SubStates, Clone, PartialEq, Eq, Hash, Debug, Default)]
#[source(AppState = AppState::InGame)]
enum GamePhase {
    #[default]
    Setup,
    Battle,
    Conclusion
}

App::new()
    .add_sub_state(GamePhase::should_exist)

[Azorlogh]

I didn't figure out how to make the derive attribute #[states(computed)] instead of just #[computed]

Actually I'm not sure what's best between #[computed] and #[states(computed)]. On one hand the latter feels more "scoped" but on the other hand, the substates attribute's syntax is #[source(AppState = AppState::InGame)] which doesn't have that prefix So I'll probably leave it like that unless someone wants me to change it

[Azorlogh]

I updated the documentation on the trait definition. For the PR description, I'm not sure what should go in the Changelog & Migration sections. Should I describe the change as if updating from 0.13 to 0.14, or from 0.14.0-rc.3 to 0.14.0?

[alice-i-cecile]

As if migrating from 0.14 to 0.15 please :) This won't be in 0.14, and migration guides should be written to cover the span between released Bevy versions.

[benfrankel]

I'm in favor of this feature (in fact I support the same thing in my own states crate). However, my preferred implementation would require some bigger changes to bevy_state.

There's a Discord thread documenting high-level proposals for bevy_state 0.15, so this feature may get in as part of that work but in a different form. I'm holding off on reviewing this PR for that reason.