July 13, 2008

Collaboration

Disclaimer: This post might come across a bit like a product endorsement. My goal here is to endorse a development technique, not a specific product.

This post isn't directly related to game AI, but it's too important not to talk about it.

---

The scale of game development has grown by orders of magnitude since I joined the industry in 1994. As a result, the amount of code we deal with on a daily basis has grown enormously. Engineering teams have grown ever larger, and it's become increasingly challenging to deal with the massive game engines of today and to find engineers with the skills required to work effectively in every part of the codebase.

(Yes, lots of developers have tried to get around this by using licensed engines, but that's a topic for another day).

As a result of this huge increase in complexity, studios have started to recognize the need to find ways to improve training and teamwork. New hires need to be brought up-to-speed on the company's coding standards and the workings of all of the different parts of the codebase. There's also been a growing realization that all the engineers need to continually improve one another's skills and to eliminate as many bugs as possible before the product goes into Quality Assurance (QA).

There's been no shortage of games that have suffered as a result of engineering teams that found themselves on the sharp end of the scale problem. Technical failures have resulted in botched product launches, product cancellations, and even studio closures.

Surprisingly, as an industry, we've been very slow to address the problem.

Everyone recognizes that training and teamwork are key, but there's no consensus on how to go about improving those.

A few game developers have experimented with "extreme programming," but usually without much success. The developers I've spoken with who tried "XP" quickly grew frustrated with the pair programming aspect, which requires two engineers to do the job that one engineer should be able to do. In most cases, this just wasted valuable engineering time. They also claimed that XP ultimately only acts as a tutoring program to bring the skill of the most junior engineers up to the level of the most senior engineers, and it really had no benefit for the most senior engineers.

Other developers use code reviews to various degrees. Many of the studios I've visited over the past month claim to be using a code review process to some extent -- for all checkins, for large checkins only, or only for new programmers for the first three months.

That process can have enormous benefits, but it's also a huge pain in the butt. It's hard to get multiple programmers together in the same room at a time -- particularly since an effective code review usually requires two or more additional engineers to perform the review. Code reviews are most important toward the end of the project, when people have the least amount of time to participate. And having to look over someone's shoulder to read their code -- potentially for several hours -- can be really tiresome.

But there's a better way, and it's online collaboration.

At a previous employer, we used SmartBear Software's Code Collaborator™ to review all checkins. Every Perforce changelist had to be reviewed by two additional engineers before it could be checked into the repository.

When you're ready to check something in, you go to Perforce, right-click on your checkin to create a code review, and invite two or more other engineers to review it. Those engineers then get an e-mail, and that e-mail takes them to a web page that shows them your summary of your checkin and allows them to review all the diffs of the files you changed. They can then add comments or open "defects" for any of your changes to tell you what to fix before you check it in -- "This looks like a memory leak," "Please check for NULL here," "This is a coding standards violation," and so on.

The ostensible reason for this process was to get more engineers familiar with different parts of the codebase that they wouldn't have had any exposure to otherwise.

What we found was far more surprising -- we uncovered an amazing number of bugs before they ever reached QA.

Furthermore, the bugs were coming from everywhere, not just the junior engineers. All the engineers, including all of the most senior developers, had plenty of room for improvement.

Over time, it made all of us better engineers by making us more paranoid about potential sources of engineering defects. We learned to look at our code with a critical eye -- "What would Steve say about this code? Yeah, I'd better fix that ..."

There are always times when it's important to discuss things in person. Sometimes you do need to walk over to somebody's desk to discuss code changes in-person. But 90% of the changes are smaller or more obvious changes that don't need to be done in-person. Collaborator allows you to fire off a review that can be performed at the reviewers' convenience -- they can take as much time as they need to inspect your code, and they can fit it into their own schedule when it's most appropriate for them to do so.

I can't possibly recommend online collaboration more highly.

• Collaboration removes defects before they reach QA, saving time in testing and improving the integrity of the codebase.
• It trains developers to spot errors they might not see otherwise, and makes them much more cautious about their checkins.
• It brings all engineers in line with the company's coding standards.
• It makes engineers more familiar with parts of the codebase they might never see otherwise, breaking down the "my code/your code" silos and giving your engineering team much broader exposure to their own codebase.
• Unlike in-person code reviews, it allows reviewers to review changes at their convenience, whenever their own schedules allow them to participate, and to do so at their own pace.

Posted by PaulT at July 13, 2008 11:51 AM
Comments

Hmmm...

Although I'm not as quick as you to write of pairs programming, I can see the benefits to this.

However, a process is only as good as the people who are using it. And making sure people use this accurately and consistently would be the challenge, as always.

Posted by: Dave Mark at July 14, 2008 07:08 AM

Part of what makes the process work is that everyone is working closely on the same code and reading each other's feedback, so if someone is giving poor feedback or not giving enough feedback, everyone notices.

The one problem we did run into was engineers who were too busy to do reviews or sat on them for too long, and in that case, it's important to make sure the engineering lead is managing the process, and can reassign the reviews to other people with more time and/or get the engineer in question to make the reviews a priority.

Posted by: Paul Tozour at July 15, 2008 01:22 PM

I like it. I never heard of that software before, but it sounds really useful. We never really had a checkin review procedure, which I think was pretty silly on our part, but this sounds like it might actually help people improve as a side benefit to the review process.

Its also a technological solution, which sits well with me :)

Posted by: Phil Carlisle at July 23, 2008 03:12 AM

Guido van Rossum, of Python fame and working for Google, developed a code review tool for Google named Mondrian. It is now open source and available as Rietveld at http://code.google.com/p/rietveld/
Useful for those using Subversion.

Posted by: Jeroen Ruigrok van der Werven at August 1, 2008 06:50 AM