Add OnTransition::Any that runs on any change to the state S

[jannik4]

Objective

I want to be able to run a system in between OnExit and OnEnter that runs on any change to the state. This can be useful if, for example, you want to cover all the possibilities of the transition with a match statement.

Solution

1. Replace the OnTransition { from, to } schedule with an enum:

enum OnTransition {
    Any,
    Exact { from, to },
}

2. Add the ActiveTransition resource that is made available by the apply_state_transition system during the transition. This can be used to access the exiting and entering states.

Open Questions

• Currently the ActiveTransition resource is made available to all of OnExit/OnTransition/OnEnter. Maybe it should only be made available for OnTransition? It is now only available to the OnTransition schedules.

Changelog

• Replace OnTransition { from, to } struct with OnTransition { Any, Exact { from, to } } enum
• Add ActiveTransition resource

Migration Guide

// old
OnTransition { from: MyState::A, to: MyState::B }

// new
OnTransition::Exact { from: MyState::A, to: MyState::B }

[alice-i-cecile]

@lee-orr, following your work in #10088 I'm really curious about what you think of this design.

I'm fond of the simple incremental change. Still forming an opinion on the temporary resource 🤔 I think maybe it could be replaced by just looking at the current and next state resource that already exists?

[jannik4]

Yeah, I am also not 100% sure about the temporary resource. But I think it is not really possible with the NextState<S> resource, as this contains an Option<S>, so users would always have to unwrap.

If I am not missing anything, at least it's quite clean, because nobody can create, clone or modify the StateTransition resource from outside. So misuse is difficult/not really possible.

[alice-i-cecile]

Yeah, very nice encapsulation :) I think I'm on board with it now.

[lee-orr]

I think the design makes sense. There is actually a similar-ish temporary resource in #10088 - the ActiveTransition resource, which uses a tuple rather than being based on two fields. The only comment I'd have here is that it might be worth agreeing on an API for this so that we don't run into the need to change things later?

An option for that is to have the state transition resource expose a "from" and "to" function rather than methods, since that will make it relatively easy to replace ActiveTransition with it in a way that preserves ActiveTransition's additional functionality (the ability to swap ordering for some transition evaluation things).

[jannik4]

I definitely like the ActiveTransition name better. But I am not sure I can follow you here:

"from" and "to" function rather than methods

What exactly do you mean by this?

---

Answering my own question, whether the resource should only be made available for OnTransition:

I think yes, because

1. OnEnter/OnExit systems should only care about entering/exiting. Otherwise they should be in the OnTransition schedule in the first place.
2. Otherwise, to be consistent, the resource should also be available in run_enter_schedule where there is no reasonable from state.

[lee-orr]

Sorry! I meant methods rather than fields... I.e. abstract away how things are stored internally.

[lee-orr]

As for your second point - I think it might be better to have it present for the entire transition, but make the values Options.

This could allow a janky version of matching on enter/exit as well, where you check the resource for the outgoing/incoming value respectively in the system you're running.

[jannik4]

Okay, so for the methods vs fields: The fields are already private, so the methods are the only way to access them.

Regarding the ActiveTransition resource, can you explain what fields would then be set to Some/None for which schedule.
The one thing I really dislike about Options is that you would have to unwrap them in the OnTransition schedules. This is the point at which the resource would be most likely be used, and you would always need to unwrap guaranteed Somes.
And as I have already said, I think if you need the other state in your system, you should probably be in the OnTransition schedules and not in OnEnter/OnExit anyways.

Please feel free to disagree, it's just the way I currently see it:)

[lee-orr]

I disagree there, primarily on grounds of making the option explicit - I don't want people to find things crashing if they try to use a system for both OnTransition and OnEnter, for example - and OnEnter also runs the first time.

Another option is to default to having it be a transition where both states are identical if it's the first entrance/exit?

[jannik4]

Can you give me an example where that would make sense? For me, the two are actually so different that I can't imagine what I would reuse.

The way I see it, OnEnter/OnExit are the standard choice to handle changing between different states of your app.
OnTransition only makes sense if you want to "prepare" something between Exit-Enter depending on the transition.

I think the root problem for providing the ActiveTransition resource to all schedules is the OnEnter run on the first time. There is simply no sensible previous state. I would also find it very unintuitive to simply use the same one. But then forcing an Option everywhere else doesn't feel right either.

[lee-orr]

OK - sorry, it took me a bit to respond...

I don't know why I was under the impression the resource exists during OnEnter/OnExit as well... It may have just been mis-remembering since that's what happens in #10088.

I think my concern really comes from how ActiveTransition from this PR would interact with the version in #10088. In that case it felt like the appropriate way to handle things was having an ActiveTransition object that exists across the entire transition rather than just during the "OnTransition" portion(I'll put fuller details of the reasoning in that PR below if you're interested, but it doesn't directly affect this one).

Given the currently available API's and what you are proposing, I think you are probably right. And since I don't know if/when #10088 will get merged, maybe we don't need to actually plan for a compatible API.

---

If you're interested - the reasons #10088 exposes ActiveTransition across all 3 transition schedules are:

• When you are matching more than a single, discreet state, you might only want to match if the other state in the transition isn't part of the you are matching.
  • the classic example is App::InGame::Running and App::InGame::Paused - there are actions I want to take whenever I enter App:::InGame::*, like spawning the world, regardless of which it is - but only if we're not already in one of the matching states. Other things, like clearing the UI, I might want to do whenever I exit one of these states regardless of whether I'm moving to another matching state - since the HUD doesn't need to be visible while paused and vice versa.
• The other reason is a bit weirder, but due to some ergonomic constraints that PR has untyped variations of OnEnter/OnExit (Entering and Exiting) and actually swaps the order of states in ActiveTransition between the two. This is primarily to allow one of the following: avoiding the need to declare the types in multiple places, allowing one state type to react to a transition in another state type, and allowing you to re-use types that match a transition in both directions. A "Transitioning" schedule used to exist as well, but it felt redundant since "Entering" and "Exiting" basically had the same impact - only with inverted matching.

One possible API compromise is doing something like this:

// This code is not actually valid or fully formed, it's just expressing the idea

ActiveTransition<S> {
 main: S,
 secondary: Option<S>,
 direction: TransitionDirection
}

enum TransitionDirection {
  Entering,
  Exiting
}

impl<S> ActiveTransition<S> {
  fn entering(&self) -> S {
     match self.direction {
      Entering => self.main,
      Exiting => self.secondary.unwrap_or(self.main)
     }
  }

  fn exiting(&self) -> S {
    match self.direction {
    Entering => self.secondary.unwrap_or(self.main),
    Exiting => self.main
    }
  }
}

[jannik4]

Ok, thanks for the feedback.
I updated the PR description. From my point of view this is good to go. @alice-i-cecile @lee-orr

---

I also took a quick look at #10088: To be honest, it does offer some new/powerful possibilities, but it seems very complex to me and also has a lot of subtle cleverness hidden in it.

[lee-orr]

@jannik4 @alice-i-cecile - Should be able to close this at this point I think?