Show every player the table from the moment the game starts
A guest saw nothing until the next state push, which on most tables means until its own turn came round: no board, no cards, no scoreboard. The cause is a gap in who is listening. A guest's link is opened in the lobby and read there; the table only subscribes to it once the host's start message has been through a state change, a recomposition and a LaunchedEffect. The host publishes the opening position immediately, into a link whose table has not attached yet, and incoming is a SharedFlow with replay = 0 — so that first push went to the lobby's collector and nowhere else. Not a rare race but the normal order of events, which is why it looked like a rule rather than a bug. The guest now asks. A new Resync message is sent from inside onSubscription, so the request cannot leave before the collector is attached and the reply cannot arrive before there is anything to receive it. The host answers one peer with that seat's redacted view. A request that arrives before the deal needs no answer, since a peer that has asked is listening and the opening publish will reach it. Second reason a guest could not see its hand, on any table with more than one guest: guests worked out their own seat by matching PlayerSlot.peerId, which is the address the host's transport invented for the incoming link — an IP on Wi-Fi, a MAC over Bluetooth. A guest has never seen that value, so no guest ever matched it and every one fell through to "the first seat held by somebody remote". With one guest that is right by luck; with two, both claimed the same seat and the other drew a hand belonging to a seat it was not sitting in, which redacted is no hand at all. The host now records the id a guest introduced itself by, as PlayerSlot.deviceId, kept separate from peerId because the two answer different questions: peerId is how the host recognises a link, deviceId is how a guest recognises itself. The lookup moved out of LobbyScreen into seatForDevice, where it can be tested, and a dropped player stops claiming its old seat. Neither change needs a protocol bump. Resync is ignored by a host that does not know it, deviceId is an unknown key to a guest that does not read it, and the old fallback is kept for exactly that case. Tests reproduce the real start-up order — host publishes, then the guest subscribes a frame later — rather than the order that happens to work, and cover a board, a dealt hand, and a guest that is on the clock at the deal. Both regressions were confirmed to fail before the fix: the guest had no state at all, and two guests both came back as seat 1. 216 JVM tests pass.
This commit is contained in:
Claude
@@ -5,6 +5,7 @@ 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 kotlinx.serialization.encodeToString
|
||||
import kotlinx.serialization.json.Json
|
||||
@@ -23,6 +24,13 @@ import org.prolibertate.games.game.chess.ChessPhase
|
||||
import org.prolibertate.games.game.chess.ChessRules
|
||||
import org.prolibertate.games.game.chess.ChessState
|
||||
import org.prolibertate.games.game.chess.squareFromName
|
||||
import org.prolibertate.games.game.crazyeights.CrazyEightsAi
|
||||
import org.prolibertate.games.game.crazyeights.CrazyEightsMove
|
||||
import org.prolibertate.games.game.crazyeights.CrazyEightsOptions
|
||||
import org.prolibertate.games.game.crazyeights.CrazyEightsRules
|
||||
import org.prolibertate.games.game.crazyeights.CrazyEightsState
|
||||
import org.prolibertate.games.game.engine.GameAi
|
||||
import org.prolibertate.games.game.engine.GameRules
|
||||
import org.prolibertate.games.game.engine.PlayerKind
|
||||
import org.prolibertate.games.game.engine.PlayerSlot
|
||||
import org.prolibertate.games.game.engine.TableConfig
|
||||
@@ -36,30 +44,139 @@ import java.net.Socket
|
||||
*
|
||||
* The engines are covered exhaustively elsewhere; what is worth testing here is
|
||||
* the part no rules test can reach — a game with a live connection in it being
|
||||
* played, finished, and above all *left*, which is where a networked table has
|
||||
* to let go of a socket while the screen that owns it is being torn down.
|
||||
* started, played, and left.
|
||||
*
|
||||
* The order matters and is reproduced faithfully. A guest's link is opened in the
|
||||
* lobby and is already being read there; the match screen only subscribes to it
|
||||
* once the host's "start" message has been through a recomposition. So the host
|
||||
* is always publishing into a link whose match screen has not attached yet, which
|
||||
* is the gap [Table.start] leaves open on purpose.
|
||||
*/
|
||||
class MatchControllerNetworkTest {
|
||||
|
||||
private val tables = mutableListOf<Table>()
|
||||
private val tables = mutableListOf<Table<*, *>>()
|
||||
|
||||
@After
|
||||
fun tearDown() {
|
||||
tables.forEach { it.dispose() }
|
||||
}
|
||||
|
||||
private fun table(): Table = Table().also { tables += it }
|
||||
private fun chess(hostSeat: Int = HOST_SEAT): Table<ChessState, ChessMove> = Table(
|
||||
rules = ChessRules,
|
||||
ai = ChessAi(),
|
||||
gameId = GameCatalog.CHESS,
|
||||
optionsJson = testJson.encodeToString(ChessOptions()),
|
||||
seatCount = 2,
|
||||
hostSeat = hostSeat,
|
||||
).also { tables += it }
|
||||
|
||||
private fun crazyEights(): Table<CrazyEightsState, CrazyEightsMove> = Table(
|
||||
rules = CrazyEightsRules,
|
||||
ai = CrazyEightsAi(),
|
||||
gameId = GameCatalog.CRAZY_EIGHTS,
|
||||
optionsJson = testJson.encodeToString(CrazyEightsOptions(playerCount = 2)),
|
||||
seatCount = 2,
|
||||
).also { tables += it }
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// Everyone can see the table from the moment it starts
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* The guest's screen has to have something to draw as soon as the game
|
||||
* starts. It used to have nothing until the next state push, which on most
|
||||
* tables means until the guest's own turn came round — a board that does not
|
||||
* appear, and a hand of cards that does not either.
|
||||
*/
|
||||
@Test
|
||||
fun `a guest can see the board as soon as the game starts`() = runBlocking {
|
||||
val table = chess()
|
||||
table.start()
|
||||
|
||||
assertNotNull(
|
||||
"The guest has no state to draw: the opening position never reached it",
|
||||
table.client.state.value,
|
||||
)
|
||||
assertEquals(ChessPhase.PLAYING, table.client.state.value?.phase)
|
||||
// 32 men on the board, before anybody has moved.
|
||||
assertEquals(32, table.client.state.value?.board?.count { it != null })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a guest can see its own hand as soon as the game starts`() = runBlocking {
|
||||
val table = crazyEights()
|
||||
table.start()
|
||||
|
||||
val view = table.client.state.value
|
||||
assertNotNull("The guest was never dealt a visible hand", view)
|
||||
assertTrue(
|
||||
"The guest cannot see its own cards: hand was ${view?.hands?.get(GUEST_SEAT)}",
|
||||
(view?.hands?.get(GUEST_SEAT)?.size ?: 0) > 0,
|
||||
)
|
||||
// The table itself, not just the hand: something has been turned up to
|
||||
// play on to.
|
||||
assertNotNull("No discard pile to play on to", view?.topCard)
|
||||
// And the opponent's hand is still none of the guest's business.
|
||||
assertEquals(emptyList<Any>(), view?.hands?.get(HOST_SEAT))
|
||||
assertTrue(
|
||||
"Opponent's card count should still be known",
|
||||
(view?.handCounts?.get(HOST_SEAT) ?: 0) > 0,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the player on the clock is given its moves as soon as the game starts`() = runBlocking {
|
||||
// Crazy 8s deals to seat 0 first, so on this table it is the host that is
|
||||
// on the clock; the guest correctly has nothing to play yet.
|
||||
val table = crazyEights()
|
||||
table.start()
|
||||
|
||||
assertEquals(HOST_SEAT, table.host.state.value?.turn)
|
||||
assertTrue("The host has no moves to make on its own turn", table.host.legalMoves.value.isNotEmpty())
|
||||
assertEquals(emptyList<CrazyEightsMove>(), table.client.legalMoves.value)
|
||||
}
|
||||
|
||||
/**
|
||||
* A guest can be the one to move first — a card game's turn order follows the
|
||||
* deal, not the seating — and a guest that can see the table but has nothing
|
||||
* it is allowed to play is no better off than one that can see nothing.
|
||||
*/
|
||||
@Test
|
||||
fun `a guest on the clock at the deal is given its moves`() = runBlocking {
|
||||
// The host takes Black, which leaves the guest playing White and first to
|
||||
// move before either device has done anything.
|
||||
val table = chess(hostSeat = 1)
|
||||
table.start()
|
||||
|
||||
assertNotNull(table.client.state.value)
|
||||
assertEquals(
|
||||
"The guest has to move but was given nothing to play",
|
||||
20,
|
||||
table.client.legalMoves.value.size,
|
||||
)
|
||||
assertEquals(emptyList<ChessMove>(), table.host.legalMoves.value)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a guest is given its moves when its turn arrives`() = runBlocking {
|
||||
val table = chess()
|
||||
table.start()
|
||||
|
||||
assertEquals(emptyList<ChessMove>(), table.client.legalMoves.value)
|
||||
table.host.submit(ChessMove(squareFromName("e2"), squareFromName("e4")))
|
||||
table.settle()
|
||||
// Twenty legal replies to 1. e4.
|
||||
assertEquals(20, table.client.legalMoves.value.size)
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// Playing and finishing
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
@Test
|
||||
fun `a networked game plays to a result on both devices`() = runBlocking {
|
||||
val table = table()
|
||||
table.settle()
|
||||
|
||||
assertEquals(ChessPhase.PLAYING, table.client.state.value?.phase)
|
||||
// Black is not on the clock, so has nothing to play.
|
||||
assertEquals(0, table.client.legalMoves.value.size)
|
||||
val table = chess()
|
||||
table.start()
|
||||
|
||||
table.white("f2", "f3")
|
||||
table.black("e7", "e5")
|
||||
@@ -72,9 +189,11 @@ class MatchControllerNetworkTest {
|
||||
assertTrue(table.client.finished.value)
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// Leaving
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* The regression this file exists for.
|
||||
*
|
||||
* Leaving a game runs on the thread drawing the screen — Android's main
|
||||
* thread — by way of AppRoot.pop() and PlayScreen's onDispose. It closed the
|
||||
* buffered reader first, which wants the same lock readLine() holds for the
|
||||
@@ -85,8 +204,8 @@ class MatchControllerNetworkTest {
|
||||
@Test
|
||||
fun `leaving a game in progress does not block the thread that leaves`() = runBlocking {
|
||||
for (side in listOf(Side.HOST, Side.GUEST)) {
|
||||
val table = table()
|
||||
table.settle()
|
||||
val table = chess()
|
||||
table.start()
|
||||
table.white("e2", "e4")
|
||||
|
||||
// Left on its own thread, and a daemon one: should this ever hang
|
||||
@@ -105,25 +224,25 @@ class MatchControllerNetworkTest {
|
||||
|
||||
@Test
|
||||
fun `a player leaving is reported to the device still at the table`() = runBlocking {
|
||||
val guestLeft = table()
|
||||
guestLeft.settle()
|
||||
val guestLeft = chess()
|
||||
guestLeft.start()
|
||||
guestLeft.leave(Side.GUEST)
|
||||
guestLeft.settle()
|
||||
assertEquals("Guest left the game.", guestLeft.host.abandoned.value)
|
||||
|
||||
val hostLeft = table()
|
||||
hostLeft.settle()
|
||||
val hostLeft = chess()
|
||||
hostLeft.start()
|
||||
hostLeft.leave(Side.HOST)
|
||||
hostLeft.settle()
|
||||
assertEquals("The host left the game.", hostLeft.client.abandoned.value)
|
||||
// Nothing more can be played on a table with no host.
|
||||
assertEquals(0, hostLeft.client.legalMoves.value.size)
|
||||
assertEquals(emptyList<ChessMove>(), hostLeft.client.legalMoves.value)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `closing a link on purpose is not reported as the other player leaving`() = runBlocking {
|
||||
val table = table()
|
||||
table.settle()
|
||||
val table = chess()
|
||||
table.start()
|
||||
assertNull(table.host.abandoned.value)
|
||||
|
||||
// The host closes its own end, as leaving does. Its own screen has no
|
||||
@@ -139,8 +258,15 @@ class MatchControllerNetworkTest {
|
||||
|
||||
private enum class Side { HOST, GUEST }
|
||||
|
||||
/** One two-seat chess table with a real socket between the two devices. */
|
||||
private class Table {
|
||||
/** One two-seat table with a real socket between the two devices. */
|
||||
private class Table<S : Any, M : Any>(
|
||||
private val rules: GameRules<S, M>,
|
||||
ai: GameAi<S, M>,
|
||||
gameId: String,
|
||||
optionsJson: String,
|
||||
seatCount: Int,
|
||||
private val hostSeat: Int = HOST_SEAT,
|
||||
) {
|
||||
private val server = ServerSocket(0)
|
||||
private val clientSocket = Socket()
|
||||
private val hostSocket: Socket
|
||||
@@ -149,8 +275,8 @@ class MatchControllerNetworkTest {
|
||||
|
||||
val hostLink: StreamConnection
|
||||
private val clientLink: StreamConnection
|
||||
val host: MatchController<ChessState, ChessMove>
|
||||
val client: MatchController<ChessState, ChessMove>
|
||||
val host: MatchController<S, M>
|
||||
val client: MatchController<S, M>
|
||||
|
||||
init {
|
||||
clientSocket.connect(InetSocketAddress("127.0.0.1", server.localPort), CONNECT_MILLIS)
|
||||
@@ -175,19 +301,28 @@ class MatchControllerNetworkTest {
|
||||
)
|
||||
|
||||
val config = TableConfig(
|
||||
gameId = GameCatalog.CHESS,
|
||||
seats = listOf(
|
||||
PlayerSlot(0, "Host", PlayerKind.HUMAN_LOCAL, team = 0),
|
||||
PlayerSlot(1, "Guest", PlayerKind.HUMAN_REMOTE, team = 1, peerId = guestPeerId),
|
||||
),
|
||||
optionsJson = Json { encodeDefaults = true }.encodeToString(ChessOptions()),
|
||||
gameId = gameId,
|
||||
seats = (0 until seatCount).map { seat ->
|
||||
PlayerSlot(
|
||||
seat = seat,
|
||||
name = if (seat == hostSeat) "Host" else "Guest",
|
||||
kind = if (seat == hostSeat) {
|
||||
PlayerKind.HUMAN_LOCAL
|
||||
} else {
|
||||
PlayerKind.HUMAN_REMOTE
|
||||
},
|
||||
team = seat,
|
||||
peerId = if (seat == hostSeat) null else guestPeerId,
|
||||
)
|
||||
},
|
||||
optionsJson = optionsJson,
|
||||
seed = 1L,
|
||||
)
|
||||
|
||||
fun controller(role: MatchController.Role, seat: Int, scope: CoroutineScope) =
|
||||
MatchController<ChessState, ChessMove>(
|
||||
rules = ChessRules,
|
||||
ai = ChessAi(),
|
||||
MatchController(
|
||||
rules = rules,
|
||||
ai = ai,
|
||||
config = config,
|
||||
scope = scope,
|
||||
role = role,
|
||||
@@ -196,22 +331,44 @@ class MatchControllerNetworkTest {
|
||||
aiThinkingMillis = { 0L },
|
||||
)
|
||||
|
||||
host = controller(MatchController.Role.HOST, 0, hostScope)
|
||||
client = controller(MatchController.Role.CLIENT, 1, clientScope)
|
||||
client.startAsClient(clientLink)
|
||||
host = controller(MatchController.Role.HOST, hostSeat, hostScope)
|
||||
client = controller(
|
||||
MatchController.Role.CLIENT,
|
||||
(0 until seatCount).first { it != hostSeat },
|
||||
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
|
||||
// would be the only reader, which is not the arrangement the app has.
|
||||
clientScope.launch { clientLink.incoming.collect { /* the lobby's collector */ } }
|
||||
}
|
||||
|
||||
/**
|
||||
* Starts the match the way the two devices do: the host first, then the
|
||||
* guest once its screen has been through a frame. Anything the host
|
||||
* publishes in between is published into a link the guest's match screen
|
||||
* has not subscribed to yet.
|
||||
*/
|
||||
suspend fun start() {
|
||||
host.startAsHost(listOf(hostLink))
|
||||
delay(NAVIGATION_MILLIS)
|
||||
client.startAsClient(clientLink)
|
||||
settle()
|
||||
}
|
||||
|
||||
/** Lets whatever is in flight cross the socket and be applied. */
|
||||
suspend fun settle() = delay(SETTLE_MILLIS)
|
||||
|
||||
suspend fun white(from: String, to: String) {
|
||||
host.submit(move(from, to))
|
||||
@Suppress("UNCHECKED_CAST")
|
||||
host.submit(chessMove(from, to) as M)
|
||||
settle()
|
||||
}
|
||||
|
||||
suspend fun black(from: String, to: String) {
|
||||
client.submit(move(from, to))
|
||||
@Suppress("UNCHECKED_CAST")
|
||||
client.submit(chessMove(from, to) as M)
|
||||
settle()
|
||||
}
|
||||
|
||||
@@ -244,19 +401,30 @@ class MatchControllerNetworkTest {
|
||||
clientScope.cancel()
|
||||
}
|
||||
|
||||
private fun move(from: String, to: String) =
|
||||
private fun chessMove(from: String, to: String) =
|
||||
ChessMove(squareFromName(from), squareFromName(to))
|
||||
}
|
||||
|
||||
private companion object {
|
||||
const val HOST_SEAT = 0
|
||||
const val GUEST_SEAT = 1
|
||||
const val CONNECT_MILLIS = 2_000
|
||||
const val SETTLE_MILLIS = 300L
|
||||
|
||||
/**
|
||||
* Stands in for the guest's trip from the lobby to the table: a state
|
||||
* change, a recomposition and a LaunchedEffect, none of which the host
|
||||
* waits for.
|
||||
*/
|
||||
const val NAVIGATION_MILLIS = 250L
|
||||
|
||||
/**
|
||||
* Generous on purpose: leaving takes single-digit milliseconds, and the
|
||||
* failure this guards against is not a slow close but one that never
|
||||
* returns at all.
|
||||
*/
|
||||
const val BLOCKED_MILLIS = 5_000L
|
||||
|
||||
val testJson = Json { encodeDefaults = true }
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,105 @@
|
||||
package org.prolibertate.games.net
|
||||
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Test
|
||||
import org.prolibertate.games.game.GameCatalog
|
||||
import org.prolibertate.games.game.engine.PlayerKind
|
||||
import org.prolibertate.games.game.engine.PlayerSlot
|
||||
import org.prolibertate.games.game.engine.TableConfig
|
||||
|
||||
/**
|
||||
* Which seat a guest sits in, worked out from the seat list the host sends.
|
||||
*
|
||||
* A guest that gets this wrong draws the wrong seat's hand — and since the host
|
||||
* redacts every hand but the one it addressed the view to, the wrong seat's hand
|
||||
* is an empty one. So this is what stands between a guest and a table it cannot
|
||||
* see itself at.
|
||||
*/
|
||||
class SeatForDeviceTest {
|
||||
|
||||
@Test
|
||||
fun `a guest finds itself by its own device id`() {
|
||||
val config = table(
|
||||
host(0),
|
||||
guest(1, deviceId = "device-a"),
|
||||
guest(2, deviceId = "device-b"),
|
||||
)
|
||||
|
||||
assertEquals(1, seatForDevice(config, "device-a"))
|
||||
assertEquals(2, seatForDevice(config, "device-b"))
|
||||
}
|
||||
|
||||
/**
|
||||
* The regression this exists for.
|
||||
*
|
||||
* The seat used to be looked up by [PlayerSlot.peerId], which is the address
|
||||
* the host's own transport made up for the incoming link — an IP on Wi-Fi, a
|
||||
* MAC over Bluetooth. A guest has never seen that value and so never matched
|
||||
* it, and every guest fell through to "the first seat held by somebody
|
||||
* remote". With one guest that is right by luck. With two, both of them
|
||||
* claimed seat 1, and the one really sitting in seat 2 was left looking at a
|
||||
* table with no cards in front of it.
|
||||
*/
|
||||
@Test
|
||||
fun `two guests do not both claim the first remote seat`() {
|
||||
val config = table(
|
||||
host(0),
|
||||
// Filed under a transport address, as the host files them.
|
||||
guest(1, deviceId = "device-a", peerId = "192.168.1.20"),
|
||||
guest(2, deviceId = "device-b", peerId = "192.168.1.21"),
|
||||
)
|
||||
|
||||
val seats = listOf(seatForDevice(config, "device-a"), seatForDevice(config, "device-b"))
|
||||
assertEquals(listOf(1, 2), seats)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `seats held by the computer are never claimed by a guest`() {
|
||||
val config = table(host(0), guest(1, deviceId = "device-a"), ai(2))
|
||||
|
||||
assertEquals(1, seatForDevice(config, "device-a"))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a guest dropped for the computer no longer claims its old seat`() {
|
||||
// What setSeatKind leaves behind when the host drops somebody.
|
||||
val config = table(host(0), ai(1), guest(2, deviceId = "device-b"))
|
||||
|
||||
assertEquals(2, seatForDevice(config, "device-b"))
|
||||
}
|
||||
|
||||
/**
|
||||
* A host on an older version sends no device ids at all. One guest still
|
||||
* lands in the right seat, which is what keeps a mixed pair of devices
|
||||
* playable rather than silently wrong.
|
||||
*/
|
||||
@Test
|
||||
fun `a single guest still finds its seat without device ids`() {
|
||||
val config = table(host(0), guest(1, deviceId = null), ai(2), ai(3))
|
||||
|
||||
assertEquals(1, seatForDevice(config, "device-a"))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an unknown device falls back to a seat rather than failing`() {
|
||||
assertEquals(0, seatForDevice(table(host(0), ai(1)), "who-is-this"))
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
private fun table(vararg seats: PlayerSlot) = TableConfig(
|
||||
gameId = GameCatalog.CRAZY_EIGHTS,
|
||||
seats = seats.toList(),
|
||||
optionsJson = "{}",
|
||||
seed = 1L,
|
||||
)
|
||||
|
||||
private fun host(seat: Int) =
|
||||
PlayerSlot(seat, "Host", PlayerKind.HUMAN_LOCAL, team = seat)
|
||||
|
||||
private fun guest(seat: Int, deviceId: String?, peerId: String = "10.0.0.$seat") =
|
||||
PlayerSlot(seat, "Guest $seat", PlayerKind.HUMAN_REMOTE, team = seat, peerId, deviceId)
|
||||
|
||||
private fun ai(seat: Int) =
|
||||
PlayerSlot(seat, "Computer $seat", PlayerKind.AI, team = seat)
|
||||
}
|
||||
Reference in New Issue
Block a user