From 7c839a924235d36cb979b9fc47afdef27250bcd1 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 20:45:37 +0000 Subject: [PATCH 1/9] Model card play as cards landing on a table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cards appeared and disappeared. A card was drawn at its resting place the frame it was played, and a finished trick slid off in a straight line at a constant speed with every card starting and stopping together. Nothing about that reads as a hand putting a card down. What a real card does is arrive from the side of the person who threw it, carry past the spot it was aimed at and rock back, and finish a little crooked and a little off the mark. That is the whole model. There are no collisions and no ragdolls, because the difference between cards on a table and cards thrown down a stairwell is entirely the size of the numbers: the tilt is capped at four and a half degrees and the drift at five per cent of a card width, and both are smaller than they want to be. The flight is one underdamped spring on a single 0-to-1 value, with position, angle and height above the table all read off it. The overshoot is the spring passing 1 and returning, so the card carries and settles as one motion rather than as three animations that happen to start together. Driving them apart was tried and looked like it. The scatter is hashed out of the card's identity rather than drawn from a generator. A generator would give a card a new angle on every recomposition, which twitches; hashing the list position instead would make a card jump whenever another landed beside it. Hashed this way a card lies the same way for as long as it lies there, and no two cards in a deck lie the same way as each other. The pure half of that — the hash, the bounds, the sweep stagger — is unit tested, since it is not something a screenshot would show. The sweep now staggers and accelerates: cards break away over the first third of it rather than in lockstep, each carrying out the same turn it carried in. Piles are drawn as the last few cards of the heap and stocks as a pack with depth, so a card played onto a discard pile lands on something. Every card gains the shadow of a card lying on felt. Applied to Euchre, Kaiser, Wizard and Hearts at the trick, and to Crazy 8s, President and Golf at the pile; hands get a tilt small enough that nobody sees it and large enough that a row of cards stops looking printed. Motion follows the existing animation-speed setting through a composition local, so the slider still governs all of it. Series bumped to 1.3 for the dev channel. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0132V9gE6DF9itz2z3Hn7BK9 --- README.md | 4 +- .../java/org/prolibertate/games/ui/AppRoot.kt | 23 +- .../games/ui/common/Components.kt | 19 + .../prolibertate/games/ui/game/CardPhysics.kt | 456 ++++++++++++++++++ .../games/ui/game/CrazyEightsScreen.kt | 11 +- .../games/ui/game/EuchreScreen.kt | 121 +++-- .../prolibertate/games/ui/game/GolfScreen.kt | 11 +- .../games/ui/game/HeartsScreen.kt | 36 +- .../games/ui/game/KaiserScreen.kt | 104 ++-- .../games/ui/game/PresidentScreen.kt | 40 +- .../games/ui/game/WizardScreen.kt | 96 ++-- .../games/ui/game/CardPhysicsTest.kt | 165 +++++++ gradle.properties | 2 +- 13 files changed, 950 insertions(+), 138 deletions(-) create mode 100644 app/src/main/java/org/prolibertate/games/ui/game/CardPhysics.kt create mode 100644 app/src/test/java/org/prolibertate/games/ui/game/CardPhysicsTest.kt diff --git a/README.md b/README.md index c70579e..f452991 100644 --- a/README.md +++ b/README.md @@ -173,7 +173,7 @@ production package instead. - `.github/workflows/ci.yml` — builds, tests and lints on `dev` and on pull requests into `dev` or `main`. Its APK artifact is a dev build, versioned `.0-dev` — the series being whatever `plgVersionSeries` says in - `gradle.properties`, currently **1.1**. + `gradle.properties`, currently **1.3**. - `.github/workflows/release.yml` — on push to `main`, builds the APK and publishes it as a GitHub Release tagged `v.`. The in-app updater reads that tag, which is why the APK's `versionCode` is the same run number. @@ -201,7 +201,7 @@ A version is **`.`**. The series is set by hand, once, in `gradle.properties`: ```properties -plgVersionSeries=1.1 +plgVersionSeries=1.3 ``` Both workflows read that same property when they name a release tag, and the diff --git a/app/src/main/java/org/prolibertate/games/ui/AppRoot.kt b/app/src/main/java/org/prolibertate/games/ui/AppRoot.kt index 5c2c3bd..98bf6af 100644 --- a/app/src/main/java/org/prolibertate/games/ui/AppRoot.kt +++ b/app/src/main/java/org/prolibertate/games/ui/AppRoot.kt @@ -26,6 +26,7 @@ import androidx.compose.material3.Text import androidx.compose.material3.TextButton import androidx.compose.material3.TopAppBar import androidx.compose.runtime.Composable +import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf @@ -45,6 +46,7 @@ import org.prolibertate.games.net.LobbyController import org.prolibertate.games.score.ScorekeeperRepository import org.prolibertate.games.settings.Settings import org.prolibertate.games.settings.SettingsRepository +import org.prolibertate.games.ui.game.LocalCardSpeed import org.prolibertate.games.update.UpdateChannel import org.prolibertate.games.update.Updater @@ -121,14 +123,19 @@ fun AppRoot(env: AppEnv) { var splashDone by remember { mutableStateOf(false) } Box(modifier = Modifier.fillMaxSize()) { - AppContent( - env = env, - settings = settings, - stack = stack, - showComingSoon = showComingSoon, - push = push, - pop = pop, - ) + // Card motion reads its speed from here rather than from a parameter. + // Every card screen animates, most of them several layers deep, and the + // alternative was passing one unchanging number down through all of it. + CompositionLocalProvider(LocalCardSpeed provides settings.animationSpeed) { + AppContent( + env = env, + settings = settings, + stack = stack, + showComingSoon = showComingSoon, + push = push, + pop = pop, + ) + } // Over the top, so the menu behind it is already composed when it fades. if (!splashDone) { diff --git a/app/src/main/java/org/prolibertate/games/ui/common/Components.kt b/app/src/main/java/org/prolibertate/games/ui/common/Components.kt index 8ef3193..5bb4965 100644 --- a/app/src/main/java/org/prolibertate/games/ui/common/Components.kt +++ b/app/src/main/java/org/prolibertate/games/ui/common/Components.kt @@ -18,6 +18,7 @@ import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.alpha import androidx.compose.ui.draw.clip +import androidx.compose.ui.draw.shadow import androidx.compose.ui.graphics.Color import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.unit.Dp @@ -41,6 +42,13 @@ fun PlayingCardView( width: Dp = 64.dp, selected: Boolean = false, enabled: Boolean = true, + /** + * How far off the surface the card is sitting. The default is the thickness + * of a card lying on a table, which is what separates a card *on* the felt + * from a card painted onto it; a card in flight is raised further by + * whatever is animating it. + */ + elevation: Dp = CARD_RESTING_ELEVATION, /** * Short note printed on the face, for when the rank and suit alone do not * say what the card does — a Sequence jack being the obvious case, since @@ -54,6 +62,7 @@ fun PlayingCardView( modifier = modifier .width(width) .aspectRatio(CARD_ASPECT) + .shadow(elevation, RoundedCornerShape(width * 0.12f)) .clip(RoundedCornerShape(width * 0.12f)) .background(Color.White) .border( @@ -102,11 +111,13 @@ fun PlayingCardView( fun CardBackView( modifier: Modifier = Modifier, width: Dp = 64.dp, + elevation: Dp = CARD_RESTING_ELEVATION, ) { Box( modifier = modifier .width(width) .aspectRatio(CARD_ASPECT) + .shadow(elevation, RoundedCornerShape(width * 0.12f)) .clip(RoundedCornerShape(width * 0.12f)) .background(MaterialTheme.colorScheme.primary) .border(1.dp, Color(0x33000000), RoundedCornerShape(width * 0.12f)), @@ -121,3 +132,11 @@ fun CardBackView( } const val CARD_ASPECT = 5f / 7f + +/** + * A card lying on a surface. Small enough to read as the thickness of a piece + * of card rather than as a floating tile — the miniature card backs used as + * hand counters are twelve dp wide, and anything heavier turns those into + * smudges. + */ +val CARD_RESTING_ELEVATION: Dp = 1.5.dp diff --git a/app/src/main/java/org/prolibertate/games/ui/game/CardPhysics.kt b/app/src/main/java/org/prolibertate/games/ui/game/CardPhysics.kt new file mode 100644 index 0000000..710bcb9 --- /dev/null +++ b/app/src/main/java/org/prolibertate/games/ui/game/CardPhysics.kt @@ -0,0 +1,456 @@ +package org.prolibertate.games.ui.game + +import androidx.compose.animation.core.Animatable +import androidx.compose.animation.core.spring +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.offset +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.width +import androidx.compose.runtime.Composable +import androidx.compose.runtime.Immutable +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.getValue +import androidx.compose.runtime.key +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue +import androidx.compose.runtime.staticCompositionLocalOf +import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.rotate +import androidx.compose.ui.graphics.graphicsLayer +import androidx.compose.ui.platform.LocalDensity +import androidx.compose.ui.unit.Dp +import androidx.compose.ui.unit.IntOffset +import androidx.compose.ui.unit.dp +import kotlin.math.PI +import kotlin.math.cos +import kotlin.math.roundToInt +import kotlin.math.sin +import kotlinx.coroutines.delay +import org.prolibertate.games.game.cards.Card +import org.prolibertate.games.settings.Settings +import org.prolibertate.games.ui.common.CARD_ASPECT +import org.prolibertate.games.ui.common.CARD_RESTING_ELEVATION +import org.prolibertate.games.ui.common.CardBackView +import org.prolibertate.games.ui.common.PlayingCardView + +/** + * How cards move. + * + * A card thrown onto a table does three things that a card faded into place + * does not: it arrives from the side of the person who threw it, it overshoots + * and rocks back rather than stopping dead, and it comes to rest a little + * crooked and a little off the spot it was aimed at. This file is those three + * things, and nothing else — no ragdolls, no collisions. Real cards played by + * a real hand are only slightly random, and the whole difficulty is in keeping + * the randomness *small*: a table where every card lands at a jaunty angle + * stops looking like cards and starts looking like a screensaver. + * + * The randomness is therefore bounded and deterministic. Bounded because every + * scatter is a fraction of a card width or a handful of degrees; deterministic + * because it is hashed out of the card's own identity rather than drawn from a + * generator, which is what stops a card twitching to a new angle every time + * something else on the screen recomposes. The same card in the same place + * always lies the same way, and two cards never lie the same way as each other. + */ + +// --------------------------------------------------------------------------- +// Bounds. Every one of these is deliberately smaller than it wants to be. +// --------------------------------------------------------------------------- + +/** Most a settled card is left turned by, in degrees. */ +const val REST_TILT_DEGREES = 4.5f + +/** Most a settled card misses its spot by, as a fraction of its own width. */ +const val REST_DRIFT_FRACTION = 0.055f + +/** Extra turn a card carries while it is still travelling. */ +const val THROW_SPIN_DEGREES = 15f + +/** Tilt of a card held in a hand, where the pack has been squared up. */ +const val HAND_TILT_DEGREES = 1.6f + +/** How far off the table a thrown card starts, in card widths. */ +const val THROW_DISTANCE = 2.2f + +/** How much bigger a card looks while it is above the table rather than on it. */ +private const val THROW_SCALE = 1.11f + +/** + * Shadow under a card in flight. What it settles to is the ordinary resting + * elevation every other card on the screen already has, read from there rather + * than repeated, so a card that has just landed is indistinguishable from one + * that has been lying there all along. + */ +private const val THROW_ELEVATION_DP = 9f +private val REST_ELEVATION_DP = CARD_RESTING_ELEVATION.value + +/** + * Bounce in the settle. Below 1 is an underdamped spring, so the card carries + * past its resting place and comes back — which is the whole point. Much below + * this and it wobbles like rubber. + */ +private const val SETTLE_DAMPING = 0.62f + +/** Stiffness that gives [CARD_THROW_MILLIS] at normal speed. */ +private const val SETTLE_STIFFNESS = 420f + +/** Nominal flight time of a card, before the animation-speed setting. */ +const val CARD_THROW_MILLIS = 380 + +/** Nominal time a finished trick takes to be gathered up. */ +const val CARD_SWEEP_MILLIS = 520 + +/** + * How much of the sweep is spent letting the stragglers go. Cards do not all + * leave at once when a hand scoops them up, but they do all leave quickly. + */ +private const val SWEEP_SPREAD = 0.3f + +// --------------------------------------------------------------------------- +// The pure part: where a card lands, and how far through leaving it is. +// --------------------------------------------------------------------------- + +/** + * A hash, not a random number. + * + * Two rounds of a linear congruential step with a shift between them, which is + * more than enough to stop neighbouring seeds producing neighbouring angles — + * the visible failure would be a whole suit landing at the same tilt. + */ +private fun scramble(seed: Int): Int { + var h = seed + repeat(2) { + h = h * 1_664_525 + 1_013_904_223 + h = h xor (h ushr 15) + } + return h +} + +/** + * A value in `[-1, 1)` for this [seed] and [salt], stable for the life of the + * program and identical between runs. [salt] separates the several independent + * numbers one card needs from a single seed. + */ +fun unitNoise(seed: Int, salt: Int): Float { + val bits = scramble(seed * 31 + salt * 0x5F5F) ushr 8 + return bits / 8_388_608f - 1f +} + +/** A seed for one card, optionally distinguished by where it is lying. */ +fun cardSeed(card: Card, place: Int = 0): Int = + (card.rank.ordinal * 4 + card.suit.ordinal) * 71 + place * 7919 + +/** + * Where a card has come to rest, and how it got there. + * + * All four scatters come off one seed, so the same card always lies the same + * way: [tiltDegrees] and the two drifts are where it ended up, [spinDegrees] is + * the turn it was still carrying on the way in, and [lateness] is its place in + * the queue when a group of cards moves together. + */ +@Immutable +data class CardRest( + val tiltDegrees: Float, + val driftX: Float, + val driftY: Float, + val spinDegrees: Float, + val lateness: Float, +) + +/** The resting scatter for [seed], within the bounds set at the top of the file. */ +fun cardRest(seed: Int): CardRest = CardRest( + tiltDegrees = unitNoise(seed, 1) * REST_TILT_DEGREES, + driftX = unitNoise(seed, 2) * REST_DRIFT_FRACTION, + driftY = unitNoise(seed, 3) * REST_DRIFT_FRACTION, + spinDegrees = unitNoise(seed, 4) * THROW_SPIN_DEGREES, + lateness = (unitNoise(seed, 5) + 1f) / 2f, +) + +/** The small tilt of a card sitting in a squared-up hand. */ +fun handTilt(seed: Int): Float = unitNoise(seed, 6) * HAND_TILT_DEGREES + +/** + * Where a card thrown by a seat comes in from, in card widths, for a four-seat + * table where relative seat 0 sits at the bottom and the rest run clockwise. + * A card arrives from beyond its own player's edge, which is the direction it + * would have been thrown from. + */ +fun seatOrigin(relative: Int, distance: Float = THROW_DISTANCE): Pair = + when (relative) { + 0 -> 0f to distance + 1 -> -distance to 0f + 2 -> 0f to -distance + else -> distance to 0f + } + +/** + * Where a card lands on a pile from, in card widths, when there is no seat to + * read a direction off — a discard pile takes cards from everyone. The angle is + * hashed rather than drawn, so a card always arrives from the same side. + */ +fun pileOrigin(seed: Int, distance: Float = THROW_DISTANCE): Pair { + val angle = (unitNoise(seed, 7) + 1f) * PI.toFloat() + return cos(angle) * distance to sin(angle) * distance +} + +/** + * Acceleration curve for cards being dragged off a table: slow to break away, + * then gone. Written out rather than taken from an easing class so the sweep + * arithmetic stays plain Kotlin and can be tested without a composition. + */ +private fun accelerate(x: Float): Float = x * x * (0.35f + 0.65f * x) + +/** + * How far through leaving a single card is, given how far through the sweep as + * a whole is. Cards start over a window of [spread] rather than together, so a + * trick gathers up in one motion rather than four cards moving in lockstep. + */ +fun sweepProgress(sweep: Float, rest: CardRest, spread: Float = SWEEP_SPREAD): Float { + val start = rest.lateness.coerceIn(0f, 1f) * spread.coerceIn(0f, 0.9f) + val raw = ((sweep - start) / (1f - start)).coerceIn(0f, 1f) + return accelerate(raw) +} + +private fun blend(from: Float, to: Float, fraction: Float): Float = + from + (to - from) * fraction + +// --------------------------------------------------------------------------- +// The Compose part. +// --------------------------------------------------------------------------- + +/** + * The animation-speed setting, so card motion follows the same slider as + * everything else. A composition local rather than a parameter because it would + * otherwise have to be threaded through every card screen and down into every + * pile inside them, to say one number that never changes mid-game. + */ +val LocalCardSpeed = staticCompositionLocalOf { 1f } + +/** A nominal duration, adjusted for the speed the player has chosen. */ +@Composable +fun motionMillis(nominal: Int): Int { + val speed = LocalCardSpeed.current.coerceIn(Settings.MIN_SPEED, Settings.MAX_SPEED) + return (nominal / speed).toInt().coerceAtLeast(1) +} + +/** + * Where a card is, and what it looks like, part way through being thrown. + * + * [alpha] is only ever 0 or 1, and only ever 0 for a card waiting its turn in a + * set that is being played together. Without it a delayed card would sit + * plainly visible two card widths off the table until its moment came, which + * reads as a card teleporting out and flying back rather than as one still in + * somebody's hand. + */ +@Immutable +data class CardLanding( + val offsetX: Float, + val offsetY: Float, + val rotation: Float, + val scale: Float, + val elevation: Dp, + val alpha: Float = 1f, +) + +/** + * Follows one card from the moment it is played until it has stopped moving. + * + * The whole flight is one underdamped spring on a single 0-to-1 value, and + * everything else is read off it. That is why the card overshoots its resting + * place and rocks back instead of easing to a halt: the spring passes 1 and + * returns, and the position, the angle and the height above the table all + * follow it there and back together. Driving them separately was tried and + * looked like three animations that happened to start at the same time. + * + * [key] is what counts as a new card arriving. It must change when the card in + * this place changes and at no other time, or the card either fails to animate + * or animates again every recomposition. + */ +@Composable +fun rememberCardLanding( + key: Any?, + rest: CardRest, + facingDegrees: Float, + fromX: Float, + fromY: Float, + cardWidthPx: Float, + durationMillis: Int, + delayMillis: Int = 0, +): CardLanding { + val settle = remember(key) { Animatable(0f) } + var launched by remember(key) { mutableStateOf(delayMillis <= 0) } + LaunchedEffect(key) { + if (delayMillis > 0) { + delay(delayMillis.toLong()) + launched = true + } + settle.animateTo( + targetValue = 1f, + animationSpec = spring( + dampingRatio = SETTLE_DAMPING, + stiffness = SETTLE_STIFFNESS * CARD_THROW_MILLIS / durationMillis, + ), + ) + } + + val travelled = settle.value + // Height above the table is the one thing that must not overshoot: a card + // that has landed cannot rise again, whatever the spring is still doing. + val landed = travelled.coerceIn(0f, 1f) + return CardLanding( + offsetX = blend(fromX, rest.driftX * cardWidthPx, travelled), + offsetY = blend(fromY, rest.driftY * cardWidthPx, travelled), + rotation = blend( + facingDegrees + rest.spinDegrees, + facingDegrees + rest.tiltDegrees, + travelled, + ), + scale = blend(THROW_SCALE, 1f, landed), + elevation = blend(THROW_ELEVATION_DP, REST_ELEVATION_DP, landed).dp, + alpha = if (launched) 1f else 0f, + ) +} + +/** Puts a card where its [landing] says it is. For cards drawn on their own. */ +fun Modifier.landed(landing: CardLanding): Modifier = this + .offset { IntOffset(landing.offsetX.roundToInt(), landing.offsetY.roundToInt()) } + .graphicsLayer { + scaleX = landing.scale + scaleY = landing.scale + rotationZ = landing.rotation + alpha = landing.alpha + } + +/** How many cards of a pile are worth drawing. Below this it is not a heap. */ +private const val PILE_DEPTH = 4 + +/** Holder for [rememberPileWatermark]. Deliberately not snapshot state. */ +private class PileWatermark(var value: Int) + +/** + * How much of a pile was already lying there rather than played onto it. + * + * Cards above this line get thrown on; cards below it are simply drawn where + * they lie. Two things have to be true and only one of them is obvious. The + * obvious one is that a pile you have just opened the screen on was not built + * in front of you, so none of it should fly in. The other is that a pile gets + * *shorter* between rounds — a new deal replaces a twenty-card heap with a + * single turned card — and a mark left at twenty would then sit above every + * card of the new round and quietly switch the animation off for the rest of + * the game. So the mark follows the pile down, and only ever down. + * + * A plain holder rather than snapshot state on purpose: this is written and + * read within one composition, and it must not be the reason anything + * recomposes. + */ +@Composable +private fun rememberPileWatermark(size: Int): Int { + val mark = remember { PileWatermark(size) } + if (size < mark.value) mark.value = size + return mark.value +} + +/** + * A pile of cards seen from above. + * + * Only the top few are drawn, each lying where it fell, which is enough to read + * as a heap without painting fifty cards nobody can see. Cards already on the + * pile when the screen opens are drawn settled — the pile was built before you + * got here — and only cards played after that are thrown on. + */ +@Composable +fun CardPile( + cards: List, + width: Dp, + modifier: Modifier = Modifier, + depth: Int = PILE_DEPTH, +) { + if (cards.isEmpty()) return + + // Position in the pile, not position in the window, so a card keeps its + // identity — and so its resting angle — as the window slides past it. + val bottom = (cards.size - depth).coerceAtLeast(0) + val alreadyDown = rememberPileWatermark(cards.size) + val cardWidthPx = with(LocalDensity.current) { width.toPx() } + val throwMillis = motionMillis(CARD_THROW_MILLIS) + + // Room for the scatter, so the outermost card is not clipped by whatever + // this pile has been placed inside. + val margin = width * REST_DRIFT_FRACTION + Box( + modifier = modifier + .padding(horizontal = margin, vertical = margin) + .width(width) + .height(width / CARD_ASPECT), + ) { + for (index in bottom until cards.size) { + key(index) { + val card = cards[index] + val rest = cardRest(cardSeed(card, index)) + val landing = if (index < alreadyDown) { + CardLanding( + offsetX = rest.driftX * cardWidthPx, + offsetY = rest.driftY * cardWidthPx, + rotation = rest.tiltDegrees, + scale = 1f, + elevation = REST_ELEVATION_DP.dp, + ) + } else { + val (fromX, fromY) = pileOrigin(cardSeed(card, index)) + rememberCardLanding( + key = index, + rest = rest, + facingDegrees = 0f, + fromX = fromX * cardWidthPx, + fromY = fromY * cardWidthPx, + cardWidthPx = cardWidthPx, + durationMillis = throwMillis, + ) + } + PlayingCardView( + card = card, + width = width, + elevation = landing.elevation, + modifier = Modifier.landed(landing), + ) + } + } + } +} + +/** + * A stock of face-down cards, drawn with enough of the pack under the top card + * to show there is something left in it. The offsets are a squared-up deck's + * worth — a couple of percent — rather than the scatter a played card gets. + */ +@Composable +fun CardBackStack( + count: Int, + width: Dp, + modifier: Modifier = Modifier, + depth: Int = 3, +) { + if (count <= 0) return + + val shown = count.coerceAtMost(depth) + val step = width * 0.018f + Box( + modifier = modifier + .padding(end = step * shown, bottom = step * shown) + .width(width) + .height(width / CARD_ASPECT), + ) { + // Bottom of the pack first, so the top card is drawn last and sits on top. + for (below in (shown - 1) downTo 0) { + CardBackView( + width = width, + elevation = if (below == 0) REST_ELEVATION_DP.dp else 0.dp, + modifier = Modifier + .offset(x = step * below, y = step * below) + .rotate(unitNoise(below, 8) * 0.8f), + ) + } + } +} diff --git a/app/src/main/java/org/prolibertate/games/ui/game/CrazyEightsScreen.kt b/app/src/main/java/org/prolibertate/games/ui/game/CrazyEightsScreen.kt index 98f4553..6511970 100644 --- a/app/src/main/java/org/prolibertate/games/ui/game/CrazyEightsScreen.kt +++ b/app/src/main/java/org/prolibertate/games/ui/game/CrazyEightsScreen.kt @@ -27,6 +27,7 @@ import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.rotate import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp @@ -181,7 +182,10 @@ private fun PileArea(state: CrazyEightsState, cardWidth: Dp) { color = MaterialTheme.colorScheme.onPrimary, ) } else { - CardBackView(width = cardWidth) + // A pack, not a single back: the stock is the one thing on + // this screen that is visibly a stack of card, and drawing + // it as one is most of what makes the table look real. + CardBackStack(count = state.stock.size, width = cardWidth) } } Column(horizontalAlignment = Alignment.CenterHorizontally) { @@ -190,7 +194,9 @@ private fun PileArea(state: CrazyEightsState, cardWidth: Dp) { style = MaterialTheme.typography.labelSmall, color = MaterialTheme.colorScheme.onPrimary, ) - state.topCard?.let { PlayingCardView(card = it, width = cardWidth) } + // The discard heap rather than only its top card, so a card + // played onto it lands on something. + CardPile(cards = state.discard, width = cardWidth) } } // An eight on top says nothing about what may be played next, so the @@ -331,6 +337,7 @@ private fun HandRow( enabled = canPlay, selected = card == pendingWild, caption = if (isWild(card)) "WILD" else null, + modifier = Modifier.rotate(handTilt(cardSeed(card))), onClick = { onTap(card) }, ) } diff --git a/app/src/main/java/org/prolibertate/games/ui/game/EuchreScreen.kt b/app/src/main/java/org/prolibertate/games/ui/game/EuchreScreen.kt index 3165a39..8f2bc38 100644 --- a/app/src/main/java/org/prolibertate/games/ui/game/EuchreScreen.kt +++ b/app/src/main/java/org/prolibertate/games/ui/game/EuchreScreen.kt @@ -1,5 +1,6 @@ package org.prolibertate.games.ui.game +import androidx.compose.animation.core.LinearEasing import androidx.compose.animation.core.animateFloatAsState import androidx.compose.animation.core.tween import androidx.compose.foundation.background @@ -25,6 +26,7 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.collectAsState import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue +import androidx.compose.runtime.key import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.setValue @@ -33,6 +35,7 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.draw.alpha import androidx.compose.ui.draw.rotate +import androidx.compose.ui.graphics.graphicsLayer import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.IntOffset @@ -184,9 +187,6 @@ private fun seatLabel(relative: Int): String = when (relative) { /** How long a completed trick rests on the table before being swept away. */ const val TRICK_HOLD_MILLIS = 1_000L -/** Duration of the sweep itself. */ -private const val TRICK_SWEEP_MILLIS = 450 - @Composable private fun TrickArea( state: EuchreState, @@ -206,9 +206,12 @@ private fun TrickArea( sweepStarted = true } } + // Driven linearly, because the shape of the sweep belongs to each card + // rather than to the clock: sweepProgress gives them their own start and + // their own acceleration off this one value. val sweep by animateFloatAsState( targetValue = if (sweeping && sweepStarted) 1f else 0f, - animationSpec = tween(durationMillis = TRICK_SWEEP_MILLIS), + animationSpec = tween(durationMillis = motionMillis(CARD_SWEEP_MILLIS), easing = LinearEasing), label = "trickSweep", ) @@ -220,7 +223,9 @@ private fun TrickArea( 2 -> 0f to -1f else -> 1f to 0f } - val travelPx = with(LocalDensity.current) { (cardWidth * 2.5f).toPx() } + val cardWidthPx = with(LocalDensity.current) { cardWidth.toPx() } + val travelPx = cardWidthPx * 2.5f + val throwMillis = motionMillis(CARD_THROW_MILLIS) Box(modifier = Modifier.fillMaxSize().padding(4.dp)) { @@ -249,45 +254,74 @@ private fun TrickArea( ) } + // Keyed by seat: one card per seat per trick, so this holds a card's + // flight steady while the trick fills up and while the finished trick + // is swapped for the copy that gets swept away. Without it the list + // position would decide identity and every card would jump back into + // the air whenever another one landed. cardsOnTable.forEach { played -> - val relative = relativePosition(played.seat, localSeat) - val alignment = when (relative) { - 0 -> Alignment.BottomCenter - 1 -> Alignment.CenterStart - 2 -> Alignment.TopCenter - else -> Alignment.CenterEnd - } - // Rotate to face the player who played it: the side seats end up - // landscape, the partner's card upside down from your side. - val rotation = when (relative) { - 1 -> 90f - 2 -> 180f - 3 -> -90f - else -> 0f - } + key(played.seat) { + val relative = relativePosition(played.seat, localSeat) + val alignment = when (relative) { + 0 -> Alignment.BottomCenter + 1 -> Alignment.CenterStart + 2 -> Alignment.TopCenter + else -> Alignment.CenterEnd + } + // Turned to face the player who played it: the side seats end + // up landscape, the partner's card upside down from your side. + val facing = when (relative) { + 1 -> 90f + 2 -> 180f + 3 -> -90f + else -> 0f + } - Column( - modifier = Modifier - .align(alignment) - .offset { - IntOffset( - x = (dirX * travelPx * sweep).roundToInt(), - y = (dirY * travelPx * sweep).roundToInt(), - ) - } - .alpha(1f - sweep), - horizontalAlignment = Alignment.CenterHorizontally, - ) { - PlayingCardView( - card = played.card, - width = cardWidth, - modifier = Modifier.rotate(rotation), - ) - Text( - text = seatLabel(relative), - style = MaterialTheme.typography.labelSmall, - color = MaterialTheme.colorScheme.onPrimary, + val rest = cardRest(cardSeed(played.card, played.seat)) + val (fromX, fromY) = seatOrigin(relative) + val landing = rememberCardLanding( + key = played.card, + rest = rest, + facingDegrees = facing, + fromX = fromX * cardWidthPx, + fromY = fromY * cardWidthPx, + cardWidthPx = cardWidthPx, + durationMillis = throwMillis, ) + val leaving = sweepProgress(sweep, rest) + + Column( + modifier = Modifier + .align(alignment) + .offset { + IntOffset( + x = (landing.offsetX + dirX * travelPx * leaving).roundToInt(), + y = (landing.offsetY + dirY * travelPx * leaving).roundToInt(), + ) + } + .graphicsLayer { + scaleX = landing.scale + scaleY = landing.scale + alpha = landing.alpha * (1f - leaving) + }, + horizontalAlignment = Alignment.CenterHorizontally, + ) { + PlayingCardView( + card = played.card, + width = cardWidth, + elevation = landing.elevation, + // The turn it picks up on the way out is the same turn + // it carried on the way in, so a card leaves the table + // the way it arrived. + modifier = Modifier + .rotate(landing.rotation + rest.spinDegrees * leaving), + ) + Text( + text = seatLabel(relative), + style = MaterialTheme.typography.labelSmall, + color = MaterialTheme.colorScheme.onPrimary, + ) + } } } @@ -449,6 +483,11 @@ private fun HandRow( card = card, width = cardWidth, enabled = canPlay || canDiscard, + // A hand that has been squared up is still not square. Two + // degrees is below the threshold at which anyone notices a + // tilt and above the one at which a row of cards stops + // looking printed. + modifier = Modifier.rotate(handTilt(cardSeed(card))), onClick = { when { canDiscard -> onPlay(Discard(card)) diff --git a/app/src/main/java/org/prolibertate/games/ui/game/GolfScreen.kt b/app/src/main/java/org/prolibertate/games/ui/game/GolfScreen.kt index 1a19ba6..5b3a782 100644 --- a/app/src/main/java/org/prolibertate/games/ui/game/GolfScreen.kt +++ b/app/src/main/java/org/prolibertate/games/ui/game/GolfScreen.kt @@ -252,7 +252,14 @@ private fun StockAndDiscard( Column(horizontalAlignment = Alignment.CenterHorizontally) { Text("Stock", style = MaterialTheme.typography.labelSmall, color = MaterialTheme.colorScheme.onPrimary) - CardBackView(width = cardWidth) + // Drawing the pack rather than one back also means an empty stock + // now looks empty, where before it showed a card that was not there. + if (state.stock.isEmpty()) { + Text("empty", style = MaterialTheme.typography.labelSmall, + color = MaterialTheme.colorScheme.onPrimary) + } else { + CardBackStack(count = state.stock.size, width = cardWidth) + } if (legal.contains(DrawFromStock)) { Button(onClick = { onMove(DrawFromStock) }) { Text("Draw") } } @@ -261,7 +268,7 @@ private fun StockAndDiscard( Column(horizontalAlignment = Alignment.CenterHorizontally) { Text("Discard", style = MaterialTheme.typography.labelSmall, color = MaterialTheme.colorScheme.onPrimary) - state.discard.lastOrNull()?.let { PlayingCardView(card = it, width = cardWidth) } + CardPile(cards = state.discard, width = cardWidth) if (legal.contains(DrawFromDiscard)) { Button(onClick = { onMove(DrawFromDiscard) }) { Text("Take") } } diff --git a/app/src/main/java/org/prolibertate/games/ui/game/HeartsScreen.kt b/app/src/main/java/org/prolibertate/games/ui/game/HeartsScreen.kt index dd70ea6..259d64b 100644 --- a/app/src/main/java/org/prolibertate/games/ui/game/HeartsScreen.kt +++ b/app/src/main/java/org/prolibertate/games/ui/game/HeartsScreen.kt @@ -18,6 +18,7 @@ import androidx.compose.material3.TextButton import androidx.compose.runtime.Composable import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue +import androidx.compose.runtime.key import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.setValue @@ -25,6 +26,8 @@ import androidx.compose.runtime.snapshots.SnapshotStateList import androidx.compose.runtime.mutableStateListOf import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.rotate +import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.unit.dp import org.prolibertate.games.game.cards.Card @@ -126,6 +129,7 @@ fun HeartsScreen( selected = chosen.contains(card), enabled = if (selectable) true else playable.contains(card), caption = pointsOf(card).takeIf { it > 0 }?.let { "−$it" }, + modifier = Modifier.rotate(handTilt(cardSeed(card))), onClick = { if (selectable) { toggle(chosen, card) @@ -188,13 +192,35 @@ private fun TrickView(state: HeartsState) { if (showing.isEmpty()) { Text("Nothing played yet.", style = MaterialTheme.typography.bodySmall) } else { + // Hearts lays its trick out in a row rather than round a table, so + // there is no seat direction to throw a card in from — each one + // comes in from its own side, which the seed decides and keeps. + val width = 46.dp + val widthPx = with(LocalDensity.current) { width.toPx() } + val throwMillis = motionMillis(CARD_THROW_MILLIS) FlowRow(horizontalArrangement = Arrangement.spacedBy(4.dp)) { showing.forEach { played -> - PlayingCardView( - card = played.card, - width = 46.dp, - caption = "seat ${played.seat}", - ) + key(played.seat) { + val seed = cardSeed(played.card, played.seat) + val rest = cardRest(seed) + val (fromX, fromY) = pileOrigin(seed) + val landing = rememberCardLanding( + key = played.card, + rest = rest, + facingDegrees = 0f, + fromX = fromX * widthPx, + fromY = fromY * widthPx, + cardWidthPx = widthPx, + durationMillis = throwMillis, + ) + PlayingCardView( + card = played.card, + width = width, + caption = "seat ${played.seat}", + elevation = landing.elevation, + modifier = Modifier.landed(landing), + ) + } } } } diff --git a/app/src/main/java/org/prolibertate/games/ui/game/KaiserScreen.kt b/app/src/main/java/org/prolibertate/games/ui/game/KaiserScreen.kt index c907d9b..fe51d58 100644 --- a/app/src/main/java/org/prolibertate/games/ui/game/KaiserScreen.kt +++ b/app/src/main/java/org/prolibertate/games/ui/game/KaiserScreen.kt @@ -1,5 +1,6 @@ package org.prolibertate.games.ui.game +import androidx.compose.animation.core.LinearEasing import androidx.compose.animation.core.animateFloatAsState import androidx.compose.animation.core.tween import androidx.compose.foundation.background @@ -25,6 +26,7 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue +import androidx.compose.runtime.key import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.setValue @@ -32,6 +34,7 @@ import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.alpha import androidx.compose.ui.draw.rotate +import androidx.compose.ui.graphics.graphicsLayer import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.unit.Dp @@ -181,9 +184,6 @@ private fun seatLabel(relative: Int): String = when (relative) { else -> "Right" } -/** Duration of the sweep towards the winner. */ -private const val SWEEP_MILLIS = 450 - @Composable private fun TrickArea( state: KaiserState, @@ -203,7 +203,7 @@ private fun TrickArea( } val sweep by animateFloatAsState( targetValue = if (sweeping && sweepStarted) 1f else 0f, - animationSpec = tween(durationMillis = SWEEP_MILLIS), + animationSpec = tween(durationMillis = motionMillis(CARD_SWEEP_MILLIS), easing = LinearEasing), label = "kaiserSweep", ) @@ -214,7 +214,9 @@ private fun TrickArea( 2 -> 0f to -1f else -> 1f to 0f } - val travelPx = with(LocalDensity.current) { (cardWidth * 2.5f).toPx() } + val cardWidthPx = with(LocalDensity.current) { cardWidth.toPx() } + val travelPx = cardWidthPx * 2.5f + val throwMillis = motionMillis(CARD_THROW_MILLIS) Box(modifier = Modifier.fillMaxSize().padding(4.dp)) { if (cardsOnTable.isEmpty()) { @@ -230,43 +232,64 @@ private fun TrickArea( } cardsOnTable.forEach { played -> - val relative = relativePosition(played.seat, localSeat) - val alignment = when (relative) { - 0 -> Alignment.BottomCenter - 1 -> Alignment.CenterStart - 2 -> Alignment.TopCenter - else -> Alignment.CenterEnd - } - val rotation = when (relative) { - 1 -> 90f - 2 -> 180f - 3 -> -90f - else -> 0f - } + key(played.seat) { + val relative = relativePosition(played.seat, localSeat) + val alignment = when (relative) { + 0 -> Alignment.BottomCenter + 1 -> Alignment.CenterStart + 2 -> Alignment.TopCenter + else -> Alignment.CenterEnd + } + val facing = when (relative) { + 1 -> 90f + 2 -> 180f + 3 -> -90f + else -> 0f + } - Column( - modifier = Modifier - .align(alignment) - .offset { - IntOffset( - x = (dirX * travelPx * sweep).roundToInt(), - y = (dirY * travelPx * sweep).roundToInt(), - ) - } - .alpha(1f - sweep), - horizontalAlignment = Alignment.CenterHorizontally, - ) { - PlayingCardView( - card = played.card, - width = cardWidth, - caption = counterCaption(played.card), - modifier = Modifier.rotate(rotation), - ) - Text( - text = seatLabel(relative), - style = MaterialTheme.typography.labelSmall, - color = MaterialTheme.colorScheme.onPrimary, + val rest = cardRest(cardSeed(played.card, played.seat)) + val (fromX, fromY) = seatOrigin(relative) + val landing = rememberCardLanding( + key = played.card, + rest = rest, + facingDegrees = facing, + fromX = fromX * cardWidthPx, + fromY = fromY * cardWidthPx, + cardWidthPx = cardWidthPx, + durationMillis = throwMillis, ) + val leaving = sweepProgress(sweep, rest) + + Column( + modifier = Modifier + .align(alignment) + .offset { + IntOffset( + x = (landing.offsetX + dirX * travelPx * leaving).roundToInt(), + y = (landing.offsetY + dirY * travelPx * leaving).roundToInt(), + ) + } + .graphicsLayer { + scaleX = landing.scale + scaleY = landing.scale + alpha = landing.alpha * (1f - leaving) + }, + horizontalAlignment = Alignment.CenterHorizontally, + ) { + PlayingCardView( + card = played.card, + width = cardWidth, + caption = counterCaption(played.card), + elevation = landing.elevation, + modifier = Modifier + .rotate(landing.rotation + rest.spinDegrees * leaving), + ) + Text( + text = seatLabel(relative), + style = MaterialTheme.typography.labelSmall, + color = MaterialTheme.colorScheme.onPrimary, + ) + } } } @@ -406,6 +429,7 @@ private fun HandRow( width = cardWidth, enabled = canPlay, caption = counterCaption(card), + modifier = Modifier.rotate(handTilt(cardSeed(card))), onClick = { if (canPlay) onPlay(PlayCard(card)) }, ) } diff --git a/app/src/main/java/org/prolibertate/games/ui/game/PresidentScreen.kt b/app/src/main/java/org/prolibertate/games/ui/game/PresidentScreen.kt index 8f8d8ff..5894bc8 100644 --- a/app/src/main/java/org/prolibertate/games/ui/game/PresidentScreen.kt +++ b/app/src/main/java/org/prolibertate/games/ui/game/PresidentScreen.kt @@ -20,8 +20,11 @@ import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue +import androidx.compose.runtime.key import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.rotate +import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp @@ -155,9 +158,36 @@ private fun PileView(state: PresidentState, cardWidth: Dp) { style = MaterialTheme.typography.labelSmall, color = MaterialTheme.colorScheme.onPrimary, ) + // The set on top of the pile. A set is played as one gesture but not as + // one card: they leave the hand together and land one after another, + // which is what the stagger off each card's own seed is for. + val onTop = state.pile.takeLast(state.setSize) + val topIndex = state.pile.size - onTop.size + val cardWidthPx = with(LocalDensity.current) { cardWidth.toPx() } + val throwMillis = motionMillis(CARD_THROW_MILLIS) Row(horizontalArrangement = Arrangement.spacedBy(6.dp)) { - state.pile.takeLast(state.setSize).forEach { card -> - PlayingCardView(card = card, width = cardWidth) + onTop.forEachIndexed { offsetInSet, card -> + val place = topIndex + offsetInSet + key(place) { + val rest = cardRest(cardSeed(card, place)) + val (fromX, fromY) = pileOrigin(cardSeed(card, place)) + val landing = rememberCardLanding( + key = place, + rest = rest, + facingDegrees = 0f, + fromX = fromX * cardWidthPx, + fromY = fromY * cardWidthPx, + cardWidthPx = cardWidthPx, + durationMillis = throwMillis, + delayMillis = (rest.lateness * throwMillis * 0.3f).toInt(), + ) + PlayingCardView( + card = card, + width = cardWidth, + elevation = landing.elevation, + modifier = Modifier.landed(landing), + ) + } } } Text( @@ -258,7 +288,11 @@ private fun HandView(state: PresidentState, localSeat: Int, cardWidth: Dp) { Text("—", style = MaterialTheme.typography.bodySmall) } hand.forEach { card -> - PlayingCardView(card = card, width = cardWidth * 0.8f) + PlayingCardView( + card = card, + width = cardWidth * 0.8f, + modifier = Modifier.rotate(handTilt(cardSeed(card))), + ) } } } diff --git a/app/src/main/java/org/prolibertate/games/ui/game/WizardScreen.kt b/app/src/main/java/org/prolibertate/games/ui/game/WizardScreen.kt index 56fcf7e..c780608 100644 --- a/app/src/main/java/org/prolibertate/games/ui/game/WizardScreen.kt +++ b/app/src/main/java/org/prolibertate/games/ui/game/WizardScreen.kt @@ -1,5 +1,6 @@ package org.prolibertate.games.ui.game +import androidx.compose.animation.core.LinearEasing import androidx.compose.animation.core.animateFloatAsState import androidx.compose.animation.core.tween import androidx.compose.foundation.background @@ -26,6 +27,7 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue +import androidx.compose.runtime.key import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.setValue @@ -33,6 +35,7 @@ import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.alpha import androidx.compose.ui.draw.rotate +import androidx.compose.ui.graphics.graphicsLayer import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.unit.Dp @@ -190,9 +193,6 @@ private fun BidTally(state: WizardState, localSeat: Int) { private fun seatLabel(seat: Int, localSeat: Int): String = if (seat == localSeat) "You" else "Seat $seat" -/** Duration of the sweep towards the winner. */ -private const val SWEEP_MILLIS = 450 - @Composable private fun TrickArea( state: WizardState, @@ -213,7 +213,7 @@ private fun TrickArea( } val sweep by animateFloatAsState( targetValue = if (sweeping && sweepStarted) 1f else 0f, - animationSpec = tween(durationMillis = SWEEP_MILLIS), + animationSpec = tween(durationMillis = motionMillis(CARD_SWEEP_MILLIS), easing = LinearEasing), label = "wizardSweep", ) @@ -226,7 +226,9 @@ private fun TrickArea( // Cards sit on a circle that fits inside whatever space the table has. val radius = (minOf(maxWidth, maxHeight) / 2 - cardWidth * 0.7f).coerceAtLeast(0.dp) val radiusPx = with(LocalDensity.current) { radius.toPx() } - val travelPx = with(LocalDensity.current) { (cardWidth * 2f).toPx() } + val cardWidthPx = with(LocalDensity.current) { cardWidth.toPx() } + val travelPx = cardWidthPx * 2f + val throwMillis = motionMillis(CARD_THROW_MILLIS) if (cardsOnTable.isEmpty()) { Text( @@ -241,37 +243,62 @@ private fun TrickArea( } cardsOnTable.forEach { played -> - val relative = ((played.seat - localSeat) % players + players) % players - val angle = 2.0 * PI * relative / players - // Relative seat 0 is you, at the bottom; the rest run clockwise. - val x = -sin(angle) * radiusPx - val y = cos(angle) * radiusPx - // Turn each card to face whoever played it. - val rotation = (relative * 360f / players) + key(played.seat) { + val relative = ((played.seat - localSeat) % players + players) % players + val angle = 2.0 * PI * relative / players + // Relative seat 0 is you, at the bottom; the rest run clockwise. + val x = -sin(angle) * radiusPx + val y = cos(angle) * radiusPx + // Turn each card to face whoever played it. + val facing = (relative * 360f / players) - Column( - modifier = Modifier - .align(Alignment.Center) - .offset { - IntOffset( - x = (x + -sin(winnerAngle) * travelPx * sweep).roundToInt(), - y = (y + cos(winnerAngle) * travelPx * sweep).roundToInt(), - ) - } - .alpha(1f - sweep), - horizontalAlignment = Alignment.CenterHorizontally, - ) { - PlayingCardView( - card = played.card, - width = cardWidth, - caption = specialCaption(played.card), - modifier = Modifier.rotate(rotation), - ) - Text( - text = seatLabel(played.seat, localSeat), - style = MaterialTheme.typography.labelSmall, - color = MaterialTheme.colorScheme.onPrimary, + // A card comes in along the ray it sits on, from further out — + // which on a round table is the direction its player is in, + // however many of them there are. + val rest = cardRest(cardSeed(played.card, played.seat)) + val landing = rememberCardLanding( + key = played.card, + rest = rest, + facingDegrees = facing, + fromX = (-sin(angle) * THROW_DISTANCE * cardWidthPx).toFloat(), + fromY = (cos(angle) * THROW_DISTANCE * cardWidthPx).toFloat(), + cardWidthPx = cardWidthPx, + durationMillis = throwMillis, ) + val leaving = sweepProgress(sweep, rest) + + Column( + modifier = Modifier + .align(Alignment.Center) + .offset { + IntOffset( + x = (x + landing.offsetX + + -sin(winnerAngle) * travelPx * leaving).roundToInt(), + y = (y + landing.offsetY + + cos(winnerAngle) * travelPx * leaving).roundToInt(), + ) + } + .graphicsLayer { + scaleX = landing.scale + scaleY = landing.scale + alpha = landing.alpha * (1f - leaving) + }, + horizontalAlignment = Alignment.CenterHorizontally, + ) { + PlayingCardView( + card = played.card, + width = cardWidth, + caption = specialCaption(played.card), + elevation = landing.elevation, + modifier = Modifier + .rotate(landing.rotation + rest.spinDegrees * leaving), + ) + Text( + text = seatLabel(played.seat, localSeat), + style = MaterialTheme.typography.labelSmall, + color = MaterialTheme.colorScheme.onPrimary, + ) + } } } @@ -395,6 +422,7 @@ private fun HandRow( width = cardWidth, enabled = canPlay, caption = specialCaption(card), + modifier = Modifier.rotate(handTilt(cardSeed(card))), onClick = { if (canPlay) onPlay(PlayCard(card)) }, ) } diff --git a/app/src/test/java/org/prolibertate/games/ui/game/CardPhysicsTest.kt b/app/src/test/java/org/prolibertate/games/ui/game/CardPhysicsTest.kt new file mode 100644 index 0000000..0457f35 --- /dev/null +++ b/app/src/test/java/org/prolibertate/games/ui/game/CardPhysicsTest.kt @@ -0,0 +1,165 @@ +package org.prolibertate.games.ui.game + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotEquals +import org.junit.Assert.assertTrue +import org.junit.Test +import kotlin.math.abs +import org.prolibertate.games.game.cards.Card +import org.prolibertate.games.game.cards.Decks +import org.prolibertate.games.game.cards.Rank +import org.prolibertate.games.game.cards.Suit + +/** + * The scatter under the card animations. + * + * Two properties matter and neither is visible in a screenshot. It has to be + * *bounded*, because the whole difference between cards on a table and cards + * thrown down a flight of stairs is the size of the numbers; and it has to be + * *stable*, because a card that picks a new angle on every recomposition + * twitches, and a card whose angle depends on where it sits in a list jumps + * whenever a card is played next to it. + */ +class CardPhysicsTest { + + private val deck = Decks.standard52() + + @Test + fun `noise stays inside minus one to one`() { + for (seed in -500..500) { + for (salt in 0..8) { + val n = unitNoise(seed, salt) + assertTrue("$seed/$salt gave $n", n >= -1f && n < 1f) + } + } + } + + @Test + fun `noise is the same every time it is asked`() { + assertEquals(unitNoise(12345, 3), unitNoise(12345, 3), 0f) + assertEquals(unitNoise(-7, 1), unitNoise(-7, 1), 0f) + } + + /** + * The salt is what lets one seed produce a tilt, two drifts, a spin and a + * queue position that are not all the same number. If it stopped separating + * them, every card would drift exactly as far as it tilted. + */ + @Test + fun `salts do not agree with each other`() { + val fromOneSeed = (0..8).map { unitNoise(4242, it) } + assertEquals(fromOneSeed.size, fromOneSeed.toSet().size) + } + + /** Sequential seeds are the normal case: adjacent cards in a hand. */ + @Test + fun `neighbouring seeds do not give neighbouring values`() { + val runs = (0 until 200).count { abs(unitNoise(it, 1) - unitNoise(it + 1, 1)) < 0.05f } + assertTrue("$runs of 200 neighbouring seeds landed together", runs < 30) + } + + @Test + fun `every card in a deck rests within the stated bounds`() { + deck.forEach { card -> + val rest = cardRest(cardSeed(card)) + assertTrue(abs(rest.tiltDegrees) <= REST_TILT_DEGREES) + assertTrue(abs(rest.driftX) <= REST_DRIFT_FRACTION) + assertTrue(abs(rest.driftY) <= REST_DRIFT_FRACTION) + assertTrue(abs(rest.spinDegrees) <= THROW_SPIN_DEGREES) + assertTrue(rest.lateness in 0f..1f) + assertTrue(abs(handTilt(cardSeed(card))) <= HAND_TILT_DEGREES) + } + } + + @Test + fun `a card lies the same way however often it is asked`() { + val card = Card(Rank.QUEEN, Suit.SPADES) + assertEquals(cardRest(cardSeed(card)), cardRest(cardSeed(card))) + assertEquals(cardRest(cardSeed(card, 3)), cardRest(cardSeed(card, 3))) + } + + /** + * Two cards lying at the same angle in the same place is the one thing that + * makes a scattered pile look printed rather than dealt. + */ + @Test + fun `no two cards in a deck share a seed`() { + val seeds = deck.map { cardSeed(it) } + assertEquals(seeds.size, seeds.toSet().size) + } + + /** + * A pile can hold the same card twice — Golf and Spider deal more than one + * deck — so where a card is lying has to be part of its seed. + */ + @Test + fun `the same card in two places lies two ways`() { + val card = Card(Rank.SEVEN, Suit.HEARTS) + assertNotEquals(cardRest(cardSeed(card, 0)), cardRest(cardSeed(card, 1))) + } + + /** Every seat's card arrives from that seat, and from a card-width or two out. */ + @Test + fun `a card comes in from its own player's side`() { + assertEquals(0f to THROW_DISTANCE, seatOrigin(0)) + assertEquals(-THROW_DISTANCE to 0f, seatOrigin(1)) + assertEquals(0f to -THROW_DISTANCE, seatOrigin(2)) + assertEquals(THROW_DISTANCE to 0f, seatOrigin(3)) + } + + @Test + fun `a pile card comes in from somewhere off the pile`() { + deck.forEach { card -> + val (x, y) = pileOrigin(cardSeed(card)) + val distance = kotlin.math.hypot(x, y) + assertEquals(THROW_DISTANCE, distance, 0.001f) + } + } + + @Test + fun `nothing has left the table before the sweep starts`() { + deck.take(8).forEach { card -> + assertEquals(0f, sweepProgress(0f, cardRest(cardSeed(card))), 0f) + } + } + + @Test + fun `everything has left the table by the end of the sweep`() { + deck.take(8).forEach { card -> + assertEquals(1f, sweepProgress(1f, cardRest(cardSeed(card))), 0.0001f) + } + } + + @Test + fun `a card only ever gets further from the table`() { + val rest = cardRest(cardSeed(Card(Rank.ACE, Suit.CLUBS))) + var previous = 0f + for (step in 0..100) { + val now = sweepProgress(step / 100f, rest) + assertTrue("went backwards at $step", now >= previous - 0.0001f) + assertTrue(now in 0f..1f) + previous = now + } + } + + /** + * The stagger: a trick gathers up as one motion, so the cards must not all + * be at the same point in it, and none of them may still be sitting there + * when the sweep is nearly over. + */ + @Test + fun `cards leave at different moments but none is left behind`() { + val hand = deck.take(4).map { cardRest(cardSeed(it)) } + val halfway = hand.map { sweepProgress(0.5f, it) } + assertEquals(halfway.size, halfway.toSet().size) + hand.forEach { assertTrue(sweepProgress(0.95f, it) > 0.5f) } + } + + /** A trick that never completes must not leave a card part way off the table. */ + @Test + fun `a sweep that never starts moves nothing`() { + deck.take(4).forEach { card -> + assertEquals(0f, sweepProgress(0f, cardRest(cardSeed(card)), spread = 0f), 0f) + } + } +} diff --git a/gradle.properties b/gradle.properties index cf29b77..8d326d4 100644 --- a/gradle.properties +++ b/gradle.properties @@ -22,4 +22,4 @@ android.nonTransitiveRClass=true # Only the series is set by hand. The number after it is the CI run number, so # it keeps climbing on its own and an APK's versionCode never goes backwards # when this changes. -plgVersionSeries=1.2 +plgVersionSeries=1.3 From a7427b0cb56b6829ab07345933ac590c29786e02 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 22:00:58 +0000 Subject: [PATCH 2/9] Play onto the pile rather than instead of it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit President drew only the set on top. Everything played before it was in the state and simply not on the screen, so beating a pair looked like the previous pair being taken away and replaced rather than covered. The pile is the point of the game — what is under the top set is the history of the round — and it was invisible. The whole pile is now drawn in one loop keyed by position in it. The top set is laid out square and side by side, because that is the set that has to be beaten and the only one that has to be readable; everything under it is pushed out towards the rim of the ground the top set covers, so a corner shows. Buried dead centre is buried, and then drawing it achieves nothing. Keying by position is what makes the transition work. A superseded set is the same composable it always was, so when its target moves from a row slot to the rim it animates from wherever it actually is — no bookkeeping about where it used to sit, and no card vanishing from the row to reappear under the heap a frame later. The throw stays relative to wherever a card belongs, so the two compose: a card lands in its slot, and is shoved aside from it later without the landing having to know. The heap scatter is also now its own pair of bounds, looser than the table's: eight degrees and fifteen per cent of a card width against four and a half and five. This was not cosmetic. At the tight bounds a discard pile of five cards was five cards hidden to within a twentieth of a card width of each other, which reads as one card — Crazy 8s and Golf were technically stacking and visibly not. A card keeps whichever scatter it was given for as long as it lies there, since a card that shuffled sideways the moment something landed on it would be a card moving on its own. Cribbage's series is the same shape of thing and was already keeping its cards, so it gains the landing to match; its hand gains the tilt the other hands have. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0132V9gE6DF9itz2z3Hn7BK9 --- .../prolibertate/games/ui/game/CardPhysics.kt | 60 ++++++- .../games/ui/game/CribbageScreen.kt | 34 +++- .../games/ui/game/PresidentScreen.kt | 155 ++++++++++++++---- .../games/ui/game/CardPhysicsTest.kt | 54 ++++++ 4 files changed, 261 insertions(+), 42 deletions(-) diff --git a/app/src/main/java/org/prolibertate/games/ui/game/CardPhysics.kt b/app/src/main/java/org/prolibertate/games/ui/game/CardPhysics.kt index 710bcb9..82319f7 100644 --- a/app/src/main/java/org/prolibertate/games/ui/game/CardPhysics.kt +++ b/app/src/main/java/org/prolibertate/games/ui/game/CardPhysics.kt @@ -24,6 +24,7 @@ import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.IntOffset import androidx.compose.ui.unit.dp import kotlin.math.PI +import kotlin.math.abs import kotlin.math.cos import kotlin.math.roundToInt import kotlin.math.sin @@ -65,6 +66,16 @@ const val REST_TILT_DEGREES = 4.5f /** Most a settled card misses its spot by, as a fraction of its own width. */ const val REST_DRIFT_FRACTION = 0.055f +/** + * The same two bounds for a card thrown onto a heap rather than laid in front + * of a player, which is a good deal less tidy. They are also what makes a heap + * legible: with the tight bounds a discard pile of five cards looks like one + * card, because the four underneath are hidden to within a twentieth of a card + * width of the top one. + */ +const val HEAP_TILT_DEGREES = 8f +const val HEAP_DRIFT_FRACTION = 0.15f + /** Extra turn a card carries while it is still travelling. */ const val THROW_SPIN_DEGREES = 15f @@ -160,14 +171,28 @@ data class CardRest( ) /** The resting scatter for [seed], within the bounds set at the top of the file. */ -fun cardRest(seed: Int): CardRest = CardRest( - tiltDegrees = unitNoise(seed, 1) * REST_TILT_DEGREES, - driftX = unitNoise(seed, 2) * REST_DRIFT_FRACTION, - driftY = unitNoise(seed, 3) * REST_DRIFT_FRACTION, +fun cardRest( + seed: Int, + tilt: Float = REST_TILT_DEGREES, + drift: Float = REST_DRIFT_FRACTION, +): CardRest = CardRest( + tiltDegrees = unitNoise(seed, 1) * tilt, + driftX = unitNoise(seed, 2) * drift, + driftY = unitNoise(seed, 3) * drift, spinDegrees = unitNoise(seed, 4) * THROW_SPIN_DEGREES, lateness = (unitNoise(seed, 5) + 1f) / 2f, ) +/** + * The same, at the looser bounds a heap gets. + * + * A card keeps this scatter for as long as it is on the heap, including once + * something has been played on top of it. Switching a card to a wider scatter + * the moment it stopped being the top card would make it shuffle sideways on + * its own, which is the one thing on a table that never happens. + */ +fun heapRest(seed: Int): CardRest = cardRest(seed, HEAP_TILT_DEGREES, HEAP_DRIFT_FRACTION) + /** The small tilt of a card sitting in a squared-up hand. */ fun handTilt(seed: Int): Float = unitNoise(seed, 6) * HAND_TILT_DEGREES @@ -195,6 +220,24 @@ fun pileOrigin(seed: Int, distance: Float = THROW_DISTANCE): Pair return cos(angle) * distance to sin(angle) * distance } +/** + * Where a card that has been played *on* lies, given how much ground the cards + * now covering it take up — half the width and half the height of it, in + * pixels. + * + * Pushed out towards the edge of that ground rather than scattered evenly + * across it. A card buried dead centre under a set of four is a card nobody can + * see, and the entire point of still drawing it is that you can tell the heap + * has a history. Out at the edge a corner shows, which is what a real heap + * looks like from above. + */ +fun buriedOffset(seed: Int, halfWidth: Float, halfHeight: Float): Pair { + val angle = (unitNoise(seed, 11) + 1f) * PI.toFloat() + // Never right at the rim and never far inside it. + val reach = 0.72f + 0.28f * abs(unitNoise(seed, 12)) + return cos(angle) * halfWidth * reach to sin(angle) * halfHeight * reach +} + /** * Acceleration curve for cards being dragged off a table: slow to break away, * then gone. Written out rather than taken from an easing class so the sweep @@ -213,7 +256,8 @@ fun sweepProgress(sweep: Float, rest: CardRest, spread: Float = SWEEP_SPREAD): F return accelerate(raw) } -private fun blend(from: Float, to: Float, fraction: Float): Float = +/** Straight interpolation, [fraction] being free to overshoot past one. */ +fun blend(from: Float, to: Float, fraction: Float): Float = from + (to - from) * fraction // --------------------------------------------------------------------------- @@ -324,7 +368,7 @@ fun Modifier.landed(landing: CardLanding): Modifier = this } /** How many cards of a pile are worth drawing. Below this it is not a heap. */ -private const val PILE_DEPTH = 4 +private const val PILE_DEPTH = 5 /** Holder for [rememberPileWatermark]. Deliberately not snapshot state. */ private class PileWatermark(var value: Int) @@ -378,7 +422,7 @@ fun CardPile( // Room for the scatter, so the outermost card is not clipped by whatever // this pile has been placed inside. - val margin = width * REST_DRIFT_FRACTION + val margin = width * HEAP_DRIFT_FRACTION Box( modifier = modifier .padding(horizontal = margin, vertical = margin) @@ -388,7 +432,7 @@ fun CardPile( for (index in bottom until cards.size) { key(index) { val card = cards[index] - val rest = cardRest(cardSeed(card, index)) + val rest = heapRest(cardSeed(card, index)) val landing = if (index < alreadyDown) { CardLanding( offsetX = rest.driftX * cardWidthPx, diff --git a/app/src/main/java/org/prolibertate/games/ui/game/CribbageScreen.kt b/app/src/main/java/org/prolibertate/games/ui/game/CribbageScreen.kt index 13400a4..920cd67 100644 --- a/app/src/main/java/org/prolibertate/games/ui/game/CribbageScreen.kt +++ b/app/src/main/java/org/prolibertate/games/ui/game/CribbageScreen.kt @@ -22,13 +22,16 @@ import androidx.compose.material3.TextButton import androidx.compose.runtime.Composable import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue +import androidx.compose.runtime.key import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.rotate import androidx.compose.ui.geometry.Offset import androidx.compose.ui.graphics.Color +import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp @@ -280,8 +283,34 @@ private fun TableArea(state: CribbageState, localSeat: Int, cardWidth: Dp) { color = ink, ) } - state.series.forEach { pegged -> - PlayingCardView(card = pegged.card, width = cardWidth * 0.7f) + // The series stays on the table until it is counted out, so each + // card is laid beside the last rather than replacing it. Keyed + // by its place in the series so only the card just pegged is + // thrown down. + val pegWidth = cardWidth * 0.7f + val pegWidthPx = with(LocalDensity.current) { pegWidth.toPx() } + val throwMillis = motionMillis(CARD_THROW_MILLIS) + state.series.forEachIndexed { place, pegged -> + key(place) { + val seed = cardSeed(pegged.card, place) + val rest = cardRest(seed) + val (fromX, fromY) = pileOrigin(seed) + val landing = rememberCardLanding( + key = place, + rest = rest, + facingDegrees = 0f, + fromX = fromX * pegWidthPx, + fromY = fromY * pegWidthPx, + cardWidthPx = pegWidthPx, + durationMillis = throwMillis, + ) + PlayingCardView( + card = pegged.card, + width = pegWidth, + elevation = landing.elevation, + modifier = Modifier.landed(landing), + ) + } } } } @@ -415,6 +444,7 @@ private fun HandRow( width = cardWidth, enabled = enabled, selected = card in chosen, + modifier = Modifier.rotate(handTilt(cardSeed(card))), onClick = { onTap(card) }, ) } diff --git a/app/src/main/java/org/prolibertate/games/ui/game/PresidentScreen.kt b/app/src/main/java/org/prolibertate/games/ui/game/PresidentScreen.kt index 5894bc8..5b9a2ac 100644 --- a/app/src/main/java/org/prolibertate/games/ui/game/PresidentScreen.kt +++ b/app/src/main/java/org/prolibertate/games/ui/game/PresidentScreen.kt @@ -1,5 +1,7 @@ package org.prolibertate.games.ui.game +import androidx.compose.animation.core.animateFloatAsState +import androidx.compose.animation.core.tween import androidx.compose.foundation.background import androidx.compose.foundation.horizontalScroll import androidx.compose.foundation.layout.Arrangement @@ -11,7 +13,9 @@ import androidx.compose.foundation.layout.FlowRow import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.offset import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.size import androidx.compose.foundation.rememberScrollState import androidx.compose.material3.Button import androidx.compose.material3.MaterialTheme @@ -24,10 +28,13 @@ import androidx.compose.runtime.key import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.rotate +import androidx.compose.ui.graphics.graphicsLayer import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.unit.Dp +import androidx.compose.ui.unit.IntOffset import androidx.compose.ui.unit.dp +import kotlin.math.roundToInt import org.prolibertate.games.game.president.PassTurn import org.prolibertate.games.game.president.PlayCards import org.prolibertate.games.game.president.PresidentMove @@ -37,6 +44,7 @@ import org.prolibertate.games.game.president.titleFor import org.prolibertate.games.net.MatchController import org.prolibertate.games.ui.PrimaryAction import org.prolibertate.games.ui.ScreenScaffold +import org.prolibertate.games.ui.common.CARD_ASPECT import org.prolibertate.games.ui.common.CardBackView import org.prolibertate.games.ui.common.PlayingCardView import org.prolibertate.games.ui.theme.FeltGreenDark @@ -158,38 +166,7 @@ private fun PileView(state: PresidentState, cardWidth: Dp) { style = MaterialTheme.typography.labelSmall, color = MaterialTheme.colorScheme.onPrimary, ) - // The set on top of the pile. A set is played as one gesture but not as - // one card: they leave the hand together and land one after another, - // which is what the stagger off each card's own seed is for. - val onTop = state.pile.takeLast(state.setSize) - val topIndex = state.pile.size - onTop.size - val cardWidthPx = with(LocalDensity.current) { cardWidth.toPx() } - val throwMillis = motionMillis(CARD_THROW_MILLIS) - Row(horizontalArrangement = Arrangement.spacedBy(6.dp)) { - onTop.forEachIndexed { offsetInSet, card -> - val place = topIndex + offsetInSet - key(place) { - val rest = cardRest(cardSeed(card, place)) - val (fromX, fromY) = pileOrigin(cardSeed(card, place)) - val landing = rememberCardLanding( - key = place, - rest = rest, - facingDegrees = 0f, - fromX = fromX * cardWidthPx, - fromY = fromY * cardWidthPx, - cardWidthPx = cardWidthPx, - durationMillis = throwMillis, - delayMillis = (rest.lateness * throwMillis * 0.3f).toInt(), - ) - PlayingCardView( - card = card, - width = cardWidth, - elevation = landing.elevation, - modifier = Modifier.landed(landing), - ) - } - } - } + PileHeap(state, cardWidth) Text( text = "Beat it with ${state.setSize} card(s)", style = MaterialTheme.typography.labelSmall, @@ -198,6 +175,120 @@ private fun PileView(state: PresidentState, cardWidth: Dp) { } } +/** How much of what is under the top set is worth drawing. */ +private const val BURIED_DEPTH = 5 + +/** + * The pile, as a pile. + * + * Every card played since the pile was last cleared is still lying there, and a + * new set is played *onto* them rather than instead of them — which is what the + * pile is for. Only the top set has to be readable, since that is the one that + * has to be beaten, so it is laid out square and side by side; everything under + * it is pushed out towards the edge of the ground the top set covers, so a + * corner of it shows. + * + * The whole pile is drawn in one loop keyed by position, which is what lets a + * set slide out of the way as the next one lands on it instead of vanishing + * from the row and reappearing under the heap. Each card animates from wherever + * it actually is to wherever it now belongs, so being buried needs no + * bookkeeping about where it used to sit. + */ +@Composable +private fun PileHeap(state: PresidentState, cardWidth: Dp) { + val density = LocalDensity.current + val cardWidthPx = with(density) { cardWidth.toPx() } + val cardHeightPx = cardWidthPx / CARD_ASPECT + val gapPx = with(density) { 6.dp.toPx() } + val throwMillis = motionMillis(CARD_THROW_MILLIS) + + // Where the cards of the top set sit: centred, side by side. + val step = cardWidthPx + gapPx + val slotOffset = (state.setSize - 1) / 2f + + // Half the ground the top set covers, which is what the cards underneath + // have to get out from under. + val halfWidth = (step * state.setSize) / 2f + val halfHeight = cardHeightPx * 0.34f + + val lowest = (state.pile.size - state.setSize - BURIED_DEPTH).coerceAtLeast(0) + + Box( + modifier = Modifier.size( + width = with(density) { (halfWidth * 2f + cardWidthPx * 0.9f).toDp() }, + height = with(density) { (cardHeightPx + halfHeight * 2f).toDp() }, + ), + contentAlignment = Alignment.Center, + ) { + for (place in lowest until state.pile.size) { + key(place) { + val card = state.pile[place] + val seed = cardSeed(card, place) + val depth = state.pile.size - 1 - place + val onTop = depth < state.setSize + + val rest = heapRest(seed) + val (buriedX, buriedY) = buriedOffset(seed, halfWidth, halfHeight) + val (targetX, targetY) = if (onTop) { + // Slot 0 is the leftmost card of the set, which is the + // deepest of them — they were dealt onto the pile in order. + val slot = state.setSize - 1 - depth + (slot - slotOffset) * step to 0f + } else { + buriedX to buriedY + } + + val slideSpec = tween(durationMillis = throwMillis) + val x by animateFloatAsState(targetX, slideSpec, label = "pileX") + val y by animateFloatAsState(targetY, slideSpec, label = "pileY") + // Buried cards are knocked further askew than the tidy set on + // top of them, which is the other half of reading as a heap. + val tilt by animateFloatAsState( + targetValue = if (onTop) rest.tiltDegrees * 0.4f else rest.tiltDegrees, + animationSpec = slideSpec, + label = "pileTilt", + ) + + // The throw is relative to wherever the card belongs, so the two + // compose: a card lands in its slot, and is later shoved aside + // from it without the landing having to know. + val (fromX, fromY) = pileOrigin(seed) + val landing = rememberCardLanding( + key = place, + rest = rest, + facingDegrees = 0f, + fromX = fromX * cardWidthPx, + fromY = fromY * cardWidthPx, + cardWidthPx = cardWidthPx, + durationMillis = throwMillis, + // A set leaves the hand together and lands one after + // another rather than as a block. + delayMillis = (rest.lateness * throwMillis * 0.3f).toInt(), + ) + + PlayingCardView( + card = card, + width = cardWidth, + elevation = landing.elevation, + modifier = Modifier + .offset { + IntOffset( + (x + landing.offsetX).roundToInt(), + (y + landing.offsetY).roundToInt(), + ) + } + .graphicsLayer { + scaleX = landing.scale + scaleY = landing.scale + rotationZ = tilt + (landing.rotation - rest.tiltDegrees) + alpha = landing.alpha + }, + ) + } + } + } +} + /** Everyone else, as card counts — their hands are never sent to this device. */ @Composable private fun OpponentRow(state: PresidentState, localSeat: Int) { diff --git a/app/src/test/java/org/prolibertate/games/ui/game/CardPhysicsTest.kt b/app/src/test/java/org/prolibertate/games/ui/game/CardPhysicsTest.kt index 0457f35..636aadc 100644 --- a/app/src/test/java/org/prolibertate/games/ui/game/CardPhysicsTest.kt +++ b/app/src/test/java/org/prolibertate/games/ui/game/CardPhysicsTest.kt @@ -98,6 +98,60 @@ class CardPhysicsTest { assertNotEquals(cardRest(cardSeed(card, 0)), cardRest(cardSeed(card, 1))) } + /** + * A heap is untidier than a card laid in front of a player, and has to be: + * at the tight bounds five cards on a discard pile are hidden to within a + * twentieth of a card width of each other and the pile looks like one card. + */ + @Test + fun `a heap lies looser than a table but still within bounds`() { + deck.forEach { card -> + val seed = cardSeed(card) + val heap = heapRest(seed) + val table = cardRest(seed) + assertTrue(abs(heap.tiltDegrees) <= HEAP_TILT_DEGREES) + assertTrue(abs(heap.driftX) <= HEAP_DRIFT_FRACTION) + assertTrue(abs(heap.driftY) <= HEAP_DRIFT_FRACTION) + assertTrue(abs(heap.tiltDegrees) >= abs(table.tiltDegrees)) + assertTrue(abs(heap.driftX) >= abs(table.driftX)) + } + } + + /** + * The two have to agree about everything except how far the card strayed, + * or a card would jump when it was drawn one way rather than the other. + */ + @Test + fun `a heap and a table agree about the throw`() { + deck.take(6).forEach { card -> + val seed = cardSeed(card) + assertEquals(cardRest(seed).spinDegrees, heapRest(seed).spinDegrees, 0f) + assertEquals(cardRest(seed).lateness, heapRest(seed).lateness, 0f) + } + } + + /** + * A buried card has to end up near the rim of whatever is covering it. One + * left in the middle is one nobody can see, and then drawing the pile as a + * pile achieves nothing. + */ + @Test + fun `a buried card ends up out where a corner of it shows`() { + deck.forEach { card -> + val (x, y) = buriedOffset(cardSeed(card), halfWidth = 100f, halfHeight = 40f) + val howFarOut = kotlin.math.hypot(x / 100f, y / 40f) + assertTrue("$card sat at $howFarOut of the way out", howFarOut >= 0.719f) + assertTrue(howFarOut <= 1.001f) + } + } + + /** Two cards buried under the same set must not end up in the same place. */ + @Test + fun `buried cards do not stack on each other`() { + val places = (0 until 6).map { buriedOffset(cardSeed(deck[it], it), 100f, 40f) } + assertEquals(places.size, places.toSet().size) + } + /** Every seat's card arrives from that seat, and from a card-width or two out. */ @Test fun `a card comes in from its own player's side`() { From cba66906bbb2dd7ddd91fb5c0d20a4c7f6a3ed40 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 22:10:04 +0000 Subject: [PATCH 3/9] Keep won tricks in front of the player who took them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A trick slid off towards its winner and stopped existing. Where it went was the only record that it had been won at all, and by the third trick of a hand there was nothing on the table to say who was sitting on what — which on a real table is the most-glanced-at thing in the game, and the whole reason cards are gathered rather than binned. Each player now has a face-down pile in front of them: one card back per trick, because a trick is squared into a single bundle and put down, and from above that is what you see. The piles sit beside each player's own card, one place clockwise along their edge, which is both where a player would actually put them and the one spot on each edge nothing else was using. Wizard seats three to six so its piles go half a seat clockwise round the same ring the cards are played on. The count follows the cards, not the rules. A trick is counted the instant its last card is played, which is a beat before the gather and two before the cards arrive; reading the state directly grew the winner's pile while the trick was still lying in the middle of the table, and the cards then slid over and landed on a pile that had already counted them. rememberArrivedTricks holds the count back until the sweep has actually run. The sweep changes to suit its new destination. A gathered card is squared down towards the size of the bundle it is joining, and stays whole until the last stretch instead of fading the whole way over: it now vanishes at the same moment the pile grows by one, and that swap is only invisible if there was something there to swap. Kaiser needed a per-seat trick count, which it had never had — it scores in points, so no rule wanted one. It is defaulted for the wire and carries a test, because a field no rule depends on is a field that can quietly stop being maintained. Hearts already keeps the cards themselves and counts them by the deal. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0132V9gE6DF9itz2z3Hn7BK9 --- .../games/game/kaiser/KaiserModel.kt | 8 + .../games/game/kaiser/KaiserRules.kt | 5 + .../prolibertate/games/ui/game/CardPhysics.kt | 208 ++++++++++++++++++ .../games/ui/game/EuchreScreen.kt | 22 +- .../games/ui/game/HeartsScreen.kt | 10 + .../games/ui/game/KaiserScreen.kt | 21 +- .../games/ui/game/WizardScreen.kt | 24 +- .../games/game/kaiser/KaiserRulesTest.kt | 33 +++ .../games/ui/game/CardPhysicsTest.kt | 28 +++ 9 files changed, 350 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/org/prolibertate/games/game/kaiser/KaiserModel.kt b/app/src/main/java/org/prolibertate/games/game/kaiser/KaiserModel.kt index 127e5d2..233e659 100644 --- a/app/src/main/java/org/prolibertate/games/game/kaiser/KaiserModel.kt +++ b/app/src/main/java/org/prolibertate/games/game/kaiser/KaiserModel.kt @@ -92,6 +92,14 @@ data class KaiserState( val lastTrickWinner: Int?, /** Points taken this hand, per team. */ val handPoints: List, + /** + * Tricks taken this hand, per seat. Kaiser scores in points rather than + * tricks, so nothing in the rules needs this — it is what the table draws + * in front of each player, and a player who has taken four tricks and no + * counters has still taken four tricks. Defaulted so a client on an older + * build can still read a state from the wire. + */ + val tricksWon: List = List(4) { 0 }, val scores: List, val log: List, ) diff --git a/app/src/main/java/org/prolibertate/games/game/kaiser/KaiserRules.kt b/app/src/main/java/org/prolibertate/games/game/kaiser/KaiserRules.kt index de33e81..5fe41c7 100644 --- a/app/src/main/java/org/prolibertate/games/game/kaiser/KaiserRules.kt +++ b/app/src/main/java/org/prolibertate/games/game/kaiser/KaiserRules.kt @@ -62,6 +62,7 @@ object KaiserRules : GameRules { leader = (dealer + 1) % 4, lastTrickWinner = null, handPoints = listOf(0, 0), + tricksWon = List(4) { 0 }, scores = scores, log = log, ) @@ -224,10 +225,14 @@ object KaiserRules : GameRules { val handPoints = played.handPoints.toMutableList() handPoints[teamOf(winner)] = handPoints[teamOf(winner)] + points + val tricksWon = played.tricksWon.toMutableList() + tricksWon[winner] = tricksWon[winner] + 1 + val afterTrick = played.copy( trick = emptyList(), completedTrick = trick, handPoints = handPoints, + tricksWon = tricksWon, leader = winner, turn = winner, lastTrickWinner = winner, diff --git a/app/src/main/java/org/prolibertate/games/ui/game/CardPhysics.kt b/app/src/main/java/org/prolibertate/games/ui/game/CardPhysics.kt index 82319f7..d766cca 100644 --- a/app/src/main/java/org/prolibertate/games/ui/game/CardPhysics.kt +++ b/app/src/main/java/org/prolibertate/games/ui/game/CardPhysics.kt @@ -3,6 +3,7 @@ package org.prolibertate.games.ui.game import androidx.compose.animation.core.Animatable import androidx.compose.animation.core.spring import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.BoxScope import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.offset import androidx.compose.foundation.layout.padding @@ -16,6 +17,7 @@ import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.runtime.staticCompositionLocalOf +import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.rotate import androidx.compose.ui.graphics.graphicsLayer @@ -119,6 +121,16 @@ const val CARD_SWEEP_MILLIS = 520 */ private const val SWEEP_SPREAD = 0.3f +/** + * The last stretch of the sweep, over which a gathered card fades out. It has + * to be late: the cards vanish exactly as the winner's pile grows by one, and + * that swap is only invisible if they are still whole most of the way there. + */ +private const val SWEEP_FADE = 0.42f + +/** How small four cards squared into a bundle end up against one card face. */ +private const val GATHERED_SCALE = 0.62f + // --------------------------------------------------------------------------- // The pure part: where a card lands, and how far through leaving it is. // --------------------------------------------------------------------------- @@ -260,6 +272,16 @@ fun sweepProgress(sweep: Float, rest: CardRest, spread: Float = SWEEP_SPREAD): F fun blend(from: Float, to: Float, fraction: Float): Float = from + (to - from) * fraction +/** + * How solid a card being gathered up still is. Whole for most of the journey + * and gone at the end of it, so it disappears at the moment the winner's pile + * grows rather than dissolving on the way over. + */ +fun gatherAlpha(leaving: Float): Float = ((1f - leaving) / SWEEP_FADE).coerceIn(0f, 1f) + +/** And how far it has been squared down into the bundle it is joining. */ +fun gatherScale(leaving: Float): Float = blend(1f, GATHERED_SCALE, leaving.coerceIn(0f, 1f)) + // --------------------------------------------------------------------------- // The Compose part. // --------------------------------------------------------------------------- @@ -370,6 +392,192 @@ fun Modifier.landed(landing: CardLanding): Modifier = this /** How many cards of a pile are worth drawing. Below this it is not a heap. */ private const val PILE_DEPTH = 5 +/** + * How many taken tricks are worth drawing separately. Past this the pile stops + * growing rather than climbing off the table — Wizard's last round is twenty + * tricks, and twenty bundles of stagger is not a pile, it is a staircase. + */ +private const val WON_STACK_DEPTH = 8 + +/** How far each taken trick sits off the one under it, in card widths. */ +private const val WON_STACK_STEP = 0.045f + +/** + * The tricks a player has taken, squared up and turned face down in front of + * them. + * + * One card back per trick rather than four, which is what a real table looks + * like: a trick is gathered, squared and put down as a single bundle, and from + * above you see the top card of each bundle and the edges of the rest. The + * count is the pile — it is the thing a player actually glances at to see how + * the hand is going, and it is why cards are kept rather than binned. + */ +@Composable +fun WonTrickStack( + tricks: Int, + width: Dp, + modifier: Modifier = Modifier, + facingDegrees: Float = 0f, + /** Where on the table these were gathered from, in pixels. */ + fromX: Float = 0f, + fromY: Float = 0f, +) { + if (tricks <= 0) return + + val shown = tricks.coerceAtMost(WON_STACK_DEPTH) + val step = width * WON_STACK_STEP + val widthPx = with(LocalDensity.current) { width.toPx() } + val throwMillis = motionMillis(CARD_THROW_MILLIS) + + Box( + modifier = modifier + .padding(end = step * shown, bottom = step * shown) + .width(width) + .height(width / CARD_ASPECT), + ) { + for (bundle in 0 until shown) { + key(bundle) { + // Each bundle is squared up, not thrown down, so it gets a + // fraction of the scatter a played card gets. + val rest = cardRest(bundle * 613, tilt = REST_TILT_DEGREES * 0.5f) + val top = bundle == shown - 1 + val landing = if (!top) { + CardLanding( + offsetX = 0f, + offsetY = 0f, + rotation = facingDegrees + rest.tiltDegrees, + scale = 1f, + elevation = REST_ELEVATION_DP.dp, + ) + } else { + // Only the newest bundle arrives; the ones under it have + // been sitting there since earlier in the hand. + rememberCardLanding( + key = tricks, + rest = rest.copy(driftX = 0f, driftY = 0f), + facingDegrees = facingDegrees, + fromX = fromX, + fromY = fromY, + cardWidthPx = widthPx, + durationMillis = throwMillis, + ) + } + CardBackView( + width = width, + elevation = landing.elevation, + modifier = Modifier + .offset(x = step * bundle, y = step * bundle) + .landed(landing), + ) + } + } + } +} + +/** How wide a taken-trick pile is drawn against a card on the table. */ +private const val WON_STACK_SCALE = 0.5f + +/** + * Everyone's taken tricks, at a table of four with partners opposite. + * + * Each pile sits beside its own player's card, one place clockwise along their + * edge, which is both where a real player would put them and the one spot on + * each edge that nothing else on the table is using. + */ +@Composable +fun BoxScope.TakenTricks(tricksWon: List, localSeat: Int, cardWidth: Dp) { + val stackWidth = cardWidth * WON_STACK_SCALE + val stackWidthPx = with(LocalDensity.current) { stackWidth.toPx() } + + for (seat in 0 until 4) { + val relative = ((seat - localSeat) % 4 + 4) % 4 + val alignment = when (relative) { + 0 -> Alignment.BottomCenter + 1 -> Alignment.CenterStart + 2 -> Alignment.TopCenter + else -> Alignment.CenterEnd + } + val (nudgeX, nudgeY) = when (relative) { + 0 -> cardWidth * 1.6f to 0.dp + 1 -> 0.dp to cardWidth * 1.4f + 2 -> -(cardWidth * 1.6f) to 0.dp + else -> 0.dp to -(cardWidth * 1.4f) + } + // Gathered in from the middle of the table, which is the opposite way + // from the edge this player sits at. + val (outX, outY) = seatOrigin(relative, 2.5f) + + WonTrickStack( + tricks = tricksWon.getOrElse(seat) { 0 }, + width = stackWidth, + facingDegrees = relative * 90f, + fromX = -outX * stackWidthPx, + fromY = -outY * stackWidthPx, + modifier = Modifier.align(alignment).offset(x = nudgeX, y = nudgeY), + ) + } +} + +/** + * The same, round a table of any size: each pile sits half a seat clockwise + * from its player's card, just inside the ring the cards are played on. + */ +@Composable +fun BoxScope.TakenTricksAround( + tricksWon: List, + localSeat: Int, + players: Int, + cardWidth: Dp, + radiusPx: Float, +) { + val stackWidth = cardWidth * WON_STACK_SCALE + val stackWidthPx = with(LocalDensity.current) { stackWidth.toPx() } + + for (seat in 0 until players) { + val relative = ((seat - localSeat) % players + players) % players + val angle = 2.0 * PI * relative / players + PI / players + val x = -sin(angle) * radiusPx + val y = cos(angle) * radiusPx + + WonTrickStack( + tricks = tricksWon.getOrElse(seat) { 0 }, + width = stackWidth, + facingDegrees = relative * 360f / players, + fromX = (sin(angle) * stackWidthPx * 2.5f).toFloat(), + fromY = (-cos(angle) * stackWidthPx * 2.5f).toFloat(), + modifier = Modifier + .align(Alignment.Center) + .offset { IntOffset(x.roundToInt(), y.roundToInt()) }, + ) + } +} + +/** + * Trick counts, held back until the cards have actually got there. + * + * The rules count a trick the instant its last card is played, which is a beat + * before the cards are gathered and two before they arrive. Reading the count + * straight from the state would grow the winner's pile while the trick was + * still lying in the middle of the table, and the cards would then slide over + * and land on a pile that had already counted them. + */ +@Composable +fun rememberArrivedTricks( + tricksWon: List, + sweeping: Boolean, + sweepStarted: Boolean, + sweepMillis: Int, +): List { + var arrived by remember { mutableStateOf(tricksWon) } + LaunchedEffect(tricksWon, sweeping, sweepStarted) { + // Still on the table waiting to be read: the pile has not won it yet. + if (sweeping && !sweepStarted) return@LaunchedEffect + if (sweeping) delay(sweepMillis.toLong()) + arrived = tricksWon + } + return arrived +} + /** Holder for [rememberPileWatermark]. Deliberately not snapshot state. */ private class PileWatermark(var value: Int) diff --git a/app/src/main/java/org/prolibertate/games/ui/game/EuchreScreen.kt b/app/src/main/java/org/prolibertate/games/ui/game/EuchreScreen.kt index 8f2bc38..40bfe03 100644 --- a/app/src/main/java/org/prolibertate/games/ui/game/EuchreScreen.kt +++ b/app/src/main/java/org/prolibertate/games/ui/game/EuchreScreen.kt @@ -227,8 +227,21 @@ private fun TrickArea( val travelPx = cardWidthPx * 2.5f val throwMillis = motionMillis(CARD_THROW_MILLIS) + // The count follows the cards rather than the rules, so a pile grows as the + // trick reaches it and not the moment the last card was played. + val arrived = rememberArrivedTricks( + tricksWon = state.tricksWon, + sweeping = sweeping, + sweepStarted = sweepStarted, + sweepMillis = motionMillis(CARD_SWEEP_MILLIS), + ) + Box(modifier = Modifier.fillMaxSize().padding(4.dp)) { + // What each player has taken, face down in front of them. Drawn first + // so a trick still being gathered passes over the pile it is joining. + TakenTricks(arrived, localSeat, cardWidth) + // The turn card sits in the middle of the table during bidding. state.upCard?.let { up -> Column( @@ -300,9 +313,12 @@ private fun TrickArea( ) } .graphicsLayer { - scaleX = landing.scale - scaleY = landing.scale - alpha = landing.alpha * (1f - leaving) + // Squared down into the bundle it is joining, and + // still whole until it gets there. + val gathered = landing.scale * gatherScale(leaving) + scaleX = gathered + scaleY = gathered + alpha = landing.alpha * gatherAlpha(leaving) }, horizontalAlignment = Alignment.CenterHorizontally, ) { diff --git a/app/src/main/java/org/prolibertate/games/ui/game/HeartsScreen.kt b/app/src/main/java/org/prolibertate/games/ui/game/HeartsScreen.kt index 259d64b..53bbcb2 100644 --- a/app/src/main/java/org/prolibertate/games/ui/game/HeartsScreen.kt +++ b/app/src/main/java/org/prolibertate/games/ui/game/HeartsScreen.kt @@ -240,6 +240,16 @@ private fun ScoreRow(state: HeartsState, localSeat: Int) { style = MaterialTheme.typography.labelSmall, fontWeight = if (seat == localSeat) FontWeight.Bold else FontWeight.Normal, ) + // The tricks this player has taken, face down in front of them. + // Hearts keeps the cards themselves rather than a count, since + // it is the cards that are scored at the end of the round, so + // the number of tricks is however many players-worth of them + // there are. + WonTrickStack( + tricks = state.taken.getOrElse(seat) { emptyList() }.size / + state.options.playerCount, + width = 26.dp, + ) Text("$score", fontWeight = FontWeight.Bold) // What this round has cost so far, which is the number anybody // actually watches while a hand is being played. diff --git a/app/src/main/java/org/prolibertate/games/ui/game/KaiserScreen.kt b/app/src/main/java/org/prolibertate/games/ui/game/KaiserScreen.kt index fe51d58..3568f59 100644 --- a/app/src/main/java/org/prolibertate/games/ui/game/KaiserScreen.kt +++ b/app/src/main/java/org/prolibertate/games/ui/game/KaiserScreen.kt @@ -218,7 +218,21 @@ private fun TrickArea( val travelPx = cardWidthPx * 2.5f val throwMillis = motionMillis(CARD_THROW_MILLIS) + // Kaiser scores in points rather than tricks, but the cards are still won + // and still kept, and how many each player is sitting on is worth seeing. + val arrived = rememberArrivedTricks( + tricksWon = state.tricksWon, + sweeping = sweeping, + sweepStarted = sweepStarted, + sweepMillis = motionMillis(CARD_SWEEP_MILLIS), + ) + Box(modifier = Modifier.fillMaxSize().padding(4.dp)) { + + // Drawn first, so a trick still being gathered passes over the pile it + // is on its way to joining. + TakenTricks(arrived, localSeat, cardWidth) + if (cardsOnTable.isEmpty()) { Text( text = when { @@ -270,9 +284,10 @@ private fun TrickArea( ) } .graphicsLayer { - scaleX = landing.scale - scaleY = landing.scale - alpha = landing.alpha * (1f - leaving) + val gathered = landing.scale * gatherScale(leaving) + scaleX = gathered + scaleY = gathered + alpha = landing.alpha * gatherAlpha(leaving) }, horizontalAlignment = Alignment.CenterHorizontally, ) { diff --git a/app/src/main/java/org/prolibertate/games/ui/game/WizardScreen.kt b/app/src/main/java/org/prolibertate/games/ui/game/WizardScreen.kt index c780608..6227fc7 100644 --- a/app/src/main/java/org/prolibertate/games/ui/game/WizardScreen.kt +++ b/app/src/main/java/org/prolibertate/games/ui/game/WizardScreen.kt @@ -230,6 +230,23 @@ private fun TrickArea( val travelPx = cardWidthPx * 2f val throwMillis = motionMillis(CARD_THROW_MILLIS) + val arrived = rememberArrivedTricks( + tricksWon = state.tricksWon, + sweeping = sweeping, + sweepStarted = sweepStarted, + sweepMillis = motionMillis(CARD_SWEEP_MILLIS), + ) + + // What each player has taken, face down beside their own place at the + // ring. Drawn first so a trick being gathered passes over its pile. + TakenTricksAround( + tricksWon = arrived, + localSeat = localSeat, + players = players, + cardWidth = cardWidth, + radiusPx = radiusPx, + ) + if (cardsOnTable.isEmpty()) { Text( text = when { @@ -279,9 +296,10 @@ private fun TrickArea( ) } .graphicsLayer { - scaleX = landing.scale - scaleY = landing.scale - alpha = landing.alpha * (1f - leaving) + val gathered = landing.scale * gatherScale(leaving) + scaleX = gathered + scaleY = gathered + alpha = landing.alpha * gatherAlpha(leaving) }, horizontalAlignment = Alignment.CenterHorizontally, ) { diff --git a/app/src/test/java/org/prolibertate/games/game/kaiser/KaiserRulesTest.kt b/app/src/test/java/org/prolibertate/games/game/kaiser/KaiserRulesTest.kt index 37ab58a..84876c6 100644 --- a/app/src/test/java/org/prolibertate/games/game/kaiser/KaiserRulesTest.kt +++ b/app/src/test/java/org/prolibertate/games/game/kaiser/KaiserRulesTest.kt @@ -250,6 +250,39 @@ class KaiserRulesTest { assertEquals("all four held for display", 4, state.completedTrick.size) } + /** + * Kaiser scores in points, not tricks, so nothing in the rules reads this + * count — the table does, to draw the cards each player has taken. That is + * exactly why it is worth a test: a field no rule depends on is a field + * that can quietly stop being maintained. + */ + @Test + fun `tricks are counted to the seat that took them`() { + // Everything is clubs and nothing is trump, so each trick simply goes + // to the highest card in it: the ace, then the king. + val hands = listOf( + listOf(Card(Rank.ACE, Suit.CLUBS), Card(Rank.SEVEN, Suit.CLUBS)), + listOf(Card(Rank.EIGHT, Suit.CLUBS), Card(Rank.NINE, Suit.CLUBS)), + listOf(Card(Rank.TEN, Suit.CLUBS), Card(Rank.JACK, Suit.CLUBS)), + listOf(Card(Rank.QUEEN, Suit.CLUBS), Card(Rank.KING, Suit.CLUBS)), + ) + var state = playingState(hands, trump = Suit.SPADES) + assertEquals("nothing taken at the deal", listOf(0, 0, 0, 0), state.tricksWon) + + repeat(4) { + val seat = KaiserRules.currentSeat(state)!! + state = KaiserRules.applyMove(state, seat, PlayCard(state.hands[seat].first())) + } + assertEquals("the ace takes the first", listOf(1, 0, 0, 0), state.tricksWon) + + repeat(4) { + val seat = KaiserRules.currentSeat(state)!! + state = KaiserRules.applyMove(state, seat, PlayCard(state.hands[seat].first())) + } + assertEquals("the king takes the second", listOf(1, 0, 0, 1), state.tricksWon) + assertEquals("and they add up", 2, state.tricksWon.sum()) + } + // -- Scoring ------------------------------------------------------------ @Test diff --git a/app/src/test/java/org/prolibertate/games/ui/game/CardPhysicsTest.kt b/app/src/test/java/org/prolibertate/games/ui/game/CardPhysicsTest.kt index 636aadc..4eee44b 100644 --- a/app/src/test/java/org/prolibertate/games/ui/game/CardPhysicsTest.kt +++ b/app/src/test/java/org/prolibertate/games/ui/game/CardPhysicsTest.kt @@ -209,6 +209,34 @@ class CardPhysicsTest { hand.forEach { assertTrue(sweepProgress(0.95f, it) > 0.5f) } } + /** + * A gathered card has to still be whole when it reaches the winner's pile, + * because it vanishes at the same moment the pile grows by one and that + * swap is only invisible if there was something there to swap. + */ + @Test + fun `a gathered card is whole most of the way and gone at the end`() { + assertEquals(1f, gatherAlpha(0f), 0f) + assertEquals(1f, gatherAlpha(0.5f), 0f) + assertEquals(0f, gatherAlpha(1f), 0f) + var previous = 1f + for (step in 0..100) { + val now = gatherAlpha(step / 100f) + assertTrue("went back up at $step", now <= previous + 0.0001f) + assertTrue(now in 0f..1f) + previous = now + } + } + + /** And is squared down into the bundle rather than arriving full size. */ + @Test + fun `a gathered card is squared down on the way`() { + assertEquals(1f, gatherScale(0f), 0f) + assertTrue(gatherScale(1f) < 1f) + assertTrue(gatherScale(1f) > 0.5f) + assertTrue(gatherScale(0.5f) < gatherScale(0.25f)) + } + /** A trick that never completes must not leave a card part way off the table. */ @Test fun `a sweep that never starts moves nothing`() { From f8f02dd93e9da68a5829ece53c5b9ad706688b34 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Aug 2026 23:52:39 +0000 Subject: [PATCH 4/9] Make LAN play work over a phone's hotspot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two phones on one phone's hotspot could not find or reach each other. Nothing was wrong with the Wi-Fi. There were three faults, and the first one is the whole bug. A hotspot has no internet behind it, and Android takes that literally: the Wi-Fi network fails its connectivity check, stays unvalidated, and mobile data remains the *default* network. Every socket opened without naming a network is therefore routed out over cellular — where 192.168.43.1 means nothing. The join socket is now bound to the Wi-Fi network before it connects, and the search for that network deliberately does not ask whether it has internet, because asking is what breaks it. Second, the manifest has asked for CHANGE_WIFI_MULTICAST_STATE since the beginning, with a comment saying discovery needs it, and nothing ever took the lock. Without it the radio drops multicast frames not addressed to this device before the system sees them, and mDNS is multicast. Both advertising and discovering now hold it. Third, and not fixable: a phone sharing its own hotspot does not reliably advertise over the tethering interface, whatever the other end does. So the host now shows where it is and the joiner can type it in. That path is offered always rather than kept back for when discovery has failed, because discovery here does not fail loudly — it just never finds anything, and a screen saying only "no games found" leaves nowhere to go. Hosts take a known port when they can get one, so what gets typed is an address and not an address and five more digits. The lobby scrolls instead of filling. Its lazy lists took the whole screen for a handful of rows and pushed the Start button off the bottom; they would have done the same to the address box. Parsing what somebody types off another phone's screen is the one pure piece of this and is tested, including that what a host displays is exactly what the other end accepts. The rest needs two phones and a hotspot, which CI does not have. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0132V9gE6DF9itz2z3Hn7BK9 --- .../prolibertate/games/net/LanTransport.kt | 36 ++++- .../prolibertate/games/net/LobbyController.kt | 41 +++++ .../prolibertate/games/net/LocalNetwork.kt | 145 ++++++++++++++++++ .../org/prolibertate/games/net/Transport.kt | 22 +++ .../org/prolibertate/games/ui/LobbyScreen.kt | 144 ++++++++++++----- .../games/net/ManualAddressTest.kt | 106 +++++++++++++ 6 files changed, 452 insertions(+), 42 deletions(-) create mode 100644 app/src/main/java/org/prolibertate/games/net/LocalNetwork.kt create mode 100644 app/src/test/java/org/prolibertate/games/net/ManualAddressTest.kt diff --git a/app/src/main/java/org/prolibertate/games/net/LanTransport.kt b/app/src/main/java/org/prolibertate/games/net/LanTransport.kt index 506b550..941ba87 100644 --- a/app/src/main/java/org/prolibertate/games/net/LanTransport.kt +++ b/app/src/main/java/org/prolibertate/games/net/LanTransport.kt @@ -7,6 +7,9 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.channels.awaitClose import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.callbackFlow import kotlinx.coroutines.isActive import kotlinx.coroutines.launch @@ -22,7 +25,9 @@ import java.util.Collections * * NSD is used rather than a UDP broadcast ping because it works across the * Wi-Fi isolation settings most home routers ship with, and because Android - * gives us the resolver for free. + * gives us the resolver for free. It is not, however, trusted to be enough: + * see [endpoint] and [LocalNetwork] for what a phone's own hotspot does to + * both discovery and routing, and for the way round it. */ class LanTransport(private val context: Context) : Transport { @@ -31,10 +36,17 @@ class LanTransport(private val context: Context) : Transport { private val nsdManager: NsdManager get() = context.getSystemService(Context.NSD_SERVICE) as NsdManager + private val multicast = MulticastGuard(context) + private var serverSocket: ServerSocket? = null private var registrationListener: NsdManager.RegistrationListener? = null private var discoveryListener: NsdManager.DiscoveryListener? = null + private val _endpoint = MutableStateFlow(null) + + /** Where this device is listening while hosting, so it can be read out. */ + val endpoint: StateFlow = _endpoint.asStateFlow() + override fun isAvailable(): Boolean = true // ----------------------------------------------------------------------- @@ -42,9 +54,15 @@ class LanTransport(private val context: Context) : Transport { // ----------------------------------------------------------------------- override fun host(displayName: String, scope: CoroutineScope): Flow = callbackFlow { - // Port 0 lets the OS pick a free port, which is then advertised. - val server = ServerSocket(0) + // The known port when it can be had, so that joining by hand needs an + // address and nothing else; anything free if something already has it. + val server = runCatching { ServerSocket(DEFAULT_HOST_PORT) } + .getOrElse { ServerSocket(0) } serverSocket = server + _endpoint.value = HostEndpoint(localIpv4Addresses(), server.localPort) + + // Advertising is mDNS, and mDNS is multicast. + multicast.acquire() val serviceInfo = NsdServiceInfo().apply { serviceName = "$SERVICE_NAME @ $displayName" @@ -82,6 +100,7 @@ class LanTransport(private val context: Context) : Transport { acceptJob.cancel() stopAdvertising() runCatching { server.close() } + _endpoint.value = null } } @@ -163,6 +182,10 @@ class LanTransport(private val context: Context) : Transport { } discoveryListener = listener + // Listening for mDNS replies needs the multicast filter held open just + // as much as answering does. + multicast.acquire() + publish() runCatching { nsdManager.discoverServices(SERVICE_TYPE, NsdManager.PROTOCOL_DNS_SD, listener) @@ -178,6 +201,11 @@ class LanTransport(private val context: Context) : Transport { override suspend fun join(host: DiscoveredHost, scope: CoroutineScope): Connection = withContext(Dispatchers.IO) { val socket = Socket() + // Before connecting, and this is the whole fix for hotspot play: a + // hotspot has no internet behind it, so Android leaves mobile data + // as the default network and an unbound socket goes out over + // cellular looking for a 192.168 address. + bindToWifi(context, socket) socket.connect(InetSocketAddress(host.address, host.port), CONNECT_TIMEOUT_MS) socket.tcpNoDelay = true StreamConnection( @@ -205,8 +233,10 @@ class LanTransport(private val context: Context) : Transport { override fun stop() { stopAdvertising() stopDiscovery() + multicast.release() runCatching { serverSocket?.close() } serverSocket = null + _endpoint.value = null } private companion object { diff --git a/app/src/main/java/org/prolibertate/games/net/LobbyController.kt b/app/src/main/java/org/prolibertate/games/net/LobbyController.kt index 22cefd2..35f5ea9 100644 --- a/app/src/main/java/org/prolibertate/games/net/LobbyController.kt +++ b/app/src/main/java/org/prolibertate/games/net/LobbyController.kt @@ -34,6 +34,8 @@ class LobbyController( val hostName: String = "", val seats: List = emptyList(), val discovered: List = emptyList(), + /** Where this device is listening while hosting, for reading out. */ + val endpoint: HostEndpoint? = null, val message: String? = null, val started: TableConfig? = null, ) @@ -91,6 +93,15 @@ class LobbyController( } } } + + // Discovery is not guaranteed — a phone sharing its own hotspot cannot + // be relied on to advertise over the tethering interface — so where the + // host is listening is put on its screen to be read out. + jobs += scope.launch { + lan.endpoint.collect { endpoint -> + _state.value = _state.value.copy(endpoint = endpoint) + } + } } private fun onClientConnected(connection: Connection) { @@ -205,6 +216,36 @@ class LobbyController( } } + /** + * Joins a host at an address somebody typed in. + * + * The way in when discovery cannot work. Two phones on one phone's hotspot + * is exactly that case: mDNS over a tethering interface is unreliable in + * both directions, and no amount of listening harder fixes it, but the + * address is right there on the host's screen and a TCP connection to it + * works perfectly well once the socket is pointed at the right network. + */ + fun joinAt(typed: String, displayName: String, peerId: String) { + val parsed = parseManualAddress(typed) + if (parsed == null) { + _state.value = _state.value.copy( + message = "That doesn't look like an address. Try 192.168.43.1", + ) + return + } + join( + host = DiscoveredHost( + id = "lan:${parsed.address}:${parsed.port}", + name = parsed.address, + kind = TransportKind.LAN, + address = parsed.address, + port = parsed.port, + ), + displayName = displayName, + peerId = peerId, + ) + } + fun join(host: DiscoveredHost, displayName: String, peerId: String) { jobs += scope.launch { _state.value = _state.value.copy(message = "Connecting to ${host.name}…") diff --git a/app/src/main/java/org/prolibertate/games/net/LocalNetwork.kt b/app/src/main/java/org/prolibertate/games/net/LocalNetwork.kt new file mode 100644 index 0000000..cb8c9f7 --- /dev/null +++ b/app/src/main/java/org/prolibertate/games/net/LocalNetwork.kt @@ -0,0 +1,145 @@ +package org.prolibertate.games.net + +import android.content.Context +import android.net.ConnectivityManager +import android.net.Network +import android.net.NetworkCapabilities +import android.net.wifi.WifiManager +import java.net.Inet4Address +import java.net.NetworkInterface +import java.net.Socket + +/** + * Finding the network the other phone is actually on. + * + * A phone's hotspot has no internet behind it, and Android takes that + * literally: the Wi-Fi network fails its captive-portal check, stays + * unvalidated, and **mobile data remains the default network**. Every socket + * this app opens without saying otherwise is therefore routed out over + * cellular, where 192.168.x.x means nothing at all. That is the whole of the + * "two phones on one phone's hotspot cannot see each other" bug: nothing is + * wrong with the Wi-Fi, the packets are simply being posted to the wrong + * network. + * + * The cure is to name the network explicitly rather than accept the default — + * which is why everything here deliberately looks for a Wi-Fi network *without* + * asking whether it has internet. Asking is what breaks it. + */ + +/** The Wi-Fi network this device is on, internet or no internet. */ +fun wifiNetwork(context: Context): Network? { + val manager = context.applicationContext + .getSystemService(Context.CONNECTIVITY_SERVICE) as? ConnectivityManager + ?: return null + @Suppress("DEPRECATION") + return manager.allNetworks.firstOrNull { network -> + val capabilities = manager.getNetworkCapabilities(network) + capabilities != null && + capabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) + } +} + +/** + * Sends this socket out over the Wi-Fi rather than wherever the system would + * otherwise send it. Silent if there is no Wi-Fi network to bind to: a phone + * running the hotspot itself has no Wi-Fi *client* network, and reaches its own + * guests over the tethering interface's own route without any help. + */ +fun bindToWifi(context: Context, socket: Socket) { + val network = wifiNetwork(context) ?: return + runCatching { network.bindSocket(socket) } +} + +/** + * The addresses another device on the same Wi-Fi could reach this one at. + * + * Read off the interfaces rather than asked of [WifiManager], because the phone + * running the hotspot is not a Wi-Fi client and has no client address to give — + * its guests reach it at whatever the tethering interface is numbered, which + * only the interface list knows. + */ +fun localIpv4Addresses(): List { + val interfaces = runCatching { NetworkInterface.getNetworkInterfaces() } + .getOrNull() ?: return emptyList() + + val found = mutableListOf>() + for (nic in interfaces) { + if (runCatching { !nic.isUp || nic.isLoopback }.getOrDefault(true)) continue + for (address in nic.inetAddresses) { + if (address !is Inet4Address || address.isLoopbackAddress) continue + val text = address.hostAddress ?: continue + // Tethering interfaces first: on the phone running the hotspot that + // is the only address its guests can reach, and it is the case the + // list exists to serve. + val rank = when { + nic.name.startsWith("ap") || nic.name.startsWith("swlan") -> 0 + nic.name.startsWith("wlan") -> 1 + else -> 2 + } + found += rank to text + } + } + return found.sortedBy { it.first }.map { it.second }.distinct() +} + +/** + * Holds the Wi-Fi chip's multicast filter open. + * + * mDNS is multicast, and without this lock the radio drops multicast frames + * that are not addressed to this device before the system ever sees them. The + * manifest has asked for the permission since the beginning; nothing ever took + * the lock. + */ +class MulticastGuard(context: Context) { + + private val wifi = context.applicationContext + .getSystemService(Context.WIFI_SERVICE) as? WifiManager + + private var lock: WifiManager.MulticastLock? = null + + fun acquire() { + if (lock != null) return + lock = runCatching { + wifi?.createMulticastLock("plgames-discovery")?.apply { + setReferenceCounted(false) + acquire() + } + }.getOrNull() + } + + fun release() { + runCatching { lock?.takeIf { it.isHeld }?.release() } + lock = null + } +} + +/** An address and port typed in by hand, once it has been made sense of. */ +data class ManualAddress(val address: String, val port: Int) + +/** + * Reads what somebody typed into the "connect to" box. + * + * Accepts a bare address, since [DEFAULT_HOST_PORT] means the port is usually + * the same one and asking a person to copy five more digits off another screen + * is a good way to have them type four of them. A port may still be given + * after a colon for the case where the usual one was taken. + */ +fun parseManualAddress(typed: String, defaultPort: Int = DEFAULT_HOST_PORT): ManualAddress? { + val text = typed.trim() + if (text.isEmpty()) return null + + val colon = text.lastIndexOf(':') + if (colon < 0) return ManualAddress(text, defaultPort).takeIf { looksLikeHost(it.address) } + + val address = text.substring(0, colon).trim() + val port = text.substring(colon + 1).trim().toIntOrNull() ?: return null + if (port !in 1..65_535) return null + return ManualAddress(address, port).takeIf { looksLikeHost(it.address) } +} + +/** + * Enough of a check to catch a slip, and no more. Anything that survives this + * is handed to the socket, which is the real judge of whether it exists. + */ +private fun looksLikeHost(address: String): Boolean = + address.isNotEmpty() && address.none { it.isWhitespace() } diff --git a/app/src/main/java/org/prolibertate/games/net/Transport.kt b/app/src/main/java/org/prolibertate/games/net/Transport.kt index 8e9a6b0..cad96af 100644 --- a/app/src/main/java/org/prolibertate/games/net/Transport.kt +++ b/app/src/main/java/org/prolibertate/games/net/Transport.kt @@ -33,6 +33,28 @@ data class DiscoveredHost( val port: Int = 0, ) +/** + * Where a host is listening, for reading out loud. + * + * Discovery is not always possible — a phone sharing its own hotspot does not + * reliably advertise over the tethering interface, whatever the other end does + * — so a host has to be able to say where it is in a form somebody can type + * into the other phone. + */ +data class HostEndpoint(val addresses: List, val port: Int) { + /** The one to read out, tethering address first. */ + val primary: String? get() = addresses.firstOrNull()?.let { "$it:$port" } +} + +/** + * The port a host listens on when it can have it. + * + * Fixed rather than assigned by the system, so that joining by hand is typing + * an address and not an address and a five-digit number. A host that finds it + * taken falls back to whatever it is given and says so. + */ +const val DEFAULT_HOST_PORT = 47_654 + /** * One peer-to-peer link carrying [NetMessage]s. * diff --git a/app/src/main/java/org/prolibertate/games/ui/LobbyScreen.kt b/app/src/main/java/org/prolibertate/games/ui/LobbyScreen.kt index d31e32d..ee28ecc 100644 --- a/app/src/main/java/org/prolibertate/games/ui/LobbyScreen.kt +++ b/app/src/main/java/org/prolibertate/games/ui/LobbyScreen.kt @@ -6,24 +6,30 @@ import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding -import androidx.compose.foundation.lazy.LazyColumn -import androidx.compose.foundation.lazy.items +import androidx.compose.foundation.rememberScrollState +import androidx.compose.foundation.text.KeyboardOptions +import androidx.compose.foundation.verticalScroll +import androidx.compose.material3.Button import androidx.compose.material3.Card import androidx.compose.material3.Divider import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.OutlinedTextField 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.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.text.font.FontWeight +import androidx.compose.ui.text.input.KeyboardType import androidx.compose.ui.unit.dp import org.prolibertate.games.game.engine.PlayerKind import org.prolibertate.games.game.engine.TableConfig -import org.prolibertate.games.net.DiscoveredHost import org.prolibertate.games.net.LobbyController import org.prolibertate.games.net.seatForDevice @@ -75,7 +81,13 @@ fun LobbyScreen( val title = if (route.hosting) "Hosting" else "Join a game" ScreenScaffold(title = title, onBack = onBack) { modifier -> - Column(modifier = modifier, verticalArrangement = Arrangement.spacedBy(12.dp)) { + // Scrolls rather than filling: a lazy list here took the whole screen + // and pushed the Start button — and now the address to type in — off + // the bottom of it. There are never more than a handful of rows. + Column( + modifier = modifier.verticalScroll(rememberScrollState()), + verticalArrangement = Arrangement.spacedBy(12.dp), + ) { val available = env.lobby.availableTransports() Text( text = if (available.isEmpty()) { @@ -94,7 +106,7 @@ fun LobbyScreen( if (route.hosting) { HostingBody(env.lobby, state) } else { - JoiningBody(env, state.discovered, playerName) + JoiningBody(env, state, playerName) } } } @@ -102,34 +114,47 @@ fun LobbyScreen( @Composable private fun HostingBody(lobby: LobbyController, state: LobbyController.State) { + // Where to find this phone, for when the other one cannot find it by + // itself. Over a phone's own hotspot that is the normal case rather than + // the exception, so this is stated plainly rather than tucked away. + state.endpoint?.primary?.let { where -> + Card(modifier = Modifier.fillMaxWidth()) { + Column(modifier = Modifier.padding(12.dp)) { + Text("This phone is at", style = MaterialTheme.typography.labelSmall) + Text(where, fontWeight = FontWeight.Bold) + Text( + text = "If the other phone doesn't find the game on its own — which " + + "happens when you're sharing this phone's hotspot — type that " + + "into its Join screen.", + style = MaterialTheme.typography.bodySmall, + ) + } + } + } + Text("Seats", fontWeight = FontWeight.Bold) - LazyColumn( - verticalArrangement = Arrangement.spacedBy(6.dp), - modifier = Modifier.fillMaxWidth(), - ) { - items(state.seats) { slot -> - Card(modifier = Modifier.fillMaxWidth()) { - Row( - modifier = Modifier.fillMaxWidth().padding(12.dp), - verticalAlignment = Alignment.CenterVertically, - horizontalArrangement = Arrangement.SpaceBetween, - ) { - Column { - Text("Seat ${slot.seat} — ${slot.name}") - Text( - text = when (slot.kind) { - PlayerKind.HUMAN_LOCAL -> "You" - PlayerKind.HUMAN_REMOTE -> "Joined" - PlayerKind.AI -> "Computer" - } + " · Team ${slot.team}", - style = MaterialTheme.typography.bodySmall, - ) - } - if (slot.kind == PlayerKind.HUMAN_REMOTE) { - TextButton(onClick = { - lobby.setSeatKind(slot.seat, PlayerKind.AI, "Computer ${slot.seat}") - }) { Text("Drop") } - } + state.seats.forEach { slot -> + Card(modifier = Modifier.fillMaxWidth()) { + Row( + modifier = Modifier.fillMaxWidth().padding(12.dp), + verticalAlignment = Alignment.CenterVertically, + horizontalArrangement = Arrangement.SpaceBetween, + ) { + Column { + Text("Seat ${slot.seat} — ${slot.name}") + Text( + text = when (slot.kind) { + PlayerKind.HUMAN_LOCAL -> "You" + PlayerKind.HUMAN_REMOTE -> "Joined" + PlayerKind.AI -> "Computer" + } + " · Team ${slot.team}", + style = MaterialTheme.typography.bodySmall, + ) + } + if (slot.kind == PlayerKind.HUMAN_REMOTE) { + TextButton(onClick = { + lobby.setSeatKind(slot.seat, PlayerKind.AI, "Computer ${slot.seat}") + }) { Text("Drop") } } } } @@ -150,20 +175,21 @@ private fun HostingBody(lobby: LobbyController, state: LobbyController.State) { @Composable private fun JoiningBody( env: AppEnv, - discovered: List, + state: LobbyController.State, playerName: String, ) { + val discovered = state.discovered + if (discovered.isEmpty()) { Text("No games found yet. Make sure the host has started theirs.") Text( - text = "Both devices need to be on the same Wi-Fi network.", + text = "Both phones need to be on the same Wi-Fi — including one phone's " + + "hotspot, though over a hotspot the game often has to be joined by " + + "address rather than found.", style = MaterialTheme.typography.bodySmall, ) - return - } - - LazyColumn(verticalArrangement = Arrangement.spacedBy(6.dp)) { - items(discovered) { host -> + } else { + discovered.forEach { host -> Card( modifier = Modifier .fillMaxWidth() @@ -179,4 +205,44 @@ private fun JoiningBody( } } } + + Divider() + ManualJoin(env, playerName) +} + +/** + * Joining by address. + * + * Always offered rather than kept for when discovery has failed, because over a + * phone's hotspot discovery does not fail loudly — it just never finds + * anything, and a screen that only says "no games found" gives no way forward. + */ +@Composable +private fun ManualJoin(env: AppEnv, playerName: String) { + var typed by remember { mutableStateOf("") } + + Text("Join by address", fontWeight = FontWeight.Bold) + Text( + text = "The host's screen shows where it is. The port can be left off unless " + + "the host is showing a different one.", + style = MaterialTheme.typography.bodySmall, + ) + Row( + modifier = Modifier.fillMaxWidth(), + horizontalArrangement = Arrangement.spacedBy(8.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + OutlinedTextField( + value = typed, + onValueChange = { typed = it }, + singleLine = true, + label = { Text("192.168.43.1") }, + keyboardOptions = KeyboardOptions(keyboardType = KeyboardType.Uri), + modifier = Modifier.weight(1f), + ) + Button( + onClick = { env.lobby.joinAt(typed, playerName, env.peerId) }, + enabled = typed.isNotBlank(), + ) { Text("Connect") } + } } diff --git a/app/src/test/java/org/prolibertate/games/net/ManualAddressTest.kt b/app/src/test/java/org/prolibertate/games/net/ManualAddressTest.kt new file mode 100644 index 0000000..f8369da --- /dev/null +++ b/app/src/test/java/org/prolibertate/games/net/ManualAddressTest.kt @@ -0,0 +1,106 @@ +package org.prolibertate.games.net + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +/** + * Reading an address somebody has typed off another phone's screen. + * + * This is the way into a game when discovery cannot work, which over a phone's + * own hotspot is the normal case — so it has to accept what a person will + * actually type, including the trailing space the keyboard adds and the port + * they will usually leave off. + */ +class ManualAddressTest { + + @Test + fun `a bare address gets the usual port`() { + assertEquals( + ManualAddress("192.168.43.1", DEFAULT_HOST_PORT), + parseManualAddress("192.168.43.1"), + ) + } + + @Test + fun `a port after a colon is used instead`() { + assertEquals( + ManualAddress("192.168.43.1", 50000), + parseManualAddress("192.168.43.1:50000"), + ) + } + + /** Typing on a phone puts spaces in places nobody intended. */ + @Test + fun `surrounding space is ignored`() { + assertEquals( + ManualAddress("192.168.1.20", DEFAULT_HOST_PORT), + parseManualAddress(" 192.168.1.20 "), + ) + assertEquals( + ManualAddress("192.168.1.20", 47654), + parseManualAddress(" 192.168.1.20 : 47654 "), + ) + } + + @Test + fun `a hostname works as well as an address`() { + assertEquals( + ManualAddress("pixel-9.local", DEFAULT_HOST_PORT), + parseManualAddress("pixel-9.local"), + ) + } + + @Test + fun `nothing typed is nothing to connect to`() { + assertNull(parseManualAddress("")) + assertNull(parseManualAddress(" ")) + } + + @Test + fun `a port that is not a number is refused rather than guessed`() { + assertNull(parseManualAddress("192.168.43.1:door")) + assertNull(parseManualAddress("192.168.43.1:")) + } + + @Test + fun `a port outside the range is refused`() { + assertNull(parseManualAddress("192.168.43.1:0")) + assertNull(parseManualAddress("192.168.43.1:70000")) + assertNull(parseManualAddress("192.168.43.1:-1")) + } + + @Test + fun `an address with a hole in it is refused`() { + assertNull(parseManualAddress(":47654")) + assertNull(parseManualAddress("192.168.43.1 20:47654")) + } + + /** + * The default is only a default. A host that found the usual port taken + * shows a different one, and that is what gets typed. + */ + @Test + fun `the default port is only used when none is given`() { + assertEquals(DEFAULT_HOST_PORT, parseManualAddress("10.0.0.5")?.port) + assertEquals(1234, parseManualAddress("10.0.0.5:1234")?.port) + } + + @Test + fun `an endpoint reads out as address and port`() { + val endpoint = HostEndpoint(listOf("192.168.43.1", "10.0.0.5"), DEFAULT_HOST_PORT) + assertEquals("192.168.43.1:$DEFAULT_HOST_PORT", endpoint.primary) + } + + @Test + fun `an endpoint with no addresses has nothing to read out`() { + assertNull(HostEndpoint(emptyList(), DEFAULT_HOST_PORT).primary) + } + + /** What the host shows must be what the joiner can type. */ + @Test + fun `what a host shows is what the other end accepts`() { + val shown = HostEndpoint(listOf("192.168.43.1"), 47654).primary!! + assertEquals(ManualAddress("192.168.43.1", 47654), parseManualAddress(shown)) + } +} From deb4acf96be4d2f00dc7f1322257364445dcfd4e Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 16 Aug 2026 00:25:54 +0000 Subject: [PATCH 5/9] Give the hotspot's owner a route to its guests too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Binding the join socket to the Wi-Fi network covers a phone that has joined a hotspot. It does nothing for the phone that is sharing one: that phone has no Wi-Fi *client* network, so there was nothing to bind to and the code gave up — leaving the socket on the default network, which is mobile data, and a connection to a guest going out over cellular to die. ConnectivityManager does not offer the tethering interface as a Network, so there is no network to name. Binding this end of the socket to one of our own addresses on the guest's subnet picks the route instead, which is the only lever available. The subnet test assumes a /24, which every Android hotspot hands out, and getting it wrong costs a bind that would not have helped anyway. This is the direction that was left broken: the owner of the hotspot trying to join a game hosted by one of its guests. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0132V9gE6DF9itz2z3Hn7BK9 --- .../prolibertate/games/net/LanTransport.kt | 4 +- .../prolibertate/games/net/LocalNetwork.kt | 44 ++++++++++++++++--- .../games/net/ManualAddressTest.kt | 30 +++++++++++++ 3 files changed, 69 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/org/prolibertate/games/net/LanTransport.kt b/app/src/main/java/org/prolibertate/games/net/LanTransport.kt index 941ba87..cf939ce 100644 --- a/app/src/main/java/org/prolibertate/games/net/LanTransport.kt +++ b/app/src/main/java/org/prolibertate/games/net/LanTransport.kt @@ -201,11 +201,11 @@ class LanTransport(private val context: Context) : Transport { override suspend fun join(host: DiscoveredHost, scope: CoroutineScope): Connection = withContext(Dispatchers.IO) { val socket = Socket() - // Before connecting, and this is the whole fix for hotspot play: a + // Before connecting, and this is the fix for hotspot play: a // hotspot has no internet behind it, so Android leaves mobile data // as the default network and an unbound socket goes out over // cellular looking for a 192.168 address. - bindToWifi(context, socket) + bindForTarget(context, socket, host.address) socket.connect(InetSocketAddress(host.address, host.port), CONNECT_TIMEOUT_MS) socket.tcpNoDelay = true StreamConnection( diff --git a/app/src/main/java/org/prolibertate/games/net/LocalNetwork.kt b/app/src/main/java/org/prolibertate/games/net/LocalNetwork.kt index cb8c9f7..29ce3c0 100644 --- a/app/src/main/java/org/prolibertate/games/net/LocalNetwork.kt +++ b/app/src/main/java/org/prolibertate/games/net/LocalNetwork.kt @@ -6,6 +6,8 @@ import android.net.Network import android.net.NetworkCapabilities import android.net.wifi.WifiManager import java.net.Inet4Address +import java.net.InetAddress +import java.net.InetSocketAddress import java.net.NetworkInterface import java.net.Socket @@ -40,14 +42,42 @@ fun wifiNetwork(context: Context): Network? { } /** - * Sends this socket out over the Wi-Fi rather than wherever the system would - * otherwise send it. Silent if there is no Wi-Fi network to bind to: a phone - * running the hotspot itself has no Wi-Fi *client* network, and reaches its own - * guests over the tethering interface's own route without any help. + * Points this socket at the network [target] is actually on, before it + * connects. + * + * Two different problems wearing the same hat, depending on which end of the + * hotspot this phone is. + * + * A phone that has *joined* a hotspot has a Wi-Fi network, and naming it is + * enough. A phone that is *sharing* one does not: its guests live on the + * tethering interface, and ConnectivityManager does not offer that as a + * [Network] anything can bind to — so the socket falls back to the default + * network, which is mobile data, and a connection to a guest goes out over + * cellular and dies. Binding this end to an address on the guest's own subnet + * picks the route instead, which is the only lever available. */ -fun bindToWifi(context: Context, socket: Socket) { - val network = wifiNetwork(context) ?: return - runCatching { network.bindSocket(socket) } +fun bindForTarget(context: Context, socket: Socket, target: String) { + val network = wifiNetwork(context) + if (network != null) { + runCatching { network.bindSocket(socket) } + return + } + val source = localIpv4Addresses().firstOrNull { sameSubnet(it, target) } ?: return + runCatching { socket.bind(InetSocketAddress(InetAddress.getByName(source), 0)) } +} + +/** + * Whether two IPv4 addresses are on the same /24. + * + * A guess, but a safe one here: every Android hotspot hands out a /24, and the + * only thing this decides is which of our own addresses to send from. Getting + * it wrong costs a bind that would not have helped anyway. + */ +fun sameSubnet(a: String, b: String): Boolean { + val left = a.split('.') + val right = b.split('.') + if (left.size != 4 || right.size != 4) return false + return left.subList(0, 3) == right.subList(0, 3) } /** diff --git a/app/src/test/java/org/prolibertate/games/net/ManualAddressTest.kt b/app/src/test/java/org/prolibertate/games/net/ManualAddressTest.kt index f8369da..755eab7 100644 --- a/app/src/test/java/org/prolibertate/games/net/ManualAddressTest.kt +++ b/app/src/test/java/org/prolibertate/games/net/ManualAddressTest.kt @@ -1,7 +1,9 @@ package org.prolibertate.games.net import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue import org.junit.Test /** @@ -103,4 +105,32 @@ class ManualAddressTest { val shown = HostEndpoint(listOf("192.168.43.1"), 47654).primary!! assertEquals(ManualAddress("192.168.43.1", 47654), parseManualAddress(shown)) } + + // -- Choosing which of our own addresses to send from -------------------- + + /** + * The phone sharing the hotspot has no Wi-Fi network to bind to, so it + * picks a source address on the guest's subnet instead. Getting this wrong + * sends the connection out over mobile data. + */ + @Test + fun `addresses on one hotspot subnet match`() { + assertTrue(sameSubnet("192.168.43.1", "192.168.43.117")) + assertTrue(sameSubnet("192.168.1.20", "192.168.1.20")) + } + + @Test + fun `addresses on different subnets do not`() { + assertFalse(sameSubnet("192.168.43.1", "192.168.1.5")) + assertFalse(sameSubnet("192.168.43.1", "10.0.0.5")) + } + + /** Anything that is not a dotted quad is not a match, rather than a crash. */ + @Test + fun `nonsense is not a match`() { + assertFalse(sameSubnet("", "192.168.43.1")) + assertFalse(sameSubnet("192.168.43", "192.168.43.1")) + assertFalse(sameSubnet("pixel-9.local", "192.168.43.1")) + assertFalse(sameSubnet("192.168.43.1.7", "192.168.43.1")) + } } From f0c8895747564ade2fcf5f45df402644463e8549 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 16 Aug 2026 00:28:05 +0000 Subject: [PATCH 6/9] Get back into a game a locked phone dropped out of MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A phone locked when its turn came round could not play once it was unlocked. The game was never wrong — the host still had the position and the seat was still the player's — but the only route to it was gone, and nothing rebuilt it. The board came back with the state it had before the turn arrived, and every tap on it went nowhere. A locked phone puts its Wi-Fi radio into power save, and a link carrying nothing between one turn and the next is exactly what that drops. Four things were missing, and all four are needed: The radio is held awake for as long as there is a game on, which stops most of these drops happening at all. FULL_HIGH_PERF rather than the LOW_LATENCY its deprecation notice points at: that one only applies with the screen on and the app in front, which is the case that worked. A link with nothing on it is pinged, and answered. TCP does not report a link that died this way — the read never returns and the next write goes into a socket that leads nowhere — so without traffic of its own the app could not tell a player who was thinking from one who was gone. Both ends have to speak it or a quiet link would be dropped as dead, hence the protocol bump; the lobby already turns a mismatch away with "update both devices". A write that fails now reports the link as broken instead of closing it as though this end had meant to. That silence was the reason a dead board answered nothing and said nothing about why. And a guest that loses its link dials the host again — which is still listening, on the same port, for the whole match — and is put back in the seat it never actually left, by the device id it is already known by. The table then resyncs, which is what fills the screen back in. Coming back to the front asks for the same thing, since a client draws nothing but what it was sent and anything pushed while the link was down was pushed at nothing. Sends are serialised while I am here: a message is a line, a write is three calls, and the heartbeat writes on its own schedule. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GarwvAPvWHELNspro4foFY --- app/src/main/AndroidManifest.xml | 5 + .../prolibertate/games/net/LanTransport.kt | 40 ++++ .../prolibertate/games/net/LobbyController.kt | 203 +++++++++++++++--- .../prolibertate/games/net/MatchController.kt | 123 +++++++++-- .../org/prolibertate/games/net/Protocol.kt | 33 ++- .../org/prolibertate/games/net/Transport.kt | 134 ++++++++++-- .../org/prolibertate/games/ui/PlayScreen.kt | 31 ++- .../games/net/MatchControllerNetworkTest.kt | 128 ++++++++++- .../net/StreamConnectionHeartbeatTest.kt | 181 ++++++++++++++++ 9 files changed, 806 insertions(+), 72 deletions(-) create mode 100644 app/src/test/java/org/prolibertate/games/net/StreamConnectionHeartbeatTest.kt diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 1e167ba..bdb6768 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -6,6 +6,11 @@ + + diff --git a/app/src/main/java/org/prolibertate/games/net/LanTransport.kt b/app/src/main/java/org/prolibertate/games/net/LanTransport.kt index 506b550..160ff4a 100644 --- a/app/src/main/java/org/prolibertate/games/net/LanTransport.kt +++ b/app/src/main/java/org/prolibertate/games/net/LanTransport.kt @@ -3,6 +3,7 @@ package org.prolibertate.games.net import android.content.Context import android.net.nsd.NsdManager import android.net.nsd.NsdServiceInfo +import android.net.wifi.WifiManager import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.channels.awaitClose @@ -31,17 +32,49 @@ class LanTransport(private val context: Context) : Transport { private val nsdManager: NsdManager get() = context.getSystemService(Context.NSD_SERVICE) as NsdManager + private val wifiManager: WifiManager + get() = context.applicationContext.getSystemService(Context.WIFI_SERVICE) as WifiManager + private var serverSocket: ServerSocket? = null private var registrationListener: NsdManager.RegistrationListener? = null private var discoveryListener: NsdManager.DiscoveryListener? = null + private var wifiLock: WifiManager.WifiLock? = null override fun isAvailable(): Boolean = true + /** + * Keeps the Wi-Fi radio properly awake for as long as there is a game on. + * + * Android puts the radio into power save when the screen goes off, which is + * what a locked phone is, and a link left to it drops out from under a game + * that is still being played. FULL_HIGH_PERF is the mode that says otherwise + * — the deprecation notice points at LOW_LATENCY, which is the wrong one + * here: that one only applies while the screen is on and the app is in + * front, which is precisely the case that was never broken. + */ + @Suppress("DEPRECATION") + private fun holdRadio() { + if (wifiLock?.isHeld == true) return + val lock = wifiLock ?: runCatching { + wifiManager.createWifiLock(WifiManager.WIFI_MODE_FULL_HIGH_PERF, WIFI_LOCK_TAG) + .also { it.setReferenceCounted(false) } + }.getOrNull() ?: return + wifiLock = lock + runCatching { lock.acquire() } + } + + private fun releaseRadio() { + wifiLock?.let { lock -> runCatching { if (lock.isHeld) lock.release() } } + wifiLock = null + } + // ----------------------------------------------------------------------- // Hosting // ----------------------------------------------------------------------- override fun host(displayName: String, scope: CoroutineScope): Flow = callbackFlow { + holdRadio() + // Port 0 lets the OS pick a free port, which is then advertised. val server = ServerSocket(0) serverSocket = server @@ -66,6 +99,7 @@ class LanTransport(private val context: Context) : Transport { while (isActive && !server.isClosed) { val socket = runCatching { server.accept() }.getOrNull() ?: break socket.tcpNoDelay = true + socket.keepAlive = true val connection = StreamConnection( peerId = socket.inetAddress?.hostAddress ?: "unknown", kind = TransportKind.LAN, @@ -90,6 +124,8 @@ class LanTransport(private val context: Context) : Transport { // ----------------------------------------------------------------------- override fun discover(scope: CoroutineScope): Flow> = callbackFlow { + holdRadio() + val found = Collections.synchronizedMap(linkedMapOf()) fun publish() { @@ -177,9 +213,11 @@ class LanTransport(private val context: Context) : Transport { override suspend fun join(host: DiscoveredHost, scope: CoroutineScope): Connection = withContext(Dispatchers.IO) { + holdRadio() val socket = Socket() socket.connect(InetSocketAddress(host.address, host.port), CONNECT_TIMEOUT_MS) socket.tcpNoDelay = true + socket.keepAlive = true StreamConnection( peerId = host.id, kind = TransportKind.LAN, @@ -207,9 +245,11 @@ class LanTransport(private val context: Context) : Transport { stopDiscovery() runCatching { serverSocket?.close() } serverSocket = null + releaseRadio() } private companion object { const val CONNECT_TIMEOUT_MS = 8_000 + const val WIFI_LOCK_TAG = "ProLibertateGames:lan" } } diff --git a/app/src/main/java/org/prolibertate/games/net/LobbyController.kt b/app/src/main/java/org/prolibertate/games/net/LobbyController.kt index 22cefd2..f4af96d 100644 --- a/app/src/main/java/org/prolibertate/games/net/LobbyController.kt +++ b/app/src/main/java/org/prolibertate/games/net/LobbyController.kt @@ -3,6 +3,7 @@ package org.prolibertate.games.net import android.content.Context import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Job +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow @@ -10,6 +11,7 @@ import kotlinx.coroutines.launch import org.prolibertate.games.game.engine.PlayerKind import org.prolibertate.games.game.engine.PlayerSlot import org.prolibertate.games.game.engine.TableConfig +import java.util.concurrent.atomic.AtomicBoolean /** * The pre-game lobby. @@ -48,8 +50,27 @@ class LobbyController( /** Peer id -> live connection, for every seated remote player. */ private val connections = linkedMapOf() private var clientLink: Connection? = null + private var clientJob: Job? = null private val jobs = mutableListOf() + /** + * Where this device joined, so it can find its way back. + * + * The host goes on listening on the same port for as long as it is hosting, + * which is the whole match, so a guest that has lost its link can dial the + * address it already knows rather than go round discovery again. + */ + private var joinedHost: DiscoveredHost? = null + private var joinedName: String = "" + private var joinedPeerId: String = "" + private val reconnecting = AtomicBoolean(false) + + /** + * The table currently being played, if any, so a replaced link can be given + * to it. Set by the match screen for as long as it is on screen. + */ + var rebinder: LinkRebinder? = null + val hostConnections: List get() = connections.values.toList() val clientConnection: Connection? get() = clientLink @@ -118,6 +139,34 @@ class LobbyController( private suspend fun seatPlayer(connection: Connection, hello: Hello) { val current = _state.value val seats = current.seats.toMutableList() + + // A device the table already knows is not somebody new asking for a + // seat: it is a player whose link went away — a phone locked long + // enough for the radio to drop it is the usual way — coming back to the + // one it already has. Seating it again would find no free seat and turn + // it away from its own game, so it is put back where it was. + val returning = seats.indexOfFirst { it.deviceId == hello.peerId } + if (returning >= 0) { + val stale = seats[returning].peerId?.let { connections.remove(it) } + seats[returning] = seats[returning].copy( + name = hello.displayName, + kind = PlayerKind.HUMAN_REMOTE, + peerId = connection.peerId, + ) + connections[connection.peerId] = connection + if (stale != null && stale !== connection) runCatching { stale.close() } + _state.value = current.copy( + seats = seats, + message = "${hello.displayName} is back.", + ) + connection.send(Welcome(current.hostName, current.gameId.orEmpty(), accepted = true)) + broadcastLobby() + // A match already under way keeps the seat and the position it is + // sitting in. Only the link is new. + rebinder?.rebindPeer(seats[returning].seat, connection) + return + } + val index = seats.indexOfFirst { it.kind == PlayerKind.AI } if (index < 0) { connection.send( @@ -208,45 +257,122 @@ class LobbyController( fun join(host: DiscoveredHost, displayName: String, peerId: String) { jobs += scope.launch { _state.value = _state.value.copy(message = "Connecting to ${host.name}…") - val transport = transports.firstOrNull { it.kind == host.kind } ?: return@launch - val connection = runCatching { transport.join(host, scope) }.getOrElse { error -> - _state.value = _state.value.copy(message = "Couldn't connect: ${error.message}") - return@launch - } - clientLink = connection + joinedHost = host + joinedName = displayName + joinedPeerId = peerId + val connection = dial(host) ?: return@launch + attachClientLink(connection) + } + } - launch { - connection.incoming.collect { message -> - when (message) { - is Welcome -> _state.value = if (message.accepted) { - _state.value.copy( - connected = true, - gameId = message.gameId, - hostName = message.hostName, - message = "Joined ${message.hostName}. Waiting for the host to start…", - ) - } else { - _state.value.copy(message = message.reason ?: "The host declined.") - } + private suspend fun dial(host: DiscoveredHost): Connection? { + val transport = transports.firstOrNull { it.kind == host.kind } ?: return null + return runCatching { transport.join(host, scope) }.getOrElse { error -> + _state.value = _state.value.copy(message = "Couldn't connect: ${error.message}") + null + } + } - is LobbyUpdate -> _state.value = _state.value.copy( - seats = message.seats, + /** + * Starts reading [connection] as this device's way to the host, and + * introduces itself on it. + * + * Any previous reader is stopped first: collecting a connection's messages + * never ends on its own, so a link that has been replaced would otherwise + * leave its collector behind writing into the same lobby state. + */ + private suspend fun attachClientLink(connection: Connection) { + clientJob?.cancel() + clientLink = connection + clientJob = scope.launch { + connection.incoming.collect { message -> + when (message) { + is Welcome -> _state.value = if (message.accepted) { + _state.value.copy( + connected = true, gameId = message.gameId, - optionsJson = message.optionsJson, hostName = message.hostName, + message = "Joined ${message.hostName}. Waiting for the host to start…", ) - - is StartGame -> _state.value = _state.value.copy(started = message.config) - - is Bye -> _state.value = - _state.value.copy(connected = false, message = "The host left.") - - else -> Unit + } else { + _state.value.copy(message = message.reason ?: "The host declined.") } + + is LobbyUpdate -> _state.value = _state.value.copy( + seats = message.seats, + gameId = message.gameId, + optionsJson = message.optionsJson, + hostName = message.hostName, + ) + + is StartGame -> _state.value = _state.value.copy(started = message.config) + + is Bye -> { + _state.value = + _state.value.copy(connected = false, message = "The host left.") + // A link that has gone is worth replacing while there is + // still a table to come back to. In its own job, because + // taking the new link into use stops this one. + if (rebinder != null) jobs += scope.launch { reconnect() } + } + + else -> Unit } } + } - connection.send(Hello(peerId = peerId, displayName = displayName)) + connection.send(Hello(peerId = joinedPeerId, displayName = joinedName)) + } + + /** + * Client: makes sure there is still a way to the host, and asks for the + * table again. + * + * Called when the app comes back to the front. A phone that was locked when + * its turn came round has missed whatever the host pushed in the meantime — + * a client draws nothing but what it was sent — and may have lost the link + * entirely while the radio was down, in which case there is no point asking + * politely and it dials again instead. + * + * Costs nothing when nothing is wrong: a live link is sent one [Resync], + * and the answer is the state it already has. + */ + fun restoreLink() { + if (joinedHost == null) return + jobs += scope.launch { + val live = clientLink + if (live != null && live.isOpen) live.send(Resync) else reconnect() + } + } + + /** + * Dials the host again and hands the new link to whatever is being played. + * + * One attempt at a time: the link can be found to be gone twice over — the + * reader announcing it and the screen coming back both arrive at the same + * conclusion — and two dials would leave the host holding two links for one + * seat, having closed the one actually in use. + */ + private suspend fun reconnect() { + val host = joinedHost ?: return + if (!reconnecting.compareAndSet(false, true)) return + try { + repeat(RECONNECT_ATTEMPTS) { attempt -> + if (attempt > 0) delay(RECONNECT_BACKOFF_MILLIS * attempt) + if (clientLink?.isOpen == true) return + val connection = dial(host) + if (connection != null) { + // Hello first and rebind second, in that order: the host has + // to have put this device back in its seat before it can + // answer the table's resync with anything. + attachClientLink(connection) + rebinder?.rebindHostLink(connection) + return + } + } + _state.value = _state.value.copy(message = "Couldn't get back to ${host.name}.") + } finally { + reconnecting.set(false) } } @@ -257,11 +383,28 @@ class LobbyController( fun stop() { jobs.forEach { it.cancel() } jobs.clear() + clientJob?.cancel() + clientJob = null connections.values.forEach { runCatching { it.close() } } connections.clear() runCatching { clientLink?.close() } clientLink = null + // Nothing left to get back to, and nothing left to hand a link to. + joinedHost = null + joinedName = "" + joinedPeerId = "" + rebinder = null transports.forEach { runCatching { it.stop() } } _state.value = State() } + + private companion object { + /** + * Enough to ride out a radio that has not finished waking up, and few + * enough that a host which is genuinely gone is reported as gone rather + * than dialled at forever. + */ + const val RECONNECT_ATTEMPTS = 4 + const val RECONNECT_BACKOFF_MILLIS = 750L + } } diff --git a/app/src/main/java/org/prolibertate/games/net/MatchController.kt b/app/src/main/java/org/prolibertate/games/net/MatchController.kt index 89dde9c..d175acd 100644 --- a/app/src/main/java/org/prolibertate/games/net/MatchController.kt +++ b/app/src/main/java/org/prolibertate/games/net/MatchController.kt @@ -2,6 +2,7 @@ package org.prolibertate.games.net import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job import kotlinx.coroutines.delay import kotlinx.coroutines.withContext import kotlinx.coroutines.flow.MutableStateFlow @@ -56,7 +57,7 @@ class MatchController( * waits for [confirmAdvance]. */ private val awaitsConfirmation: (S) -> Boolean = { false }, -) { +) : LinkRebinder { enum class Role { HOST, CLIENT } @@ -111,15 +112,32 @@ class MatchController( private var released = false private val peers = mutableMapOf() + + /** + * The reader attached to each peer's link. + * + * Held so a replaced link's collector can be stopped. Collecting a + * [Connection.incoming] never ends on its own — a SharedFlow has no + * completion — so a link swapped out mid-match would otherwise leave its + * collector behind for the rest of the game. + */ + private val peerJobs = mutableMapOf() private var hostLink: Connection? = null + private var hostLinkJob: Job? = null private val seatKinds: Map = config.seats.associate { it.seat to it.kind } - private val seatPeers: Map = - config.seats.associate { it.seat to it.peerId } + + /** + * Which link each seat is reached on. Mutable because a player who drops + * and comes back is the same player in the same seat, but not necessarily + * on the same address — a reconnecting phone can be handed a different one. + */ + private val seatPeers: MutableMap = + config.seats.associate { it.seat to it.peerId }.toMutableMap() private fun nameOfPeer(peerId: String): String = - config.seats.firstOrNull { it.peerId == peerId }?.name ?: "The other player" + config.seats.firstOrNull { it.peerId == peerId }?.name ?: "the other player" /** * Records that this table cannot be played any further. @@ -140,12 +158,7 @@ class MatchController( /** Host: begin the match and start pushing state to [clients]. */ fun startAsHost(clients: List) { check(role == Role.HOST) { "Not hosting" } - clients.forEach { connection -> - peers[connection.peerId] = connection - scope.launch { - connection.incoming.collect { message -> onHostMessage(connection, message) } - } - } + clients.forEach { connection -> listenToPeer(connection) } scope.launch { val initial = runCatching { rules.initialState(config) }.getOrElse { error -> // A table the rules will not accept is a dead end, not a reason @@ -162,11 +175,18 @@ class MatchController( } } + private fun listenToPeer(connection: Connection) { + peers[connection.peerId] = connection + peerJobs[connection.peerId] = scope.launch { + connection.incoming.collect { message -> onHostMessage(connection, message) } + } + } + /** Client: render whatever arrives from [link]. */ fun startAsClient(link: Connection) { check(role == Role.CLIENT) { "Not a client" } hostLink = link - scope.launch { + hostLinkJob = scope.launch { link.incoming // Asked for from inside onSubscription, which is what makes it // reliable: the request cannot leave before this collector is @@ -196,11 +216,14 @@ class MatchController( is Rejected -> _notice.value = message.reason is Bye -> { - // The host has gone: the game cannot go on, and - // saying so is what gives the screen something to - // offer a way out of. + // The link to the host has gone, either because the + // host left or because the link died under it. The + // two are indistinguishable from this end, so this + // says only what is actually known — and it stops + // the table, which gives the screen a way out to + // offer if getting back in does not work. _legalMoves.value = emptyList() - abandon("The host left the game.") + abandon("The connection to the host was lost.") } else -> Unit @@ -209,6 +232,59 @@ class MatchController( } } + // ----------------------------------------------------------------------- + // Getting back in + // ----------------------------------------------------------------------- + + /** + * Host: a player who dropped is back, on [connection]. + * + * The seat, and the position it is sitting in, were never given up — only + * the link was — so nothing here touches the game. The old link is let go + * of, the new one is read from, and the seat is sent the table as it now + * stands so the returning player has something to play on. + */ + override fun rebindPeer(seat: Int, connection: Connection) { + if (role != Role.HOST) return + val previous = seatPeers[seat] + if (previous != null) { + peerJobs.remove(previous)?.cancel() + // Not closed: the lobby owns these sockets and has already dealt + // with the one being replaced. Closing a link this end has stopped + // reading would only race the reader that is on its way out. + peers.remove(previous) + } + seatPeers[seat] = connection.peerId + listenToPeer(connection) + // Only once everybody is reachable again. At a table of four, one player + // coming back says nothing about the other one who has not. + if (seatPeers.values.filterNotNull().all { peers[it]?.isOpen == true }) { + clearAbandoned() + } + scope.launch { sendStateTo(connection) } + } + + /** + * Client: the way back to the host is [connection]. + * + * The collector asks for a resync as it attaches, which is what fills the + * screen back in: everything a client draws came off the wire, and whatever + * was pushed while the link was down was pushed into nothing. + */ + override fun rebindHostLink(connection: Connection) { + if (role != Role.CLIENT) return + if (hostLink === connection) return + hostLinkJob?.cancel() + clearAbandoned() + startAsClient(connection) + } + + /** Puts a table that was stopped back into play. */ + private fun clearAbandoned() { + _abandoned.value = null + _notice.value = null + } + // ----------------------------------------------------------------------- // Input // ----------------------------------------------------------------------- @@ -232,10 +308,15 @@ class MatchController( private suspend fun onHostMessage(connection: Connection, message: NetMessage) { if (message is Bye) { - // One of the players has left. The host keeps the position — there is - // nothing wrong with it — but a table that cannot go on should say so - // rather than leave somebody waiting on a move that is not coming. - abandon("${nameOfPeer(connection.peerId)} left the game.") + // Anything arriving on a link that has since been replaced belongs + // to the link, not to the seat: a player who dropped and came back + // must not be reported as having left on the way in. + if (peers[connection.peerId] !== connection) return + // One of the players is no longer reachable. The host keeps the + // position — there is nothing wrong with it — but a table that + // cannot go on should say so rather than leave somebody waiting on a + // move that is not coming. + abandon("The connection to ${nameOfPeer(connection.peerId)} was lost.") return } if (message is Resync) { @@ -433,8 +514,12 @@ class MatchController( * [StreamConnection.close], which this leans on to not block. */ fun close() { + peerJobs.values.forEach { it.cancel() } + peerJobs.clear() peers.values.forEach { runCatching { it.close() } } peers.clear() + hostLinkJob?.cancel() + hostLinkJob = null runCatching { hostLink?.close() } hostLink = null } diff --git a/app/src/main/java/org/prolibertate/games/net/Protocol.kt b/app/src/main/java/org/prolibertate/games/net/Protocol.kt index 789bb7f..109b314 100644 --- a/app/src/main/java/org/prolibertate/games/net/Protocol.kt +++ b/app/src/main/java/org/prolibertate/games/net/Protocol.kt @@ -16,7 +16,14 @@ import org.prolibertate.games.game.engine.TableConfig * takes effect. Each client is sent a state redacted for its own seat, so an * opponent's hand is never transmitted in the first place. */ -const val PROTOCOL_VERSION = 1 +/** + * 2 added the heartbeat. A peer that does not answer [Ping] cannot be told + * apart from one that has gone away, so both ends have to speak it or a quiet + * link would be dropped as dead the first time nobody moved for a while. The + * lobby already refuses a mismatch with "update both devices", which is the + * honest thing to say about it. + */ +const val PROTOCOL_VERSION = 2 /** Service type and name advertised over mDNS. */ const val SERVICE_TYPE = "_plgames._tcp" @@ -116,6 +123,30 @@ data object Resync : NetMessage @SerialName("rejected") data class Rejected(val reason: String) : NetMessage +/** + * Sent to a link nobody has spoken on for a while, and answered with [Pong]. + * + * Two things need it. A phone whose screen has gone off puts its Wi-Fi radio + * into power save and, given long enough, stops the app's networking + * altogether; an access point ages an idle association out of its tables. A + * link carrying nothing between one player's turn and the next is idle for + * minutes at a time, which is exactly what both of those collect. And a TCP + * link that dies that way dies quietly: the read never returns and the next + * write is buffered into a socket that no longer goes anywhere, so without + * traffic of its own the app cannot tell a link that is waiting from one that + * is gone. + * + * Neither reaches a screen — see [StreamConnection], which answers and swallows + * them, so nothing above the transport has to know they exist. + */ +@Serializable +@SerialName("ping") +data object Ping : NetMessage + +@Serializable +@SerialName("pong") +data object Pong : NetMessage + @Serializable @SerialName("bye") data object Bye : NetMessage diff --git a/app/src/main/java/org/prolibertate/games/net/Transport.kt b/app/src/main/java/org/prolibertate/games/net/Transport.kt index 8e9a6b0..8adc43e 100644 --- a/app/src/main/java/org/prolibertate/games/net/Transport.kt +++ b/app/src/main/java/org/prolibertate/games/net/Transport.kt @@ -3,11 +3,14 @@ package org.prolibertate.games.net import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.NonCancellable +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.SharedFlow import kotlinx.coroutines.isActive import kotlinx.coroutines.launch +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext import kotlinx.serialization.encodeToString import java.io.BufferedReader @@ -60,12 +63,35 @@ class StreamConnection( private val output: OutputStream, private val scope: CoroutineScope, private val onClosed: (StreamConnection) -> Unit = {}, + /** How often a link with nothing to say is pinged. */ + private val heartbeatMillis: Long = HEARTBEAT_MILLIS, + /** How long a link may go completely silent before it counts as gone. */ + private val silenceMillis: Long = SILENCE_MILLIS, ) : Connection { private val reader: BufferedReader = input.bufferedReader() private val writer: BufferedWriter = output.bufferedWriter() + private val writeLock = Mutex() private val closed = AtomicBoolean(false) + /** + * Whether this end closed the link on purpose, as leaving a game does. + * + * Kept apart from [closed] because the two answer different questions. A + * link is closed either way; only one of the two ways is worth telling + * anybody about, and the heartbeat closes links that very much are. + */ + private val closedOnPurpose = AtomicBoolean(false) + + /** + * When the peer was last heard from, on the monotonic clock. + * + * Wall time would do the wrong thing across a clock adjustment, and + * SystemClock is Android's, which the unit tests do not have. + */ + @Volatile + private var lastHeardNanos: Long = System.nanoTime() + private val _incoming = MutableSharedFlow(replay = 0, extraBufferCapacity = 64) override val incoming: SharedFlow = _incoming @@ -73,8 +99,6 @@ class StreamConnection( init { scope.launch(Dispatchers.IO) { - // Told apart from a link this side closed on purpose, because only - // one of the two is worth telling anybody about. var droppedByPeer = false try { while (isActive && !closed.get()) { @@ -83,11 +107,22 @@ class StreamConnection( droppedByPeer = true break } + // Anything at all counts as a sign of life, including a line + // this version cannot make sense of: the point is that the + // peer is still there and still talking. + lastHeardNanos = System.nanoTime() if (line.isBlank()) continue val message = runCatching { protocolJson.decodeFromString(line) }.getOrNull() ?: continue // ignore anything we can't parse - _incoming.emit(message) + // The heartbeat is the transport's own business and stops + // here. Answering on this thread is safe: send() hops to IO, + // which is where this already is. + when (message) { + is Ping -> send(Pong) + is Pong -> Unit + else -> _incoming.emit(message) + } } } catch (_: Exception) { // A dropped link is normal: someone walked out of range or @@ -98,30 +133,62 @@ class StreamConnection( // lock, so it is the only one that can close it without // waiting. See close(). runCatching { reader.close() } - val weClosedIt = closed.get() - close() + val deliberate = closedOnPurpose.get() + shutdown() // A link that died on its own reaches the screens on the same - // channel as everything else, so leaving a game is something the + // channel as everything else, so losing a player is something the // other end finds out about the same way it finds out about a // move. Withdrawing deliberately needs no announcement, and the // emit is not cancellable — the whole point of it is to arrive // while everything around it is being torn down. - if (droppedByPeer && !weClosedIt) { + if (droppedByPeer && !deliberate) { withContext(NonCancellable) { runCatching { _incoming.emit(Bye) } } } } } + + // Keeps the link warm and, more to the point, keeps it answerable: a + // link nobody has spoken on cannot be told from a link that no longer + // exists, and between two turns of a card game that is most of the game. + scope.launch(Dispatchers.IO) { + while (isActive && !closed.get()) { + delay(heartbeatMillis) + if (closed.get()) break + val silentFor = (System.nanoTime() - lastHeardNanos) / 1_000_000 + if (silentFor > silenceMillis) { + // Closing the stream is what unblocks the read; the reader + // then announces the drop from its own thread, exactly as it + // would for a peer that had hung up. + shutdown() + break + } + send(Ping) + } + } } override suspend fun send(message: NetMessage) { if (closed.get()) return withContext(Dispatchers.IO) { - try { - writer.write(protocolJson.encodeToString(message)) - writer.newLine() - writer.flush() - } catch (_: Exception) { - close() + // One message at a time. The framing is what makes a line a message, + // and a write is three calls — the text, the newline, the flush — so + // two of them running at once would splice one message through the + // middle of another and neither would arrive. The heartbeat is what + // makes that likely: it writes on its own schedule, without caring + // what the game happens to be sending. + writeLock.withLock { + try { + writer.write(protocolJson.encodeToString(message)) + writer.newLine() + writer.flush() + } catch (_: Exception) { + // Not close(): a write that failed is a link that broke, and + // the screen has to hear about that. Going through close() + // here used to mark it deliberate, so a player whose link + // had died tapped a board that answered nothing and was + // never told why. + shutdown() + } } } } @@ -143,11 +210,52 @@ class StreamConnection( * buffered in it. */ override fun close() { + closedOnPurpose.set(true) + shutdown() + } + + /** Lets go of the streams without saying anything about why. */ + private fun shutdown() { if (!closed.compareAndSet(false, true)) return runCatching { input.close() } runCatching { output.close() } onClosed(this) } + + private companion object { + /** + * Often enough to hold an access point's idea of the association open + * and to notice a dead link inside a turn, seldom enough that it is a + * few dozen bytes a minute. + */ + const val HEARTBEAT_MILLIS = 5_000L + + /** + * Several missed beats rather than one. A phone with its screen off + * answers late — the radio is in power save and the timer that would + * have sent the ping is deferred — and a link is not dead just because + * it is slow. + */ + const val SILENCE_MILLIS = 25_000L + } +} + +/** + * A table that can be handed a replacement for a link it was given. + * + * A dropped link is not the end of a game — the position is untouched and the + * player is still sitting there — so the lobby, which owns the sockets and goes + * on listening for the whole match, reconnects and passes the new link through + * this. Narrow on purpose: the lobby has no business knowing what game is being + * played on the other side of it. + */ +interface LinkRebinder { + + /** Host: [connection] is the seat's link from now on. */ + fun rebindPeer(seat: Int, connection: Connection) + + /** Client: [connection] is the way to the host from now on. */ + fun rebindHostLink(connection: Connection) } /** diff --git a/app/src/main/java/org/prolibertate/games/ui/PlayScreen.kt b/app/src/main/java/org/prolibertate/games/ui/PlayScreen.kt index 3359b69..4235d36 100644 --- a/app/src/main/java/org/prolibertate/games/ui/PlayScreen.kt +++ b/app/src/main/java/org/prolibertate/games/ui/PlayScreen.kt @@ -9,6 +9,9 @@ import androidx.compose.runtime.mutableIntStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.runtime.rememberCoroutineScope +import androidx.compose.ui.platform.LocalLifecycleOwner +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleEventObserver import org.prolibertate.games.game.GameCatalog import org.prolibertate.games.game.chess.ChessAi import org.prolibertate.games.game.chess.ChessMove @@ -683,6 +686,32 @@ private fun StartMatch( } } DisposableEffect(controller) { - onDispose { controller.close() } + // The lobby owns the sockets and goes on listening for the whole match, + // so it is the lobby that reconnects a dropped player — and this is + // where it finds the table to hand the new link to. + env.lobby.rebinder = controller + onDispose { + env.lobby.rebinder = null + controller.close() + } + } + + // Coming back to a table that has been out of sight. + // + // A locked phone is a stopped activity, and its Wi-Fi radio goes to sleep + // with the screen. Anything the host pushed in the meantime — the player's + // own turn arriving is the one that matters — was pushed at a link that may + // no longer have been there, and a client draws nothing but what it was + // sent. So the table is asked for again on the way back in, over a fresh + // link if the old one did not survive. + val lifecycleOwner = LocalLifecycleOwner.current + DisposableEffect(lifecycleOwner, route.hosting) { + val observer = LifecycleEventObserver { _, event -> + if (event == Lifecycle.Event.ON_START && !route.hosting) { + env.lobby.restoreLink() + } + } + lifecycleOwner.lifecycle.addObserver(observer) + onDispose { lifecycleOwner.lifecycle.removeObserver(observer) } } } diff --git a/app/src/test/java/org/prolibertate/games/net/MatchControllerNetworkTest.kt b/app/src/test/java/org/prolibertate/games/net/MatchControllerNetworkTest.kt index 8b37c9b..953aebb 100644 --- a/app/src/test/java/org/prolibertate/games/net/MatchControllerNetworkTest.kt +++ b/app/src/test/java/org/prolibertate/games/net/MatchControllerNetworkTest.kt @@ -228,17 +228,78 @@ class MatchControllerNetworkTest { guestLeft.start() guestLeft.leave(Side.GUEST) guestLeft.settle() - assertEquals("Guest left the game.", guestLeft.host.abandoned.value) + assertEquals("The connection to Guest was lost.", guestLeft.host.abandoned.value) val hostLeft = chess() hostLeft.start() hostLeft.leave(Side.HOST) hostLeft.settle() - assertEquals("The host left the game.", hostLeft.client.abandoned.value) + assertEquals("The connection to the host was lost.", hostLeft.client.abandoned.value) // Nothing more can be played on a table with no host. assertEquals(emptyList(), hostLeft.client.legalMoves.value) } + // ----------------------------------------------------------------------- + // Coming back + // ----------------------------------------------------------------------- + + /** + * The bug this is here for: a phone locked when its turn came round could + * not play once it was unlocked again. + * + * A locked phone puts its Wi-Fi radio to sleep, and a link carrying nothing + * between one turn and the next is exactly what that drops. The game was + * never wrong — the host still had the position and the seat was still the + * player's — but the only route to it was gone, and nothing rebuilt it. The + * board came back on unlocking with the state it had before the turn + * arrived, and every tap on it went nowhere. + */ + @Test + fun `a guest whose link died can play again once it is back`() = runBlocking { + val table = chess() + table.start() + table.white("e2", "e4") + // Twenty replies to 1. e4 — the guest is on the clock. + assertEquals(20, table.client.legalMoves.value.size) + + table.dropGuestLink() + assertNotNull("A dead link has to stop the table", table.client.abandoned.value) + assertNotNull(table.host.abandoned.value) + + table.reconnectGuest() + + assertNull("The table is playable again and must not say otherwise", table.client.abandoned.value) + assertNull(table.host.abandoned.value) + assertNotNull("The guest came back to an empty board", table.client.state.value) + assertEquals( + "The guest is back but has nothing it is allowed to play", + 20, + table.client.legalMoves.value.size, + ) + + // And a move made on the new link is a move that actually happens. + table.black("e7", "e5") + assertNotNull(table.host.state.value?.board?.get(squareFromName("e5"))) + assertNull(table.host.state.value?.board?.get(squareFromName("e7"))) + } + + /** + * A guest coming back must not be treated as a guest arriving. The host end + * of the old link is the one that died; anything it has left to say about + * itself belongs to it and not to the seat, which is occupied again. + */ + @Test + fun `a guest coming back is not reported as having left`() = runBlocking { + val table = chess() + table.start() + table.dropGuestLink() + table.reconnectGuest() + table.settle() + + assertNull(table.host.abandoned.value) + assertNull(table.client.abandoned.value) + } + @Test fun `closing a link on purpose is not reported as the other player leaving`() = runBlocking { val table = chess() @@ -273,6 +334,11 @@ class MatchControllerNetworkTest { private val hostScope = CoroutineScope(Job() + Dispatchers.Default) private val clientScope = CoroutineScope(Job() + Dispatchers.Default) + /** Every socket a reconnection has opened since, so dispose can close them. */ + private val laterSockets = mutableListOf() + private val guestPeerId: String + private val guestSeat: Int + val hostLink: StreamConnection private val clientLink: StreamConnection val host: MatchController @@ -281,7 +347,8 @@ class MatchControllerNetworkTest { init { clientSocket.connect(InetSocketAddress("127.0.0.1", server.localPort), CONNECT_MILLIS) hostSocket = server.accept() - val guestPeerId = hostSocket.inetAddress?.hostAddress ?: "guest" + guestPeerId = hostSocket.inetAddress?.hostAddress ?: "guest" + guestSeat = (0 until seatCount).first { it != hostSeat } hostLink = StreamConnection( peerId = guestPeerId, @@ -332,11 +399,7 @@ class MatchControllerNetworkTest { ) host = controller(MatchController.Role.HOST, hostSeat, hostScope) - client = controller( - MatchController.Role.CLIENT, - (0 until seatCount).first { it != hostSeat }, - clientScope, - ) + client = controller(MatchController.Role.CLIENT, guestSeat, clientScope) // The lobby is already reading the guest's link when the game starts, // and goes on reading it for the whole match. Without this the test @@ -372,6 +435,54 @@ class MatchControllerNetworkTest { settle() } + /** + * The guest's link dies under it, as a sleeping radio drops one. + * + * The sockets are killed rather than the connections closed: a link + * either end let go of on purpose is a player leaving, which is a + * different thing entirely and is already tested above. + */ + suspend fun dropGuestLink() { + runCatching { hostSocket.close() } + runCatching { clientSocket.close() } + settle() + } + + /** + * The guest dials the host again, which is still listening on the same + * port, and both ends are handed the new link — the host from its lobby + * on the guest's Hello, the guest from its own once that has gone + * through. + */ + suspend fun reconnectGuest() { + val socket = Socket() + socket.connect(InetSocketAddress("127.0.0.1", server.localPort), CONNECT_MILLIS) + val accepted = server.accept() + laterSockets += socket + laterSockets += accepted + + val newHostLink = StreamConnection( + peerId = guestPeerId, + kind = TransportKind.LAN, + input = accepted.getInputStream(), + output = accepted.getOutputStream(), + scope = hostScope, + onClosed = { runCatching { accepted.close() } }, + ) + val newClientLink = StreamConnection( + peerId = "lan:host", + kind = TransportKind.LAN, + input = socket.getInputStream(), + output = socket.getOutputStream(), + scope = clientScope, + onClosed = { runCatching { socket.close() } }, + ) + + host.rebindPeer(guestSeat, newHostLink) + client.rebindHostLink(newClientLink) + settle() + } + /** Exactly what AppRoot.pop() and PlayScreen's onDispose do, in that order. */ fun leave(side: Side) { if (side == Side.HOST) { @@ -392,6 +503,7 @@ class MatchControllerNetworkTest { fun dispose() { runCatching { hostSocket.close() } runCatching { clientSocket.close() } + laterSockets.forEach { runCatching { it.close() } } runCatching { server.close() } runCatching { hostLink.close() } runCatching { clientLink.close() } diff --git a/app/src/test/java/org/prolibertate/games/net/StreamConnectionHeartbeatTest.kt b/app/src/test/java/org/prolibertate/games/net/StreamConnectionHeartbeatTest.kt new file mode 100644 index 0000000..6d64097 --- /dev/null +++ b/app/src/test/java/org/prolibertate/games/net/StreamConnectionHeartbeatTest.kt @@ -0,0 +1,181 @@ +package org.prolibertate.games.net + +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job +import kotlinx.coroutines.cancel +import kotlinx.coroutines.delay +import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test +import java.io.InputStream +import java.io.OutputStream +import java.net.InetSocketAddress +import java.net.ServerSocket +import java.net.Socket +import java.util.Collections +import java.util.concurrent.CountDownLatch + +/** + * The heartbeat, which exists because a link with nothing on it cannot be told + * from a link that is no longer there. + * + * Between one turn of a card game and the next, nothing at all crosses the + * socket. A phone whose screen has gone off puts its Wi-Fi radio into power + * save and, given long enough, the link goes with it — and TCP does not say so: + * the read simply never returns, and the next write disappears into a socket + * that goes nowhere. So the transport talks to itself, and treats a peer that + * has stopped answering as gone. + */ +class StreamConnectionHeartbeatTest { + + private val scopes = mutableListOf() + private val closeables = mutableListOf<() -> Unit>() + + @After + fun tearDown() { + closeables.forEach { runCatching { it() } } + scopes.forEach { it.cancel() } + } + + private fun scope(): CoroutineScope = + CoroutineScope(Job() + Dispatchers.Default).also { scopes += it } + + // ----------------------------------------------------------------------- + + /** + * The case the whole thing has to get right: two players thinking, nobody + * moving, for far longer than a link is allowed to be silent. Both ends + * answer each other's pings, so neither concludes the other has gone. + */ + @Test + fun `a quiet link between two live peers is left alone`() = runBlocking { + val server = ServerSocket(0) + val clientSocket = Socket() + closeables += { server.close() } + closeables += { clientSocket.close() } + + clientSocket.connect(InetSocketAddress("127.0.0.1", server.localPort), CONNECT_MILLIS) + val hostSocket = server.accept() + closeables += { hostSocket.close() } + + val hostScope = scope() + val clientScope = scope() + val a = connection("host", hostSocket.getInputStream(), hostSocket.getOutputStream(), hostScope) + val b = connection("guest", clientSocket.getInputStream(), clientSocket.getOutputStream(), clientScope) + + val heardByA = subscribe(a, hostScope) + val heardByB = subscribe(b, clientScope) + + // Several times the silence allowance, with not one game message sent. + delay(SILENCE_MILLIS * 4) + + assertTrue("The host's end gave up on a peer that was answering", a.isOpen) + assertTrue("The guest's end gave up on a peer that was answering", b.isOpen) + // And the pings are the transport's own business: nothing above it sees + // them, so a screen is not asked to redraw twice a second for nothing. + assertEquals(emptyList(), heardByA) + assertEquals(emptyList(), heardByB) + } + + /** + * A link that has died without saying so. The reader would wait on it for + * as long as the game lasted, which is what left a player tapping a board + * that answered nothing. + */ + @Test + fun `a peer that stops answering is given up on and reported`() = runBlocking { + val scope = scope() + val input = BlockingInputStream() + closeables += { input.close() } + + val link = connection("gone", input, DiscardingOutputStream(), scope) + val heard = subscribe(link, scope) + + // Long enough for the silence to be noticed, and then some. + delay(SILENCE_MILLIS * 3) + + assertFalse("A link nobody is answering on was left open", link.isOpen) + assertEquals( + "The screen was never told the link had gone", + listOf(Bye), + heard, + ) + } + + /** A link this end let go of is not somebody else going away. */ + @Test + fun `closing a link on purpose says nothing`() = runBlocking { + val scope = scope() + val input = BlockingInputStream() + closeables += { input.close() } + + val link = connection("ours", input, DiscardingOutputStream(), scope) + val heard = subscribe(link, scope) + + link.close() + delay(SILENCE_MILLIS) + + assertFalse(link.isOpen) + assertEquals(emptyList(), heard) + } + + // ----------------------------------------------------------------------- + + private fun connection( + peerId: String, + input: InputStream, + output: OutputStream, + scope: CoroutineScope, + ) = StreamConnection( + peerId = peerId, + kind = TransportKind.LAN, + input = input, + output = output, + scope = scope, + heartbeatMillis = HEARTBEAT_MILLIS, + silenceMillis = SILENCE_MILLIS, + ) + + /** Everything a screen would have been shown, in order. */ + private fun subscribe(link: Connection, scope: CoroutineScope): List { + val heard = Collections.synchronizedList(mutableListOf()) + scope.launch { link.incoming.collect { heard += it } } + return heard + } + + /** + * A stream that has nothing to say and never will, which is what a link + * whose far end has vanished looks like from here. Blocks until it is + * closed, then fails the read the way a closed socket does. + */ + private class BlockingInputStream : InputStream() { + private val gate = CountDownLatch(1) + + override fun read(): Int { + gate.await() + throw java.io.IOException("stream closed") + } + + override fun close() = gate.countDown() + } + + private class DiscardingOutputStream : OutputStream() { + override fun write(b: Int) = Unit + } + + private companion object { + const val CONNECT_MILLIS = 2_000 + + /** + * The real intervals are seconds; these are the same arrangement run + * fast, so the tests take a moment rather than a minute. + */ + const val HEARTBEAT_MILLIS = 40L + const val SILENCE_MILLIS = 200L + } +} From b98d4649547766acefc692dae4e4ba9c3a7b7af8 Mon Sep 17 00:00:00 2001 From: James Wallace Date: Fri, 2 Oct 2026 21:19:58 +0000 Subject: [PATCH 7/9] Scorekeeper: show an entered 0, tap a past score to edit it Zeros are now stored so a typed 0 shows as 0 while a blank cell stays blank. Tapping a score in an earlier round opens that round for correction with the cursor in the tapped cell, instead of needing the row number first. Co-Authored-By: Claude Sonnet 5.5 --- .../prolibertate/games/score/ScoreSheet.kt | 12 ++-- .../games/ui/ScorekeeperScreen.kt | 59 ++++++++++++------- .../games/score/ScoreSheetTest.kt | 9 +++ 3 files changed, 52 insertions(+), 28 deletions(-) diff --git a/app/src/main/java/org/prolibertate/games/score/ScoreSheet.kt b/app/src/main/java/org/prolibertate/games/score/ScoreSheet.kt index 812f484..adb109f 100644 --- a/app/src/main/java/org/prolibertate/games/score/ScoreSheet.kt +++ b/app/src/main/java/org/prolibertate/games/score/ScoreSheet.kt @@ -21,9 +21,9 @@ data class ScorePlayer(val id: Int, val name: String = "") /** * What each player scored in one round, keyed by [ScorePlayer.id]. * - * A player absent from the map scored nothing, which is the same as zero — so - * a round where only one person scored carries one entry rather than a column - * of noughts. + * A player absent from the map had nothing entered, which counts as zero in the + * totals — so a round where only one person scored carries one entry rather + * than a column of noughts. A 0 that was typed is kept, so it can be shown. */ @Serializable data class ScoreRound(val deltas: Map = emptyMap()) { @@ -114,12 +114,12 @@ data class ScoreSheet( } /** - * A round holding only what is worth storing: entries for players who are - * still at the table, and only where somebody actually scored. + * A round holding only entries for players who are still at the table. + * Zeros stay: a typed 0 is different from a cell left blank. */ private fun roundOf(deltas: Map): ScoreRound { val ids = players.map { it.id }.toSet() - return ScoreRound(deltas.filterKeys { it in ids }.filterValues { it != 0 }) + return ScoreRound(deltas.filterKeys { it in ids }) } /** Drops points recorded against anybody no longer at the table. */ diff --git a/app/src/main/java/org/prolibertate/games/ui/ScorekeeperScreen.kt b/app/src/main/java/org/prolibertate/games/ui/ScorekeeperScreen.kt index ef756bc..e9e276f 100644 --- a/app/src/main/java/org/prolibertate/games/ui/ScorekeeperScreen.kt +++ b/app/src/main/java/org/prolibertate/games/ui/ScorekeeperScreen.kt @@ -42,6 +42,8 @@ import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color import androidx.compose.ui.graphics.SolidColor +import androidx.compose.ui.focus.FocusRequester +import androidx.compose.ui.focus.focusRequester import androidx.compose.ui.graphics.graphicsLayer import androidx.compose.ui.input.pointer.pointerInput import androidx.compose.ui.platform.LocalDensity @@ -193,9 +195,21 @@ private fun Sheet(modifier: Modifier, sheet: ScoreSheet, onChange: (ScoreSheet) var renaming by remember { mutableStateOf(-1) } // Which cell the ± button flips: the last one typed into. var lastTyped by remember { mutableStateOf(-1) } + // The cell to put the cursor in when a round is opened by tapping it. + var focusPlayer by remember { mutableStateOf(-1) } val editing = if (correcting >= 0) correction else pending + // Opens a round for correction. Both the round number and any of its cells + // come through here, so tapping a score edits that score directly. + val startCorrection: (Int, Int) -> Unit = { index, focus -> + correcting = index + correction.clear() + sheet.rounds[index].deltas.forEach { (id, points) -> correction[id] = points.toString() } + lastTyped = -1 + focusPlayer = focus + } + // Keep the row being filled in in view when a round is written down. LaunchedEffect(sheet.rounds.size) { rows.animateScrollTo(rows.maxValue) } @@ -264,15 +278,7 @@ private fun Sheet(modifier: Modifier, sheet: ScoreSheet, onChange: (ScoreSheet) if (underCorrection) { correcting = -1 } else { - correcting = index - correction.clear() - sheet.players.forEach { player -> - val points = round.delta(player.id) - if (points != 0) { - correction[player.id] = points.toString() - } - } - lastTyped = -1 + startCorrection(index, -1) } }, ) @@ -282,13 +288,17 @@ private fun Sheet(modifier: Modifier, sheet: ScoreSheet, onChange: (ScoreSheet) if (underCorrection) { PointsField( text = correction[player.id].orEmpty(), + autoFocus = focusPlayer == player.id, onText = { correction[player.id] = it lastTyped = player.id }, ) } else { - PointsCell(text = signed(round.delta(player.id))) + PointsCell( + text = signed(round.deltas[player.id]), + onClick = { startCorrection(index, player.id) }, + ) } } } @@ -358,17 +368,20 @@ private fun Sheet(modifier: Modifier, sheet: ScoreSheet, onChange: (ScoreSheet) pending.clear() } lastTyped = -1 + focusPlayer = -1 }, onDelete = { onChange(sheet.withoutRound(correcting)) correcting = -1 correction.clear() lastTyped = -1 + focusPlayer = -1 }, onCancel = { correcting = -1 correction.clear() lastTyped = -1 + focusPlayer = -1 }, ) } @@ -583,11 +596,12 @@ private fun RoundLabel(text: String, selected: Boolean, onClick: () -> Unit) { } @Composable -private fun PointsCell(text: String, muted: Boolean = false) { +private fun PointsCell(text: String, muted: Boolean = false, onClick: (() -> Unit)? = null) { Box( modifier = Modifier .fillMaxSize() - .border(Dp.Hairline, MaterialTheme.colorScheme.outlineVariant), + .border(Dp.Hairline, MaterialTheme.colorScheme.outlineVariant) + .then(if (onClick != null) Modifier.clickable { onClick() } else Modifier), contentAlignment = Alignment.Center, ) { Text( @@ -603,9 +617,11 @@ private fun PointsCell(text: String, muted: Boolean = false) { } } -/** A cell being typed into. Blank means nothing scored, which is the same as nought. */ +/** A cell being typed into. Blank means nothing entered; a typed 0 is kept and shown. */ @Composable -private fun PointsField(text: String, onText: (String) -> Unit) { +private fun PointsField(text: String, onText: (String) -> Unit, autoFocus: Boolean = false) { + val focus = remember { FocusRequester() } + LaunchedEffect(autoFocus) { if (autoFocus) focus.requestFocus() } BasicTextField( value = text, onValueChange = { onText(sanitiseAmount(it)) }, @@ -616,7 +632,7 @@ private fun PointsField(text: String, onText: (String) -> Unit) { ), cursorBrush = SolidColor(MaterialTheme.colorScheme.primary), keyboardOptions = KeyboardOptions(keyboardType = KeyboardType.Number), - modifier = Modifier.fillMaxSize(), + modifier = Modifier.fillMaxSize().focusRequester(focus), decorationBox = { field -> Box( modifier = Modifier @@ -758,14 +774,13 @@ private fun flipSign(text: String): String { return (-value).toString() } -/** The typed cells as points. Blanks, a lone minus and noughts all drop out. */ +/** The typed cells as points. Blanks and a lone minus drop out; a typed 0 stays. */ private fun amountsAsPoints(amounts: Map): Map = - amounts.mapNotNull { (id, text) -> text.toIntOrNull()?.takeIf { it != 0 }?.let { id to it } } - .toMap() + amounts.mapNotNull { (id, text) -> text.toIntOrNull()?.let { id to it } }.toMap() -/** Nothing scored shows as an empty cell rather than a nought in every column. */ -private fun signed(points: Int): String = when { +/** Not entered shows as an empty cell; an entered 0 shows as 0. */ +private fun signed(points: Int?): String = when { + points == null -> "" points > 0 -> "+$points" - points < 0 -> points.toString() - else -> "" + else -> points.toString() } diff --git a/app/src/test/java/org/prolibertate/games/score/ScoreSheetTest.kt b/app/src/test/java/org/prolibertate/games/score/ScoreSheetTest.kt index 9454760..32e6e71 100644 --- a/app/src/test/java/org/prolibertate/games/score/ScoreSheetTest.kt +++ b/app/src/test/java/org/prolibertate/games/score/ScoreSheetTest.kt @@ -182,4 +182,13 @@ class ScoreSheetTest { assertEquals(0, sheet.totals()[sheet.players[0].id]) assertEquals(4, sheet.totals()[sheet.players[1].id]) } + + @Test + fun `a zero that was entered is kept, unlike a cell left blank`() { + val sheet = sheetOf("Alice", "Bob").let { it.withRound(mapOf(it.players[0].id to 0)) } + val round = sheet.rounds.first() + assertEquals(0, round.deltas[sheet.players[0].id]) + assertEquals(null, round.deltas[sheet.players[1].id]) + assertEquals(0, sheet.total(sheet.players[0].id)) + } } From 45bc16594ed0a2e3a3e41f6a96647bb12f9300d5 Mon Sep 17 00:00:00 2001 From: James Wallace Date: Fri, 2 Oct 2026 21:23:38 +0000 Subject: [PATCH 8/9] Bump version series to 1.4 Co-Authored-By: Claude Sonnet 5.5 --- gradle.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle.properties b/gradle.properties index 8d326d4..8b173d2 100644 --- a/gradle.properties +++ b/gradle.properties @@ -22,4 +22,4 @@ android.nonTransitiveRClass=true # Only the series is set by hand. The number after it is the CI run number, so # it keeps climbing on its own and an APK's versionCode never goes backwards # when this changes. -plgVersionSeries=1.3 +plgVersionSeries=1.4 From c89c8d15c7fb871183b279d0cb7094b28840a25e Mon Sep 17 00:00:00 2001 From: James Wallace Date: Fri, 9 Oct 2026 19:34:12 +0000 Subject: [PATCH 9/9] CI: bump setup-android to v4 v3 runs sdkmanager with the tools package, which Google's repository no longer has, so the Set up Android SDK step failed before any build. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01CBG7URddsUZjQ6KE1Kqf6G --- .github/workflows/ci.yml | 2 +- .github/workflows/release.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9a0aaac..fc2f564 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -54,7 +54,7 @@ jobs: cache: gradle - name: Set up Android SDK - uses: android-actions/setup-android@v3 + uses: android-actions/setup-android@v4 - name: Build debug APK run: ./gradlew assembleDebug --no-daemon diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 8bd1fc9..4ef3a4a 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -40,7 +40,7 @@ jobs: cache: gradle - name: Set up Android SDK - uses: android-actions/setup-android@v3 + uses: android-actions/setup-android@v4 - name: Build debug APK run: ./gradlew assembleDebug --no-daemon --stacktrace