Merge pull request #264 from Hoikas/keylist

Refactor RegistryKeyList

Sources/Plasma/NucleusLib/pnKeyedObject/hsKeyedObject.cpp

@@ -107,6 +107,10 @@ plKey hsKeyedObject::RegisterAs(plFixedKeyId fixedKey)
     if (key == nil)
     {
         key = hsgResMgr::ResMgr()->NewKey(meUoid, this);
+
+        //  the key list "helpfully" assigns us an object id.
+        // we don't want one for fixed keys however (initialization order might bite us in the ass)
+        static_cast<plKeyImp*>(key)->SetObjectID(0);
     }
     else
     {

Sources/Plasma/PubUtilLib/plResMgr/plKeyFinder.cpp

@@ -438,15 +438,9 @@ plKey plKeyFinder::IFindSceneNodeKey(plRegistryPageNode* page) const
     plRegistryKeyList* keyList = page->IGetKeyList(CLASS_INDEX_SCOPED(plSceneNode));
     if (keyList)
     {
-        if (keyList->fStaticKeys.size() == 1)
+        if (keyList->fKeys.size() == 1)
         {
-            return plKey::Make((plKeyData*)keyList->fStaticKeys[0]);
-        }
-        else if (keyList->fDynamicKeys.size() == 1) // happens during export
-        {
-            plRegistryKeyList::DynSet::const_iterator it = keyList->fDynamicKeys.begin();
-            plKeyImp* keyImp = *it;
-            return plKey::Make(keyImp);
+            return plKey::Make((plKeyData*)keyList->fKeys[0]);
         }
     }
 
@@ -459,9 +453,9 @@ plKey plKeyFinder::IFindSceneNodeKey(plRegistryPageNode* page) const
     // Get the list of all sceneNodes
     plKey retVal(nil);
     keyList = page->IGetKeyList(CLASS_INDEX_SCOPED(plSceneNode));
-    if (keyList && keyList->fStaticKeys.size() == 1)
+    if (keyList && keyList->fKeys.size() == 1)
     {
-        retVal = plKey::Make((plKeyData*)keyList->fStaticKeys[0]);
+        retVal = plKey::Make((plKeyData*)keyList->fKeys[0]);
     }
     // If we just loaded up all the keys for this page, then we
     // may have a bunch of keys with a refcount of 0. For any of 

Sources/Plasma/PubUtilLib/plResMgr/plRegistryKeyList.cpp

@@ -39,83 +39,44 @@ You can contact Cyan Worlds, Inc. by email legal@cyan.com
       Mead, WA   99021
 
 *==LICENSE==*/
-#include "plRegistryKeyList.h"
-#include "plRegistryHelpers.h"
-#include "hsStream.h"
+
 #include <algorithm>
 
-plRegistryKeyList::plRegistryKeyList(uint16_t classType)
-{
-    fClassType = classType;
-    fReffedStaticKeys = 0;
-    fLocked = 0;
-    fFlags = 0;
-}
+#include "HeadSpin.h"
+#include "hsStream.h"
+
+#include "pnKeyedObject/plKeyImp.h"
+#include "plRegistryHelpers.h"
+#include "plRegistryKeyList.h"
 
 plRegistryKeyList::~plRegistryKeyList()
 {
-    hsAssert(fLocked == 0, "Key list still locked on delete");
-
-    for (int i = 0; i < fStaticKeys.size(); i++)
-    {
-        plKeyImp* keyImp = fStaticKeys[i];
-        if (!keyImp->ObjectIsLoaded())
-            delete keyImp;
-    }
+    std::for_each(fKeys.begin(), fKeys.end(),
+        [] (plKeyImp* key) { if (!key->ObjectIsLoaded()) delete key; }
+    );
 }
 
-// Special dummy key that lets us set the return value of the GetName call.
-// Makes it easier to do STL searches.
-class plSearchKeyImp : public plKeyImp
+plKeyImp* plRegistryKeyList::FindKey(const plString& keyName) const
 {
-public:
-    plString fSearchKeyName;
-    const plString& GetName() const { return fSearchKeyName; }
-};
-
-plKeyImp* plRegistryKeyList::FindKey(const plString& keyName)
-{
-    static plSearchKeyImp searchKey;
-    searchKey.fSearchKeyName = keyName;
-
-    // Search the static key list
-    if (fFlags & kStaticUnsorted)
-    {
-        // We're unsorted, brute force it.  May do a separate search table in the
-        // future if this is a bottlneck
-        for (int i = 0; i < fStaticKeys.size(); i++)
-        {
-            plKeyImp* curKey = fStaticKeys[i];
-            if (curKey && !keyName.Compare(curKey->GetName(), plString::kCaseInsensitive))
-                return curKey;
-        }
-    }
+    auto it = std::find_if(fKeys.begin(), fKeys.end(),
+        [&] (plKeyImp* key) { return key->GetName().CompareI(keyName) == 0; }
+    );
+    if (it != fKeys.end())
+        return *it;
     else
-    {
-        // We're sorted, do a fast lookup
-        StaticVec::const_iterator it = std::lower_bound(fStaticKeys.begin(), fStaticKeys.end(), &searchKey, KeySorter());
-        if (it != fStaticKeys.end() && !keyName.Compare((*it)->GetName(), plString::kCaseInsensitive))
-            return *it;
-    }
-
-    // Search the dynamic key list
-    DynSet::const_iterator dynIt = fDynamicKeys.find(&searchKey);
-    if (dynIt != fDynamicKeys.end())
-        return *dynIt;
-
-    return nil;
+        return nullptr;
 }
 
-plKeyImp* plRegistryKeyList::FindKey(const plUoid& uoid)
+plKeyImp* plRegistryKeyList::FindKey(const plUoid& uoid) const
 {
     uint32_t objectID = uoid.GetObjectID();
 
-    // Key is dynamic or doesn't know it's index.  Do a find by name.
+    // Key is dynamic or doesn't know its index.  Do a find by name.
     if (objectID == 0)
         return FindKey(uoid.GetObjectName());
 
     // Direct lookup
-    if (objectID <= fStaticKeys.size())
+    if (objectID <= fKeys.size())
     {
 #ifdef PLASMA_EXTERNAL_RELEASE
         return fStaticKeys[objectID-1];
@@ -123,8 +84,8 @@ plKeyImp* plRegistryKeyList::FindKey(const plUoid& uoid)
         // If this is an internal release, our objectIDs might not match
         // because of local data. Verify that we have the right key by
         // name, and if it's wrong, do the slower find-by-name.
-        plKeyImp *keyImp = fStaticKeys[objectID-1];
-        if (keyImp->GetName().Compare(uoid.GetObjectName(), plString::kCaseInsensitive) != 0)
+        plKeyImp *keyImp = fKeys[objectID-1];
+        if (keyImp->GetName().CompareI(uoid.GetObjectName()) != 0)
             return FindKey(uoid.GetObjectName());
         else
             return keyImp;
@@ -135,29 +96,17 @@ plKeyImp* plRegistryKeyList::FindKey(const plUoid& uoid)
     // because no one was using them. No worries. The resManager will catch this and 
     // reload our keys, then try again.
 
-    return nil;
-}
-
-void plRegistryKeyList::ILock()
-{
-    fLocked++;
-}
-
-void plRegistryKeyList::IUnlock()
-{
-    fLocked--;
-    if (fLocked == 0)
-        IRepack();
+    return nullptr;
 }
 
 bool plRegistryKeyList::IterateKeys(plRegistryKeyIterator* iterator)
 {
     ILock();
 
-    for (int i = 0; i < fStaticKeys.size(); i++)
+    for (auto it = fKeys.begin(); it != fKeys.end(); ++it)
     {
-        plKeyImp* keyImp = fStaticKeys[i];
-        if (keyImp != nil)
+        plKeyImp* keyImp = *it;
+        if (keyImp)
         {
             if (!iterator->EatKey(plKey::Make(keyImp)))
             {
@@ -167,18 +116,6 @@ bool plRegistryKeyList::IterateKeys(plRegistryKeyIterator* iterator)
         }
     }
 
-    DynSet::const_iterator it;
-    for (it = fDynamicKeys.begin(); it != fDynamicKeys.end(); it++)
-    {
-        plKeyImp* keyImp = *it;
-        hsAssert(keyImp, "Shouldn't ever have a nil dynamic key");
-        if (!iterator->EatKey(plKey::Make(keyImp)))
-        {
-            IUnlock();
-            return false;
-        }
-    }
-
     IUnlock();
     return true;
 }
@@ -188,23 +125,30 @@ void plRegistryKeyList::AddKey(plKeyImp* key, LoadStatus& loadStatusChange)
     loadStatusChange = kNoChange;
 
     hsAssert(fLocked == 0, "Don't currently support adding keys while locked");
-    if (fLocked == 0 && key != nil)
+    if (fLocked == 0 && key)
     {
-        // If this is the first key added, we just became loaded
-        if (fDynamicKeys.empty())
-            loadStatusChange = kDynLoaded;
+        hsAssert(std::find(fKeys.begin(), fKeys.end(), key) == fKeys.end(), "Key already added");
 
-        hsAssert(fDynamicKeys.find(key) == fDynamicKeys.end(), "Key already added");
-        fDynamicKeys.insert(key);
-    }
-}
+        // first key to be added?
+        if (fKeys.empty())
+            loadStatusChange = kTypeLoaded;
 
-void plRegistryKeyList::SetKeyUsed(plKeyImp* key)
-{
-    // If this is a static key, mark that we used it.  Otherwise, just ignore it.
-    uint32_t id = key->GetUoid().GetObjectID();
-    if (id > 0)
-        fReffedStaticKeys++;
+        // Objects that already have an object ID will be respected.
+        // Totally new keys will not have one, but keys from other sources (patches) will.
+        if (key->GetUoid().GetObjectID() == 0)
+        {
+            fKeys.push_back(key);
+            key->SetObjectID(fKeys.size());
+        }
+        else
+        {
+            uint32_t id = key->GetUoid().GetObjectID();
+            if (fKeys.size() < id)
+                fKeys.resize(id);
+            fKeys[id - 1] = key;
+        }
+        ++fReffedKeys;
+    }
 }
 
 bool plRegistryKeyList::SetKeyUnused(plKeyImp* key, LoadStatus& loadStatusChange)
@@ -219,122 +163,50 @@ bool plRegistryKeyList::SetKeyUnused(plKeyImp* key, LoadStatus& loadStatusChange
         return true;
     }
 
-    // Check if it's a static key
     uint32_t id = key->GetUoid().GetObjectID();
-    hsAssert(id <= fStaticKeys.size(), "Bad static key id");
-    if (id != 0 && id <= fStaticKeys.size())
-    {
-        fReffedStaticKeys--;
-        if (fLocked == 0)
-            IRepack();
-
-        // That was our last used static key, we're static unloaded
-        if (fReffedStaticKeys == 0)
-            loadStatusChange = kStaticUnloaded;
-
-        return true;
-    }
+    hsAssert(id <= fKeys.size(), "Bad static key id");
 
-    // Try to find it in the dynamic key list
-    DynSet::iterator dynIt = fDynamicKeys.find(key);
-    if (dynIt != fDynamicKeys.end())
+    // Fixed Keys will have id == 0. Let's just make sure we have it before we toss it.
+    if (id == 0)
     {
-        hsAssert(fLocked == 0, "Don't currently support removing dynamic keys while locked");
-        if (fLocked == 0)
+        hsAssert(key->GetUoid().GetLocation() == plLocation::kGlobalFixedLoc, "key id == 0 but not fixed?");
+        if (!FindKey(key->GetName()))
         {
-            fDynamicKeys.erase(dynIt);
-            delete key;
-
-            // That was our last dynamic key, notify of dynamic unloaded
-            if (fDynamicKeys.empty())
-                loadStatusChange = kDynUnloaded;
-
-            return true;
+            hsAssert(false, "Couldn't find fixed key!");
+            return false;
         }
-        return false;
-    }
-
-    hsAssert(0, "Couldn't find this key, what is it?");
-    return false;
-}
-
-//// IRepack /////////////////////////////////////////////////////////////////
-//  Frees the memory for our static key array if none of them are loaded
-void plRegistryKeyList::IRepack()
-{
-    if (fReffedStaticKeys == 0 && !fStaticKeys.empty())
-    {
-
-        for (int i = 0; i < fStaticKeys.size(); i++)
-            delete fStaticKeys[i];
-
-        fStaticKeys.clear();
-    }
-}
-
-void plRegistryKeyList::PrepForWrite()
-{
-    // If we have any static keys already, we were read in.  To keep from
-    // invalidating old key indexes any new keys have to go on the end, hence we're
-    // unsorted now.
-    if (!fStaticKeys.empty())
-        fFlags |= kStaticUnsorted;
-
-    // If a dynamic keys doesn't have an object assigned to it, we're not writing
-    // it out.  Figure out how many valid keys we have.
-    int numDynKeys = 0;
-    DynSet::const_iterator cIt;
-    for (cIt = fDynamicKeys.begin(); cIt != fDynamicKeys.end(); cIt++)
-    {
-        plKeyImp* key = *cIt;
-        // We're only going to write out keys that have objects
-        if (key->ObjectIsLoaded())
-            numDynKeys++;
     }
+    else if (id > fKeys.size())
+        return false;
 
-    // Start our new object id's after any already created ones
-    uint32_t objectID = fStaticKeys.size()+1;
-    // Make room for our new keys
-    fStaticKeys.resize(fStaticKeys.size()+numDynKeys);
-
-    DynSet::iterator it = fDynamicKeys.begin();
-    while (it != fDynamicKeys.end())
-    {
-        plKeyImp* key = *it;
-        it++;
-
-        // If we're gonna use this key, tag it with it's object id and move it to the static array.
-        if (key->ObjectIsLoaded())
-        {
-            key->SetObjectID(objectID);
-            fStaticKeys[objectID-1] = key;
-
-            objectID++;
-            fReffedStaticKeys++;
-            fDynamicKeys.erase(key);
-        }
-    }
+    // Got that key, decrement the key counter
+    --fReffedKeys;
+    if (fReffedKeys == 0)
+        loadStatusChange = kTypeUnloaded;
+    return true;
 }
 
 void plRegistryKeyList::Read(hsStream* s)
 {
     uint32_t keyListLen = s->ReadLE32();
-    if (!fStaticKeys.empty())
+    if (!fKeys.empty())
     {
         s->Skip(keyListLen);
         return;
     }
 
-    fFlags = s->ReadByte();
+    // deprecated flags. used to indicate alphabetically sorted keys for some "optimization"
+    // that really appeared to do nothing. no loss.
+    s->ReadByte();
 
     uint32_t numKeys = s->ReadLE32();
-    fStaticKeys.resize(numKeys);
+    fKeys.reserve(numKeys);
 
-    for (int i = 0; i < numKeys; i++)
+    for (uint32_t i = 0; i < numKeys; ++i)
     {
         plKeyImp* newKey = new plKeyImp;
         newKey->Read(s);
-        fStaticKeys[i] = newKey;
+        fKeys.push_back(newKey);
     }
 }
 
@@ -343,17 +215,13 @@ void plRegistryKeyList::Write(hsStream* s)
     // Save space for the length of our data
     uint32_t beginPos = s->GetPosition();
     s->WriteLE32(0);
-    s->WriteByte(fFlags);
+    s->WriteByte(0); // Deprecated flags
 
-    int numKeys = fStaticKeys.size();
-    s->WriteLE32(numKeys);
+    s->WriteLE32(fKeys.size());
 
-    // Write out all our keys (anything in dynamic is unused, so just ignore those)
-    for (int i = 0; i < numKeys; i++)
-    {
-        plKeyImp* key = fStaticKeys[i];
-        key->Write(s);
-    }
+    // Write out all our keys
+    for (auto it = fKeys.begin(); it != fKeys.end(); ++it)
+        (*it)->Write(s);
 
     // Go back to the start and write the length of our data
     uint32_t endPos = s->GetPosition();

Sources/Plasma/PubUtilLib/plResMgr/plRegistryKeyList.h

@@ -42,22 +42,13 @@ You can contact Cyan Worlds, Inc. by email legal@cyan.com
 #ifndef plRegistryKeyList_h_inc
 #define plRegistryKeyList_h_inc
 
-#include "HeadSpin.h"
-#include "pnKeyedObject/plKeyImp.h"
 #include <vector>
-#include <set>
 
+class plKeyImp;
 class plRegistryKeyIterator;
-
-class KeySorter
-{
-public:
-    bool operator() (plKeyImp* k1, plKeyImp* k2) const
-    {
-        hsAssert(k1 && k2, "Should have valid keys here");
-        return k1->GetName().Compare(k2->GetName(), plString::kCaseInsensitive) < 0;
-    }
-};
+class hsStream;
+class plString;
+class plUoid;
 
 //
 //  List of keys for a single class type.
@@ -68,60 +59,41 @@ protected:
     friend class plKeyFinder;
 
     uint16_t fClassType;
-    // Lock counter for iterating. If this is >0, don't do any ops that
-    // can change key positions in the array (instead, just leave holes)
-    uint16_t fLocked;
+    uint32_t fLocked;
+    uint32_t fReffedKeys;
 
-    enum Flags { kStaticUnsorted = 0x1 };
-    uint8_t fFlags;
-
-    // Static keys are one's we read off disk.  These don't change and are
-    // assumed to be already sorted when they're read in.
-    typedef std::vector<plKeyImp*> StaticVec;
-    StaticVec fStaticKeys;
-    uint32_t fReffedStaticKeys;   // Number of static keys that are loaded
-
-    // Dynamic keys are anything created at runtime.  They are put in the
-    // correct sorted position when they are added
-    typedef std::set<plKeyImp*, KeySorter> DynSet;
-    DynSet fDynamicKeys;
+    std::vector<plKeyImp*> fKeys;
 
     plRegistryKeyList() {}
 
-    void ILock();
-    void IUnlock();
-
     void IRepack();
+    void ILock() { ++fLocked; }
+    void IUnlock() { --fLocked; }
 
 public:
-    plRegistryKeyList(uint16_t classType);
+    enum LoadStatus
+    {
+        kNoChange,
+        kTypeLoaded,
+        kTypeUnloaded
+    };
+
+    plRegistryKeyList(uint16_t classType)
+        : fClassType(classType), fReffedKeys(0), fLocked(0)
+    { }
     ~plRegistryKeyList();
 
     uint16_t GetClassType() const { return fClassType; }
 
-    // Find a key by name (case-insensitive)
-    plKeyImp* FindKey(const plString& keyName);
-    // Find a key by uoid index.
-    plKeyImp* FindKey(const plUoid& uoid);
+    plKeyImp* FindKey(const plString& keyName) const;
+    plKeyImp* FindKey(const plUoid& uoid) const;
 
     bool IterateKeys(plRegistryKeyIterator* iterator);
 
-    // Changes in our load status that can be caused by loading or unloading a key
-    enum LoadStatus
-    {
-        kNoChange,
-        kDynLoaded,
-        kDynUnloaded,
-        kStaticUnloaded,
-    };
     void AddKey(plKeyImp* key, LoadStatus& loadStatusChange);
-    void SetKeyUsed(plKeyImp* key);
+    void SetKeyUsed(plKeyImp* key) { ++fReffedKeys; }
     bool SetKeyUnused(plKeyImp* key, LoadStatus& loadStatusChange);
 
-    // Export time only.  Before we write to disk, assign all the static keys
-    // object ID's that they can use to do fast lookups at load time.
-    void PrepForWrite();
-
     void Read(hsStream* s);
     void Write(hsStream* s);
 };

Sources/Plasma/PubUtilLib/plResMgr/plRegistryNode.cpp

@@ -55,8 +55,7 @@ You can contact Cyan Worlds, Inc. by email legal@cyan.com
 plRegistryPageNode::plRegistryPageNode(const plFileName& path)
     : fValid(kPageCorrupt)
     , fPath(path)
-    , fDynLoadedTypes(0)
-    , fStaticLoadedTypes(0)
+    , fLoadedTypes(0)
     , fOpenRequests(0)
     , fIsNewPage(false)
 {
@@ -73,8 +72,7 @@ plRegistryPageNode::plRegistryPageNode(const plLocation& location, const plStrin
                                        const plString& page, const plFileName& dataPath)
     : fValid(kPageOk)
     , fPageInfo(location)
-    , fDynLoadedTypes(0)
-    , fStaticLoadedTypes(0)
+    , fLoadedTypes(0)
     , fOpenRequests(0)
     , fIsNewPage(true)
 {
@@ -182,7 +180,7 @@ void plRegistryPageNode::LoadKeys()
 
     stream->SetPosition(oldPos);
     CloseStream();
-    fStaticLoadedTypes = fKeyLists.size();
+    fLoadedTypes = fKeyLists.size();
 }
 
 void plRegistryPageNode::UnloadKeys()
@@ -195,8 +193,7 @@ void plRegistryPageNode::UnloadKeys()
     }
     fKeyLists.clear();
 
-    fDynLoadedTypes = 0;
-    fStaticLoadedTypes = 0;
+    fLoadedTypes = 0;
 }
 
 //// plWriteIterator /////////////////////////////////////////////////////////
@@ -235,8 +232,6 @@ void plRegistryPageNode::Write()
     for (it = fKeyLists.begin(); it != fKeyLists.end(); it++)
     {
         plRegistryKeyList* keyList = it->second;
-        keyList->PrepForWrite();
-
         int ver = plVersion::GetCreatableVersion(keyList->GetClassType());
         fPageInfo.AddClassVersion(keyList->GetClassType(), ver);
     }
@@ -358,8 +353,8 @@ void plRegistryPageNode::AddKey(plKeyImp* key)
     plRegistryKeyList::LoadStatus loadStatusChange;
     keys->AddKey(key, loadStatusChange);
 
-    if (loadStatusChange == plRegistryKeyList::kDynLoaded)
-        fDynLoadedTypes++;
+    if (loadStatusChange == plRegistryKeyList::kTypeLoaded)
+        ++fLoadedTypes;
 }
 
 void plRegistryPageNode::SetKeyUsed(plKeyImp* key)
@@ -381,10 +376,8 @@ bool plRegistryPageNode::SetKeyUnused(plKeyImp* key)
     bool removed = keys->SetKeyUnused(key, loadStatusChange);
 
     // If the key type just changed load status, update our load counts
-    if (loadStatusChange == plRegistryKeyList::kDynUnloaded)
-        fDynLoadedTypes--;
-    else if (loadStatusChange == plRegistryKeyList::kStaticUnloaded)
-        fStaticLoadedTypes--;
+    if (loadStatusChange == plRegistryKeyList::kTypeUnloaded)
+        --fLoadedTypes;
 
     return removed;
 }

Sources/Plasma/PubUtilLib/plResMgr/plRegistryNode.h

@@ -74,8 +74,7 @@ protected:
     // Map from class type to a list of keys of that type
     typedef std::map<uint16_t, plRegistryKeyList*> KeyMap;
     KeyMap fKeyLists;
-    int fDynLoadedTypes;    // The number of key types that have dynamic keys loaded
-    int fStaticLoadedTypes; // The number of key types that have all their keys loaded
+    uint32_t fLoadedTypes;      // The number of key types that have dynamic keys loaded
 
     PageCond    fValid;         // Condition of the page
     plFileName  fPath;          // Path to the page file
@@ -104,9 +103,9 @@ public:
     PageCond GetPageCondition() { return fValid; }
 
     // True if we have any static or dynamic keys loaded
-    bool IsLoaded() const     { return fDynLoadedTypes > 0 || fStaticLoadedTypes > 0; }
+    bool IsLoaded() const     { return fLoadedTypes > 0; }
     // True if all of our static keys are loaded
-    bool IsFullyLoaded() const    { return (fStaticLoadedTypes == fKeyLists.size() && !fKeyLists.empty()) || fIsNewPage; }
+    bool IsFullyLoaded() const    { return (fLoadedTypes == fKeyLists.size() && !fKeyLists.empty()) || fIsNewPage; }
 
     // Export time only.  If we want to reuse a page, load the keys we want then
     // call SetNewPage, so it will be considered a new page from now on.  That