diff --git a/app/src/main/java/com/jsjdesigns/poker/ActionButtons.kt b/app/src/main/java/com/jsjdesigns/poker/ActionButtons.kt new file mode 100644 index 0000000..856014f --- /dev/null +++ b/app/src/main/java/com/jsjdesigns/poker/ActionButtons.kt @@ -0,0 +1,46 @@ +package com.jsjdesigns.poker + +import com.jsjdesigns.poker.game.DecisionOffer + +/** + * What the action bar should show for a decision. + * + * Kept as a pure function of the offer so it can be unit-tested without a Compose + * runtime. The UI only renders this; it makes no decisions of its own. + */ +data class ActionButtons( + val showFold: Boolean, + val checkOrCallLabel: String, + val showRaise: Boolean, + /** False when there is exactly one legal raise size, so a slider is pointless. */ + val showSlider: Boolean, + val sliderMin: Int, + val sliderMax: Int, + /** The amount to submit when the slider is hidden. */ + val fixedRaiseTo: Int, +) + +fun buttonsFor(offer: DecisionOffer): ActionButtons { + // When a stack cannot cover a full min-raise it may still shove; the engine + // clamps such a raise to maxRaiseTo. Requiring maxRaiseTo > minRaiseTo to show + // the button hid both that short all-in AND an exact-minimum raise, even + // though the engine accepts both. + val shoveOnly = offer.maxRaiseTo <= offer.minRaiseTo + val low = if (shoveOnly) offer.maxRaiseTo else offer.minRaiseTo + val high = offer.maxRaiseTo + + return ActionButtons( + // Folding a free hand is never correct, so don't invite an accidental muck. + showFold = !offer.canCheck, + checkOrCallLabel = if (offer.canCheck) "Check" else "Call ${offer.toCall}", + showRaise = offer.canRaise, + showSlider = high > low, + sliderMin = low, + sliderMax = high, + fixedRaiseTo = low, + ) +} + +/** Label for the raise button at [amount]. */ +fun raiseLabel(buttons: ActionButtons, amount: Int): String = + if (amount >= buttons.sliderMax) "All in" else "Raise $amount" diff --git a/app/src/main/java/com/jsjdesigns/poker/PokerViewModel.kt b/app/src/main/java/com/jsjdesigns/poker/PokerViewModel.kt index bd42d38..21ecba4 100644 --- a/app/src/main/java/com/jsjdesigns/poker/PokerViewModel.kt +++ b/app/src/main/java/com/jsjdesigns/poker/PokerViewModel.kt @@ -44,11 +44,10 @@ data class UiState( fun liveOffer(offer: DecisionOffer?): DecisionOffer? { val snap = snapshot ?: return null if (offer == null) return null - return offer.takeIf { - it.handNumber == snap.handNumber && - snap.toAct == HERO_SEAT && - it.street == snap.street - } + // Match on the engine's decision token, not hand+street. A player can face + // two decisions on one street (bet, get raised, act again), so hand+street + // is ambiguous and would briefly pair a new offer with a stale board. + return offer.takeIf { snap.toActToken == it.token } } } diff --git a/app/src/main/java/com/jsjdesigns/poker/TableScreen.kt b/app/src/main/java/com/jsjdesigns/poker/TableScreen.kt index 16cf227..2a39cd4 100644 --- a/app/src/main/java/com/jsjdesigns/poker/TableScreen.kt +++ b/app/src/main/java/com/jsjdesigns/poker/TableScreen.kt @@ -57,7 +57,10 @@ fun TableScreen(vm: PokerViewModel) { .padding(12.dp), ) { Text( - text = "Hand ${state.handsPlayed + 1} ${snap?.street ?: ""}", + // snapshot.handNumber, not handsPlayed + 1: the counter increments when + // the hand *finishes*, so during the showdown hold it would label the + // result of hand 1 as "Hand 2". + text = "Hand ${snap?.handNumber ?: 1} ${snap?.street ?: ""}", color = Color.White.copy(alpha = 0.6f), fontSize = 12.sp, ) @@ -229,26 +232,25 @@ private fun ActionBar( return } - var raiseTo by remember { mutableIntStateOf(offer.minRaiseTo) } - // Reset the slider whenever a new decision arrives, or it keeps the old hand's value. - LaunchedEffect(offer) { raiseTo = offer.minRaiseTo } + val buttons = buttonsFor(offer) + var raiseTo by remember { mutableIntStateOf(buttons.fixedRaiseTo) } + // Reset whenever a new decision arrives, or it keeps the previous one's value. + LaunchedEffect(offer.token) { raiseTo = buttons.fixedRaiseTo } Column(Modifier.fillMaxWidth()) { - if (offer.canRaise && offer.maxRaiseTo > offer.minRaiseTo) { + if (buttons.showRaise && buttons.showSlider) { Text("Raise to $raiseTo", color = Color.White, fontSize = 13.sp) Slider( - value = raiseTo.toFloat(), + value = raiseTo.coerceIn(buttons.sliderMin, buttons.sliderMax).toFloat(), onValueChange = { raiseTo = it.toInt() }, - valueRange = offer.minRaiseTo.toFloat()..offer.maxRaiseTo.toFloat(), + valueRange = buttons.sliderMin.toFloat()..buttons.sliderMax.toFloat(), ) } Row( modifier = Modifier.fillMaxWidth(), horizontalArrangement = Arrangement.spacedBy(8.dp), ) { - // No Fold button when checking is free: folding a free hand is never - // correct, and offering it invites the player to muck by accident. - if (!offer.canCheck) { + if (buttons.showFold) { Button( onClick = { onFold(offer.token) }, modifier = Modifier.weight(1f), @@ -260,14 +262,19 @@ private fun ActionBar( onClick = { onCheckCall(offer.token) }, modifier = Modifier.weight(1f), colors = ButtonDefaults.buttonColors(containerColor = Color(0xFF2C6E49)), - ) { Text(if (offer.canCheck) "Check" else "Call ${offer.toCall}") } + ) { Text(buttons.checkOrCallLabel) } - if (offer.canRaise && offer.maxRaiseTo > offer.minRaiseTo) { + if (buttons.showRaise) { + val amount = if (buttons.showSlider) { + raiseTo.coerceIn(buttons.sliderMin, buttons.sliderMax) + } else { + buttons.fixedRaiseTo + } Button( - onClick = { onRaise(offer.token, raiseTo) }, + onClick = { onRaise(offer.token, amount) }, modifier = Modifier.weight(1f), colors = ButtonDefaults.buttonColors(containerColor = Color(0xFF14357A)), - ) { Text(if (raiseTo >= offer.maxRaiseTo) "All in" else "Raise") } + ) { Text(raiseLabel(buttons, amount)) } } } } diff --git a/app/src/test/java/com/jsjdesigns/poker/ActionButtonsTest.kt b/app/src/test/java/com/jsjdesigns/poker/ActionButtonsTest.kt new file mode 100644 index 0000000..106d07c --- /dev/null +++ b/app/src/test/java/com/jsjdesigns/poker/ActionButtonsTest.kt @@ -0,0 +1,99 @@ +package com.jsjdesigns.poker + +import com.jsjdesigns.poker.game.DecisionOffer +import com.jsjdesigns.poker.game.Street +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +class ActionButtonsTest { + + private fun offer( + toCall: Int = 10, + minRaiseTo: Int = 20, + maxRaiseTo: Int = 500, + canCheck: Boolean = false, + canRaise: Boolean = true, + stack: Int = 500, + ) = DecisionOffer( + token = 1L, + handNumber = 1, + street = Street.FLOP, + seat = 0, + hole = listOf(0, 1), + board = emptyList(), + pot = 40, + toCall = toCall, + minRaiseTo = minRaiseTo, + maxRaiseTo = maxRaiseTo, + stack = stack, + canCheck = canCheck, + canRaise = canRaise, + ) + + // ---------- the raise omission ---------- + + /** + * The UI used to require maxRaiseTo > minRaiseTo, which hid an exact-minimum + * raise even though the engine accepts it. + */ + @Test + fun `an exact minimum raise is still offered`() { + val b = buttonsFor(offer(minRaiseTo = 100, maxRaiseTo = 100)) + assertTrue("a raise with exactly one legal size must still be offered", b.showRaise) + assertFalse("no slider is useful for a single size", b.showSlider) + assertEquals(100, b.fixedRaiseTo) + } + + /** + * A stack too short for a full min-raise may still shove; the engine clamps + * such a raise to maxRaiseTo. That was hidden too. + */ + @Test + fun `a short all-in is still offered`() { + val b = buttonsFor(offer(minRaiseTo = 200, maxRaiseTo = 120, stack = 120)) + assertTrue("a legal short all-in must be offered", b.showRaise) + assertFalse(b.showSlider) + assertEquals(120, b.fixedRaiseTo) + assertEquals("All in", raiseLabel(b, b.fixedRaiseTo)) + } + + @Test + fun `a genuine range gets a slider`() { + val b = buttonsFor(offer(minRaiseTo = 20, maxRaiseTo = 500)) + assertTrue(b.showRaise) + assertTrue(b.showSlider) + assertEquals(20, b.sliderMin) + assertEquals(500, b.sliderMax) + } + + @Test + fun `no raise button when the engine would not accept one`() { + assertFalse(buttonsFor(offer(canRaise = false)).showRaise) + } + + // ---------- fold and call ---------- + + @Test + fun `fold is hidden when checking is free`() { + assertFalse( + "folding a free hand is never correct; offering it invites an accidental muck", + buttonsFor(offer(toCall = 0, canCheck = true)).showFold, + ) + assertTrue(buttonsFor(offer(toCall = 10, canCheck = false)).showFold) + } + + @Test + fun `the call button names the price`() { + assertEquals("Call 35", buttonsFor(offer(toCall = 35)).checkOrCallLabel) + assertEquals("Check", buttonsFor(offer(toCall = 0, canCheck = true)).checkOrCallLabel) + } + + @Test + fun `the raise label switches to all in at the top of the range`() { + val b = buttonsFor(offer(minRaiseTo = 20, maxRaiseTo = 500)) + assertEquals("Raise 60", raiseLabel(b, 60)) + assertEquals("All in", raiseLabel(b, 500)) + } +} diff --git a/app/src/test/java/com/jsjdesigns/poker/LiveOfferTest.kt b/app/src/test/java/com/jsjdesigns/poker/LiveOfferTest.kt new file mode 100644 index 0000000..ccfd6ee --- /dev/null +++ b/app/src/test/java/com/jsjdesigns/poker/LiveOfferTest.kt @@ -0,0 +1,100 @@ +package com.jsjdesigns.poker + +import com.jsjdesigns.poker.game.DecisionOffer +import com.jsjdesigns.poker.game.SeatSnapshot +import com.jsjdesigns.poker.game.Street +import com.jsjdesigns.poker.game.TableSnapshot +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +/** + * The action bar must never be rendered against a table it does not belong to. + */ +class LiveOfferTest { + + private fun snapshot( + handNumber: Int = 1, + street: Street = Street.FLOP, + toAct: Int? = HERO_SEAT, + toActToken: Long? = 7L, + ) = TableSnapshot( + handNumber = handNumber, + street = street, + phase = TableSnapshot.Phase.BETTING, + board = emptyList(), + pot = 40, + currentBet = 10, + minRaiseSize = 10, + button = 0, + seats = listOf( + SeatSnapshot(0, "You", 500, 0, 0, false, false, listOf(0, 1), false, true), + ), + toAct = toAct, + toActToken = toActToken, + lastAction = null, + ) + + private fun offer(token: Long = 7L, handNumber: Int = 1, street: Street = Street.FLOP) = + DecisionOffer( + token = token, + handNumber = handNumber, + street = street, + seat = HERO_SEAT, + hole = listOf(0, 1), + board = emptyList(), + pot = 40, + toCall = 10, + minRaiseTo = 20, + maxRaiseTo = 500, + stack = 500, + canCheck = false, + canRaise = true, + ) + + @Test + fun `an offer matching the displayed decision is live`() { + val state = UiState(snapshot = snapshot(toActToken = 7L)) + assertEquals(7L, state.liveOffer(offer(token = 7L))?.token) + } + + @Test + fun `an offer from a later decision is withheld until its frame is shown`() { + // The engine has moved on; the board on screen is still the older one. + val state = UiState(snapshot = snapshot(toActToken = 7L)) + assertNull( + "showing a newer offer against a stale table is what made actions look wrong", + state.liveOffer(offer(token = 8L)), + ) + } + + /** + * Hand+street matching was not enough: a player can face two decisions on the + * same street, e.g. bet, get raised, act again. + */ + @Test + fun `two decisions on the same street are distinguished`() { + val state = UiState(snapshot = snapshot(handNumber = 1, street = Street.FLOP, toActToken = 12L)) + assertNull( + "same hand and street, different decision — must not match", + state.liveOffer(offer(token = 11L, handNumber = 1, street = Street.FLOP)), + ) + assertEquals(12L, state.liveOffer(offer(token = 12L, handNumber = 1, street = Street.FLOP))?.token) + } + + @Test + fun `nothing is live when no one is being asked to act`() { + val state = UiState(snapshot = snapshot(toAct = null, toActToken = null)) + assertNull(state.liveOffer(offer())) + } + + @Test + fun `nothing is live before the first frame arrives`() { + assertNull(UiState(snapshot = null).liveOffer(offer())) + } + + @Test + fun `no offer means no action bar`() { + assertNull(UiState(snapshot = snapshot()).liveOffer(null)) + } +} diff --git a/engine/src/commonMain/kotlin/com/jsjdesigns/poker/game/HumanAgent.kt b/engine/src/commonMain/kotlin/com/jsjdesigns/poker/game/HumanAgent.kt index fb06098..f77c89b 100644 --- a/engine/src/commonMain/kotlin/com/jsjdesigns/poker/game/HumanAgent.kt +++ b/engine/src/commonMain/kotlin/com/jsjdesigns/poker/game/HumanAgent.kt @@ -24,7 +24,6 @@ class HumanAgent : PlayerAgent { private val lock = Mutex() private var pending: CompletableDeferred? = null private var pendingToken = 0L - private var nextToken = 1L private val _offer = MutableStateFlow(null) @@ -35,11 +34,12 @@ class HumanAgent : PlayerAgent { override suspend fun act(ctx: DecisionContext): Action { val deferred = CompletableDeferred() - val token: Long + // The engine owns decision identity, so the offer and the snapshot that + // announced this turn carry the same token. + val token = ctx.decisionToken lock.withLock { check(pending == null) { "already awaiting a decision for this agent" } pending = deferred - token = nextToken++ pendingToken = token } // Published after the deferred is installed, so a UI that reacts instantly diff --git a/engine/src/commonMain/kotlin/com/jsjdesigns/poker/game/Table.kt b/engine/src/commonMain/kotlin/com/jsjdesigns/poker/game/Table.kt index 2c4e9a7..5177e3f 100644 --- a/engine/src/commonMain/kotlin/com/jsjdesigns/poker/game/Table.kt +++ b/engine/src/commonMain/kotlin/com/jsjdesigns/poker/game/Table.kt @@ -67,6 +67,14 @@ class DecisionContext( val history: List, /** Increments once per hand; use it to detect hand boundaries. */ val handNumber: Int, + /** + * Identifies this specific decision, table-wide and monotonic. + * + * Owned by the engine so a snapshot and the decision it invites carry the + * same value; matching on hand and street alone is ambiguous, because a + * player can face two decisions on one street. + */ + val decisionToken: Long, /** TDA Rule 47: is this player facing at least a full raise since acting? */ val bettingReopened: Boolean, ) { @@ -139,6 +147,8 @@ class Table( private var minRaiseSize = 0 private var currentStreet = Street.PREFLOP private val revealed = HashSet() + private var decisionSeq = 0L + private var pendingToken = 0L private fun snapshot(phase: TableSnapshot.Phase, toAct: Int?, potOverride: Int?) = TableSnapshot( handNumber = handNumber, @@ -167,6 +177,7 @@ class Table( ) }, toAct = toAct, + toActToken = if (toAct != null) pendingToken else null, lastAction = events.lastOrNull(), ) @@ -406,7 +417,9 @@ class Table( val seat = seats[i] if (seat.canAct && (!seat.hasActed || seat.committedThisRound < currentBet)) { val toCall = (currentBet - seat.committedThisRound).coerceAtLeast(0) - // Publish before asking, so the UI can show whose turn it is. + // One token per decision, stamped before the snapshot so the frame + // and the offer it invites carry the same identity. + pendingToken = ++decisionSeq emit(TableSnapshot.Phase.BETTING, toAct = seat.index) val ctx = buildContext(street, seat, toCall) val action = sanitise(seat, toCall, seat.agent.act(ctx)) @@ -453,6 +466,7 @@ class Table( bigBlind = bigBlind, history = events, handNumber = handNumber, + decisionToken = pendingToken, bettingReopened = mayRaise(seat), ) } diff --git a/engine/src/commonMain/kotlin/com/jsjdesigns/poker/game/TableSnapshot.kt b/engine/src/commonMain/kotlin/com/jsjdesigns/poker/game/TableSnapshot.kt index c8e381b..a0a8638 100644 --- a/engine/src/commonMain/kotlin/com/jsjdesigns/poker/game/TableSnapshot.kt +++ b/engine/src/commonMain/kotlin/com/jsjdesigns/poker/game/TableSnapshot.kt @@ -22,6 +22,8 @@ data class TableSnapshot( val seats: List, /** Seat currently owed an action, or null between streets and at showdown. */ val toAct: Int?, + /** Identity of the decision [toAct] is being asked for; null when nobody is. */ + val toActToken: Long?, val lastAction: HandEvent?, ) { enum class Phase { DEALT, BETTING, STREET_COMPLETE, SHOWDOWN, COMPLETE } diff --git a/engine/src/commonTest/kotlin/com/jsjdesigns/poker/game/FoldAndTokenTest.kt b/engine/src/commonTest/kotlin/com/jsjdesigns/poker/game/FoldAndTokenTest.kt index b20ded2..2923832 100644 --- a/engine/src/commonTest/kotlin/com/jsjdesigns/poker/game/FoldAndTokenTest.kt +++ b/engine/src/commonTest/kotlin/com/jsjdesigns/poker/game/FoldAndTokenTest.kt @@ -32,7 +32,7 @@ private class FoldsOnce : PlayerAgent { class FoldAndTokenTest { - private fun context(seat: Seat, canCheck: Boolean = false) = DecisionContext( + private fun context(seat: Seat, canCheck: Boolean = false, token: Long = 1L) = DecisionContext( street = Street.FLOP, seat = seat, board = IntArray(0), @@ -45,6 +45,7 @@ class FoldAndTokenTest { bigBlind = 10, history = emptyList(), handNumber = 1, + decisionToken = token, bettingReopened = true, ) @@ -111,14 +112,14 @@ class FoldAndTokenTest { val human = HumanAgent() val seat = Seat(0, "You", 500, human) - val first = async { human.act(context(seat)) } + val first = async { human.act(context(seat, token = 10L)) } while (!human.isAwaitingInput) yield() val staleToken = human.offer.value!!.token assertTrue(human.submit(staleToken, Action(ActionType.FOLD))) first.await() // A second decision arrives with a new token. - val second = async { human.act(context(seat)) } + val second = async { human.act(context(seat, token = 11L)) } while (!human.isAwaitingInput) yield() val freshToken = human.offer.value!!.token assertNotEquals(staleToken, freshToken, "each decision gets its own token") @@ -151,8 +152,8 @@ class FoldAndTokenTest { val seat = Seat(0, "You", 500, human) val seen = mutableListOf() - repeat(3) { - val d = async { human.act(context(seat)) } + repeat(3) { i -> + val d = async { human.act(context(seat, token = 100L + i)) } while (!human.isAwaitingInput) yield() val t = human.offer.value!!.token seen += t @@ -163,3 +164,56 @@ class FoldAndTokenTest { assertEquals(seen.sorted(), seen, "and should advance monotonically") } } + +/** The token has to be the same value on both sides of the UI boundary. */ +class EngineTokenIdentityTest { + + @Test + fun `the snapshot announcing a turn carries the same token as the offer`() = runTest { + val seen = mutableListOf() + val human = HumanAgent() + val seats = listOf(Seat(0, "You", 500, human), Seat(1, "Bot", 500, AlwaysChecks())) + val table = Table( + seats, 5, 10, Random(1), + StackedDeck.of(listOf("Ah Ad", "Kh Kd"), "2c 7d 9s Jc 3h"), + observer = { seen += it }, + ) + + val hand = kotlinx.coroutines.CoroutineScope(kotlin.coroutines.EmptyCoroutineContext).let { + async { table.playHand() } + } + while (!human.isAwaitingInput) yield() + + val offerToken = human.offer.value!!.token + val announcing = seen.last { it.toAct == 0 } + assertEquals( + announcing.toActToken, offerToken, + "the frame that says it is your turn must identify the same decision", + ) + + human.submit(offerToken, Action(ActionType.FOLD)) + hand.await() + } + + @Test + fun `every decision in a hand gets a distinct token`() = runTest { + val seen = mutableListOf() + val seats = listOf( + Seat(0, "A", 500, AlwaysChecks()), + Seat(1, "B", 500, AlwaysChecks()), + Seat(2, "C", 500, AlwaysChecks()), + ) + Table( + seats, 5, 10, Random(1), + StackedDeck.of(listOf("Ah Ad", "Kh Kd", "Qh Qd"), "2c 7d 9s Jc 3h"), + observer = { seen += it }, + ).playHand() + + val tokens = seen.mapNotNull { it.toActToken } + assertTrue(tokens.size > 3, "several decisions should have occurred") + assertEquals(tokens.distinct().sorted(), tokens.distinct(), "tokens advance") + // Each announcing frame is a distinct decision. + val announcing = seen.filter { it.toAct != null }.mapNotNull { it.toActToken } + assertEquals(announcing.size, announcing.distinct().size, "no token reused") + } +} diff --git a/engine/src/commonTest/kotlin/com/jsjdesigns/poker/game/SnapshotAndHumanAgentTest.kt b/engine/src/commonTest/kotlin/com/jsjdesigns/poker/game/SnapshotAndHumanAgentTest.kt index 2517530..39f8899 100644 --- a/engine/src/commonTest/kotlin/com/jsjdesigns/poker/game/SnapshotAndHumanAgentTest.kt +++ b/engine/src/commonTest/kotlin/com/jsjdesigns/poker/game/SnapshotAndHumanAgentTest.kt @@ -35,7 +35,7 @@ private class Raiser(private val to: Int) : PlayerAgent { class SnapshotAndHumanAgentTest { - private fun context(seat: Seat) = DecisionContext( + private fun context(seat: Seat, token: Long = 1L) = DecisionContext( street = Street.PREFLOP, seat = seat, board = IntArray(0), @@ -48,6 +48,7 @@ class SnapshotAndHumanAgentTest { bigBlind = 10, history = emptyList(), handNumber = 1, + decisionToken = token, bettingReopened = true, ) @@ -200,7 +201,7 @@ class SnapshotAndHumanAgentTest { class HumanOfferAndStreetStateTest { - private fun context(seat: Seat) = DecisionContext( + private fun context(seat: Seat, token: Long = 1L) = DecisionContext( street = Street.PREFLOP, seat = seat, board = IntArray(0), @@ -213,6 +214,7 @@ class HumanOfferAndStreetStateTest { bigBlind = 10, history = emptyList(), handNumber = 1, + decisionToken = token, bettingReopened = true, )