Skip to content

Commit

Permalink
Only clear PlayerOptions when player disconnects intentionally.
Browse files Browse the repository at this point in the history
  • Loading branch information
Askaholic committed Jan 4, 2024
1 parent 335b3fc commit 2633b9b
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 18 deletions.
18 changes: 16 additions & 2 deletions server/gameconnection.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
"""
Expand Down Expand Up @@ -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
Expand Down
15 changes: 11 additions & 4 deletions server/games/game.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)

Expand Down
4 changes: 0 additions & 4 deletions server/lobbyconnection.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
6 changes: 2 additions & 4 deletions tests/integration_tests/test_game.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]},
]


Expand Down Expand Up @@ -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]},
]


Expand Down
1 change: 1 addition & 0 deletions tests/unit_tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
2 changes: 1 addition & 1 deletion tests/unit_tests/test_game.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions tests/unit_tests/test_gameconnection.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down

0 comments on commit 2633b9b

Please sign in to comment.