From eabc7899102f851e0ce6243b77ccba027a0b4dd9 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 12 Jun 2020 10:26:00 -0400 Subject: [PATCH] CAnimSet: Simplify allocation code We can make use of std::unique_ptr to prevent any potential memory leaks from occurring and simplifying the destruction code. --- src/Core/Resource/Animation/CAnimSet.h | 19 ++----- src/Core/Resource/Animation/CSourceAnimData.h | 28 ++++------- .../Resource/Animation/IMetaAnimation.cpp | 49 +++++++------------ src/Core/Resource/Animation/IMetaAnimation.h | 16 +++--- .../Resource/Animation/IMetaTransition.cpp | 21 ++++---- src/Core/Resource/Animation/IMetaTransition.h | 8 ++- src/Core/Resource/Factory/CAnimSetLoader.cpp | 18 +++---- 7 files changed, 64 insertions(+), 95 deletions(-) diff --git a/src/Core/Resource/Animation/CAnimSet.h b/src/Core/Resource/Animation/CAnimSet.h index a1db3f3e..ab47f182 100644 --- a/src/Core/Resource/Animation/CAnimSet.h +++ b/src/Core/Resource/Animation/CAnimSet.h @@ -29,7 +29,7 @@ struct SAdditiveAnim struct SAnimation { TString Name; - IMetaAnimation *pMetaAnim; + std::unique_ptr pMetaAnim; }; struct STransition @@ -37,13 +37,13 @@ struct STransition uint32 Unknown; uint32 AnimIdA; uint32 AnimIdB; - IMetaTransition *pMetaTrans; + std::unique_ptr pMetaTrans; }; struct SHalfTransition { uint32 AnimID; - IMetaTransition *pMetaTrans; + std::unique_ptr pMetaTrans; }; // Character structures @@ -96,7 +96,7 @@ class CAnimSet : public CResource std::vector mAnimPrimitives; std::vector mAnimations; std::vector mTransitions; - IMetaTransition *mpDefaultTransition = nullptr; + std::unique_ptr mpDefaultTransition; std::vector mAdditiveAnims; float mDefaultAdditiveFadeIn = 0.0f; float mDefaultAdditiveFadeOut = 0.0f; @@ -110,17 +110,6 @@ public: ~CAnimSet() { - for (auto& anim : mAnimations) - delete anim.pMetaAnim; - - for (auto& trans : mTransitions) - delete trans.pMetaTrans; - - for (auto& half : mHalfTransitions) - delete half.pMetaTrans; - - delete mpDefaultTransition; - // For MP2, anim events need to be cleaned up manually for ([[maybe_unused]] const auto& event : mAnimEvents) { diff --git a/src/Core/Resource/Animation/CSourceAnimData.h b/src/Core/Resource/Animation/CSourceAnimData.h index 49e70765..d1a2b4af 100644 --- a/src/Core/Resource/Animation/CSourceAnimData.h +++ b/src/Core/Resource/Animation/CSourceAnimData.h @@ -4,6 +4,8 @@ #include "Core/Resource/CResource.h" #include "IMetaTransition.h" +#include + class CSourceAnimData : public CResource { DECLARE_RESOURCE_TYPE(SourceAnimData) @@ -13,35 +15,25 @@ class CSourceAnimData : public CResource { CAssetID AnimA; CAssetID AnimB; - IMetaTransition *pTransition; + std::unique_ptr pTransition; }; struct SHalfTransition { CAssetID Anim; - IMetaTransition *pTransition; + std::unique_ptr pTransition; }; std::vector mTransitions; std::vector mHalfTransitions; - IMetaTransition *mpDefaultTransition; + std::unique_ptr mpDefaultTransition; public: explicit CSourceAnimData(CResourceEntry *pEntry = nullptr) : CResource(pEntry) - , mpDefaultTransition(nullptr) {} - ~CSourceAnimData() - { - for (uint32 TransIdx = 0; TransIdx < mTransitions.size(); TransIdx++) - delete mTransitions[TransIdx].pTransition; - - for (uint32 HalfIdx = 0; HalfIdx < mHalfTransitions.size(); HalfIdx++) - delete mHalfTransitions[HalfIdx].pTransition; - - delete mpDefaultTransition; - } + ~CSourceAnimData() = default; std::unique_ptr BuildDependencyTree() const override { @@ -74,16 +66,16 @@ public: // Find all relevant primitives std::set PrimSet; - if (UsedTransitions.find(mpDefaultTransition) == UsedTransitions.end()) + if (UsedTransitions.find(mpDefaultTransition.get()) == UsedTransitions.end()) { mpDefaultTransition->GetUniquePrimitives(PrimSet); - UsedTransitions.insert(mpDefaultTransition); + UsedTransitions.insert(mpDefaultTransition.get()); } for (uint32 TransitionIdx = 0; TransitionIdx < mTransitions.size(); TransitionIdx++) { const STransition& rkTransition = mTransitions[TransitionIdx]; - IMetaTransition *pTransition = rkTransition.pTransition; + IMetaTransition *pTransition = rkTransition.pTransition.get(); if ( pTree->HasDependency(rkTransition.AnimA) && pTree->HasDependency(rkTransition.AnimB) && @@ -97,7 +89,7 @@ public: for (uint32 HalfIdx = 0; HalfIdx < mHalfTransitions.size(); HalfIdx++) { const SHalfTransition& rkHalfTrans = mHalfTransitions[HalfIdx]; - IMetaTransition *pTransition = rkHalfTrans.pTransition; + IMetaTransition *pTransition = rkHalfTrans.pTransition.get(); if ( pTree->HasDependency(rkHalfTrans.Anim) && UsedTransitions.find(pTransition) == UsedTransitions.end() ) diff --git a/src/Core/Resource/Animation/IMetaAnimation.cpp b/src/Core/Resource/Animation/IMetaAnimation.cpp index 9c7939c2..a91ce7b2 100644 --- a/src/Core/Resource/Animation/IMetaAnimation.cpp +++ b/src/Core/Resource/Animation/IMetaAnimation.cpp @@ -3,27 +3,27 @@ // ************ CMetaAnimFactory ************ CMetaAnimFactory gMetaAnimFactory; -IMetaAnimation* CMetaAnimFactory::LoadFromStream(IInputStream& rInput, EGame Game) +std::unique_ptr CMetaAnimFactory::LoadFromStream(IInputStream& rInput, EGame Game) const { - EMetaAnimType Type = (EMetaAnimType) rInput.ReadLong(); + const auto Type = static_cast(rInput.ReadLong()); switch (Type) { case EMetaAnimType::Play: - return new CMetaAnimPlay(rInput, Game); + return std::make_unique(rInput, Game); case EMetaAnimType::Blend: case EMetaAnimType::PhaseBlend: - return new CMetaAnimBlend(Type, rInput, Game); + return std::make_unique(Type, rInput, Game); case EMetaAnimType::Random: - return new CMetaAnimRandom(rInput, Game); + return std::make_unique(rInput, Game); case EMetaAnimType::Sequence: - return new CMetaAnimSequence(rInput, Game); + return std::make_unique(rInput, Game); default: - errorf("Unrecognized meta-animation type: %d", Type); + errorf("Unrecognized meta-animation type: %d", static_cast(Type)); return nullptr; } } @@ -64,11 +64,7 @@ CMetaAnimBlend::CMetaAnimBlend(EMetaAnimType Type, IInputStream& rInput, EGame G mUnknownB = rInput.ReadBool(); } -CMetaAnimBlend::~CMetaAnimBlend() -{ - delete mpMetaAnimA; - delete mpMetaAnimB; -} +CMetaAnimBlend::~CMetaAnimBlend() = default; EMetaAnimType CMetaAnimBlend::Type() const { @@ -84,7 +80,7 @@ void CMetaAnimBlend::GetUniquePrimitives(std::set& rPrimSet) con // ************ CMetaAnimRandom ************ CMetaAnimRandom::CMetaAnimRandom(IInputStream& rInput, EGame Game) { - uint32 NumPairs = rInput.ReadLong(); + const uint32 NumPairs = rInput.ReadLong(); mProbabilityPairs.reserve(NumPairs); for (uint32 iAnim = 0; iAnim < NumPairs; iAnim++) @@ -92,15 +88,11 @@ CMetaAnimRandom::CMetaAnimRandom(IInputStream& rInput, EGame Game) SAnimProbabilityPair Pair; Pair.pAnim = gMetaAnimFactory.LoadFromStream(rInput, Game); Pair.Probability = rInput.ReadLong(); - mProbabilityPairs.push_back(Pair); + mProbabilityPairs.push_back(std::move(Pair)); } } -CMetaAnimRandom::~CMetaAnimRandom() -{ - for (uint32 iPair = 0; iPair < mProbabilityPairs.size(); iPair++) - delete mProbabilityPairs[iPair].pAnim; -} +CMetaAnimRandom::~CMetaAnimRandom() = default; EMetaAnimType CMetaAnimRandom::Type() const { @@ -109,28 +101,23 @@ EMetaAnimType CMetaAnimRandom::Type() const void CMetaAnimRandom::GetUniquePrimitives(std::set& rPrimSet) const { - for (uint32 iPair = 0; iPair < mProbabilityPairs.size(); iPair++) - mProbabilityPairs[iPair].pAnim->GetUniquePrimitives(rPrimSet); + for (auto& pair : mProbabilityPairs) + pair.pAnim->GetUniquePrimitives(rPrimSet); } // ************ CMetaAnimSequence ************ CMetaAnimSequence::CMetaAnimSequence(IInputStream& rInput, EGame Game) { - uint32 NumAnims = rInput.ReadLong(); + const uint32 NumAnims = rInput.ReadLong(); mAnimations.reserve(NumAnims); for (uint32 iAnim = 0; iAnim < NumAnims; iAnim++) { - IMetaAnimation *pAnim = gMetaAnimFactory.LoadFromStream(rInput, Game); - mAnimations.push_back(pAnim); + mAnimations.push_back(gMetaAnimFactory.LoadFromStream(rInput, Game)); } } -CMetaAnimSequence::~CMetaAnimSequence() -{ - for (uint32 iAnim = 0; iAnim < mAnimations.size(); iAnim++) - delete mAnimations[iAnim]; -} +CMetaAnimSequence::~CMetaAnimSequence() = default; EMetaAnimType CMetaAnimSequence::Type() const { @@ -139,6 +126,6 @@ EMetaAnimType CMetaAnimSequence::Type() const void CMetaAnimSequence::GetUniquePrimitives(std::set& rPrimSet) const { - for (uint32 iAnim = 0; iAnim < mAnimations.size(); iAnim++) - mAnimations[iAnim]->GetUniquePrimitives(rPrimSet); + for (auto& anim : mAnimations) + anim->GetUniquePrimitives(rPrimSet); } diff --git a/src/Core/Resource/Animation/IMetaAnimation.h b/src/Core/Resource/Animation/IMetaAnimation.h index 9780b98c..bf3b6f54 100644 --- a/src/Core/Resource/Animation/IMetaAnimation.h +++ b/src/Core/Resource/Animation/IMetaAnimation.h @@ -18,7 +18,7 @@ enum class EMetaAnimType class CMetaAnimFactory { public: - class IMetaAnimation* LoadFromStream(IInputStream& rInput, EGame Game); + std::unique_ptr LoadFromStream(IInputStream& rInput, EGame Game) const; }; extern CMetaAnimFactory gMetaAnimFactory; @@ -65,7 +65,7 @@ public: virtual void GetUniquePrimitives(std::set& rPrimSet) const = 0; // Static - static IMetaAnimation* LoadFromStream(IInputStream& rInput, EGame Game); + static std::unique_ptr LoadFromStream(IInputStream& rInput, EGame Game); }; // CMetaAnimPlay - plays an animation @@ -93,8 +93,8 @@ class CMetaAnimBlend : public IMetaAnimation { protected: EMetaAnimType mType; - IMetaAnimation *mpMetaAnimA; - IMetaAnimation *mpMetaAnimB; + std::unique_ptr mpMetaAnimA; + std::unique_ptr mpMetaAnimB; float mUnknownA; bool mUnknownB; @@ -105,8 +105,8 @@ public: void GetUniquePrimitives(std::set& rPrimSet) const override; // Accessors - IMetaAnimation* BlendAnimationA() const { return mpMetaAnimA; } - IMetaAnimation* BlendAnimationB() const { return mpMetaAnimB; } + IMetaAnimation* BlendAnimationA() const { return mpMetaAnimA.get(); } + IMetaAnimation* BlendAnimationB() const { return mpMetaAnimB.get(); } float UnknownA() const { return mUnknownA; } bool UnknownB() const { return mUnknownB; } }; @@ -114,7 +114,7 @@ public: // SAnimProbabilityPair - structure used by CMetaAnimationRandom to associate an animation with a probability value struct SAnimProbabilityPair { - IMetaAnimation *pAnim; + std::unique_ptr pAnim; uint32 Probability; }; @@ -135,7 +135,7 @@ public: class CMetaAnimSequence : public IMetaAnimation { protected: - std::vector mAnimations; + std::vector> mAnimations; public: CMetaAnimSequence(IInputStream& rInput, EGame Game); diff --git a/src/Core/Resource/Animation/IMetaTransition.cpp b/src/Core/Resource/Animation/IMetaTransition.cpp index 2494c1a6..858f0240 100644 --- a/src/Core/Resource/Animation/IMetaTransition.cpp +++ b/src/Core/Resource/Animation/IMetaTransition.cpp @@ -4,41 +4,38 @@ // ************ CMetaTransFactory ************ CMetaTransFactory gMetaTransFactory; -IMetaTransition* CMetaTransFactory::LoadFromStream(IInputStream& rInput, EGame Game) +std::unique_ptr CMetaTransFactory::LoadFromStream(IInputStream& rInput, EGame Game) const { - EMetaTransType Type = (EMetaTransType) rInput.ReadLong(); + const auto Type = static_cast(rInput.ReadLong()); switch (Type) { case EMetaTransType::MetaAnim: - return new CMetaTransMetaAnim(rInput, Game); + return std::make_unique(rInput, Game); case EMetaTransType::Trans: case EMetaTransType::PhaseTrans: - return new CMetaTransTrans(Type, rInput, Game); + return std::make_unique(Type, rInput, Game); case EMetaTransType::Snap: - return new CMetaTransSnap(rInput, Game); + return std::make_unique(rInput, Game); case EMetaTransType::Type4: - return new CMetaTransType4(rInput, Game); + return std::make_unique(rInput, Game); default: - errorf("Unrecognized meta-transition type: %d", Type); + errorf("Unrecognized meta-transition type: %d", static_cast(Type)); return nullptr; } } // ************ CMetaTransMetaAnim ************ CMetaTransMetaAnim::CMetaTransMetaAnim(IInputStream& rInput, EGame Game) + : mpAnim{gMetaAnimFactory.LoadFromStream(rInput, Game)} { - mpAnim = gMetaAnimFactory.LoadFromStream(rInput, Game); } -CMetaTransMetaAnim::~CMetaTransMetaAnim() -{ - delete mpAnim; -} +CMetaTransMetaAnim::~CMetaTransMetaAnim() = default; EMetaTransType CMetaTransMetaAnim::Type() const { diff --git a/src/Core/Resource/Animation/IMetaTransition.h b/src/Core/Resource/Animation/IMetaTransition.h index 8d25d390..08714ea3 100644 --- a/src/Core/Resource/Animation/IMetaTransition.h +++ b/src/Core/Resource/Animation/IMetaTransition.h @@ -2,9 +2,13 @@ #define IMETATRANSITION_H #include "IMetaAnimation.h" +#include +#include +class IInputStream; class IMetaAnimation; class IMetaTransition; +enum class EGame; enum class EMetaTransType { @@ -19,7 +23,7 @@ enum class EMetaTransType class CMetaTransFactory { public: - class IMetaTransition* LoadFromStream(IInputStream& rInput, EGame Game); + std::unique_ptr LoadFromStream(IInputStream& rInput, EGame Game) const; }; extern CMetaTransFactory gMetaTransFactory; @@ -36,7 +40,7 @@ public: // CMetaTransMetaAnim class CMetaTransMetaAnim : public IMetaTransition { - IMetaAnimation *mpAnim; + std::unique_ptr mpAnim; public: CMetaTransMetaAnim(IInputStream& rInput, EGame Game); diff --git a/src/Core/Resource/Factory/CAnimSetLoader.cpp b/src/Core/Resource/Factory/CAnimSetLoader.cpp index 1de50e47..d87056d6 100644 --- a/src/Core/Resource/Factory/CAnimSetLoader.cpp +++ b/src/Core/Resource/Factory/CAnimSetLoader.cpp @@ -52,7 +52,7 @@ void CAnimSetLoader::LoadCorruptionCHAR(IInputStream& rCHAR) SAnimation Anim; Anim.Name = rCHAR.ReadString(); Anim.pMetaAnim = gMetaAnimFactory.LoadFromStream(rCHAR, mGame); - pSet->mAnimations.push_back(Anim); + pSet->mAnimations.push_back(std::move(Anim)); } // Animation Bounds @@ -141,8 +141,8 @@ void CAnimSetLoader::LoadReturnsCHAR(IInputStream& rCHAR) // small hack - create a meta-anim for it so we can generate asset names for the ANIM files correctly SAnimation Anim; Anim.Name = AnimName; - Anim.pMetaAnim = new CMetaAnimPlay( CAnimPrimitive(AnimID, AnimIdx, AnimName), 0.f, 0 ); - pSet->mAnimations.push_back(Anim); + Anim.pMetaAnim = std::make_unique(CAnimPrimitive(AnimID, AnimIdx, AnimName), 0.f, 0); + pSet->mAnimations.push_back(std::move(Anim)); } // The only other thing we care about right now is the dependency list. If this file doesn't have a dependency list, exit out. @@ -322,7 +322,7 @@ void CAnimSetLoader::LoadAnimationSet(IInputStream& rANCS) SAnimation Anim; Anim.Name = rANCS.ReadString(); Anim.pMetaAnim = gMetaAnimFactory.LoadFromStream(rANCS, mGame); - pSet->mAnimations.push_back(Anim); + pSet->mAnimations.push_back(std::move(Anim)); } // Transitions @@ -336,7 +336,7 @@ void CAnimSetLoader::LoadAnimationSet(IInputStream& rANCS) Trans.AnimIdA = rANCS.ReadLong(); Trans.AnimIdB = rANCS.ReadLong(); Trans.pMetaTrans = gMetaTransFactory.LoadFromStream(rANCS, mGame); - pSet->mTransitions.push_back(Trans); + pSet->mTransitions.push_back(std::move(Trans)); } pSet->mpDefaultTransition = gMetaTransFactory.LoadFromStream(rANCS, mGame); @@ -351,7 +351,7 @@ void CAnimSetLoader::LoadAnimationSet(IInputStream& rANCS) Anim.AnimID = rANCS.ReadLong(); Anim.FadeInTime = rANCS.ReadFloat(); Anim.FadeOutTime = rANCS.ReadFloat(); - pSet->mAdditiveAnims.push_back(Anim); + pSet->mAdditiveAnims.push_back(std::move(Anim)); } pSet->mDefaultAdditiveFadeIn = rANCS.ReadFloat(); @@ -368,7 +368,7 @@ void CAnimSetLoader::LoadAnimationSet(IInputStream& rANCS) SHalfTransition Trans; Trans.AnimID = rANCS.ReadLong(); Trans.pMetaTrans = gMetaTransFactory.LoadFromStream(rANCS, mGame); - pSet->mHalfTransitions.push_back(Trans); + pSet->mHalfTransitions.push_back(std::move(Trans)); } } @@ -673,7 +673,7 @@ std::unique_ptr CAnimSetLoader::LoadSAND(IInputStream& rSAND, C Transition.AnimA = CAssetID(rSAND, EIDLength::k64Bit); Transition.AnimB = CAssetID(rSAND, EIDLength::k64Bit); Transition.pTransition = gMetaTransFactory.LoadFromStream(rSAND, pEntry->Game()); - pData->mTransitions.push_back(Transition); + pData->mTransitions.push_back(std::move(Transition)); } // Half Transitions @@ -687,7 +687,7 @@ std::unique_ptr CAnimSetLoader::LoadSAND(IInputStream& rSAND, C CSourceAnimData::SHalfTransition HalfTrans; HalfTrans.Anim = CAssetID(rSAND, EIDLength::k64Bit); HalfTrans.pTransition = gMetaTransFactory.LoadFromStream(rSAND, pEntry->Game()); - pData->mHalfTransitions.push_back(HalfTrans); + pData->mHalfTransitions.push_back(std::move(HalfTrans)); } // Default Transition