diff --git a/server/player_service.py b/server/player_service.py index a31185aa6..5b53f5025 100644 --- a/server/player_service.py +++ b/server/player_service.py @@ -148,8 +148,11 @@ async def _fetch_player_ratings(self, player, conn): def remove_player(self, player: Player): if player.id in self._players: + # This signals that the player is now disconnected + del player.lobby_connection del self._players[player.id] metrics.players_online.set(len(self._players)) + self.mark_dirty(player) async def has_permission_role(self, player: Player, role_name: str) -> bool: async with self._db.acquire() as conn: diff --git a/server/players.py b/server/players.py index 13529005e..5f8b37129 100644 --- a/server/players.py +++ b/server/players.py @@ -130,37 +130,37 @@ def write_message(self, message: dict) -> None: with suppress(DisconnectedError): self.lobby_connection.write(message) - def to_dict(self): + def to_dict(self) -> dict: """ Return a dictionary representing this player object """ - - def filter_none(t): - _, v = t - return v is not None - - return dict( - filter( - filter_none, ( - ("id", self.id), - ("login", self.login), - ("avatar", self.avatar), - ("country", self.country), - ("clan", self.clan), - ("ratings", { - rating_type: { - "rating": self.ratings[rating_type], - "number_of_games": self.game_count[rating_type] - } - for rating_type in self.ratings - }), - # Deprecated - ("global_rating", self.ratings[RatingType.GLOBAL]), - ("ladder_rating", self.ratings[RatingType.LADDER_1V1]), - ("number_of_games", self.game_count[RatingType.GLOBAL]), - ) - ) - ) + assert self.state is not None and self.state.value is not None + + cmd = { + "id": self.id, + "login": self.login, + "avatar": self.avatar, + "country": self.country, + "clan": self.clan, + # NOTE: We are only sending an 'offline' state for now to signal to + # the client when a player disconnects. However, this could be + # expanded in the future to expose more of the internal state + # tracking to the client to make the UI for showing players in game + # more correct. + "state": None if self.lobby_connection else "offline", + "ratings": { + rating_type: { + "rating": self.ratings[rating_type], + "number_of_games": self.game_count[rating_type] + } + for rating_type in self.ratings + }, + # DEPRECATED: Use ratings instead + "global_rating": self.ratings[RatingType.GLOBAL], + "ladder_rating": self.ratings[RatingType.LADDER_1V1], + "number_of_games": self.game_count[RatingType.GLOBAL], + } + return {k: v for k, v in cmd.items() if v is not None} def __str__(self) -> str: return (f"Player({self.login}, {self.id}, " diff --git a/tests/integration_tests/test_server.py b/tests/integration_tests/test_server.py index 6c8f7ba01..7cb4fabf6 100644 --- a/tests/integration_tests/test_server.py +++ b/tests/integration_tests/test_server.py @@ -341,7 +341,7 @@ async def test_drain( assert "Graceful shutdown period ended! 1 games are still live!" in caplog.messages -@fast_forward(5) +@fast_forward(15) async def test_player_info_broadcast(lobby_server): p1 = await connect_client(lobby_server) p2 = await connect_client(lobby_server) @@ -350,8 +350,23 @@ async def test_player_info_broadcast(lobby_server): await perform_login(p2, ("Rhiza", "puff_the_magic_dragon")) await read_until( - p2, lambda m: "player_info" in m.values() - and any(map(lambda d: d["login"] == "test", m["players"])) + p2, + lambda m: ( + m["command"] == "player_info" + and any(map(lambda d: d["login"] == "test", m["players"])) + ) + ) + + await p1.close() + await read_until( + p2, + lambda m: ( + m["command"] == "player_info" + and any(map( + lambda d: d["login"] == "test" and d.get("state") == "offline", + m["players"] + )) + ) ) diff --git a/tests/unit_tests/test_players.py b/tests/unit_tests/test_players.py index d03ad99c4..8768b51fb 100644 --- a/tests/unit_tests/test_players.py +++ b/tests/unit_tests/test_players.py @@ -91,6 +91,7 @@ def test_serialize(): "id": 42, "login": "Something", "clan": "TOAST", + "state": "offline", "ratings": { "global": { "rating": (1234, 68), @@ -107,6 +108,15 @@ def test_serialize(): } +def test_serialize_state(): + conn = mock.Mock() + p = Player(lobby_connection=conn) + assert "state" not in p.to_dict() + + del p.lobby_connection + assert p.to_dict()["state"] == "offline" + + async def test_send_message(): p = Player("Test")