Thirteen Years of Bad Game Code
Alone on a Friday night, in need of some inspiration, you decide to relive some of your past programming conquests.
The old archive hard drive slowly spins up, and the source code of the glory days scrolls by...
Oh no. This is not at all what you expected. Were things really this bad? Why did no one tell you? Why were you like this? Is it even possible to have that many gotos in a single function? You quickly close the project. For a brief second, you consider deleting it and scrubbing the hard drive.
What follows is a compilation of lessons, snippets, and words of warning salvaged from my own excursion into the past. Names have not been changed, to expose the guilty.
I was thirteen. The project was called Red Moon — a wildly ambitious third-person jet combat game. The few bits of code that were not copied verbatim out of Developing Games in Java were patently atrocious. Let's look at an example.
I wanted to give the player multiple weapons to switch between. The plan was to rotate the weapon model down inside the player model, swap it out for the next weapon, then rotate it back. Here's the animation code. Don't think about it too hard.
I want to point out two fun facts. First, observe how many state variables are involved:
Even with all that, it feels like something's missing... ah yes, we need a variable to track which weapon is currently equipped. Of course, that's in another file entirely.
The other fun fact is that I never actually created more than one weapon model. Every weapon used the same model. All that weapon model code was just a liability.
How to Improve
Remove redundant variables. In this case, the state could be captured by two variables: weaponSwitchTimer and weaponCurrent. Everything else can be derived from those two variables.
Explicitly initialize everything. This function checks if the weapon is null and initializes it if necessary. Thirty seconds of contemplation would reveal that the player always has a weapon in this game, and if they don't, the game is unplayable and might as well crash anyway.
Clearly, at some point, I encountered a NullPointerException in this function, and instead of thinking about why it happened, I threw in a quick null check and moved on. In fact, most of the functions dealing with weapons have a check like this!
Be proactive and make decisions upfront! Don't leave them for the computer to figure out.
Name your booleans positively. If you find yourself writing code like this, re-evaluate your life decisions:
Snippets like this are sprinkled liberally throughout the codebase:
You might be thinking "it should handle that error more gracefully! Show a message to the user or something." But I actually think the opposite.
You can never have too much error checking, but you can definitely have too much error handling. In this case, the game is unplayable without the weapon model, so I might as well let it crash. Don't try to gracefully recover from unrecoverable errors.
Once again, this requires you to make an upfront decision as to which errors are recoverable. Unfortunately, Sun decided that almost all Java errors must be recoverable, which results in lazy error handling like the above.
At this point I learned C++ and DirectX. I decided to write a reusable engine so that mankind could benefit from the vast wealth of knowledge and experience I had acquired in my fourteen years on the earth.
If you thought the last trailer was cringey, just wait.
By now I learned that Object-Oriented Programming is Good™, which resulted in monstrosities like this:
I also learned that comments are Good™, which led me to write gems like this:
This class presents more serious problems though. The idea of a Mesh is a confusing abstraction that has no real-world equivalent. I was confused about it even as I wrote it. Is it a container that holds vertices, indices, and other data? Is it a resource manager that loads and unloads that data from disk? Is it a renderer that sends the data to the GPU? It's all of these things.
How to Improve
The Mesh class should be a "plain old data structure". It should have no "smarts", which means we can safely trash all the useless getters and setters and make all the fields public.
Then we can separate the resource management and rendering into separate systems which operate on the inert data. Yes, systems, not objects. Don't shoehorn every problem into an object-oriented abstraction when another abstraction might be a better fit.
The comments can be improved, mostly, by deleting them. Comments easily fall out of date and become misleading liabilities, since they're not checked by the compiler. I posit that comments should be eliminated unless they fall into one of these categories:
- Comments explaining why, rather than what. These are the most useful.
- Comments with a few words explaining what the following giant chunk of code does. These are useful for navigation and reading.
- Comments in the declaration of a data structure, explaining what each field means. These are often unnecessary, but sometimes it's not possible to map a concept intuitively to memory, and comments are necessary to describe the mapping.
I call these years "The PHP Dark Ages".
By now, I'm in college. I'm making a Python-based third-person multiplayer shooter called Acquire, Attack, Asplode, Pwn. I have no excuse at this point. The cringe just keeps getting worse, and now it comes with a healthy dose of copyright infringing background music.
When I wrote this game, the most recent piece of wisdom I had picked up was that global variables are Bad™. They lead to spaghetti code. They allow function "A" to break a completely unrelated function "B" by modifying global state. They don't work with threads.
However, almost all gameplay code needs access to the entire world state. I "solved" this problem by storing everything in a "world" object and passed the world into every single function. No more globals! I thought this was great because I could theoretically run multiple, separate worlds simultaneously.
In practice, the "world" functioned as a de facto global state container. The idea of multiple worlds was of course never needed, never tested, and I'm convinced, would never work without significant refactoring.
Once you join the strange cult of global tea-totallers, you discover a whole world of creative methods to delude yourself. The worst is the singleton:
Poof, magic! Not a global variable in sight! And yet, a singleton is much worse than a global, for the following reasons:
- All the potential pitfalls of global variables still apply. If you think a singleton is not a global, you're just lying to yourself.
- At best, accessing a singleton adds an expensive branch instruction to your program. At worst, it's a full function call.
- You don't know when a singleton will be initialized until you actually run the program. This is another case of a programmer lazily offloading a decision that should be made at design time.
How to Improve
If something needs to be global, just make it global. Consider the whole of your project when making this decision. Experience helps.
The real problem is code interdependence. Global variables make it easy to create invisible dependencies between disparate bits of code. Group interdependent code together into cohesive systems to minimize these invisible dependencies. A good way to enforce this is to throw everything related to a system onto its own thread, and force the rest of the code to communicate with it via messaging.
Boolean ParametersMaybe you've written code like this:
Here we have four different "delete" operations that are highly similar, with a few minor differences depending on two boolean parameters. Seems perfectly reasonable. Now let's look at the client code that calls this function:
Not so readable, huh?
How to Improve
This is a case-by-case thing. However, one piece of advice from Casey Muratori certainly applies here: write the client code first. I'm sure that no sane person would write the above client code. You might write something like this instead:
And then go write out the implementation of the killLocal() function.
It may seem strange to focus so heavily on naming, but as the old joke goes, it's one of the two remaining unsolved problems in computer science. The other being cache invalidation and off-by-one errors.
Take a look at these functions:
Clearly the first two functions are related to each other, and the last two functions are related. But they are not named to reflect that reality. If I start typing self. in an IDE, these functions will not show up next to each other in the autocomplete menu.
Better to make each name start with the general and end with the specific, like this:
The autocomplete menu will make much more sense with this code.
After only 12 years of work, I actually finished a game.
Despite all I had learned up to this point, this game featured some of my biggest blunders.
At this time, people were just starting to get excited about "reactive" UI frameworks like Microsoft's MVVM and Google's Angular. Today, this style of programming lives on mainly in React.
All of these frameworks start with the same basic promise. They show you an HTML text field, an empty <span> element, and a single line of code that inextricably binds the two together. Type in the text field, and pow! The <span> magically updates.
In the context of a game, it looks something like this:
Wow, now the UI automatically updates based on the player's name! I can keep the UI and game code totally separate. This is appealing because we're eliminating the state of the UI and instead deriving it from the state of the game.
There were some red flags, however. I had to turn every single field in the game into a Property object, which included a list of bindings that depended on it:
Every single field in the game, down to the last boolean, had an unwieldy dynamically allocated array attached to it.
Take a look at the loop that notifies the bindings of a property change to get an idea of the issues I ran into with this paradigm. It has to iterate through the binding list backward, since a binding could actually add or delete UI elements, causing the binding list to change.
Still, I loved data binding so much that I built the entire game on top of it. I broke down objects into components and bound their properties together. Things quickly got out of hand.
I ran into tons of problems. I created binding cycles that caused infinite loops. I found out that initialization order is often important, and initialization is a nightmare with data binding, with some properties getting initialized multiple times as bindings are added.
When it came time to add animation, I found that data binding made it difficult and non-intuitive to animate between two states. And this isn't just me. Watch this Netflix talk which gushes about how great React is before explaining how they have to turn it off any time they run an animation.
I too realized the power of turning a binding on or off, so I added a new field:
Unfortunately, this defeated the purpose of data binding. I wanted to get rid of UI state, and this code actually added some. How can I eliminate this state?
I know! Data binding!
Yes, I really did try this briefly. It was bindings all the way down. I soon realized how crazy it was.
How can we improve on data binding? Try making your UI actually functional and stateless. dear imgui is a great example of this. Separate behavior and state as much as possible. Avoid techniques that make it easy to create state. It should be a pain for you to create state.
There are many, many more embarrassing mistakes to discuss. I discovered another "creative" method to avoid globals. For some time I was obsessed with closures. I designed an "entity" "component" "system" that was anything but. I tried to multithread a voxel engine by sprinkling locks everywhere.
Here's the takeaway:
- Make decisions upfront instead of lazily leaving them to the computer.
- Separate behavior and state.
- Write pure functions.
- Write the client code first.
- Write boring code.
That's my story. What horrors from your past are you willing to share?
If you enjoyed this article, try these: