Issue/#786 send player update on disconnect (#832)

* Add player state to player_info message and send update on disconnect

* Assert state is not None

* Only use offline state

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:

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}, "

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"]
+            ))
+        )
     )
 
 

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")