From 1ae5462cd7b9e1649314053cc19433451747101d Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 12 Jun 2020 14:56:23 -0400 Subject: [PATCH] CGameProject: Make use of unique_ptr where applicable Makes the ownership semantics explicit. --- src/Core/GameProject/CGameExporter.cpp | 15 +++--- src/Core/GameProject/CGameExporter.h | 3 +- src/Core/GameProject/CGameProject.cpp | 63 +++++++++-------------- src/Core/GameProject/CGameProject.h | 70 +++++++++++--------------- src/Editor/CEditorApplication.cpp | 19 +++---- src/Editor/CEditorApplication.h | 17 ++++--- 6 files changed, 83 insertions(+), 104 deletions(-) diff --git a/src/Core/GameProject/CGameExporter.cpp b/src/Core/GameProject/CGameExporter.cpp index ae88e31f..7acf9b27 100644 --- a/src/Core/GameProject/CGameExporter.cpp +++ b/src/Core/GameProject/CGameExporter.cpp @@ -94,7 +94,7 @@ bool CGameExporter::Export(nod::DiscBase *pDisc, const TString& rkOutputDir, CAs // Export finished! mProjectPath = mpProject->ProjectPath(); - delete mpProject; + mpProject.reset(); if (pOldStore) gpResourceStore = pOldStore; return !mpProgress->ShouldCancel(); } @@ -280,7 +280,7 @@ void CGameExporter::LoadPaks() } TString RelPakPath = FileUtil::MakeRelative(PakPath.GetFileDirectory(), mpProject->DiscFilesystemRoot(false)); - CPackage *pPackage = new CPackage(mpProject, PakPath.GetFileName(false), RelPakPath); + auto pPackage = std::make_unique(mpProject.get(), PakPath.GetFileName(false), RelPakPath); // MP1-MP3Proto if (mGame < EGame::Corruption) @@ -327,12 +327,14 @@ void CGameExporter::LoadPaks() mAreaDuplicateMap[ResID] = AreaHasDuplicates; AreaHasDuplicates = false; } - else if (!AreaHasDuplicates && PakResourceSet.find(ResID) != PakResourceSet.end()) + { AreaHasDuplicates = true; - + } else + { PakResourceSet.insert(ResID); + } } } } @@ -423,11 +425,12 @@ void CGameExporter::LoadPaks() } // Add package to project and save - mpProject->AddPackage(pPackage); #if SAVE_PACKAGE_DEFINITIONS - bool SaveSuccess = pPackage->Save(); + [[maybe_unused]] const bool SaveSuccess = pPackage->Save(); ASSERT(SaveSuccess); #endif + + mpProject->AddPackage(std::move(pPackage)); } #endif } diff --git a/src/Core/GameProject/CGameExporter.h b/src/Core/GameProject/CGameExporter.h index 0453b87d..a5ca15c5 100644 --- a/src/Core/GameProject/CGameExporter.h +++ b/src/Core/GameProject/CGameExporter.h @@ -9,6 +9,7 @@ #include #include #include +#include #include enum class EDiscType @@ -21,7 +22,7 @@ enum class EDiscType class CGameExporter { // Project Data - CGameProject *mpProject; + std::unique_ptr mpProject; TString mProjectPath; CResourceStore *mpStore; EGame mGame; diff --git a/src/Core/GameProject/CGameProject.cpp b/src/Core/GameProject/CGameProject.cpp index fc395fb8..68a17f07 100644 --- a/src/Core/GameProject/CGameProject.cpp +++ b/src/Core/GameProject/CGameProject.cpp @@ -14,16 +14,13 @@ CGameProject::~CGameProject() { - if (mpResourceStore) - { - ASSERT(!mpResourceStore->IsCacheDirty()); + if (!mpResourceStore) + return; - if (gpResourceStore == mpResourceStore.get()) - gpResourceStore = nullptr; - } + ASSERT(!mpResourceStore->IsCacheDirty()); - for (uint32 iPkg = 0; iPkg < mPackages.size(); iPkg++) - delete mPackages[iPkg]; + if (gpResourceStore == mpResourceStore.get()) + gpResourceStore = nullptr; } bool CGameProject::Save() @@ -60,15 +57,14 @@ bool CGameProject::Serialize(IArchive& rArc) { ASSERT(mPackages.empty()); - for (uint32 iPkg = 0; iPkg < PackageList.size(); iPkg++) + for (const TString& packagePath : PackageList) { - const TString& rkPackagePath = PackageList[iPkg]; - TString PackageName = rkPackagePath.GetFileName(false); - TString PackageDir = rkPackagePath.GetFileDirectory(); + TString PackageName = packagePath.GetFileName(false); + TString PackageDir = packagePath.GetFileDirectory(); - CPackage *pPackage = new CPackage(this, PackageName, PackageDir); - bool PackageLoadSuccess = pPackage->Load(); - mPackages.push_back(pPackage); + auto pPackage = std::make_unique(this, PackageName, PackageDir); + const bool PackageLoadSuccess = pPackage->Load(); + mPackages.push_back(std::move(pPackage)); if (!PackageLoadSuccess) { @@ -126,10 +122,8 @@ bool CGameProject::MergeISO(const TString& rkIsoPath, nod::DiscWii *pOriginalIso void CGameProject::GetWorldList(std::list& rOut) const { - for (uint32 iPkg = 0; iPkg < mPackages.size(); iPkg++) + for (const auto& pPkg : mPackages) { - CPackage *pPkg = mPackages[iPkg]; - // Little workaround to fix some of Retro's paks having worlds listed in the wrong order... // Construct a sorted list of worlds in this package std::list PackageWorlds; @@ -147,23 +141,20 @@ void CGameProject::GetWorldList(std::list& rOut) const }); // Add sorted worlds to the output world list - for (auto Iter = PackageWorlds.begin(); Iter != PackageWorlds.end(); Iter++) + for (const auto* res : PackageWorlds) { - const SNamedResource *pkRes = *Iter; - rOut.push_back(pkRes->ID); + rOut.push_back(res->ID); } } } CAssetID CGameProject::FindNamedResource(const TString& rkName) const { - for (uint32 iPkg = 0; iPkg < mPackages.size(); iPkg++) + for (const auto& pkg : mPackages) { - CPackage *pPkg = mPackages[iPkg]; - - for (uint32 iRes = 0; iRes < pPkg->NumNamedResources(); iRes++) + for (uint32 iRes = 0; iRes < pkg->NumNamedResources(); iRes++) { - const SNamedResource& rkRes = pPkg->NamedResourceByIndex(iRes); + const SNamedResource& rkRes = pkg->NamedResourceByIndex(iRes); if (rkRes.Name == rkName) return rkRes.ID; @@ -175,20 +166,18 @@ CAssetID CGameProject::FindNamedResource(const TString& rkName) const CPackage* CGameProject::FindPackage(const TString& rkName) const { - for (uint32 iPkg = 0; iPkg < mPackages.size(); iPkg++) + for (const auto& pPackage : mPackages) { - CPackage *pPackage = mPackages[iPkg]; - if (pPackage->Name() == rkName) { - return pPackage; + return pPackage.get(); } } return nullptr; } -CGameProject* CGameProject::CreateProjectForExport( +std::unique_ptr CGameProject::CreateProjectForExport( const TString& rkProjRootDir, EGame Game, ERegion Region, @@ -196,7 +185,7 @@ CGameProject* CGameProject::CreateProjectForExport( float BuildVer ) { - CGameProject *pProj = new CGameProject; + auto pProj = std::unique_ptr(new CGameProject()); pProj->mGame = Game; pProj->mRegion = Region; pProj->mGameID = rkGameID; @@ -204,15 +193,15 @@ CGameProject* CGameProject::CreateProjectForExport( pProj->mProjectRoot = rkProjRootDir; pProj->mProjectRoot.Replace("\\", "/"); - pProj->mpResourceStore = std::make_unique(pProj); + pProj->mpResourceStore = std::make_unique(pProj.get()); pProj->mpGameInfo->LoadGameInfo(Game); return pProj; } -CGameProject* CGameProject::LoadProject(const TString& rkProjPath, IProgressNotifier *pProgress) +std::unique_ptr CGameProject::LoadProject(const TString& rkProjPath, IProgressNotifier *pProgress) { // Init project - CGameProject *pProj = new CGameProject; + auto pProj = std::unique_ptr(new CGameProject()); pProj->mProjectRoot = rkProjPath.GetFileDirectory(); pProj->mProjectRoot.Replace("\\", "/"); @@ -228,7 +217,6 @@ CGameProject* CGameProject::LoadProject(const TString& rkProjPath, IProgressNoti if (!Reader.IsValid()) { - delete pProj; return nullptr; } @@ -238,7 +226,7 @@ CGameProject* CGameProject::LoadProject(const TString& rkProjPath, IProgressNoti { // Load resource database pProgress->Report("Loading resource database"); - pProj->mpResourceStore = std::make_unique(pProj); + pProj->mpResourceStore = std::make_unique(pProj.get()); LoadSuccess = pProj->mpResourceStore->LoadDatabaseCache(); // Removed database validation step. We used to do this on project load to make sure all data was correct, but this takes a long @@ -270,7 +258,6 @@ CGameProject* CGameProject::LoadProject(const TString& rkProjPath, IProgressNoti if (!LoadSuccess) { - delete pProj; return nullptr; } diff --git a/src/Core/GameProject/CGameProject.h b/src/Core/GameProject/CGameProject.h index e8e6189e..b53caee5 100644 --- a/src/Core/GameProject/CGameProject.h +++ b/src/Core/GameProject/CGameProject.h @@ -13,6 +13,7 @@ #include #include #include +#include namespace nod { class DiscWii; } @@ -23,41 +24,30 @@ enum class EProjectVersion // Add new versions before this line Max, - Current = EProjectVersion::Max - 1 + Current = Max - 1 }; class CGameProject { - TString mProjectName; - EGame mGame; - ERegion mRegion; - TString mGameID; - float mBuildVersion; + TString mProjectName{"Unnamed Project"}; + EGame mGame{EGame::Invalid}; + ERegion mRegion{ERegion::Unknown}; + TString mGameID{"000000"}; + float mBuildVersion = 0.f; TString mProjectRoot; - std::vector mPackages; + std::vector> mPackages; std::unique_ptr mpResourceStore; - std::unique_ptr mpGameInfo; - std::unique_ptr mpAudioManager; - std::unique_ptr mpTweakManager; + std::unique_ptr mpGameInfo = std::make_unique(); + std::unique_ptr mpAudioManager = std::make_unique(this); + std::unique_ptr mpTweakManager = std::make_unique(this); // Keep file handle open for the .prj file to prevent users from opening the same project // in multiple instances of PWE CFileLock mProjFileLock; // Private Constructor - CGameProject() - : mProjectName("Unnamed Project") - , mGame(EGame::Invalid) - , mRegion(ERegion::Unknown) - , mGameID("000000") - , mBuildVersion(0.f) - , mpResourceStore(nullptr) - { - mpGameInfo = std::make_unique(); - mpAudioManager = std::make_unique(this); - mpTweakManager = std::make_unique(this); - } + CGameProject() = default; public: ~CGameProject(); @@ -71,7 +61,7 @@ public: CPackage* FindPackage(const TString& rkName) const; // Static - static CGameProject* CreateProjectForExport( + static std::unique_ptr CreateProjectForExport( const TString& rkProjRootDir, EGame Game, ERegion Region, @@ -79,7 +69,7 @@ public: float BuildVer ); - static CGameProject* LoadProject(const TString& rkProjPath, IProgressNotifier *pProgress); + static std::unique_ptr LoadProject(const TString& rkProjPath, IProgressNotifier *pProgress); // Directory Handling TString ProjectRoot() const { return mProjectRoot; } @@ -93,23 +83,23 @@ public: TString DiscFilesystemRoot(bool Relative) const { return DiscDir(Relative) + (IsWiiBuild() ? "DATA/" : "") + "files/"; } // Accessors - void SetProjectName(const TString& rkName) { mProjectName = rkName; } + void SetProjectName(const TString& rkName) { mProjectName = rkName; } - TString Name() const { return mProjectName; } - uint32 NumPackages() const { return mPackages.size(); } - CPackage* PackageByIndex(uint32 Index) const { return mPackages[Index]; } - void AddPackage(CPackage *pPackage) { mPackages.push_back(pPackage); } - CResourceStore* ResourceStore() const { return mpResourceStore.get(); } - CGameInfo* GameInfo() const { return mpGameInfo.get(); } - CAudioManager* AudioManager() const { return mpAudioManager.get(); } - CTweakManager* TweakManager() const { return mpTweakManager.get(); } - EGame Game() const { return mGame; } - ERegion Region() const { return mRegion; } - TString GameID() const { return mGameID; } - float BuildVersion() const { return mBuildVersion; } - bool IsWiiBuild() const { return mBuildVersion >= 3.f; } - bool IsTrilogy() const { return mGame <= EGame::Corruption && mBuildVersion >= 3.593f; } - bool IsWiiDeAsobu() const { return mGame <= EGame::Corruption && mBuildVersion >= 3.570f && mBuildVersion < 3.593f; } + TString Name() const { return mProjectName; } + uint32 NumPackages() const { return mPackages.size(); } + CPackage* PackageByIndex(uint32 Index) const { return mPackages[Index].get(); } + void AddPackage(std::unique_ptr&& package) { mPackages.push_back(std::move(package)); } + CResourceStore* ResourceStore() const { return mpResourceStore.get(); } + CGameInfo* GameInfo() const { return mpGameInfo.get(); } + CAudioManager* AudioManager() const { return mpAudioManager.get(); } + CTweakManager* TweakManager() const { return mpTweakManager.get(); } + EGame Game() const { return mGame; } + ERegion Region() const { return mRegion; } + TString GameID() const { return mGameID; } + float BuildVersion() const { return mBuildVersion; } + bool IsWiiBuild() const { return mBuildVersion >= 3.f; } + bool IsTrilogy() const { return mGame <= EGame::Corruption && mBuildVersion >= 3.593f; } + bool IsWiiDeAsobu() const { return mGame <= EGame::Corruption && mBuildVersion >= 3.570f && mBuildVersion < 3.593f; } }; #endif // CGAMEPROJECT_H diff --git a/src/Editor/CEditorApplication.cpp b/src/Editor/CEditorApplication.cpp index 7e883a7c..bb84cca0 100644 --- a/src/Editor/CEditorApplication.cpp +++ b/src/Editor/CEditorApplication.cpp @@ -77,11 +77,8 @@ bool CEditorApplication::CloseProject() NDolphinIntegration::KillQuickplay(); // Emit before actually deleting the project to allow editor references to clean up - CGameProject *pOldProj = mpActiveProject; - mpActiveProject = nullptr; + auto pOldProj = std::move(mpActiveProject); emit ActiveProjectChanged(nullptr); - delete pOldProj; - return true; } @@ -96,14 +93,15 @@ bool CEditorApplication::OpenProject(const QString& rkProjPath) CProgressDialog Dialog("Opening " + TO_QSTRING(Path.GetFileName()), true, true, mpWorldEditor); Dialog.DisallowCanceling(); - QFuture Future = QtConcurrent::run(&CGameProject::LoadProject, Path, &Dialog); - mpActiveProject = Dialog.WaitForResults(Future); + // Gross, but necessary until QtConcurrent supports move only types. + QFuture Future = QtConcurrent::run([](const auto& path, auto* dialog) { return CGameProject::LoadProject(path, dialog).release(); }, Path, &Dialog); + mpActiveProject = std::unique_ptr(Dialog.WaitForResults(Future)); Dialog.close(); if (mpActiveProject) { gpResourceStore = mpActiveProject->ResourceStore(); - emit ActiveProjectChanged(mpActiveProject); + emit ActiveProjectChanged(mpActiveProject.get()); return true; } else @@ -277,9 +275,8 @@ bool CEditorApplication::RebuildResourceDatabase() if (mpActiveProject && CloseAllEditors()) { // Fake-close the project, but keep it in memory so we can modify the resource store - CGameProject *pProj = mpActiveProject; + auto pProj = std::move(mpActiveProject); mpActiveProject->TweakManager()->ClearTweaks(); - mpActiveProject = nullptr; emit ActiveProjectChanged(nullptr); // Rebuild @@ -292,9 +289,9 @@ bool CEditorApplication::RebuildResourceDatabase() Dialog.close(); // Set project to active again - mpActiveProject = pProj; + mpActiveProject = std::move(pProj); mpActiveProject->TweakManager()->LoadTweaks(); - emit ActiveProjectChanged(pProj); + emit ActiveProjectChanged(mpActiveProject.get()); UICommon::InfoMsg(mpWorldEditor, "Success", "Resource database rebuilt successfully!"); return true; diff --git a/src/Editor/CEditorApplication.h b/src/Editor/CEditorApplication.h index 2c58e721..6680cd6f 100644 --- a/src/Editor/CEditorApplication.h +++ b/src/Editor/CEditorApplication.h @@ -5,6 +5,7 @@ #include #include #include +#include class CBasicViewport; class CProjectSettingsDialog; @@ -19,7 +20,7 @@ class CEditorApplication : public QApplication { Q_OBJECT - CGameProject *mpActiveProject; + std::unique_ptr mpActiveProject; CWorldEditor *mpWorldEditor; CResourceBrowser *mpResourceBrowser; CProjectSettingsDialog *mpProjectDialog; @@ -47,16 +48,16 @@ public: bool RebuildResourceDatabase(); - inline CResourceBrowser* ResourceBrowser() const { return mpResourceBrowser; } + CResourceBrowser* ResourceBrowser() const { return mpResourceBrowser; } // Accessors - inline CGameProject* ActiveProject() const { return mpActiveProject; } - inline CWorldEditor* WorldEditor() const { return mpWorldEditor; } - inline CProjectSettingsDialog* ProjectDialog() const { return mpProjectDialog; } - inline EGame CurrentGame() const { return mpActiveProject ? mpActiveProject->Game() : EGame::Invalid; } + CGameProject* ActiveProject() const { return mpActiveProject.get(); } + CWorldEditor* WorldEditor() const { return mpWorldEditor; } + CProjectSettingsDialog* ProjectDialog() const { return mpProjectDialog; } + EGame CurrentGame() const { return mpActiveProject ? mpActiveProject->Game() : EGame::Invalid; } - inline void SetEditorTicksEnabled(bool Enabled) { Enabled ? mRefreshTimer.start(gkTickFrequencyMS) : mRefreshTimer.stop(); } - inline bool AreEditorTicksEnabled() const { return mRefreshTimer.isActive(); } + void SetEditorTicksEnabled(bool Enabled) { Enabled ? mRefreshTimer.start(gkTickFrequencyMS) : mRefreshTimer.stop(); } + bool AreEditorTicksEnabled() const { return mRefreshTimer.isActive(); } public slots: void AddEditor(IEditor *pEditor);