[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.
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.
No it wasn't a game about mums ... although I wonder if there is a game there somewhere ...
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( );
int GetNumberOfCharacters( void );
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.