スマホゲームのコードレビュー文化
Graffity エンジニアのCovaです。
近年（といってもソシャゲ黎明期からだいぶ経ちますが）、ゲーム業界にもコードレビュー文化がだいぶ入ってきました。元々はWeb系の文脈ですが、オンラインRPGのような「運用系コンテンツ」がスマホゲームにも増えてきているため「継続的な改善」が求められることが非常に多くなりました。
コードレビューの歴史的背景
古いコンシューマー系の開発だととにかく動くことが正義という思想が強く「早く」「動く」ことが第一で、コードの品質は二の次と考える人が少なくありません。これは「マスターアップ」がゴールなため、「マスターアップ」の「期日」に間に合わせることが大事で、マスターアップが終わればそこで終了という制作フローの都合から出てきていると思われます。
しかし、最近のコンシューマーゲームも「0day パッチ」や「シーズンパス」「DLC」などの有償/無償の「アップデートコンテンツ」など、ゲームリリース後も継続的にコンテンツ制作を続けることもあり、「運用系コンテンツ」の文脈に近くなってきました。
一方スマホゲーム（ソシャゲ、といってもソーシャル要素は近年かなり減ってきていますが）の場合、Web系の企業が作ってきたという経緯から「Web系の技術」がゲーム制作現場に入ってきました。また、ほとんどが「運用コンテンツ」と言われるオンラインゲームの系譜を受け継いでいるため、継続的にコンテンツをリリース、改善ということが求められます。そのため継続的インテグレーション(CI/CD) が非常に重要になってきます。
さて、運用を続けてると新しい仕様が古い仕様とぶつかる、古い仕様のせいで新規機能の作成コストが非常に高くなるという問題が経験則的に知られています。特に昔のコードで「動いているものには手を加えてはならない」みたいなルールがある場合「古いやり方を強制させられる」「新しいやりかたとぶつかる」など様々な障害にしかなりません。このようなルールの場合、結果として大盛りのスパゲティコードが量産されるだけで、運用コストが跳ね上がります。
そのため、継続的にリファクタリングを行い「古いものは新しく」、しかし「バグは出さない」ようにするためにコードレビュー文化が非常に重要になります。
コードレビューの問題点
コードレビューをいきなり行おうとすると様々な壁に激突します。
1. 面倒くさい
2. 何書けばいいのかわからない
3. レビュワーのコスト（コードレビューおじさんの発生)
これらの問題に対して以下、弊社なりのアプローチ方法を紹介します
1. コードレビュー面倒問題
回答はシンプルで「説明」と「強制力」です。
前者はチームメンバーになぜコードレビューを行うのか、コードレビューを行うとどういう恩恵があるのかをメンバーが理解できる言葉で説明します。
チームメンバーのレベルに依存しますが、基本的には一番下のレベルの人でも理解できるように説明しましょう。そのため説明に小難しい技術用語とかはなるべく用いずに行うことが望ましいです。
後者の強制力は「main(master) or 開発ブランチへの直PUSHの禁止」「一人以上のApproveをもらわないとマージ不可」を設定しています。
理由としては「異物混入防止」「レビュー文化の習慣づけ」です。
異物混入とありますが、以前別の現場で働いていたときにとある方が「別のUnityバージョンで勝手に検証＆直Push」されたせいで、その方以外全員の環境がエラーで動かないという開発上深刻な問題が起きました。また、次のバージョンのリソースが間違って入ることによるネタバレ防止の意味合いでも非常に重要な設定です。
コードレビューは「文化・習慣」にしないと真っ先に忘れ去られ、直Push祭りになり、上記のような問題が起きます。そういう意味でも設定で強制的に行うようにして習慣化、その次にチームの文化に昇華して初めて「継続的」になります。
2. 何書けばいいのかわからない問題
チームメンバーに初心者がいる場合「何を書いていいのかわからない」からPullRequest(PR)をあげづらい問題があります。またレビュワー視点でも真っ白で何も説明が書かれてないPRがあげられても何を中心に見てあげればいいのかわからず「レビューコストが高い」問題もあります。
Unityを用いたゲーム開発では特に、画像リソースやPrefab, .meta ファイルなど、非プログラムコードもPRに含まれるため、UIの機能実装一つで300 File changed なんてのが普通にあります。いくらフィルタがあるからとはいえ、レビュワーも色々チェックするのが大変なため「予め見て欲しい部分」は伝えてもらいたいものです。
そこでGithub のPRテンプレート機能を使います。
と、ここまではWeb系の技術ブログで載っているよくある話です。
ポイントはここからでテンプレートの中身が重要です。
1. What/Why/Howが端的に伝わる
2. 見やすい
3. 書きやすい
を意識してテンプレートを用意しています。
結論からいうとこのようにしています
ポイントは説明をHTMLコメントで記載していることで、ベタ書きだと説明のテキストがそのままPRの説明に入ってしまいます。このようにしておくことで表示の時には消えるため以下のように見た目が綺麗になります。
また、ゲーム開発特有のポイントとしては
1. 進行管理上、何のタスクと紐づいているかを明示的に記入する
2. 機能実装/修正チェック用のチェックシートのリンクを必須にする
ようにしています。特に後者ですが「コードレビューは動作確認ではない」ことと、初心者特有の「動作チェックの甘さ」をチェックするために必須にしています。
レビュワーはコード的な問題とチェックシートの不備を基本的にレビューします。動作確認は「実装した本人が責任を持って行う」ようにすることで、責務を明確化しています。
3. レビュワーのコスト（レビューおじさんの発生)問題
コードレビューを行うことの理由としては「知識の共有」「俗人化の防止」が主な理由です。（動作チェック・バグチェックは本質ではありません）
そのため基本的には全員が目を通すようにしています。
その上で「実装方法の意図がわからない」「処理内容がよくわからない」などがあれば「質問をする」ことで参加するように促しています。
また、「良いところがあればイイねアピールをする」ことを強く推奨しています。
これが非常に重要で指摘ばかりのレビューは初心者目線では参加ハードルが非常に高いです。またギスりがちになるためギスるくらいならレビューを止めるとなると負の循環に入りと二度とレビュー文化は醸成されません。
そのため「気軽にレビューに参加できるように敷居を下げる」ことが特に重要で、この敷居が下がれば下がるほどレビュワーの数を増やせるため結果としてレビュー専業になってしまう人、通称コードレビューおじさんの発生を抑えられることにつながります。
もちろん、高度な知識がレビューに求められる場合は人を指定してレビュワーになってもらうこともありますが、とにかく本質である「知識の共有」「俗人化の防止」を忘れないことが大切です。
まとめ
コードレビューは文化であり継続することが大事です。
多くのチームメンバーが参加しやすくするためにも「見やすい」
「書きやすい」「敷居を下げる」を意識して行うのが大切です。
・・・
そんなGraffityについて
https://wantedly.com/companies/graffity
スマホ用ARゲーム開発を手伝ってくれるUnityエンジニア、サーバーエンジニアなど開発仲間も募集中！
この記事が気に入ったらサポートをしてみませんか？