Best Practice

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

2. Message system calls with untyped object[] parameters

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

3. Accessing shared data in Awake/Start

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

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

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

5. 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

6. Unsolicited UI code changes

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

Performance

7. 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*

8. 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

9. 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

10. gameObject.tag == string comparison

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

Runtime Safety

11. 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

12. 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

13. Circular dependency risk

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

14. Missing .asmdef references

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

15. Incorrect namespace conventions

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

16. 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

17. Excessive null checks

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

18. 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