Merge multiplayer chess crash fix into dev
Leaving a networked game deadlocked the main thread in BufferedReader.close(), so a multiplayer chess game could not be ended. Also adds an End game action, tells the other device when a player leaves, and stops a leave landing back in a live lobby.
This commit is contained in:
Claude
@@ -71,8 +71,22 @@ class MatchController<S : Any, M : Any>(
|
||||
val finished: StateFlow<Boolean> = _finished.asStateFlow()
|
||||
|
||||
private val _notice = MutableStateFlow<String?>(null)
|
||||
|
||||
/** A passing remark about the last thing that happened, such as a refused move. */
|
||||
val notice: StateFlow<String?> = _notice.asStateFlow()
|
||||
|
||||
private val _abandoned = MutableStateFlow<String?>(null)
|
||||
|
||||
/**
|
||||
* Why this table can no longer be played, or null while it still can.
|
||||
*
|
||||
* Kept apart from [notice] because the two want opposite treatment: a
|
||||
* refused move is worth a word in passing, whereas a game with nobody left
|
||||
* to play it has to be brought to the player's attention along with the way
|
||||
* out of it.
|
||||
*/
|
||||
val abandoned: StateFlow<String?> = _abandoned.asStateFlow()
|
||||
|
||||
private val _awaitingConfirmation = MutableStateFlow(false)
|
||||
|
||||
/** True while a scoreboard is up and the table is waiting to be released. */
|
||||
@@ -91,6 +105,21 @@ class MatchController<S : Any, M : Any>(
|
||||
private val seatPeers: Map<Int, String?> =
|
||||
config.seats.associate { it.seat to it.peerId }
|
||||
|
||||
private fun nameOfPeer(peerId: String): String =
|
||||
config.seats.firstOrNull { it.peerId == peerId }?.name ?: "The other player"
|
||||
|
||||
/**
|
||||
* Records that this table cannot be played any further.
|
||||
*
|
||||
* Set on both channels on purpose: screens that show [notice] as a line of
|
||||
* text carry on doing so, and one that wants to stop the player and offer
|
||||
* the way out reads [abandoned] instead.
|
||||
*/
|
||||
private fun abandon(reason: String) {
|
||||
_notice.value = reason
|
||||
_abandoned.value = reason
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// Start-up
|
||||
// -----------------------------------------------------------------------
|
||||
@@ -105,8 +134,15 @@ class MatchController<S : Any, M : Any>(
|
||||
}
|
||||
}
|
||||
scope.launch {
|
||||
val initial = runCatching { rules.initialState(config) }.getOrElse { error ->
|
||||
// A table the rules will not accept is a dead end, not a reason
|
||||
// to take the app down with it: this runs in the screen's own
|
||||
// scope, where nothing would catch it.
|
||||
abandon("This table could not be set up: ${error.message}")
|
||||
return@launch
|
||||
}
|
||||
lock.withLock {
|
||||
authoritative = rules.initialState(config)
|
||||
authoritative = initial
|
||||
publish()
|
||||
}
|
||||
driveIdleSeats()
|
||||
@@ -121,13 +157,30 @@ class MatchController<S : Any, M : Any>(
|
||||
link.incoming.collect { message ->
|
||||
when (message) {
|
||||
is StateSync -> {
|
||||
_state.value = rules.decodeState(message.stateJson)
|
||||
_legalMoves.value = message.legalMoves.map { rules.decodeMove(it) }
|
||||
_finished.value = _state.value?.let { rules.isFinished(it) } ?: false
|
||||
// Anything off the wire is decoded defensively. This
|
||||
// collector runs in the screen's own scope, so throwing
|
||||
// here would take the app down rather than the message.
|
||||
val decoded = runCatching {
|
||||
val state = rules.decodeState(message.stateJson)
|
||||
state to message.legalMoves.map { rules.decodeMove(it) }
|
||||
}.getOrNull()
|
||||
if (decoded == null) {
|
||||
_notice.value = "The host sent something this version cannot read."
|
||||
return@collect
|
||||
}
|
||||
_state.value = decoded.first
|
||||
_legalMoves.value = decoded.second
|
||||
_finished.value = rules.isFinished(decoded.first)
|
||||
}
|
||||
|
||||
is Rejected -> _notice.value = message.reason
|
||||
is Bye -> _notice.value = "The host left the game."
|
||||
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.
|
||||
_legalMoves.value = emptyList()
|
||||
abandon("The host left the game.")
|
||||
}
|
||||
|
||||
else -> Unit
|
||||
}
|
||||
}
|
||||
@@ -156,6 +209,13 @@ class MatchController<S : Any, M : Any>(
|
||||
}
|
||||
|
||||
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.")
|
||||
return
|
||||
}
|
||||
if (message !is MoveIntent) return
|
||||
val seat = seatPeers.entries.firstOrNull { it.value == connection.peerId }?.key
|
||||
if (seat == null) {
|
||||
@@ -244,13 +304,21 @@ class MatchController<S : Any, M : Any>(
|
||||
val applied = lock.withLock {
|
||||
val live = authoritative ?: return
|
||||
// Nothing else can move while an AI seat is on the clock, but
|
||||
// the state is re-read rather than assumed.
|
||||
// the state is re-read rather than assumed. The move was chosen
|
||||
// off this thread and a chess search takes seconds, so it is
|
||||
// checked against the position as it stands now: applyMove
|
||||
// throws on a move that is no longer legal, and there is nothing
|
||||
// above this to catch it.
|
||||
if (rules.currentSeat(live) != seat) return@withLock false
|
||||
if (move !in rules.legalMoves(live, seat)) return@withLock false
|
||||
authoritative = rules.applyMove(live, seat, move)
|
||||
publish()
|
||||
true
|
||||
}
|
||||
if (!applied) return
|
||||
// The position moved on under the search. Go round again and pick a
|
||||
// move for the position as it now stands; the loop returns on its own
|
||||
// as soon as a person is on the clock.
|
||||
if (!applied) continue
|
||||
}
|
||||
}
|
||||
|
||||
@@ -299,6 +367,15 @@ class MatchController<S : Any, M : Any>(
|
||||
scope.launch { driveIdleSeats() }
|
||||
}
|
||||
|
||||
/** Lets a player go on looking at a table they have been told is over. */
|
||||
fun dismissAbandoned() {
|
||||
_abandoned.value = null
|
||||
}
|
||||
|
||||
/**
|
||||
* Lets go of every link. Safe on the thread drawing the screen — see
|
||||
* [StreamConnection.close], which this leans on to not block.
|
||||
*/
|
||||
fun close() {
|
||||
peers.values.forEach { runCatching { it.close() } }
|
||||
peers.clear()
|
||||
|
||||
@@ -2,6 +2,7 @@ package org.prolibertate.games.net
|
||||
|
||||
import kotlinx.coroutines.CoroutineScope
|
||||
import kotlinx.coroutines.Dispatchers
|
||||
import kotlinx.coroutines.NonCancellable
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
import kotlinx.coroutines.flow.MutableSharedFlow
|
||||
import kotlinx.coroutines.flow.SharedFlow
|
||||
@@ -73,9 +74,16 @@ 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()) {
|
||||
val line = reader.readLine() ?: break
|
||||
val line = reader.readLine()
|
||||
if (line == null) {
|
||||
droppedByPeer = true
|
||||
break
|
||||
}
|
||||
if (line.isBlank()) continue
|
||||
val message = runCatching {
|
||||
protocolJson.decodeFromString<NetMessage>(line)
|
||||
@@ -85,8 +93,23 @@ class StreamConnection(
|
||||
} catch (_: Exception) {
|
||||
// A dropped link is normal: someone walked out of range or
|
||||
// closed the app. Fall through to close().
|
||||
droppedByPeer = true
|
||||
} finally {
|
||||
// Safe here and nowhere else: this thread owns the reader's
|
||||
// lock, so it is the only one that can close it without
|
||||
// waiting. See close().
|
||||
runCatching { reader.close() }
|
||||
val weClosedIt = closed.get()
|
||||
close()
|
||||
// A link that died on its own reaches the screens on the same
|
||||
// channel as everything else, so leaving a game 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) {
|
||||
withContext(NonCancellable) { runCatching { _incoming.emit(Bye) } }
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -104,10 +127,24 @@ class StreamConnection(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Safe to call from any thread, including the one drawing the screen.
|
||||
*
|
||||
* Only the raw streams are closed here, and that is deliberate. A reader
|
||||
* parked in [BufferedReader.readLine] holds the reader's own lock for the
|
||||
* whole of the blocking read, and [BufferedReader.close] wants that same
|
||||
* lock — so closing the wrapper from another thread waits for a read that
|
||||
* only this close can end. Since the read is waiting on the socket, and the
|
||||
* caller is usually the main thread leaving a game, that wait is the app
|
||||
* hanging for good.
|
||||
*
|
||||
* Closing the underlying stream is what actually ends the read. The reader
|
||||
* coroutine then unblocks and closes its own wrapper on its own thread. The
|
||||
* writer needs no closing: every [send] flushes, so nothing is ever left
|
||||
* buffered in it.
|
||||
*/
|
||||
override fun close() {
|
||||
if (!closed.compareAndSet(false, true)) return
|
||||
runCatching { reader.close() }
|
||||
runCatching { writer.close() }
|
||||
runCatching { input.close() }
|
||||
runCatching { output.close() }
|
||||
onClosed(this)
|
||||
|
||||
@@ -86,12 +86,23 @@ fun AppRoot(env: AppEnv) {
|
||||
|
||||
val pop: () -> Unit = {
|
||||
if (stack.size > 1) {
|
||||
val leaving = stack.last()
|
||||
// Leaving a lobby or a table tears the networking down with it.
|
||||
when (stack.last()) {
|
||||
when (leaving) {
|
||||
is Route.Lobby, is Route.Play -> env.lobby.stop()
|
||||
else -> Unit
|
||||
}
|
||||
stack = stack.dropLast(1)
|
||||
stack = stack.dropLast(1).let { remaining ->
|
||||
// Leaving a table must not land back in the lobby that started
|
||||
// it. A lobby starts advertising the moment it is drawn, so
|
||||
// ending a game would have put the player straight into another
|
||||
// one — which is no way to leave a game.
|
||||
if (leaving is Route.Play) {
|
||||
remaining.dropLastWhile { it is Route.Lobby }
|
||||
} else {
|
||||
remaining
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -72,16 +72,32 @@ fun ChessScreen(
|
||||
) {
|
||||
val state by controller.state.collectAsState()
|
||||
val legal by controller.legalMoves.collectAsState()
|
||||
val abandoned by controller.abandoned.collectAsState()
|
||||
|
||||
var selected by remember { mutableStateOf<Int?>(null) }
|
||||
// A pawn reaching the last rank is one tap that stands for four moves, so
|
||||
// the destination is held while the piece is chosen.
|
||||
var promotionTo by remember { mutableStateOf<Int?>(null) }
|
||||
// A game with nobody left to play it, or one being walked away from, both end
|
||||
// through a dialog rather than by silently dropping off the screen.
|
||||
var confirmingEnd by remember { mutableStateOf(false) }
|
||||
|
||||
val endGameAction: @Composable () -> Unit = {
|
||||
TextButton(onClick = { confirmingEnd = true }) { Text("End game") }
|
||||
}
|
||||
|
||||
val current = state
|
||||
if (current == null) {
|
||||
ScreenScaffold(title = "Chess", onBack = onExit) { modifier ->
|
||||
ScreenScaffold(title = "Chess", onBack = onExit, actions = endGameAction) { modifier ->
|
||||
Box(modifier, contentAlignment = Alignment.Center) { Text("Setting up…") }
|
||||
// A table that never arrives — a host that went away mid-handshake —
|
||||
// still has to be leavable.
|
||||
abandoned?.let {
|
||||
LeftTheTableDialog(it, onExit = onExit, onStay = controller::dismissAbandoned)
|
||||
}
|
||||
if (confirmingEnd) {
|
||||
ConfirmEndDialog(onEnd = onExit, onKeepPlaying = { confirmingEnd = false })
|
||||
}
|
||||
}
|
||||
return
|
||||
}
|
||||
@@ -104,7 +120,7 @@ fun ChessScreen(
|
||||
}
|
||||
}
|
||||
|
||||
ScreenScaffold(title = "Chess", onBack = onExit) { modifier ->
|
||||
ScreenScaffold(title = "Chess", onBack = onExit, actions = endGameAction) { modifier ->
|
||||
BoxWithConstraints(modifier = modifier) {
|
||||
val wide = maxWidth > maxHeight
|
||||
val boardSide = if (wide) {
|
||||
@@ -152,10 +168,21 @@ fun ChessScreen(
|
||||
}
|
||||
}
|
||||
|
||||
promotionTo?.let { destination ->
|
||||
PromotionDialog(
|
||||
// One dialog at a time, most pressing first. A game nobody can finish
|
||||
// comes before the result of one that finished, and before a
|
||||
// half-made promotion: there is no point choosing a piece for a move
|
||||
// that has nowhere left to go.
|
||||
val over = abandoned
|
||||
when {
|
||||
over != null ->
|
||||
LeftTheTableDialog(over, onExit = onExit, onStay = controller::dismissAbandoned)
|
||||
|
||||
confirmingEnd ->
|
||||
ConfirmEndDialog(onEnd = onExit, onKeepPlaying = { confirmingEnd = false })
|
||||
|
||||
promotionTo != null -> PromotionDialog(
|
||||
white = current.whiteToMove,
|
||||
choices = fromSelected.filter { it.to == destination },
|
||||
choices = fromSelected.filter { it.to == promotionTo },
|
||||
onPick = {
|
||||
controller.submit(it)
|
||||
promotionTo = null
|
||||
@@ -163,15 +190,48 @@ fun ChessScreen(
|
||||
},
|
||||
onDismiss = { promotionTo = null },
|
||||
)
|
||||
}
|
||||
|
||||
if (current.phase == ChessPhase.GAME_OVER) {
|
||||
ResultDialog(current, onExit)
|
||||
current.phase == ChessPhase.GAME_OVER -> ResultDialog(current, onExit)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Leaving a game on purpose.
|
||||
*
|
||||
* Chess is the one game here that a player wants out of before the position is
|
||||
* settled: a card hand is over in minutes, whereas a game of chess between two
|
||||
* people who are no longer enjoying it has no natural end at all.
|
||||
*/
|
||||
@Composable
|
||||
private fun ConfirmEndDialog(onEnd: () -> Unit, onKeepPlaying: () -> Unit) {
|
||||
AlertDialog(
|
||||
onDismissRequest = onKeepPlaying,
|
||||
title = { Text("End this game?") },
|
||||
text = {
|
||||
Text(
|
||||
"This game ends here and the position is not saved. Anyone else " +
|
||||
"at the table is told you have left."
|
||||
)
|
||||
},
|
||||
confirmButton = { TextButton(onClick = onEnd) { Text("End the game") } },
|
||||
dismissButton = { TextButton(onClick = onKeepPlaying) { Text("Keep playing") } },
|
||||
)
|
||||
}
|
||||
|
||||
/** The other end has gone, so no further move is ever going to arrive. */
|
||||
@Composable
|
||||
private fun LeftTheTableDialog(notice: String, onExit: () -> Unit, onStay: () -> Unit) {
|
||||
AlertDialog(
|
||||
onDismissRequest = onStay,
|
||||
title = { Text("The game has ended") },
|
||||
text = { Text("$notice There are no more moves to play.") },
|
||||
confirmButton = { TextButton(onClick = onExit) { Text("Leave the game") } },
|
||||
dismissButton = { TextButton(onClick = onStay) { Text("Look at the board") } },
|
||||
)
|
||||
}
|
||||
|
||||
@Composable
|
||||
private fun Board(
|
||||
state: ChessState,
|
||||
@@ -372,7 +432,7 @@ private fun ResultDialog(state: ChessState, onExit: () -> Unit) {
|
||||
)
|
||||
}
|
||||
},
|
||||
confirmButton = { TextButton(onClick = onExit) { Text("Back to the menu") } },
|
||||
confirmButton = { TextButton(onClick = onExit) { Text("Leave the game") } },
|
||||
dismissButton = { TextButton(onClick = { dismissed = true }) { Text("Look at the board") } },
|
||||
)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user