Engineering Game Development

The IGDA Is Not For Me And (Probably) You

The IGDA is having a rough time at the moment. The statements regarding 60+ hour work weeks by Mike Capps of Epic, the recent revelations regarding Tim Langdell and his aggressive (and often dubious) use and enforcement of his EDGE Trade Mark and the responses on this from Tom Buscaglia have done nothing to make the IGDA shine.

But these issues (and I’m sure others) are not the reason for this post, instead I wanted to talk about why, even if these issues were not sticking in the headlines, I don’t think the IGDA is an organisation that can support me and stand up for my rights, and the rights of my colleagues across the globe.

An International Organisation

Having an umbrella organisation, covering the needs of developers across the world sounds great. We are still a relatively small industry and you would think small enough that one group could speak for us all. But the games industry is a global industry, not a national one. The needs and requirements of a game developer in the US are vastly different from the needs of one in the EU. Lobbying for change, whether that be at government or company level, is hard enough when you are in one country, with one set of regulations and one set of work ethics. But to expect one organisation to cover all bases, for all developers is expecting too much and it is bound to start catering for one group over another. Even within the EU there are vast differences between what is legally allowed, what is expected and even developers work ethics and personal requirements can change from country to country or even state to state.

How many members of the IGDA Board do not live and work in the US? While it is possible for developers outside of the US to stand in elections and get voted in, how practical would that be? How often are meetings conducted outside of the States and with such vast time differences even remote conferencing can be difficult?

While it shows good judgement on the part of the IGDA that it has legal representation/advice in the form of ‘The Game Attorney’ Tom Buscaglia, the law from one country to another can be vastly different, or even worse subtle in the extreme. Tom can help the IGDA, but only in the US, and this is shown in some of the documents coming out of the Quality of Life SIG group.

TIGA (The Independent Game Developers Association) is a UK organisation that was formed to fight for the Games Industry in the UK, and is even making tentative steps into the EU. Even though it is an organisation whose main aim is to lobby the government, they are still fighting for change at different levels. It has a hard enough job doing this within one country, let alone within the EU. Expand this to include the US or Asia and the difficultly level doesn’t just start to increase, it becomes a brick wall.

Change always happens slowly, and it always starts with small steps. National industries need their own organisations, with their own members working for change in their area. There is obviously nothing stopping these organisations working together for the greater good of the global industry, in fact I would hope this would be the case. But smaller organisations, working on smaller scale change for their members is the only way to eventually get the wide spread changes that people seem to want.

Directors, Owners and Other Important People

The games industry is one headed up by CEO’s, company directors and financial movers. When awards are given, when technology is showcased or when magazine interviews take place, it’s nearly always the company owners, VP’s or directors that are on the front line. There is obviously nothing wrong with this and it is the way many industries are run as these are the stars, the well known people who the public are already familiar with. But look at the make-up of the IGDA board and you will see that these people are also responsible for their own companies or their own specific areas. And the IGDA think tanks and round-tables that are also headed up by people carrying out these duties on a day to day basis.

Whether we like it or not, companies are there to make money. The people leading these companies, on the whole, want to make a profit and they want the company to keep making a profit. And so do we, because a company in profit is a company which can continue to employ developers and continue developing games. Sometimes (and not always but sometimes) the needs of the rank and file conflict with the needs of the company. For an organisation that stands up for the industry’s wellbeing (and you would assume this means the people on the shop floor) it needs to be driven, and led, by people who do the work, or who’s only driving goal is the welfare of these developers. Being led by those who also heap-up the companies we work for, or having think tanks and discussions led by those who’s goals may be different to the people it represents will only lead to conflict. This is the reason why 60 hour work weeks are seriously being discussed rather than being instantly shot down by everyone present.

Taking an organisation I used to be a member of, the NUT (National Union of Teachers in the UK), as an example. It would be beyond comprehension for this Union (yes, I used the word Union…) to be headed by a group of Head Teachers or Government officials. It simply would not be able to function in a way that would benefit its members, and neither can the IGDA. This is the way an industry ‘rank and file’ organisation should be structured. Lead by those people working on the shop floor, those people that have relevant and recent experience of what it is like working in the games industry and being an actual games developer. Granted the NUT is probably a step to far for most, and I may even agree, but it puts a point across in a very obvious and clear manner.

To Wide a Scope

Future developers are probably still at college. Some of them may still be in school or about to graduate from University. Some of them will be on Games’ Degrees, traditional degrees or something totally outside the scope of what we see as a ‘requirement’. Either way, the people who will be coming into our industry over the next few years should be supported and we should ensure that whatever path they take they will have the skills, or access to the right information, to allow them to find and get through the industries door.

But this doesn’t mean they should be members of a Game Developers Association.

An association working for a group of people needs to be focused on those people. Work or special interest groups can look out for, lobby and support areas outside of its immediate scope, but as soon as you have paid or associated members you become responsible for their development and required to work for them and anyone else who has paid their dues (whether that is paid membership or any other type of membership requirement).

One thing the IGDA is good at is releasing its findings to the wider public. This is great, because it gives those people who are not members the opportunity to see how the organisation is working, what its goals are and how well it is doing meeting them. There is nothing stopping these findings being available in a more national organisation, available to students, people looking to get into the industry and anyone else who is interested. In fact doing this can help your cause as people who would otherwise not know that work was being done in certain areas can become involved in their own way.

But first and foremost, the main focus of a Game Developers Association should be those people working on and developing on the shop floor.

Local Chapters

Local Chapters are one of the saving graces of the IGDA and the IGDA brand. Having groups named after the IGDA across the globe, ideally encompassing nearly every development house out there is a great idea, allowing them to work together, access shared resources and drive the industry forward in their local area. It’s also a great way to get people involved as you have Chapter Forums (though my local forum seems remarkably quiet) and excellent chapters like London or Boston can really make a name for themselves. But the question remains - does the IGDA name lend enough credence to a chapter that not being associated with the IGDA would make the chapter either irrelevant or short-lived?

The Boston Chapter proves this isn’t the case, being a group already in existence before it became associated with the IGDA. The Midlands Chapter could quite easily be called the West Midlands Game Developer Association and with the same push people would still attend. The only issue I would see with this is sponsorship of events, as it may be easier to gain sponsorship to run or start a Chapter if it’s based on others already in existence.

But if there was a national association, working together with other organisations, and the chapters were based under that, you would be in the same situation of having named chapters working at a more local level. You could even say that if there was a more national, more local and more focused group of developers, more people would be willing to get involved, attend these events and make a difference.

Final Thoughts?

The IGDA is not a bad organisation. There are some questionable messages (both directly and indirectly) coming from the group and sometimes directly from the board, but on the whole the IGDA is full of hard working, honest people who really do want to work for the betterment of the games industry. Unfortunately, I think the time has come to accept that a global group, primarily based in one country, is not an organisation that can work for everyone. America, Europe and the massive industries in Asia simply cannot be grouped under one banner and one common goal.

But the ideas of the IGDA can be taken forward by more local, more focused groups, and by allowing these groups to work for the betterment of their local industry, and to work together for the good of the industry as a whole, we can have a group that works for people on the shop floor without causing the conflict and issues that a group like the IGDA is bound to encounter.

The points raised here are what I see, as a developer working in the UK, as the main reasons why the IGDA is not for me. Depending on your location, your current situation and previous experience, you may have different opinions, both for or against the IGDA. But we can only move forward if people are willing to discuss the issues we have, as an industry both nationally and globally, without the need to ‘close ranks’ and focus on one particular area or one particular issue. Hopefully this post can go someway to being involved in that.

Title image by Jsd_Quas0

Unit Testing an Object with Dependent Behaviour

It would be a good idea to start with a bit of history. In previous posts I’ve talked about our custom STL implementation which contains a basic vector, but also a fixed::vector which takes a fixed size and allocates all memory within that container (usually on the stack) and cannot increase or decrease in size. This comes with a lot of benefits and provides game teams with a more consistent memory model when using containers.

But it comes with a lot of duplication. Those two containers are classes in their own right, each with over 100 tests (written with UnitTest++) which is a lot to maintain and (more importantly) keep consistent. A change to one usually means a change to the other and additional tests and checks need to be rolled out. I wasn’t happy about this, so moved all the vector code into a base class and the fixed and normal vectors are now types based on that, with a different allocator being the only difference (I didn’t want to go for inheritance when a templated structure could do all the work).

That was the easy bit. But now, how do you unit test an object (in this case the vector) who’s behaviour greatly depends on an internal object (in this case the different allocator models). The allocators have been unit tested, so we know their behaviour is correct, but the behaviour of the vector will depend on the allocator it is using (in some cases it will allocate more memory - so it’s capacity varies - in others it won’t allocate memory at all - so it’s ability to grow is different). And this behaviour may be obvious or it may be much more subtle.

And it’s this which causes the problem. The behaviour should be more or less the same, but in some cases, the behaviour will be different. push_back/insert may fail, reserve may reserve more memory than has been requested and construction behaviour will greatly depend on the allocator being used.

So to start - the simple and easy option. Create all the unit tests for one type and then duplicate them for each different allocator we come up against. But we went down this road to reduce the amount of duplication so there is still going to be a large number of tests that are the same and need to be maintained. In fact you could say it would be harder to maintain because the behaviour in some tests would be suitably different rather than clearly the same.

So I started to look for a more intelligent approach to this.

The first thing was go back to the basics - what was unit testing for? Such basics are often lost in the grand scheme of things, but in this case the point of the tests was to make sure the behaviour was correct, or more importantly, what was expected. There are obviously other aspects to unit tests (which are outside the scope of this post) but this is my primary concern.

When thought about it from this level the conflicting behaviour becomes quite clear - or at least it should. We can split the various methods into fixed and dependant calls, ones which will be the same regardless of the allocator and those which depend on what is being used. If we cannot split the behaviour then we have a problem with either our interface or our implementation. Luckily the allocation code is quite contained, but there were little tendrils of code that assumed things about the allocation of memory that it shouldn’t. Not a problem as the tests will easily pick these out - I hope.

So I may as well start with the easy stuff to test, the stuff that I now know is common across a vector regardless of the allocator. Iteration, removal (since the vector doesn’t de-allocate on removal), access, equality checks (vectors are equal based on their contents) - in effect anything that reads, accesses or removes content, nothing else. And since I have tests for these already this is quickly done.

This is testing against the vectors behaviour as I know this behaviour is constant and will never change.

The hard part is now a little bit easier. Looking at what I have left I can easily see what I need to test. Since the changes in behaviour are based on nothing other than the allocator, we can easily extract that and create a couple of mocks specifying specific allocator behaviour. One which allocates the memory as you request, one which allocates a different amount of memory and one which returns no memory at all.

But now I’m not testing the behaviour of the vector, I’m testing the implementation of the vector - knowing how it works internally and how that affects its external state.

At first I tried to be a bit clever with this. Each test was extracted into a small templated helper function, with the checks being done in those. That way I only had one implementation of a test and simply passed in the vector, source data and expected values. I had to make a few tweaks to the UT++ source to enable checks to be done outside the TEST(…) call, but this was pretty simple.

At first this worked well. As long as the set of tests were the same across all objects using our various mocks it was fine. The same number of expected results, the same or similar  input values and the same general behaviour. But it started to get tricky when the tests needed to deviate. For example, when testing a vector that uses a fixed size allocator, I need to test that the vector works fine on the boundaries, boundaries which the other vectors I’m testing don’t have. So we either end up with a large number of superficial tests, or much worse, I forget the extract the specific test cases for the specific method of implementation.

But since there is nothing wrong with a bit of duplication, especially when each ‘duplicated’ test is testing a specific thing I don’t need to be so clever. By having very discrete tests based on the implementation of the vector means I am free to deviate the test suite as I see fit, when I know that the behaviour of the vector will be different based on the type of allocator it is using. It would be beautiful to be able to use the same tests to test each type, but frankly that’s just not possible when you have to understand the internal behaviour of the object to test it correctly.

So the final solution - duplicate some tests but only those that you know have different behaviour based on an implementation or an expected outcome. There might be a bit more code but it produces the best results, the best output (it’s much easier to see what has failed and why) and the easiest to understand.

If the object depended on multiple internal objects (a policy based smart pointer is an excellent example) then I don’t believe this solution would work. The more variations you have, the blurrier the lines between fixed and dependant behaviour become. In those cases I probably would expect to be writing many more tests based on the final object, simply for piece of mind.

You can only go so far with this, and unit tests are designed to aid you and give you a security blanket. In an ideal world (one where TDD rules all) you don’t need to know what the internal implementation is doing, only the final outcome. But if you are spending more time figuring out your tests (let along writing them) then you need to take a step back and reassess what you are doing.

Title image by fisserman.

Why We Use ReviewBoard For Code Reviews

I was going to write a couple of reviews covering the various code reviews tools I’ve tried over the past year or so, but the notes I made on each one seem to have mysteriously vanished. And since it wouldn’t be fair on the various tools we used to try and remember why we moved on, I’m just going to cover the one tool we stuck with… ReviewBoard.

Web Based
This was one of the features that I really wanted from a review tool. No installs, no applications that need to be running in the background, simply a web address to access and an e-mail address for notifications.

Easy To Use
When you first look at the UI it’s pretty obvious what you need to do and how you can review the work in front of you. As with any UI, it has it’s quirks but I’ve not had anyone say “I can’t use this!” like I did with some other tools we tried.

Conversations
It’s easy to have a conversation in ReviewBoard and this is what I always want to see when code is up for review. Clicking on other people’s comments and seeing a clear ‘Reply’ button and being able to see the entire thread on the main page allows people to dive right in.

Openness
ReviewBoard doesn’t block people who are not on the review from seeing the code and making their own comments. As long as they are signed up to the site they can see anything being reviewed and what has been reviewed in the past. If I’m writing an internal blog post about a feature or implementation I am working on, I always add a link to the code review allowing everyone to have their say. After all, there is a good chance that they might use the code at some point in the future, so why shouldn’t they?

Quick to Review
Once you get notified of a review you have been assigned, getting to that review is as quick as hitting the provided link and selecting the diff. to review. Everything is done in-page and with a click on a button your entire set of comments and reviews will be published for everyone to see. It can be hard to persuade people that reviewing code will make them more productive, and having a fast process can make the argument easier.

Actively Developed
Nothing is perfect and ReviewBoard does have its bugs, but when you can raise an issue on their bug tracker and know that someone will actually look at it you start to feel a little more secure about any problems you find.

Widely Used
Having only one group of people use a tool can sometime be a benefit as it can be tailored specifically for you, but that can be a dead-end. ReviewBoard is widely used by some pretty big companies and you know it stands up to a certain level of quality and that the development being done will be for a wide range of users.

Stable
I’ve used tools in the past that didn’t work. They would fall over, become unresponsive or simple not do what they should. I’ve not encountered any of that with ReviewBoard. A few people using it have found some issues that get in their way, but as I’ve already mentioned these are quickly noted and will hopefully be resolved in the future.

So those are some of the main reasons we’ve stuck with ReviewBoard. It’s very easy to get hold of and install, so I really would recommend that if you want to try something new, then give it a go.

What Else Did I Try?
While I don’t want to go into the whys and why-nots of each tool I tried, I thought I would list some of the tools we did try so you can give them a go. What might not have been suitable for us could pretty much fit the hole you are looking to fill, so go and check these out too.

SmartBear’s CodeCollaborator - Try this out, it’s fantastic
SmartBear’s Code Reviewer
CodeStriker

Thanks and remember to enjoy your code reviews!

Code Review Methods

In the last post I went over some of the reasons why code reviews are important in building a stable and secure project. I wanted to follow that by covering the different review methods we’ve used in the past and the ways in which each one worked or in some cases didn’t.

Paper Based Reviews
One of the simplest, and first, methods we used - simply take the code you want to review, print it out in whatever format you want (we found small A5 booklets worked best) pass them around and then start reviewing. Usually people would be given a couple of days to review the code before everyone involved would sit around a table and discuss what they had found. The advantage to this is that it gets people talking. You are sat around a table together and it’s a great way to get people participating. Unfortunately it also forces people into thinking they must contribute rather than simply being happy with what is in front of them and it’s a drain on resources and time. It’s surprising how long it takes to get code from an IDE into a suitable (and readable) format, especially if all you have is a word processor. And wasting all that paper is something that I simply don’t want to continue doing.

Forums
We also tried posting code onto an internal forum when the review didn’t justify a full sit-down session. This worked very well in making the code easily visible - even to people outside the review group though that might not be suitable in every case. It also reduced the time it takes to set up and cuts down on waste. But discussing specific parts of the code simply didn’t work. Referencing line 316 of PlayerCamera.cpp in a post makes it very hard for people to link to the work, and while forums are good for limited threaded discussions, trying to quote and re-quote just becomes hard to track, read and find out what needs to actually change.

Automated Code Reviews
By automated I mean using a tool that has been created specifically for code reviews, one which can display code correctly, generate or show diffs and allow people to reference files and lines of code easily. I won’t go over the actual tools we have used (I’ll do that next time) but suffice to say this is what we use and the method we have stuck with.

Of course, when using automated tools you have more scope as to when to do you reviews (do you review individual diffs, whole files or something in-between?) and I’ll spend the rest of this article going over the different methods we’ve used so far.

Blocking Code Reviews: Blocking code reviews are used when you want every piece of code reviewed before it is even committed to the repository. When a developer has made any changes to the code base they fire off a review and the developer cannot check in their work until everyone in the review group has flagged it as shippable. Any changes or requests need to be done before the code review can be submitted and checked into the repository.

This didn’t work. Developers need to get into a work-flow and having to constantly stop and wait for permission to continue or to review someone else’s code stops them in their tracks and is a serious drain on their productivity. And let’s face it. Game development might be a multi-million dollar industry but lives are not on the line if someone checks in something that might not be perfect.

Non-Blocking Code Reviews: The next obvious step. Every check-in needs to be reviewed so the review is generated, sent off to a selected group, but the developer is then free to commit the code with any changes or improvements being done in another commit. This is beneficial on two counts. The original developer is able to continue working, either on the same area of code or another without it conflicting on any waiting commits, and the reviewer is able to continue their work, leaving any reviews the need to complete for when it’s suitable, maybe performing a group of them in one go.

While this worked well in regards to making sure that everything is reviewed, we found that you needed to monitor the size of the review groups and to check that people are not being over-loaded. Filtering your reviews only to come to them at the end of the day with 10+ of them waiting for you can be quite disheartening.

Feature Based Reviews: This is generally where we sit when it comes to diff based code reviews and it puts the responsibility back into the hands of the developer. If the developer is working on a new feature then they are free to check in their changes with a view to putting the whole feature up for review when it is finished. This greatly reduces the number of reviews and for new code allows the developer to put the whole code into context. It’s very similar to paper based reviews but in a more suitable environment. But there is one caveat. It only works if there has been a decent design stage to the feature being written. Having people bring up basic design flaws when the thing it practically finished can be a killer blow but this is showing up the work flow process more than anything else.
This can obviously be extended to large scale changes that someone might be working on. The can check in their small changes, which a review done on the final product, maybe generating the diff based on a collection of commits rather than just one.

Bug fixes are another matter. Once the main implementation is done and people are into a bug fixing stage then these should be reviewed as and when they are being committed (usually in the same manner as the non-blocking reviews). This is especially important when it’s coming up to a milestone and is the way continually developed projects (for example with Middleware or Technology) will eventually spend most of their time.

Other Methods

Obviously there are other methods we could try.  Review buddies, review masters (one person assigned responsibility for checking all commits - sometimes this will usually be the lead or programming manager), e-mail based discussions, ”important changes only’ reviews, and plenty of other variations on the same theme.  We haven’t tried these because the automated reviews have been working so well for us.

Review buddies would be the one I’d like to try (alongside, rather than replacing, the automated review process) but I would usually have this done between two developers working on similar parts of the code - though I would hope these developers would be talking to each other already!

Review Groups
One thing I’ve not covered is who does these check-in reviews, which can be just as important as what is being reviewed.

Initially we used review groups, which contained 2/3 developers and were mixed quite randomly. Every review would be assigned to a random group and fired off. While this worked by spreading the work and the knowledge of the code base, it wasn’t producing the best results. Having one person involved in 2 out of 3 reviews for a particular feature, only for someone else to replace them on the last one would produce duplicated, or sometimes conflicting, results. And the new person isn’t as up-to-date with the feature as the original set of reviewers, which can limit the scope of the review and lead to it taking longer.

While this does spread the knowledge throughout the team, it isn’t necessary for everyone to know everything about the code base.

So now reviews are generally more focused, being sent directly to specific people who will have the best input on a particular feature. This does need monitoring to make sure people are not being over-loaded (or over-looked) but that’s a pretty simple management issues. People can also be asked whether they want to be involved in a certain review. Do they have a particular interest in that area, or does it use a new piece of technology that they would like experience with?

So that’s a brief over-view of the code review systems we’ve used over the past couple of years. We’ve settled on our automated code reviews as they produce the best results in the shortest time and give you much more scope in regards to how and what is reviewed. Choosing the correct piece of software to run this system is an important decision and I’ll go through the tools we used and most importantly the ones we stuck with in the next post.

Industry Broadcast Reaches 100

This is just a quick shout of congratulations to Ryan Wiancko for hitting 100 articles on Industry Broadcast.

If you haven’t visited IB before, then I would seriously recommend going over there and having a look at what’s on offer, or subscribing via iTunes. There’s some great content on there, whether you are a programmer, designer, artist or play any other role in this great industry.

I’ll Show You Mine If…

Code reviews have a bad reputation.  Claims of wasting time, pedantic reviewers and hot-shot programmers all crop up as reasons why teams shouldn’t and don’t bother reviewing any of the code that is being used in what could be a multi-million dollar project.  But why would you want to have a code review?  What does it bring to the project that would otherwise be missing - because that’s the point really, if it is a waste of time then should we be doing it at all?

The Reason For A Review
Every review needs a reason.  Throwing some code in front of a group of developers without purpose will generally end up with a bunch of incoherent comments, suggestions and ideas that might be good, but depending on the project or the time scale, totally irrelevant or unfeasible.  The following are just some of the reasons you might want people to look over a set of code.  There are, of course, many more reasons but I think the majority will fall under one or more of these banners, regardless of who is doing it or what you hope the outcome will be.

Finding Bugs
It’s funny, and it might just be me, but every time I’ve asked someone to put up something for the first time, at least one bug is always found.  It might be a misplaced define, or an = instead of an == or any number of things but there’s always one.  And I know that the time it took to post and complete that review will be much less than the time it takes for that bug to be found, a bug report written, the author to read the bug, find the code in question, do some tests and then finally fix it.

Obviously not every bug will be found by a review.  It might be 1 in 10, it might be worse or it might be better.  But the point is that bugs will be found that otherwise would sneak through into QA or milestone builds, and each one will take time to find, report and fix.  Time that could be better spent doing something much more productive.

Stability
While I can pretty much guarantee that a bug will be found in the first review someone does, I would also place bets on the number of bugs being found as the reviewing continues goes down.  The number of suggestions and improvements people make will become less demanding and more of a conversation.  This isn’t because the reviewers are losing heart, or that less code is being reviewed, it’s due to one simple fact - the build is become more and more stable as more reviews are done.

This could be down to a number of reasons, but one of the biggest reasons in my opinion is peer pressure.  Programmers are suddenly aware that they are not the only people who will see this code, that their peers will be looking at and critiquing the work being done.  And most people want to look good next to their peers.  This is not a bad thing despite the connotations ‘peer pressure’ can bring to the mind.  Mutual respect with your co-workers and the need to drive yourself forward are often driven by peer pressure, and when one or two people start to move forward, a good team will always follow.

Sharing Domain Knowledge
The more people know about a particular system the better.  Illness, team moves and resignations will always be part of the work environment and it means that the key team member you are relying on could be gone before you know it.  And if you need someone else to dive in there and fix a couple of issues that have been found by QA, how long do you think that will take?

Code reviews can help.  As code is written, modified or fixed it is being looked at by more than one person.  The author has to explain their decisions, how things work and how things plug together.  Because of this you might have 2 or 3 people on your team that are happy to dive in and help.  Even if you need two developers to pair because the knowledge is spread between them, it’s still better than having no-one know what the heck that particular piece of code is doing.

Sharing Experience
People learn best when they are surrounded by people they can learn from.  Books and blogs are excellent resources (as an avid reader of books I would say that), but there is no substitute for someone sharing their years of experience with you in a 5 minute conversation.  Having people suggest different ways to structure, optimise and write code might seem a little late when it’s already written, but it’s something people can take with them into their next task, rather than going back and re-writing large chunks of code.

And it works the other way too.  There is nothing better than graduates coming into a company with their crazy ideas about how code should be written or used and teaching the old guard a new trick or two (C++? Inheritance? Madness I tell you!).

Coding Standards
This is one of the reasons people hate code reviews.  They don’t want them to turn into conversations about where the bracket should go, or if that variable should have an ‘i_’ prefix or not.  And on the whole they are right, and if this is what your code reviews turn into then you are simply ‘doing it wrong’.

But this has its place.  Developers new to the team can be given documentation about coding standards, but it is one of those things that tend to easily slip, and there is no reason why this shouldn’t be picked up in a review.  If it continues and every review becomes about how the standards are not being met, then you have an issue that should be resolved outside the review before it makes the whole process a pointless and much less useful exercise.

Problems With Code Reviews
Of course code reviews have their problems but then what doesn’t?  Most of the time these have nothing to do with the actual review process and more to do with the management of the team, or what has come before.

• Arrogant reviewers & victim syndrome - Code reviews are there to improve the code base for the benefit of the team.  They are not there to ‘go on the attack’, to rip someone’s code apart or to make someone feel like they are simply not good enough.  If you start to find that one or two programmers are simply making blanket statements or not taking anything on board then this is certainly a management issue and should be resolved outside the review process.

• Pointless reviews - This was pretty much covered in the Coding Standards section, but there is always a chance that code reviews start to only skirt the surface of the code without looking at anything to deeply.  If this starts to happen, you need to reassess what each review is for and what each person’s role is.  And remember, there is nothing wrong with simply saying “This looks great”.

• Lack of outcomes - People will argue, and people will disagree, especially when it comes to code.  ”You should do it like this”, “I don’t want to change that because…”.  Discussions are great and should always be encouraged, but at some point there needs to be a decision.  You have to know who has the final say - whether this is a manager who is on every review, or a senior who has the last say.  A simple comment along the lines of “That’s a great suggestion for the future” can put an end to most arguments with most people being happy with the outcome.  If they are not then again this needs to be resolved outside the review process before it goes any further.

• Lack of time - It’s an important milestone week, code needs to be fixed quickly and you don’t have time to review it.  This can happen and sometimes you need to take it on the chin.  But isn’t this the time when it’s even more important that everything is checked and double-checked?  If you have a milestone week and your changes are coming thick and fast, then maybe you don’t have time to review everything, but why is so much changing at such a critical time?

Using Your Own Tools

This is the true story… of seven(ish) programmers… picked to make a game… to work with the tools they created… to find out what happens… when tools programmers… start using the tools they make… in the real world.

Ok, so it’s terrible but I couldn’t resist - it puts across exactly what I’m about to describe.  Last week we did an interesting experiment on BlitzTech by taking a group of our dedicated tool programmers and asked them to use the tools they develop to make a game in exactly the same way a BlitzTech end user would.

But what can you get out of this?  A day certainly isn’t long enough to stress test a large scale SDK (and we have some great games doing that for us already) and to get a true appreciation of a toolset you certainly need a good length of time to see everything.  But what it does is take people out of their comfort zone, away from the areas that they are most comfortable with and gives them a real opportunity to experience much more of the environment that houses their work.  It’s also a very good chance to get solid feedback on the accessibility and initial impressions of a set of tools.

So was it a success?  I would certainly say so, and it’s something I wish other tool developers would do.  One group of people will have a very different outlook on an application than another group - which is why we also have a group of artists on the BlitzTech team full time.  By making sure that the people creating the tools actually use them (hummm, that sounds very similar to one of the main unit testing advantages) will always lead to a better user experience as people appreciate exactly what is needed and how the tools will be used.

So will we do it again?  I certainly hope so, with a different group of programmers, but even extending it so the last group can take what they did even further will simply allow us to expand on what was worked on and give the group a chance to see even more.

And that can only be a good thing.

Continuous Integration - Taking It Further

Our current Continuous Integration process works quite nicely and has greatly benefited our team, but these systems can always be extended and improved upon.  The following post will simply cover some of the ideas I have for the future and how it might improve the whole process.

Visual Unit Testing

Visual Unit Testing is the process of creating small, scripted tests that run the game and allow you to test that certain states have been reached.  For example, having a test which runs the game, scripts the input so one character shoots another and then tests if the enemy is dead or the score has been increased would be a very basic test.  It could also be used to perform long soak tests with more intelligent input rather than the kind of tests that are used at the moment.

Allowing the process to take screen shots and post them to a web server with the test results would allow a huge number of systems to be tested in the environment they will actually be used in.  Extending this to allow QA Technicians to create rafts of tests would also remove the need to continually test the more mundane aspects of games, such as does the menu flow behave as it should, do the leaderboards work as intended etc.

Obviously running this process would require a large amount of time, especially if run on multiple platforms.  But it could easily be hooked into the nightly build process meaning hours of testing would be carried out when no-one was in the office.

Some Arcade titles originally supported simple scripting using Lua to perform automatic controller input, but this wasn’t taken far enough, and the BlitzTech engine now has its own state machine system which would most likely be used instead.

Automated Code Reviews

Teams at Blitz carry out code reviews at different times and to different levels, but one thing that is usually done is checking that the code conforms to a given Blitz coding standards (with people moving around teams as they do, a consistent standard is pretty much essential).  But again, checking something that is based against a set of rules is something that an automated system is much better at that our human co-workers.

Adding a code metrics step to the Cruise Control build step, meaning the code is run through after every check-in could catch if the code is not standards-compliant below a certain threshold.  If this was the case, the build would simply fail (and this step could be removed if we had a designer/artist build machine as discussed in a previous post) and the programmers would have to re-examine the code they posted.

While this could seem quite draconian, it would free up code reviews to concentrate on the bigger picture, rather than getting stuck in the rut of commenting on the use of ‘m_’ or class name pre-fixes.

Extending The Nightly Build

As I mentioned in Extending Cruise Control we collect information on how often the build is broken and the ratio of who breaks the build the most.  We also generate a massive amount of information by using Subversion, such as check-in rates and lines changed per check-in etc.

The nightly build could easily be extended to incorporate all this information and, along with links to the Visual Unit Testing web service, could produce reports on the activity and stability of the game for that day.  This would be a very useful tool for the programming managers, especially as some people already use this information, but it is usually done on an ad-hoc basis when the tools are manually run and the information manually collated.

Automatic CI Support

A large portion (at the moment pretty much all) of my current work is involved with extending the Arcade technology I developed and integrating it into the BlitzTech engine.  It has the ability to automatically generates the required configuration files to set up a Cruise Control server, but the current plug-in system (like adding automated e-mail generation, stats collection etc.) is pretty much set up to do only what it needs to do, without much flexibility.

I fully intend to make all these plug-in’s come as standard, allowing developers to cherry pick the elements they want and hook them into their CI process with very little work.  While people may start to ask why I do not use NANT since they are all hooked into CruiseControl.net, I can simply do more, and do it faster, with Ruby.

So I have a few ideas on where I want to go with our Continuous Integration process in the next few months (years?) but there is nothing like lofty goals to keep you going.  Visual Unit Testing is obviously the biggest, and will require the largest amount of time to developer, but will generate the biggest payback once it’s being used to its fullest potential.

So what kind of things would you like to see in a CI process?  Or is there something your process is doing that you think is pretty special?

Continuous Integration - Self Testing Builds

So far we have a process which guarantees that the game builds and that the assets can be generated.  We have processes for doing this automatically and for making sure people are as up to date as possible.  But there is still one question which none of this answers…

When I run the game, will it actually work?

Games are a mass of systems, all interacting together to produce something that is fun and interesting to play.  It’s QA’s job (on the whole but not exclusively) to make sure that when it plays it plays well and there are no glaring bugs, but if we are automating so much, we can also automate some of the testing too?

Unit Testing

A Self Testing Build in this case is really just a fancy term for Unit Testing, something a lot of people have heard of but just as many havn’t.

The basic premise behind this is that at the end of every single compile small pieces of code are run that test various systems in the game.  These are run as post build steps so any changes to the code are instantly tested against the tests that already exist.  If some code has changed that makes one of the tests fail then the whole build fails.  This means Cruise Control fails, the nightly build fails, but most importantly everyone knows it’s broken and they should avoid updating until it is fixed.

The following are a couple of very simple tests, that are run every time the code changes to make sure the vector’s behave the same and as expected.  This might look simple (one of the main reasons people don’t unit test is because it will ‘obviously work’).  But with vector code being platform centric, a change on one platform can effects all the others.  The tests are written using UnitTest++ which is the library we use to write all our tests.

Feed back to the developers tells them something has broken, the test results are displayed in Cruise Control and the person responsible instantly sets about fixing the build.

Game Based Unit Testing

It’s often difficult to think about games from the point of Unit Tests but it often results in code being written in a more encapsulated and less coupled way, which in itself results in code being easier to write and maintain.  But as an example, the following could easily be tested in a small suite of Unit Tests…

• Does and AI character ’see’ the player when they are in front of them
• Is the player ignored if they are outside the enemies cone of vision?
• What state do they enter when they hear a noise?

By testing the under-lying logic of the system we can be confident that when something breaks we will have a better idea of where the problem lies that having to look at every system at every level.

Running Tests On Different Platforms

Unit Tests are only useful if they are run often and on the platforms the code is destined to be used on.  This can cause problems when trying to run Unit Tests on development kits as the code usually needs to be loaded onto the machine before it can be run and the results fed back to the host.  While this might only take 10 seconds, doing it after every build would be a massive drain on a programmers time.  While this isn’t a problem on the PC platform, we generally expect the console tests to be run as part of the nightly build when it has all the time it needs to run and feed back the results.

But, even if it isn’t being run on every platform all of the time, we still have a system running on the PC and some testing is still better than no testing at all.

So this is how the Blitz Arcade system adds a level of testing to the Continuous Integration process.  It’s one of the hardest parts of the process because Unit Testing does take a while to get into, and to understand how code can be tested when at first it looks like it is a series of black box objects.

In the final part, I’ll take a look at what I would like the process to do in the future, and the kind of things that would be cool to have in a fully integrated Continuous Integration process.

Invincible Tiger: The Legend of Han Tao

Blitz Arcade’s next title, ‘Invincible Tiger: The Legend of Han Tao’, was announced last week and it’s been on show at the New York Comic Con over the weekend.

It seems to be getting an excellent reception from the people who’ve had a hands-on play (and have been lucky enough to get into Comic Con!) and I’ve come across a few preview links on Twitter that I thought would be cool to share on here.

• Joystiq
• GameSpot
• AskMen
• PalGN

There’s also a TwitPic of the demo booth at the NYCC Namco-Bandai stand (taken by Christian Arca and posted by Mike Bithell)

Here’s a couple of screen shots taken of the game that have already been released to…