Sunday, August 23, 2009

Heisenbugs, or Why const-correctness is more than pedantic esthetics

Heisenbugs, or Why const-correctness is more than pedantic esthetics

I recall, on more than one project, making the mistake of writing a const-correct serialization system, only to find that 90% of the code in the game wasn't const-correct already. It can be a long trip down that rabbit hole.

For one title in particular, the game needed to save world state. The "World" code was ported from a previous title, and worked pretty well for the game in progress.

void Game::save(const World & w) const
{
    FileStream fs;
    
    const Player * p = w.getPlayer();
    if(p)
    {
        fs << *p;
    }

    int objectCount = w.getObjectCount();
    for(i = 0; i < objectCount; ++i)
    {
        GameObject * o = w.getObjectAt(i);
        if(o)
        {
            fs << *o;
        }
    }

    // etc...
}

It seemed like straightforward logic that shouldn't take long to implement and test. Of course, it didn't compile. I'll contrive some of the World interface code to protect the guilty:
class World
{
public:
    Player * getPlayer()
    {
        return m_player;
    }

    GameObject * getObjectAt(int index)
    {
        return m_objects[index];
    }

private:
    Player * m_player;
    CustomFastArray m_objects;
};

Well, the problem here is obvious, Game::save() is passed a const reference to a World object, but the accessors for Player and Objects do not provide a const guarantee that retrieving them won't change the state of the World object. Decorating the accessors seems like a good place to start.
class World
{
public:
    const Player * getPlayer() const
    {
        return m_player;
    }

    const GameObject * getObjectAt(int index) const
    {
        return m_objects[index];
    }

private:
    Player * m_player;
    CustomFastArray m_objects;
};
After this change, all hell breaks loose in the compiler output. About 25% of Player and 5% of GameObject implement const correct accessors that are used in the << and >> operators for those types. Another 30% of the code use those accessors to actually change the objects returned. What started as a 2 or 3 hour task has balooned into 2 or 3 days of work correcting all of the code. The team lead makes an executive decision, "Yes, it's nice to have const correct code, but so much of what is in place already is incorrect that it requires too much time to fix."

Time is something there is never enough of on the schedule when making a game. Proponents of writing const-correct code are lambasted as pedants that want const for const's sake. There may be some truth to that, but the decision was made to simply pile on more const-incorrect code to a system that was already "too big to fix".

The day of reckoning arrives when QA finds an obscure save/load bug. They would play for a little while, save the game, play some more, the load the saved game. The bug was that the loaded game wasn't the same as the game that was saved. After 4 days and a trail of tracking down several layers of cause-and-effect that is too long to describe in this article, the problem turned out that the World object was a Heisenberg freak-show. The act of observing the world, namely Game::save(), was changing it!

That project never did receive const-correct treatment, and there were more Heisenbugs in the system. Overall, the cost to track down the bugs was greater than the cost to simply refactor the existing code and use the compiler to eliminate a whole class bug that is difficult to find and fix.

Const-correctness is more than esthetics for computer science wankers. If methods are written with strong const guarantees, new code that *does* try to mutate an object won't survive past the first attempt to compile it. The class design is preserved not by convention, but by the rules of the language.

The lesson to be learned here is: it always takes less time to fix code to let the compiler find bugs than it does to hope problems are caught at runtime during QA, or worse, after the game ships!

No comments:

Post a Comment