このエントリはUnity Advent Calendar 2015の6日目の記事になります。

前日のエントリは@hecomiさんのUnity で実行時にコードやコマンドを補完つきで実行できる uREPL を作ってみたでした。

デバッグがカッコ良くなるのもイケてますし、用途としてはコードをランタイムで実行出来るのがとても便利ですね！

さて、僕の方は『UnityProject内でのコードレビューとComponent設計をする時に気をつけてる事』について書いてきます。

コードレビューやコードを書く時の実体験に基づくあるあるネタを書きましたので、少しでも共感があれば幸いです＞＜

アジェンダ

前置き
コードレビューの時に気をつけてる事
UnityProjectのコードレビュー時に困る事
設計の時に気をつけてる事
さいごの感想
- 次の方の紹介

前置き

この記事内でのComponentとはUnityEngine.MonoBehaviorを継承したcsharpのclassの事を指します
コードレビューはgithubを使用して行っています

コードレビューの時に気をつけてる事

以下につらつらと書いてみました。少しだけピックアップして列挙した項目を説明していきます。

そもそものpull−requestの作り方など
汎用性があるか無いかによってディレクトリとclass名に気を使って欲しい
メモリ管理
null代入しないといけないとか
マジックナンバー
Componentの役割が明確か？
一つのComponentに機能がつまりすぎて無いか？
- ComponentのSetup関数は包有してるComponentの数だけ行数が多くなるので関数やComponentに分割したい
アニメーション処理などの座標計算処理に意味を持たせられてるか？
- 関数名orコメントで説明するべき
車輪の再開発をしていないか
- 『これと同じ機能は昔書いたよ』というやりとりがたまにある

汎用性があるかor無いか

汎用性があれば汎用的なComponent名にしてディレクトリも分けて欲しい
汎用的でなければSceneや機能に適した命名にして欲しい

理由

汎用性があれば汎用的なComponent名にしてほしい

汎用的であるComponentは再利用される事を想定して作っています。

車輪の再開発にならない為に、チーム内で汎用Componet専用のディレクトリを作成した方が良いです。

(同じ開発チーム内で同じ機能のComponentを作ってしまい、それをコードレビューで気づく時がたまにあったので。。。)

僕は汎用Componentを作成したら他メンバーへ周知する様にしています。

汎用的で無ければSceneなどに特化した命名にすべき

これをしないと、ComponentのAutherはゲームのTOP画面でしか使う想定が無かったとしても、他の画面で別の開発者に使われてしまう可能性があるので。

(もちろんディレクトリ配置も気を使うべき)

アニメーション処理などの座標計算処理に意味を持たせられてるか？

そもそもコードレビューでアニメーションをレビューするのか？という問題がありそうですが、個人的にはコードだけ見て分かり辛い処理は気持ち悪いので説明を求めたりしちゃいます><

以下の様なコードが特に関数にまとめられてなくコメントもなく書かれてると何してるのか分からり辛い。

public voi Setup()
{
    // Componentの設定処理が書かれる

    HOTween.To(transform, 0.3f, new TweenParms().Prop("localPosition", new Vector3(1.2f, 2.5f, 3.4f)).Delay(0.1f));
    yield return new WaitForSeconds(0.3f);
    HOTween.To(transform, 1.1f, new TweenParms().Prop("localPosition", new Vector3(10.2f, 22.5f, 33.4f)));
}
......途中に上記の様なTweenアニメーションの処理がつらつらと書かれてる

こういったコードを見ると、『コメントでアニメーションを説明して！』とか『アニメーションを説明する関数にこの処理書いて！』とか言います。

一つのComponentに機能がつまりすぎて無いか？

Componentの良いところは他の画面で再利用出来る様に作ると開発が楽になる事ですよね。

なので、Componentは基本的に最小単位で実装するに限ると思っています。

そもそも500行とか超えたclassがそこらにあると、僕は読む時に疲れちゃいます（ヽ´ω｀）

(ライブラリやUtility系のclassなら許せる)

こういうのとか見ると辛いなぁと思ってしまいます。

(僕はこの2倍のfieldが定義されてるclassを見た事があるので、以下は許容範囲かもとは思いますが)

public class HogeSceneUI : MonoBehaviour
{
    [SerializeField]
    private Label _coinNumberLabel;

    [SerializeField]
    private Sprite _coinIconSprite;

    [SerializeField]
    private Label _enemyNameLabel;

    [SerializeField]
    private Sprite _enemyIconSprite;

    [SerializeField]
    private Label _playerNameLabel;

    [SerializeField]
    private Sprite _playerIconSprite;

    [SerializeField]
    private Button _okButton;

    [SerializeField]
    private Label _okButtonLabel;

    [SerializeField]
    private CheckBox _checkBox;

    [SerializeField]
    private Label _checkBoxCaptionLabel;
}

こんな感じにIconのComponent作っちゃおうとか

public class CoinIcon : MonoBehaviour
{
    [SerializeField]
    private Label _coinNumberLabel;

    [SerializeField]
    private Sprite _coinIconSprite;
}

public class EnemyIcon : MonoBehaviour
{
    [SerializeField]
    private Label _enemyNameLabel;

    [SerializeField]
    private Sprite _enemyIconSprite;
}

public class PlayerIcon : MonoBehaviour
{
    [SerializeField]
    private Label _playerNameLabel;

    [SerializeField]
    private Sprite _playerIconSprite;
}

public class HogeSceneUI : MonoBehaviour
{
    [SerializeField]
    private CoinIcon _coinIcon;

    [SerializeField]
    private EnemyIcon _enemyIcon;

    [SerializeField]
    private PlayerIcon _playerIcon;

    [SerializeField]
    private Button _okButton;

    [SerializeField]
    private Label _okButtonLabel;

    [SerializeField]
    private CheckBox _checkBox;

    [SerializeField]
    private Label _checkBoxCaptionLabel;
}

そもそもUIにはヘッダーとフッターがあるからそれぞれのComponetを作ろう！とか

public class HogeSceneHeaderUI : MonoBehaviour
{
    [SerializeField]
    private CoinIcon _coinIcon;

    [SerializeField]
    private EnemyIcon _enemyIcon;

    [SerializeField]
    private PlayerIcon _playerIcon;
}

public class HogeSceneFooterUI : MonoBehaviour
{
    [SerializeField]
    private Button _okButton;

    [SerializeField]
    private Label _okButtonLabel;

    [SerializeField]
    private CheckBox _checkBox;

    [SerializeField]
    private Label _checkBoxCaptionLabel;
}

public class HogeSceneUI : MonoBehaviour
{
    [SerializeField]
    private HogeSceneHeaderUI _header;

    [SerializeField]
    private HogeSceneFooterUI _footer;
}

UnityProjectのコードレビュー時に困る事

SceneのHierarchyが見えない
- Hierarchyの構造とComponentの親子関係に矛盾があっても気付けない(個人的になBroadcastMessageの様な機能があるのでHierarchyの構造は整理整頓されてあるべきだと思ってる)
- レビューに渡されたコード以外のComponentがGameObectに与えてる振る舞いをコードを見るだけでは検知できない
  - GUIDを目で見れば良いけどそれならUnityでScene開いちゃった方が確実に早い
大きいpull−requestを作ってしまう人が多い気がする

Hierarchyの構造とComponentの親子関係に矛盾があっても気付けない

以下の様な構成になってるHierarchyを見るとモヤモヤする。 UIControllerの中にCoinやTitleLabelなどのUIのGameObjectを入れて管理すれば良いと思う。

HogeScene
 - UIController
  - 色々なUIを表現する為のGameObject達
 - Coin
 - TitleLabel

大きいpull−requestを作ってしまう人が多い気がする

ゲームの新規開発の場合だと、一つScene の画面を完全に作りきってからpull−requestのレビュー依頼が来る時が辛い。ただでさえコードから画面の振る舞いなどを想定してレビューするのが難しいのに...ぐぬぬ！！

個人的にUnityでの作業では、以下の様な粒度で細かくpull−requestを作って作業できると良さそうだと思ってます。 (辛い時には僕も気づくとたくさんの作業を一つのbranchでやってしまいますが。。。)

デザインを再現したSceneのみ(Hierarchyの設計も含む)
Componentのinterfaceのみ設計(開発に慣れてない人はおすすめ)
Componentの実装
画面の演出などをブラッシュアップ

以下は各工程の軽い説明です

デザインを再現したSceneのみ(Hierarchyの設計も含む)

個人的にはSceneのレビューは手間がかかるので、縛りの強いRailを敷きたい感じが最近はしております。 (レビューしなくても最低限の品質が保たれるように)

開発チーム内で決めたSceneの構成になってるかをチェック
drawcallの無駄遣いをしてないか？なども見る
レビューの度に作業branchをcheckoutして見るのはシンドイのでチームに入りたての人のみ見たりしてる

Componentのinterfaceのみ設計(開発に慣れてない人はおすすめ)

主に実装完了してからのレビュワーによるチャブ台返しへの対策。トータルの開発時間の節約にもなるし、レビュワーは段階を踏んで学びを得られそう。

開発経験が浅いorチームの開発ルールに慣れてない人が対象
Componentが持つ変数・関数・他Component間のデータの流れなどを評価

設計の時に気をつけてる事

今まで書いた事で設計の時に考える事が大体網羅されてしまったので新しいネタを思い出してました...

僕がSceneの実装前に考えてる事は以下の事です。

他の画面と共通要素になりそうな箇所をデザイナーと相談しよう
Hierarchyの構造を設計しよう。
とりあえず空のGameObjectでもいいから置いておく
Hierarchyの設計をしたらComponent設計を始めよう
Hierarchyの構造が決まった後だと画面を構成する部品を頭の中で分解した後なので、役割分担を想像しやすく設計しやすい

特にゲームの開発初期にはとても時間をかけていた気がします。

汎用Componentや拡張性を考慮したコードをプロジェクトの初期に書いておくと、後でリリース前の辛い時にコードを書いても品質がある程度守られるからです。

さいごの感想

Component設計やコードレビューの話を書いてみましたが、正直持論的な部分があるのでもっと本を読まないとな...と思いました。

今読んでる途中のCodeComplete(上)には設計的なノウハウのエッセンスが多く書いてあるので勉強になります。

(薦められてから1年経ちますがまだ読了出来ておりません。。。)

また、他のUnityエンジニアの方が仕事でどのようにコードを書いているのか意外と知らないので気になりました。

次の方の紹介

さて、次の12/7のエントリはLINQおじさんこと @RyotaMurohoshi さんによる『Androidネイティヴプラグインを作る際の注意点！』という題材で書いてくださいます。

今回LINQネタは出ないでしょうが、きっと面白い内容になると思うので楽しみです！

p_chinのおっぱいブログ

UnityとPerlなど

UnityProject内でのコードレビューとComponent設計をする時に気をつけてる事