diff --git a/server/gameconnection.py b/server/gameconnection.py index ed0a1321e..0a0ce2ddf 100644 --- a/server/gameconnection.py +++ b/server/gameconnection.py @@ -42,7 +42,8 @@ def __init__( protocol: Protocol, player_service: PlayerService, games: GameService, - state: GameConnectionState = GameConnectionState.INITIALIZING + state: GameConnectionState = GameConnectionState.INITIALIZING, + setup_timeout: int = 60, ): """ Construct a new GameConnection @@ -52,38 +53,35 @@ def __init__( f"{self.__class__.__qualname__}.{game.id}" ) self._db = database - self._logger.debug("GameConnection initializing") self.protocol = protocol self._state = state self.game_service = games self.player_service = player_service - self._player = player + self.player = player player.game_connection = self # Set up weak reference to self - self._game = game + self.game = game - self.finished_sim = False + self.setup_timeout = setup_timeout - @property - def state(self) -> GameConnectionState: - return self._state + self.finished_sim = False - @property - def game(self) -> Game: - return self._game + self._logger.debug("GameConnection initializing") + if self.state is GameConnectionState.INITIALIZING: + asyncio.get_event_loop().create_task( + self.timeout_game_connection(setup_timeout) + ) - @game.setter - def game(self, val: Game): - self._game = val + async def timeout_game_connection(self, timeout): + await asyncio.sleep(timeout) + if self.state is GameConnectionState.INITIALIZING: + self._logger.debug("GameConection timed out...") + await self.abort("Player took too long to start the game") @property - def player(self) -> Player: - return self._player - - @player.setter - def player(self, val: Player): - self._player = val + def state(self) -> GameConnectionState: + return self._state def is_host(self) -> bool: if not self.game or not self.player: @@ -122,6 +120,7 @@ async def _handle_idle_state(self): self.game.add_game_connection(self) self.player.state = PlayerState.HOSTING else: + self._state = GameConnectionState.INITIALIZED self.player.state = PlayerState.JOINING async def _handle_lobby_state(self): diff --git a/server/games/game.py b/server/games/game.py index fe47226ec..11ef9ac3e 100644 --- a/server/games/game.py +++ b/server/games/game.py @@ -103,6 +103,7 @@ def __init__( self.displayed_rating_range = displayed_rating_range or InclusiveRange() self.enforce_rating_range = enforce_rating_range self.matchmaker_queue_id = matchmaker_queue_id + self.setup_timeout = setup_timeout self.state = GameState.INITIALIZING self._connections = {} self._configured_player_ids: set[int] = set() diff --git a/server/ladder_service/ladder_service.py b/server/ladder_service/ladder_service.py index ee73d3061..39cda2709 100644 --- a/server/ladder_service/ladder_service.py +++ b/server/ladder_service/ladder_service.py @@ -611,8 +611,6 @@ def make_game_options(player: Player) -> GameLaunchOptions: game_id = game.id if game else None msg = {"command": "match_cancelled", "game_id": game_id} for player in all_players: - if player.state == PlayerState.STARTING_AUTOMATCH: - player.state = PlayerState.IDLE player.write_message(msg) if abandoning_players: diff --git a/server/lobbyconnection.py b/server/lobbyconnection.py index 45c0ca130..937876dbd 100644 --- a/server/lobbyconnection.py +++ b/server/lobbyconnection.py @@ -1069,6 +1069,10 @@ def _prepare_launch_game( ): assert self.player is not None assert self.game_connection is None + assert self.player.state in ( + PlayerState.IDLE, + PlayerState.STARTING_AUTOMATCH, + ) # TODO: Fix setting up a ridiculous amount of cyclic pointers here if is_host: @@ -1080,9 +1084,13 @@ def _prepare_launch_game( player=self.player, protocol=self.protocol, player_service=self.player_service, - games=self.game_service + games=self.game_service, + setup_timeout=game.setup_timeout, ) + if self.player.state is PlayerState.IDLE: + self.player.state = PlayerState.STARTING_GAME + self.player.game = game cmd = { "command": "game_launch", diff --git a/server/players.py b/server/players.py index 5f8b37129..0380d7602 100644 --- a/server/players.py +++ b/server/players.py @@ -21,6 +21,7 @@ class PlayerState(Enum): JOINING = 4 SEARCHING_LADDER = 5 STARTING_AUTOMATCH = 6 + STARTING_GAME = 7 class Player: diff --git a/tests/integration_tests/test_game.py b/tests/integration_tests/test_game.py index df3aba269..bdf63d154 100644 --- a/tests/integration_tests/test_game.py +++ b/tests/integration_tests/test_game.py @@ -632,7 +632,12 @@ async def test_double_host_message(lobby_server): "mod": "faf", "visibility": "public", }) - await read_until_command(proto, "game_launch", timeout=10) + msg = await read_until_command(proto, "notice", timeout=10) + assert msg == { + "command": "notice", + "style": "error", + "text": "Can't host a game while in state STARTING_GAME", + } @fast_forward(30) @@ -661,7 +666,12 @@ async def test_double_join_message(lobby_server): "command": "game_join", "uid": game_id }) - await read_until_command(guest_proto, "game_launch", timeout=10) + msg = await read_until_command(guest_proto, "notice", timeout=10) + assert msg == { + "command": "notice", + "style": "error", + "text": "Can't join a game while in state STARTING_GAME", + } @fast_forward(30) diff --git a/tests/integration_tests/test_matchmaker_violations.py b/tests/integration_tests/test_matchmaker_violations.py index 542df8452..ca941c834 100644 --- a/tests/integration_tests/test_matchmaker_violations.py +++ b/tests/integration_tests/test_matchmaker_violations.py @@ -120,12 +120,24 @@ async def test_violation_persisted_across_logins(mocker, lobby_server): await read_until_command(host, "match_cancelled", timeout=120) await read_until_command(guest, "match_cancelled", timeout=10) + await read_until_command(host, "game_info", timeout=10, state="closed") + + # DEPRECATED: Because the game sends a game_launch to the guest after the + # host times out, we need to simulate opening and closing the game before + # we can queue again. If we wait for the timeout, our violations expire. + await open_fa(guest) + await guest.send_message({ + "target": "game", + "command": "GameState", + "args": ["Ended"] + }) # Second time searching there is no ban await start_search(host) await start_search(guest) await read_until_command(host, "match_cancelled", timeout=120) await read_until_command(guest, "match_cancelled", timeout=10) + await read_until_command(host, "game_info", timeout=10, state="closed") # Third time searching there is a short ban await host.send_message({ @@ -177,6 +189,17 @@ async def test_violation_persisted_across_parties(mocker, lobby_server): await read_until_command(host, "match_cancelled", timeout=120) await read_until_command(guest, "match_cancelled", timeout=10) + await read_until_command(host, "game_info", timeout=10, state="closed") + + # DEPRECATED: Because the game sends a game_launch to the guest after the + # host times out, we need to simulate opening and closing the game before + # we can queue again. If we wait for the timeout, our violations expire. + await open_fa(guest) + await guest.send_message({ + "target": "game", + "command": "GameState", + "args": ["Ended"] + }) # Second time searching there is no ban await start_search(host) @@ -184,6 +207,16 @@ async def test_violation_persisted_across_parties(mocker, lobby_server): await read_until_command(host, "match_cancelled", timeout=120) await read_until_command(guest, "match_cancelled", timeout=10) + # DEPRECATED: Because the game sends a game_launch to the guest after the + # host times out, we need to simulate opening and closing the game before + # we can queue again. If we wait for the timeout, our violations expire. + await open_fa(guest) + await guest.send_message({ + "target": "game", + "command": "GameState", + "args": ["Ended"] + }) + # Third time searching there is a short ban await host.send_message({ "command": "game_matchmaking", diff --git a/tests/integration_tests/test_server.py b/tests/integration_tests/test_server.py index 107fd0676..9a5d38d14 100644 --- a/tests/integration_tests/test_server.py +++ b/tests/integration_tests/test_server.py @@ -733,6 +733,35 @@ async def test_game_host_name_non_ascii(lobby_server): } +@fast_forward(30) +async def test_game_host_then_queue(lobby_server): + player_id, session, proto = await connect_and_sign_in( + ("test", "test_password"), + lobby_server + ) + await read_until_command(proto, "game_info") + + # Send game_host but don't send GameState: Idle, then immediately queue + await proto.send_message({ + "command": "game_host", + "title": "My Game", + "mod": "faf", + "visibility": "public", + }) + await proto.send_message({ + "command": "game_matchmaking", + "state": "start", + "faction": "uef" + }) + + msg = await read_until_command(proto, "notice", timeout=10) + assert msg == { + "command": "notice", + "style": "error", + "text": "Can't join a queue while test is in state STARTING_GAME", + } + + @fast_forward(10) async def test_play_game_while_queueing(lobby_server): player_id, session, proto = await connect_and_sign_in( diff --git a/tests/unit_tests/conftest.py b/tests/unit_tests/conftest.py index 7d65cdbb7..b7f3093c9 100644 --- a/tests/unit_tests/conftest.py +++ b/tests/unit_tests/conftest.py @@ -74,7 +74,7 @@ async def violation_service(): @pytest.fixture -def game_connection( +async def game_connection( request, database, game, diff --git a/tests/unit_tests/test_gameconnection.py b/tests/unit_tests/test_gameconnection.py index 47efb5b16..ab94f2a24 100644 --- a/tests/unit_tests/test_gameconnection.py +++ b/tests/unit_tests/test_gameconnection.py @@ -62,12 +62,12 @@ async def fake_send_dc(player_id): return "OK" # Set up a peer that will disconnect without error - ok_disconnect = mock.create_autospec(GameConnection) + ok_disconnect = mock.create_autospec(game_connection) ok_disconnect.state = GameConnectionState.CONNECTED_TO_HOST ok_disconnect.send_DisconnectFromPeer = fake_send_dc # Set up a peer that will throw an exception - fail_disconnect = mock.create_autospec(GameConnection) + fail_disconnect = mock.create_autospec(game_connection) fail_disconnect.send_DisconnectFromPeer.return_value = Exception("Test exception") fail_disconnect.state = GameConnectionState.CONNECTED_TO_HOST @@ -81,7 +81,7 @@ async def fake_send_dc(player_id): async def test_connect_to_peer(game_connection): - peer = mock.create_autospec(GameConnection) + peer = mock.create_autospec(game_connection) await game_connection.connect_to_peer(peer) @@ -92,7 +92,7 @@ async def test_connect_to_peer_disconnected(game_connection): # Weak reference has dissapeared await game_connection.connect_to_peer(None) - peer = mock.create_autospec(GameConnection) + peer = mock.create_autospec(game_connection) peer.send_ConnectToPeer.side_effect = DisconnectedError("Test error") # The client disconnects right as we send the message diff --git a/tests/unit_tests/test_lobbyconnection.py b/tests/unit_tests/test_lobbyconnection.py index c0bd7e70a..6085992db 100644 --- a/tests/unit_tests/test_lobbyconnection.py +++ b/tests/unit_tests/test_lobbyconnection.py @@ -264,7 +264,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.IDLE + assert lobbyconnection.player.state == PlayerState.STARTING_GAME lobbyconnection.send.assert_called_once() @@ -791,7 +791,6 @@ async def test_game_connection_not_restored_if_game_state_prohibits( game_service: GameService, game_stats_service, game_state, - mocker, database ): del lobbyconnection.player.game_connection @@ -998,8 +997,9 @@ async def test_command_matchmaker_info( }) -async def test_connection_lost(lobbyconnection): +async def test_connection_lost(lobbyconnection, players): lobbyconnection.game_connection = mock.create_autospec(GameConnection) + lobbyconnection.game_connection.player = players.hosting await lobbyconnection.on_connection_lost() lobbyconnection.game_connection.on_connection_lost.assert_called_once()