13年分の未熟なゲームのコード

1人きりの金曜日の夜、何らかのインスピレーションを求めていた私は、以前のプログラミングのいくつかを再現することに決めました。

昔のハードドライブがゆっくりと回転し、栄光の日のソースコードが表示されます。

しかし、蓋を開けてみると、全く期待していたものではありませんでした。”ここまでひどかったのか。なぜ誰も言ってくれなかったのだろう。なぜ自分はこんなにひどかったのだろう。1つの関数内に、これほど多くのgoto文をよく入れられたものだ。“ 私はすぐに自分の試みを諦めました。そして一も二もなく、コードの削除とハードディスクの完全消去を考え出しました。

以下は、過去の自分を見ることで得た教訓や断片、警告などをまとめたものです。過去の過ちをそのまま公開するためにも、名前などは変更していません。

2004年

私は13歳でした。このプロジェクトはRed Moonという、野性的で野心的なサードパーソン・ジェット戦闘ゲームです。『Developing Games in Java(Javaでゲーム開発)』から、そのままコピーしたもの以外のコードの中には、明らかにひどいと言わざるを得ないものも少なくありません。その一例を以下に挙げます。

私は、プレイヤーが複数の武器を切り替えられるようにしたいと思っていました。その方法は、プレイヤーモデル内で武器モデルを回転させ、次の武器に交換してから、再び回転して戻す、といったものです。以下にアニメーションのコードを記していますが、お手柔らかに見てください。

public void updateAnimation(long eTime) {
    if(group.getGroup("gun") == null) {
        group.addGroup((PolygonGroup)gun.clone());
    }
    changeTime -= eTime;
    if(changing && changeTime <= 0) {
        group.removeGroup("gun");
        group.addGroup((PolygonGroup)gun.clone());
        weaponGroup = group.getGroup("gun");
        weaponGroup.xform.velocityAngleX.set(.003f, 250);
        changing = false;
    }
}

ここで、おかしな点を2つ挙げたいと思います。1つ目は状態変数についてです。

  • changeTime
  • changing
  • weaponGroup
  • waponGroup.xform.velocityAngleX

これだけあっても、何かが欠けているように思えます。そう、どの武器が現在、装備されているかを追跡する変数が必要です。もちろん、それは全く別のファイルです。

もう1つのおかしな点は、複数の武器モデルを実際には作らなかったということです。全ての武器が同じモデルを使用していました。つまり、武器モデルの全てのコードは、無駄な作業でしかなかったのです。

改善策

冗長な変数を削除する。この場合、状態はweaponSwitchTimerとweaponCurrentの2つの変数でキャプチャでき、他は全て、この2つの変数から派生させることができます。

全てを明示的に初期化する。この関数は、武器がNullかどうかをチェックし、必要に応じて武器を初期化します。30秒も考えれば、このゲームではプレイヤーが常に武器を持つことが明らかで、持たない場合、ゲームはプレイできないか、プレイできたとしてもすぐに終わってしまうでしょう。

ある時点において、この関数でNullPointerExceptionに遭遇しましたが、それが起こった理由を考えるのではなく、Nullチェックをスローし、先に進みました。実際のところ、武器を扱う関数のほとんどには、このようなチェックがあります。

先回りして、事前に意思決定をすることが大事です。その決定をコンピュータに任せないようにしましょう。

命名

boolean noenemies = true; // why oh why

ブーリアン値の名前を確実に指定します。このようなコードを書いている場合は、考え直しましょう。

if (!noenemies) {
    // are there enemies or not??
}

エラー処理

このようなスニペットは、コードベース全体に散見されます。

static {
    try {
        gun = Resources.parseModel("images/gun.txt");
    } catch (FileNotFoundException e) {} // *shrug*
    catch (IOException e) {}
}

“エラーの処理はより手際よく。メッセージをユーザに表示したりすればいい”という考えを持つ人もいるかもしれませんが、私の考えは実はその反対です。

エラーチェックを十二分に行うことは決してできませんが、エラー処理を十二分に行うことならできます。この場合、ゲームは武器モデルなしではプレイできないため、私ならクラッシュさせるでしょう。回復不能なエラーからは、正常に回復しようとしない方がいいと思います。

繰り返しになりますが、こうしたことは、どのエラーを回復可能にするかということに関して、事前の決定がものを言います。残念ながらSunは、ほとんど全てのJavaエラーを回復すべきという決定を下しました。それが結果的に、上記のようなダラダラしたエラー処理の元凶になっています。

2005-2006

この年代に、私はC++とDirectXを学びました。そして、再利用可能なエンジンを作成することに決めました。私が、それまで14年間生きてきた中で得た膨大な知識と経験の産物を、他の人と共有したいと思ったからです。

その言葉にたじろいだ方、ちょっと待ってください。

この時には、オブジェクト指向プログラミングが優れている(当時)と学びました。その結果が、以下のような膨大なコードです。

class Mesh {
public:
    static std::list<Mesh*> meshes; // Static list of meshes; used for caching and rendering
    Mesh(LPCSTR file); // Loads the x file specified
    Mesh();
    Mesh(const Mesh& vMesh);
    ~Mesh();
    void LoadMesh(LPCSTR xfile); // Loads the x file specified
    void DrawSubset(DWORD index); // Draws the specified subset of the mesh
    DWORD GetNumFaces(); // Returns the number of faces (triangles) in the mesh
    DWORD GetNumVertices(); // Returns the number of vertices (points) in the mesh
    DWORD GetFVF(); // Returns the Flexible Vertex Format of the mesh
    int GetNumSubsets(); // Returns the number of subsets (materials) in the mesh
    Transform transform; // World transform
    std::vector<Material>* GetMaterials(); // Gets the list of materials in this mesh
    std::vector<Cell*>* GetCells(); // Gets the list of cells this mesh is inside
    D3DXVECTOR3 GetCenter(); // Gets the center of the mesh
    float GetRadius(); // Gets the distance from the center to the outermost vertex of the mesh
    bool IsAlpha(); // Returns true if this mesh has alpha information
    bool IsTranslucent(); // Returns true if this mesh needs access to the back buffer
    void AddCell(Cell* cell); // Adds a cell to the list of cells this mesh is inside
    void ClearCells(); // Clears the list of cells this mesh is inside
protected:
    ID3DXMesh* d3dmesh; // Actual mesh data
    LPCSTR filename; // Mesh file name; used for caching
    DWORD numSubsets; // Number of subsets (materials) in the mesh
    std::vector<Material> materials; // List of materials; loaded from X file
    std::vector<Cell*> cells; // List of cells this mesh is inside
    D3DXVECTOR3 center; // The center of the mesh
    float radius; // The distance from the center to the outermost vertex of the mesh
    bool alpha; // True if this mesh has alpha information
    bool translucent; // True if this mesh needs access to the back buffer
    void SetTo(Mesh* mesh);
}

また、コメントも優れている(当時)と学び、以下のように記述するに至りました。

D3DXVECTOR3 GetCenter(); // Gets the center of the mesh

ただし、Meshクラスはより難しい問題を提示しています。Meshの考えは、現実世界に類似するものがない非常に分かりにくい抽象化です。私自身も書きながら混乱してしまいました。頂点やインデックス、その他のデータを保持するコンテナなのか、ディスクからデータを読み込んだり書き出したりするリソースマネージャなのか、それともデータをGPUに送るレンダラなのか。実は、これら全てを包含するものなのです。

改善策

Meshクラスは”簡潔な古いデータ構造”でなければならず、”スマートさ”は必要ありません。つまり、安全に不要なゲッターやセッターを削除可能で、全てのフィールドを公開することができます。

それから、リソース管理とレンダリングを、不活性データで動作する別々のシステムに分けることができます。そう、システムであり、オブジェクトではありません。問題がある際は、全てをオブジェクト指向の抽象化に振り分けず、別の抽象化が適している場合には、そちらに振り分けるようにします。

コメントに関しては、主に、コメント自体を削除することで改善が望めます。なぜなら、コメントはコンパイラによってチェックされないので、簡単に古くなり、誤解を招く大本となり得るからです。以下に該当しない場合は、コメントは消すべきだと考えます。

  • ではなくなぜを説明しているコメント。これらは最も有用です。
  • 後続の膨大なコードの挙動を説明するコメント。これらはナビゲーションや解読に便利です。
  • 各フィールドの内容を説明するデータ構造の宣言についてのコメント。これは不必要なことも多いですが、場合によっては概念を直感的にメモリにマップすることは不可能なため、マッピングを記述するためのコメントが必要となります。

2007-2008

私はこの年代を、”The PHP Dark Ages(暗黒のPHP時代)”と呼んでいます。


注釈:
検閲対象画像

2009-2010

この時の私は大学生で、Acquire, Attack, Asplode, PwnというPythonベースのサードパーソン・複数プレイヤーシューティングゲームを作っていました。まったく…何も言い訳できません。ひどく身のすくむ思いで、BGMの著作権侵害などを平気で行っています。

このゲームを書いている時の最新の知識は、グローバル変数は有害で(当時)、スパゲッティコードを生み出すというものでした。グローバルな状態を変更することで、”A”関数が、完全に無関係な”B”関数を破壊できるようになります。また、スレッドでは動作しません。

しかし、ゲームのほとんど全てのコードは、世界全体の状態にアクセスする必要があります。私はこの問題を、グローバル変数は使わずに、”world”のオブジェクトに全てを格納することで”解決”し、世界をそれぞれの関数に渡しました。これで、理論的には複数の世界を同時に動かすことができます。素晴らしい解決策だと思っていました。

しかし実際のところ、”world”はグローバル状態の実質的なコンテナとして機能しました。複数の世界というアイデアも決して必要ではありませんし、一度もテストしていません。恐らく、きちんとしたリファクタリングなしに、今後、これについて作業することはないでしょう。

いったんグローバル禁酒教などに入会すると、いろんな創造的な方法で自分自身をだますようになります。最悪な例がシングルトンです。

class Thing
{
    static Thing i = null;
    public static Thing Instance()
    {
        if (i == null)
            i = new Thing();
        return i;
    }
}

Thing thing = Thing.Instance();

これは、一見するとグローバル変数ではありません。しかし、シングルトンはグローバルよりもずっとタチが悪いと言えます。理由は以下のとおりです。

  • グローバル変数の潜在的な落とし穴の全てがシングルトンにも当てはまります。シングルトンをグローバルと思っていないというのは、単に自分にウソをついているだけです。
  • よくてもシングルトンにアクセスすると、プログラムにコストの高い分岐命令が追加されます。最悪の場合、完全な関数呼び出しとなります。
  • 実際にプログラムを実行するまで、シングルトンがいつ初期化されるかは分かりません。これは、プログラマが設計時にすべき決定を怠たった別の事例と言えるでしょう。

改善策

何かをグローバルにする必要がある場合は、グローバルにしましょう。この決定を下す時は、プロジェクト全体を検討してください。経験が役立ちます。

本当の問題は、コードの相互依存性です。グローバル変数を使用すると、異種のコード間で目に見えない依存関係が簡単に作成されます。これら不可視の依存関係を最小限に抑えるために、相互依存的なコードをまとめることをお勧めします。これを行ういい方法は、システムに関連する全てのものをそれ自身のスレッドにスローし、残りのコードはメッセージングを通じてそれと通信させることです。

ブーリアン値パラメータ

恐らく多くの人は以下のようなコードを書くでしょう。

class ObjectEntity:
    def delete(self, killed, local):
        # ...
        if killed:
            # ...
        if local:
            # ...

ここでは、互いに類似した4つの”削除”操作があり、それぞれは2つのブーリアン値パラメータのわずかな違いしかありません。見た目的には、完全に合理的だと思われます。では、この関数を呼び出すクライアントコードを見てみましょう。

obj.delete(True, False)

それほど理解しやすいわけではないですよね?

改善策

これに関してはケースバイケースです。ただ、Casey Muratoriのアドバイス、最初にクライアントコードを書くはここに適用していいと思います。もちろん、まともな人が上記のクライアントコードを書くことはないと私は確信しています。その代わり、以下のようなものを書いてください。

obj.killLocal()

その次に、killLocal()関数の実装を書いてください。

命名

命名に重点を置くのは奇妙に思えるかもしれませんが、それはキャッシュの無効化やオフ-バイ-ワンエラーと並んで、コンピュータ科学における未解決の問題とも言えます。

以下の関数を見てください。

class TeamEntityController(Controller):

    def buildSpawnPacket(self):
        # ...

    def readSpawnPacket(self):
        # ...

    def serverUpdate(self):
        # ...

    def clientUpdate(self):
        # ...

最初の2つの関数、そして最後の2つの関数は明らかに関連していますが、その関連性を反映するようには命名されていません。IDEでself.と入力した場合、これらの関数はオートコンプリートメニューでは隣り合って表示されないでしょう。

命名する際は、一般的なものを頭に、固有のものをお尻に持ってくることをお勧めします。

class TeamEntityController(Controller):

    def packetSpawnBuild(self):
        # ...

    def packetSpawnRead(self):
        # ...

    def updateServer(self):
        # ...

    def updateClient(self):
        # ...

このコードの方が、オートコンプリートメニューも理解しやすいはずです。

2010-2015

12年間のプログラミングの後、私はゲーム作成に終止符を打ちました。

この時点までに多くのことを学んだにもかかわらず、このゲームには最も大きな失敗のいくつかが含まれています。

データバインディング

この時は、MicrosoftのMVVMやGoogleのAngularのような「リアクティブな」UIフレームワークが流行の兆しを見せていました。今日では、このスタイルのプログラミングは主にReactで生き続けています。

これらのフレームワークには、共通する基本的な約束事があります。それは、HTMLテキストフィールド、空の要素、そしてその2つの要素を密接にバインドする1行のコードです。テキストフィールドに入力をすると、が魔法のように更新されます。

ゲームのコンテキストでは、以下のようになります。

public class Player
{
    public Property<string> Name = new Property<string> { Value = "Ryu" };
}

public class TextElement : UIComponent
{
    public Property<string> Text = new Property<string> { Value = "" };
}

label.add(new Binding<string>(label.Text, player.Name));

UIはプレイヤーの名前に基づいて自動的に更新されます。また、UIとゲームコードは完全に分けられています。これは、UIの状態を消去し、代わりにゲームの状態からそれを派生させることができるため、とても魅力的です。

しかし、ここにはいくつかの危険信号がありました。ゲームの全てのフィールドをPropertyオブジェクトに変換しなければならなかったのです。これには、バインディングのリストも含まれています。

public class Property<Type> : IProperty
{
    protected Type _value;
    protected List<IPropertyBinding> bindings; 

    public Type Value
    {
        get { return this._value; }
        set
        {
            this._value = value;

            for (int i = this.bindings.Count - 1; i >= 0; i = Math.Min(this.bindings.Count - 1, i - 1))
                this.bindings[i].OnChanged(this);
        }
    }
}

ゲーム内の全てのフィールドには、最後のブーリアン値に至るまで、動的に割り当てられた手に負えないほどの配列がアタッチされていました。

この範例で遭遇した問題の概要を理解するために、プロパティ変更のバインディングを通知するループを見てみましょう。バインディングはUI要素を実際に追加または削除でき、それによってバインディングリストが変更されるため、バインディングリストを逆方向に反復処理しなければなりません。

それでも、私はデータバインディングが好きだったので、それを元にゲーム全体を構築することにしました。オブジェクトをコンポーネントに分割し、そのプロパティを一緒にバインド… しかし、すぐに自分の手には負えなくなりました。

jump.Add(new Binding<bool>(jump.Crouched, player.Character.Crouched));
jump.Add(new TwoWayBinding<bool>(player.Character.IsSupported, jump.IsSupported));
jump.Add(new TwoWayBinding<bool>(player.Character.HasTraction, jump.HasTraction));
jump.Add(new TwoWayBinding<Vector3>(player.Character.LinearVelocity, jump.LinearVelocity));
jump.Add(new TwoWayBinding<BEPUphysics.Entities.Entity>(jump.SupportEntity, player.Character.SupportEntity));
jump.Add(new TwoWayBinding<Vector3>(jump.SupportVelocity, player.Character.SupportVelocity));
jump.Add(new Binding<Vector2>(jump.AbsoluteMovementDirection, player.Character.MovementDirection));
jump.Add(new Binding<WallRun.State>(jump.WallRunState, wallRun.CurrentState));
jump.Add(new Binding<float>(jump.Rotation, rotation.Rotation));
jump.Add(new Binding<Vector3>(jump.Position, transform.Position));
jump.Add(new Binding<Vector3>(jump.FloorPosition, floor));
jump.Add(new Binding<float>(jump.MaxSpeed, player.Character.MaxSpeed));
jump.Add(new Binding<float>(jump.JumpSpeed, player.Character.JumpSpeed));
jump.Add(new Binding<float>(jump.Mass, player.Character.Mass));
jump.Add(new Binding<float>(jump.LastRollKickEnded, rollKickSlide.LastRollKickEnded));
jump.Add(new Binding<Voxel>(jump.WallRunMap, wallRun.WallRunVoxel));
jump.Add(new Binding<Direction>(jump.WallDirection, wallRun.WallDirection));
jump.Add(new CommandBinding<Voxel, Voxel.Coord, Direction>(jump.WalkedOn, footsteps.WalkedOn));
jump.Add(new CommandBinding(jump.DeactivateWallRun, (Action)wallRun.Deactivate));
jump.FallDamage = fallDamage;
jump.Predictor = predictor;
jump.Bind(model);
jump.Add(new TwoWayBinding<Voxel>(wallRun.LastWallRunMap, jump.LastWallRunMap));
jump.Add(new TwoWayBinding<Direction>(wallRun.LastWallDirection, jump.LastWallDirection));
jump.Add(new TwoWayBinding<bool>(rollKickSlide.CanKick, jump.CanKick));
jump.Add(new TwoWayBinding<float>(player.Character.LastSupportedSpeed, jump.LastSupportedSpeed));

wallRun.Add(new Binding<bool>(wallRun.IsSwimming, player.Character.IsSwimming));
wallRun.Add(new TwoWayBinding<Vector3>(player.Character.LinearVelocity, wallRun.LinearVelocity));
wallRun.Add(new TwoWayBinding<Vector3>(transform.Position, wallRun.Position));
wallRun.Add(new TwoWayBinding<bool>(player.Character.IsSupported, wallRun.IsSupported));
wallRun.Add(new CommandBinding(wallRun.LockRotation, (Action)rotation.Lock));
wallRun.Add(new CommandBinding<float>(wallRun.UpdateLockedRotation, rotation.UpdateLockedRotation));
vault.Add(new CommandBinding(wallRun.Vault, delegate() { vault.Go(true); }));
wallRun.Predictor = predictor;
wallRun.Add(new Binding<float>(wallRun.Height, player.Character.Height));
wallRun.Add(new Binding<float>(wallRun.JumpSpeed, player.Character.JumpSpeed));
wallRun.Add(new Binding<float>(wallRun.MaxSpeed, player.Character.MaxSpeed));
wallRun.Add(new TwoWayBinding<float>(rotation.Rotation, wallRun.Rotation));
wallRun.Add(new TwoWayBinding<bool>(player.Character.AllowUncrouch, wallRun.AllowUncrouch));
wallRun.Add(new TwoWayBinding<bool>(player.Character.HasTraction, wallRun.HasTraction));
wallRun.Add(new Binding<float>(wallRun.LastWallJump, jump.LastWallJump));
wallRun.Add(new Binding<float>(player.Character.LastSupportedSpeed, wallRun.LastSupportedSpeed));
player.Add(new Binding<WallRun.State>(player.Character.WallRunState, wallRun.CurrentState));

input.Bind(rollKickSlide.RollKickButton, settings.RollKick);
rollKickSlide.Add(new Binding<bool>(rollKickSlide.EnableCrouch, player.EnableCrouch));
rollKickSlide.Add(new Binding<float>(rollKickSlide.Rotation, rotation.Rotation));
rollKickSlide.Add(new Binding<bool>(rollKickSlide.IsSwimming, player.Character.IsSwimming));
rollKickSlide.Add(new Binding<bool>(rollKickSlide.IsSupported, player.Character.IsSupported));
rollKickSlide.Add(new Binding<Vector3>(rollKickSlide.FloorPosition, floor));
rollKickSlide.Add(new Binding<float>(rollKickSlide.Height, player.Character.Height));
rollKickSlide.Add(new Binding<float>(rollKickSlide.MaxSpeed, player.Character.MaxSpeed));
rollKickSlide.Add(new Binding<float>(rollKickSlide.JumpSpeed, player.Character.JumpSpeed));
rollKickSlide.Add(new Binding<Vector3>(rollKickSlide.SupportVelocity, player.Character.SupportVelocity));
rollKickSlide.Add(new TwoWayBinding<bool>(wallRun.EnableEnhancedWallRun, rollKickSlide.EnableEnhancedRollSlide));
rollKickSlide.Add(new TwoWayBinding<bool>(player.Character.AllowUncrouch, rollKickSlide.AllowUncrouch));
rollKickSlide.Add(new TwoWayBinding<bool>(player.Character.Crouched, rollKickSlide.Crouched));
rollKickSlide.Add(new TwoWayBinding<bool>(player.Character.EnableWalking, rollKickSlide.EnableWalking));
rollKickSlide.Add(new TwoWayBinding<Vector3>(player.Character.LinearVelocity, rollKickSlide.LinearVelocity));
rollKickSlide.Add(new TwoWayBinding<Vector3>(transform.Position, rollKickSlide.Position));
rollKickSlide.Predictor = predictor;
rollKickSlide.Bind(model);
rollKickSlide.VoxelTools = voxelTools;
rollKickSlide.Add(new CommandBinding(rollKickSlide.DeactivateWallRun, (Action)wallRun.Deactivate));
rollKickSlide.Add(new CommandBinding(rollKickSlide.Footstep, footsteps.Footstep));

膨大な問題に直面しました。バインディングサイクルの無限ループです。この時、初期化の順序が重要なことが分かりましたが、データバインディングでの初期化は悪夢でした。一部のプロパティはバインディングが追加されると複数回初期化されるのです。

アニメーションの追加に関しては、2つの状態をアニメーション化するのにデータバインディングは向いておらず、困難で直感的でないことが分かりました。こう感じたのは私だけではありません。このNetflixのトークを見てください。ここで彼らはReactがどれほど偉大であるかを語っていますが、アニメーションの実行時には、それをオフにしなければならない理由を説明しています。

私自身もバインディングをオン、オフする効果を実感したので、新しいフィールドを追加しました。

class Binding<T>
{
    public bool Enabled;
}

残念ながら、これだとデータバインディングの目的は果たせません。私はUI状態を取り除きたいと思っていましたが、このコードは実際のところ、いくつかを追加します。では、どうすればこの状態を取り除くことができるでしょうか。

そう、データバインディングです。

class Binding<T>
{
    public Property<bool> Enabled = new Property<bool> { Value = true };
}

これを簡単に試しましたが、全てがバインディングになってしまい、すぐに馬鹿げていると理解しました。

データバインディングをどのように改善できるでしょうか。ご自身のUIを実際に機能的かつステートレスにしてみてください。親愛なるimguiはこれの好例です。ビヘイビアと状態はできる限り分けた方がいいでしょう。また、状態の作成を容易にするテクニックは避けましょう。結果的に状態を作るのが苦痛になってくると思います。

まとめ

この他にも、まだまだ議論すべき恥ずかしい間違いはあります。例えば、グローバルを避けるための”クリエイティブな”方法を新たに発見し、しばらくの間はクロージャの虜になってみたり、”エンティティ”、”コンポーネント”、”システム”を設計してみたものの、できたのは全くの別物だったり、ロックを点在させることでボクセルエンジンをマルチスレッド化しようとしてみたり、というようなことです。

今回の記事に関して、以下に要点をまとめておきます。

  • 意思決定をコンピュータに任せるのではなく前もって行う。
  • 動作と状態を分離する。
  • 純粋な関数を書く。
  • クライアントコードを先に記述する。
  • 退屈なコードを書く。

以上が私の体験談です。皆さんのおぞましい過去もぜひ共有してください。

この記事に興味を持っていただいた方は、以下もぜひご覧ください。
* The Poor Man’s Voxel Engine(ボクセルエンジンについて)
* The Poor Man’s Character Controller(キャラクタコントローラについて)
* One Weird Trick to Write Better Code(より良いコードを書くためのトリックについて)