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

Ability to change currentPlayer without ending turn #141

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

Ability to change currentPlayer without ending turn #141

nicolodavis opened this issue Mar 12, 2018 · 27 comments
Labels

Comments

@nicolodavis
Copy link
Member

There is currently no way to change the current player without ending the turn. However, sometimes you might need other players to execute an action as part of your turn.

@janosimas
Copy link
Contributor

Maybe create the concept of a sub-turn, phases could have possible sub-turns as they have moves

@nicolodavis
Copy link
Member Author

nicolodavis commented Mar 12, 2018

If we can do it without an additional concept, that would be even better, though. It's less cognitive overload when you only have to think about phases, turns and moves. A move is already essentially a sub-turn. Right now all the moves in a turn have to be made by the same player. This feature request will enable a turn to contain moves from different players.

@janosimas
Copy link
Contributor

This way it will also need a way of blocking and requesting moves from other players.

@Stefan-Hanke
Copy link
Contributor

We could have an active player that is the owner of the current turn, and a action player that is supposed to make a move. It would be beneficial to allow multiple action players. The logic for this should reside in Flow or the Game itself s.t. games can change this behaviour.

@nicolodavis
Copy link
Member Author

I like that idea.

@nicolodavis
Copy link
Member Author

I also imagine that #142 will be relevant here.

@Stefan-Hanke
Copy link
Contributor

We'd have to modifiy the server, where the behaviour (any player etc.) is currently hard coded. At that point, game is available, and thus the flow, so we could move that behaviour to a flow method?

@philihp
Copy link
Contributor

philihp commented Mar 12, 2018

So the currentPlayer and the activePlayer are different things, yeah? And by default they're always the same, but the currentPlayer is the owner of the current turn, while the activePlayer is the person with the initiative, and the game is awaiting their move?

I like it.

@Stefan-Hanke
Copy link
Contributor

I'd like to rename currentPlayer to turnOwner. The intention is to reduce potential confusion with the player that is supposed to make a move. Since this is a breaking change, we could defer that for now. Thoughts?

@philihp good idea about the default state of activePlayer. As stated above, when modifying this, we could just as well allow multiple active players, to enable games that require action from multiple "other" players. The default then is an array with only the currentPlayer.

@nicolodavis
Copy link
Member Author

Yes, let's add no additional burden to users that don't care about separating these two concepts, i.e. activePlayer should default to an array with currentPlayer (turnOwner) in it.

Don't worry about making a breaking change. This is still an alpha release after all. I'll just document what's breaking in the changelog when I do a release.

@rzulian
Copy link
Contributor

rzulian commented Mar 13, 2018

@Stefan-Hanke not sure if the proposed solution can manage a usual board game turn scenario like this:
In a phase with TurnOrder.skip, in my turn a make a move ("Build mine"). Before I can do other moves, every other player who is satisfiyng some conditions, in the same turn order, can decide do do or not do a single specific move (Leech power). After every one passed, I will continue my turn.

Seems to be more a phase into an existing phase, but you have to retain the main phase passOrder, playerOrder and currentPlayer.

@nicolodavis
Copy link
Member Author

nicolodavis commented Mar 13, 2018

@rzulian The scenario you describe can be modelled like this:

Phase A: Build mine
Phase B: Everyone gets a chance to "Leech power"

On your turn, as soon as you build a mine in Phase A, the game moves to Phase B, and then reverts back to Phase A once everyone has had their chance. The turn does not end even if the phase gets passed around, so all of this happens in the context of a single turn (assuming that we just rotate the actionPlayer in Phase B).

@rzulian
Copy link
Contributor

rzulian commented Mar 13, 2018

Ok, but we need to store Phase A status (currentPlayer, playerOrder and passOrder), so you can run phase B with its properties and the restart from when you were. Or am I missing something of the proposed actionPlayer implementation?

@Stefan-Hanke
Copy link
Contributor

@rzulian I am referring to your scenario from above.

  • There is one action player that is building something.
  • Now, since the game knows there could be Leech powers (I think e.g. one of the games in the Tiny Epic series allowed that), this is checked for. Using the yet-to-come Events API, the game sets the action players that may or may not perform that specific move.
  • Now, when the last player did their move, the old action player is in control again.

I'd like to get terminology straight. For me, this is on a turn level, not on the phase level. A phase consists of turns, but not the other way round. This is reflected inside FlowWithPhases currently.

The proposed solution is not the be-all-and-end-all of flow-related problems. The framework had mechanisms in place to disallow players to make moves when they are not the current player. The PR just relaxes this by allowing the game to state that other players are in control now, inside of a single turn of the current player. Nothing more, nothing less.

@rzulian
Copy link
Contributor

rzulian commented Mar 13, 2018

@Stefan-Hanke ok, understood. So currentPlayer (turnOwner) it's only about who is in charge of the current turn. Usually activePlayer is the same user, but you can modify the activePlayer through Events, controlling the available moves through canMakeMoves, but still in the same turn. Right?

@janosimas
Copy link
Contributor

A phase consists of turns, but not the other way round.

@Stefan-Hanke you cleared a little confusion I had. For me was the exact opposite.
Wouldn't make sense to have a subturn for this kind of thing?
like:

  • ActivePlayer makes a move
  • trigger a subturn where any player can make a response action
  • back to ActivePlayer normal turn.

For me this subturn would be more phase-like then turn-like. It could have allowed moves, endPhaseIf...

@nicolodavis
Copy link
Member Author

nicolodavis commented Mar 13, 2018

Just to clarify a few things:

  • Even though the phase and turn concepts were originally designed for a game where multiple turns are played inside a phase, these are independent concepts and can be used the other way around. You can totally have a single turn where the game moves through different phases (Action and Buy phases in Dominion for example). The game is always in one phase and one player owns the current turn. You can modify these states independently (change phase while keeping the turn constant or change turn while keeping the phase constant).

  • This PR is just adding the ability to allow other players to play during a player's turn. Right now, the only way to achieve this is to yield the turn to the other players, which is less than ideal.

  • Since phases and turns are used in different ways in different games, I would avoid creating a hierarchy that's too rigid (I'd like these to be somewhat independent concepts). I can see some value in automatically remembering (stashing away) data about the current player / phase so that it's easy to restore back after temporarily changing the phase to another one (one where all the other players perform an action in response to something that the current player did, for example).

@nicolodavis
Copy link
Member Author

nicolodavis commented Mar 13, 2018

I think if we just store currentPlayer, actionPlayers and phase in previousPlayer, previousActionPlayers and previousPhase every time one of these values changes, that should facilitate easy restores?

We can even have API support for this via Events so that you don't restore manually.

@janosimas
Copy link
Contributor

Looks good.
What kind of restore Event are you thinking?

@nicolodavis
Copy link
Member Author

nicolodavis commented Mar 13, 2018

Something like:

  • Events.restorePhase(): Restores the phase to the previous phase.
  • Events.restorePlayerState(): Restores currentPlayer and activePlayers to their previous values.

We could also do the following instead (in order to have a single API call with options):

  • Events.restore({ phase: true, players: true }).

We might also want to provide the ability to set a snapshot point to restore to (rather than just saving the most recent value and restoring to that).

@janosimas
Copy link
Contributor

It could have a config for saving the state automatically or with another event:

  • Events.savePhase()
  • Events.savePlayerState()

Or with the snapshot:

  • Events.savePhase(key)
  • Events.savePlayerState(Key)

One question, do players have a secret state? Like the G.secret?
I'll have to deal with that for the player deck.

@nicolodavis
Copy link
Member Author

If you'd like to hide a field like deck from all players, I would recommend either putting it inside G.secret and using STRIP_SECRETS, or implementing a custom playerView that removes it. More details can be found at: http://boardgame.io/#/secret-state

@Stefan-Hanke
Copy link
Contributor

Thanks for the clarification, nicolo. I'd just like to point out that despite being correct about phases/turns, the order of eventing is such that a phase contains turns (the turn end event always comes before the phase end event). I'd like to have one consistent notion.

I kind of feel I'm loosing the overall picture of this all.

@nicolodavis
Copy link
Member Author

Yes, but we could actually go the other way around and I don't think it would make a material difference to games (really what's happening is that they're ending together, but we still need to pick an order because they can't happen in parallel). We could provide an option to configure phase-before-turn or turn-before-phase, but I think that's overkill. We should just clarify (in the docs) that phases and turns are independent concepts.

@janosimas
Copy link
Contributor

I would be more comfortable with the overkill solution but I don't see a case where it's strictly necessary.

Anyway it needs to be very clear in the docs, I didn't understand the phase-before-turn this way until @Stefan-Hanke said that.

@sladwig
Copy link
Contributor

sladwig commented Mar 16, 2018

me neither, for me it was clearly turn > phases, but I like it a lot that they are somewhat orthogonal and you could also use it the other way around and clearly there are games that need it the other way. It definitely explains some issues I had and I did somehow found it strange that phase end comes after turn end, but makes much more sense now.

btw: I am actually changing currentPlayer in the same turn, from 'any' to whoever wins the first roll of dice, I thought that's what turnOrder is for... In the current implementation if you define a turnOrder for each phase, it will get called on the beginning of each phase, thus allowing changing of players in the same turn. The documentation is not very clear about it and I had to experiment a bit to find that out, but it works. Could be better documented though, cause the behaviour is not very intuitive.

@nicolodavis
Copy link
Member Author

Closing this in favor of #154 to keep the implementation plan more focused. Feel free to continue discussions on this thread.

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

6 participants