Simultaneous Move support#356
Conversation
- Add getSimultaneousPlayers() to AbstractGameState with default implementation wrapping getCurrentPlayer() for backwards compatibility - Add SimultaneousAction class to core/actions/ extending AbstractAction stores player -> action map with LinkedHashMap for deterministic order implements execute(), copy(), getString() and toString() - Extract makeDecisionForPlayer() from oneAction() in Game handles observation copy, action computation and player decision - Add oneSimultaneousAction() to Game loops over active players, collects actions, wraps into SimultaneousAction applies once via forwardModel.next() - Wire oneAction() to delegate to oneSimultaneousAction() when getCurrentSimultaneousPlayers().size() > 1
…run operations out of loop
…layers added to SGGameState
# Conflicts: # src/main/java/core/Game.java
…to fail without me noticing in a differetn PR)
| throw new AssertionError("Action played that was not in the list of available actions: " + action); | ||
| } | ||
|
|
||
| if (actionValidation && !observedActions.contains(action)) |
There was a problem hiding this comment.
Not a big thing, but I'm wondering if in some games this may be allowed (so part of core games parameters instead, for example). Say, everyone throw a card down, if someone doesn't play a legal action, they're out of the game, or lose a point, but the rest of the actions are still applied. No game comes to mind that would use this mechanism, but I think there would be cases where the legal actions should still be applied while ignoring or penalizing the others, so I'd be in favour of keeping this definable at game level. Could be actionValidation = true by default with the behaviour being a function called on the FM for handling invalid actions (I think there may already be something like that, or at least it was at some point before on the FM), with the possibility of overriding this by individual games for different behaviours.
|
|
||
|
|
||
|
|
||
| */ |
There was a problem hiding this comment.
Odd comments, class should be nicely documented especially as a first example of using proper simultaneous actions
There was a problem hiding this comment.
Any old code should also be removed to allow proper diff and reduce confusion
| throw new AssertionError("Player " + i + " has " + gs.getPlayerHands().get(i).getSize() + " cards, expected " + expectedPlayerCards); | ||
| } | ||
| } | ||
| // int expectedPlayerCards = gs.getPlayerHands().get(0).getSize(); |
There was a problem hiding this comment.
Should be cleaned up if no longer necessary
3 participants
This provides initial support in the core framework for Simultaneous Move games. This enables new games to have simultaneous moves, or for old ones with internal implementations (SushiGo, War of theToads, Diamant, Resistance...) to be converted to the new framework. However, it is backwards compatible with all existing games, so no conversion is required.
The advantage of converting a game is that SFP algorithms may then be able to take advantage of this.
AbstractGameState
AbstractForwardModel
IExtendedSequence
Game
At the moment, only SushiGo has been converted to the new set up.
The split of _copy() and redeterminise() was required for SushiGo because the redeterminisation has to set the turnOwner, and _copy() is called at the start of copy(), before copy() then sets up the fields defined in AbstractGameState (overriding turnOwner).
This split into _copy() and redeterminise(in playerId) enables the latter to be called at the end of the copy() method, giving it the option of shuffling/obscuring data at the AGS level, which is theoretically desirable. It also provides a clean semantic split between, "the code that copies the state data", and "the code that shuffles this to account for imperfect information", which is in any case a better design that may reduce errors.
The intention is therefore in a separate PR to fully implement this _copy/redeterminise split across all existing games...but not in this one.