Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

book: how to interact between states and systems using resources #1154

Merged
merged 8 commits into from Nov 24, 2018

Conversation

@udoprog
Copy link
Member

commented Nov 15, 2018

I promised folks during an Amethyst presentation I did in Gothenburg to improve documentation around the pain points I encountered while building a test game in Amethyst.

This adds concrete documentation for how to actually implement the state transitions hinted at during the State section

I'm also actively working on trying to improve the ergonomics around this approach, so far the only relevant issue is: #1146 - but in general I'm trying to create a stronger association between a group of states and systems. If this becomes relevant, I'll update the documentation to reflect it :).

The other approach I'm aware of to handle this would be to set up event channels. But that section doesn't really talk about how to set up readers in states (it probably should!). Event channels is however a bit more complicates since the reader needs to be setup.

Future TODOs?

Add concrete documentation for input handling - I mention adding an action called open_menu, but there's no great page to link to that discusses input handling generically, apart from pong-tutorial-03.md.
It would be great if we had a canonical section that dealt with only input handling in particular!

@jojolepro
Copy link
Member

left a comment

Thanks, but the content isn't actually true. I explained the proper way to handle state transitions in the following comments.

Note that there's a secret EventChannel<TransEvent> that you can push Trans into. However, it should only be used for automation, and not by users, because it can be a huge source of bug and confusion that can happen when improperly used.

This is the reason it isn't mentioned in the book. If you do want to mention it, it should be in an "advanced" section with a big disclaimer.

Thanks for checking this out :)

impl<'s> System<'s> for MyFirstSystem {
type SystemData = (
Read<'s, InputHandler<String, String>>,
Write<'s, Gameplay>,

This comment has been minimized.

Copy link
@jojolepro

jojolepro Nov 16, 2018

Member

Also, what you can do is react to InputEvent<String> events in the state's handle method. Those are the events emitted by the InputHandler. The strings you get is the key used in your input configuration (actions).

Note: We need to add InputEvent<T = String> to the default StateEvent of amethyst for events to actually be forwarded to the State::handle_event method. I opened an issue for this yesterday as a reminder.

This comment has been minimized.

Copy link
@udoprog

udoprog Nov 16, 2018

Author Member

This does cover state transitions caused immediately by keyboard inputs, but not by other things, like the player dying because of a collision.

Honestly though, it seemed a bit off to me to handle input events both in states and in systems. I'd much rather have a clear signal to a new user to do it one way or another. I'd love more input on this though.

In a previous section we talked about [`States`][s], and how they are used to organize your game
into different logical sections.
Since systems are responsible for processing user input, they must somehow also be responsible for
causing a switch in states.

This comment has been minimized.

Copy link
@jojolepro

jojolepro Nov 16, 2018

Member

pssst, that isn't exactly true. The best way to do it is to have systems only to data processing (change components, modify resources, render things). When you have systems that manage states, they get too specialized and less re-usable.

The best practice for those case would be to emit an event specific to your game from the System. For example, a "TogglePause" event. Then, inside of your state's handle_event method, you can react to this event (in this case, by returning a Trans when you see the "TogglePause" event).

This comment has been minimized.

Copy link
@udoprog

udoprog Nov 16, 2018

Author Member

To me it seems like a system is still seems to be responsible for the state change? How would you like the phrasing to change?

The best practice for those case would be to emit an event specific to your game from the System. For example, a "TogglePause" event. Then, inside of your state's handle_event method, you can react to this event (in this case, by returning a Trans when you see the "TogglePause" event).

The reason why modifying a resource instead of an event came up as a simpler alternative is that you can still use SimpleState. Deviating from SimpleState is kinda hard in itself :/, but probably worth covering somewhere.

This comment has been minimized.

Copy link
@torkleyy

torkleyy Nov 20, 2018

Member

This is an interesting discussion. In any case, we can currently process input in both location, so I think this statement can be confusing.

This comment has been minimized.

Copy link
@udoprog

udoprog Nov 20, 2018

Author Member

I added some clarification with an example. Please check it out!

@udoprog

This comment has been minimized.

Copy link
Member Author

commented Nov 16, 2018

@jojolepro thanks for the review! <3

@azriel91
Copy link
Member

left a comment

Hiya! Thanks for writing an example on how to use resources to communicate with states.

The writing style is good; the example has a number of things that can be improved:

  • It sounds as though MainMenuState is below the GameplayState in the stack, but the code has the GameplayState as the bottom state.

  • Design wise, I'm still undecided between two options:

    1. Instead of having a global resource that all states have knowledge of, use a State specific status resource, and each state queries its own status type. That way, we move away from the "God state" and towards clearer data segregation. This would encourage newcomers to follow that pattern, and I think it would save them maintenance in the long run.

      But this is an example, so creating a fully fledged design can overwhelm readers.

    2. Clean up how the states and system interact with the Game resource. Reading the flag before setting it is an idempotence check which makes the application function how you'd expect it, but breaks the mental flow (or at least, it breaks mine 😅), which means the example becomes harder to understand.

      Perhaps we should work out how to cleanly implement this one.

Let's see what the others think about the code example.


Must end this with a great work!

Since systems are responsible for processing user input, they must somehow also be responsible for
causing a switch in states.
So how can we effect states from systems?

This comment has been minimized.

Copy link
@azriel91

azriel91 Nov 16, 2018

Member

effect -> affect

This comment has been minimized.

Copy link
@udoprog

udoprog Nov 16, 2018

Author Member

Thanks, fixed!

Let's say you have the following two states:
* `MainMenuState` - which starts the game by transitioning into the GameplayState.
* `GameplayState` - which we enter after the main menu, while the game is running.

This comment has been minimized.

Copy link
@azriel91

azriel91 Nov 16, 2018

Member

Hi 👋 I'm a pedant comment 🗒. Please could you use this style:

* `MainMenuState`: Starts the game by transitioning into the `GameplayState`.
* `GameplayState`: State in which the game is running.

Changes:

  • No leading space for bullet points.
  • Colon instead of hyphen before explanation – to allow hyphens within the explanation if necessary.
  • Capitalized declarative sentences. This allows explanations with multiple sentences to appear consistent.

Yours pedantically,

Pedant

p/s: Nice content 💯

This comment has been minimized.

Copy link
@udoprog

udoprog Nov 16, 2018

Author Member

Done!

```
Now whenever you are playing the game and you press the button associated with the `open_menu`
action, the `MenuState` will resume and the the `GameplayState` will pause.

This comment has been minimized.

Copy link
@azriel91

azriel91 Nov 16, 2018

Member

missing a Main here

This comment has been minimized.

Copy link
@udoprog

udoprog Nov 16, 2018

Author Member

Thanks, I ended up renaming this, but this line also had a typo (the the) which I missed.

@udoprog

This comment has been minimized.

Copy link
Member Author

commented Nov 16, 2018

@azriel91

It sounds as though MainMenuState is below the GameplayState in the stack, but the code has the GameplayState as the bottom state.

I renamed the MainMenuState to GameMenuState, as it was intended.

Design wise, I'm still undecided between two options: /* */

Right now I want to make sure we capture some status quo in the book that allows people to use the engine more independently. As it evolves, we either find or build better ways to do this, the documentation should be updated to reflect that.

Must end this with a great work!

Thank you for the review! <3

@jojolepro

This comment has been minimized.

Copy link
Member

commented Nov 16, 2018

image

I made a "pretty" drawing of how I mentally think about the ECS.

Of course, you can have Systems that are aware of the State, but in all cases you'll end up with game-specific systems that cannot be re-used. If you have a System that listens for a "PauseKey" input event, and emit a "PauseGame" event, you can reuse that system in all your games that know about the "PauseGame" event.

Also, you wouldn't want a system doing a State Trans to PauseMenu while you are inside of the MainMenu. To prevent that, you'd need pausable systems. So now, in addition to your State's current State, you end up also managing which Systems are running and which aren't, which is more maintenance.

I hope I was able to explain that correctly.

@udoprog

This comment has been minimized.

Copy link
Member Author

commented Nov 17, 2018

I'll reiterate some things that were discussed over Discord. Sorry if I miss things.

It's important to emphasize that this PR is not the do-all, end-all way to trigger state transitions.
If another suggestion or new pattern emerges tomorrow which is superior at this stage of an Amethyst developer's journey, then I'd personally want to make sure that gets into the book.
For now I'd encourage us to at least include something and not letting perfect getting in the way of good.

I do believe that triggering state transitions through resources is actually not that bad all things considered.

There is probably a better way to trigger state transitions

  • We can capture input events directly in SimpleState after #1148 is implemented. This does however not handle non-input based state transitions (e.g. player dying because their health ran out).
  • The correct way to communicate events is to implement handle_event. This does however require the reader to deviate from SimpleState, which is a big task at this stage of the documentation.

There was concerns raised about coupling systems to states

That would be the Game resource mentioned above, and the CurrentState enum.
The Game resource acts as a bridge between states and systems, but it's important to note that it is a declared data dependency (Write<'s, Game>), just like any other resource in the ECS. That means it doesn't care where the resource originates from.

We do maintain the value of the CurrentState enum. This is intended to implement conditional logic in systems, and is something I eventually want to solve better with #1146 (pausable systems).

@udoprog

This comment has been minimized.

Copy link
Member Author

commented Nov 20, 2018

@torkleyy
Copy link
Member

left a comment

Other than the comment, I like it! Thanks for this PR 👍

In a previous section we talked about [`States`][s], and how they are used to organize your game
into different logical sections.
Since systems are responsible for processing user input, they must somehow also be responsible for
causing a switch in states.

This comment has been minimized.

Copy link
@torkleyy

torkleyy Nov 20, 2018

Member

This is an interesting discussion. In any case, we can currently process input in both location, so I think this statement can be confusing.

@torkleyy
Copy link
Member

left a comment

LGTM!

@jojolepro

This comment has been minimized.

Copy link
Member

commented Nov 20, 2018

Moxi wants to review

@Moxinilian
Copy link
Member

left a comment

Thank you!
Are you sure the chapter on systems is the best place to put this? It's already one of the longest, and you cover stuff that goes beyond systems. But I can't think of a better place either, so I guess it's good.

> A [`Resource`][r] is any type that stores data that you might need for your game AND that is not
> specific to an entity.
The data in a resource is available both to systems and resources.

This comment has been minimized.

Copy link
@Moxinilian

Moxinilian Nov 21, 2018

Member

What does that mean?

This comment has been minimized.

Copy link
@udoprog

udoprog Nov 21, 2018

Author Member

Typo, s/resources/states. Thanks!

The following example shows how to keep track of which state we are currently in.
This allows us to do a bit of conditional logic in our systems to determine what to do depending on
which state is currently active. And manipulating the states by setting flags:

This comment has been minimized.

Copy link
@Moxinilian

Moxinilian Nov 21, 2018

Member

", and"

This comment has been minimized.

Copy link
@udoprog

udoprog Nov 21, 2018

Author Member

Sentence would become a bit long. Do you think this is an issue?

This comment has been minimized.

Copy link
@Moxinilian

Moxinilian Nov 21, 2018

Member

Nah I think it will be fine

This comment has been minimized.

Copy link
@udoprog

udoprog Nov 21, 2018

Author Member

Ok, done!

@udoprog udoprog force-pushed the udoprog:book branch from cd6d110 to 697fc81 Nov 21, 2018

@Moxinilian
Copy link
Member

left a comment

LGTM!
@jojolepro everything is ready if I was holding your green checkmark back

Outvoted, didnt rereview yet

@torkleyy torkleyy requested a review from azriel91 Nov 22, 2018

@torkleyy

This comment has been minimized.

Copy link
Member

commented Nov 22, 2018

This should go into 0.10, please approve or request further changes.

@azriel91
Copy link
Member

left a comment

Happy with the wording, I did change the code around to use an enum instead of booleans, see azriel91@b407a9c. Github isn't letting me PR into the PR.

Essentially I just changed the boolean flags with an enum, I guess manually upholding the state invariant felt like a footgun

@udoprog udoprog force-pushed the udoprog:book branch from 4e6e42c to cf473b9 Nov 23, 2018

@udoprog

This comment has been minimized.

Copy link
Member Author

commented Nov 23, 2018

@azriel91 applied your change, thanks!

@torkleyy
Copy link
Member

left a comment

Thanks!

bors=Moxinilian,azriel91,torkleyy

@Moxinilian

This comment has been minimized.

Copy link
Member

commented Nov 24, 2018

Seems like bors was tired.

bors=Moxinilian,azriel91,torkleyy

@Moxinilian

This comment has been minimized.

Copy link
Member

commented Nov 24, 2018

Oh we're stupid.

bors r=Moxinilian,azriel91,torkleyy

bors bot added a commit that referenced this pull request Nov 24, 2018

Merge #1154
1154: book: how to interact between states and systems using resources r=Moxinilian,azriel91,torkleyy a=udoprog

I promised folks during an Amethyst presentation I did in Gothenburg to improve documentation around the pain points I encountered while building [a test game](https://github.com/udoprog/asteroids-amethyst) in Amethyst.

This adds concrete documentation for how to actually implement the state transitions hinted at during the `State` section

I'm also actively working on trying to improve the ergonomics around this approach, so far the only relevant issue is: #1146 - but in general I'm trying to create a stronger association between a group of states and systems. If this becomes relevant, I'll update the documentation to reflect it :).

The other approach I'm aware of to handle this would be to set up event channels. But that section doesn't really talk about how to set up readers in states (it probably should!). Event channels is however a bit more complicates since the reader needs to be setup.

## Future TODOs?

Add concrete documentation for input handling - I mention adding an action called `open_menu`, but there's no great page to link to that discusses input handling generically, apart from [`pong-tutorial-03.md`](https://github.com/amethyst/amethyst/blob/master/book/src/pong-tutorial/pong-tutorial-03.md).
It would be great if we had a canonical section that dealt with only input handling in particular!

Co-authored-by: John-John Tedro <udoprog@tedro.se>
Co-authored-by: Azriel Hoh <azriel91@gmail.com>
@bors

This comment has been minimized.

Copy link
Contributor

commented Nov 24, 2018

@bors bors bot merged commit cf473b9 into amethyst:master Nov 24, 2018

4 checks passed

bors Build succeeded
Details
concourse-ci/linux-book-tests Concourse CI build success
Details
concourse-ci/linux-tests Concourse CI build success
Details
concourse-ci/rustfmt Concourse CI build success
Details

@marot marot referenced this pull request May 1, 2019

Merged

Add pause button #38

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.