best-practice— agent skill

Script path fallback: qq scripts are invoked as bare commands (e.g. unity-test.sh). If "command not found", use ${CLAUDE_PLUGIN_ROOT}/bin/<command> instead.

Respond in the user's preferred language (detect from their recent messages, or fall back to the language setting in CLAUDE.md).

Quick best-practice check for Unity C# code. Run this right after editing code — it scans for anti-patterns, performance traps, and runtime safety issues against 18 rules.

Arguments: $ARGUMENTS

Scope Detection

Intelligently determine what to check:

1. If the user specified files or a scope → use that
2. If .cs files were edited in this conversation → check those files
3. If there are uncommitted changes → git diff --name-only HEAD -- '*.cs'
4. If none of the above → ask the user what to check

Do NOT review the entire codebase by default. Focus on what just changed.

Review Rules

Deterministic Policy Layer

If qq-policy-check.sh is available, run it first against the selected scope:

qq-policy-check.sh --json <files...>

Treat those results as the first-pass findings. Do not ask the model to rediscover the same high-confidence issues from scratch. Use the model for:

• additional contextual review
• prioritization
• explanation
• fix suggestions

Anti-Pattern Detection

1. FindObjectOfType / FindObjectsOfType

• Severity: High
• Alternative: use the appropriate Registry/Manager singleton for your project
• Exception: Editor code (Assets/Editor/, inside #if UNITY_EDITOR blocks) may use these

1. Message system calls with untyped `object[]` parameters

• Severity: Medium
• Alternative: strongly-typed interfaces or events

1. Accessing shared data in Awake/Start

• Severity: High
• Alternative: implement the appropriate lifecycle interface and access data in the ready callback

1. Caching a read-only interface reference then mutating through it

• Severity: High
• Alternative: read-only access only

1. SendMessage / BroadcastMessage / SendMessageUpwards

• Severity: Medium
• Uses reflection, no compile-time safety, string-based (typos cause silent failures)
• Alternative: C# events, UnityEvents, or interface-based dispatch

1. Unsolicited UI code changes

• Severity: Notice
• UI code should not be modified unless the user explicitly requested it

Performance

1. GetComponent in Update / FixedUpdate / LateUpdate

• Severity: High
• GetComponent uses native interop + type lookup per call; in hot loops this causes measurable CPU overhead and GC pressure
• Alternative: cache component references in Awake/Start or use [SerializeField]
• Also flag GetComponent inside OnCollision, OnTrigger

1. Per-frame heap allocations

• Severity: High
• Flag inside Update/FixedUpdate/LateUpdate: new List, new Dictionary, string + or $"" interpolation, .ToString(), LINQ queries (.Where, .Select, .ToList), lambda closures
• Alternative: pre-allocate and reuse, use StringBuilder, use non-alloc APIs

1. Coroutines started without cleanup

• Severity: High
• StartCoroutine without corresponding StopCoroutine or StopAllCoroutines in OnDisable causes orphan coroutines when objects are pooled or re-enabled
• Alternative: cache coroutine references, stop in OnDisable

1. `gameObject.tag ==` string comparison

• Severity: Medium
• Allocates a string on the heap every call
• Alternative: use CompareTag() (allocation-free)

Runtime Safety

1. Event subscription without unsubscription

• Severity: High
• Every += event subscription must have a matching -= unsubscription
• Subscribe in OnEnable, unsubscribe in OnDisable; failing to do so causes memory leaks and double-firing

1. Missing [RequireComponent] for GetComponent dependencies

• Severity: Medium
• If Awake/Start calls GetComponent<T>() and the result is used without null check, the class should have [RequireComponent(typeof(T))]
• Makes hidden dependencies explicit and auto-adds them in the Inspector

Architecture Checks

1. Circular dependency risk

• Check that new using directives do not violate the project's established dependency direction

1. Missing .asmdef references

• If a file uses a namespace from another Service module, verify the corresponding .asmdef reference exists

1. Incorrect namespace conventions

• Severity: Medium
• Check that namespaces follow the project's established naming scheme

1. Public fields instead of [SerializeField] private

• Severity: Medium
• Public fields on MonoBehaviours break encapsulation; any script can modify them
• Alternative: [SerializeField] private for Inspector-assigned fields

Code Quality

1. Excessive null checks

• Project style: minimal null checks, rely on exceptions to surface problems
• Only validate at system boundaries (user input, external APIs)

1. Missing documentation comments

• Public classes and complex methods should have summary comments

Project-Specific Rules

Add your project's coding standards and anti-patterns here. Example:

• No raw SQL queries
• All public APIs must have documentation

Output Format

Group output by severity:

## 🔴 Critical (High severity — must fix)
- [file:line] Issue description → suggested fix

## 🟠 Moderate (Medium severity — should fix)
- [file:line] Issue description → suggested fix

## 🟡 Suggestions (Notice — nice to fix)
- [file:line] Issue description → suggested fix

## ✅ Code Quality Highlights
- Brief list of things done well

Execution

1. Determine scope (see Scope Detection above)
2. Run qq-policy-check.sh --json if available
3. Read all relevant .cs files
4. Check each of the 18 rules above, but treat deterministic policy findings as already-established
5. Also read AGENTS.md (if it exists) for project-specific architecture rules
6. Merge deterministic findings with model findings in the output format above
7. If critical issues are found, ask whether to auto-fix them

Handoff

After the check completes, recommend the next step:

• No issues found → "Clean. Want to run /qq:test to verify, or /qq:claude-code-review for a deeper review?"
• Issues found and fixed → "Fixed N issues. Want to re-run /qq:best-practice to confirm, or proceed to /qq:test?"
• Issues found, user declined fix → "N issues remain. Proceed to /qq:test anyway, or fix first?"

`--auto` mode: skip asking, take the strictest path: → auto-fix all issues → re-run self until clean → /qq:claude-code-review --auto