From e5c9561fefd87fb862d9c756f67707b1cf1e0765 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 19:40:38 +0000 Subject: [PATCH 1/2] Keeping dice is a set, not a list Holding dice did nothing unless they happened to be tapped left to right. The screen appends each die as it is tapped, so holding the third and then the first produced [2, 0]. legalMoves only ever generated ascending subsets, and RollDice is a data class over a List, so [2, 0] was a different move from [0, 2] and simply was not among the legal ones. MatchController drops a move it cannot find there, silently -- hence a highlighted die, a Throw that did nothing, and no error to go on. A keep is a set of positions. Holding the third die and then the first is the same act as holding the first and then the third, and a set says so: the two are now equal, and the order they were built up in cannot matter. That kills the whole class of bug rather than sorting at the one call site that happened to show it. Positions rather than faces still stands -- two fives are two different dice. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Urn6eEBA6XzrnfoibL7faC --- .../games/game/yahtzee/YahtzeeAi.kt | 5 +-- .../games/game/yahtzee/YahtzeeModel.kt | 8 +++- .../games/game/yahtzee/YahtzeeRules.kt | 9 ++-- .../games/ui/game/YahtzeeScreen.kt | 2 +- .../games/game/yahtzee/YahtzeeRulesTest.kt | 42 ++++++++++++++++--- 5 files changed, 50 insertions(+), 16 deletions(-) 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))) From 39ce5aa0eeb66beb568740a2dd5d3799b34efc47 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 19:56:19 +0000 Subject: [PATCH 2/2] Keep held dice held between throws Picking the same three dice again after every throw is work the screen can do. A kept die does not move and does not change, so the hold a player just made is still the hold they want, and clearing it was throwing away a decision they had already expressed. The selection has to go when the turn does, though. A turn opens with the dice back in the cup and its first throw must keep nothing -- the rules refuse anything else -- so a selection carried over from the last turn would be rejected and Throw would look broken in exactly the way it did before. Keyed on the round as well as the seat, because a solo game never changes seat: turn stays at zero and only the round moves on. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Urn6eEBA6XzrnfoibL7faC --- .../prolibertate/games/ui/game/YahtzeeScreen.kt | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) 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 68d539e..7c0e401 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 @@ -20,6 +20,7 @@ import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.material3.TextButton import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateListOf @@ -82,6 +83,11 @@ fun YahtzeeScreen( return } + // A turn starts with the dice back in the cup, and its first throw must + // keep nothing — a selection carried over from the last turn would be + // refused, and the Throw button would look broken again. + LaunchedEffect(current.turn, current.round) { kept.clear() } + val yours = current.turn == localSeat && !current.isOver val canRoll = legal.any { it is RollDice } val writable = legal.filterIsInstance().map { it.category }.toSet() @@ -114,9 +120,11 @@ fun YahtzeeScreen( Button( onClick = { 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() + // The selection stays. A kept die does not move or + // change, so the hold a player just made is still the + // hold they want — asking them to pick the same three + // again between every throw is work the screen can do + // for them. }, ) { Text(if (current.hasRolled) "Throw the rest" else "Throw")