From 27db31c976c977cf8dde3bbcff84515c36299b90 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 | 2 ++ src/mudlib/player.py | 22 ++++++++++++++++++ src/mudlib/server.py | 5 +++++ tests/test_embedded_if.py | 9 ++++++++ tests/test_gmcp.py | 47 +++++++++++++++++++++++++++++++++++++++ tests/test_paint_mode.py | 7 ++++++ 6 files changed, 92 insertions(+) diff --git a/src/mudlib/gmcp.py b/src/mudlib/gmcp.py index 1bf22f6..2e981bc 100644 --- a/src/mudlib/gmcp.py +++ b/src/mudlib/gmcp.py @@ -108,6 +108,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..0b94b44 100644 --- a/src/mudlib/server.py +++ b/src/mudlib/server.py @@ -9,6 +9,7 @@ import tomllib from typing import cast import telnetlib3 +from telnetlib3 import GMCP, MSDP, WILL from telnetlib3.server_shell import readline2 import mudlib.combat.commands @@ -238,6 +239,10 @@ 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() 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..4a78444 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,45 @@ 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 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)