CResourceStore: Use unique_ptr where applicable

Makes the ownership semantics explicit and prevents leaks from occurring
a little better.

src/Core/GameProject/CResourceEntry.cpp

@@ -11,23 +11,17 @@
 #include <Common/Serialization/CXMLWriter.h>
 
 CResourceEntry::CResourceEntry(CResourceStore *pStore)
-    : mpResource(nullptr)
+    : mpStore(pStore)
-    , mpTypeInfo(nullptr)
+    , mID(CAssetID::InvalidID(pStore->Game()))
-    , mpStore(pStore)
-    , mpDependencies(nullptr)
-    , mID( CAssetID::InvalidID(pStore->Game()) )
-    , mpDirectory(nullptr)
-    , mMetadataDirty(false)
-    , mCachedSize(-1)
 {}
 
 // Static constructors
-CResourceEntry* CResourceEntry::CreateNewResource(CResourceStore *pStore, const CAssetID& rkID,
+std::unique_ptr<CResourceEntry> CResourceEntry::CreateNewResource(CResourceStore *pStore, const CAssetID& rkID,
                                                                   const TString& rkDir, const TString& rkName,
-                                                    EResourceType Type, bool ExistingResource /*= false*/)
+                                                                  EResourceType Type, bool ExistingResource)
 {
     // Initialize all entry info with the input data.
-    CResourceEntry *pEntry = new CResourceEntry(pStore);
+    auto pEntry = std::unique_ptr<CResourceEntry>(new CResourceEntry(pStore));
     pEntry->mID = rkID;
     pEntry->mName = rkName;
     pEntry->mCachedUppercaseName = rkName.ToUpper();
@@ -37,7 +31,7 @@ CResourceEntry* CResourceEntry::CreateNewResource(CResourceStore *pStore, const
 
     pEntry->mpDirectory = pStore->GetVirtualDirectory(rkDir, true);
     ASSERT(pEntry->mpDirectory);
-    pEntry->mpDirectory->AddChild("", pEntry);
+    pEntry->mpDirectory->AddChild("", pEntry.get());
 
     pEntry->mMetadataDirty = true;
 
@@ -45,7 +39,7 @@ CResourceEntry* CResourceEntry::CreateNewResource(CResourceStore *pStore, const
     // then instantiate the new resource data so it can be saved as soon as possible.
     if (!ExistingResource)
     {
-        pEntry->mpResource = CResourceFactory::CreateResource(pEntry);
+        pEntry->mpResource = CResourceFactory::CreateResource(pEntry.get());
 
         if (pEntry->mpResource)
         {
@@ -56,30 +50,30 @@ CResourceEntry* CResourceEntry::CreateNewResource(CResourceStore *pStore, const
     return pEntry;
 }
 
-CResourceEntry* CResourceEntry::BuildFromArchive(CResourceStore *pStore, IArchive& rArc)
+std::unique_ptr<CResourceEntry> CResourceEntry::BuildFromArchive(CResourceStore *pStore, IArchive& rArc)
 {
     // Load all entry info from the archive.
-    CResourceEntry *pEntry = new CResourceEntry(pStore);
+    auto pEntry = std::unique_ptr<CResourceEntry>(new CResourceEntry(pStore));
     pEntry->SerializeEntryInfo(rArc, false);
     ASSERT(pEntry->mpTypeInfo);
     ASSERT(pEntry->mpDirectory);
     return pEntry;
 }
 
-CResourceEntry* CResourceEntry::BuildFromDirectory(CResourceStore *pStore, CResTypeInfo *pTypeInfo,
+std::unique_ptr<CResourceEntry> CResourceEntry::BuildFromDirectory(CResourceStore *pStore, CResTypeInfo *pTypeInfo,
                                                                    const TString& rkDirPath, const TString& rkName)
 {
     // Initialize as much entry info as possible from the input data, then load the rest from the metadata file.
     ASSERT(pTypeInfo);
 
-    CResourceEntry *pEntry = new CResourceEntry(pStore);
+    auto pEntry = std::unique_ptr<CResourceEntry>(new CResourceEntry(pStore));
     pEntry->mpTypeInfo = pTypeInfo;
     pEntry->mName = rkName;
     pEntry->mCachedUppercaseName = rkName.ToUpper();
 
     pEntry->mpDirectory = pStore->GetVirtualDirectory(rkDirPath, true);
     ASSERT(pEntry->mpDirectory);
-    pEntry->mpDirectory->AddChild("", pEntry);
+    pEntry->mpDirectory->AddChild("", pEntry.get());
 
     // Make sure we're valid, then load the remaining data from the metadata file
     ASSERT(pEntry->HasCookedVersion() || pEntry->HasRawVersion());

src/Core/GameProject/CResourceEntry.h

@@ -8,10 +8,12 @@
 #include <Common/CAssetID.h>
 #include <Common/CFourCC.h>
 #include <Common/Flags.h>
+#include <memory>
 
-class CResource;
-class CGameProject;
 class CDependencyTree;
+class CGameProject;
+class CResource;
+class IInputStream;
 
 enum class EResEntryFlag
 {
@@ -28,27 +30,27 @@ DECLARE_FLAGS(EResEntryFlag, FResEntryFlags)
 class CResourceEntry
 {
     std::unique_ptr<CResource> mpResource;
-    CResTypeInfo *mpTypeInfo;
+    CResTypeInfo *mpTypeInfo = nullptr;
     CResourceStore *mpStore;
     std::unique_ptr<CDependencyTree> mpDependencies;
     CAssetID mID;
-    CVirtualDirectory *mpDirectory;
+    CVirtualDirectory *mpDirectory = nullptr;
     TString mName;
     FResEntryFlags mFlags;
 
-    mutable bool mMetadataDirty;
+    mutable bool mMetadataDirty = false;
-    mutable uint64 mCachedSize;
+    mutable uint64 mCachedSize = UINT64_MAX;
     mutable TString mCachedUppercaseName; // This is used to speed up case-insensitive sorting and filtering.
 
     // Private constructor
     explicit CResourceEntry(CResourceStore *pStore);
 
 public:
-    static CResourceEntry* CreateNewResource(CResourceStore *pStore, const CAssetID& rkID,
+    static std::unique_ptr<CResourceEntry> CreateNewResource(CResourceStore *pStore, const CAssetID& rkID,
                                                              const TString& rkDir, const TString& rkName,
                                                              EResourceType Type, bool ExistingResource = false);
-    static CResourceEntry* BuildFromArchive(CResourceStore *pStore, IArchive& rArc);
+    static std::unique_ptr<CResourceEntry> BuildFromArchive(CResourceStore *pStore, IArchive& rArc);
-    static CResourceEntry* BuildFromDirectory(CResourceStore *pStore, CResTypeInfo *pTypeInfo,
+    static std::unique_ptr<CResourceEntry> BuildFromDirectory(CResourceStore *pStore, CResTypeInfo *pTypeInfo,
                                                               const TString& rkDirPath, const TString& rkName);
     ~CResourceEntry();
 

src/Core/GameProject/CResourceIterator.h

@@ -8,15 +8,14 @@ class CResourceIterator
 {
 protected:
     const CResourceStore *mpkStore;
-    std::map<CAssetID, CResourceEntry*>::const_iterator mIter;
+    std::map<CAssetID, std::unique_ptr<CResourceEntry>>::const_iterator mIter;
-    CResourceEntry *mpCurEntry;
+    CResourceEntry *mpCurEntry = nullptr;
 
 public:
-    CResourceIterator(const CResourceStore *pkStore = gpResourceStore)
+    explicit CResourceIterator(const CResourceStore *pkStore = gpResourceStore)
         : mpkStore(pkStore)
-        , mpCurEntry(nullptr)
     {
-        mIter = mpkStore->mResourceEntries.begin();
+        mIter = mpkStore->mResourceEntries.cbegin();
         Next();
     }
 
@@ -26,9 +25,9 @@ public:
     {
         do
         {
-            if (mIter != mpkStore->mResourceEntries.end())
+            if (mIter != mpkStore->mResourceEntries.cend())
             {
-                mpCurEntry = mIter->second;
+                mpCurEntry = mIter->second.get();
                 ++mIter;
             }
             else mpCurEntry = nullptr;
@@ -76,7 +75,7 @@ template<EResourceType ResType>
 class TResourceIterator : public CResourceIterator
 {
 public:
-    TResourceIterator(CResourceStore *pStore = gpResourceStore)
+    explicit TResourceIterator(CResourceStore *pStore = gpResourceStore)
         : CResourceIterator(pStore)
     {
         if (mpCurEntry && mpCurEntry->ResourceType() != ResType)

src/Core/GameProject/CResourceStore.cpp

@@ -38,9 +38,6 @@ CResourceStore::~CResourceStore()
 {
     CloseProject();
     DestroyUnreferencedResources();
-
-    for (auto It = mResourceEntries.begin(); It != mResourceEntries.end(); It++)
-        delete It->second;
 }
 
 void RecursiveGetListOfEmptyDirectories(CVirtualDirectory *pDir, TStringList& rOutList)
@@ -69,11 +66,9 @@ bool CResourceStore::SerializeDatabaseCache(IArchive& rArc)
         {
             // Make sure deleted resources aren't included in the count.
             // We can't use CResourceIterator because it skips MarkedForDeletion resources.
-            for (auto Iter = mResourceEntries.begin(); Iter != mResourceEntries.end(); Iter++)
+            for (const auto& entry : mResourceEntries)
             {
-                CResourceEntry* pEntry = Iter->second;
+                if (entry.second->IsMarkedForDeletion())
-
-                if (pEntry->IsMarkedForDeletion())
                 {
                     ResourceCount--;
                 }
@@ -88,9 +83,9 @@ bool CResourceStore::SerializeDatabaseCache(IArchive& rArc)
             {
                 if (rArc.ParamBegin("Resource", 0))
                 {
-                    CResourceEntry *pEntry = CResourceEntry::BuildFromArchive(this, rArc);
+                    auto pEntry = CResourceEntry::BuildFromArchive(this, rArc);
-                    ASSERT( FindEntry(pEntry->ID()) == nullptr );
+                    ASSERT(FindEntry(pEntry->ID()) == nullptr);
-                    mResourceEntries[pEntry->ID()] = pEntry;
+                    mResourceEntries.insert_or_assign(pEntry->ID(), std::move(pEntry));
                     rArc.ParamEnd();
                 }
             }
@@ -236,13 +231,7 @@ void CResourceStore::CloseProject()
     }
 
     // Delete all entries from old project
-    auto It = mResourceEntries.begin();
+    mResourceEntries.clear();
-
-    while (It != mResourceEntries.end())
-    {
-        delete It->second;
-        It = mResourceEntries.erase(It);
-    }
 
     // Clear deleted files from previous runs
     TString DeletedPath = DeletedResourcePath();
@@ -304,12 +293,12 @@ CResourceEntry* CResourceStore::FindEntry(const CAssetID& rkID) const
     {
         auto Found = mResourceEntries.find(rkID);
 
-        if (Found != mResourceEntries.end())
+        if (Found != mResourceEntries.cend())
         {
-            CResourceEntry* pEntry = Found->second;
+            auto& pEntry = Found->second;
 
             if (!pEntry->IsMarkedForDeletion())
-                return pEntry;
+                return pEntry.get();
         }
     }
 
@@ -346,8 +335,6 @@ void CResourceStore::ClearDatabase()
     }
 
     // Clear out existing resource entries and directories
-    for (auto Iter = mResourceEntries.begin(); Iter != mResourceEntries.end(); Iter++)
-        delete Iter->second;
     mResourceEntries.clear();
 
     delete mpDatabaseRoot;
@@ -389,19 +376,20 @@ bool CResourceStore::BuildFromDirectory(bool ShouldGenerateCacheFile)
             }
 
             // Create resource entry
-            CResourceEntry *pEntry = CResourceEntry::BuildFromDirectory(this, pTypeInfo, DirPath, ResName);
+            auto pEntry = CResourceEntry::BuildFromDirectory(this, pTypeInfo, DirPath, ResName);
 
             // Validate the entry
-            CAssetID ID = pEntry->ID();
+            const CAssetID ID = pEntry->ID();
-            ASSERT( mResourceEntries.find(ID) == mResourceEntries.end() );
+            ASSERT(mResourceEntries.find(ID) == mResourceEntries.cend());
-            ASSERT( ID.Length() == CAssetID::GameIDLength(mGame) );
+            ASSERT(ID.Length() == CAssetID::GameIDLength(mGame));
 
-            mResourceEntries[ID] = pEntry;
+            mResourceEntries.insert_or_assign(ID, std::move(pEntry));
         }
-
         else if (FileUtil::IsDirectory(Path))
+        {
             CreateVirtualDirectory(RelPath);
         }
+    }
 
     // Generate new cache file
     if (ShouldGenerateCacheFile)
@@ -448,29 +436,33 @@ CResourceEntry* CResourceStore::CreateNewResource(const CAssetID& rkID, EResourc
     CResourceEntry *pEntry = FindEntry(rkID);
 
     if (pEntry)
+    {
         errorf("Attempted to register resource that's already tracked in the database: %s / %s / %s", *rkID.ToString(), *rkDir, *rkName);
-
+    }
     else
     {
         // Validate directory
         if (IsValidResourcePath(rkDir, rkName))
         {
-            pEntry = CResourceEntry::CreateNewResource(this, rkID, rkDir, rkName, Type, ExistingResource);
+            auto res = CResourceEntry::CreateNewResource(this, rkID, rkDir, rkName, Type, ExistingResource);
-            mResourceEntries[rkID] = pEntry;
+            auto resPtr = res.get();
+
+            mResourceEntries.insert_or_assign(rkID, std::move(res));
             mDatabaseCacheDirty = true;
 
-            if (pEntry->IsLoaded())
+            if (resPtr->IsLoaded())
             {
-                TrackLoadedResource(pEntry);
+                TrackLoadedResource(resPtr);
             }
 
             if (!ExistingResource)
             {
-                debugf("CREATED NEW RESOURCE: [%s] %s", *rkID.ToString(), *pEntry->CookedAssetPath());
+                debugf("CREATED NEW RESOURCE: [%s] %s", *rkID.ToString(), *resPtr->CookedAssetPath());
-            }
+            }
+
+            return resPtr;
         }
 
-        else
         errorf("Invalid resource path, failed to register: %s%s", *rkDir, *rkName);
     }
 
@@ -479,19 +471,18 @@ CResourceEntry* CResourceStore::CreateNewResource(const CAssetID& rkID, EResourc
 
 CResource* CResourceStore::LoadResource(const CAssetID& rkID)
 {
-    if (!rkID.IsValid()) return nullptr;
+    if (!rkID.IsValid())
+        return nullptr;
 
     CResourceEntry *pEntry = FindEntry(rkID);
-
+    if (!pEntry)
-    if (pEntry)
-        return pEntry->Load();
-
-    else
     {
         // Resource doesn't seem to exist
         warnf("Can't find requested resource with ID \"%s\"", *rkID.ToString());
         return nullptr;
     }
+
+    return pEntry->Load();
 }
 
 CResource* CResourceStore::LoadResource(const CAssetID& rkID, EResourceType Type)
@@ -515,7 +506,6 @@ CResource* CResourceStore::LoadResource(const CAssetID& rkID, EResourceType Type
         }
     }
 
-    else
     return nullptr;
 }
 
@@ -544,7 +534,7 @@ CResource* CResourceStore::LoadResource(const TString& rkPath)
         return pEntry->Load();
     }
 
-    else return nullptr;
+    return nullptr;
 }
 
 void CResourceStore::TrackLoadedResource(CResourceEntry *pEntry)
@@ -573,8 +563,10 @@ void CResourceStore::DestroyUnreferencedResources()
                 It = mLoadedResources.erase(It);
                 NumDeleted++;
             }
-
+            else
-            else It++;
+            {
+                ++It;
+            }
         }
     } while (NumDeleted > 0);
 }

src/Core/GameProject/CResourceStore.h

@@ -8,6 +8,7 @@
 #include <Common/FileUtil.h>
 #include <Common/TString.h>
 #include <map>
+#include <memory>
 #include <set>
 
 class CGameExporter;
@@ -30,7 +31,7 @@ class CResourceStore
     CGameProject *mpProj = nullptr;
     EGame mGame{EGame::Prime};
     CVirtualDirectory *mpDatabaseRoot = nullptr;
-    std::map<CAssetID, CResourceEntry*> mResourceEntries;
+    std::map<CAssetID, std::unique_ptr<CResourceEntry>> mResourceEntries;
     std::map<CAssetID, CResourceEntry*> mLoadedResources;
     bool mDatabaseCacheDirty = false;