warrentyexpired/ModernUO
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions
ModernUO/CLAUDE.md
Kamron Batman 61e41df00c
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

7.4 KiB
Raw Blame History

ModernUO

.NET 10 Ultima Online server emulator. Single-threaded game loop. All game logic runs on one thread.

  • Server engine: Projects/Server/ — do NOT modify without explicit request
  • Game content: Projects/UOContent/ — primary editing target
  • Build: dotnet build from repo root

Code Audit Rules

Apply these when writing or reviewing .cs files under Projects/.

  1. LINQ — Tier 1 (zero-cost patterns) free on hot paths; Tier 2 (low overhead) OK on warm paths; Tier 3 (allocating) forbidden on hot paths → dev-docs/code-standards.md
  2. No Console.WriteLine — use LogFactory.GetLogger(typeof(MyClass)) → logger.Information(...) (requires using Server.Logging;)
  3. No concurrency primitives — no lock, volatile, ConcurrentDictionary, Mutex, etc. Server is single-threaded.
  4. No World.Mobiles/World.Items iteration — use spatial queries: map.GetMobilesInRange<T>(), map.GetItemsInRange<T>()
  5. Clean up refs in OnDelete()/OnAfterDelete() — null out Item/Mobile references
  6. Cancel timers in OnDelete()/OnAfterDelete() — call _token.Cancel() or _timer?.Stop()
  7. STArrayPool<T>.Shared not ArrayPool<T>.Shared — single-threaded optimized, no locks
  8. PooledRefList<T> not new List<T>() on hot paths — zero GC pressure, stack-allocated ref struct
  9. Serialization — class must be partial, constructor needs [Constructible], TimerExecutionToken must NOT have [SerializableField]. New classes: use [SerializationGenerator(version)] (omit encoded). When bumping versions, add MigrateFrom(VXContent) (X = previous version). Never modify Deserialize(reader, version) for version bumps — that method is only for pre-codegen legacy saves. When migrating from pre-codegen Serialize/Deserialize: pass false if old code used reader.ReadInt(), bump version +1, and keep old logic as private void Deserialize(IGenericReader reader, int version) → dev-docs/runuo-migration-docs/02-serialization.md
  10. No Task.Run/new Thread() in game code — game logic is single-threaded event loop
  11. Never assume era — if code uses Core.AOS/Core.SE/etc., ask which expansion to target
  12. Naming — _camelCase private fields, PascalCase properties/methods/classes; don't flag legacy m_ but use _ for new code
  13. No empty gumps — every gump must produce visual elements. An empty gump leaks on client+server (no way to close it). Use static DisplayTo() to validate before constructing → dev-docs/gump-system.md
  14. PropertyList string literals must be holes — $"{"Map"}\t{value}" not $"Map\t{value}". The handler treats bare text as delimiters, {} holes as arguments. Only \t should be a bare literal → dev-docs/property-lists.md
  15. Braces required on all control flow — if, else, for, foreach, while, do, switch must always have braces, even for single-line bodies → dev-docs/code-standards.md
  16. Prefer switch expressions and switch-when — use switch expressions for value mapping and switch-when for pattern matching where they improve readability. Exception: skip if unreadable or cold path → dev-docs/code-standards.md
  17. No System.Text.StringBuilder — use ValueStringBuilder with stackalloc (bounded output) or ValueStringBuilder.Create() (unbounded). Supports $"..." interpolation directly. Always use using var for disposal. Use Reset() instead of reassigning → dev-docs/string-handling.md

Dev-Docs Reference

Topic File
Code standards & LINQ tiers dev-docs/code-standards.md
Serialization system dev-docs/serialization.md
Content patterns (Items, Mobiles, Creatures) dev-docs/content-patterns.md
Era & expansion handling dev-docs/era-expansion.md
Timer system dev-docs/timers.md
Event scheduler (wall-clock/calendar) dev-docs/event-scheduler.md
Object property lists (tooltips) dev-docs/property-lists.md
Gump (UI dialog) system dev-docs/gump-system.md
Commands & targeting dev-docs/commands-targeting.md
Event system dev-docs/events.md
Threading model dev-docs/threading-model.md
Configuration system dev-docs/configuration.md
Networking & packets dev-docs/networking-packets.md
Region system dev-docs/regions.md
String handling & ValueStringBuilder dev-docs/string-handling.md
RunUO migration (overview) dev-docs/runuo-migration-docs/00-overview.md
RunUO migration (all docs) dev-docs/runuo-migration-docs/

Claude Skills (Opt-In)

Detailed Claude Code skills live in dev-docs/claude-skills/. They are not auto-loaded — they must be copied to .claude/skills/ to activate.

When to offer: If the user is building complex content (new items, creatures, spells, gumps, quests, packets, serialization work, etc.), ask:

I have detailed Claude Code skills for this kind of work. Want me to enable them? I'll copy the relevant files from dev-docs/claude-skills/ to .claude/skills/.

Then copy only the relevant skill files based on the task:

Task Skills to enable
New Item or Mobile modernuo-content-patterns, modernuo-serialization, modernuo-property-lists
Creature / spawn modernuo-content-patterns, modernuo-serialization, modernuo-timers
Spell or ability modernuo-content-patterns, modernuo-serialization, modernuo-timers, modernuo-era-expansion
Gump / UI dialog modernuo-gump-system, modernuo-commands-targeting
Quest or event system modernuo-events, modernuo-content-patterns, modernuo-configuration
Scheduled / seasonal / holiday events modernuo-event-scheduler, modernuo-timers
Custom regions / dynamic areas modernuo-regions, modernuo-content-patterns
Packet / networking modernuo-networking, modernuo-threading
Commands modernuo-commands-targeting
Timer work modernuo-timers, modernuo-serialization
Config system modernuo-configuration
Era-conditional code modernuo-era-expansion
String building / formatting modernuo-string-handling
Code review / audit modernuo-code-audit
Any .cs file edit modernuo-code-audit (always offer for code changes)
RunUO Migration
Migrate any RunUO script migrate-from-runuo/migrate-foundation (always), plus system-specific skills below
Migrate Item/Mobile/Creature migrate-from-runuo/migrate-foundation, migrate-from-runuo/migrate-serialization, migrate-from-runuo/migrate-items-mobiles
Migrate serialization migrate-from-runuo/migrate-serialization
Migrate timers migrate-from-runuo/migrate-timers
Migrate gumps migrate-from-runuo/migrate-gumps
Migrate packets migrate-from-runuo/migrate-packets
Migrate property lists migrate-from-runuo/migrate-property-lists
Migrate events/commands migrate-from-runuo/migrate-commands-events
Migrate persistence (WorldSave) migrate-from-runuo/migrate-persistence
Migrate multi-file system migrate-from-runuo/migrate-systems

To enable a skill: cp dev-docs/claude-skills/<name>.md .claude/skills/

Migration skills reference the deep docs in dev-docs/runuo-migration-docs/ and point to existing ModernUO skills for best practices.

The modernuo-code-audit skill auto-triggers on .cs file edits and flags convention violations (warnings only, asks before fixing).