Various issues researching items in library

Issue #251 new
cadmal created an issue

When using the Knowledge (Magic) skill to research items in an Ancient Library:

  1. Most unidentified items inside your pack are listed, but wands are not listed unless you take them out of the pack, for some reason. Though I'm not sure about staves, as I didn't have any at the time.

  2. Occasionally the list would only contain a small subset of the valid items (the list population must be terminating early). I don't know what triggered this to happen. Swapping items around in my inventory eventually got the full list back.

  3. It would be nice if items that cannot be researched (because a previous attempt failed) were either not listed or marked/disabled in the list somehow (maybe with an asterisk or grayed out). Otherwise, the player has to remember which items previously failed, which can be tedious if you have a lot of unidentified items.

Note: I suspect that (1) and (2) are due to problems with general inventory/item handling because the logic that populates the list is very simple:

// from Creature::Research() in Skills.cpp (lines 2545-2548)
          for (it=FirstInv();it;it=NextInv())
            if (!it->isKnown(KN_MAGIC|KN_NATURE|KN_CURSE))
              { c++; //Candidates[c++] = it->myHandle;
                thisp->MyTerm->LOption(it->Name(0),it->myHandle); }

Comments (3)

  1. RMTEW FULL NAME repo owner

    Yes, if something else iterates over the inventory in some code within a function you call when you're iterating over the inventory, the state gets clobbered. This is something we need to fix. If you have any ideas, they'd be appreciated.

  2. cadmal reporter

    Ah, does it get clobbered because the Character::GetInv() function stores the inventory item in a local static and expects the value to persist between successive calls? That seems risky, at least. Not sure if that is the cause of the problem, but when I debug the missing wands problem, GetInv() iterates through the linked list representing the pack contents and the wands aren't on the list.

    EDIT: To clarify (if this is the actual source of the problem), each caller of GetInv() would need to maintain a separate copy of the "current item" iterator(s), rather than the current scheme, which has a single copy shared across callers. For example, you could change the "current item" to be a parameter instead of a local variable.

    Item* Character::GetInv(bool first) {
    // get rid of these
        static Item *it, *it2; 
        static int8 sl;
    
  3. Log in to comment