Negotiate Pueblo and decode its ANSI colour

CHANGELOG.md

@@ -37,6 +37,9 @@ and a release's notes are its section. The procedure is in [`docs/RELEASING.md`]
 
 ### Fixed
 
+- Pueblo worlds work. The client answers a server's Pueblo greeting and switches to Pueblo markup when the
+  server does, and Pueblo worlds now show their ANSI colours instead of the escape codes. A world set to
+  `mxp` or `ansi` on F5 does not answer the greeting.
 - A character's on-disconnect commands are now sent, before the client closes the connection itself
   (Disconnect, Reconnect or quitting). They were saved and never used.
 - Lua scripts now run. A trigger set's script files are loaded from the `scripts` folder beside

CLAUDE.md

@@ -1743,8 +1743,8 @@ markup (`[bold #rrggbb on #rrggbb]…[/]`, `[[`/`]]` escaping, `[link=url]…[/]
     is parser state too: MXP has to be negotiated again on the new socket to take the session off ANSI.
     Left alone, a server's `ESC[6z` made the *next* connection start in secure default, and the black-on-black
     above survived the reconnect that is the obvious way a user tries to get rid of it.
-  - **`AnsiParser` and `MxpParser` hold near-duplicate escape state machines, deliberately
-    unconsolidated — so a fix to one has to be applied to the other, with its own test in each suite.**
+  - **`AnsiParser`, `MxpParser` and `PuebloParser` hold near-duplicate escape state machines, deliberately
+    unconsolidated — so a fix to one has to be applied to the others, with its own test in each suite.**
     (Same states, same CSI ranges, same OSC/DCS/ST handling; `MxpParser`'s differs only by the `z`
     line-tag arm and by disarming TEMP SECURE on the way out.) This was weighed and kept: extracting a
     shared scanner is a structural refactor, and it was not going into a fix wave that already carried
@@ -1785,12 +1785,21 @@ markup (`[bold #rrggbb on #rrggbb]…[/]`, `[[`/`]]` escaping, `[link=url]…[/]
   written while the mode never reverted in production, where "lands on an OPEN line" was not true of a
   session a server had ever secured. Closing the buffering hole itself needs its own change at the
   telnet layer and is out of this scope.
-- **`PuebloParser` decodes no ANSI, and nothing else decodes it for it.** Its header used to say escapes
-  were "handled upstream" — the same false premise `MxpParser` carried, in the same words, and there is
-  no upstream either way: a session gets one parser. So a Pueblo world's colour arrives on screen as a
-  literal `<ESC>[0;33m`. The behaviour clause was accurate; only the justification was invented. The fix
-  is the one `MxpParser` already had — its own escape state machine over `SgrCodes` — and until someone
-  does it this is a known gap rather than a design.
+- **`PuebloParser` decodes ANSI itself, through `SgrCodes`**, for `MxpParser`'s reason: a session gets one
+  parser and nothing chains another in front of it. Until it did, a Pueblo world's colour arrived as a
+  literal `<ESC>[0;33m`. Its escape machine is the third copy (see the paragraph on the duplicates above).
+- **Pueblo is negotiated the way MXP is** (`PuebloProtocol`, TelnetNegotiationCore; `TelnetSession`'s
+  `OnPuebloOffered`/`OnPuebloEnabled`; `WorldSession.UpgradeParser`). Pueblo has no telnet option: the server
+  prints `This world is Pueblo 1.10 Enhanced.`, the client answers with the text line `PUEBLOCLIENT 2.50`,
+  and the server's `</xch_mudtext><img xch_mode=purehtml…>` switches the connection. Three rules:
+  - **The answer goes only to a server that said hello, and only from `auto` or `pueblo`**
+    (`TelnetSessionOptions.AnnouncePueblo`). It is a line at a login prompt, where a server without Pueblo
+    reads it as a name — the unsolicited-negotiation rule, in text.
+  - **The first markup a connection negotiates keeps it.** The parser is swapped only off the plain
+    `AnsiParser`, and the hello is not answered once MXP is on, so a server offering both is not flipped
+    between them by the order the handshakes finish in.
+  - **The library consumes both lines; `_pending` is cleared in both callbacks**, because `CallbackOnByteAsync`
+    has already collected their bytes and a prompt boundary would otherwise flush them as text.
 - **`<VERSION>`/`<SUPPORT>` are answered, and the `SUPPORTS` list is held to the same rule as the MTTS
   bit vector: an honest claim, not an aspirational one.** The spec requires both replies to go out as a
   secure-tagged line (`ESC[1z` prefix, `MxpParser.SecureLinePrefix`) specifically so a server can tell

src/SharpMUTerm.Core/Protocols/PuebloParser.cs

@@ -22,26 +22,29 @@ namespace SharpMUTerm.Core.Protocols;
 /// deciding whether a world is Pueblo-enabled is a session concern handled elsewhere.
 ///
 /// <para>
-/// <b>ANSI/VT escape sequences are not decoded, and nothing else decodes them either.</b> An ESC
-/// (0x1b) byte is passed through untouched into the span text — so a Pueblo world's colour arrives on
-/// screen as a literal <c>&lt;ESC&gt;[0;33m</c>. This used to claim the escapes were "handled
-/// upstream"; there is no upstream. <c>WorldSession.CreateParser</c> hands a session <i>one</i> parser
-/// for its content format, never a chain, which is the same false premise
-/// <see cref="MxpParser"/> carried until it grew its own escape state machine (through
-/// <see cref="SharpMUTerm.Core.Text.SgrCodes"/>). Doing the same here is the fix; it is a known gap
-/// rather than a design, and the behaviour clause above is what actually happens today.
+/// <b>ANSI is decoded here, through <see cref="SgrCodes"/></b>, because a session has one parser and nothing
+/// chains another in front of it (<see cref="MxpParser"/>'s reason, in the same words). SGR changes the style;
+/// every other CSI, OSC, DCS/SOS/PM/APC string and two-byte escape is consumed. The escape state machine is a
+/// near-copy of <see cref="MxpParser"/>'s and <see cref="AnsiParser"/>'s, deliberately not shared — a fix to
+/// one belongs in all three, with its own test in each suite.
 /// </para>
 /// </summary>
 public sealed class PuebloParser : ILineParser
 {
     private const int MaxTagLength = 4096;
     private const int MaxEntityLength = 32;
+    private const int MaxSequenceLength = 128;
 
     private enum State
     {
         Ground,
         Tag,
         Entity,
+        Escape,
+        EscapeIntermediate,
+        Csi,
+        Osc,
+        OscEscape,
     }
 
     /// <summary>A saved rendition frame, pushed when an element opens and restored when it closes.</summary>
@@ -68,6 +71,7 @@ public Frame(string name, TextStyle style, SpanInteraction? interaction)
     private readonly List<StyledSpan> _lineSpans = new();
     private readonly StringBuilder _tag = new();
     private readonly StringBuilder _entity = new();
+    private readonly StringBuilder _seq = new();
     private readonly List<Frame> _stack = new();
 
     /// <summary>The rendition state that will apply to the next printed character.</summary>
@@ -102,6 +106,13 @@ public IReadOnlyList<StyledLine> Feed(ReadOnlySpan<char> text)
     /// </summary>
     public StyledLine? Flush()
     {
+        // The telnet layer strips the terminator, so this is the line boundary in production, and an
+        // escape cannot span it — see Process.
+        if (IsEscapeState(_state))
+        {
+            EndSequence();
+        }
+
         FlushRun();
         CloseInteractionsAtBoundary();
         if (_lineSpans.Count == 0)
@@ -124,11 +135,19 @@ public void Reset()
         _lineSpans.Clear();
         _tag.Clear();
         _entity.Clear();
+        _seq.Clear();
         _stack.Clear();
     }
 
     private void Process(char ch, ref List<StyledLine>? lines)
     {
+        // A newline ends the line whatever sequence is in flight, or an unterminated ESC ] that a player
+        // typed would swallow every following line. MxpParser and AnsiParser carry the same rule.
+        if (ch == '\n' && IsEscapeState(_state))
+        {
+            EndSequence();
+        }
+
         switch (_state)
         {
             case State.Ground:
@@ -142,9 +161,135 @@ private void Process(char ch, ref List<StyledLine>? lines)
             case State.Entity:
                 ProcessEntity(ch, ref lines);
                 break;
+
+            case State.Escape:
+                ProcessEscape(ch);
+                break;
+
+            case State.EscapeIntermediate:
+                // The single trailing byte of e.g. ESC ( B.
+                EndSequence();
+                break;
+
+            case State.Csi:
+                ProcessCsi(ch);
+                break;
+
+            case State.Osc:
+                ProcessOsc(ch);
+                break;
+
+            case State.OscEscape:
+                // ESC inside a string sequence; a following '\' is the ST terminator.
+                if (ch == '\\')
+                {
+                    EndSequence();
+                }
+                else
+                {
+                    _state = State.Osc;
+                }
+
+                break;
         }
     }
 
+    private static bool IsEscapeState(State state) =>
+        state is State.Escape or State.EscapeIntermediate or State.Csi or State.Osc or State.OscEscape;
+
+    private void ProcessEscape(char ch)
+    {
+        switch (ch)
+        {
+            case '[':
+                _seq.Clear();
+                _state = State.Csi;
+                break;
+
+            case ']':
+            case 'P': // DCS
+            case 'X': // SOS
+            case '^': // PM
+            case '_': // APC
+                // String sequences, terminated by BEL or ST — consumed so the payload never shows.
+                _seq.Clear();
+                _state = State.Osc;
+                break;
+
+            case '(':
+            case ')':
+            case '*':
+            case '+':
+            case '#':
+            case '%':
+                _state = State.EscapeIntermediate;
+                break;
+
+            default:
+                // A two-byte escape (ESC c, ESC M, …), consumed.
+                EndSequence();
+                break;
+        }
+    }
+
+    private void ProcessCsi(char ch)
+    {
+        if (ch is >= '\x40' and <= '\x7e')
+        {
+            if (ch == 'm')
+            {
+                // Text so far keeps the style it was written in.
+                FlushRun();
+                _current = SgrCodes.Apply(_current, _seq.ToString());
+            }
+
+            // Every other final byte — cursor movement, erase — means nothing to a line-oriented view.
+            EndSequence();
+            return;
+        }
+
+        if (ch is >= '\x20' and <= '\x3f')
+        {
+            if (_seq.Length < MaxSequenceLength)
+            {
+                _seq.Append(ch);
+            }
+
+            return;
+        }
+
+        // A control character inside the sequence aborts it as malformed.
+        EndSequence();
+    }
+
+    private void ProcessOsc(char ch)
+    {
+        switch (ch)
+        {
+            case '\x07':
+                EndSequence();
+                break;
+
+            case '\x1b':
+                _state = State.OscEscape;
+                break;
+
+            default:
+                if (_seq.Length < MaxSequenceLength)
+                {
+                    _seq.Append(ch);
+                }
+
+                break;
+        }
+    }
+
+    private void EndSequence()
+    {
+        _seq.Clear();
+        _state = State.Ground;
+    }
+
     private void ProcessGround(char ch, ref List<StyledLine>? lines)
     {
         switch (ch)
@@ -167,6 +312,11 @@ private void ProcessGround(char ch, ref List<StyledLine>? lines)
                 // Carriage returns are dropped; scrollback is line-based.
                 break;
 
+            case '\x1b':
+                _seq.Clear();
+                _state = State.Escape;
+                break;
+
             default:
                 _run.Append(ch);
                 break;

src/SharpMUTerm.Core/Session/WorldSession.cs

@@ -262,9 +262,23 @@ public async Task<bool> SendGmcpAsync(string package, string json)
         return true;
     }
 
-    private void OnMxpEnabled(object? sender, EventArgs e)
+    private void OnMxpEnabled(object? sender, EventArgs e) => UpgradeParser(ContentFormat.Mxp);
+
+    /// <summary>
+    /// Upgrades to the Pueblo parser when the server switches the connection to Pueblo — the same rule as
+    /// MXP's, above: only from the automatic default.
+    /// </summary>
+    private void OnPuebloEnabled(object? sender, EventArgs e) => UpgradeParser(ContentFormat.Pueblo);
+
+    /// <summary>
+    /// The one parser swap. Only from <see cref="ContentFormat.Ansi"/> and only off the plain ANSI parser, so
+    /// <b>the first markup a connection negotiates keeps it</b>: a server that offers both MXP and Pueblo is
+    /// not switched back and forth by the order its handshakes finish in. (The telnet layer also declines to
+    /// answer a Pueblo hello once MXP is on.)
+    /// </summary>
+    private void UpgradeParser(ContentFormat format)
     {
-        if (World.ContentFormat != ContentFormat.Ansi || _parser is MxpParser)
+        if (World.ContentFormat != ContentFormat.Ansi || _parser is not AnsiParser)
         {
             return;
         }
@@ -274,7 +288,7 @@ private void OnMxpEnabled(object? sender, EventArgs e)
             ProcessOutputLine(tail);
         }
 
-        _parser = NewParser(ContentFormat.Mxp);
+        _parser = NewParser(format);
     }
 
     /// <summary>
@@ -456,6 +470,7 @@ public async Task ConnectAsync(CancellationToken cancellationToken = default)
         telnet.EncodingChanged += OnEncodingChanged;
         telnet.Disconnected += OnDisconnected;
         telnet.MxpEnabled += OnMxpEnabled;
+        telnet.PuebloEnabled += OnPuebloEnabled;
 
         try
         {
@@ -980,6 +995,7 @@ private ITelnetSession DefaultSessionFactory(ConnectionOptions options)
             options: new TelnetSessionOptions
             {
                 GmcpModules = World.GmcpModules.ToArray(),
+                AnnouncePueblo = World.ContentFormat is ContentFormat.Ansi or ContentFormat.Pueblo,
                 CharsetOrder = order,
                 EncodingOverride = encodingOverride,
                 KeepaliveInterval = TelnetSessionOptions.ResolveKeepalive(World.KeepaliveSeconds),

src/SharpMUTerm.Core/Telnet/ITelnetSession.cs

@@ -39,6 +39,12 @@ public interface ITelnetSession : IAsyncDisposable
     /// </summary>
     event EventHandler? MxpEnabled;
 
+    /// <summary>
+    /// Raised once the server has switched this connection to Pueblo (its start sequence, after our
+    /// <c>PUEBLOCLIENT</c>). The <see cref="MxpEnabled"/> of Pueblo.
+    /// </summary>
+    event EventHandler? PuebloEnabled;
+
     /// <summary>
     /// Raised once the client has agreed to the server's MSDP, so a consumer can ask it to <c>REPORT</c> the
     /// variables it wants. MSDP servers commonly send nothing until asked.