fixup! Add MSDP status bar and protocol negotiation to mud.tin

fixup! Add MSDP status bar and protocol negotiation to mud.tin

Remove manual IAC WILL event handlers that were killing the connection.
TinTin++ auto-negotiates GMCP and MSDP natively (responds DO to WILL).
The raw #send bytes were corrupting the telnet stream.

docs/how/protocols.rst

@@ -1,169 +0,0 @@
-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,7 +27,6 @@ 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,13 +8,9 @@
 #variable {MSDP_STAMINA} {-}
 #variable {MSDP_STAMINA_MAX} {-}
 
-#NOP protocol negotiation: tintin++ does NOT auto-negotiate MSDP or
-#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 MSDP: tintin++ auto-negotiates (responds DO to server's WILL).
+#NOP store variables as they arrive and refresh the status bar.
 
-#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,8 +13,6 @@ 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",
         {
@@ -28,8 +26,6 @@ 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",
         {
@@ -43,8 +39,6 @@ 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
@@ -84,8 +78,6 @@ 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
@@ -115,19 +107,14 @@ def send_map_data(player: Player) -> None:
 
 
 def send_msdp_vitals(player: Player) -> None:
-    """Send MSDP variable updates for real-time gauges.
-
-    Skips sending if values haven't changed since last send.
-    """
+    """Send MSDP variable updates for real-time gauges."""
     if not player.msdp_enabled:
         return
-    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)
+    player.send_msdp(
+        {
+            "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)),
+        }
+    )

src/mudlib/player.py

@@ -30,7 +30,6 @@ 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,7 +10,6 @@ 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
@@ -226,18 +225,6 @@ 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,
@@ -252,18 +239,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:
-            _writer.close()
-            return
-        if name_input.strip():
-            break
+    name_input = await readline2(_reader, _writer)
+    if name_input is None or not name_input.strip():
+        _writer.close()
+        return
 
     player_name = name_input.strip()
 
@@ -545,11 +532,7 @@ 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,
-        protocol_factory=MudTelnetServer,
+        host=HOST, port=PORT, shell=shell, connect_maxwait=0.5
     )
     log.info("listening on %s:%d", HOST, PORT)
 

tests/test_gmcp.py

@@ -231,21 +231,6 @@ 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"})
@@ -564,16 +549,3 @@ 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()