CResource: Make BuildDependencyTree() return a unique_ptr

Makes the functions more memory safe in terms of freeing memory in
exceptional paths .

src/Core/GameProject/CResourceEntry.cpp

@@ -92,7 +92,6 @@ CResourceEntry* CResourceEntry::BuildFromDirectory(CResourceStore *pStore, CResT
 CResourceEntry::~CResourceEntry()
 {
     if (mpResource) delete mpResource;
-    if (mpDependencies) delete mpDependencies;
 }
 
 bool CResourceEntry::LoadMetadata()
@@ -171,15 +170,11 @@ void CResourceEntry::SerializeEntryInfo(IArchive& rArc, bool MetadataOnly)
 
 void CResourceEntry::UpdateDependencies()
 {
-    if (mpDependencies)
-    {
-        delete mpDependencies;
-        mpDependencies = nullptr;
-    }
+    mpDependencies.reset();
 
     if (!mpTypeInfo->CanHaveDependencies())
     {
-        mpDependencies = new CDependencyTree();
+        mpDependencies = std::make_unique<CDependencyTree>();
         return;
     }
 
@@ -191,7 +186,7 @@ void CResourceEntry::UpdateDependencies()
     if (!mpResource)
     {
         errorf("Unable to update cached dependencies; failed to load resource");
-        mpDependencies = new CDependencyTree();
+        mpDependencies = std::make_unique<CDependencyTree>();
         return;
     }
 

src/Core/GameProject/CResourceEntry.h

@@ -30,7 +30,7 @@ class CResourceEntry
     CResource *mpResource;
     CResTypeInfo *mpTypeInfo;
     CResourceStore *mpStore;
-    CDependencyTree *mpDependencies;
+    std::unique_ptr<CDependencyTree> mpDependencies;
     CAssetID mID;
     CVirtualDirectory *mpDirectory;
     TString mName;
@@ -100,7 +100,7 @@ public:
     CResource* Resource() const              { return mpResource; }
     CResTypeInfo* TypeInfo() const           { return mpTypeInfo; }
     CResourceStore* ResourceStore() const    { return mpStore; }
-    CDependencyTree* Dependencies() const    { return mpDependencies; }
+    CDependencyTree* Dependencies() const    { return mpDependencies.get(); }
     CAssetID ID() const                      { return mID; }
     CVirtualDirectory* Directory() const     { return mpDirectory; }
     TString DirectoryPath() const            { return mpDirectory->FullPath(); }

src/Core/Resource/Animation/CAnimEventData.h

@@ -21,10 +21,10 @@ public:
     {
     }
 
-    CDependencyTree* BuildDependencyTree() const override
+    std::unique_ptr<CDependencyTree> BuildDependencyTree() const override
     {
-        auto *pTree = new CDependencyTree();
-        AddDependenciesToTree(pTree);
+        auto pTree = std::make_unique<CDependencyTree>();
+        AddDependenciesToTree(pTree.get());
         return pTree;
     }
 

src/Core/Resource/Animation/CAnimSet.h

@@ -128,14 +128,14 @@ public:
         }
     }
 
-    CDependencyTree* BuildDependencyTree() const
+    std::unique_ptr<CDependencyTree> BuildDependencyTree() const
     {
-        CDependencyTree *pTree = new CDependencyTree();
+        auto pTree = std::make_unique<CDependencyTree>();
 
         // Character dependencies
-        for (uint32 iChar = 0; iChar < mCharacters.size(); iChar++)
+        for (const auto& character : mCharacters)
         {
-            CSetCharacterDependency *pCharTree = CSetCharacterDependency::BuildTree( mCharacters[iChar] );
+            CSetCharacterDependency *pCharTree = CSetCharacterDependency::BuildTree(character);
             ASSERT(pCharTree);
             pTree->AddChild(pCharTree);
         }
@@ -150,42 +150,38 @@ public:
                 pTree->AddChild(pAnimTree);
             }
         }
-
         else if (Game() <= EGame::Corruption)
         {
             const SSetCharacter& rkChar = mCharacters[0];
             std::set<CAnimPrimitive> PrimitiveSet;
 
             // Animations
-            for (uint32 iAnim = 0; iAnim < mAnimations.size(); iAnim++)
+            for (auto& anim : mAnimations)
             {
-                const SAnimation& rkAnim = mAnimations[iAnim];
-                rkAnim.pMetaAnim->GetUniquePrimitives(PrimitiveSet);
+                anim.pMetaAnim->GetUniquePrimitives(PrimitiveSet);
             }
 
             CSourceAnimData *pAnimData = gpResourceStore->LoadResource<CSourceAnimData>(rkChar.AnimDataID);
             if (pAnimData)
-                pAnimData->AddTransitionDependencies(pTree);
+                pAnimData->AddTransitionDependencies(pTree.get());
 
-            for (auto Iter = PrimitiveSet.begin(); Iter != PrimitiveSet.end(); Iter++)
+            for (auto& prim : PrimitiveSet)
             {
-                const CAnimPrimitive& rkPrim = *Iter;
-                pTree->AddDependency(rkPrim.Animation());
+                pTree->AddDependency(prim.Animation());
             }
 
             // Event sounds
-            for (uint32 iSound = 0; iSound < rkChar.SoundEffects.size(); iSound++)
+            for (const auto& effect : rkChar.SoundEffects)
             {
-                pTree->AddDependency(rkChar.SoundEffects[iSound]);
+                pTree->AddDependency(effect);
             }
         }
-
         else
         {
             const SSetCharacter& rkChar = mCharacters[0];
 
-            for (uint32 iDep = 0; iDep < rkChar.DKDependencies.size(); iDep++)
-                pTree->AddDependency(rkChar.DKDependencies[iDep]);
+            for (const auto& dep : rkChar.DKDependencies)
+                pTree->AddDependency(dep);
         }
 
         return pTree;

src/Core/Resource/Animation/CAnimation.cpp

@@ -9,9 +9,9 @@ CAnimation::CAnimation(CResourceEntry *pEntry /*= 0*/)
 {
 }
 
-CDependencyTree* CAnimation::BuildDependencyTree() const
+std::unique_ptr<CDependencyTree> CAnimation::BuildDependencyTree() const
 {
-    CDependencyTree *pTree = new CDependencyTree();
+    auto pTree = std::make_unique<CDependencyTree>();
     pTree->AddDependency(mpEventData);
     return pTree;
 }

src/Core/Resource/Animation/CAnimation.h

@@ -37,8 +37,8 @@ class CAnimation : public CResource
     TResPtr<CAnimEventData> mpEventData;
 
 public:
-    CAnimation(CResourceEntry *pEntry = 0);
-    CDependencyTree* BuildDependencyTree() const override;
+    explicit CAnimation(CResourceEntry *pEntry = nullptr);
+    std::unique_ptr<CDependencyTree> BuildDependencyTree() const override;
     void EvaluateTransform(float Time, uint32 BoneID, CVector3f *pOutTranslation, CQuaternion *pOutRotation, CVector3f *pOutScale) const;
     bool HasTranslation(uint32 BoneID) const;
 

src/Core/Resource/Animation/CSourceAnimData.h

@@ -43,13 +43,13 @@ public:
         delete mpDefaultTransition;
     }
 
-    CDependencyTree* BuildDependencyTree() const override
+    std::unique_ptr<CDependencyTree> BuildDependencyTree() const override
     {
         // SAND normally has dependencies from meta-transitions and events
         // However, all of these can be character-specific. To simplify things, all SAND
         // dependencies are being added to the CHAR dependency tree instead. Therefore the
         // SAND dependency tree is left empty.
-        return new CDependencyTree();
+        return std::make_unique<CDependencyTree>();
     }
 
     void GetUniquePrimitives(std::set<CAnimPrimitive>& rPrimSet) const

src/Core/Resource/Area/CGameArea.cpp

@@ -23,16 +23,16 @@ CGameArea::~CGameArea()
         delete mScriptLayers[iSCLY];
 }
 
-CDependencyTree* CGameArea::BuildDependencyTree() const
+std::unique_ptr<CDependencyTree> CGameArea::BuildDependencyTree() const
 {
     // Base dependencies
-    CAreaDependencyTree *pTree = new CAreaDependencyTree();
+    auto pTree = std::make_unique<CAreaDependencyTree>();
 
     std::set<CAssetID> MatTextures;
     mpMaterialSet->GetUsedTextureIDs(MatTextures);
 
-    for (auto Iter = MatTextures.begin(); Iter != MatTextures.end(); Iter++)
-        pTree->AddDependency(*Iter);
+    for (const auto& id : MatTextures)
+        pTree->AddDependency(id);
 
     pTree->AddDependency(mPathID);
 
@@ -43,8 +43,8 @@ CDependencyTree* CGameArea::BuildDependencyTree() const
     }
     
     // Extra deps
-    for (uint32 iDep = 0; iDep < mExtraAreaDeps.size(); iDep++)
-        pTree->AddDependency(mExtraAreaDeps[iDep]);
+    for (const auto& dep : mExtraAreaDeps)
+        pTree->AddDependency(dep);
 
     // Layer dependencies
     std::vector<CAssetID> DummyDeps;

src/Core/Resource/Area/CGameArea.h

@@ -65,9 +65,9 @@ class CGameArea : public CResource
     std::vector< std::vector<CAssetID> > mExtraLayerDeps;
 
 public:
-    CGameArea(CResourceEntry *pEntry = nullptr);
+    explicit CGameArea(CResourceEntry *pEntry = nullptr);
     ~CGameArea();
-    CDependencyTree* BuildDependencyTree() const override;
+    std::unique_ptr<CDependencyTree> BuildDependencyTree() const override;
 
     void AddWorldModel(CModel *pModel);
     void MergeTerrain();

src/Core/Resource/CAudioMacro.h

@@ -16,9 +16,9 @@ public:
         : CResource(pEntry)
     {}
 
-    CDependencyTree* BuildDependencyTree() const override
+    std::unique_ptr<CDependencyTree> BuildDependencyTree() const override
     {
-        auto *pTree = new CDependencyTree();
+        auto pTree = std::make_unique<CDependencyTree>();
 
         for (const auto& sample : mSamples)
             pTree->AddDependency(sample);

src/Core/Resource/CDependencyGroup.h

@@ -9,19 +9,19 @@ class CDependencyGroup : public CResource
     std::vector<CAssetID> mDependencies;
 
 public:
-    CDependencyGroup(CResourceEntry *pEntry = 0) : CResource(pEntry) {}
+    explicit CDependencyGroup(CResourceEntry *pEntry = nullptr) : CResource(pEntry) {}
 
-    inline void Clear()                                     { mDependencies.clear(); }
-    inline uint32 NumDependencies() const                   { return mDependencies.size(); }
-    inline CAssetID DependencyByIndex(uint32 Index) const   { return mDependencies[Index]; }
+    void Clear()                                     { mDependencies.clear(); }
+    uint32 NumDependencies() const                   { return mDependencies.size(); }
+    CAssetID DependencyByIndex(uint32 Index) const   { return mDependencies[Index]; }
 
-    inline void AddDependency(const CAssetID& rkID)
+    void AddDependency(const CAssetID& rkID)
     {
         if (!HasDependency(rkID))
             mDependencies.push_back(rkID);
     }
 
-    inline void AddDependency(CResource *pRes)
+    void AddDependency(CResource *pRes)
     {
         if ( pRes && !HasDependency(pRes->ID()) )
             mDependencies.push_back(pRes->ID());
@@ -50,12 +50,12 @@ public:
         return false;
     }
 
-    CDependencyTree* BuildDependencyTree() const
+    std::unique_ptr<CDependencyTree> BuildDependencyTree() const
     {
-        CDependencyTree *pTree = new CDependencyTree();
+        auto pTree = std::make_unique<CDependencyTree>();
 
-        for (auto DepIt = mDependencies.begin(); DepIt != mDependencies.end(); DepIt++)
-            pTree->AddDependency(*DepIt);
+        for (const auto& dep : mDependencies)
+            pTree->AddDependency(dep);
 
         return pTree;
     }

src/Core/Resource/CFont.cpp

@@ -22,9 +22,9 @@ inline float PtsToFloat(int32 Pt)
     return 0.00208333f * Pt;
 }
 
-CDependencyTree* CFont::BuildDependencyTree() const
+std::unique_ptr<CDependencyTree> CFont::BuildDependencyTree() const
 {
-    CDependencyTree *pOut = new CDependencyTree();
+    auto pOut = std::make_unique<CDependencyTree>();
     pOut->AddDependency(mpFontTexture);
     return pOut;
 }

src/Core/Resource/CFont.h

@@ -59,9 +59,9 @@ class CFont : public CResource
 
 
 public:
-    CFont(CResourceEntry *pEntry = nullptr);
+    explicit CFont(CResourceEntry *pEntry = nullptr);
     ~CFont();
-    CDependencyTree* BuildDependencyTree() const override;
+    std::unique_ptr<CDependencyTree> BuildDependencyTree() const override;
     CVector2f RenderString(const TString& rkString, CRenderer *pRenderer, float AspectRatio,
                            CVector2f Position = CVector2f(0,0),
                            CColor FillColor = CColor::skWhite, CColor StrokeColor = CColor::skBlack,

src/Core/Resource/CMapArea.h

@@ -10,13 +10,13 @@ class CMapArea : public CResource
     CAssetID mNameString;
 
 public:
-    CMapArea(CResourceEntry *pEntry = nullptr)
+    explicit CMapArea(CResourceEntry *pEntry = nullptr)
         : CResource(pEntry)
     {}
 
-    CDependencyTree* BuildDependencyTree() const override
+    std::unique_ptr<CDependencyTree> BuildDependencyTree() const override
     {
-        auto *pTree = new CDependencyTree();
+        auto pTree = std::make_unique<CDependencyTree>();
         pTree->AddDependency(mNameString);
         return pTree;
     }

src/Core/Resource/CResource.h

@@ -10,6 +10,7 @@
 #include <Common/CFourCC.h>
 #include <Common/TString.h>
 #include <Common/Serialization/IArchive.h>
+#include <memory>
 
 // This macro creates functions that allow us to easily identify this resource type.
 // Must be included on every CResource subclass.
@@ -41,9 +42,9 @@ public:
     }
 
     virtual ~CResource() {}
-    virtual CDependencyTree* BuildDependencyTree() const    { return new CDependencyTree(); }
-    virtual void Serialize(IArchive& /*rArc*/)              {}
-    virtual void InitializeNewResource()                    {}
+    virtual std::unique_ptr<CDependencyTree> BuildDependencyTree() const { return std::make_unique<CDependencyTree>(); }
+    virtual void Serialize(IArchive& /*rArc*/) {}
+    virtual void InitializeNewResource()       {}
 
     CResourceEntry* Entry() const    { return mpEntry; }
     CResTypeInfo* TypeInfo() const   { return mpEntry->TypeInfo(); }

src/Core/Resource/CWorld.cpp

@@ -17,14 +17,14 @@ CWorld::~CWorld()
 {
 }
 
-CDependencyTree* CWorld::BuildDependencyTree() const
+std::unique_ptr<CDependencyTree> CWorld::BuildDependencyTree() const
 {
-    CDependencyTree *pTree = new CDependencyTree();
+    auto pTree = std::make_unique<CDependencyTree>();
 
-    for (uint32 iArea = 0; iArea < mAreas.size(); iArea++)
+    for (const auto& area : mAreas)
     {
-        pTree->AddDependency(mAreas[iArea].AreaResID);
-        pTree->AddDependency(mAreas[iArea].pAreaName);
+        pTree->AddDependency(area.AreaResID);
+        pTree->AddDependency(area.pAreaName);
     }
     
     pTree->AddDependency(mpWorldName);

src/Core/Resource/CWorld.h

@@ -90,7 +90,7 @@ public:
     explicit CWorld(CResourceEntry *pEntry = nullptr);
     ~CWorld();
 
-    CDependencyTree* BuildDependencyTree() const override;
+    std::unique_ptr<CDependencyTree> BuildDependencyTree() const override;
     void SetAreaLayerInfo(CGameArea *pArea);
     TString InGameName() const;
     TString AreaInGameName(uint32 AreaIndex) const;

src/Core/Resource/Factory/CScriptLoader.cpp

@@ -164,7 +164,7 @@ void CScriptLoader::ReadProperty(IProperty *pProp, uint32 Size, IInputStream& rS
                 {
                     warnf("%s [0x%X]: Asset property \"%s\" (%s) has a reference to an illegal asset type: %s",
                           *rSCLY.GetSourceString(),
-                          rSCLY.Tell() - ID.Length(),
+                          rSCLY.Tell() - static_cast<uint32>(ID.Length()),
                           *pAsset->Name(),
                           *pAsset->IDString(true),
                           *pEntry->CookedExtension().ToString());

src/Core/Resource/Model/CModel.cpp

@@ -30,19 +30,18 @@ CModel::~CModel()
 }
 
 
-CDependencyTree* CModel::BuildDependencyTree() const
+std::unique_ptr<CDependencyTree> CModel::BuildDependencyTree() const
 {
-    CDependencyTree *pTree = new CDependencyTree();
+    auto pTree = std::make_unique<CDependencyTree>();
     std::set<CAssetID> TextureIDs;
 
-    for (uint32 iSet = 0; iSet < mMaterialSets.size(); iSet++)
+    for (CMaterialSet* set : mMaterialSets)
     {
-        CMaterialSet *pSet = mMaterialSets[iSet];
-        pSet->GetUsedTextureIDs(TextureIDs);
+        set->GetUsedTextureIDs(TextureIDs);
     }
 
-    for (auto Iter = TextureIDs.begin(); Iter != TextureIDs.end(); Iter++)
-        pTree->AddDependency(*Iter);
+    for (const auto& id : TextureIDs)
+        pTree->AddDependency(id);
 
     return pTree;
 }

src/Core/Resource/Model/CModel.h

@@ -25,7 +25,7 @@ public:
     CModel(CMaterialSet *pSet, bool OwnsMatSet);
     ~CModel();
 
-    CDependencyTree* BuildDependencyTree() const override;
+    std::unique_ptr<CDependencyTree> BuildDependencyTree() const override;
     void BufferGL();
     void GenerateMaterialShaders();
     void ClearGLBuffer() override;

src/Core/Resource/Scan/CScan.cpp

@@ -43,9 +43,9 @@ CBoolRef CScan::IsCriticalPropertyRef() const
 }
 
 /** CResource interface */
-CDependencyTree* CScan::BuildDependencyTree() const
+std::unique_ptr<CDependencyTree> CScan::BuildDependencyTree() const
 {
-    CDependencyTree* pTree = new CDependencyTree();
+    auto pTree = std::make_unique<CDependencyTree>();
     pTree->ParseProperties(Entry(), ScanData().Property(), ScanData().DataPointer());
     return pTree;
 }

src/Core/Resource/Scan/CScan.h

@@ -28,7 +28,7 @@ public:
     CBoolRef IsCriticalPropertyRef() const;
 
     /** CResource interface */
-    CDependencyTree* BuildDependencyTree() const override;
+    std::unique_ptr<CDependencyTree> BuildDependencyTree() const override;
 };
 
 #endif // CSCAN_H

src/Core/Resource/StringTable/CStringTable.cpp

@@ -254,19 +254,17 @@ void CStringTable::Serialize(IArchive& Arc)
 }
 
 /** Build the dependency tree for this resource */
-CDependencyTree* CStringTable::BuildDependencyTree() const
+std::unique_ptr<CDependencyTree> CStringTable::BuildDependencyTree() const
 {
     // STRGs can reference FONTs with the &font=; formatting tag and TXTRs with the &image=; tag
-    CDependencyTree* pTree = new CDependencyTree();
+    auto pTree = std::make_unique<CDependencyTree>();
     EIDLength IDLength = CAssetID::GameIDLength( Game() );
 
-    for (uint LanguageIdx = 0; LanguageIdx < mLanguages.size(); LanguageIdx++)
+    for (const SLanguageData& language : mLanguages)
     {
-        const SLanguageData& kLanguage = mLanguages[LanguageIdx];
-
-        for (uint StringIdx = 0; StringIdx < kLanguage.Strings.size(); StringIdx++)
+        for (const auto& stringData : language.Strings)
         {
-            const TString& kString = kLanguage.Strings[StringIdx].String;
+            const TString& kString = stringData.String;
 
             for (int TagIdx = kString.IndexOf('&'); TagIdx != -1; TagIdx = kString.IndexOf('&', TagIdx + 1))
             {
@@ -320,7 +318,7 @@ CDependencyTree* CStringTable::BuildDependencyTree() const
                     else if (ImageType == "B")
                         TexturesStart = 2;
 
-                    else if (ImageType.IsHexString(false, IDLength * 2))
+                    else if (ImageType.IsHexString(false, static_cast<int>(IDLength) * 2))
                         TexturesStart = 0;
 
                     else

src/Core/Resource/StringTable/CStringTable.h

@@ -89,7 +89,7 @@ public:
     void Serialize(IArchive& Arc) override;
 
     /** Build the dependency tree for this resource */
-    CDependencyTree* BuildDependencyTree() const override;
+    std::unique_ptr<CDependencyTree> BuildDependencyTree() const override;
 
     /** Static - Strip all formatting tags for a given string */
     static TString StripFormatting(const TString& kInString);