Skip to main content
Every pull request is a promise that the change is correct, consistent, and complete. This checklist makes that promise concrete. It also makes reviews faster: a reviewer whose first job is ticking checklist boxes rather than catching the same naming violation for the tenth time can spend their attention on logic and design instead. Work through every section before marking a PR as Ready for Review. A PR submitted with unchecked items will be returned immediately without a full review.
If any item in this checklist cannot be ticked, the PR is not ready to merge. No exceptions are granted by “we’ll fix it in a follow-up.” The follow-up never comes.
The diagram below shows the gate sequence a PR must pass in order. Each gate depends on the previous one — a PR blocked at Architecture cannot be reviewed for Tests until the architectural violation is resolved.

​
Naming & Style

Namespaces, class names, method names, and field names follow the SET: 3D Edition Naming Conventions
No #region directives anywhere in the changed files
No public mutable fields on any entity, value object, or DTO — only properties
All new classes are sealed unless they are explicitly documented as base classes
All boolean members use an Is, Has, or Can prefix (IsValid, HasPenalty, CanClaim)
Method length does not exceed ~40 lines (document any FSM exhaustive-switch exception)
No method accepts more than 3 parameters without a parameter object

​
Architecture & Layer Boundaries

No using UnityEngine; in SET.Domain or SET.Application assemblies
No using Nakama; in SET.Domain or SET.Application assemblies
No FindObjectOfType, GameObject.Find, or GetComponent calls outside the Presentation layer
No static mutable fields anywhere in the Domain layer
Infrastructure classes implement interfaces declared in Application or Domain — not the other way around
SET.Presentation accesses Infrastructure only through injected interfaces, never by direct assembly reference
The fastest way to verify these rules: check the .asmdef dependency graph and grep for using UnityEngine in the Domain and Application folders. CI will also catch violations via the Roslyn analyzer.

​
Dependency Injection

No new ConcreteClass() inside Application or Domain methods (factories are the only exception)
All dependencies are injected as interface types via the constructor
Any new binding is registered in the Bootstrap scene’s VContainer composition root
No service-locator calls (ServiceLocator.Get<T>(), Container.Resolve<T>() from inside a class) — resolution happens only at the composition root

​
Reactive UI

No game state polling in any Update() method — use R3 reactive subscriptions
All Subscribe(...) return values are stored in a CompositeDisposable field
CompositeDisposable is disposed in OnDestroy() (for MonoBehaviours) or in the class’s Dispose() method
DistinctUntilChanged() applied on ViewModel properties bound to UI elements
Any Nakama callback that touches Unity objects or ViewModel state is marshalled to the main thread via .ObserveOnMainThread()

​
Game Logic

All match state changes go through GameSession.HandleCommand() — no direct mutation of Board, Player, or Deck from outside GameSession
AnySetExists() is called after every board mutation (card removal, card addition, board expansion)
End-game condition is evaluated after every state transition
Input events are discarded (not queued) while GameSession is in an animation lock state (Validating, Refilling, MatchEnd)
Penalty logic uses the PenaltyMode from GameRules — no hard-coded penalty values

​
Multiplayer (complete only if this PR touches multiplayer code)

The client does not call SetValidator.Validate() on a multiplayer code path — claim validation is server-authoritative only
The client sends claim intent only: SendClaim(cardSlotIds) — not a pre-validated result
Server state is applied to GameSession only via ApplyServerState() — not via direct board mutation
Disconnect and reconnect scenarios handled: grace period countdown shown, full state sync on rejoin
No Nakama types (IMatch, IMatchState, ISocket) referenced outside NakamaMultiplayerService

​
Tests

All new Domain and Application logic has corresponding unit tests in SET.Tests.EditMode
All new test methods follow the MethodName_Scenario_ExpectedBehavior naming pattern
All tests follow the Arrange-Act-Assert (AAA) structure with a blank line separating each section
All tests pass locally (Run All Tests in Unity Test Runner) before pushing
No tests are commented out or marked [Ignore] without a linked ticket number in a comment
Tests assert on observable behaviour (return values, emitted events, state snapshots) — not on internal call counts or implementation details

​
Definition of Done (Per Feature)

A feature is not done until every item below is true. “Done” does not mean “the happy path works in my dev build.”
Code compiles with zero errors and zero new warnings on the CI build server
All CI checks pass: dotnet format, Roslyn analyzer, unit tests (EditMode), integration tests (PlayMode)
At least one reviewer has approved the PR with no unresolved blocking comments
The Hard Boundaries document confirms this feature is in scope for v1.0 — no out-of-scope work has been introduced
Every new public interface and every non-obvious public method has an XML-doc comment (/// <summary>...)
The PR description explains what changed and why — not just a list of files edited
If this feature introduces a new third-party SDK or package, the package version is pinned and documented

​
Common Reasons PRs Are Returned

These are the most frequent violations. If you catch yourself doing any of these, fix them before submitting.

Coding Conventions

The naming, formatting, and class design rules this checklist enforces.

Approved Patterns

The architectural patterns and banned anti-patterns checked in the Architecture section.

Testing Standards

Unit test naming, AAA structure, and coverage targets referenced in the Tests section.

Phase Breakdown

Per-phase Definitions of Done that feed into this checklist.