diff --git a/server/ladder_service.py b/server/ladder_service.py index 33a79f032..ab620f869 100644 --- a/server/ladder_service.py +++ b/server/ladder_service.py @@ -463,17 +463,19 @@ def get_player_mean(player): game_options=game_options ) - def game_options(player: Player) -> GameLaunchOptions: + def make_game_options(player: Player) -> GameLaunchOptions: return options._replace( team=game.get_player_option(player.id, "Team"), faction=player.faction, map_position=game.get_player_option(player.id, "StartSpot") ) - await host.lobby_connection.launch_game( - game, is_host=True, options=game_options(host) - ) try: + host.lobby_connection.write_launch_game( + game, + is_host=True, + options=make_game_options(host) + ) await game.wait_hosted(60) finally: # TODO: Once the client supports `match_cancelled`, don't @@ -482,13 +484,13 @@ def game_options(player: Player) -> GameLaunchOptions: # think it is searching for ladder, even though the server has # already removed it from the queue. - await asyncio.gather(*[ - guest.lobby_connection.launch_game( - game, is_host=False, options=game_options(guest) - ) - for guest in all_guests - if guest.lobby_connection is not None - ]) + for guest in all_guests: + if guest.lobby_connection is not None: + guest.lobby_connection.write_launch_game( + game, + is_host=False, + options=make_game_options(guest) + ) await game.wait_launched(60 + 10 * len(all_guests)) self._logger.debug("Ladder game launched successfully %s", game) except Exception as e: @@ -503,7 +505,8 @@ def game_options(player: Player) -> GameLaunchOptions: if game: await game.on_game_end() - msg = {"command": "match_cancelled"} + 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 diff --git a/server/lobbyconnection.py b/server/lobbyconnection.py index f8be4a6f4..e3b5b5ea1 100644 --- a/server/lobbyconnection.py +++ b/server/lobbyconnection.py @@ -41,6 +41,7 @@ CoopGame, CustomGame, FeaturedModType, + Game, GameConnectionState, GameState, InitMode, @@ -86,11 +87,11 @@ def __init__( self.rating_service = rating_service self.oauth_service = oauth_service self._authenticated = False - self.player = None # type: Player - self.game_connection = None # type: GameConnection - self.peer_address = None # type: Optional[Address] + self.player: Optional[Player] = None + self.game_connection: Optional[GameConnection] = None + self.peer_address: Optional[Address] = None self.session = int(random.randrange(0, 4294967295)) - self.protocol: Protocol = None + self.protocol: Optional[Protocol] = None self.user_agent = None self.version = None @@ -1010,15 +1011,42 @@ async def command_match_ready(self, message): async def launch_game( self, - game, - is_host=False, - options=GameLaunchOptions(), - ): - assert self.player is not None - # TODO: Fix setting up a ridiculous amount of cyclic pointers here + game: Game, + is_host: bool = False, + options: GameLaunchOptions = GameLaunchOptions(), + ) -> None: if self.game_connection: await self.game_connection.abort("Player launched a new game") + self.game_connection = None + + await self.send(self._prepare_launch_game( + game, + is_host=is_host, + options=options + )) + + def write_launch_game( + self, + game: Game, + is_host: bool = False, + options: GameLaunchOptions = GameLaunchOptions(), + ) -> None: + self.write(self._prepare_launch_game( + game, + is_host=is_host, + options=options + )) + + def _prepare_launch_game( + self, + game: Game, + is_host: bool = False, + options: GameLaunchOptions = GameLaunchOptions(), + ): + assert self.player is not None + assert self.game_connection is None + # TODO: Fix setting up a ridiculous amount of cyclic pointers here if is_host: game.host = self.player @@ -1051,7 +1079,7 @@ async def launch_game( **options._asdict() } - await self.send({k: v for k, v in cmd.items() if v is not None}) + return {k: v for k, v in cmd.items() if v is not None} async def command_modvault(self, message): type = message["type"] diff --git a/tests/integration_tests/test_matchmaker.py b/tests/integration_tests/test_matchmaker.py index e82989442..1eb0114d2 100644 --- a/tests/integration_tests/test_matchmaker.py +++ b/tests/integration_tests/test_matchmaker.py @@ -303,7 +303,7 @@ async def test_game_matchmaking_disconnect(lobby_server): msg = await read_until_command(proto2, "match_cancelled", timeout=120) - assert msg == {"command": "match_cancelled"} + assert msg == {"command": "match_cancelled", "game_id": 41956} @fast_forward(130) diff --git a/tests/unit_tests/test_ladder_service.py b/tests/unit_tests/test_ladder_service.py index aacd690df..ad7ce472d 100644 --- a/tests/unit_tests/test_ladder_service.py +++ b/tests/unit_tests/test_ladder_service.py @@ -142,18 +142,14 @@ async def test_start_game_1v1( monkeypatch.setattr(LadderGame, "wait_hosted", mock.AsyncMock()) monkeypatch.setattr(LadderGame, "wait_launched", mock.AsyncMock()) monkeypatch.setattr(LadderGame, "timeout_game", mock.AsyncMock()) - for player in (player1, player2): - player.lobby_connection.launch_game = mock.AsyncMock( - spec=LobbyConnection.launch_game - ) await ladder_service.start_game([player1], [player2], queue) game = game_service[game_service.game_id_counter] - assert player1.lobby_connection.launch_game.called + assert player1.lobby_connection.write_launch_game.called # TODO: Once client supports `match_cancelled` change this to `assert not` - assert player2.lobby_connection.launch_game.called + assert player2.lobby_connection.write_launch_game.called assert isinstance(game, LadderGame) assert game.rating_type == queue.rating_type assert game.max_players == 2 @@ -206,16 +202,60 @@ async def test_start_game_timeout( LadderGame.timeout_game.assert_called_once() LadderGame.on_game_end.assert_called() - p1.lobby_connection.write.assert_called_once_with({"command": "match_cancelled"}) - p2.lobby_connection.write.assert_called_once_with({"command": "match_cancelled"}) - assert p1.lobby_connection.launch_game.called + p1.lobby_connection.write.assert_called_once_with({ + "command": "match_cancelled", + "game_id": 41956 + }) + p2.lobby_connection.write.assert_called_once_with({ + "command": "match_cancelled", + "game_id": 41956 + }) + assert p1.lobby_connection.write_launch_game.called # TODO: Once client supports `match_cancelled` change this to `assert not` # and uncomment the following lines. - assert p2.lobby_connection.launch_game.called + assert p2.lobby_connection.write_launch_game.called # assert p1.state is PlayerState.IDLE # assert p2.state is PlayerState.IDLE +@fast_forward(200) +async def test_start_game_timeout_on_send( + ladder_service: LadderService, + player_factory, + monkeypatch +): + queue = ladder_service.queues["ladder1v1"] + p1 = player_factory("Dostya", player_id=1, lobby_connection_spec="auto") + p2 = player_factory("Rhiza", player_id=2, lobby_connection_spec="auto") + + monkeypatch.setattr(LadderGame, "timeout_game", mock.AsyncMock()) + monkeypatch.setattr(LadderGame, "on_game_end", mock.AsyncMock()) + + async def wait_forever(*args, **kwargs): + await asyncio.sleep(1000) + # Even though launch_game isn't called by start_game, these mocks are + # important for the test in case someone refactors the code to call it. + p1.lobby_connection.launch_game.side_effect = wait_forever + p2.lobby_connection.launch_game.side_effect = wait_forever + + await asyncio.wait_for( + ladder_service.start_game([p1], [p2], queue), + timeout=150 + ) + + LadderGame.timeout_game.assert_called_once() + LadderGame.on_game_end.assert_called() + p1.lobby_connection.write.assert_called_once_with({ + "command": "match_cancelled", + "game_id": 41956 + }) + p2.lobby_connection.write.assert_called_once_with({ + "command": "match_cancelled", + "game_id": 41956 + }) + assert p1.lobby_connection.write_launch_game.called + + @given( player1=st_players("p1", player_id=1, lobby_connection_spec="mock"), player2=st_players("p2", player_id=2, lobby_connection_spec="mock"), @@ -238,10 +278,6 @@ async def test_start_game_with_teams( monkeypatch.setattr(LadderGame, "wait_hosted", mock.AsyncMock()) monkeypatch.setattr(LadderGame, "wait_launched", mock.AsyncMock()) monkeypatch.setattr(LadderGame, "timeout_game", mock.AsyncMock()) - for player in (player1, player2, player3, player4): - player.lobby_connection.launch_game = mock.AsyncMock( - spec=LobbyConnection.launch_game - ) await ladder_service.start_game( [player1, player3], @@ -251,10 +287,10 @@ async def test_start_game_with_teams( game = game_service[game_service.game_id_counter] - assert player1.lobby_connection.launch_game.called - assert player2.lobby_connection.launch_game.called - assert player3.lobby_connection.launch_game.called - assert player4.lobby_connection.launch_game.called + assert player1.lobby_connection.write_launch_game.called + assert player2.lobby_connection.write_launch_game.called + assert player3.lobby_connection.write_launch_game.called + assert player4.lobby_connection.write_launch_game.called assert isinstance(game, LadderGame) assert game.rating_type == queue.rating_type assert game.max_players == 4