game-code-review

AlterLab GameForge -- ゲームコードレビューのワークフロー

ゲームコードには、Webコードにはない失敗要因があります。リクエストハンドラーでメモリを割り当てるWebアプリでは、多少のレイテンシが発生する程度です。一方、更新ループでメモリを割り当てるゲームでは、プレイヤーが手で感じるほどのフレーム落ちが発生します。状態管理に問題があるWebアプリでは、古いデータが表示されます。ゲームで状態管理に問題があると、プレイヤーが壁をすり抜けたり、見えない弾を撃ったりします。Celesteのコードベースが伝説的なのは、Maddy ThorsonとNoel Berryがフレーム単位で正確な入力処理を、最優先のエンジニアリング課題として扱ったからです。その結果、これまでにリリースされた中で最も操作性の高いプラットフォーマーが生まれました。

このワークフローでは、標準的なソフトウェア品質（命名、構造、テスト、ドキュメント）とゲーム固有の正しさ（フレームレートからの独立性、ホットパスのパフォーマンス、ステートマシンの整合性、リソースのライフサイクル）という2つの視点から、ゲームコードを同時にレビューします。どちらの視点も重要です。Wube Softwareがアーキテクチャをゲームプレイ機能として扱っているからこそ、Factorioは60fpsで100万個のエンティティをシミュレーションできます。どちらか一方の視点を無視すると、構造は整っていても実行時に壊れるコードか、高速でも保守できないコードになります。

目的と実行タイミング

次のような場合に、このワークフローを使用してください。

• 開発者から「ゲームコードをレビューして」「アーキテクチャを確認して」と依頼されたとき
• 重要な機能ブランチをマージする前
• パフォーマンスの問題をデバッグしていて、構造に原因があると考えられるとき
• 新しい開発者が参加し、コード品質の基準を把握する必要があるとき
• プロトタイプの作成後、どのコードを本番環境に移行するか決めるとき
• プロトタイプから本番品質のコードベースへ移行するとき

このワークフローで解決する問題：

• 誰も気づかなかったフレームごとのメモリ割り当てによるフレームレートの低下
• 30fps、60fps、144fpsで挙動が異なる物理演算
• 端の条件で不可能な状態に陥るステートマシン
• シーン遷移時にリソースが解放されず、徐々にメモリが増加する問題
• 調整のたびに再コンパイルが必要な、ハードコードされたゲームプレイの値
• 各システムを個別にテストできなくする循環依存
• 抽象化されず、ゲームプレイコード内に散在する生の入力処理

重要なルール

1. まずゲーム固有のチェックを行う。 標準的なコード品質も重要ですが、ゲーム固有の問題は診断がより困難な不具合につながります。命名規則違反は厄介です。一方、フレームレートに依存する物理演算は、開発者のマシン以外のあらゆるハードウェア構成でリリース後のバグになります。

2. 状況を考慮して重大度を判断する。 メニュー画面のハンドラーでの new による割り当ては問題ありません。同じ割り当てでも、毎フレーム実行されるパーティクル更新処理では、GC に大きな負荷をかける重大な要因になります。重大度を評価する前に、コードがどこで実行されるかを必ず考慮してください。

3. 推測ではなく、計測する。 根拠のない仮説上のパフォーマンス問題を指摘しないでください。処理が重そうな場合は、「潜在的な懸念事項――最適化の前にプロファイリングしてください」と記してください。早すぎる最適化は実際に問題となります。早すぎる最適化への不安も同様です。

4. エンジンごとの慣習を尊重する。 各エンジンにはそれぞれの慣習があります。Godot のシグナルは Unity のイベントとは異なり、Unreal のデリゲートとも異なります。抽象的な理想ではなく、使用するエンジンの慣習に照らしてコードをレビューしてください。Noita のピクセル物理システムが機能しているのは、Nolla Games がエンジンのアーキテクチャに逆らわず、その強みを生かす実装をしたからです。エンジン固有の基準については、該当するエンジン専門のスキルを参照してください。

5. ゲームプレイの値はコードではなくデータに置く。 デザイナーが調整したいと考える可能性のある数値（ダメージ、速度、クールダウン、出現率、ドロップ率）は、ソースコード内の定数ではなく、設定ファイルまたはデータテーブルに置く必要があります。これは譲れない要件です。docs/collaboration-protocol.md に記載されている、ゲームアーキテクチャの基本要件です。

6. テスト困難なものもテストする。 ゲームシステムはユニットテストが非常に難しいことで知られています。それでも、テスト可能なアーキテクチャを目指してください。エンジンとの密結合が原因でシステムをテストできない場合、その結合は指摘すべき設計上の問題です。Factorio の開発者は、ゲームロジックをレンダリングから分離することでシミュレーション層に包括的な自動テストを維持しています。この分離があるからこそ、自信を持ってリリースできるのです。

ワークフロー

Step 1：アーキテクチャの概要

個々のファイルを読む前に、コードベースの全体像を把握します。

• 主要なシステム（レンダリング、物理、ゲームプレイ、AI、UI、オーディオ、入力、ネットワーク）を整理する
• システム間の依存関係を特定する
• 明確なアーキテクチャパターン（ECS、コンポーネントベース、シーンツリー、MVC の派生形）があるか確認する
• ゲームプレイとエンジンの間に抽象化レイヤーがあるかどうかを記録する

ARCHITECTURE MAP TEMPLATE
-------------------------------------------------
System            | Location          | Dependencies       | Abstracted?
-------------------------------------------------
Game Loop / Core  | [path]            | [all systems]      | Y/N
Player Controller | [path]            | Input, Physics     | Y/N
Enemy AI          | [path]            | Pathfinding, State | Y/N
UI System         | [path]            | Game State         | Y/N
Audio Manager     | [path]            | Events             | Y/N
Save System       | [path]            | Serialization      | Y/N
-------------------------------------------------

Step 2：ゲーム固有のコードチェック

これらは、ゲームのコードレビューを一般的なコードレビューと区別するチェック項目です。 各チェックでは、リアルタイムのインタラクティブソフトウェアに固有の障害パターンを対象とします。

Check 2.1 -- デルタタイムの使用

時間に依存する計算にはすべて、デルタタイム（前のフレームからの経過時間）を使用する必要があります。デルタタイムを使用しないと、ゲームの動作がフレームレートによって変わります。

DELTA TIME AUDIT
-------------------------------------------------
FIND: Any += or -= in update/process/tick functions without * delta
FLAG: position += speed (WRONG: frame-rate dependent)
FIX:  position += speed * delta_time (CORRECT: frame-rate independent)

Also check:
- Timers that count frames instead of elapsed time
- Animation speed tied to frame rate
- Cooldowns that expire faster on faster machines
- Velocity calculations that assume fixed frame rate

Severity: CRITICAL when found in gameplay-affecting code
          MINOR when found in cosmetic-only code (particle trails, etc.)
-------------------------------------------------

Check 2.2 -- ホットパスでのメモリ割り当て

毎フレーム実行されるコードでは、ヒープメモリを割り当ててはいけません。ガベージコレクターが最終的に回収しますが、GC の一時停止によって目に見えるフレーム落ちが発生します。

HOT PATH ALLOCATION AUDIT
-------------------------------------------------
FIND: new, Array(), Object.create(), .push() growing arrays, string
      concatenation, closures capturing variables, lambda creation --
      in any function called per-frame

FLAG: Allocations in update(), _process(), _physics_process(), Tick(),
      FixedUpdate(), any function called from the main loop

FIX:  Object pooling, pre-allocated arrays, cached references,
      struct-based data (for engines that support it)

Also check:
- Dictionary/Map creation inside loops
- Temporary vector/matrix objects in physics calculations
- String formatting for debug output left in production code
- LINQ queries or functional chain calls in hot paths (C# / Unity)

Severity: CRITICAL in physics or rendering code
          MAJOR in gameplay update code
          MINOR in low-frequency systems (menu transitions, save games)
-------------------------------------------------

Check 2.3 -- 状態管理

ゲームオブジェクトは通常、複雑な状態（待機、走行、ジャンプ、攻撃、スタン、死亡中、死亡済み）を持ちます。状態管理の不備は、ゲームにおける視覚的な不具合やロジック上のバグの最大の原因です。

STATE MANAGEMENT AUDIT
-------------------------------------------------
FIND: Boolean flags controlling behavior (is_jumping, is_attacking, can_move)
FLAG: "Boolean soup" -- multiple booleans that create implicit state combinations,
      some of which are invalid (is_jumping AND is_dead should be impossible)

PREFER: Explicit state machines with defined transitions
  - Enum-based states with switch/match statements
  - State pattern with enter/exit/update per state
  - Hierarchical state machines for complex characters

Also check:
- Can the object enter an invalid state combination?
- Are state transitions guarded (can only go from A to B, not A to D)?
- Is state entry/exit cleanup handled (stop animation on state exit)?
- Are states visualizable for debugging (can you see current state in editor)?

Severity: CRITICAL when state bugs cause gameplay-breaking behavior
          MAJOR when state bugs cause visual glitches
          MINOR when state management works but is hard to extend
-------------------------------------------------

Check 2.4 -- フレームレートからの独立性

物理演算は固定タイムステップで実行する必要があります。レンダリングは可変レートで実行できます。これらを混在させると、ハードウェアによって動作が変わります。

FRAME-RATE INDEPENDENCE AUDIT
-------------------------------------------------
FIND: Physics calculations in variable-rate update functions
FLAG: Movement, collision, force application in Update() instead of
      FixedUpdate() (Unity) or _process() instead of _physics_process() (Godot)

Also check:
- Is the fixed timestep actually fixed, or is it accidentally variable?
- Are interpolation/extrapolation used for smooth rendering between physics steps?
- Do input-driven actions respect the physics timestep boundary?
- Are animation events synchronized with physics or rendering?

Severity: CRITICAL -- frame-rate dependent physics is always a shipped bug
-------------------------------------------------

Check 2.5 -- リソースのクリーンアップ

シーンの切り替え、レベルの読み込み、リスポーンは、メモリリークが発生しやすい箇所です。取得したリソースはすべて解放する必要があります。

RESOURCE CLEANUP AUDIT
-------------------------------------------------
CHECK: Scene/level transition code
- Are all dynamic objects properly freed/destroyed?
- Are event listeners/signals disconnected?
- Are audio sources stopped and released?
- Are particle systems stopped before their parent is destroyed?
- Are async operations cancelled or properly awaited?
- Are pooled objects returned to pools?

CHECK: Object lifecycle
- Do spawned objects have a corresponding despawn path?
- Are references cleared when objects are destroyed?
- Are weak references used where appropriate?

Severity: MAJOR -- resource leaks cause gradual performance degradation
          CRITICAL -- if leaks cause crashes during extended play sessions
-------------------------------------------------

Check 2.6 -- 依存関係の方向

システムは具体的な実装ではなく、抽象に依存する必要があります。循環依存があると、システムのテストや変更が困難になります。

DEPENDENCY DIRECTION AUDIT
-------------------------------------------------
CHECK: Do high-level systems (gameplay) depend on low-level systems (rendering)?
       This is fine. The reverse is a red flag.
CHECK: Do peer systems reference each other directly?
       Use events, signals, or a mediator pattern instead.
CHECK: Can you instantiate a system in a test without bringing up the entire game?
       If not, coupling is too tight.

FLAG: Circular dependencies (A depends on B depends on A)
FLAG: God objects that everything references (GameManager with 50 public fields)
FLAG: Direct references to singletons scattered throughout gameplay code

Severity: MAJOR -- architectural coupling compounds over time
-------------------------------------------------

Check 2.7 -- データ駆動の値

ゲームプレイの数値は、ソースコードではなくデータファイルに記述する必要があります。

DATA-DRIVEN AUDIT
-------------------------------------------------
FIND: Hardcoded numeric literals in gameplay code
FLAG: speed = 5.0, damage = 10, cooldown = 0.5, spawnRate = 3
FIX:  Load from JSON, XML, CSV, or engine-native resource files

Why this matters:
- Designers cannot tune values without programmer intervention
- A/B testing requires code changes instead of config changes
- Balancing requires recompilation instead of data edits
- Platform-specific tuning (mobile vs. desktop) becomes a code branch

Exception: Mathematical constants (PI, gravity constant) and engine
configuration (target FPS, physics substeps) can be hardcoded with
a comment explaining why.

Severity: MAJOR for player-facing values (damage, speed, etc.)
          MINOR for internal system values (pool sizes, buffer lengths)
-------------------------------------------------

Check 2.8 -- 入力処理

入力は、ゲームプレイコードで直接読み取るのではなく、抽象化レイヤーを通して処理する必要があります。

INPUT HANDLING AUDIT
-------------------------------------------------
FLAG: Direct keyboard/mouse/gamepad checks in gameplay code
      (Input.GetKey, Input.is_action_pressed without action mapping)
FLAG: Hardcoded key bindings (KeyCode.Space, KEY_W)
FLAG: Mouse/touch handling that assumes a specific screen resolution

PREFER:
- Input action maps that players can remap
- Input abstraction that translates raw input to game actions
- Context-sensitive input (same button does different things based on state)
- Input buffering for action games (queue inputs during animations)

Severity: MAJOR for shipping products (accessibility and user experience)
          MINOR for prototypes (acceptable to defer)
-------------------------------------------------

Check 2.9 -- 並行処理の安全性

ゲームでスレッド（オーディオ、ネットワーク、アセット読み込み）を使用する場合は、共有状態を保護する必要があります。

CONCURRENCY AUDIT
-------------------------------------------------
CHECK: Is game state accessed from multiple threads?
CHECK: Are async operations properly synchronized?
CHECK: Is there a clear thread ownership model (which thread owns which data)?
FLAG:  Shared mutable state without locks or atomic operations
FLAG:  Race conditions in network code (state update vs. render)
FLAG:  Callback chains that assume execution order

Severity: CRITICAL when concurrency bugs cause crashes or data corruption
          MAJOR when they cause visual glitches or desync
-------------------------------------------------

Check 2.10 -- AI 生成コードのアンチパターン

AI 支援によるコード生成が一般的になるにつれ、ゲームのコードベースには、特有の種類のバグを引き起こす AI 生成コードがますます含まれるようになっています。これらのアンチパターンは、一見もっともらしく見えながら、微妙に誤っていることが多いため、人間が書いたコードのバグとは異なります。

AI CODE ANTI-PATTERN AUDIT
-------------------------------------------------
CHECK 2.10.1 -- Hallucinated APIs:
  Verify that EVERY API call, method, property, and enum value referenced in
  the code actually exists in the current engine version. AI models frequently
  generate calls to APIs that existed in older versions, exist only in a
  different engine, or never existed at all.

  FLAG: Any API call that does not appear in the engine's official documentation
        for the project's engine version.
  FLAG: Method signatures that do not match the current API (wrong parameter
        count, wrong parameter types, wrong return type).
  FLAG: Enum values that do not exist in the current engine version.

  Severity: CRITICAL -- hallucinated APIs cause compile errors at best,
            silent runtime failures at worst.

CHECK 2.10.2 -- Engine-Specific Pattern Violations:
  AI models often generate code that follows generic programming patterns
  rather than engine-specific idioms. Check against `@docs/coding-standards.md`
  engine version table.

  FLAG: Using generic design patterns where the engine provides a built-in
        solution (e.g., hand-rolling an observer pattern in Godot when signals
        exist, implementing a custom coroutine system in Unity when UniTask or
        native coroutines are available).
  FLAG: Anti-patterns for the specific engine (e.g., using GetComponent() in
        Update() in Unity, using get_node() with long paths in Godot hot loops).
  FLAG: Version-specific API usage -- code written for engine version X running
        on engine version Y.

  Severity: MAJOR -- pattern violations cause maintenance burden and potential
            performance issues.

CHECK 2.10.3 -- License Compliance:
  AI-generated code can reproduce copyrighted patterns, licensed algorithms,
  or proprietary API wrappers from its training data.

  FLAG: Code blocks that appear to be direct reproductions of copyrighted
        source code (exact variable names, exact comment text, identical
        structure to known open-source implementations).
  FLAG: Algorithm implementations that match patented methods without license.
  FLAG: Middleware or SDK wrapper code that bypasses licensing requirements.

  Severity: MAJOR -- license violations create legal liability.

CHECK 2.10.4 -- Attribution:
  Document AI-assisted code sections per the project's AI usage policy.

  FLAG: Significant code blocks generated by AI without attribution comments
        indicating AI assistance.
  FLAG: Missing documentation of which AI tool was used, when, and what prompt
        produced the code.

  Follow the project's AI content policy (reference `@templates/ai-content-policy.md`
  if available). At minimum, add a comment indicating AI-assisted generation for
  any function or class substantially produced by AI tools.

  Severity: MINOR -- attribution is a process issue, not a runtime issue, but
            is important for audit trails and legal compliance.
-------------------------------------------------

Check 2.11 -- マジックナンバー

すべての数値定数には名前を付け、可能であれば、その値を選んだ設計上の理由を説明するコメントを付ける必要があります。

MAGIC NUMBER AUDIT
-------------------------------------------------
FLAG: Unnamed numeric literals in any context
BAD:  if velocity.y > 4.5:    # what is 4.5? why 4.5?
GOOD: if velocity.y > TERMINAL_VELOCITY:
      # TERMINAL_VELOCITY = 4.5 -- chosen to match the visual free-fall
      # animation length (0.6s at 60fps). Tuned via playtest session #3.

The comment matters as much as the name. "SPEED = 5" tells you the name.
"SPEED = 5 -- matched to feel responsive on 1920x1080 at 60fps, may need
 scaling for other resolutions" tells you the reasoning.

Severity: MINOR individually, but accumulated magic numbers become MAJOR
          (a codebase full of unexplained numbers is a codebase nobody can tune)
-------------------------------------------------

Step 3：標準的なコード品質チェック

ゲーム固有のチェックの後、標準的なソフトウェア品質を確認します。

• 命名規則：一貫していますか？エンジンの規約に従っていますか？
• コードの整理：ファイルやディレクトリは論理的に構成されていますか？
• エラー処理：失敗ケースは処理されていますか？それとも、何も通知されずにクラッシュしますか？
• ドキュメント：公開 API にコメントがありますか？複雑なアルゴリズムは説明されていますか？
• テストカバレッジ：ゲームプレイシステムはテストされていますか？テスト戦略はどのようなものですか？
• コードの重複：抽出すべき大幅なコピー＆ペーストコードはありませんか？
• デッドコード：未使用の関数、クラス、コメントアウトされたブロックはありませんか？

出力形式

# Game Code Review Report
**Project:** [name]
**Engine:** [detected engine]
**Reviewer:** GameForge Code Review
**Date:** [date]
**Files Reviewed:** [count]

## Summary
[2-3 sentences: overall code health, top concern, biggest strength]

## Game-Specific Findings

### Critical
- **[GS-CRIT-01]** [Title] -- [file:line]
  [Description of the issue, why it matters for gameplay, and fix suggestion]

### Major
- **[GS-MAJ-01]** [Title] -- [file:line]
  [Description and fix suggestion]

### Minor
- **[GS-MIN-01]** [Title] -- [file:line]
  [Description and fix suggestion]

## Standard Quality Findings
[Same severity structure for non-game-specific issues]

## Architecture Assessment
[Dependency map, coupling analysis, pattern evaluation]

## Positive Highlights
[What the code does well -- always acknowledge good work]

## Recommended Actions
1. [Highest priority fix with estimated effort]
2. [Second priority]
3. [Third priority]

品質基準

ゲームコードレビューが成功したといえるのは、次のすべてを満たす場合です。

• すべての指摘に、具体的なファイルと行の参照が含まれている
• スタイル上の懸念より、ゲーム固有のチェックが優先されている
• 重大度の評価が、理論上の純粋さではなく、実際のゲームプレイへの影響を反映している
• プロトタイプコード（簡略化が許容される）と本番コード（許容されない）を区別している
• パフォーマンス上の懸念は推測で決めつけず、「まずプロファイリング」として指摘している
• 言語一般のアドバイスではなく、エンジン固有の規約を適用している
• アーキテクチャ評価で、最も危険な結合箇所を特定している
• 良いプラクティスを定着させるため、優れたパターンも取り上げている
• 労力に対する効果の比率に基づいて、推奨事項に優先順位を付けている

MCP 統合

コードレビューのワークフローは、バージョン管理の統合とエンジン固有のリアルタイムコード分析のために MCP サーバーに接続します。これにより、実際の PR 差分を対象にレビューし、エンジンのドキュメントに照らしてコードをリアルタイムで検証できます。

接続済みの MCP サーバー

MCP サーバー	コードレビューでの用途	役立つ点
GitHub（接続済み）	PR 統合、差分分析	対象を絞ったレビューのために PR 差分を取得し、変更されたファイルを前後の文脈とともに読み、行を指定した参照付きでレビューコメントを投稿し、レビュー前に CI のステータスとテスト結果を確認し、ブランチ保護ルールが満たされているか確認します
Context7（接続済み）	エンジンのドキュメントの検証	幻覚による API を含むコードをレビューする際（チェック 2.10.1）、Context7 でエンジンの最新ドキュメントを検索し、すべての API 呼び出し、メソッド、列挙型がプロジェクトのエンジンバージョンに実在するか検証します
エンジン MCP（プロジェクトごとにインストール）	リアルタイムコード分析	適切なエンジン MCP に接続し、実行中のエディターでコードの動作を検証します。
- Coding-Solo/godot-mcp（2,616 stars）	Godot コードの検証	エディターで GDScript を実行してデルタタイムの挙動を検証し、シグナル接続をテストし、シーンツリーの構造を検証します
- CoplayDev/unity-mcp（7,540 stars）	Unity コードの検証	Unity エディターの状態を調査し、コンポーネント参照を検証し、アセットの依存関係を確認します
- chongdashu/unreal-mcp（1,637 stars）	Unreal コードの検証	Unreal Editor を操作して Blueprint の接続をテストし、C++ のコンパイルを検証し、アクター階層を調査します

ワークフローの例

PR ベースのコードレビュー：

1. GitHub MCP を使って PR 差分と変更されたファイルの一覧を取得します
2. Filesystem ツールを使って、変更された各ファイルを読み込みます
3. 変更されたコードに対して、ゲーム固有の 11 項目のチェック（デルタタイム、ホットパスでのアロケーション、状態管理など）をすべて実施します
4. Context7 を使って、不慣れな API 呼び出しがあれば、エンジンの最新ドキュメントに照らして検証します
5. インラインのコード参照と重大度の評価を添えて、レビュー結果を GitHub の PR コメントとして投稿します

エンジンを活用したリアルタイムレビュー：

1. プロジェクトに適したエンジン MCP（Godot、Unity、または Unreal）に接続します
2. パフォーマンスに懸念のあるコードについては、エンジン MCP を使ってエディターで実行し、フレームタイミングを確認します
3. 状態管理の問題については、エンジン MCP を使ってゲームプレイ中のオブジェクトの実行時状態を調査します
4. エンジンで検証した指摘を、静的解析の結果とともにレビュー報告書に記載します

使用例

1. 「プレイヤーコントローラーのコードをレビューしてください。」 デルタタイムの使用方法、状態管理、入力処理、フレームレートからの独立性に注目します。プレイヤーコントローラーで最も影響の大きい確認項目は、この4つです。

2. 「戦闘中にフレーム落ちが発生しています。原因を見つけてもらえますか？」 戦闘システムにおけるホットパスでのメモリ割り当て、オブジェクトプーリング、リソースのクリーンアップに注目します。指摘事項ごとに、プロファイリングの提案も添えます。

3. 「プロトタイプを本番環境に移行しようとしています。何を変更する必要がありますか？」 データ駆動型の値、依存関係の方向、リソースのクリーンアップを重視して、全体をレビューします。テストでは許容されていたプロトタイプの近道も、本番環境では技術的負債になります。

4. 「新しい開発者が加わります。まずコードを整理するのを手伝ってください。」 命名規則、ドキュメント、マジックナンバー、アーキテクチャの評価に注目します。目標は、新しいメンバーがコードベースを把握しやすくすることです。

5. 「チームメイトのコンピューターでは、ゲームの挙動が異なります。」 フレームレートからの独立性とデルタタイムが、まず疑うべき点です。ハードコードされた画面解像度、プラットフォーム固有の前提、タイミングに依存するロジックも確認します。