Coding Standards¶
Concrete and enforceable. Every rule here is either checkable by a compiler, a linter, or a reviewer with a ruler. Rules without a check are guidelines and are marked as such.
1. Size limits¶
| Unit | Preferred | Hard maximum |
|---|---|---|
| File | 500 lines | 1000 |
| Class | 300 lines | 500 |
| Method | 30 lines | 100 |
| Method parameters | 4 | 7 — beyond that, take a parameter object |
| Nesting depth | 3 | 5 |
| Cyclomatic complexity | 8 | 15 |
Exceeding a hard maximum requires a comment at the top of the file explaining why, and it will be raised at review. Generated code and asset definitions are exempt.
Rationale: the legacy codebase has a 58 KB command file, a 965-line player metatable and a 924-line chat client (D-14). Every one of them started reasonable.
2. Type design¶
One responsibility per class. If the class name needs "and" or "Manager" or "Helper" to describe it, split it.
Composition over inheritance. Inheritance is permitted only when there is a genuine substitutability relationship and the hierarchy is at most two levels deep. In practice: prefer a component, an interface, or a delegate.
No God objects. Nothing owns more than one subsystem's state. See Legacy/13_DEPENDENCY_GRAPH.md.
Prefer interfaces at module boundaries. A module exposes interfaces; it does not export
concrete types. Concrete types are internal unless there is a reason.
Immutable by default. Prefer readonly, init-only properties and record types for
data. Mutable state must be owned by exactly one type.
No static mutable state. Statics may hold constants and pure functions only. Anything
with a lifetime belongs to a service. This rule exists specifically to prevent the
ply.cider failure mode.
No global mutation, ever. Do not replace, patch or wrap engine or framework globals. See D-07.
3. Nullability and validation¶
- Nullable reference types are enabled solution-wide (
<Nullable>enable</Nullable>). Warnings are errors. - Public methods validate their arguments and throw
ArgumentException/ArgumentNullExceptionon programmer error. - Player input is never a programmer error. Anything arriving from a client is validated and rejected with a message, never thrown on. See §7.
- Prefer returning a result type over throwing for expected failures ("you can't afford this" is not exceptional). See Architecture/13_ERROR_HANDLING.md.
4. Naming¶
Full rules in 01_NAMING.md. The short version:
- No abbreviations.
inventory, notinv.configuration, notcfg. Established domain terms (Id,Rpc,Ui,Hp) are allowed. - Names say what a thing is, not what it is made of.
PropertyRegistry, notPropertyDictionary. - Booleans read as assertions:
IsLocked,HasAccess,CanAfford. - Async methods end in
Async. - No Hungarian notation, no
m_, no_prefix except on private fields.
5. Async¶
- Async all the way down; never
.Result,.Wait(), orTask.Runto fake sync. - Anything touching persistence or HTTP is async. Gameplay code that needs a value synchronously must read from a cache that the async layer maintains.
- Every async method that can be cancelled takes a
CancellationToken. async voidis forbidden except in event handlers the engine requires.
6. S&box specifics¶
These follow from the engine and are not negotiable.
Serialisation. Only properties with both a getter and a setter serialize. Public fields do not. Anything persisted or exposed to the inspector is a property.
Components do one thing. A component is a behaviour, not a bag. Prefer three small components on a GameObject to one large one.
Do not wrap engine types. No AppleJackGameObject, no IEntity. Use GameObject,
Component, Scene, GameResource directly. The framework abstracts cross-cutting
concerns only — logging, config, persistence, events, commands, permissions.
Network attributes are deliberate. Every [Sync] property carries a comment stating who
may write it and why. Default to [Sync(SyncFlags.FromHost)]; plain [Sync] requires
justification. See Architecture/07_NETWORKING.md.
No polling. State replicates on change. A timer that re-sends unchanged data is a defect (D-11).
Assets over code. If a designer might reasonably want to change it — a price, a capacity,
a cooldown, a model — it is a GameResource property, not a constant.
7. Server authority¶
Non-negotiable, and reviewed on every change that touches networking:
- Every client→server request is
[Rpc.Host]with typed parameters. - The host re-validates everything: authority, ownership, distance, state, cooldown, affordability. Client-side checks are advisory UI only.
- No client value is trusted, including ones the client "could not" have forged.
- Rate limits are declared, not hand-rolled per handler.
- Any state a player could profit from forging is host-owned.
8. Testing¶
Full rules in 03_TESTING_STANDARDS.md. The binding constraint:
Rules and arithmetic live in engine-agnostic types.
Capacity maths, stacking, pricing, permission resolution, save migration and access
evaluation must be testable under dotnet test with no scene, no host, and no player. If a
rule can only be exercised by launching the game, it is in the wrong place.
9. Comments and documentation¶
Full rules in 02_DOCUMENTATION_STANDARDS.md. The binding rules:
- XML
<summary>on every public type and member. No exceptions. - Comments explain why, not what.
- Every deviation from the obvious approach carries a comment naming the reason.
- Every value inherited from the original cites its legacy source, so a future reader knows it is deliberate tuning rather than an arbitrary constant:
// Legacy: sh_config.lua config["Door Tax Amount"] = 50, charged per door per payday.
// The tax is what stops door hoarding -- see Documentation/Legacy/07_ECONOMY.md §4.
[Property] public int DoorTaxAmount { get; set; } = 50;
10. Forbidden¶
A short list of things that will be rejected at review regardless of justification:
| Forbidden | Because |
|---|---|
| Hand-built SQL or query strings | D-01 |
| Patching or replacing globals | D-07 |
| Static mutable state | The ply.cider failure mode |
catch { } swallowing an exception silently |
Failures must be logged or handled |
| Trusting a client-supplied value | §7 |
| A magic number in gameplay code | It belongs in configuration |
.Result / .Wait() |
Deadlocks |
| Reaching into another module's concrete types | Module boundaries |
| Reflection over gameplay types at runtime | Cost and fragility; use interfaces. One named, audited exception: Code/Core/Plugins/PluginDiscovery.cs, per ADR-0010 -- "Reflection is scoped to one audited file," guarded by UnitTests/Core/Plugins/ReflectionCarveOutTests.cs asserting no other file under Code/ uses it. The exception is named explicitly so the rule keeps its teeth everywhere else. |
| Committing a compiler warning | Definition of Done |
11. Formatting¶
.editorconfigat the repository root is authoritative; the build treats formatting violations as warnings and warnings as errors.- Four spaces, no tabs. Allman braces. Braces always, even for one-line bodies.
- One type per file; the filename matches the type.
usingdirectives outside the namespace,Systemfirst, then sorted.- File-scoped namespaces.
12. Enforcement¶
| Rule class | Enforced by |
|---|---|
| Formatting, nullability, warnings-as-errors | Build |
| Naming, complexity, forbidden constructs | Analyzers + .editorconfig |
| Size limits, responsibility, module boundaries | Review |
| Testability, documentation, authority | Review + Definition of Done |
A rule that is repeatedly violated is either wrong or unenforced. Fix one or the other — do not let it rot.