Skip to content

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 / ArgumentNullException on 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, not inv. configuration, not cfg. Established domain terms (Id, Rpc, Ui, Hp) are allowed.
  • Names say what a thing is, not what it is made of. PropertyRegistry, not PropertyDictionary.
  • 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(), or Task.Run to 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 void is 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:

  1. Every client→server request is [Rpc.Host] with typed parameters.
  2. The host re-validates everything: authority, ownership, distance, state, cooldown, affordability. Client-side checks are advisory UI only.
  3. No client value is trusted, including ones the client "could not" have forged.
  4. Rate limits are declared, not hand-rolled per handler.
  5. 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

  • .editorconfig at 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.
  • using directives outside the namespace, System first, 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.