Merge pull request #26: keeping dice is a set, not a list

Holding dice did nothing unless they were tapped left to right: a tap-ordered
list was not among the ascending ones legalMoves offered, and the controller
drops what it cannot find there. Verified by ci.yml run 118.

app/src/main/java/org/prolibertate/games/game/yahtzee/YahtzeeAi.kt

@@ -28,9 +28,8 @@ object YahtzeeAi : GameAi<YahtzeeState, YahtzeeMove> {
         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()
     }
 
     /**

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<Int> = emptyList()) : YahtzeeMove
+data class RollDice(val keep: Set<Int> = emptySet()) : YahtzeeMove
 
 @Serializable
 @SerialName("score")

app/src/main/java/org/prolibertate/games/game/yahtzee/YahtzeeRules.kt

@@ -53,7 +53,7 @@ object YahtzeeRules : GameRules<YahtzeeState, YahtzeeMove> {
             // 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<YahtzeeState, YahtzeeMove> {
     }
 
     /** Every set of dice positions a player might hold back — thirty-two of them. */
-    private fun keepSubsets(): List<List<Int>> =
+    private fun keepSubsets(): List<Set<Int>> =
         (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<YahtzeeState, YahtzeeMove> {
 
     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<YahtzeeState, YahtzeeMove> {
         )
     }
 
-    private fun throwDice(state: YahtzeeState, keep: List<Int>): List<Int> {
+    private fun throwDice(state: YahtzeeState, keep: Set<Int>): List<Int> {
         val random = Random(
             state.seed +
                 state.round * 10_000L +

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()

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<RollDice>()
+        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)))