Agree MSP when sound can play (TelnetNegotiationCore 4.5.3)

CHANGELOG.md

@@ -71,6 +71,8 @@ and a release's notes are its section. The procedure is in [`docs/RELEASING.md`]
   captions*, which prints a dim `♪ …` line for each sound. Ctrl+P ▸ *Mute sound* silences everything until
   you unmute or restart, and the status row says `♪ muted` meanwhile. Worlds & Characters (F5) has a
   `sounds` field to stop a world downloading new files.
+- MSP sound. A game that offers the MUD Sound Protocol now gets the client's agreement when sound can play,
+  and its `!!SOUND(…)` and `!!MUSIC(…)` lines play like MCMP's instead of showing as text.
 - Packages. A package is a `.zip` of trigger sets, Lua scripts and sound files for one or more games.
   Ctrl+P ▸ *Install package…* (or `/package install <file or https link>`) shows what it adds first,
   including how many of its rules send commands and which scripts it runs, and writes nothing until you

CLAUDE.md

@@ -166,10 +166,10 @@ fallbacks) for inline images/maps.
     scrollback spill under the platform cache folder, not beside `config.json`.
   - **MSP triggers are read only on a connection that agreed option 90** (`MspTrigger`,
     `ITelnetSession.MspChanged`): a whole line `!!SOUND(…)`/`!!MUSIC(…)` becomes a `MediaCommand` and is taken
-    out before the triggers; anywhere else it is text a player typed. **It is inert until the library ships
-    `MSPProtocol`** (TelnetNegotiationCore PR #171): no release has it, so `TelnetSession` registers nothing,
-    the server's `WILL MSP` is refused, and `MspChanged` is never raised. The bump registers the plugin only
-    when `PlaysMedia` is true and raises the event from its two callbacks.
+    out before the triggers; anywhere else it is text a player typed. The library's `MSPProtocol` is
+    registered only when `PlaysMedia` is true (`TelnetSessionOptions.PlaysMsp`), because registering it is
+    the statement that this client plays sound; otherwise the server's `WILL MSP` is refused. Its two
+    callbacks raise `MspChanged`.
 - **Packages** (`Core/Packages`: `PackageManifest`, `PackageArchive`, `PackageStore`; Tui `PackageScreen`,
   `ListOverlay`, `SharpMUTermApp.Packages.cs`). A `.zip` with `package.json` (id, name, SemVer version,
   `games` as `host[:port]`, `*.host` or `*`), `sets/*.json`, `scripts/**.lua`, `media/**`; anything else is

Directory.Packages.props

@@ -10,7 +10,7 @@
     <PackageVersion Include="SharpConsoleUI" Version="2.7.8" />
 
     <!-- Telnet negotiation: TELOPT/NAWS/MTTS/EOR/CHARSET/MSSP/GMCP/MSDP/MCCP/MXP negotiation. -->
-    <PackageVersion Include="TelnetNegotiationCore" Version="4.5.2" />
+    <PackageVersion Include="TelnetNegotiationCore" Version="4.5.3" />
 
     <!-- The version, from the git tag (see Directory.Build.props). Build-time only. -->
     <PackageVersion Include="MinVer" Version="8.0.0" />

src/SharpMUTerm.Core/Session/WorldSession.cs

@@ -424,7 +424,8 @@ public ILogger Logger
 
     /// <summary>
     /// Whether this client will play what the server sends: set by the app when it has an audio device and game
-    /// sound is on. True adds <c>Client.Media 1</c> to the GMCP modules, which is how MCMP asks; read on connect.
+    /// sound is on. True adds <c>Client.Media 1</c> to the GMCP modules, which is how MCMP asks, and agrees MSP when
+    /// the server offers it; read on connect.
     /// </summary>
     public bool PlaysMedia { get; set; }
 
@@ -1349,6 +1350,7 @@ private ITelnetSession DefaultSessionFactory(ConnectionOptions options)
             {
                 GmcpModules = GmcpModulesFor(World, PlaysMedia),
                 AnnouncePueblo = World.ContentFormat is ContentFormat.Ansi or ContentFormat.Pueblo,
+                PlaysMsp = PlaysMedia,
                 CharsetOrder = order,
                 EncodingOverride = encodingOverride,
                 KeepaliveInterval = TelnetSessionOptions.ResolveKeepalive(World.KeepaliveSeconds),

src/SharpMUTerm.Core/Telnet/TelnetSession.cs

@@ -147,6 +147,14 @@ public static IReadOnlyDictionary<string, string> MnesIdentity(IReadOnlyList<str
     /// </summary>
     public bool AnnouncePueblo { get; init; } = true;
 
+    /// <summary>
+    /// Whether to agree MSP (option 90) when a server offers it. Registering the library's plugin is the
+    /// statement that this client plays sound, so a session with nothing to play leaves it off and the
+    /// server's <c>WILL MSP</c> is refused. Off by default; <c>WorldSession</c> sets it from
+    /// <c>PlaysMedia</c>.
+    /// </summary>
+    public bool PlaysMsp { get; init; }
+
     /// <summary>The version this client announces as a Pueblo client; PennMUSH reads only the word.</summary>
     public const string PuebloVersion = "2.50";
 
@@ -402,6 +410,7 @@ private Encoding? Negotiated
     public event EventHandler? MxpEnabled;
     public event EventHandler? MsdpEnabled;
     public event EventHandler? PuebloEnabled;
+    public event EventHandler<bool>? MspChanged;
 
     public async Task ConnectAsync(CancellationToken cancellationToken = default)
     {
@@ -476,6 +485,13 @@ private Task<TelnetInterpreter> BuildInterpreterAsync()
                 .OnMcpMessage(MooEditing.AwnsStatus, OnMcpStatusAsync)
                 .OnMcpNegotiationComplete(OnMcpAgreedAsync);
 
+        if (_options.PlaysMsp)
+        {
+            builder = builder.AddPlugin<MSPProtocol>()
+                .OnMSPEnabled(() => OnMspChangedAsync(true))
+                .OnMSPDisabled(() => OnMspChangedAsync(false));
+        }
+
         // An idle keepalive, when the world asks for one: IAC NOP after the configured silence, so a
         // NAT or load balancer doesn't evict a connection that is merely quiet. It is filler traffic,
         // not a liveness probe — it proves our write succeeded, not that the server answered.
@@ -834,6 +850,13 @@ private ValueTask OnMxpEnabledAsync()
         return ValueTask.CompletedTask;
     }
 
+    private ValueTask OnMspChangedAsync(bool active)
+    {
+        _logger.LogInformation(active ? "MSP negotiated." : "MSP withdrawn.");
+        MspChanged?.Invoke(this, active);
+        return ValueTask.CompletedTask;
+    }
+
     private async Task ReadLoopAsync(CancellationToken cancellationToken)
     {
         var buffer = new byte[_options.ReceiveBufferSize];

tests/SharpMUTerm.Core.Tests/Telnet/MspNegotiationTests.cs

@@ -0,0 +1,68 @@
+using Microsoft.Extensions.Logging.Abstractions;
+using SharpMUTerm.Core.Telnet;
+
+namespace SharpMUTerm.Core.Tests.Telnet;
+
+/// <summary>
+/// MSP (option 90) from the client's side: agreed only by a session that plays sound, and reported through
+/// <see cref="ITelnetSession.MspChanged"/> both ways, because a <c>!!SOUND</c> line is a request only while the
+/// option is agreed.
+/// </summary>
+public class MspNegotiationTests
+{
+    private const byte Iac = 255, Will = 251, Wont = 252, Do = 253, Dont = 254, Msp = 90;
+
+    private static async Task<bool> WaitForAsync(Func<bool> condition)
+    {
+        for (var i = 0; i < 100 && !condition(); i++)
+        {
+            await Task.Delay(10);
+        }
+
+        return condition();
+    }
+
+    [Test]
+    public async Task ASessionThatPlaysSoundAgreesAndReportsBothWays()
+    {
+        var transport = new ScriptedTransport { Greeting = [Iac, Will, Msp] };
+        await using var session = new TelnetSession(
+            transport, NullLogger.Instance, new TelnetSessionOptions { PlaysMsp = true });
+        var changes = new List<bool>();
+        session.MspChanged += (_, active) => { lock (changes) changes.Add(active); };
+        await session.ConnectAsync();
+
+        await Assert.That(await transport.WaitForSentAsync([Iac, Do, Msp])).IsTrue();
+        await Assert.That(await WaitForAsync(() => { lock (changes) return changes.Count == 1; })).IsTrue();
+
+        transport.SendToClient([Iac, Wont, Msp]);
+        await Assert.That(await transport.WaitForSentAsync([Iac, Dont, Msp])).IsTrue();
+        await Assert.That(await WaitForAsync(() => { lock (changes) return changes.Count == 2; })).IsTrue();
+
+        bool[] seen;
+        lock (changes)
+        {
+            seen = [.. changes];
+        }
+
+        await Assert.That(seen).IsEquivalentTo(new[] { true, false });
+    }
+
+    [Test]
+    public async Task ASessionWithNothingToPlayRefusesAndSaysNothing()
+    {
+        var transport = new ScriptedTransport { Greeting = [Iac, Will, Msp] };
+        await using var session = new TelnetSession(transport, NullLogger.Instance);
+        var raised = 0;
+        session.MspChanged += (_, _) => Interlocked.Increment(ref raised);
+        await session.ConnectAsync();
+
+        await Assert.That(await transport.WaitForSentAsync([Iac, Dont, Msp])).IsTrue();
+        await Task.Delay(50);
+        await Assert.That(Volatile.Read(ref raised)).IsEqualTo(0);
+        await Assert.That(Contains(transport.Sent, [Iac, Do, Msp])).IsFalse();
+    }
+
+    private static bool Contains(byte[] haystack, byte[] needle) =>
+        haystack.AsSpan().IndexOf(needle) >= 0;
+}