Skip to content

Code standard

rebaserHEAD edited this page Oct 17, 2026 · 40 revisions

explanation, committed verified 2026-10-17 bootstrap 48bb36d

Why the code looks the way it does, and why the build rather than a reviewer holds it there. The rules themselves are in CONTRIBUTING.md; this page carries the reasoning behind them.

The shape of it

Mostly Microsoft's conventions

We start from Microsoft's published C# conventions: the coding conventions and identifier naming pages, which the .NET runtime team's own style grew into. Anyone who knows .NET already knows most of our rules. Where we differ, the project gave us a reason, and the reasons are below.

The build holds the line

A rule nobody's build checks is a rule reviewers end up arguing about. So every rule an analyser can check lives in .editorconfig, and the build treats every warning as an error, style rules included: a violation fails the build. It's the same stance the codec design takes on wire layouts, where a message field without an explicit width is a build error, not a review comment.

CI adds four checks: before it builds, every commit message since main, a scan for em dashes in .cs and .md files, and a scan of the same files for words that say how a client fact was found; after it builds, dotnet format confirming it would change nothing. The format check has to follow the build, because it loads the codec's source generator from the build output. What only a reviewer can see, such as whether a summary says something useful, stays with review.

The analysis level is pinned to a release, 10-recommended, not latest. A newer SDK can't add rules to a commit nobody touched; raising the level is a commit of its own, with whatever it finds fixed in the same change.

Where it came from

The code we started from followed NexusForever's house style: British spelling, a region around injected fields, enum values aligned on the =, past-tense commit messages. We kept it while we were contributing upstream, because matching the maintainer's style is part of being a good contributor. With no upstream left to match, every rule had to earn its place again. We picked the standard in one go, before the codec's converter rewrote about 1,250 files, so nothing was restyled twice.

Our choices, and why

American spelling in identifiers

The client's own names are American, and so are .NET's (Initialize, Color). The old code modelled the client's HousingNeighborhoodInfo table as HousingNeighbourhoodInfoEntry, and even had a Neighbourhood class living in Neighborhood.cs. So a table or a message takes the client's own name where the client has one: the table model is now HousingNeighborhoodInfoEntry. Spelling in comments and prose isn't policed.

A wire message only gets a name once it's been checked against the client. Until then it's named after its opcode and its fields by position, so 0x009A is Client009A with fields Unknown0 to Unknown3, and the code never claims a meaning nobody has checked. The message audit has the whole scheme.

Underscored fields, and s_ for statics

Private and internal fields are _camelCase, which is Microsoft's convention, and the _ tells a field from a parameter without this..

Static ones take s_. That's the runtime team's convention, and it earns its keep here for a reason of our own: each owner in the server core design is single-threaded over its own state, so a static field is the one place state can cross between map threads. The prefix flags every touch of shared state right where it happens. Constants stay PascalCase.

File-scoped namespaces

One level of indentation less in every file, and the default in .NET's own templates. Mixing the two forms is worse than either, so the whole tree moved at once.

No hand alignment, no regions

dotnet format collapses column alignment anyway, and aligned blocks make diffs lie: rename one enum value and every line around it changes. Trailing commas are welcome for the same reason, since adding a value then touches one line. Regions hide code, the constructor most of all, and nothing checks them.

Summaries where they carry meaning

Every public type and member gets a /// <summary>, except in data declarations. Game table row models and enums document the type, and a member only when its name can't carry its meaning: units, a bit layout, why an unknown value is still unknown. Ten thousand summaries saying "The item id." would be noise that drifts.

Wire messages are documented by how far they've been checked against the client, because their documentation is what that check found. A message at the Purpose rung has a name for what it does in place of its neutral number, and a summary on its type. At Fields, every wire member of every layout it reaches has a summary too, whatever its accessibility, and a member that keeps its neutral Unknown<n> name begins its summary with "The client gives this value no meaning." The registry verification tests hold every message to its rung, so the compiler's missing-summary warning, CS1591, doesn't govern wire declarations. A message nobody has checked carries no documentation rather than a guess.

Sealed by default

A class is sealed unless it was built to be inherited from. The JIT can devirtualise calls on a sealed type, and inheritance becomes a decision somebody made rather than a door left open.

Nullable on everywhere

Nullable reference types are on in every project, and nullable warnings are errors. A lookup that can miss returns T? and its callers handle the miss; ! appears only where a null is impossible, with a comment saying why. A required configuration member is also validated where it's bound, because the configuration binder sets members through reflection and ignores required.

Imperative commits with a blank line

Commit headlines are imperative, with no full stop, in 72 characters or fewer: git's own convention, and how git and GitHub phrase every message they write. The blank line under the headline is the part that isn't taste. Git treats everything above the first blank line as the subject, so without it git log --oneline and GitHub run the whole message together into one title.

What the build lets through

Suppressions, each with its reason

A rule that genuinely doesn't fit some code is switched off for that code, in .editorconfig, with a comment saying why. Working around a rule to quiet it isn't allowed.

Where Off Why
Everywhere CA1716 It guards against keywords of other .NET languages, such as Visual Basic's Static, which names our Static namespaces. The codebase is C# only
Everywhere CA1711, for the Flags suffix A [Flags] enum named ...Flags reads as what it is, as the BCL's BindingFlags does
Game.Static, GameTable CA1707, CA1711, CA1720 Data-shape code keeps the client's own names: its enum values, type names and value kinds
GameTable/Model CA1051 Table row models are field records the loader fills by position, so their fields stay public fields
ClientConnector/Native CA1051, CA1707 Win32 interop structs keep the API's names and field layout, which the marshaller reads
Network.Codec.Generator RS2008 Release tracking serves analysers published as packages; this generator ships only inside the repository, built with the projects that use it
Network.World, Network.Auth with <NoWarn>; Network's message models and opcode list in .editorconfig CS1591 Wire declarations carry the summaries their verification rung requires, which the registry verification tests enforce. The opcode list is a data declaration

The hot-path tier

A switch per project

Code that runs on every tick or for every message is held to more than the general rules, because at 40 ticks a second on every busy map, an allocation or a blocking call isn't a style problem. A project opts in with <HotPath>true</HotPath>. Directory.Build.targets then adds the banned-API analyser with the list in Build/HotPath/BannedSymbols.txt, and raises the performance rules to errors through Build/HotPath/HotPath.globalconfig. The codec is the first project to set it; the map tick, replication, the session writer and the timer wheel join it when the core is built.

Banned Why
LINQ (System.Linq.Enumerable) It allocates. Write the loop
Activator Reflection. Construct the type directly
Enum.ToObject, Convert They box the value. The old packet writer boxed every enum this way
Stream, MemoryStream Hot code reads and writes spans
DateTime.Now, DateTime.UtcNow Time comes from the injected clock, which the test harness swaps for a virtual one
Thread.Sleep Only the timer sleeps
Task.Result, Task.Wait They block the owner's thread
GC.Collect The tick never forces a collection

On top of the ban, the switch raises CA1825, CA1829, CA1848, CA1851, CA1852, CA1860, CA2014, CA2254, IDE0250 and IDE0251 to errors. Two of those mean logging goes through source-generated LoggerMessage methods, so a log call on the tick neither formats nor boxes.

Where it stops

A test run with a banned call planted in the codec failed the build at that line, which is the switch doing its job. Analysers don't see generated code, though, so the generator's own tests hold its output to the same rules, including one that measures a generated Write and requires zero allocated bytes.

And analysers only catch intent. The truth is a measurement: once the core exists, the test harness's zero-allocation check on the map tick has the final word. Lambdas that capture, boxing through an interface or a generic, and exceptions used for control flow are still for review to catch.

The knobs

File Controls
.editorconfig Every style and naming rule, with its severity, and every suppression with its reason
Directory.Build.props Nullable on for every project; every warning an error; code-style rules run in the build; the analysis level pinned to 10-recommended
Directory.Build.targets, Build/HotPath/ The hot-path switch, its ban list and the rules it raises to errors
.github/workflows/test.yml The commit message, em dash and method word checks before the build, and the format check after it
global.json The SDK band (10.0.400, latest patch), so every machine and CI run the same compiler, analysers and formatter
.gitattributes Text stored with LF, checked out with CRLF everywhere; a byte order mark only on .cs files, set by .editorconfig
.git-blame-ignore-revs Repository-wide mechanical commits that git blame skips

Where the code lives

Piece Path
Style and naming rules .editorconfig
Build-wide properties Directory.Build.props
The hot-path switch Directory.Build.targets
The ban list and hot-path rules Build/HotPath/
CI checks .github/workflows/test.yml
SDK pin global.json
Line endings .gitattributes
Commits git blame skips .git-blame-ignore-revs
The rules as a contributor reads them CONTRIBUTING.md
The documentation each verification rung requires Tests/NexusTomorrow.Network.Codec.Tests/Verification/

Clone this wiki locally