[RFC] State Separation

[udoprog]

Here is the RFC to replace amethyst/amethyst#1199

Related to amethyst/amethyst#1152 (About the simplicity issue)

This started it's life as a working prototype implementation of what is being proposed.

Rendered

[torkleyy]

Something I wanted to bring up for a long time is that state is actually not something that should be limited to "one global state" if that makes sense. Let me elaborate:

There are of course high-level states of the game; these mostly represent different parts of the game (loading, main menu, in game, etc). However, there are actually multiple independent graphs for states in practice. Imagine things like animation, AI behaviour, player controls (on foot, driving, etc). Wouldn't it be nice if our states could achieve all that?

I've experimented with a generic state machine a while ago (machinae). However, there was some issue that made it impossible to use in certain situations. I can't recall what that was exactly. Just wanted to bring that up.

This also relates a little bit to the states vs systems issue and the thread on Discourse. I think I'll actually create a new thread about this. Link-

[udoprog]

AI behaviour, player controls (on foot, driving, etc). Wouldn't it be nice if our states could achieve all that?

It seems like you have one in mind, if you could outline a concrete example that would help.

There are currently two potential solutions that allow you to emulate the old architecture:

1. Use a custom state that has effectively non-finite values and build and destroy as many as you want/need dynamically (e.g. u64).
2. Maintain your own stack of "data" inside a single state callback instance, i.e. push to it on on_start, pop on on_stop.

I do find it now compelling to have a single instance because that is the use case I see being overrepresented in both the examples and games currently being built. So in the spirit of making the default thing easy, that is what I propose.

[udoprog]

I had some inspiration and realized this could support "complex states" quite easily through the derive:

#[derive(State, Debug, Clone, PartialEq, Eq, Hash, Default)]
struct Complex {
    name: String,
}

#[derive(State, Debug, Clone, PartialEq, Eq)]
enum State {
    A,
    Named(&'static str),
    Complex(Complex),
    StopIt,
}

The State trait had to be modified a bit with the type parameter T specifying what is stored in the state:

/// The trait associated with a state.
///
/// # Type Parameters
///
/// - `E`: The event associated with the state.
/// - `T`: The value stored in state storage.
///        This can be used to implement forward storage for other types, like when nesting states.
pub trait State<E, T = Box<dyn StateCallback<Self, E>>>: Clone + Default + fmt::Debug
where
    Self: Sized,
{
    /// The storage used for storing callbacks for the given state.
    type Storage: StateStorage<Self, T>;
}

Deriving State on a struct builds the following implementation:

impl<E, T> State<E, T> for Complex {
    type Storage = amethyst::MapStateStorage<Self, T>;
}

I've added an example for it here: https://github.com/udoprog/amethyst/blob/enum-state-machine/examples/states/main.rs

Or in other words: It is highly recommended to use the derive or one of the primitive types.

[AnneKitsune]

GameData was there to allow having shared dispatchers between states. How can we do this under this architecture?

[udoprog]

GameData was there to allow having shared dispatchers between states. How can we do this under this architecture?

My long term goal is to strive to provide the necessary architecture to only need one dispatcher.

That being said, we have global callbacks that can drive additional dispatchers depending on which value the state resource has. Something like this:

#[derive(State, Debug, Clone, Copy, PartialEq, Eq)]
pub enum State {
    First,
    Second,
}

struct DispatcherDriver<S>(S, Dispatcher<'static, 'static>);

impl<S, E> GlobalCallback<S, E> for DispatcherDriver<S>
where
    S: 'static + Send + Sync + PartialEq + Eq,
{
    fn update(&mut self, world: &mut World) -> Trans<S> {
        if self.0 == *world.read_resource::<S>() {
            self.1.dispatch(&world.res);
        }

        Trans::None
    }
}

Note have the above is:

• Modular, you can drive as many dispatchers as you want with this single impl.
• Generic over the the specific State it's running for.
• Only responsible for doing "one thing" in isolation, which is generally easier to document and teach.

EDIT: I've expanded the states example to include some global callbacks: https://github.com/udoprog/amethyst/blob/enum-state-machine/examples/states/main.rs

[azriel91]

Finally got here, and your design is excellent; thanks for going through that cray cray amethyst_test crate as well.

There are of course high-level states of the game; these mostly represent different parts of the game (loading, main menu, in game, etc). However, there are actually multiple independent graphs for states in practice. Imagine things like animation, AI behaviour, player controls (on foot, driving, etc). Wouldn't it be nice if our states could achieve all that?

I have a feeling that some of the methods on the StateCallback aren't always applicable to the lower level state machine, some example cases:

• entity being limited to only one state at a time, without stacking (on_resume, on_pause not applicable)
• state does an update based on the data in the world (pulled events) rather than pushed events (handle_event not applicable). sounds like I'm talking about a System rather than a State
• not sure if both update and fixed_update are always applicable

That said, does leaving a trait method empty for all implementations get optimized out? If so, it wouldn't hurt at runtime, perhaps there's a little confusion at development time if people look at docs and ask "what about these trait methods".

---

I haven't understood the global state callback use case + complex states yet, shall take another look later. Removing GameData makes sense given we can use a resource to determine if a System should run. And, I think it's possible with the SystemExt trait, as well as the previous comment, but I'd like the ability to specify the logic for "this group of systems should run in this State" in the same place, which is separate from the system's own logic.

[udoprog]

Hello,

I'm back, and I've rebased the refactored the prototype on top of the latest master (thanks @torkleyy for changing every state implementation :D).

It would help if people could outline any concerns that they still have, and I can try and address them to get this RFC rolling again.

Thank you!

[AnneKitsune]

Here's my proposal for a more data-driven pushdown FSM

Should I open a separate rfc or should this be discussed here?

Short explanation: Instead of deciding at runtime which event triggers which state transition, they are all determined in advanced as data.

For example, the MyEvent::Play user event triggers a transition from StateA to StateB using Push. In the State, you can just return Option<MyEvent> instead of the complex Trans type.

This only works if states aren't depending on runtime data (same as this PR).

It would fit well within this PR I think. Systems could trigger the state transitions by emitting the events into the TransCallback (which would now accept the MyEvent user type, and not Trans). Events would only have an effect if you are in the starting state of the transition (A -> B, you need to be in A to go to B). This means that Systems wouldn't need to be aware of which state you are currently in, while still being able to trigger state transitions.

This solves my concern with this pr, and should answer what @Xaeroxe asked.

Also we have way less Box like this :D

[azriel91]

In the proper FSM part in Jojo's comment, sometimes we want state C to jump straight back to state A; sometimes we want it to pop to state B. For example, "VS mode", "choose characters", "match statistics". In the match statistics State, the player(s) may go, "return to main menu" or "go back to character selection". Simply using Trans::Switches feels simpler, instead of figuring out how to double pop – probably because it's easier to grasp.

Disclaimer: Random brain dump, just to get the idea written down

That spawned another thought: could the FSM transitions be type parameterized, such that:

• The FSM is defined at the beginning, with transitions specified as "this event type signals transition to this other state".
• By default, the event type is the State enum itself,
• Consumers can specify "actually, use this custom event type".

Defining the FSM:

• State enum – List of States.

• Derive a type for each of the variants. Each of these types will implement the following traits:

  • trait StateHandler<T>, where T indicates the type of event that a State can use to transition
  • trait StateTransitions<T>. A given StateHandler can only return T, and each value of T points to a State to transition to.

  Hoping the T can be different for each state variant type – this means you can specify "StateA can transition to StateB", StateB can transition to StateA and StateC", though this isn't a strong requirement.

• Fsm type to glue all of the states and transitions together.

There's probably something wrong in there since I just woke up, haha.

[Xaeroxe]

@jojolepro @azriel91 That's an interesting idea, however I'm concerned it adds too much complexity to the implementation for the benefit it gives.

If we're to make a distinction between "code" and "data" then I would argue States and their transitions are very firmly in the camp of "code". I don't feel a data driven approach for states gives us many benefits, and it mostly seems to add complexity and increase maintenance costs for both engine and game developers.

It could possibly give us a tiny squeak of performance boost from not having to use Box but I'm not convinced at this time that such a boost is worth the maintenance overhead that introduces.

[azriel91]

That's an interesting idea, however I'm concerned it adds too much complexity to the implementation for the benefit it gives.

Gotcha, am already super happy with the current proposed mechanism (and sometimes hope that we one day reach stability 📆). Also, I probably get too caught up in things that we could do, and don't stop enough to think if it's something we should 😅.

[udoprog]

@jojolepro @azriel91

Should I open a separate rfc or should this be discussed here?

If I'm reading this correctly, your goals are to only allow a certain set of "legal" state transitions driven by events?

In that case this does seem like something we should punt until a later RFC since it adds another step of indirection to and fundamentally changes (limits?) how state transitions work. If anything I'd like proposals how to simplify this without sacrificing the separation it tries to accomplish.

[azriel91]

If I'm reading this correctly, your goals are to only allow a certain set of "legal" state transitions driven by events?

yeap that was one of the goals I had. The other one was to be able to separate "what state am I transitioning to" out of the state handler, into "I'm done!" or "The user cancelled", and let something outside decide what state it should transition to (data vs code like Xaeroxe said).

[udoprog]

So I've discovered another way to describe why this change has a positive impact on how Amethyst applications are modeled with the current state machine.

Many problems can be phrased as "I want to do x when the application is in a given state" - if you've been around for a while this seems to be a very common request in #help. Whether that is drive a dispatcher or have some custom logic that runs for a specific state.

In order to do something for a specific state today, we are required to add that something to a single large State implementation. This leads to the state implementation being rather complex, and to push back against this complexity we've done things like SimpleState.

Forcing us to name states, and allowing us to associate specific pieces of logic with this name makes this cleaner.

• Systems can read the resource that tells it which state it is currently in.
• State handlers can be more modular and do one thing well, since it doesn't have to be able to do all the things we need a state handler to do today*.

*: Ultimately this might even be an argument for allowing associating multiple handlers to any one state, but I'm not gonna push for that right now.

[torkleyy]

Systems can read the resource that tells it which state it is currently in.

That would, however, make systems dependent on the State. The big problem with that is that it doesn't compose well; systems should be stateless.

I think however, there is a need for state specific functionality that doesn't just get thrown into the state impl.

[torkleyy]

One thing I'd also like to bring up is that I don't really like our current Application because while it has a bunch of useful features, it means you have to give up control of how your game is ran.

[udoprog]

Systems can read the resource that tells it which state it is currently in.

That would, however, make systems dependent on the State. The big problem with that is that it doesn't compose well; systems should be stateless.

Consider systems which are generic over the state. For example:

struct Example(S);

impl<'a, S> System<'a> for Example<S> where S: State + PartialEq {
  type SystemData = Read<'a, S>;

  /* ... */
}

This can do things with any state S, without dictating exactly which state it is. In that sense it composes rather nicely.

One thing I'd also like to bring up is that I don't really like our current Application because while it has a bunch of useful features, it means you have to give up control of how your game is ran.

I think many of the things in Application could in principle be refactored into distinct GlobalHandler implementations. But I'm gonna suggest punting on this for now as out of scope for this RFC.

[Rhuagh]

GameData was there to allow having shared dispatchers between states. How can we do this under this architecture?

This is not exactly true, the main reason for GameData is to be able to share data between states that for various reasons can't be put into World.

[Rhuagh]

I've experimented with a generic state machine a while ago (machinae). However, there was some issue that made it impossible to use in certain situations. I can't recall what that was exactly. Just wanted to bring that up.

The problem was that it was impossible to get the lifetimes to work for amethyst's specific use case of DynState and &mut data.