Dedupe MSDP vitals to avoid spamming idle clients

Cache last-sent values on Player and skip send_msdp() when
nothing changed. Idle players no longer get a packet every second.

docs/how/protocols.rst

@@ -0,0 +1,169 @@
+protocol negotiation — GMCP, MSDP, and client detection
+========================================================
+
+how the server offers GMCP and MSDP to connecting clients, how clients
+accept or reject them, and how the game guards against sending data to
+clients that don't support it.
+
+see also: ``mud.tin`` (tintin++ client config), ``docs/lessons/charset-vs-mtts.txt``
+(related telnet negotiation gotcha).
+
+
+the lifecycle
+-------------
+
+1. client connects via telnet
+2. telnetlib3 calls ``begin_negotiation()`` on our ``MudTelnetServer``
+3. server sends ``IAC WILL GMCP`` and ``IAC WILL MSDP``
+4. client replies ``IAC DO <option>`` (accept) or ``IAC DONT <option>`` (reject)
+5. telnetlib3 updates its internal option state tables
+6. ``Player.gmcp_enabled`` / ``Player.msdp_enabled`` check those tables
+7. every send function guards on the relevant property before transmitting
+
+all of this happens during the telnet handshake, before the player sees the
+name prompt. by the time game code runs, the protocol state is settled.
+
+
+server side — MudTelnetServer
+-----------------------------
+
+``server.py`` subclasses telnetlib3's ``TelnetServer``::
+
+    class MudTelnetServer(TelnetServer):
+        def begin_negotiation(self) -> None:
+            super().begin_negotiation()
+            self.writer.iac(WILL, GMCP)
+            self.writer.iac(WILL, MSDP)
+
+``begin_negotiation()`` is called once by telnetlib3 right after the TCP
+connection is established. the parent handles baseline telnet options (TTYPE,
+NAWS, etc). we add GMCP and MSDP offers on top.
+
+there's no ``begin_advanced_negotiation()`` override — GMCP/MSDP offers go
+in the initial phase because clients need to know about them before the game
+starts sending data.
+
+
+player properties
+-----------------
+
+``player.py`` exposes two boolean properties::
+
+    @property
+    def gmcp_enabled(self) -> bool:
+        if self.writer is None:
+            return False
+        return bool(
+            self.writer.local_option.enabled(GMCP)
+            or self.writer.remote_option.enabled(GMCP)
+        )
+
+checks both ``local_option`` and ``remote_option`` because telnet negotiation
+is symmetric — either side can initiate. ``local_option`` reflects our ``WILL``
+offer being accepted (client sent ``DO``). ``remote_option`` would reflect a
+client-initiated ``DO`` that we accepted with ``WILL``. in practice the server
+always offers first, but checking both is correct per the telnet spec.
+
+same pattern for ``msdp_enabled``.
+
+
+the guard pattern
+-----------------
+
+every function in ``gmcp.py`` that sends data checks the property first::
+
+    def send_char_vitals(player: Player) -> None:
+        if not player.gmcp_enabled:
+            return
+        player.send_gmcp("Char.Vitals", { ... })
+
+    def send_msdp_vitals(player: Player) -> None:
+        if not player.msdp_enabled:
+            return
+        player.send_msdp("HEALTH", ...)
+
+this is the only guard needed. the game loop calls these functions
+unconditionally for all players; the functions themselves decide whether to
+actually send. simple clients that don't negotiate GMCP/MSDP just never
+receive the data.
+
+
+what gets sent over each protocol
+---------------------------------
+
+**GMCP** (Generic MUD Communication Protocol) — structured JSON packages:
+
+- ``Char.Vitals`` — hp, max_hp, stamina, max_stamina
+- ``Char.Status`` — name, level, location name
+- ``Room.Info`` — room id, coordinates, exits
+- ``Room.Map`` — ASCII map fragment for client-side rendering
+
+sent on-demand when state changes (room enter, damage taken, etc).
+
+**MSDP** (MUD Server Data Protocol) — key/value variable updates:
+
+- ``HEALTH``, ``HEALTH_MAX``, ``STAMINA``, ``STAMINA_MAX``
+
+sent on a timer, once per second (every ``TICK_RATE`` ticks in the game loop).
+intended for status bar gauges.
+
+
+client side — tintin++ example
+------------------------------
+
+tintin++ does NOT auto-negotiate GMCP or MSDP. it sends ``IAC DONT`` by
+default, rejecting the server's offer. the ``CATCH`` event intercepts the
+incoming ``IAC WILL`` before tintin's default handler runs::
+
+    #EVENT {CATCH IAC WILL GMCP} {#send {\xFF\xFD\xC9\}}
+    #EVENT {CATCH IAC WILL MSDP} {#send {\xFF\xFD\x45\}}
+
+the hex bytes:
+
+- ``\xFF\xFD\xC9`` = ``IAC DO GMCP`` (255, 253, 201)
+- ``\xFF\xFD\x45`` = ``IAC DO MSDP`` (255, 253, 69)
+
+MSDP variables arrive as subnegotiation events and get stored in tintin
+variables for the status bar::
+
+    #EVENT {IAC SB MSDP} {
+        #variable {MSDP_%0} {%1};
+        #showme {[HP: $MSDP_HEALTH/$MSDP_HEALTH_MAX] ...} {-2}
+    }
+
+``{-2}`` puts the status bar on the split line at the bottom of the terminal.
+
+this was the trickiest part of the integration — the CATCH mechanism is
+not well-documented in tintin++ and the raw byte sending syntax changed
+across versions.
+
+
+the client command
+------------------
+
+players can type ``client`` in-game to see what got negotiated::
+
+    client protocols
+      GMCP: active
+      MSDP: active
+    terminal
+      type: xterm-256color
+      size: 120x40
+      color: 256
+      ...
+
+useful for debugging connection issues — if a player reports missing status
+bars, ``client`` shows whether MSDP actually negotiated.
+
+
+adding a protocol
+-----------------
+
+to add support for another telnet option:
+
+1. import the option byte constant (or define it)
+2. add ``self.writer.iac(WILL, NEW_OPTION)`` in ``MudTelnetServer.begin_negotiation()``
+3. add a ``new_option_enabled`` property on ``Player``
+4. guard all send functions on that property
+5. add it to the ``client`` command output
+6. add CATCH handler in ``mud.tin`` if tintin++ doesn't auto-negotiate it

docs/index.rst

@@ -27,6 +27,7 @@ mostly standalone subsystems.
 - ``docs/how/terrain-generation.txt`` — perlin noise, elevation thresholds, river tracing
 - ``docs/how/persistence.txt`` — SQLite storage, what's persisted vs runtime
 - ``docs/how/prompt-system.txt`` — modal prompts, color markup, per-player customization
+- ``docs/how/protocols.rst`` — GMCP/MSDP negotiation, client detection, guard pattern
 
 combat
 ------

mud.tin

@@ -8,9 +8,13 @@
 #variable {MSDP_STAMINA} {-}
 #variable {MSDP_STAMINA_MAX} {-}
 
-#NOP MSDP: tintin++ auto-negotiates (responds DO to server's WILL).
+#NOP protocol negotiation: tintin++ does NOT auto-negotiate MSDP or
-#NOP store variables as they arrive and refresh the status bar.
+#NOP GMCP. CATCH intercepts before the default DONT response, and
+#NOP #send handles hex escapes natively (no raw option needed).
+#EVENT {CATCH IAC WILL GMCP} {#send {\xFF\xFD\xC9\}}
+#EVENT {CATCH IAC WILL MSDP} {#send {\xFF\xFD\x45\}}
 
+#NOP store incoming MSDP variables and refresh status bar
 #EVENT {IAC SB MSDP} {
     #variable {MSDP_%0} {%1};
     #showme {[HP: $MSDP_HEALTH/$MSDP_HEALTH_MAX] [ST: $MSDP_STAMINA/$MSDP_STAMINA_MAX]} {-2}

src/mudlib/gmcp.py

@@ -13,6 +13,8 @@ log = logging.getLogger(__name__)
 
 def send_char_vitals(player: Player) -> None:
     """Send Char.Vitals — pl, max_pl, stamina, max_stamina."""
+    if not player.gmcp_enabled:
+        return
     player.send_gmcp(
         "Char.Vitals",
         {
@@ -26,6 +28,8 @@ def send_char_vitals(player: Player) -> None:
 
 def send_char_status(player: Player) -> None:
     """Send Char.Status — flying, resting, mode, in_combat."""
+    if not player.gmcp_enabled:
+        return
     player.send_gmcp(
         "Char.Status",
         {
@@ -39,6 +43,8 @@ def send_char_status(player: Player) -> None:
 
 def send_room_info(player: Player) -> None:
     """Send Room.Info — zone, coordinates, terrain, exits."""
+    if not player.gmcp_enabled:
+        return
     from mudlib.zone import Zone
 
     zone = player.location
@@ -78,6 +84,8 @@ def send_room_info(player: Player) -> None:
 
 def send_map_data(player: Player) -> None:
     """Send Room.Map — terrain viewport around player for client rendering."""
+    if not player.gmcp_enabled:
+        return
     from mudlib.zone import Zone
 
     zone = player.location
@@ -107,14 +115,19 @@ def send_map_data(player: Player) -> None:
 
 
 def send_msdp_vitals(player: Player) -> None:
-    """Send MSDP variable updates for real-time gauges."""
+    """Send MSDP variable updates for real-time gauges.
+
+    Skips sending if values haven't changed since last send.
+    """
     if not player.msdp_enabled:
         return
-    player.send_msdp(
+    data = {
-        {
         "HEALTH": str(round(player.pl, 1)),
         "HEALTH_MAX": str(round(player.max_pl, 1)),
         "STAMINA": str(round(player.stamina, 1)),
         "STAMINA_MAX": str(round(player.max_stamina, 1)),
     }
-    )
+    if data == player._last_msdp:
+        return
+    player._last_msdp = data
+    player.send_msdp(data)

src/mudlib/player.py

@@ -30,6 +30,7 @@ class Player(Entity):
     paint_mode: bool = False
     painting: bool = False
     paint_brush: str = "."
+    _last_msdp: dict = field(default_factory=dict, repr=False)
 
     @property
     def mode(self) -> str:

src/mudlib/server.py

@@ -10,6 +10,7 @@ from typing import cast
 
 import telnetlib3
 from telnetlib3 import GMCP, MSDP, WILL
+from telnetlib3.server import TelnetServer
 from telnetlib3.server_shell import readline2
 
 import mudlib.combat.commands
@@ -225,6 +226,18 @@ async def handle_login(
             return {"success": False, "player_data": None}
 
 
+class MudTelnetServer(TelnetServer):
+    """Telnet server that offers GMCP and MSDP during negotiation."""
+
+    def begin_negotiation(self) -> None:
+        """Offer GMCP and MSDP as part of initial negotiation."""
+        super().begin_negotiation()
+        assert self.writer is not None
+        gmcp_ok = self.writer.iac(WILL, GMCP)
+        msdp_ok = self.writer.iac(WILL, MSDP)
+        log.debug("offered GMCP=%s MSDP=%s", gmcp_ok, msdp_ok)
+
+
 async def shell(
     reader: telnetlib3.TelnetReader | telnetlib3.TelnetReaderUnicode,
     writer: telnetlib3.TelnetWriter | telnetlib3.TelnetWriterUnicode,
@@ -239,18 +252,18 @@ async def shell(
 
     log.debug("new connection from %s", _writer.get_extra_info("peername"))
 
-    # Offer GMCP and MSDP so clients can negotiate
-    _writer.iac(WILL, GMCP)
-    _writer.iac(WILL, MSDP)
-
     _writer.write("Welcome to the MUD!\r\n")
     _writer.write("What is your name? ")
     await _writer.drain()
 
+    # Skip empty lines from client negotiation bytes, only close on actual disconnect
+    while True:
         name_input = await readline2(_reader, _writer)
-    if name_input is None or not name_input.strip():
+        if name_input is None:
             _writer.close()
             return
+        if name_input.strip():
+            break
 
     player_name = name_input.strip()
 
@@ -532,7 +545,11 @@ async def run_server() -> None:
     # MUD clients like tintin++ reject CHARSET immediately via MTTS, but
     # telnetlib3 still waits for the full timeout. 0.5s is plenty.
     server = await telnetlib3.create_server(
-        host=HOST, port=PORT, shell=shell, connect_maxwait=0.5
+        host=HOST,
+        port=PORT,
+        shell=shell,
+        connect_maxwait=0.5,
+        protocol_factory=MudTelnetServer,
     )
     log.info("listening on %s:%d", HOST, PORT)
 

tests/test_gmcp.py

@@ -231,6 +231,21 @@ def test_send_msdp_vitals(player):
     )
 
 
+def test_send_msdp_vitals_dedup(player):
+    """Test MSDP skips sending when values haven't changed."""
+    send_msdp_vitals(player)
+    assert player.writer.send_msdp.call_count == 1
+
+    # second call with same values — should be suppressed
+    send_msdp_vitals(player)
+    assert player.writer.send_msdp.call_count == 1
+
+    # change a value — should send again
+    player.pl = 80.0
+    send_msdp_vitals(player)
+    assert player.writer.send_msdp.call_count == 2
+
+
 def test_player_send_gmcp(player):
     """Test Player.send_gmcp convenience method."""
     player.send_gmcp("Test.Package", {"key": "value"})
@@ -549,3 +564,16 @@ def test_gmcp_enabled_no_writer():
     """Test gmcp_enabled with no writer returns False."""
     p = Player(name="NoWriter", writer=None)
     assert not p.gmcp_enabled
+
+
+def test_gmcp_sends_skipped_when_not_negotiated(player):
+    """Test all GMCP send functions skip when GMCP is not negotiated."""
+    player.writer.local_option.enabled.return_value = False
+    player.writer.remote_option.enabled.return_value = False
+
+    send_char_vitals(player)
+    send_char_status(player)
+    send_room_info(player)
+    send_map_data(player)
+
+    player.writer.send_gmcp.assert_not_called()