From 606d75b29dd6312542d074294185efde0f392229 Mon Sep 17 00:00:00 2001 From: Askaholic Date: Mon, 13 Dec 2021 21:42:00 -0900 Subject: [PATCH] Refactor launch_game so it can be called without awaiting --- server/ladder_service.py | 24 +++++----- server/lobbyconnection.py | 50 +++++++++++++++----- tests/unit_tests/test_ladder_service.py | 62 ++++++++++++++++++------- 3 files changed, 98 insertions(+), 38 deletions(-) diff --git a/server/ladder_service.py b/server/ladder_service.py index 4f38344a7..35cf7421e 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: diff --git a/server/lobbyconnection.py b/server/lobbyconnection.py index ad8fcd89c..344c2e266 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/unit_tests/test_ladder_service.py b/tests/unit_tests/test_ladder_service.py index ab69c6a05..485f1750a 100644 --- a/tests/unit_tests/test_ladder_service.py +++ b/tests/unit_tests/test_ladder_service.py @@ -143,18 +143,14 @@ async def test_start_game_1v1( monkeypatch.setattr(LadderGame, "wait_hosted", CoroutineMock()) monkeypatch.setattr(LadderGame, "wait_launched", CoroutineMock()) monkeypatch.setattr(LadderGame, "timeout_game", CoroutineMock()) - for player in (player1, player2): - player.lobby_connection.launch_game = CoroutineMock( - 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 @@ -215,14 +211,52 @@ async def test_start_game_timeout( "command": "match_cancelled", "game_id": 41956 }) - assert p1.lobby_connection.launch_game.called + 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", CoroutineMock()) + monkeypatch.setattr(LadderGame, "on_game_end", CoroutineMock()) + + 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"), @@ -245,10 +279,6 @@ async def test_start_game_with_teams( monkeypatch.setattr(LadderGame, "wait_hosted", CoroutineMock()) monkeypatch.setattr(LadderGame, "wait_launched", CoroutineMock()) monkeypatch.setattr(LadderGame, "timeout_game", CoroutineMock()) - for player in (player1, player2, player3, player4): - player.lobby_connection.launch_game = CoroutineMock( - spec=LobbyConnection.launch_game - ) await ladder_service.start_game( [player1, player3], @@ -258,10 +288,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