best-practice

Run qq scripts as ${CLAUDE_PLUGIN_ROOT}/bin/<name>: they are not on PATH, so a bare qq-policy-check.sh exits 127.

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: anti-patterns, performance traps and runtime safety issues in what just changed.

Arguments: $ARGUMENTS

With --auto, first make sure a pipeline tracks this run, so the handoff at the end can advance it: qq-execute-checkpoint.py pipeline-start --project . --type feature --current-skill "/qq:best-practice" --branch "$(git branch --show-current)" --if-not-running. It starts one only when none is running here (you were invoked on your own) and leaves a running one alone.

Scope Detection

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

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 established findings — don't rediscover them; add contextual review, prioritization, explanation and fix suggestions on top.

Review Rules

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
   • 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
   • 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 a matching StopCoroutine / StopAllCoroutines in OnDisable (orphaned when pooled or re-enabled)
   • Alternative: cache coroutine references, stop in OnDisable
10. gameObject.tag == string comparison
    • Severity: Medium
    • Alternative: use CompareTag() (allocation-free)

Runtime Safety

11. Event subscription without unsubscription
    • Severity: High
    • Every += needs a matching -= (subscribe in OnEnable, unsubscribe in OnDisable)
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))]

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
    • Alternative: [SerializeField] private for Inspector-assigned fields

Code Quality

17. Excessive null checks
    • Minimal null checks; rely on exceptions to surface problems. Validate only at system boundaries (user input, external APIs)
18. Missing documentation comments
    • Public classes and complex methods should have summary comments

Project rules

Also apply the project's own coding rules from CLAUDE.md and AGENTS.md (read AGENTS.md if it exists).

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 the rules above, treating deterministic policy findings as already established
5. Merge deterministic findings with model findings in the output format above
6. 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. 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, then run qq-execute-checkpoint.py pipeline-advance --project . --completed-skill "/qq:best-practice" --next-skill "/qq:claude-code-review", then invoke /qq:claude-code-review --auto.