## Summary
Two workflow additions to `CLAUDE.md`, with the detail in `dev-docs/`, plus a GitHub issue form.
### Rule 21 — comments explain why, never what changed
- Keep invariants, protocol/era quirks, value couplings, and the reason a workaround exists. One line where one line will do.
- Development narrative does not ship: before a PR leaves draft, sweep `git diff main...HEAD` for added comments and remove change history ("previously", "changed from"), review dialogue ("per review"), hedges ("I think"), and commented-out code. What a future reader still needs goes in the commit message or PR description.
- New `## Comments` section in `dev-docs/code-standards.md`; rule 21 in the `modernuo-code-audit` skill.
### Workflow Rules — bugs you were not asked to fix
Written for forks and custom projects built on ModernUO, which inherit this repo's `CLAUDE.md`. Also applies here (upstream is `origin`).
1. **Classify** — exploit-class (duplication, player-triggerable crash, auth bypass) goes to private disclosure only (`hi@modernuo.com`, per `CONTRIBUTING.md`), never a public issue, PR, or Discord post.
2. **Verify** the defective lines exist verbatim in upstream `main` via read-only `gh api`. If they don't, it is the fork's bug and nothing leaves the fork. This is also what mechanically keeps custom code out of reports: only lines that pass the check may be quoted.
3. **Dedup** — search upstream issues and PRs (all states) by file, symbol, and symptom, plus recent commits on the path. A merged fix → offer to import it; an open issue → offer to comment there.
4. **Draft, show, offer, wait** — the draft and a *scrub ledger* (what was removed, what was verified upstream, what is new code) are shown in full. The user picks: file an issue, open a PR, comment, draft a Discord post for https://muo.gg/discord, or nothing. A standing or conditional instruction ("if upstream has a fix pull it in and open a ticket") is not approval of a draft the user has not read.
5. **Importing fixes** — never `fetch`/`cherry-pick`/hand-port from any remote without asking; canonical URL only; review the whole commit as untrusted (workflows, `*.csproj`, `Directory.Build.props`, scripts); apply only after a second yes. Third-party forks and unmerged PRs are never a source.
`dev-docs/bug-reporting.md` is the process; `dev-docs/claude-skills/modernuo-bug-reporting.md` is Claude's step-by-step procedure (opt-in, like the other skills).
### Issue form
- `.github/ISSUE_TEMPLATE/bug_report.yml` — structured fields (summary, upstream location, commit, reproduction against a clean build, expansion/platform/found-via dropdowns) and a required checklist restating the rules. Applies the `bug` label.
- `.github/ISSUE_TEMPLATE/config.yml` — chooser links for private security reports and Discord.
- Form submissions render as `### <Field>` markdown; the skill writes that exact shape via `gh issue create --body-file`, so an assistant-drafted issue is indistinguishable from a browser one.
## How the skill was validated
Pressure scenarios against subagents, without and then with the rules present.
- **Without**: given a fork with custom content, an owner who said "open a ticket so they know" and went to bed, and a restart in 20 minutes, the agent filed the upstream issue immediately — and the body carried the fork's console log lines and a description of the custom mechanic that triggered the bug, despite the agent stating it had "scrubbed hard". It did refuse an unreviewed third-party PR.
- **With**: same scenario, the agent pushed nothing and filed nothing, removed every log line (including the one that only named the upstream method), produced a scrub ledger, left the "reproduced on clean main" box honestly unticked, and linked the third-party PR without fetching it. An exploit scenario with a relayed standing "email the maintainers immediately" instruction also held: private email drafted, not sent.
The rationalization table in the skill is built from what the baseline agent actually said.
## Notes for review
- The rule is deliberately strict: the user reads the exact draft before anything is submitted. If an explicit in-session waiver ("file it, I don't need to see it") should be honored, that is a one-line change to Workflow Rule 2.
- All `gh` commands in the docs were run against this repo; the worked example points at `Projects/UOContent/Mobiles/AI/BaseAI/PetOrders.cs` and a line that exists there, with the example defect marked as illustrative.
- `config.yml` links private disclosure to `CONTRIBUTING.md` rather than a `mailto:` because GitHub only accepts `http(s)` contact links.
16 KiB
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 buildfrom repo root
Code Audit Rules
Apply these when writing or reviewing .cs files under Projects/.
- 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 - No
Console.WriteLine— useLogFactory.GetLogger(typeof(MyClass))→logger.Information(...)(requiresusing Server.Logging;) - Threading policy — game logic runs only on the main loop; never touch game state (
World, mobiles, items, maps, timers) from a background thread. Heavy work that needs game state must be chunked across ticks, not threaded. Heavy work that does not need game state (large-file parse, external I/O) must run on a background thread and must yield to world saves (defer whileWorld.Saving/WorldState.PendingSave). Publish results back to the loop as an immutable snapshot swapped via a singlevolatilereference — the only sanctionedvolatile. Nolock/Mutex/ConcurrentDictionaryin game logic. Rule #10 covers how background work hands results back to the loop →dev-docs/threading-model.md - No
World.Mobiles/World.Itemsiteration — use spatial queries:map.GetMobilesInRange<T>(),map.GetItemsInRange<T>() - Clean up refs in
OnDelete()/OnAfterDelete()— null outItem/Mobilereferences - Cancel timers in
OnDelete()/OnAfterDelete()— call_token.Cancel()or_timer?.Stop() STArrayPool<T>.SharednotArrayPool<T>.Shared— single-threaded optimized, no locksPooledRefList<T>notnew List<T>()on hot paths — zero GC pressure, stack-allocated ref struct- Serialization — class must be
partial, constructor needs[Constructible],TimerExecutionTokenmust NOT have[SerializableField]. New classes: use[SerializationGenerator(version)](omitencoded). Setters that coerce/veto/run side effects: use[SerializableField]argsallowFieldChange: nameof(BoolRefMethod)/fieldChanged: nameof(OldNewMethod)— reserve[SerializableProperty]for custom getters. SerializableTimermembers declare[DeserializeTimer(nameof(Method))]on the field (anchored by default — downtime preserves remaining delay;wallClock: true= absolute; method runs only when a timer was running at save). Conditional writes:[SaveFlag(nameof(Should), nameof(Default))]on the field. When bumping versions, addMigrateFrom(VXContent)(X = previous version). Never modifyDeserialize(reader, version)for version bumps — that method is only for pre-codegen legacy saves. When migrating from pre-codegen Serialize/Deserialize: passfalseif old code usedreader.ReadInt(), bump version +1, and keep old logic asprivate void Deserialize(IGenericReader reader, int version)→dev-docs/serialization.md,dev-docs/runuo-migration-docs/02-serialization.md - No
Task.Run/new Thread()for game logic (tandem with rule #3) — game logic is the single-threaded event loop. Backgrounding is allowed only for work that does not itself touch game state (external service calls, large-file parse). Prove the need before adding a thread: measure on-loop time, not wall-clock (frozen world is the cost, player latency is not), and gate onEnvironment.ProcessorCount— off-loading creates no CPU and buys nothing on 1–2 cores. New workers go in the vetted table indev-docs/threading-model.mdwith their measurement. When such work must feed game logic: run the heavy/I/O part off-loop andConfigureAwait(false)its awaits so a continuation never resumes on the loop and silently foregrounds heavy work; then hand the result back explicitly — publish an immutable snapshot swapped via avolatilereference (the loop reads it lock-free), or marshal the apply step withCore.LoopContext.Post(() => …), re-validating in the continuation whatever may have changed while it ran. Never touch game state off-thread; never let the scheduler decide where the heavy work runs →dev-docs/threading-model.md - Never assume era — if code uses
Core.AOS/Core.SE/etc., ask which expansion to target - Naming —
_camelCaseprivate fields,PascalCaseproperties/methods/classes; don't flag legacym_but use_for new code - 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 - PropertyList string literals must be holes —
$"{"Map"}\t{value}"not$"Map\t{value}". The handler treats bare text as delimiters,{}holes as arguments. Only\tshould be a bare literal →dev-docs/property-lists.md - Braces required on all control flow —
if,else,for,foreach,while,do,switchmust always have braces, even for single-line bodies →dev-docs/code-standards.md - 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 - No
System.Text.StringBuilder— useValueStringBuilderwithstackalloc(bounded output) orValueStringBuilder.Create()(unbounded). Supports$"..."interpolation directly. Always useusing varfor disposal. UseReset()instead of reassigning →dev-docs/string-handling.md - Interpolation anti-patterns on handler-aware APIs —
Send*/Say/Emote/PublicOverhead*/IPropertyList.Add/gumpAddLabel/AddHtml/Html.Center/SpanWriter.Write*all haveref RawInterpolatedStringHandleroverloads that allocate zero strings, but only when the call-site argument is a$"..."literal directly. Avoid: ternaries with interpolated branches (Send(c ? $"a" : $"b")), switch expressions with interpolated arms, pre-builtvar s = $"..."locals (single-use),.ToString()/.String()/string.Formatinside holes, string concat ({a + b}), LINQ string ops in holes. Use:Lformat spec for lowercase ({rank:L}notrank.ToString().ToLowerInvariant()) →dev-docs/string-handling.md§ Interpolation Anti-Patterns - No
InvalidateProperties()from insideGetProperties— every property aGetPropertiesoverride reads must be a pure read.InvalidateProperties()rebuilds the list in place (Reset()+ rebuild), andReset()returns the pooled interpolation buffer — which the compiler rents for the whole$"..."expression, so every hole is evaluated while it is live — and rewinds the packet cursor. A getter that invalidates therefore throwsArgumentNullException(parameter"array") out ofGetPropertiesfrom an unrelated-looking line, or silently corrupts the tooltip. The engine refuses and logs an error;DEBUGthrows. Lazy recomputation in a getter is fine — the notification is not. Invalidate in the setter that changes the value, or defer withTimer.DelayCall(InvalidateProperties)→dev-docs/property-lists.md§ Never Invalidate From InsideGetProperties - Tick-count math must be wraparound-safe — compare
Core.TickCount/GetTimestamp()values only by subtraction (a - b < 0, nevera < b), no zero/sign sentinels on tick fields, seed deadline fields from a real tick (never rely on the 0 default). Cloud hypervisors (GCP) pass through the host's never-resetting counter: ticks start enormous and can wrap negative. Linux affected in production; Windows not so far →dev-docs/tick-counts.md - Comments explain why, never what changed — keep invariants, protocol/era quirks, value couplings, and the reason a workaround exists; one line where one line will do;
///docs and terse//TODOstay. Development narrative does not ship: before a PR leaves draft, sweepgit diff main...HEADfor added//lines and remove change narrative ("previously", "changed from", "moved from", "no longer"), review dialogue ("per review", "as discussed"), hedges ("I think", "not sure"), and commented-out code — what a future reader still needs goes in the commit message or PR description →dev-docs/code-standards.md§ Comments
Workflow Rules
Apply these in every session, in this repository and in any fork or custom project built on it.
- Bugs you were not asked to fix — a latent defect, a suspicious upstream behavior, or an exploit found while doing something else is not yours to fix, file, or import on your own. Stop and follow
dev-docs/bug-reporting.md(procedure: themodernuo-bug-reportingskill). Exploit-class (anything a player could abuse: duplication, player-triggerable crash, auth bypass) → private disclosure only, never a public issue, PR, or Discord post. Otherwise: verify the buggy lines exist verbatim in upstreammain(github.com/modernuo/ModernUO) — if they don't, it is the fork's bug and nothing leaves the fork; search upstream issues, PRs (all states), and recent commits on that path for a fix or duplicate; then offer the user a choice: file an issue, open a PR, draft a Discord post for https://muo.gg/discord, comment on the existing thread, or do nothing. - Nothing leaves the fork or enters it without approval of that exact artifact — no
gh issue create,gh pr create,gh issue comment,git pushto a remote you do not own, orgit remote adduntil the user has read the exact draft and said yes. A standing or conditional instruction ("if upstream has a fix pull it in and open a ticket", "just file it") is not approval of a draft the user has not seen. Neverfetch,cherry-pick,merge, or hand-port code from any remote — includingmodernuo/ModernUO— without asking first; when approved, fetch only from the verified canonical URL, show the full diff, treat it as untrusted input (check.github/workflows,*.csproj,Directory.Build.props, scripts), and apply only after a second yes. Third-party forks and unmerged PRs are never a source. Drafts contain nothing from the fork: no credentials, IPs, hostnames, ports, account/character/player data, save files, log lines,Distribution/Configuration/, shard or custom-feature names, no description of custom mechanics, and no quoted code that is not verbatim in upstreammain. Reproduction is written against a clean upstream build.
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 |
Generic commands (where/order by/distinct, dot notation, @"" literals, [batch, [interface) |
dev-docs/generic-commands.md |
| Event system | dev-docs/events.md |
| Threading model | dev-docs/threading-model.md |
| Server hardware requirements | dev-docs/server-requirements.md |
| Debugging event-loop performance (profiling build, decomposition, GC/RAM) | dev-docs/debugging-event-loop.md |
| Tick-count overflow rules (subtraction comparisons; GCP pass-through counters) | dev-docs/tick-counts.md |
| Server lifecycle & bootstrap phases (Configure/ConfigurePrompts/Initialize) | dev-docs/server-lifecycle.md |
| Platform prerequisites (ICU, tzdata, native libs per distro) | dev-docs/platform-prerequisites.md |
| Configuration system | dev-docs/configuration.md |
| Networking & packets | dev-docs/networking-packets.md |
| IP bans, blocklists & allowlists (incl. unblocking a player) | dev-docs/ip-bans-and-allowlists.md |
| Region system | dev-docs/regions.md |
| String handling & ValueStringBuilder | dev-docs/string-handling.md |
| Reporting bugs upstream from a fork (classification, verification, dedup, disclosure, issue form, importing fixes) | dev-docs/bug-reporting.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) |
| Found a bug you were not asked to fix / reporting or importing an upstream fix | modernuo-bug-reporting |
| 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 — Claude Code loads .claude/skills/<name>/SKILL.md; a bare .md dropped
directly into .claude/skills/ is not picked up, and newly installed skills appear in the
next session:
# Standard skills (modernuo-*)
mkdir -p .claude/skills/<name> && cp dev-docs/claude-skills/<name>.md .claude/skills/<name>/SKILL.md
# Migration skills — sources live in the migrate-from-runuo/ subfolder, but install under the
# bare skill name (the table's "migrate-from-runuo/<name>" is the source path, not the name):
mkdir -p .claude/skills/<name> && cp dev-docs/claude-skills/migrate-from-runuo/<name>.md .claude/skills/<name>/SKILL.md
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).