From c1145b8620b0a48ca30884c13453ffc04f28ff77 Mon Sep 17 00:00:00 2001 From: Askaholic Date: Wed, 3 Jan 2024 20:07:56 -0500 Subject: [PATCH] Only clear PlayerOptions when player disconnects intentionally. --- server/gameconnection.py | 18 ++++++++++++++++-- server/games/game.py | 15 +++++++++++---- server/lobbyconnection.py | 4 ---- tests/integration_tests/test_game.py | 6 ++---- tests/unit_tests/conftest.py | 1 + tests/unit_tests/test_game.py | 2 +- tests/unit_tests/test_gameconnection.py | 6 +++--- 7 files changed, 34 insertions(+), 18 deletions(-) diff --git a/server/gameconnection.py b/server/gameconnection.py index 0a0ce2ddf..958a52ac0 100644 --- a/server/gameconnection.py +++ b/server/gameconnection.py @@ -485,10 +485,10 @@ async def handle_game_state(self, state: str): ) # Signals that the FA executable has been closed elif state == "Ended": - await self.on_connection_lost() + await self.on_connection_closed() self._mark_dirty() - async def handle_game_ended(self, *args: list[Any]): + async def handle_game_ended(self, *args: list[Any]): """ Signals that the simulation has ended. """ @@ -591,7 +591,21 @@ async def disconnect_all_peers(self): exc_info=True ) + async def on_connection_closed(self): + """ + The connection is closed by the player. + """ + try: + await self.game.disconnect_player(self.player) + except Exception as e: # pragma: no cover + self._logger.exception(e) + finally: + await self.abort() + async def on_connection_lost(self): + """ + The connection is lost due to a disconnect from the lobby server. + """ try: await self.game.remove_game_connection(self) except Exception as e: # pragma: no cover diff --git a/server/games/game.py b/server/games/game.py index 11ef9ac3e..d337034b2 100644 --- a/server/games/game.py +++ b/server/games/game.py @@ -391,6 +391,17 @@ def add_game_connection(self, game_connection): self._logger.info("Added game connection %s", game_connection) self._connections[game_connection.player] = game_connection + async def disconnect_player(self, player: Player): + if player.game_connection not in self._connections.values(): + return + + self._configured_player_ids.discard(player.id) + + if self.state is GameState.LOBBY and player.id in self._player_options: + del self._player_options[player.id] + + await self.remove_game_connection(player.game_connection) + async def remove_game_connection(self, game_connection): """ Remove a game connection from this game. @@ -404,10 +415,6 @@ async def remove_game_connection(self, game_connection): player = game_connection.player del self._connections[player] del player.game - self._configured_player_ids.discard(player.id) - - if self.state is GameState.LOBBY and player.id in self._player_options: - del self._player_options[player.id] self._logger.info("Removed game connection %s", game_connection) diff --git a/server/lobbyconnection.py b/server/lobbyconnection.py index 937876dbd..e7879436f 100644 --- a/server/lobbyconnection.py +++ b/server/lobbyconnection.py @@ -787,10 +787,6 @@ async def command_restore_game_session(self, message): game.add_game_connection(self.game_connection) self.player.state = PlayerState.PLAYING self.player.game = game - # TODO: When the player drops, their player options are deleted as a - # method of tracking when people are actually connected to the host - # during the lobby phase. However, that means when they reconnect here - # they will not show up in the `game_info` message. async def command_ask_session(self, message): user_agent = message.get("user_agent") diff --git a/tests/integration_tests/test_game.py b/tests/integration_tests/test_game.py index bdf63d154..e26f60288 100644 --- a/tests/integration_tests/test_game.py +++ b/tests/integration_tests/test_game.py @@ -768,8 +768,7 @@ async def test_restore_game_session_lobby(lobby_server): msg = await read_until_command(guest_proto, "game_info", timeout=10) assert msg["teams_ids"] == [ {"team_id": 1, "player_ids": [host_id]}, - # TODO: Team 2 should have guest_id! - # {"team_id": 2, "player_ids": [guest_id]}, + {"team_id": 2, "player_ids": [guest_id]}, ] @@ -810,8 +809,7 @@ async def test_restore_game_session_live(lobby_server): msg = await read_until_command(guest_proto, "game_info", timeout=10) assert msg["teams_ids"] == [ {"team_id": 1, "player_ids": [host_id]}, - # TODO: Team 2 should have guest_id! - {"team_id": 2, "player_ids": []}, + {"team_id": 2, "player_ids": [guest_id]}, ] diff --git a/tests/unit_tests/conftest.py b/tests/unit_tests/conftest.py index b7f3093c9..733b552c7 100644 --- a/tests/unit_tests/conftest.py +++ b/tests/unit_tests/conftest.py @@ -114,6 +114,7 @@ def make_mock_game_connection( gc.state = state gc.player = player gc.finished_sim = False + player.game_connection = gc return gc diff --git a/tests/unit_tests/test_game.py b/tests/unit_tests/test_game.py index 3ed8805ae..b35f6f71e 100644 --- a/tests/unit_tests/test_game.py +++ b/tests/unit_tests/test_game.py @@ -379,7 +379,7 @@ async def test_add_game_connection_twice(game: Game, players, mock_game_connecti join_conn = add_connected_player(game, players.joining) assert game.players == [players.hosting, players.joining] # Player leaves - await game.remove_game_connection(join_conn) + await game.disconnect_player(players.joining) assert game.players == [players.hosting] # Player joins again game.add_game_connection(join_conn) diff --git a/tests/unit_tests/test_gameconnection.py b/tests/unit_tests/test_gameconnection.py index ab94f2a24..6fab54fcc 100644 --- a/tests/unit_tests/test_gameconnection.py +++ b/tests/unit_tests/test_gameconnection.py @@ -264,12 +264,12 @@ async def test_handle_action_GameState_launching_when_ended( game.launch.assert_not_called() -async def test_handle_action_GameState_ended_calls_on_connection_lost( +async def test_handle_action_GameState_ended_calls_on_connection_closed( game_connection: GameConnection ): - game_connection.on_connection_lost = mock.AsyncMock() + game_connection.on_connection_closed = mock.AsyncMock() await game_connection.handle_action("GameState", ["Ended"]) - game_connection.on_connection_lost.assert_called_once_with() + game_connection.on_connection_closed.assert_called_once_with() async def test_handle_action_PlayerOption(game: Game, game_connection: GameConnection):