From ee0dc839d8f96eb3e874f5a8f19237cf83fa7010 Mon Sep 17 00:00:00 2001 From: Jared Miller Date: Wed, 11 Feb 2026 23:35:25 -0500 Subject: [PATCH] Offer GMCP/MSDP during connection and guard tick sends The server never proactively offered GMCP or MSDP to clients, so telnetlib3 logged "cannot send MSDP without negotiation" every second. Now the server sends WILL GMCP and WILL MSDP on connection, and send_msdp_vitals checks negotiation state before attempting to send. --- src/mudlib/gmcp.py | 10 +++++++ src/mudlib/player.py | 22 ++++++++++++++ src/mudlib/server.py | 19 ++++++++++++- tests/test_embedded_if.py | 9 ++++++ tests/test_gmcp.py | 60 +++++++++++++++++++++++++++++++++++++++ tests/test_paint_mode.py | 7 +++++ 6 files changed, 126 insertions(+), 1 deletion(-) diff --git a/src/mudlib/gmcp.py b/src/mudlib/gmcp.py index 1bf22f6..4654d30 100644 --- a/src/mudlib/gmcp.py +++ b/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 @@ -108,6 +116,8 @@ def send_map_data(player: Player) -> None: def send_msdp_vitals(player: Player) -> None: """Send MSDP variable updates for real-time gauges.""" + if not player.msdp_enabled: + return player.send_msdp( { "HEALTH": str(round(player.pl, 1)), diff --git a/src/mudlib/player.py b/src/mudlib/player.py index bcf5003..558e0b0 100644 --- a/src/mudlib/player.py +++ b/src/mudlib/player.py @@ -5,6 +5,8 @@ from __future__ import annotations from dataclasses import dataclass, field from typing import TYPE_CHECKING, Any +from telnetlib3 import GMCP, MSDP + from mudlib.caps import ClientCaps from mudlib.entity import Entity @@ -54,6 +56,26 @@ class Player(Entity): if self.writer is not None: self.writer.send_msdp(variables) + @property + def gmcp_enabled(self) -> bool: + """Whether this client has GMCP negotiated.""" + if self.writer is None: + return False + return bool( + self.writer.local_option.enabled(GMCP) + or self.writer.remote_option.enabled(GMCP) + ) + + @property + def msdp_enabled(self) -> bool: + """Whether this client has MSDP negotiated.""" + if self.writer is None: + return False + return bool( + self.writer.local_option.enabled(MSDP) + or self.writer.remote_option.enabled(MSDP) + ) + # Global registry of connected players players: dict[str, Player] = {} diff --git a/src/mudlib/server.py b/src/mudlib/server.py index 5c16329..64503d0 100644 --- a/src/mudlib/server.py +++ b/src/mudlib/server.py @@ -9,6 +9,8 @@ import tomllib 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 @@ -224,6 +226,17 @@ async def handle_login( return {"success": False, "player_data": None} +class MudTelnetServer(TelnetServer): + """Telnet server that offers GMCP and MSDP during negotiation.""" + + def begin_advanced_negotiation(self) -> None: + """Offer GMCP and MSDP alongside the standard options.""" + super().begin_advanced_negotiation() + assert self.writer is not None + self.writer.iac(WILL, GMCP) + self.writer.iac(WILL, MSDP) + + async def shell( reader: telnetlib3.TelnetReader | telnetlib3.TelnetReaderUnicode, writer: telnetlib3.TelnetWriter | telnetlib3.TelnetWriterUnicode, @@ -527,7 +540,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) diff --git a/tests/test_embedded_if.py b/tests/test_embedded_if.py index caea6f9..315bae4 100644 --- a/tests/test_embedded_if.py +++ b/tests/test_embedded_if.py @@ -17,6 +17,15 @@ requires_zork = pytest.mark.skipif(not ZORK_PATH.exists(), reason="zork1.z3 not @dataclass class MockWriter: + def __post_init__(self): + # Mock option negotiation state + from unittest.mock import MagicMock + + self.local_option = MagicMock() + self.local_option.enabled = MagicMock(return_value=False) + self.remote_option = MagicMock() + self.remote_option.enabled = MagicMock(return_value=False) + def write(self, data): pass diff --git a/tests/test_gmcp.py b/tests/test_gmcp.py index f163300..fa5aafb 100644 --- a/tests/test_gmcp.py +++ b/tests/test_gmcp.py @@ -47,6 +47,11 @@ def mock_writer(): writer.drain = AsyncMock() writer.send_gmcp = MagicMock() writer.send_msdp = MagicMock() + # Option negotiation state (for gmcp_enabled/msdp_enabled checks) + writer.local_option = MagicMock() + writer.local_option.enabled = MagicMock(return_value=True) + writer.remote_option = MagicMock() + writer.remote_option.enabled = MagicMock(return_value=True) return writer @@ -502,3 +507,58 @@ async def test_char_status_sent_on_rest_complete(player): ] assert len(status_calls) == 1 assert status_calls[0][0][1]["resting"] is False + + +def test_msdp_vitals_skipped_when_not_negotiated(player): + """Test send_msdp_vitals skips when MSDP is not negotiated.""" + player.writer.local_option.enabled.return_value = False + player.writer.remote_option.enabled.return_value = False + + send_msdp_vitals(player) + + player.writer.send_msdp.assert_not_called() + + +def test_msdp_enabled_property(player): + """Test msdp_enabled reflects negotiation state.""" + player.writer.local_option.enabled.return_value = False + player.writer.remote_option.enabled.return_value = False + assert not player.msdp_enabled + + player.writer.local_option.enabled.return_value = True + assert player.msdp_enabled + + +def test_gmcp_enabled_property(player): + """Test gmcp_enabled reflects negotiation state.""" + player.writer.local_option.enabled.return_value = False + player.writer.remote_option.enabled.return_value = False + assert not player.gmcp_enabled + + player.writer.remote_option.enabled.return_value = True + assert player.gmcp_enabled + + +def test_msdp_enabled_no_writer(): + """Test msdp_enabled with no writer returns False.""" + p = Player(name="NoWriter", writer=None) + assert not p.msdp_enabled + + +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() diff --git a/tests/test_paint_mode.py b/tests/test_paint_mode.py index 1b7e3eb..5a3761e 100644 --- a/tests/test_paint_mode.py +++ b/tests/test_paint_mode.py @@ -35,6 +35,13 @@ class MockWriter: def __init__(self): self.messages = [] + # Mock option negotiation state + from unittest.mock import MagicMock + + self.local_option = MagicMock() + self.local_option.enabled = MagicMock(return_value=False) + self.remote_option = MagicMock() + self.remote_option.enabled = MagicMock(return_value=False) def write(self, message: str): self.messages.append(message)