Fix race conditions for game start and tests

Resolves:
https://github.com/joffrey-bion/seven-wonders/issues/70

6 files changed, 53 insertions, 50 deletions

diff --git a/sw-bot/src/main/kotlin/org/luxons/sevenwonders/bot/SevenWondersBot.kt b/sw-bot/src/main/kotlin/org/luxons/sevenwonders/bot/SevenWondersBot.kt
index 87ba7983..6170acb8 100644
--- a/sw-bot/src/main/kotlin/org/luxons/sevenwonders/bot/SevenWondersBot.kt
+++ b/sw-bot/src/main/kotlin/org/luxons/sevenwonders/bot/SevenWondersBot.kt
@@ -2,10 +2,7 @@ package org.luxons.sevenwonders.bot
 
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.delay
-import kotlinx.coroutines.flow.collect
-import kotlinx.coroutines.flow.onCompletion
-import kotlinx.coroutines.flow.onStart
-import kotlinx.coroutines.flow.takeWhile
+import kotlinx.coroutines.flow.*
 import kotlinx.coroutines.withTimeout
 import org.luxons.sevenwonders.client.SevenWondersClient
 import org.luxons.sevenwonders.client.SevenWondersSession
@@ -38,8 +35,9 @@ class SevenWondersBot(
     suspend fun play(serverUrl: String, gameId: Long) = withTimeout(botConfig.globalTimeout) {
         val session = client.connect(serverUrl)
         session.chooseName(displayName, Icon("desktop"))
+        val gameStartedEvents = session.watchGameStarted()
         session.joinGameAndWaitLobby(gameId)
-        val firstTurn = session.awaitGameStart(gameId)
+        val firstTurn = gameStartedEvents.first()
 
         session.watchTurns()
             .onStart { emit(firstTurn) }
diff --git a/sw-client/src/commonMain/kotlin/org/luxons/sevenwonders/client/SevenWondersClient.kt b/sw-client/src/commonMain/kotlin/org/luxons/sevenwonders/client/SevenWondersClient.kt
index 6cc50f44..02f7f2da 100644
--- a/sw-client/src/commonMain/kotlin/org/luxons/sevenwonders/client/SevenWondersClient.kt
+++ b/sw-client/src/commonMain/kotlin/org/luxons/sevenwonders/client/SevenWondersClient.kt
@@ -122,10 +122,8 @@ class SevenWondersSession(private val stompSession: StompSessionWithKxSerializat
     suspend fun watchLobbyUpdates(): Flow<LobbyDTO> =
         stompSession.subscribe("/user/queue/lobby/updated", LobbyDTO.serializer())
 
-    suspend fun awaitGameStart(gameId: Long): PlayerTurnInfo {
-        val startEvents = stompSession.subscribe("/user/queue/lobby/$gameId/started", PlayerTurnInfo.serializer())
-        return startEvents.first()
-    }
+    suspend fun watchGameStarted(): Flow<PlayerTurnInfo> =
+        stompSession.subscribe("/user/queue/lobby/started", PlayerTurnInfo.serializer())
 
     suspend fun startGame() {
         stompSession.sendEmptyMsg("/app/lobby/startGame")
diff --git a/sw-server/src/main/kotlin/org/luxons/sevenwonders/server/controllers/LobbyController.kt b/sw-server/src/main/kotlin/org/luxons/sevenwonders/server/controllers/LobbyController.kt
index 34dfe4e7..79b63bc6 100644
--- a/sw-server/src/main/kotlin/org/luxons/sevenwonders/server/controllers/LobbyController.kt
+++ b/sw-server/src/main/kotlin/org/luxons/sevenwonders/server/controllers/LobbyController.kt
@@ -173,7 +173,7 @@ class LobbyController(
 
         currentTurnInfo.forEach {
             val player = lobby.getPlayers()[it.playerIndex]
-            template.convertAndSendToUser(player.username, "/queue/lobby/" + lobby.id + "/started", it)
+            template.convertAndSendToUser(player.username, "/queue/lobby/started", it)
         }
     }
 
diff --git a/sw-server/src/test/kotlin/org/luxons/sevenwonders/server/SevenWondersTest.kt b/sw-server/src/test/kotlin/org/luxons/sevenwonders/server/SevenWondersTest.kt
index 920e51d5..02f43fcf 100644
--- a/sw-server/src/test/kotlin/org/luxons/sevenwonders/server/SevenWondersTest.kt
+++ b/sw-server/src/test/kotlin/org/luxons/sevenwonders/server/SevenWondersTest.kt
@@ -1,8 +1,11 @@
 package org.luxons.sevenwonders.server
 
-import kotlinx.coroutines.*
+import kotlinx.coroutines.FlowPreview
 import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.flow.produceIn
+import kotlinx.coroutines.launch
+import kotlinx.coroutines.withTimeout
+import kotlinx.coroutines.withTimeoutOrNull
 import org.junit.runner.RunWith
 import org.luxons.sevenwonders.client.SevenWondersClient
 import org.luxons.sevenwonders.client.SevenWondersSession
@@ -42,7 +45,6 @@ class SevenWondersTest {
         val session = connectNewClient()
         val playerName = "Test User"
         val player = session.chooseName(playerName)
-        assertNotNull(player)
         assertEquals(playerName, player.displayName)
         session.disconnect()
     }
@@ -64,12 +66,12 @@ class SevenWondersTest {
         session2.joinGameAndWaitLobby(lobby.id)
 
         val outsiderSession = newPlayer("Outsider")
-        val started = launch { outsiderSession.awaitGameStart(lobby.id) }
-
+        val gameStartedEvents = outsiderSession.watchGameStarted()
         ownerSession.startGame()
-        val nothing = withTimeoutOrNull(50) { started.join() }
-        assertNull(nothing)
-        started.cancel()
+
+        val nullForTimeout = withTimeoutOrNull(50) { gameStartedEvents.first() }
+        assertNull(nullForTimeout, "outsider should not receive the game start event of this game")
+
         disconnect(ownerSession, session1, session2, outsiderSession)
     }
 
@@ -79,7 +81,6 @@ class SevenWondersTest {
 
         val gameName = "Test Game"
         val lobby = ownerSession.createGameAndWaitLobby(gameName)
-        assertNotNull(lobby)
         assertEquals(gameName, lobby.name)
 
         disconnect(ownerSession)
@@ -108,35 +109,40 @@ class SevenWondersTest {
     }
 
     @Test
-    fun startGame_3players() = runAsyncTest(30000) {
+    fun startGame_3players() = runAsyncTest {
         val session1 = newPlayer("Player1")
         val session2 = newPlayer("Player2")
 
+        val startEvents1 = session1.watchGameStarted()
         val lobby = session1.createGameAndWaitLobby("Test Game")
+
+        val startEvents2 = session2.watchGameStarted()
         session2.joinGameAndWaitLobby(lobby.id)
 
+        // player 3 connects after game creation (on purpose)
         val session3 = newPlayer("Player3")
+        val startEvents3 = session3.watchGameStarted()
         session3.joinGameAndWaitLobby(lobby.id)
 
-        listOf(session1, session2, session3).forEachIndexed { i, session ->
+        session1.startGame()
+
+        listOf(
+            session1 to startEvents1,
+            session2 to startEvents2,
+            session3 to startEvents3,
+        ).forEach { (session, startEvents) ->
             launch {
-                println("startGame_3players [launch ${i + 1}] awaiting game start...")
-                val firstTurn = session.awaitGameStart(lobby.id)
-                assertEquals(Action.SAY_READY, firstTurn.action)
-                val turns = session.watchTurns().produceIn(this)
-                println("startGame_3players [launch ${i + 1}] saying ready...")
+                val initialReadyTurn = startEvents.first()
+                assertEquals(Action.SAY_READY, initialReadyTurn.action)
+                assertNull(initialReadyTurn.hand)
+                val turns = session.watchTurns()
                 session.sayReady()
-                println("startGame_3players [launch ${i + 1}] ready, receiving first turn...")
-                val turn = turns.receive()
-                assertNotNull(turn)
-                println("startGame_3players [launch ${i + 1}] turn OK, disconnecting...")
+
+                val firstActualTurn = turns.first()
+                assertNotNull(firstActualTurn.hand)
                 session.disconnect()
             }
         }
-        println("startGame_3players: player 1 starting the game...")
-        delay(50) // ensure awaitGameStart actually subscribed in all sessions
-        session1.startGame()
-        println("startGame_3players: end of test method (main body)")
     }
 }
 
diff --git a/sw-ui/src/main/kotlin/org/luxons/sevenwonders/ui/redux/sagas/RouteBasedSagas.kt b/sw-ui/src/main/kotlin/org/luxons/sevenwonders/ui/redux/sagas/RouteBasedSagas.kt
index d4329f2f..88ecdcc1 100644
--- a/sw-ui/src/main/kotlin/org/luxons/sevenwonders/ui/redux/sagas/RouteBasedSagas.kt
+++ b/sw-ui/src/main/kotlin/org/luxons/sevenwonders/ui/redux/sagas/RouteBasedSagas.kt
@@ -19,14 +19,7 @@ suspend fun SwSagaContext.gameBrowserSaga(session: SevenWondersSession) {
 }
 
 suspend fun SwSagaContext.lobbySaga(session: SevenWondersSession) {
-    val lobby = getState().currentLobby ?: error("Lobby saga run without a current lobby")
-    coroutineScope {
-        session.watchLobbyUpdates().map { UpdateLobbyAction(it) }.dispatchAllIn(this)
-
-        val turnInfo = session.awaitGameStart(lobby.id)
-        dispatch(EnterGameAction(getState().currentLobby!!, turnInfo))
-        dispatch(Navigate(Route.GAME))
-    }
+    session.watchLobbyUpdates().map { UpdateLobbyAction(it) }.dispatchAll()
 }
 
 suspend fun SwSagaContext.gameSaga(session: SevenWondersSession) {
diff --git a/sw-ui/src/main/kotlin/org/luxons/sevenwonders/ui/redux/sagas/Sagas.kt b/sw-ui/src/main/kotlin/org/luxons/sevenwonders/ui/redux/sagas/Sagas.kt
index c9f73111..7be6f65a 100644
--- a/sw-ui/src/main/kotlin/org/luxons/sevenwonders/ui/redux/sagas/Sagas.kt
+++ b/sw-ui/src/main/kotlin/org/luxons/sevenwonders/ui/redux/sagas/Sagas.kt
@@ -31,7 +31,7 @@ suspend fun SwSagaContext.rootSaga() = try {
         }
 
         launchApiActionHandlersIn(this, session)
-        launchNavigationHandlers(this, session)
+        launchApiEventHandlersIn(this, session)
 
         val player = session.chooseName(action.playerName, null)
         dispatch(SetCurrentPlayerAction(player))
@@ -94,7 +94,14 @@ private fun SwSagaContext.launchApiActionHandlersIn(scope: CoroutineScope, sessi
     scope.launchOnEach<RequestUnprepareMove> { session.unprepareMove() }
 }
 
-private fun SwSagaContext.launchNavigationHandlers(scope: CoroutineScope, session: SevenWondersSession) {
+private fun SwSagaContext.launchApiEventHandlersIn(scope: CoroutineScope, session: SevenWondersSession) {
+
+    scope.launch {
+        session.watchLobbyJoined().collect { lobby ->
+            dispatch(EnterLobbyAction(lobby))
+            dispatch(Navigate(Route.LOBBY))
+        }
+    }
 
     scope.launch {
         session.watchLobbyLeft().collect { leftLobbyId ->
@@ -103,16 +110,17 @@ private fun SwSagaContext.launchNavigationHandlers(scope: CoroutineScope, sessio
         }
     }
 
+    scope.launch {
+        session.watchGameStarted().collect { turnInfo ->
+            val currentLobby = getState().currentLobby ?: error("Received game started event without being in a lobby")
+            dispatch(EnterGameAction(currentLobby, turnInfo))
+            dispatch(Navigate(Route.GAME))
+        }
+    }
+
     // FIXME map this actions like others and await server event instead
     scope.launchOnEach<RequestLeaveGame> {
         session.leaveGame()
         dispatch(Navigate(Route.GAME_BROWSER))
     }
-
-    scope.launch {
-        session.watchLobbyJoined().collect { lobby ->
-            dispatch(EnterLobbyAction(lobby))
-            dispatch(Navigate(Route.LOBBY))
-        }
-    }
 }