13 Years of Bad Game Code

Evan Todd
←→

Have you ever done this?

  1. Open an old project from a few years back
  2. oh no
  3. i've made a huge mistake
  4. Close project
  5. Delete project

2004

Weapon Switching


public void updateAnimation(long eTime) {
	if(group.getGroup("gun") == null) { //! defensive coding
		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; //! this variable is unnecessary
	}
}

State Variables

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

Feels like something's missing...

  • currentWeapon

The Irony

I never even modeled different weapons.

How to Improve

  • weaponSwitchTimer
  • weaponCurrent

Naming


boolean noenemies = true;
					

if (!noenemies) {
	//! re-evaluate your life decisions
}
					

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

2005 - 2006


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

You Learned: "Comments"!


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

Problems


ID3DXMesh* d3dmesh;
LPCSTR filename;
DWORD numSubsets;
std::vector<Material> materials;
std::vector<Cell*> cells;
D3DXVECTOR3 center;
float radius;
bool alpha;
bool translucent;

How to Improve


struct Mesh
{
	Array<Vec3> vertices;
	Array<int> indices;
	Vec3 center;
	float radius;
};

Comment Manifesto

Delete 'em. They're a liability. Except for:

  • Comments that explain why, rather than what.
  • Comments that label a giant chunk of code. Useful for navigation.
  • Comments that explain the representation of data in memory.

2007 - 2008

PHP dark ages

2009 - 2010

What's Wrong With Globals?


class GameObject:
	def update(self, world):
		# ...

Reality


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

Thing thing = Thing.Instance();

Just Use Globals

Boolean Parameters


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

Client Code


object.delete(True, False)

Write the Client Code First


object.deleteLocal()
object.deleteRemote()
object.killLocal()
object.killRemote()

Naming


class TeamEntityController(Controller):

	def buildSpawnPacket(self):
		# ...
	
	def readSpawnPacket(self):
		# ...
	
	def serverUpdate(self):
		# ...

	def clientUpdate(self):
		# ...

Ordering: General -> Specific

Helps with code completion and organization.


class TeamEntityController(Controller):

	def packetSpawnBuild(self):
		# ...
	
	def packetSpawnRead(self):
		# ...
	
	def updateServer(self):
		# ...

	def updateClient(self):
		# ...

2010 - 2015

Data Binding

Hello,

The Dream


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

Red Flags


public class Property<Type> : IProperty
{
	protected Type _value;
	protected List<IPropertyBinding> bindings; //! heap allocation

	public Type Value
	{
		get { return this._value; }
		set
		{
			this._value = value;
			//! weird loop necessary due to binding changes
			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));
					

class Binding<T>
{
	public bool Enabled;
}
					

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

Etc., etc.

  1. Make decisions upfront instead of lazily leaving them to the computer.
  2. Separate behavior and state.
  3. Write pure functions.
  4. Write the client code first.
  5. Write boring code.

Thank you!

Read the article: etodd.io