Wednesday, 3 December 2014

How did people ever complete Incursion?

Sometimes when I fix a bug, I wonder how people ever completed Incursion. Take this one for example.

The definition:

struct EventInfo {
    EventInfo& operator=(EventInfo &e) { /* Memory image copy for the int/bool/ptr members */
        // Implementation doing a quick copy, allocating instances of subobjects.
        return *this;
    }
    // Rest of the normal code for this struct.
}

One problem situation:

bool Magic::isTarget(EventInfo &_e, Thing *t) {
    EventInfo e = _e;
    // Rest of this function.
}

When isTarget exits, then _e is corrupted. This is because EventInfo only provides an assignment operator overload. However an assignment in a variable declaration uses the copy constructor, and in the absence of one, does a bitwise copy. Pointers to sub-objects then become shared, and the first version of EventInfo to be destroyed will destroy those sub-objects assuming itself to be the owner. In this case, it will be e which as a stack variable will be freed when the function returns.

Incursion is littered with perhaps a dozen cases where the bitwise copy constructor would have been employed. Each one a potential crash, if not source of corruption.

Good times!

Monday, 17 November 2014

planet-rldev

If you read this, and have a blog where you make posts about your work developing a roguelike, then consider submitting your blog for inclusion on Planet-RLDev.

Monday, 27 October 2014

Mysterious Incursion bug

In changes to Incursion back in August, I fixed compiler warnings related to casting. In theory, the changes should have been side-effect free. Testing of the specific lines of code, indicates this is the case. But people were reporting crashes related to the "room weights algorithm". In this case RM_Weights is an array of unsigned int, and c and tot are signed short.

    tot=0;
    for(i=0;RM_Weights[i*2+1];i++)
      tot += max(0,RM_Weights[i*2+1]);
    c = random(tot); 
    for(i=0;RM_Weights[i*2+1];i++)
      if (c < RM_Weights[i*2+1])
        goto RM_Chosen;
      else
        c -= RM_Weights[i*2+1];
    Fatal("Strange Error in room weights algorithm.");
RM_Chosen:
    RType = RM_Weights[i*2];
    RM_Weights[i*2+1] = -1;

I was unable to reproduce the crashes, although others reported doing so up to 15% of the time.

Were the crashes always there and were they merely hidden by all the other crashes, which have previously been fixed? Or are they caused by some unreproducible nuance of the casting? I'm guessing the former.

There's an obvious bug in the above logic. Points to whomever spots it without looking at my fixes on bitbucket.