ゲームでの13年間の悪いコード

インスピレーションを求めて孤独な金曜日の夜に、あなたはプログラミングの面であなたの過去の勝利を思い出すことにします。 古いハードドライブからのアーカイブがゆっくりと開き、ここに輝かしい遠い時代のコードが表示されます...

いえいえ これは、あなたが期待するものではありません。 すべてがとても悪かったのは本当ですか？ なぜ誰もあなたに言っていないのですか？ どうすればこれに到達できますか？ 単一の関数で非常に多くの重要なステートメント-それはまったく合法ですか？ プロジェクトを急いで閉じます。 少しの間、ファイルとハードドライブのすべてのコンテンツを同時に削除したいと思われます。



以下に、私自身の過去の旅から学んだ教訓、有益な例、警告のコレクションを示します。 加害者を暴露するために、すべての名前は変更されていません。

2004

私は13歳でした。 「 レッドムーン 」と題されたこのプロジェクトは、非常に野心的な三人称の空中戦ゲームでした。 「 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; } } 

いくつかの興味深い事実に注目したいと思います。 最初に、ここにある変数の数を評価します。

• changeTime
• 変わる
• weaponGroup
• weaponGroup.xform.velocityAngleX

しかし、これらすべてで、何かが欠けているように見えました...ああ、はい、まだ使用されている武器を追跡する変数が必要です。 そしてもちろん、これには別のファイルが必要です。

そしてもう1つ：最終的に、複数の武器モデルを作成するつもりはありませんでした。 つまり、これらのタイプのそれぞれに同じモデルが使用されました。 このすべてのコードを使用することはまったくありませんでした。

修正方法

不要な変数を削除します。 この場合、武器の状態は、weaponSwitchTimerとweaponCurrentの2つだけで説明できます。 他のすべての情報はそれらから抽出できます。

可能なすべてを明示的に初期化します。 この関数は、武器がヌルかどうかを確認し、必要に応じて初期化プロセスを開始します。 少なくとも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) {} } 

あなたはおそらく今考えています：「私たちは何らかの形でこの間違いをもっと優雅に処理しなければなりません！ 少なくとも、ユーザーへのメッセージなどを表示します。」 しかし、私は正直に反対の見方を固守しています。

エラーをなくすことは決して余計なことではありませんが、処理は簡単にやり直すことができます。 この場合、武器モデルなしでプレイすることはまだ不可能なので、ゲームをクラッシュさせることもできます。 それが明らかに絶望的であるならば、尊厳から尊厳から抜け出そうとしないでください。

さらに、上記に戻って、ここでは、修正可能と見なされるエラーとそうでないエラーを個別に決定する必要があります。 残念なことに、Javaのほとんどすべてのエラーは修正可能でなければならないとSunは考えており、その結果、遅延処理のケースがあります-それらの1つは上記で与えられています。

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 

ただし、このクラスにはさらに深刻な問題があります。 メッシュコンセプトは、漠然とした抽象化の一種であり、現実のものと同等のものを選択することはできません。 私がそれを書いたときでさえ、私は何も理解していませんでした。 それは何ですか-頂点、インデックス、その他のデータを含むコンテナですか？ データをディスクにロードおよびアンロードするリソースマネージャーですか？ GPUにデータを送信するレンダリングツールですか？ 一度に。

修正方法

Meshクラスは単純なデータ構造でなければなりません。 スマートタイプを使用しないでください。 そして、これは穏やかな魂ですべてのゲッターとセッターを捨てて、すべてのフィールドを公開できることを意味します。

さらに、リソース管理とレンダリングを、不活性データで動作する個別のシステムに分離することが可能です。 はい、それはシステムであり、オブジェクトではありません。 別のタイプの抽象化の方がうまくいく場合は、各問題をオブジェクト指向の抽象化に合わせようとしないでください。

コメントを修正する最も確実な方法は、原則としてコメントを削除することです。 コメントはすぐに無関係なホワイトノイズになりますが、これは紛らわしいだけです-コンパイラはとにかくそれらを見ません。 コメントは、次のグループのいずれかに属さない限り破棄されるべきだと主張します。

• 「何？」ではなく「なぜ？」という質問に答えるコメント。 それらは最も有用です。
• コメントは、後に続く巨大なコードを作成する理由を簡単に説明しています。 それらは、読書とナビゲーションを容易にします。
• 各フィールドの内容を説明するデータ構造宣言のコメント。 多くの場合、これらは不要ですが、場合によっては、構造がメモリ内でどのように配置されているかが直観的に明確ではないため、同様の説明を伴うコメントが必要です。

2007-2008

私はこれを「PHPの暗黒時代」と呼んでいます。

2009-2010

私はすでに大学にいます。 私は、Acquire、Attack、Asplode、PwnというPythonのサードパーソンマルチプレイヤーシューティングゲームに取り組んでいます。 私の弁護では何も言えません。 さらに恥ずかしいことに、著作権侵害の後遺症があります。

このゲームを書いたとき、グローバル変数は悪であると悟りました。 コードをハッシュに変換します。 これらは、グローバルな状態を変更する機能Aを、機能Bとは関係なく、機能Bを破る/違反することを許可します。 スレッドでは機能しません。

ただし、ほぼすべてのゲームプレイコードは、全体としてワールドマトリックスの状態にアクセスする必要があります。 このオブジェクトにすべてを保存し、それを各関数に渡すことで、この問題を「解決」しました。 グローバル変数はありません！ 理論的には複数の自律的な世界マトリックスを同時に実行することが可能であることが判明したため、私には素晴らしいアイデアがあったように思えました。

実際、世界は実際、世界的地位のコンテナでした。 もちろん、いくつかの世界の概念は完全に無意味であり、誰もそれをテストしたことはありませんでした。そして、それが真剣にリファクタリングされた場合にのみ機能すると思います。
グローバル変数の敵対派に入った人は、自己欺ofの創造的な方法の全世界を発見します。 それらの最悪はシングルトンです。

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

ガニガニのブーム！ 目に見える単一のグローバル変数ではありません！ しかし、1つの「しかし」があります。シングルトンは、次の理由ではるかに悪いです。

• グローバル変数の潜在的な弱点はすべてここに残ります。 シングルトンがグローバルではないと思う場合は、自分に嘘をつくのをやめてください。
• 最良の場合、シングルトンはリソースを集中的に使用する分岐演算子をプログラムに追加します。 最悪の場合、完全な関数呼び出しです。
• プログラムを開始するまで、いつシングルトンが初期化されるかはわかりません。 これは、怠zyなプログラマが設計段階で何を計画すべきかをシステムに決定させる方法の別の例です。

修正方法

何かがグローバルでなければならない場合、それをさせてください。 この決定を行うときは、プロジェクト全体にどのように影響するかを検討してください。 経験があれば、それは簡単になります。

実際、問題はコードの相互依存性です。 グローバル変数は、異種のコードの間に見えない依存関係をもたらす可能性があります。 これらの目に見えない依存関係を最小限に抑えるために、関連するコードを一貫したシステムにグループ化します。 これを実現する良い方法は、1つのシステムに関連するすべてを単一のストリームにダンプし、残りのコードにメッセージを介して対話させることです。

ブール型パラメーター

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

おそらく、次のようなコードを書く必要がありました。

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

ここでは、互いに非常に似ている4つの個別の削除操作があります。すべての小さな違いは、ブール型の2つのパラメーターに関連付けられています。 すべてが論理的なようです。 次に、この関数を呼び出すクライアントコードを見てみましょう。

 obj.delete(True, False) 

それは非常に読みやすいように見えませんか？

修正方法

ここでは、特定のケースを見る必要があります。 しかし、 Casey Muratoriから常に関連するアドバイスが1つあります。それは、クライアントコードから始めることです。 私は彼の心の中に、上記のようなクライアントコードを書く人はいないと確信しています。 むしろ、彼は次のように書くでしょう：

 obj.killLocal() 

そして、killLocal（）関数の実装を登録します。

お名前

私がそんなに名前をプッシュするのは奇妙に思えるかもしれませんが、古いジョークが言うように、これは2つの未解決のプログラミング問題の1つです（2つ目はキャッシュの無効化とユニットエラーです）。

これらの機能をよく見てください。

 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のテキストボックス、空の要素、およびそれらを密接にリンクする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)); 

プレーヤーの名前を入力すると、インターフェイスが自動的に更新されるようになりました！ インターフェイスとゲームコードは、互いに完全に独立させることができます。 これは魅力的です。インターフェイスの状態を取り除き、ゲームの状態から削除します。

しかし、いくつかの邪魔な鐘がありました。 ゲーム内のすべてのフィールドを、多くの依存関係を含むプロパティオブジェクトに変換する必要がありました。

 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); } } } 

絶対に、最後のブール値までのすべてのフィールドに、かさばる動的配列が割り当てられました。

プロパティの変更に関する関連データを通知するサイクルを見てください。そうすれば、このようなパラダイムにより私がどんな問題に遭遇したかをすぐに理解できます。 バインディングはインターフェース要素を追加または削除できるため、リスト内の変更につながるため、関連データのリストを逆の順序で繰り返す必要があります。

それにもかかわらず、私はその上にゲーム全体を構築するほどデータバインディングに染み込んでいた。 オブジェクトをコンポーネントに分割し、それらのプロパティを関連付けました。 状況は制御不能になり始めました。

 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プログラマーで見ることができます。Netflixプログラマーは、Reactを称賛して崩れ、アニメーションを開始するたびにオフにする必要があることを伝えます。

また、接続を切断するすべての力を使用することを推測しました。 そして、別のフィールドを追加しました：

 class Binding<T> { public bool Enabled; } 

残念なことに、バインディングの全体的な意味はこれから失われました。 私は状態を取り除きたかったのですが、このコードではさらに状態が変わっただけです。 この状態を解消する方法は？

わかった！ バインディングで！

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

はい、私は真剣にこのようなものを実装しようとした瞬間がありました。 接続されているすべてのものを接続しました。 しかし、私はすぐに気がついて、これがおかしいと気付きました。

バインド状況を修正するにはどうすればよいですか？ インターフェイスを正常に機能させるために、状態なしで試してください。 良い例は、 親愛なるimguiです。 可能な限り動作と状態を分離します。 状態を簡単にする手法は避けてください。 幸運を作ることはあなたにとって苦痛なステップであるべきです。

おわりに

これは私の愚かな間違いとはほど遠いものです。 グローバル変数を回避する別の「オリジナル」メソッドを作成しました。 閉鎖に夢中になっていた時期がありました。 いわゆるエンティティオブジェクトシステムを作成しましたが、それはエンティティオブジェクトシステムではありませんでした。 ボクセルエンジンにマルチスレッドを実装して、寛大にロックを設定しようとしました。

私が思いついた結論は次のとおりです。

• 自分で決定を下し、それらをコンピューターに委任しないでください。
• 別の動作と条件。
• 純粋な関数を書きます。
• クライアントコードから始めます。
• 退屈なコードを書きます。

これが私の話です。 共有したいと思う暗い過去はありますか？