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

Dispatching events from moves #142

Closed
nicolodavis opened this issue Mar 12, 2018 · 5 comments
Closed

Dispatching events from moves #142

nicolodavis opened this issue Mar 12, 2018 · 5 comments
Assignees
Labels

Comments

@nicolodavis
Copy link
Member

nicolodavis commented Mar 12, 2018

There does not exist a direct way to dispatch an event like endTurn or endPhase from within a move currently. The way to do it at the moment is to use endTurnIf or endPhaseIf. This can lead to some annoying patterns like setting booleans whose only purpose is to trigger some code elsewhere:

moves: {
  moveA(G, ctx) {
    ...
    return { ...G, flag: true };
  }
},

flow: {
  endTurnIf(G, ctx) {
    return G.flag == true;
  }
}

Rather than that, it would be nice to directly invoke events like this:

moves: {
  moveA(G, ctx) {
    ...
    ctx.events.endTurn();
  }
}

This is analogous to how the Random code works currently. It accumulates some state that has to be applied to ctx, and it applies it at the end of the move (in this case, the turn does not end immediately, but right after the move instead). Note that the game reducer still stays pure.

This will also open up other opportunities to modify ctx from moves through a well-defined API (so that it's still framework managed and not directly editable).

EDIT: Updated after #146

@Stefan-Hanke
Copy link
Contributor

I'd like to point out that currently, FlowWithPhases is just one flow implementation. The framework allows to plug other flows by means of the Game method. I'm unsure whether givinig it specialized treatment via custom addtional API may reduce the utility of other flows.

This is just a thought, though. It looks like FlowWithPhases may be the standard flow that, say, 90% of all games will use. So, giving it specialized treatment would be ok. Eventually, one generic Events.trigger(eventName) function may suffice to enable other flows as well.

Just to name one alternative, we could disallow to plug other flow implementations and offer more than one standard one, should that ever prove necessary.

@Stefan-Hanke
Copy link
Contributor

For the initial API, I'd suggest the followng methods:

  • endTurn
  • endPhase
  • trigger(eventName)

Should we also include endGame? Personally I like the current approach, however for the sake of completeness, I'd add it.

I'm unsure I understood your last sentence. Could you give an example where direct modification of ctx is appropriate?

@nicolodavis
Copy link
Member Author

nicolodavis commented Mar 13, 2018

Sounds good.

  • Let's add endGame as well. It's not an event at the moment, but we can include it for consistency.
  • About the last sentence, I just meant that dispatching an event essentially modifies ctx. We don't want direct modification of ctx, which is why we provide an API through Events and Random to modify it.

@nicolodavis
Copy link
Member Author

About the special treatment of FlowWithPhases, I think it's ok as well. Let's still retain the ability for a user to provide a custom flow (in order to cover some rare case that we may not want to bundle in the library here). I'm hoping that most users will not need to implement a custom flow.

@nicolodavis
Copy link
Member Author

I realized that we don't need trigger because the flow implementation contract requires that you return eventNames.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants