クラスターの開発改善活動~Unity屋さんとコードレビュー~

序文

こんにちは、クラスター株式会社でソフトウェアエンジニアをやっている獏星(ばくすたー)です!

さっそくですが、クラスターは3D空間を扱うメタバースを開発しており、Unity Engineを基盤としています。社内には20~30人程度のUnity Engineerが在籍しています。

また、開発チームは機能ごとの職能横断チームに分かれています。
たとえばユーザーどうしがつながる機能を提供するチーム(social team)はUnity Engineerだけではなく、Server、Web、iOS、Androidネイティブの各エンジニアが所属しています。

そのような人数規模とチームの構成スタイルのもと、クラスターでは基本的に単一のコード基盤をメンテナンスしています。
Unityの場合、クライアントコードがほぼ単一のUnityプロジェクトに集約されている、とも言い換えられます。

本記事では、そうした体制のもとコードメンテナンスをするうえでの開発改善活動を紹介します。
ゲームエンジン特有の話題は少ないため、チーム開発という文脈で広く参考になると思います。

なお、Unityの具体的な開発・実装イメージを持ちたい方にはこちらもオススメです。

tech-blog.cluster.mu

Unity屋さん

さて、タイトルにUnity屋さんという謎のワードがありましたが序文では完全に無視していました。
いったい何者でしょうか。

「Unity屋さん」はUnity Engineerのなかでも少人数(4人)がメンバーのグループで、Unityコードのメンテナンス性や信頼性を維持・改善することを目標としています。正式名称は「Unity Working Group (Unity WG)」ですが「Unity屋さん」のほうが通りがよいため、もっぱらUnity屋さんと呼ばれています。

Unity屋さんの主な仕事はこんな感じです。

  • リファクタリングをやっておく需要が高いコードをリファクタリングする
    • 小規模なものから大規模なのまで色々含みます
  • 使っていないコードやアセットを粉砕(※)する
  • 社内的に価値の高いデバッグ機能を導入
  • テストコードやコードレビューの状況を改善する

※「粉砕」はクラスターの社内用語で、良い意味で何かを廃止・削除することを意味します。

「~する」と書いていますが、Unity屋さんのメンバーが手を動かすケースに加え、ほかのUnity Engineerに依頼することもあります。

参考として、Unity屋さんの仕事ではないものも例示しておきます。

  • FPS、メモリ使用量、クラッシュ頻度など、動くアプリの健康性に関するメトリクスを改善する
    • これはコードメンテナンスとは異なる軸を持つので、分けて位置づけられます。
  • 基盤的なUIやアプリケーション挙動の改善
    • これは通常ユーザーに見える変更なので、機能開発としてPM (プロダクトマネージャー)と話しながら開発が進みます。
  • ビルド処理の最適化
    • ビルドについてはCCIWG (Client CI Working Group)というグループが別途取り組んでいます。

歴史的経緯も補足しておくと、「FPSやメモリ使用量など、動くアプリの健康性」はかつてUnity屋さん相当のグループの活動内容に含まれていました。つまり「Unityのプロジェクトをいい感じにする」みたいなグループでした。
現在はエンジニアの人数やアプリケーション規模の拡大を踏まえて、アプリ健康性は「Unity健康」という(体に良さそうな名前の)別のグループで改善に取り組んでいます。

実際にはコード基盤のメンテナンス性と機能開発やパフォーマンスは関連しがちなので、境界が曖昧だったり、管轄外に見えるタスクを拾うこともあります。

この記事で取り上げる活動例:コードレビュー

本記事ではUnity屋さんの活動のなかでもコードレビューに関する施策を一部紹介してみます。
施策の背景となる2つの前提を抑えておきます。

  • クラスターは継続的に組織拡大している多くの企業と同様に、入社したての新規メンバーからプロジェクトの全体像が分かるシニアエンジニアまでが広く在籍しています。
  • 開発業務の優先度として「コードレビューはコーディングより優先」ということが共有されています。

施策1: レビューしてほしいとき用のslack絵文字の導入

レビューに関する用語をslackの絵文字でいくつか導入しました。

それぞれ下記の意味があります。

  • レ願 (れがん/れねが) = レビューお願いします!
  • レ助 (れたす) = レビューしようとしたけど難しいので誰か助けて下さい!
  • どんマプリーズ = どんどんマージ(どんマ)できる簡単なPRなのでレビュープリーズ

ちなみに「レ助」についてはこういう4コマがあります。

note.com

絵文字があるとそういう依頼は普通にやってよい事であると明示でき、ルールを共有しやすくなります。

「レ願」の実施例: 早めにマージしたいPRに対して使われたりします。

デメリットとしては入社直後のメンバーから「この「レ願」ってなんですか??」みたいなフィードバックも貰ってます。要するに初見だと分からないと。当然ですね。

とはいえ短縮表記で絵文字があることのメリットも簡単には捨てたくないので、どうしようかと悩んでいるところです…。

施策2: 一部コードのレビュアー制限

Unityのコード/アセット基盤のなかでも、うっかり触ると壊れやすい場所がいくつかあります。

  • サードパーティのライブラリ/アセットを追加/更新/削除する
  • プロジェクト全体の設定を書き換える: 描画の基本設定など

こうした変更を含むPRについて、「Unity Engineerなら誰でもレビュー可」だったルールを厳しめに変更し、シニア層エンジニアのコードレビューを必須にしました。このルール化はGitHubのCODEOWNERSを設定して実現しています。

レビュアーを制限することで、互換性の問題とか「エイヤで変更して大丈夫かな…」といった議論の機会が確保しやすくなっています。

それ以外でも、PRを作った人に対して「なんか普段と違うレビュアーが割り当たったぞ!」という緊張感を与えるシグナルとしても役立ったり、意図せずに変更したファイルに気づいて変更を巻き戻せたり、といったメリットがあります。

施策3: 「PRの様子を見る(レビューのレビュー)」という活動の文面化

あらためて背景の確認ですが、コードレビューはクラスターの開発業務のなかでも優先度が高いものと位置づけられています。
この指針は開発文化を支える良いものですが、ストレートに運用すると以下のような課題も発生します。

  • 事象として、コードの全体像と業務優先度を把握したシニア層のエンジニアほどコードレビューする…ということが起きやすい
  • その結果として、
    • シニア層については
      • コードレビューの負荷が上がる
      • 新規メンバーがコードを読む機会が減り、レビューの負荷が分散しにくい
    • 新規メンバーについては
      • 前職 / 在学時にコードレビュー習慣が未定着だった場合、シニアエンジニアだけでコードレビューが回るので、レビューする習慣が定着しにくい
      • コードを読む機会が減るため、コードの全体像が分からないままになりやすい

この課題への改善案として、シニア層のUnity Engineerに対して次のような行動規範を提案しました。

  • 状況次第ではコードレビューの頻度を抑える
  • レビューしてほしいと宣言されたPRは積極的にレビューする (= 困ってる人は助ける)
  • Unity Engineer全体でコードレビューが滞っていたら、シニア層が責任を持って解決する
  • 直接的なレビュー以外に、他メンバーのレビューの進行状況や観点を見たりする = レビューのレビューを自発的にやる

レビューのレビューをした結果はUnity屋さんの週次ミーティングで「こんな問題があった」と議題に挙げたり、内容次第ではPRのDiscussionにリアルタイムで混ざって「これ技術的負債っぽいけど大丈夫ですか?」みたいな指摘をしたりします。

この施策の開始直後はレビュー待ちのPRが溜まりやすくなって「みんなレビュー頑張って~!!」と呼びかける事もしばしばでしたが、数ヶ月を経て現在はいい感じにレビューが回るようになっています。

ただしコードレビューに積極的な人とそうでない人の個人差はまだあるため、追加でいい感じの施策はないかと考えています。

施策4: コード規約チェックを人力レビューからRoslyn Analyzerに移行する

最後はちょっと毛色の違う話題を挙げておきます。

クラスターのUnityコードにはそれなりに分量のあるコード規約が伴っています。規約は命名規則のようにシンプルかつEditorConfigで表現できるものから、設計の構造に関するガイドラインまで様々です。

このコード規約のなかでも中程度の複雑度、すなわち「IDEの標準機能で解析できないが自前の構文解析をすればチェックできる」という水準のコード規約について、人力でのコードレビューを自動チェックに移行する取り組みを進めています。

Unityのコード基盤はC#であるため、素直にRoslyn Analyzerを使っています。手始めに「DIフレームワーク(Zenject)で依存注入するクラスのコンストラクタが特定の書き方になっているかどうかをチェックし、ルール違反ならばエラーにする」という静的解析が導入済みで、現在はさらに適用範囲を拡大しようとしています。

コード規約やコード分析の紹介はそれ単体で1記事にできそうなトピックなので、別記事での詳解をお待ちください。

さいごに

今回はUnity屋さんという開発改善活動のグループがあることの紹介、およびその活動の一環としてコードレビュー関連の施策を紹介しました。

クラスターではUnity Engineerをはじめ、機能開発と開発環境の改善の双方に興味のあるエンジニアを絶賛募集中です!

recruit.cluster.mu