Curb stomping code readability

Sam Hart

2010-05-06 14:57:12

I have many reasons for loving Python, but I think my favorite aspect of it is the inherent readability of the language. I mean, you really have to work hard to make your Python code unreadable (though, people mis-using decorators can make otherwise readable Python code extraordinarily unreadable...)

The thing about Python is how hard it can make working with other languages once you've fully ingested the Python kool-aid. The many aspects that make Python such a wonderful language to work with really amplify related weaknesses in other languages. For example, I always had complaints about Java, but I couldn't completely articulate my biggest complaint (how hard it is to make readable Java code) until I worked with Python for a few years.

Well, for the last two years I've been working the majority of my time in C# (doing games for Funavision). I have to say, I really don't mind C# as much as I expected I would- while it is obviously C/C++ and Java inspired in many ways, it has a good deal of sanity to it that reduces my frustration when working with it. It also helps that Visual Studio has handy features that make working in the language even easier.

However, it still has the big problems associated with making readable code. Inconsistent syntactic sugar (see C# switch/case), ability to hide entire code segments in property get/sets (grrrr), "Interface" objects that are supposed to be method free but which can still contain method-like code (see get/set previously), etcetera are all examples of language hurdles to readable code.

Well, a while ago I came up with what is probably the least readable C# code I've ever done. I'm not proud of it... but I've decided to share it anyway...

Ladies and gentle-jerks, I give you, my masterpiece: The embedded conditional!

if (GameConfig.BKGEffectsEnabled && GameConfig.ParticleBKGEffects &&
gameTime.IsRunningSlowly ?
timer_Diffraction > time_PerDiffractionDrawDelay :
timer_Diffraction > time_PerDiffractionDrawNormal
&& diffractionFx.Enabled && diffractionFx.NumParticlesAlive > 0)
timer_Diffraction = 0;

Yes, bitches, that is a conditional inside of another conditional. How's that for fugly?

I did try to indent to make it a bit more readable, but it's hard to get around that nested conditional. Whenever I re-read this segment of code this comes to my mind.

The really nasty bit here is that I kind of need everything that's in this conditional, and splitting it out in various creative ways generally involves extra overhead on some of the resource restricted platforms we're trying to support (which is bad when you have very high frequency code). Oh, I'm sure there could be a better way to do that, but considering our code-base is already over 40k lines of code, a good 75% of which I've written, and we have very real problems like enemy objects getting stuck in strange places on the screen, I just decided to move on and not fret about the nightmare that is the above since it works with minimal overhead.

Decomposing those conditionals

By way of a small explanation (penance, perhaps?) let me explain what's going on here.

In our upcoming second game we have all sorts of fancy graphical flourish effects going on. The aim of most of these effects is to distract the player (a great deal of the game depends on such distractions). One of the effects is something I'm calling "Diffraction" (although, that's not an entirely accurate name). This effect can be used to create light diffraction looking patterns on the screen (you can see them in our latest video of the game). Granted, it's not really doing anything mathematically complex or accurate, all it's really doing is combining a couple of clever sprite blending effects in ways to simulate the effect. Well, this diffraction effect can be resource intensive simply because it winds up doing triple buffering just for the effect every time it gets redrawn. In a game where multiple buffering is already going on elsewhere, you don't really want this extra triple buffering going on every frame as well.

So, there's really three things going on in the above code snippet.

User toggles for effects

First, we let the user turn off these (and other) effects. Could be they don't like the distractions of the effects, or that they have a system incapable of handling them (we plan on releasing on PC as well and, who knows, maybe someone with a 286 will try to play this game).

So the "GameConfig.BKGEffectsEnabled && GameConfig.ParticleBKGEffects" checks are to verify the user has the proper toggles on that this effect depends on. There isn't a single toggle for this effect, as it technically requires two other effects to be enabled before it switches on.

Code init checks

Skipping the elephant in the room for a bit, we see that the last checks deal with some basic code readiness. "diffractionFx.Enabled && diffractionFx.NumParticlesAlive > 0" tests to see if the diffraction effect object is enabled (meaning it's not in some indeterminate configuration state, which happens at the start of the game and happens whenever the user pauses the game and toggles the aforementioned effects in the config screen) as well as ensures we actually have some diffraction particles to draw (no need to re-draw the diffraction effects if there's nothing to draw).

Mise en abyme

Finally, we come to the nasty bit that fuglifies the rest of it- that damned embedded conditional inside the outer conditional.

gameTime.IsRunningSlowly ?
timer_Diffraction > time_PerDiffractionDrawDelay :
timer_Diffraction > time_PerDiffractionDrawNormal

As I said earlier, this diffraction effect is expensive and shouldn't be redrawn every frame. So, to throttle it, we put in a delay in there. It basically redraws every N seconds.

The problem is, sometimes that N seconds is still too fast (when there's other expensive effects going on). Because the diffraction effect is lowest priority of all the effects, it's the first one to take it in the shorts, and if we need to drop a bit more from a given cycle then the diffraction effect will be the first to go.

So, what we're doing is checking to see if the game is running slowly according to XNA (GameTime.IsRunningSlowly), and, if it is, we're using an even longer delay before we draw the diffraction effect.

So in the middle of our first conditional, we have a second conditional to pick which of two delays to use for throttling.

Again, I'm sure there's a better and more readable way to do all this that still doesn't compromise performance on some of the platforms we're aiming for. But what we have works and there are too many bigger problems to tackle (unfortunately) to spend much time making this more readable.