Home | Blog | Software | Reviews and Features | Forum | Help | Donate | About us
topbanner_forum
  *

avatar image

Welcome, Guest. Please login or register.
Did you miss your activation email?

Login with username, password and session length
  • December 03, 2016, 01:44:50 PM
  • Proudly celebrating 10 years online.
  • Donate now to become a lifetime supporting member of the site and get a non-expiring license key for all of our programs.
  • donate

Author Topic: Doom 3 Source Code - The neatest code I've ever seen  (Read 2169 times)

Josh

  • Charter Honorary Member
  • Joined in 2005
  • ***
  • Points: -5
  • Posts: 3,395
    • View Profile
    • Donate to Member
Doom 3 Source Code - The neatest code I've ever seen
« on: January 16, 2013, 06:56:50 PM »
Quote
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.

Source URL

Renegade

  • Charter Member
  • Joined in 2005
  • ***
  • Posts: 13,220
  • Tell me something you don't know...
    • View Profile
    • Renegade Minds
    • Donate to Member
Re: Doom 3 Source Code - The neatest code I've ever seen
« Reply #1 on: January 16, 2013, 07:38:44 PM »
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.
Slow Down Music - Where I commit thought crimes...

Freedom is the right to be wrong, not the right to do wrong. - John Diefenbaker

f0dder

  • Charter Honorary Member
  • Joined in 2005
  • ***
  • Posts: 9,029
  • [Well, THAT escalated quickly!]
    • View Profile
    • f0dder's place
    • Read more about this member.
    • Donate to Member
Re: Doom 3 Source Code - The neatest code I've ever seen
« Reply #2 on: January 17, 2013, 02:56:47 PM »
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:
Quote
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.
...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" :)

- carpe noctem

ewemoa

  • Honorary Member
  • Joined in 2008
  • **
  • Posts: 2,845
    • View Profile
    • Donate to Member
Re: Doom 3 Source Code - The neatest code I've ever seen
« Reply #3 on: January 17, 2013, 06:53:56 PM »
Wow, thanks for the detailed comments :up:

Renegade

  • Charter Member
  • Joined in 2005
  • ***
  • Posts: 13,220
  • Tell me something you don't know...
    • View Profile
    • Renegade Minds
    • Donate to Member
Re: Doom 3 Source Code - The neatest code I've ever seen
« Reply #4 on: January 18, 2013, 04:07:00 AM »
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
Slow Down Music - Where I commit thought crimes...

Freedom is the right to be wrong, not the right to do wrong. - John Diefenbaker

f0dder

  • Charter Honorary Member
  • Joined in 2005
  • ***
  • Posts: 9,029
  • [Well, THAT escalated quickly!]
    • View Profile
    • f0dder's place
    • Read more about this member.
    • Donate to Member
Re: Doom 3 Source Code - The neatest code I've ever seen
« Reply #5 on: January 18, 2013, 10:24:23 AM »
Renegade: "verbSomething" isn't necessarily always the best, though, and especially not in the case of getters...

if( getOptionEnabled() ) versus if( isOptionEnabled() ) versus if( optionIsEnabled() ) :)

IMHO option #3 quite clearly reads best, but #2 is probably the pragmatic solution wrt. IntelliSense support.
- carpe noctem

Renegade

  • Charter Member
  • Joined in 2005
  • ***
  • Posts: 13,220
  • Tell me something you don't know...
    • View Profile
    • Renegade Minds
    • Donate to Member
Re: Doom 3 Source Code - The neatest code I've ever seen
« Reply #6 on: January 18, 2013, 06:06:50 PM »
^^ I like your #2 there. Having the verb in front (most often) let's you skip remembering and lets the code completion kick in. For boolean values, "is" creates an additional level of clarity as it set up a yes/no question, which makes it far better than "get".
Slow Down Music - Where I commit thought crimes...

Freedom is the right to be wrong, not the right to do wrong. - John Diefenbaker