feat: Add zero-alloc interpolation handler to ValueStringBuilder, replace all StringBuilder usage (#2387)

## Summary - **Add a self-referencing `InterpolationHandler` to `ValueStringBuilder`** that writes directly into the builder's buffer — zero intermediate allocation, works with `stackalloc`-backed builders - **Replace all `System.Text.StringBuilder` usage** across the codebase with `ValueStringBuilder` - **Convert `ValueStringBuilder.Create()` to `stackalloc`** at 10 sites where output length is provably bounded - **Convert manual `Dispose()` to `using var`** where possible, and hoist loop-scoped builders outside loops with `Reset()` - **Convert verbose `Append()` chains to `Append($"...")`** interpolation for readability - **Add comprehensive documentation** for string handling patterns ## InterpolationHandler Design `ValueStringBuilder` is a `ref struct`, which creates challenges for C#'s interpolated string handler pattern: - **`ref` fields to ref structs are not allowed** (CS9050) - **`[InterpolatedStringHandlerArgument("")]` passes struct receivers by value**, not by ref - **`ISelfInterpolatedStringHandler` requires boxing** ref structs into interface fields **Solution: Copy-and-reconcile pattern.** The handler receives a value copy of the builder. The copy shares the same underlying `char` buffer (`Span` points to the same `stackalloc`/pooled memory), so writes go to the original buffer. `Append()` reconciles by `this = handler._builder`, updating `_length` and any buffer references changed by `Grow()`. This is safe because: - The game loop is single-threaded — no concurrent access between handler construction and reconciliation - If `Grow()` occurs in the copy, the original's stale buffer isn't accessed until `Append()` replaces it - `Dispose()` correctly returns the reconciled buffer to the pool ## Changes by Category ### ValueStringBuilder (`Projects/Server/Buffers/ValueStringBuilder.cs`) - Added nested `InterpolationHandler` ref struct with copy-and-reconcile pattern - Added `Append([InterpolatedStringHandlerArgument("")] scoped ref InterpolationHandler)` method - Removed `RawInterpolatedStringHandler` overloads (new handler replaces them) - All `AppendFormatted` overloads delegate to existing `Append` methods (no code duplication) - Alignment support via direct private field access (nested type privilege) ### StringBuilder → ValueStringBuilder (15 files) Replaced all `new StringBuilder()` with `ValueStringBuilder.Create()` or `stackalloc`: - ConPVP games: KingOfTheHill, DoubleDom, CTF, BombingRun, TourneyMatch - ConPVP infrastructure: Tournament, Participant, TourneyParticipant - ConPVP gumps: ArenaGump, TournamentBracketGump, AcceptTeamGump, ConfirmSignupGump - Commands: Handlers, Logging, Add - Other: TownCrier, SpeechLogGump, TestCenter Key patterns: - `sb = new StringBuilder()` reassignment → `sb.Reset()` - `sb.AppendFormat("{0:N0}", value)` → `sb.Append($"{value:N0}")` - `sb.Append(x).Append(y)` chains → separate statements (VSB returns void) ### Create() → stackalloc (10 files) Converted heap-allocated builders to stackalloc where output is bounded: - ClientVersion (32), MapSelection (160), HouseRaffleStone (48) - HolySense (96), UnholySense (96), ClientVerification (192) - AcceptTeamGump (64), ConfirmSignupGump (64) - BaseWeapon (160), BaseArmor (128) ### Loop optimizations (2 files) Hoisted `ValueStringBuilder` creation outside loops with `Reset()` per iteration: - TourneyMatch.cs: `using var` inside for loop → stackalloc before loop - ArenaGump.cs: `Create()` + `Dispose()` per iteration → stackalloc before loop ### Append chain → interpolation (5 files) Converted multi-line `Append()` chains to `Append($"...")`: - BountyMessage.cs: title switch (6 cases), paragraph (15→1 Append), description lines, closing - AcceptTeamGump, ConfirmSignupGump, TournamentBracketGump: tournament type strings - AdminGump: comment/tag formatting in loops ### Documentation - `dev-docs/string-handling.md`: Full reference — construction, interpolation, disposal, decision guide - `dev-docs/claude-skills/modernuo-string-handling.md`: Claude skill with quick reference - `CLAUDE.md`: Added rule 17 (no StringBuilder), dev-docs table entry, skills table entry - `dev-docs/code-standards.md`: Updated memory management section ## Test Plan - [x] `dotnet build` — 0 errors, 0 warnings - [x] `dotnet test` — 940/940 tests pass - [x] 28 ValueStringBuilder tests covering all reconciliation scenarios: - Stackalloc no-grow, stackalloc with grow (→pool transition) - Heap no-grow, heap with grow, heap double grow - Pre-existing content with and without grow - Sequential multiple `Append($"...")` calls - Mixed plain + interpolated Append - Empty interpolation, literal-only, format specifiers - Null string holes, ISpanFormattable types - Dispose after stackalloc→pool grow
2026-03-22 14:23:44 -07:00 · 2026-03-22 14:23:44 -07:00 · 61e41df00c
commit 61e41df00c
parent 9f39198fab
38 changed files with 846 additions and 200 deletions
--- a/dev-docs/claude-skills/modernuo-string-handling.md
+++ b/dev-docs/claude-skills/modernuo-string-handling.md
@ -0,0 +1,133 @@
+# ModernUO String Handling Skill
+
+## When This Skill Applies
+- Any code that builds strings dynamically (concatenation, formatting, interpolation)
+- Converting `System.Text.StringBuilder` to `ValueStringBuilder`
+- Packet string construction
+- Gump/message text building
+
+## Core Rule
+**Never use `System.Text.StringBuilder`**. Use `Server.Text.ValueStringBuilder` everywhere.
+
+## Quick Reference
+
+### Construction
+```csharp
+// Bounded output (preferred): zero heap allocation
+using var sb = new ValueStringBuilder(stackalloc char[128]);
+
+// Unbounded output: rents from STArrayPool
+using var sb = ValueStringBuilder.Create(256);
+using var sb = ValueStringBuilder.Create(); // default 64 chars
+```
+
+### String Interpolation
+Works with stackalloc — writes directly into the builder's buffer:
+```csharp
+using var sb = new ValueStringBuilder(stackalloc char[64]);
+sb.Append($"Player {name} has {kills} kills");
+```
+
+### Reuse via Reset
+Use `Reset()` instead of creating a new builder:
+```csharp
+using var sb = new ValueStringBuilder(stackalloc char[128]);
+foreach (var item in items)
+{
+    sb.Reset();
+    sb.Append($"{item.Name}: {item.Value}");
+    Process(sb.ToString());
+}
+```
+
+### Reading Results
+- `sb.ToString()` — when you need a string (allocates)
+- `sb.AsSpan()` — when consumer accepts `ReadOnlySpan<char>` (zero-alloc)
+
+## Common Mistakes
+
+### 1. Using StringBuilder
+```csharp
+// BAD
+var sb = new StringBuilder();
+sb.Append(name);
+return sb.ToString();
+
+// GOOD
+using var sb = new ValueStringBuilder(stackalloc char[64]);
+sb.Append(name);
+return sb.ToString();
+```
+
+### 2. Forgetting `using var`
+```csharp
+// BAD: pooled array may leak if Grow() happened
+var sb = ValueStringBuilder.Create();
+return sb.ToString(); // never disposed!
+
+// GOOD
+using var sb = ValueStringBuilder.Create();
+return sb.ToString();
+```
+
+### 3. Chaining Append calls
+```csharp
+// BAD: VSB Append returns void, not this
+sb.Append("a").Append("b");
+
+// GOOD
+sb.Append("a");
+sb.Append("b");
+
+// BETTER: use interpolation
+sb.Append($"a{value}b");
+```
+
+### 4. Reassigning a using variable
+```csharp
+// BAD: can't reassign using var
+using var sb = ValueStringBuilder.Create();
+sb = ValueStringBuilder.Create(); // CS1656!
+
+// GOOD: use Reset()
+using var sb = ValueStringBuilder.Create();
+sb.Reset();
+```
+
+### 5. `using var` with `ref` extension methods
+```csharp
+// BAD: CS1657 — using var can't be passed by ref
+using var sb = new ValueStringBuilder(stackalloc char[64]);
+sb.AppendSpaceWithArticle(text, articleAn); // takes ref VSB
+
+// GOOD: manual Dispose
+var sb = new ValueStringBuilder(stackalloc char[64]);
+sb.AppendSpaceWithArticle(text, articleAn);
+var result = sb.ToString();
+sb.Dispose();
+```
+
+### 6. No AppendFormat — use `$"..."` interpolation
+```csharp
+// BAD: AppendFormat doesn't exist on VSB (no object[] params equivalent)
+sb.AppendFormat("{0:N0} points, {1:N0} kills", score, kills);
+
+// GOOD: use interpolation with format specifiers (zero boxing, zero intermediate strings)
+sb.Append($"{score:N0} points, {kills:N0} kills");
+```
+
+## Capacity Sizing Guide
+
+| Content | Recommended |
+|---|---|
+| Version strings, coordinates | `stackalloc char[32-48]` |
+| Player names, short messages | `stackalloc char[64]` |
+| Item descriptions, titles | `stackalloc char[128]` |
+| Paragraph text, HTML snippets | `stackalloc char[256]` |
+| Large HTML, gump content | `Create(512)` or `Create()` |
+| Unbounded (logs, file paths) | `Create()` |
+
+## Related Docs
+- `dev-docs/string-handling.md` — full reference
+- `dev-docs/code-standards.md` — memory management rules
+- `dev-docs/property-lists.md` — IPropertyList string interpolation (different handler)
--- a/dev-docs/code-standards.md
+++ b/dev-docs/code-standards.md
@ -486,7 +486,7 @@ finally
 - Use `PooledRefList<T>` instead of `new List<T>()`
 - Use `stackalloc` for small fixed-size buffers
 - Use `STArrayPool<T>` for larger buffers
- Avoid string concatenation in loops (use `StringBuilder` or string interpolation in `IPropertyList`)
+- Use `ValueStringBuilder` with `stackalloc` for string building (never `System.Text.StringBuilder`) → `dev-docs/string-handling.md`

 ---

--- a/dev-docs/string-handling.md
+++ b/dev-docs/string-handling.md
@ -0,0 +1,220 @@
+# String Handling in ModernUO
+
+This document covers the string building utilities in `Projects/Server/Text/` and `Projects/Server/Buffers/`, when to use each, and how to avoid common allocation pitfalls.
+
+## Table of Contents
+1. [ValueStringBuilder](#valuestringbuilder)
+2. [RawInterpolatedStringHandler](#rawinterpolatedstringhandler)
+3. [StringHelpers](#stringhelpers)
+4. [TextEncoding](#textencoding)
+5. [Decision Guide](#decision-guide)
+
+---
+
+## ValueStringBuilder
+
+**Location**: `Projects/Server/Buffers/ValueStringBuilder.cs`
+**Namespace**: `Server.Text`
+
+A `ref struct` string builder that avoids heap allocations entirely when backed by `stackalloc`. This is the **preferred** string builder for all ModernUO code — do not use `System.Text.StringBuilder`.
+
+### Construction Patterns
+
+**Stackalloc (preferred for bounded output)**:
+```csharp
+// Best: zero heap allocation, zero pool overhead
+using var sb = new ValueStringBuilder(stackalloc char[128]);
+sb.Append($"Hello {name}, score: {score}");
+return sb.ToString();
+```
+
+**Pooled (for unbounded or large output)**:
+```csharp
+// Rents from STArrayPool — returned on Dispose
+using var sb = ValueStringBuilder.Create(256);
+// or with default capacity (64):
+using var sb = ValueStringBuilder.Create();
+```
+
+### Choosing Capacity
+
+| Output size | Pattern |
+|---|---|
+| Known, <=256 chars | `new ValueStringBuilder(stackalloc char[N])` |
+| Known, >256 chars | `ValueStringBuilder.Create(N)` |
+| Unbounded/unknown | `ValueStringBuilder.Create()` (grows automatically) |
+
+If the stackalloc buffer is too small, the builder automatically grows to a pooled array. This is safe but costs a pool rent — size the stackalloc to fit the expected output.
+
+### String Interpolation (`$"..."`)
+
+`ValueStringBuilder` supports `$"..."` syntax via a copy-and-reconcile `InterpolationHandler`. This writes directly into the builder's buffer — **no intermediate allocation**, even with stackalloc.
+
+```csharp
+// Works with stackalloc — zero allocation
+using var sb = new ValueStringBuilder(stackalloc char[64]);
+sb.Append($"Player {name} has {kills} kills");
+sb.Append($" and {bounty} gold bounty");
+```
+
+**How it works**: The compiler passes a value copy of the builder to the handler. The copy shares the same underlying buffer (Span points to the same memory), so writes go to the original buffer. `Append()` reconciles by copying the handler's updated state back to the original. If the handler triggers a `Grow()`, the reconciliation updates the buffer reference.
+
+### Reusing a Builder
+
+Use `Reset()` to clear the builder for reuse instead of creating a new one:
+
+```csharp
+using var sb = new ValueStringBuilder(stackalloc char[128]);
+
+foreach (var item in items)
+{
+    sb.Reset(); // clear for next iteration
+    sb.Append($"{item.Name}: {item.Value}");
+    Process(sb.ToString());
+}
+```
+
+### Reading the Result
+
+| Method | Use when |
+|---|---|
+| `sb.ToString()` | You need a `string` (allocates) |
+| `sb.AsSpan()` | You can consume a `ReadOnlySpan<char>` (zero-alloc) |
+| `sb.AsSpan(terminate: true)` | You need a null-terminated span |
+
+### Disposal
+
+Always use `using var` for automatic disposal:
+```csharp
+using var sb = new ValueStringBuilder(stackalloc char[64]);
+```
+
+If `using var` is not possible (e.g., the builder is passed by `ref` to extension methods, or `goto case` in switch blocks), use manual `Dispose()`:
+```csharp
+var sb = new ValueStringBuilder(stackalloc char[64]);
+sb.AppendSpaceWithArticle(text, articleAn); // takes ref ValueStringBuilder
+var result = sb.ToString();
+sb.Dispose();
+```
+
+For stackalloc-only builders that never grow, `Dispose()` is a no-op. But always call it defensively — if a future change triggers growth, the pooled array needs returning.
+
+### Limitations
+
+- **No `AppendFormat`**: Use `$"..."` interpolation instead — it's more readable and zero-allocation:
+  ```csharp
+  // StringBuilder (old):
+  sb.AppendFormat("{0:N0} points, {1:N0} kills", score, kills);
+
+  // ValueStringBuilder:
+  sb.Append($"{score:N0} points, {kills:N0} kills");
+  ```
+  There is no `object[] params` equivalent for format strings. All formatting goes through `$"..."` interpolation which uses `ISpanFormattable.TryFormat` directly — zero boxing, zero intermediate strings.
+
+- **No chained Append**: `Append()` returns `void`, not `this`. Write `sb.Append(a); sb.Append(b);` instead of `sb.Append(a).Append(b)`.
+- **Ref struct constraints**: Cannot be stored in fields, captured by lambdas, or used in `async` methods. Scoped to the declaring method.
+- **`using var` + `ref` conflict**: A `using` variable cannot be passed by `ref`. If extension methods take `ref ValueStringBuilder`, use manual `Dispose()` instead.
+
+---
+
+## RawInterpolatedStringHandler
+
+**Location**: `Projects/Server/Buffers/RawInterpolatedStringHandler.cs`
+**Namespace**: `Server.Buffers`
+
+A `[InterpolatedStringHandler]` ref struct used internally by `ValueStringBuilder`'s interpolation support. You should not need to use this directly — use `sb.Append($"...")` instead.
+
+---
+
+## StringHelpers
+
+**Location**: `Projects/Server/Text/StringHelpers.cs`
+**Namespace**: `Server.Text`
+
+Extension methods for common string operations:
+
+| Method | Description |
+|---|---|
+| `Wrap(string, int perLine, int maxLines)` | Word-wrap text into lines |
+| `AppendSpaceWithArticle(ref ValueStringBuilder, string, bool)` | Append with "a"/"an" article prefix |
+| `Remove(ReadOnlySpan<char>, ...)` | Filter substrings from spans |
+| `Capitalize(string)` | Title-case with "the" handling |
+| `TrimMultiline(string)` | Trim each line in multiline text |
+
+---
+
+## TextEncoding
+
+**Location**: `Projects/Server/Text/TextEncoding.cs`
+**Namespace**: `Server.Text`
+
+UTF-8/Unicode encoding utilities used by the networking layer:
+
+| Method | Description |
+|---|---|
+| `GetBytesUtf8(string, Span<byte>)` | Encode string to UTF-8 in buffer |
+| `GetBytesUtf8(ReadOnlySpan<char>, Span<byte>)` | Encode char span to UTF-8 |
+| `GetStringUtf8(Span<byte>)` | Decode UTF-8 bytes to string (with filtering) |
+
+---
+
+## Decision Guide
+
+### When to use what
+
+```
+Need to build a string?
+├── In a hot path (packets, ticks, spatial queries)?
+│   └── ValueStringBuilder with stackalloc
+├── In game content (gumps, messages, commands)?
+│   └── ValueStringBuilder with stackalloc (bounded) or Create() (unbounded)
+├── Building an ObjectPropertyList tooltip?
+│   └── Use IPropertyList.Add($"...") — has its own handler
+├── In async/multi-threaded code (rare)?
+│   └── ValueStringBuilder.CreateMT() or System.Text.StringBuilder
+└── Never → System.Text.StringBuilder
+```
+
+### Do NOT use `System.Text.StringBuilder`
+
+`ValueStringBuilder` replaces `StringBuilder` in all game code. It avoids:
+- GC pressure from `StringBuilder`'s internal `char[]` allocations
+- The `StringBuilder` object allocation itself (24+ bytes on heap)
+- Thread-safe overhead in `ArrayPool<char>.Shared` (VSB uses lock-free `STArrayPool`)
+
+### Common patterns
+
+**Instead of string concatenation**:
+```csharp
+// BAD: allocates intermediate strings
+var msg = "Player " + name + " has " + kills + " kills";
+
+// GOOD: zero allocation with stackalloc
+using var sb = new ValueStringBuilder(stackalloc char[64]);
+sb.Append($"Player {name} has {kills} kills");
+var msg = sb.ToString(); // single allocation for the final string
+```
+
+**Instead of StringBuilder**:
+```csharp
+// BAD: StringBuilder allocates on heap
+var sb = new StringBuilder();
+sb.Append(name);
+sb.Append(": ");
+sb.Append(value);
+return sb.ToString();
+
+// GOOD: ValueStringBuilder with stackalloc
+using var sb = new ValueStringBuilder(stackalloc char[64]);
+sb.Append($"{name}: {value}");
+return sb.ToString();
+```
+
+**For packet string construction** (hot path):
+```csharp
+// Use AsSpan() to avoid ToString() allocation when the consumer accepts spans
+using var sb = new ValueStringBuilder(stackalloc char[32]);
+sb.Append(bounty);
+sb.Append(" gold");
+writer.WriteString(sb.AsSpan(), textBuffer); // zero-copy
+```