Fix "Can't xxxx game while in state xxxx" messages (#872)

* Set player state only after FA is known to be opened

* Add test for trying to join twice

* Simplify test

server/gameconnection.py

@@ -114,18 +114,14 @@ async def _handle_idle_state(self):
         This message is sent by FA when it doesn't know what to do.
         """
         assert self.game
-        state = self.player.state
 
-        if state == PlayerState.HOSTING:
+        if self.player == self.game.host:
             self.game.state = GameState.LOBBY
             self._state = GameConnectionState.CONNECTED_TO_HOST
             self.game.add_game_connection(self)
-            self.game.host = self.player
-        elif state == PlayerState.JOINING:
-            return
+            self.player.state = PlayerState.HOSTING
         else:
-            self._logger.error("Unknown PlayerState: %s", state)
-            await self.abort()
+            self.player.state = PlayerState.JOINING
 
     async def _handle_lobby_state(self):
         """

server/lobbyconnection.py

@@ -1059,7 +1059,6 @@ def _prepare_launch_game(
             games=self.game_service
         )
 
-        self.player.state = PlayerState.HOSTING if is_host else PlayerState.JOINING
         self.player.game = game
         cmd = {
             "command": "game_launch",

tests/integration_tests/test_game.py

@@ -53,12 +53,22 @@ async def join_game(proto: Protocol, uid: int):
     await asyncio.sleep(0.5)
 
 
-async def client_response(proto):
-    msg = await read_until_command(proto, "game_launch", timeout=10)
+async def client_response(proto, timeout=10):
+    msg = await read_until_command(proto, "game_launch", timeout=timeout)
     await open_fa(proto)
     return msg
 
 
+async def idle_response(proto, timeout=10):
+    msg = await read_until_command(proto, "game_launch", timeout=timeout)
+    await proto.send_message({
+        "command": "GameState",
+        "target": "game",
+        "args": ["Idle"]
+    })
+    return msg
+
+
 async def open_fa(proto):
     """Simulate FA opening"""
 
@@ -409,6 +419,55 @@ async def test_game_ended_broadcasts_rating_update(lobby_server, channel):
         await asyncio.wait_for(mq_proto_all.read_message(), timeout=10)
 
 
+@fast_forward(30)
+async def test_double_host_message(lobby_server):
+    _, _, proto = await connect_and_sign_in(
+        ("test", "test_password"), lobby_server
+    )
+    await proto.send_message({
+        "command": "game_host",
+        "mod": "faf",
+        "visibility": "public",
+    })
+    await read_until_command(proto, "game_launch", timeout=10)
+
+    await proto.send_message({
+        "command": "game_host",
+        "mod": "faf",
+        "visibility": "public",
+    })
+    await read_until_command(proto, "game_launch", timeout=10)
+
+
+@fast_forward(30)
+async def test_double_join_message(lobby_server):
+    _, _, host_proto = await connect_and_sign_in(
+        ("test", "test_password"), lobby_server
+    )
+    _, _, guest_proto = await connect_and_sign_in(
+        ("Rhiza", "puff_the_magic_dragon"), lobby_server
+    )
+    await host_proto.send_message({
+        "command": "game_host",
+        "mod": "faf",
+        "visibility": "public",
+    })
+    msg = await client_response(host_proto)
+    game_id = msg["uid"]
+
+    await guest_proto.send_message({
+        "command": "game_join",
+        "uid": game_id
+    })
+    await read_until_command(guest_proto, "game_launch", timeout=10)
+
+    await guest_proto.send_message({
+        "command": "game_join",
+        "uid": game_id
+    })
+    await read_until_command(guest_proto, "game_launch", timeout=10)
+
+
 @fast_forward(100)
 async def test_game_with_foed_player(lobby_server):
     _, _, host_proto = await connect_and_sign_in(

tests/integration_tests/test_matchmaker.py

@@ -16,6 +16,7 @@
 from .conftest import connect_and_sign_in, read_until, read_until_command
 from .test_game import (
     client_response,
+    idle_response,
     open_fa,
     queue_player_for_matchmaking,
     queue_players_for_matchmaking,
@@ -169,8 +170,8 @@ async def test_game_matchmaking_timeout(lobby_server, game_service):
     proto1, proto2 = await queue_players_for_matchmaking(lobby_server)
 
     msg1, msg2 = await asyncio.gather(
-        read_until_command(proto1, "game_launch", timeout=120),
-        read_until_command(proto2, "game_launch", timeout=120)
+        idle_response(proto1, timeout=120),
+        idle_response(proto2, timeout=120)
     )
     # LEGACY BEHAVIOUR: The host does not respond with the appropriate GameState
     # so the match is cancelled. However, the client does not know how to

tests/integration_tests/test_teammatchmaker.py

@@ -12,7 +12,12 @@
 from tests.utils import fast_forward
 
 from .conftest import connect_and_sign_in, read_until, read_until_command
-from .test_game import client_response, get_player_ratings, send_player_options
+from .test_game import (
+    client_response,
+    get_player_ratings,
+    idle_response,
+    send_player_options
+)
 
 pytestmark = pytest.mark.asyncio
 
@@ -323,14 +328,18 @@ async def test_game_matchmaking_multiqueue_timeout(lobby_server):
         for proto in protos
     ])
     await read_until_command(protos[1], "search_info", state="start")
+    # tmm2v2 matches so ladder1v1 search is cancelled
     msg = await read_until_command(
         protos[0],
         "search_info",
         queue_name="ladder1v1"
     )
     assert msg["state"] == "stop"
 
-    # Don't send any GPGNet messages so the match times out
+    await client_response(protos[0])
+    await idle_response(protos[1])
+
+    # We don't send the `GameState: Lobby` command so the game should time out
     await read_until_command(protos[0], "match_cancelled", timeout=120)
 
     # Player's state is reset once they leave the game
@@ -361,6 +370,20 @@ async def test_game_matchmaking_multiqueue_timeout(lobby_server):
     with pytest.raises(asyncio.TimeoutError):
         await read_until_command(protos[1], "search_info", state="start", timeout=5)
 
+    # Player never opened FA so they can queue again
+    await protos[2].send_message({
+        "command": "game_matchmaking",
+        "state": "start",
+        "faction": "uef"
+    })
+    await read_until_command(
+        protos[2],
+        "search_info",
+        state="start",
+        queue_name="ladder1v1",
+        timeout=5
+    )
+
 
 @fast_forward(60)
 async def test_game_matchmaking_multiqueue_multimatch(lobby_server):
@@ -430,6 +453,9 @@ def other_cancelled(msg):
 async def test_game_matchmaking_timeout(lobby_server):
     protos, _ = await queue_players_for_matchmaking(lobby_server)
 
+    await client_response(protos[0])
+    await idle_response(protos[1])
+
     # We don't send the `GameState: Lobby` command so the game should time out
     await asyncio.gather(*[
         read_until_command(proto, "match_cancelled", timeout=120)
@@ -464,6 +490,20 @@ async def test_game_matchmaking_timeout(lobby_server):
     with pytest.raises(asyncio.TimeoutError):
         await read_until_command(protos[1], "search_info", state="start", timeout=5)
 
+    # Player never opened FA so they can queue again
+    await protos[2].send_message({
+        "command": "game_matchmaking",
+        "state": "start",
+        "faction": "uef"
+    })
+    await read_until_command(
+        protos[2],
+        "search_info",
+        state="start",
+        queue_name="ladder1v1",
+        timeout=5
+    )
+
 
 @fast_forward(120)
 async def test_game_ratings(lobby_server):

tests/unit_tests/test_gameconnection.py

@@ -114,7 +114,7 @@ async def test_handle_action_GameState_idle_adds_connection(
     game.add_game_connection.assert_called_with(game_connection)
 
 
-async def test_handle_action_GameState_idle_non_searching_player_aborts(
+async def test_handle_action_GameState_idle_sets_player_state(
     game_connection: GameConnection,
     players
 ):
@@ -125,7 +125,14 @@ async def test_handle_action_GameState_idle_non_searching_player_aborts(
 
     await game_connection.handle_action("GameState", ["Idle"])
 
-    game_connection.abort.assert_any_call()
+    assert players.hosting.state == PlayerState.HOSTING
+
+    game_connection.player = players.joining
+    players.joining.state = PlayerState.IDLE
+
+    await game_connection.handle_action("GameState", ["Idle"])
+
+    assert players.joining.state == PlayerState.JOINING
 
 
 async def test_handle_action_GameState_lobby_sends_HostGame(

tests/unit_tests/test_lobbyconnection.py

@@ -275,7 +275,7 @@ async def test_launch_game(lobbyconnection, game, player_factory):
     assert lobbyconnection.player.game == game
     assert lobbyconnection.player.game_connection == lobbyconnection.game_connection
     assert lobbyconnection.game_connection.player == lobbyconnection.player
-    assert lobbyconnection.player.state == PlayerState.JOINING
+    assert lobbyconnection.player.state == PlayerState.IDLE
     lobbyconnection.send.assert_called_once()