Code standard - NexusTomorrow/server GitHub Wiki
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.
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.
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.
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.
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.
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.
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.
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.
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.
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 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.
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.
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 |
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.
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.
| 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 |
| 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/ |