[In this article, originally published in Game Developer Magazine and brought to you now in association with Intel, Michael, Noel, and "Anonymous Developer" share several programming sins relevant to the game industry. It reminds me of one programming "sin" at Insomniac Games. We were very close to shipping Resistance: Fall of Man and there were a few corner cases in the multiplayer code that were driving us crazy.

Specifically, there was a timing issue with players dropping from the game. A player would leave the game and other players would release the associated resources, but eventually an old message from the Interwebs would show up and reference the dropped player. Ugh, timing was important and very difficult to repro.

This would result in a NULL pointer being accessed. Specifically, a member function would be called on a NULL object. Since we didn’t have time to track down the issue, we added a check "if( this == NULL ) return;" at the top of the offending function.

Wow, that’s ugly. But it works because member functions are ultimately independent of any instance of the object. Hacks like this are a maintenance nightmare. Ultimately, this was an artifact of "late joining" not being architected from the start of the project. As you can image, player management was a big rewrite in Resistance 2.

I’m glad Intel has the opportunity to republish this great article on Gamasutra. As Albert Einstein said, "Anyone who has never made a mistake has never tried anything new." Please share your "programming sins" in the comments!! -- Orion Granatir]

From the very first line of code an engineer writes, he or she starts to develop their personal list of "dos and don'ts" that, even if they are never written down, have a tremendous effect on how we design and build our games. The list evolves and changes over time -- we add to it, delete from it, and re-evaluate the lists of others. Experience is the driving force behind a lot of the changes; in short, we make a mistake, and we learn from it. These game programming sins are essentially a number of dos and don'ts with some recollections of how and why they came to be on this list.

Michael A. Carr

Creating Obfuscated Code

For those that don't know me, one of my major flaws is that I simply don't remember everything. My brain, it would appear, is completely incapable of storing all the facts and information I ask it to. Over the years, I have used different methods to help me remember, all with varying degrees of success. My current system is to write everything down. I carry a black leather writing notebook with me, and I take lots of notes. I use a P.D.A. for some things like contacts and mind maps, but when it comes to making lists and notes, paper and pen have yet to be beaten.

One of the effects of this forgetfulness is that I couldn't tell you the intimate details of a function I wrote three weeks ago, let alone six months or a year ago, without at least re-reading it and refreshing my memory. It's because of this that I consider obfuscated code a sin. This also fits nicely with working in a team of programmers where someone might have to debug and/or add new functionality to someone else's code. The quicker it is to understand, the easier it is for them to make the required modifications.

int m_MyMumIsBetterThanYours;

No it wasn't a game about mums ... although I wonder if there is a game there somewhere ...

bool m_BumCheeks;

Enough said.

float m_Saving;

Is this a flag to indicate saving, in which case why a float? Or is it a percentage of save completed?

void * m_pAudioSample;

Not very useful. Wouldn't SAudioSample * m_pTheWarCry; be more useful?

int CCharacter::GetLife( int y )

Nothing wrong with this function ... only why is there a variable passed in? More importantly, y isn't exactly very descriptive. Turns out this function did indeed get the amount of life and return it; and while it was there, it also updated the life value by applying the damage modifier to the life counter, and also applied the adjustment of y. When this function wasn't called every tick, the whole life counter on the character broke.

Abusing ternary operations. Consider the following code. (Remember that this would normally be on a single line, so you would have to scroll to see the entire line.)

if (CPhysicsManager::Instance().RayCast(m_Position + (CVector::Up * METRES(2.0f)), m_Position - (CVector::Up * METRES(2.0f)), &contact_data, pActor->GetPhysicsActor() ? pActor->FindRealActor()- >GetPhysicsActor() : NULL, ePhysicsShape_Static))

Would you have spotted the use of ? and : inside the function parameter list?

Some coding standards I have worked with ban the use of ? and : altogether, mainly because it's easy to abuse. As you can see, it contributes handily to the jumbled code in the example above. However, there are cases where I consider them to be fair enough. That's usually where the use is obvious. For example:

m_Level = level_specified ? start_level : default_level;

result = a > b ? b : a;

Abbreviating English. There was a time when the length of our variable or function names would have a significant effect on the performance of the compiler. This has not been the case for a very long time, but some engineers seem to like using shorthand.

int NmbrChars( );

vs.

int GetNumberOfCharacters( void );

int m_LCnt;

vs.

int m_LifeCount;

English is my first spoken and written language, and I find it easier to read code that says what it is in plain English.

Not Failing Gracefully

The world would be a dull place if we all had the same thoughts, the same ambitions, ideas, and methods of working. But there are some thoughts and methods that should be discouraged whenever and wherever they are encountered.

Working on a project that was just over halfway through its development cycle, I was investigating why the game kept crashing whenever a specific sound event was triggered. I quickly tracked the problem down to some missing audio files, an easy fix. Still, it was the fact that a missing file was causing the entire game to crash that got me looking a little more closely at the sound manager. It turned out that the manager never validated its data; even though a file had failed to load, it carried on processing the non-existent sound data as though the file load had been successful.

Talking to the engineer who maintained the system, I thought he was pulling my leg when he said he considered it acceptable behavior for the code to crash when something goes wrong. After he repeated himself, I realized that he was serious. From his perspective, the problem was the missing data, not that the manager didn't handle itself in a graceful fashion.

There are always going to be unexpected issues that arise, certainly during development and very possibly after our games have shipped. There might be a missing texture, a corrupt file, an out-of-range data value, or even a lack of resources. We can and should make our code as bulletproof as we possibly can. The game should be continuously fighting to keep itself running. This makes the game experience more stable for the player, and if something does go wrong, there should be fail-safes in place so that he or she will not even notice.

Here are three basic coding practices that I advocate as a minimum. You might also recognize what I'm talking about as essentially being "defensive programming."

Pointers. Pointers are pointers because it's possible they might be NULL (otherwise they would have been references). Validate pointers at least once before accessing them.

Validate Data. Sanity checking data can help prevent a lot of issues. Clipping them to valid ranges means you are less likely to have invalid calculations later on down the line.

Default State. Some data can't be clipped or ignored, an example might be a texture, in this scenario having a memory resident fallback is a good solution. During development it can be bright pink and yellow and stands out a mile while in shipped mode it can be transparent.

Ignoring Customer Satisfaction

During the early days of a very old project, I had added support for a trigger box to the game's editor. A few days later, a designer asked for a trigger sphere in addition to the box. I was busy, I had a pile of other work to do, and I didn't think it was that important for the milestone. I explained that I would add it to my list and implement it as soon as I could. Unfortunately, I didn't get to it soon enough; the designer announced to me the next day that he had solved the problem and didn't need my implementation anymore.

Have you ever stood on the edge of a very tall building and slowly looked over the side? Remember how your stomach felt as you slowly peeked over the edge? That was how I felt looking over the designer's shoulder at his solution. What I observed was a script that created 100 square trigger boxes, each rotated slightly more than the last. It made a pretty spiral effect in the editor and took a huge chunk out of the CPU when running in-game, but it worked exactly as he wanted it to.

This was a kick up my bum, and the designer got his sphere trigger very quickly afterwards. Designers, like everyone else in the games industry, are incredibly creative. The main thing about them in particular is that they seem to get far too much enjoyment out of abusing game systems. I consider it a sin to ignore any designer's valid request. Failure to pay attention to them might result in something creatively "ugly."

Fragmenting Memory

During the development of many projects, there's been a critical moment when everything started falling apart. The game crashes constantly, levels are broken, data builds take twice as long, the coffee machine is out of order, and you're supposed to be going to a family event on the weekend, and you must desperately try to find the right moment to tell your better half that you won't be going. For me, this usually happens around six weeks before the end of the project.

What is going wrong in these instances is varied, but I've noticed a trend that usually revolves around the fragmentation of memory. There's nothing like spending time hunting down and defragmenting memory to make you realize how many of these issues could have been prevented.

Temporary Buffers. Reading this simplified example, it might seem obvious, but it happens more than I would expect. Having looked over file histories, cases of memory fragmentation seem to evolve over time when multiple engineers introduce additional initialization between the allocation and de-allocation. This has led me to consider temporary buffer allocations a sin— at least until they have proved themselves trustworthy.

Function A

Create a temporary buffer.

Do something with the buffer.

Call Function B.

Finish processing the temporary buffer.

Release the temporary buffer.

Function B

Allocate non-temporary data.

The allocation in Function B might simply be the loading of a file, creating a new entry in a link list, or allocation of a string. The result however is identical; fragmented memory. There are a number of ways to deal with this, not least of which is using static memory, a unique heap, or even a scratch buffer.

Leaks. Okay, it makes sense that memory leaks cause fragmentation. Luckily, with some good tracking tools, these are usually easy to find and plug-up.

Keep It Simple. Each allocation should have one de-allocation in a logical place. For example, if you allocate memory in the Init function, you de-allocate it in the Deinit function. Some engineers seem to think it's fun to hide the de-allocator in obscure parts of the code, or to have multiple de-allocate commands for the same piece of memory. Keep it clean, keep it simple!

Noel Llopis

Don't Sync and Load

Late in a console project, I took on the gargantuan task of reducing level load times. They had been slowly creeping in throughout development and were up to a minute and a half. The goal was to bring that down under 30 seconds.

There were some obvious things that I was going to tackle: parsing and processing of text files, wild memory allocations, and so forth. Once I took care of all the low-hanging fruit, things were better, but load times were still way over a minute. Something was clearly wrong.

Curiously, the profile wasn't showing any huge hotspots, yet we were still spending over a minute loading levels. Where was all that time going?

After some more digging, I noticed that during the level load, we were drawing a progress bar on the screen. The loading code wasn't architected from the beginning to be multithreaded, so instead, we would load one file, update the progress bar, load another file, update the progress bar again, and so on until all files were loaded. We had about 3,000 tiny files per level (fortunately packed in a larger container file and laid out sequentially), so that made it possible to update the progress bar in a smooth way.

Other than being a bit clumsy and not very elegant, there was nothing horribly wrong with that approach. Except for one thing: the rendering code drew the progress bar, and then did a present call with vertical sync on. That meant that most of the time the console was waiting for vertical sync instead of doing an actual load. Once I removed the vsync, loading times went down to about 20 seconds!

Bonus: that weekend I went for a bike ride with a friend who is a game developer at another nearby company. I told him about our vsync issue and how they affected our loading times -- we both got a good laugh out of it. It turns out, when he went back to work, he checked out of curiosity and they were doing the same thing, so he was also able to cut down their loading times in half!

Anonymous Developer

Cut-and-Paste

When programming something up, I often copy a line or more of code, sometimes several. I then need to maintain both pieces of code identically.

What I should have done is move the code into its own function (or template, or at a pinch, a macro). The reason I don't is simply laziness. If I have some code like

DrawLine(a-w,b-h);
DrawLine(a+w,b-h);
DrawLine(a+w,b+h);
DrawLine(a-w,b+h);

and I need to do it again for c and d, it's really easy to just cut and paste, and change the variables.

DrawLine(c-w,d-h);
DrawLine(c+w,d-h);
DrawLine(c+w,d+h);
DrawLine(c-w,d+h);

When really I should do:

void DrawSquare(float x, float y, float w, float h) {
DrawLine(c-w,d-h);
DrawLine(c+w,d-h);
DrawLine(c+w,d+h);
DrawLine(c-w,d+h);
}
DrawSquare(a,b,w,h);
DrawSquare(c,d,w,h);

It's fewer characters of code, but it's more typing, and I have to think about it more. So I frequently go for the cut-and-paste "solution" first simply because it gets me the result I want quicker. Both will work the first time, but I pay for my sins, and usually end up having to refactor it away later, sometimes after a few more needless duplications.

"PrintF Debugging"

I've always found the most useful tool in debugging code has simply been to print out various values and labels that indicate where in the code we are, and what we are doing. I always feel a little guilty doing this, knowing there's a debugger with thousands of functions all designed to help me debug, but all I use it for is to look at the call-stack when I crash. It's like having a toolbox with a thousand tools, and all I use is the hammer.

The problem with this is that the console output quickly gets cluttered with pointless debug strings, and it becomes hard to spot the ones that are important. So I've got to track down the code that's spewing output, which can be hard to do if it's just something like printf("%d\n",x);.

Then, when I find the offending printf, I don't want to remove it, as it often took me several seconds to type and I might need it again in the future. So I just comment it out. Eventually, I started writing my printfs as if they were comments, so in a lazy kind of way, I documented something about the code. Of course I'd rarely re-use a printf, and when I did, it usually ended up needing re-writing anyway as the code would have been refactored and variables would have been added or removed.

Thou Shall Not Overengineer

This one is unfortunately so common that a lot of people might not even think of it as a sin. It's just the way things are done. In several of my past projects, the whole company consisted of one team working on a single game. Yet somehow, programmers were separated into game programmers and core technology programmers.

The idea was that the core technology team would write all code as reusable, game-independent libraries and tools that could be used in any other project in the future. The game team would use those libraries and then build any game-specific code they needed on top of them. Each of them had their own leads, and, of course, each of them had slightly different goals and preferences.

In practice, the division did more harm than good. The code the core technology team produced was overly general (for projects that didn't yet exist), and didn't solve the exact needs of the game team. It added extra dependencies and delays, and made things that should have been very simple much more complicated.

Was it worth it in the long term? Not at all. That code was used for direct sequels, but new games never reused the libraries and tools.

The lesson we learned the hard way is that before you can write reusable code, it first needs to solve the needs of a project. Or, put another way, there's no point to reusing code if it doesn't do the right thing in the first place.

Making a game is hard enough. Now, I just concentrate on making the best game we can. Later on, we can talk about extracting and refactoring some existing code that might benefit another project, but never try to predict and future-proof technology.

Premature Optimization is the Root of All Evil...Or Is It?

It began on a project a long time ago, in a galaxy far away ... OK, maybe not that far away but still quite a few years ago. I was a fresh college graduate and a bit wet behind the ears, with a brain full of computer science goodness.

Throughout the whole project, I kept pushing optimizations off until the end. "We aren't going to need that," and, "We'll just optimize the hot spots on the profiler," I kept repeating to my co-workers. So work went on, and performance never became a priority.

Every so often, the frame rate would tank because of some new feature that was just introduced, so we would fix that to make it playable again, but just barely. Frame rates were around 10 - 15 FPS for most of the project.

Then the day finally came. We were in beta and we had to bring frame rate up. I fired up the profiler and ... there it was! A big hot spot. I optimized it, proudly ran some benchmarks again, and noticed I saved half a millisecond. Not bad. I repeated it a few times, but curiously, the profiler soon reported a rather worrisome flat graph. Apparently, there was no single place in the code that was accounting for more than 0.05 percent of the frame time. What was going on? That's not what they taught in university!

It turns out our bottleneck wasn't CPU cycles so much as it was cache misses. And because of our lovely, heavy object-oriented design, we had constant pointer dereferencing and traversing graphs all over memory.

We managed to gain some performance back by changing some lists to contiguous arrays and doing some prefetching, but overall, it was very difficult to meet the performance requirements we wanted for the game.

Lesson learned: there are some things that are better thought of from the beginning, and memory layout and cache coherency is one of those. Don't paint yourself into a corner by waiting until the last minute to start thinking of them.

Thou Shalt Not...

These are just a handful of sins, and there are certainly a lot more. But at one point in our careers, most of us in the programming field have committed almost all of them. If we hadn't, we wouldn't have learned our lessons!