Cliffski's Blog

I thought it might be interesting to just brain dump my thoughts as I do them for a Production Line bug.

The bug is an error message which I get triggering thus:

GERROR(“No room to TrashLowPriorityResource from stockpile”);

Which is handy, as I know exactly WHAT is happening, and where in the code. Thats 95% of the work done already. Amen to asserts! I know that inside the function SIM_ResourceStockpile::TrashLowPriorityResource(), I am not able to actually do what the function needs to do. So firstly what does this function do anyway? I can’t remember… Thankfully there is a comment:

//if we find a resource not needed by current task, destroy it (only one)

Which is pretty handy. So this is a stockpile at a production task (working on a vehicle) and it needs room for a new resource presumably. The code goes through all of the objects currently in the stockpile to check we really need them. here is the code:

    iter it;
    for (it = Objects.begin(); it != Objects.end(); ++it)
    {
        SIM_ResourceObject* pobject = *it;

        //is this object needed?
        bool bneeded = false;
        SIM_ProductionTask::iter rit;
        for (int c = 0; c < SIM_ProductionSlot::MAX_RESOURCE_QUANTITIES; c++)
        {
            SIM_ResourceQuantity req = PSlot->CurrentResources[c];
            if (req.Quantity <= 0)
            {
                continue;
            }
            if (req.PResource == pobject->GetResource())
            {
                if (GetQuantity(req.PResource) > req.Quantity)
                {
                    bneeded = false;
                }
                else
                {
                    bneeded = true;
                }
            }
        }
        if (!bneeded)
        {
            RemoveObject(pobject->GetResource());
            return;
        }
    }

Thats the first half of the function, it then tries to do a simialr thing with requests, rather thanm objects. However I think I see an issue already. That GetQuantity() is checking the total quantity, not for this single object. Nope…thjat makes sense. I guess I really need to trigger the error and step through it, so time to fire up a save game from a player and run it in *slooooow* debug mode. Ok… an immediate crash from a save game..where we have 16 requests and no objects. So there seems to be a bug in the latter code:

    //ok we need to destroy a request instead of a current object.
    reqit qit;

    for (qit = Requests.begin(); qit != Requests.end(); qit++)
    {
        SIM_ResourceRequest* preq = *qit;
        //is this object needed?
        bool bneeded = false;
        for (int c = 0; c < SIM_ProductionSlot::MAX_RESOURCE_QUANTITIES; c++)
        {
            SIM_ResourceQuantity req = PSlot->CurrentResources[c];
            if (req.Quantity <= 0)
            {
                continue;
            }
            if (req.PResource == preq->PObject->GetResource())
            {
                //sure we ordered it, but do we actually have enough for this task, when com,bining requests with objects?
                int stock_and_ordered = GetStockAndOrdered(req.PResource);
                if (stock_and_ordered >= req.Quantity)
                {
                    Requests.remove(preq);
                    preq->PObject->Trash();
                    return;
                }
                bneeded = true;
            }
        }
        if (!bneeded)
        {
            Requests.remove(preq);
            preq->PObject->Trash();
            return;
        }
    }

So what could be wrong here? Ok…well hang on a darned minute. This code is just saying ‘do we really need resources of this type right now???’ rather than ‘do we need ALL of these right now?’  which is more of a problem. We are clearly somehow ‘over-ordering’. The slot in question is ‘fit trunk’. presumably it only needs trunks… They do all indeed appear to be the same type. 4 are ‘intransit’ the rest are queued up at some resource importer or a supply stockpile or component stockpile somewhere.

So the ‘solution’ is easy. First kill off (delete) a request that is not in transit yet, to make room, or if that is not possible, kill one that actually is in transit. This then frees up the required place in our stockpile. But hold on? this is just patching over the wound? how on earth did this happen? The calling code is requesting new resources, when clearly 16 are already on the way! in fact its requesting a servo…

AHAHAHAHA!

what a beautiful bug. The player obviously upgraded to a powered tailgate, which means a servo is needed for every trunk, and yet 16 trunks were already on order. Error! error! I have special code that handles when an upgrade makes an existing resource requirement obsolete (climate control replaces aircon), but not code to handle this case, where additional stuff is needed. This is FINE, I can implement the above mentioned code without worrying how it happened.

That actually wasn’t too bad at all :D