Main Area and Open Discussion > Living Room

Doom 3 Source Code - The neatest code I've ever seen

(1/2) > >>

Josh:
This is a story about Doom 3's source code and how beautiful it is. Yes, beautiful. Allow me to explain.

After releasing my video game Dyad I took a little break. I read some books and watched some movies I'd put off for too long. I was working on the European version of Dyad, but that time was mostly waiting for feedback from Sony quality assurance, so I had a lot of free time. After loafing around for a month or so I started to seriously consider what I was going to do next. I wanted to extract the reusable/engine-y parts of Dyad for a new project.

When I originally started working on Dyad there was a very clean, pretty functional game engine I created from an accumulation of years of working on other projects. By the end of Dyad I had a hideous mess.
In the final six weeks of Dyad development I added over 13k lines of code. MainMenu.cc ballooned to 24,501 lines. The once-beautiful source code was a mess riddled with #ifdefs, gratuitous function pointers, ugly inline SIMD and asm code—I learned a new term: "code entropy." I searched the internet for other projects that I could use to learn how to organize hundreds of thousands of lines of code. After looking through several large game engines I was pretty discouraged; the Dyad source code wasn't actually that bad compared to everything else out there!

Unsatisfied, I continued looking, and found a very nice analysis of id Software's Doom 3 source code by the computer expert Fabien Sanglard.
--- End quote ---

Source URL

Renegade:
Interesting article. I do like seeing good style as I like to imitate it when possible/practical. I still put braces on a separate line though, except when I'm writing in MonoDevelop, then I often put them on the same line. I tend to follow the IDE's style most of the time.

f0dder:
He has some decent points, but on other points I'd say "he'll get wiser" :) - Carmack himself has also replied, stating that "In some ways, I still think the Quake 3 code is cleaner, as a final evolution of my C style, rather than the first iteration of my C++ style" and also "In retrospect, I very much wish I had read Effective C++ and some other material." - which to me translates as "this is not how I'd do it today" and definitely not being idiomatic C++ all the way through.

A few comments...

Unified Parsing and Lexical Analysis - i.e., using (the same) text format for all resources). Shawn praises that, but here's what Carmack has to say about it:
Fabien Sanglard - So far only .map files were text-based but with idTech4 everything is text-based: Binary seems to have been abandoned. It slows down loading significantly since you have to idLexer everything....and in return I am not sure what you got. Was it to make it easier to the mod community ?

John Carmack - In hindsight, this was a mistake. There are benefits during development for text based formats, but it isn't worth the load time costs. It might have been justified for the animation system, which went through a significant development process during D3 and had to interact with an exporter from Maya, but it certainly wasn't for general static models.
--- End quote ---
...might have been a decent compromise keeping source material in text format, but create a binary representation as well - not necessarily fully specialized formats for each resource type, but a model like XAML/BAML might have been a natural fit?

Const and Rigid Parameters - pretty much spot on. C++ style const specifiers is something I miss in other languages. It's also nice to see that Carmack uses const-ref for input and pointers for output, it's IMHO good practice. It does mean you need null-checking, but IMHO it's an OK compromise (the stuff I do with output parameters tends to be hard to end up with a nullptr for).

Minimal Comments - pretty spot on, IMHO.

Spacing - disagree. The additional code-on-screen I'd get from putting braces on the same line doesn't matter too much... the readbility drop from cramped code and not being able to line up braces visually weighs a lot more. And I like blank lines between logical chunks of code. Dunno if there's been done any studies on this or if it's just down to personal preference, but my approach works a lot better for me :-). Oh, I fully agree with always using braces, even for single-line statements.

Minimal Templates - I'm a bit mixed with regards to this. Parts of the STL are somewhat sucky (remove+erase is a good example), and before C++11's lambdas, using std::algorithm was often extremely clunky and ugly. OTOH, for the most part the STL datatypes are easy to use and you get decent enough performance out of the box. Now, if you have code that's extremely sensitive to LORw or benefits massively from pooled allocation (either for speed or for avoiding heap fragmentation), it might make more sense to roll your own rather than mucking around with allocators and whatnot. But I'd definitely default to STL for 'anything normal'. And auto is a really great new feature, it doesn't make code hard to read (quite the opposite!) unless abused.

Anyway, Carmack being sceptical of STL probably made a lot of sense back when they started the doom3 codebase (game released in 2004, so several years before that - that would probably mean VC++ from VS.NET2002 (at least during start of development, perhaps VS.NET2003 for release?)), there's been several bugs and performance problems in STL implementations over the years... but it's 2012 now.

Remnants of C - getters/setters are often overkill, but I'm not fond of Shawn's examples. For immutable objects, having fields public can be OK (though one might argue that for the sake of binary compatibility for future upgrades, it might be better to use an accessor function anyway). But direct access to mutable fields? Ugh. I guess it's mostly a code smell to me since I tend to belive mutable objects implies "complex stuff", where you'd want some logic attached to the action of mutating.

StringStreams are ugly, but printf is unsafe - solution? use some safe formatting code. Been a while since I took a look, but there's several to choose from depending on your speed/flexibility needs.

Horizontal Spacing - pretty much agree.

Method Names - somewhat agree. I do prefer function names that read like English, but for simple & common & well-defined methods like size() and length(), I prefer not having the get prefix. In general, I'm not fond of getters/setters, I find that they read less naturally - still not sure what the most elegant solution is. I've toyed around with the idea of simply naming the accessor methods from the field name, which does read nicely... but is somewhat non-standard. Oh, and it feels wrong that the 'setter' functions are hard to discern from other functions, and you lose the value of having getXxx and setXxx methods grouped in auto-completion (which is nice for discoverability in a big codebase). ObjectPascal and C# properties are nice.

And finally,
Yes, it's Beautiful - the codebase might very well be, but I don't find any of Shawn's examples beautiful in themselves, more along the lines of "this looks like decently engineered code" :)

ewemoa:
Wow, thanks for the detailed comments :up:

Renegade:
I'm not really fussy on a lot of things, but I do really agree with him on method names. I like "VerbSomething" (or "verbSomething") as it really is nice to group similar methods alphabetically and to give you a quick look at what it does. I also like things like "OnSomeEvent" too. It's really just aesthetic. My main concern is that it works. :D

Navigation

[0] Message Index

[#] Next page