diff --git a/app/src/main/java/org/prolibertate/games/game/yahtzee/YahtzeeAi.kt b/app/src/main/java/org/prolibertate/games/game/yahtzee/YahtzeeAi.kt index b6ecc05..6fc9bb2 100644 --- a/app/src/main/java/org/prolibertate/games/game/yahtzee/YahtzeeAi.kt +++ b/app/src/main/java/org/prolibertate/games/game/yahtzee/YahtzeeAi.kt @@ -28,9 +28,8 @@ object YahtzeeAi : GameAi { if (rolls.isEmpty()) return best ?: writes.first() if (bestScore >= WORTH_KEEPING) return best!! - val keep = whatToKeep(state.dice) - return rolls.firstOrNull { it.keep.sorted() == keep.sorted() } - ?: rolls.first() + val keep = whatToKeep(state.dice).toSet() + return rolls.firstOrNull { it.keep == keep } ?: rolls.first() } /** diff --git a/app/src/main/java/org/prolibertate/games/game/yahtzee/YahtzeeModel.kt b/app/src/main/java/org/prolibertate/games/game/yahtzee/YahtzeeModel.kt index 4e14cc3..609ab87 100644 --- a/app/src/main/java/org/prolibertate/games/game/yahtzee/YahtzeeModel.kt +++ b/app/src/main/java/org/prolibertate/games/game/yahtzee/YahtzeeModel.kt @@ -132,10 +132,16 @@ sealed interface YahtzeeMove * Positions rather than faces: two fives are two different dice, and a player * who keeps one of them and rerolls the other is doing something a set of faces * could not express. + * + * A set rather than a list, because holding the third die and then the first is + * the same act as holding the first and then the third. As a list those were two + * different moves, and only the ascending one was ever offered as legal — so + * keeping dice in any other order was silently dropped and the throw did + * nothing. */ @Serializable @SerialName("roll") -data class RollDice(val keep: List = emptyList()) : YahtzeeMove +data class RollDice(val keep: Set = emptySet()) : YahtzeeMove @Serializable @SerialName("score") diff --git a/app/src/main/java/org/prolibertate/games/game/yahtzee/YahtzeeRules.kt b/app/src/main/java/org/prolibertate/games/game/yahtzee/YahtzeeRules.kt index 3e82893..102cca5 100644 --- a/app/src/main/java/org/prolibertate/games/game/yahtzee/YahtzeeRules.kt +++ b/app/src/main/java/org/prolibertate/games/game/yahtzee/YahtzeeRules.kt @@ -53,7 +53,7 @@ object YahtzeeRules : GameRules { // The first throw of a turn is all five dice: there is nothing on // the table yet to keep. moves += if (!state.hasRolled) { - listOf(RollDice(emptyList())) + listOf(RollDice(emptySet())) } else { keepSubsets().map { RollDice(it) } } @@ -67,9 +67,9 @@ object YahtzeeRules : GameRules { } /** Every set of dice positions a player might hold back — thirty-two of them. */ - private fun keepSubsets(): List> = + private fun keepSubsets(): List> = (0 until (1 shl DICE_COUNT)).map { mask -> - (0 until DICE_COUNT).filter { mask and (1 shl it) != 0 } + (0 until DICE_COUNT).filter { mask and (1 shl it) != 0 }.toSet() } override fun applyMove(state: YahtzeeState, seat: Int, move: YahtzeeMove): YahtzeeState { @@ -83,7 +83,6 @@ object YahtzeeRules : GameRules { private fun applyRoll(state: YahtzeeState, move: RollDice): YahtzeeState { require(state.rollsUsed < ROLLS_PER_TURN) { "No rolls left this turn" } - require(move.keep.distinct().size == move.keep.size) { "Cannot keep a die twice" } require(move.keep.all { it in 0 until DICE_COUNT }) { "No such die" } require(state.hasRolled || move.keep.isEmpty()) { "Nothing to keep before the first roll" } return state.copy( @@ -92,7 +91,7 @@ object YahtzeeRules : GameRules { ) } - private fun throwDice(state: YahtzeeState, keep: List): List { + private fun throwDice(state: YahtzeeState, keep: Set): List { val random = Random( state.seed + state.round * 10_000L + diff --git a/app/src/main/java/org/prolibertate/games/ui/game/YahtzeeScreen.kt b/app/src/main/java/org/prolibertate/games/ui/game/YahtzeeScreen.kt index abf9903..68d539e 100644 --- a/app/src/main/java/org/prolibertate/games/ui/game/YahtzeeScreen.kt +++ b/app/src/main/java/org/prolibertate/games/ui/game/YahtzeeScreen.kt @@ -113,7 +113,7 @@ fun YahtzeeScreen( if (yours && canRoll) { Button( onClick = { - controller.submit(RollDice(kept.toList())) + controller.submit(RollDice(kept.toSet())) // What was kept applied to the throw just made; the next // one starts from whatever is on the table now. kept.clear() diff --git a/app/src/test/java/org/prolibertate/games/game/yahtzee/YahtzeeRulesTest.kt b/app/src/test/java/org/prolibertate/games/game/yahtzee/YahtzeeRulesTest.kt index 7ab59b6..03d5938 100644 --- a/app/src/test/java/org/prolibertate/games/game/yahtzee/YahtzeeRulesTest.kt +++ b/app/src/test/java/org/prolibertate/games/game/yahtzee/YahtzeeRulesTest.kt @@ -118,7 +118,7 @@ class YahtzeeRulesTest { val state = YahtzeeRules.initialState(config()) assertEquals(0, YahtzeeRules.currentSeat(state)) val legal = YahtzeeRules.legalMoves(state, 0) - assertEquals(listOf(RollDice(emptyList())), legal) + assertEquals(listOf(RollDice(emptySet())), legal) // Nothing may be written in before the dice have been thrown. assertThrows { YahtzeeRules.applyMove(state, 0, ScoreIn(YahtzeeCategory.CHANCE)) } } @@ -142,15 +142,15 @@ class YahtzeeRulesTest { val again = YahtzeeRules.applyMove(state, 0, RollDice()) assertEquals(once.dice, again.dice) - val keptOnce = YahtzeeRules.applyMove(once, 0, RollDice(listOf(0, 1))) - val keptAgain = YahtzeeRules.applyMove(once, 0, RollDice(listOf(0, 1))) + val keptOnce = YahtzeeRules.applyMove(once, 0, RollDice(setOf(0, 1))) + val keptAgain = YahtzeeRules.applyMove(once, 0, RollDice(setOf(0, 1))) assertEquals(keptOnce.dice, keptAgain.dice) } @Test fun `kept dice stay put and the rest are thrown again`() { val state = YahtzeeRules.applyMove(YahtzeeRules.initialState(config()), 0, RollDice()) - val kept = YahtzeeRules.applyMove(state, 0, RollDice(listOf(0, 2, 4))) + val kept = YahtzeeRules.applyMove(state, 0, RollDice(setOf(0, 2, 4))) assertEquals(state.dice[0], kept.dice[0]) assertEquals(state.dice[2], kept.dice[2]) assertEquals(state.dice[4], kept.dice[4]) @@ -169,10 +169,40 @@ class YahtzeeRulesTest { assertTrue(YahtzeeRules.legalMoves(state, 0).all { it is ScoreIn }) } + @Test + fun `the order dice were tapped in does not matter`() { + // The screen adds each die as it is tapped, so holding the third and + // then the first once gave [2, 0]. As a list that was a different move + // from [0, 2], and only the ascending one was ever offered as legal, so + // the controller dropped it and the throw did nothing at all. + val state = YahtzeeRules.applyMove(YahtzeeRules.initialState(config()), 0, RollDice()) + assertEquals(RollDice(setOf(2, 0)), RollDice(setOf(0, 2))) + + val legal = YahtzeeRules.legalMoves(state, 0) + assertTrue( + "a keep must be legal however it was built up", + legal.contains(RollDice(setOf(2, 0))), + ) + // And it has to actually hold those dice. + val thrown = YahtzeeRules.applyMove(state, 0, RollDice(setOf(2, 0))) + assertEquals(state.dice[0], thrown.dice[0]) + assertEquals(state.dice[2], thrown.dice[2]) + assertEquals(2, thrown.rollsUsed) + } + + @Test + fun `every way of keeping dice is on offer`() { + // Thirty-two subsets of five dice, and each of them exactly once. + val state = YahtzeeRules.applyMove(YahtzeeRules.initialState(config()), 0, RollDice()) + val rolls = YahtzeeRules.legalMoves(state, 0).filterIsInstance() + assertEquals(32, rolls.size) + assertEquals(32, rolls.map { it.keep }.distinct().size) + } + @Test fun `nothing may be kept before the first throw`() { val state = YahtzeeRules.initialState(config()) - assertThrows { YahtzeeRules.applyMove(state, 0, RollDice(listOf(0))) } + assertThrows { YahtzeeRules.applyMove(state, 0, RollDice(setOf(0))) } } @Test @@ -277,7 +307,7 @@ class YahtzeeRulesTest { val state = YahtzeeRules.applyMove(YahtzeeRules.initialState(config()), 0, RollDice()) assertEquals(state, YahtzeeRules.decodeState(YahtzeeRules.encodeState(state))) - val roll: YahtzeeMove = RollDice(listOf(0, 3)) + val roll: YahtzeeMove = RollDice(setOf(0, 3)) assertEquals(roll, YahtzeeRules.decodeMove(YahtzeeRules.encodeMove(roll))) val write: YahtzeeMove = ScoreIn(YahtzeeCategory.LARGE_STRAIGHT) assertEquals(write, YahtzeeRules.decodeMove(YahtzeeRules.encodeMove(write)))