From 99dd875b430e4348b045e4577e8914848b3e1d8e Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 31 May 2020 06:54:20 -0400 Subject: [PATCH 1/7] assetnameparser: Make use of an anonymous namespace Makes the bulk of helper functions internally linked. --- assetnameparser/main.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/assetnameparser/main.cpp b/assetnameparser/main.cpp index 014a23b38..3027a5b3c 100644 --- a/assetnameparser/main.cpp +++ b/assetnameparser/main.cpp @@ -30,7 +30,9 @@ #endif #endif -static logvisor::Module Log("AssetNameParser"); +namespace { +logvisor::Module Log("AssetNameParser"); + // TODO: Clean this up #undef bswap16 #undef bswap32 @@ -148,7 +150,7 @@ typedef std::string SystemString; typedef struct stat Sstat; #endif -static FILE* Fopen(const SystemChar* path, const SystemChar* mode, FileLockType lock = FileLockType::None) { +FILE* Fopen(const SystemChar* path, const SystemChar* mode, FileLockType lock = FileLockType::None) { #if IS_UCS2 FILE* fp = _wfopen(path, mode); if (!fp) @@ -172,6 +174,7 @@ static FILE* Fopen(const SystemChar* path, const SystemChar* mode, FileLockType return fp; } +} // Anonymous namespace #if _WIN32 int wmain(int argc, const wchar_t* argv[]) From e6aed18d59ac1f44b103872c50d1229c2f3387fd Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 31 May 2020 06:59:25 -0400 Subject: [PATCH 2/7] assetnameparser: Convert typdefs into using aliases --- assetnameparser/main.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/assetnameparser/main.cpp b/assetnameparser/main.cpp index 3027a5b3c..af86b8bf0 100644 --- a/assetnameparser/main.cpp +++ b/assetnameparser/main.cpp @@ -135,19 +135,19 @@ struct SAsset { enum class FileLockType { None = 0, Read, Write }; #if IS_UCS2 -typedef wchar_t SystemChar; -typedef std::wstring SystemString; +using SystemChar = wchar_t; +using SystemString = std::wstring; #ifndef _SYS_STR #define _SYS_STR(val) L##val #endif -typedef struct _stat Sstat; +using Sstat = struct _stat; #else -typedef char SystemChar; -typedef std::string SystemString; +using SystemChar = char; +using SystemString = std::string; #ifndef _SYS_STR #define _SYS_STR(val) val #endif -typedef struct stat Sstat; +using Sstat = struct stat; #endif FILE* Fopen(const SystemChar* path, const SystemChar* mode, FileLockType lock = FileLockType::None) { From c369af4adfba9aaf8e15745c49a61639d3e2fb67 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 31 May 2020 07:04:00 -0400 Subject: [PATCH 3/7] assetnameparser: Make Fopen return a unique_ptr Prevents leaks from occurring (aside from obvious .release() calls). --- assetnameparser/main.cpp | 61 ++++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 28 deletions(-) diff --git a/assetnameparser/main.cpp b/assetnameparser/main.cpp index af86b8bf0..a4a2c6465 100644 --- a/assetnameparser/main.cpp +++ b/assetnameparser/main.cpp @@ -1,8 +1,10 @@ -#include #include +#include +#include #include + +#include #include "tinyxml2/tinyxml2.h" -#include "logvisor/logvisor.hpp" #ifndef _WIN32 #include @@ -150,25 +152,33 @@ using SystemString = std::string; using Sstat = struct stat; #endif -FILE* Fopen(const SystemChar* path, const SystemChar* mode, FileLockType lock = FileLockType::None) { +struct FILEDeleter { + void operator()(FILE* file) const { std::fclose(file); } +}; +using FILEPtr = std::unique_ptr; + +FILEPtr Fopen(const SystemChar* path, const SystemChar* mode, FileLockType lock = FileLockType::None) { #if IS_UCS2 - FILE* fp = _wfopen(path, mode); - if (!fp) + FILEPtr fp{_wfopen(path, mode)}; + if (!fp) { return nullptr; + } #else - FILE* fp = fopen(path, mode); - if (!fp) + FILEPtr fp{std::fopen(path, mode)}; + if (!fp) { return nullptr; + } #endif if (lock != FileLockType::None) { #if _WIN32 OVERLAPPED ov = {}; - LockFileEx((HANDLE)(uintptr_t)_fileno(fp), (lock == FileLockType::Write) ? LOCKFILE_EXCLUSIVE_LOCK : 0, 0, 0, 1, - &ov); + LockFileEx((HANDLE)(uintptr_t)_fileno(fp.get()), (lock == FileLockType::Write) ? LOCKFILE_EXCLUSIVE_LOCK : 0, 0, 0, + 1, &ov); #else - if (flock(fileno(fp), ((lock == FileLockType::Write) ? LOCK_EX : LOCK_SH) | LOCK_NB)) - fprintf(stderr, "flock %s: %s", path, strerror(errno)); + if (flock(fileno(fp.get()), ((lock == FileLockType::Write) ? LOCK_EX : LOCK_SH) | LOCK_NB)) { + std::fprintf(stderr, "flock %s: %s", path, strerror(errno)); + } #endif } @@ -194,8 +204,8 @@ int main(int argc, const char* argv[]) tinyxml2::XMLDocument doc; std::vector assets; - FILE* docF = Fopen(inPath.c_str(), _SYS_STR("rb")); - if (doc.LoadFile(docF) == tinyxml2::XML_SUCCESS) { + FILEPtr docF = Fopen(inPath.c_str(), _SYS_STR("rb")); + if (doc.LoadFile(docF.get()) == tinyxml2::XML_SUCCESS) { const tinyxml2::XMLElement* elm = doc.RootElement(); if (strcmp(elm->Name(), "AssetNameMap") != 0) { Log.report(logvisor::Fatal, FMT_STRING(_SYS_STR("Invalid database supplied"))); @@ -241,7 +251,7 @@ int main(int argc, const char* argv[]) elm = elm->NextSiblingElement("Asset"); } - FILE* f = Fopen(outPath.c_str(), _SYS_STR("wb")); + FILEPtr f = Fopen(outPath.c_str(), _SYS_STR("wb")); if (f == nullptr) { Log.report(logvisor::Fatal, FMT_STRING(_SYS_STR("Unable to open destination"))); return 0; @@ -249,28 +259,23 @@ int main(int argc, const char* argv[]) uint32_t assetCount = SBig(uint32_t(assets.size())); FourCC sentinel(SBIG('AIDM')); - fwrite(&sentinel, 1, 4, f); - fwrite(&assetCount, 1, 4, f); + fwrite(&sentinel, 1, sizeof(sentinel), f.get()); + fwrite(&assetCount, 1, sizeof(assetCount), f.get()); for (const SAsset& asset : assets) { - fwrite(&asset.type, 1, 4, f); + fwrite(&asset.type, 1, sizeof(asset.type), f.get()); uint64_t id = SBig(asset.id); - fwrite(&id, 1, 8, f); + fwrite(&id, 1, sizeof(id), f.get()); uint32_t tmp = SBig(uint32_t(asset.name.length())); - fwrite(&tmp, 1, 4, f); - fwrite(asset.name.c_str(), 1, SBig(tmp), f); + fwrite(&tmp, 1, sizeof(tmp), f.get()); + fwrite(asset.name.c_str(), 1, SBig(tmp), f.get()); tmp = SBig(uint32_t(asset.dir.length())); - fwrite(&tmp, 1, 4, f); - fwrite(asset.dir.c_str(), 1, SBig(tmp), f); + fwrite(&tmp, 1, sizeof(tmp), f.get()); + fwrite(asset.dir.c_str(), 1, SBig(tmp), f.get()); } - fflush(f); - fclose(f); - fclose(docF); + fflush(f.get()); return 0; } - if (docF) - fclose(docF); - Log.report(logvisor::Fatal, FMT_STRING(_SYS_STR("failed to load"))); return 1; } From 972af7c5375c5a3f2fc28d23c348101dcc3eff10 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 31 May 2020 07:13:03 -0400 Subject: [PATCH 4/7] assetnameparser: Amend transposed fwrite arguments Existing code was using the size argument for the number of elements to write and vice versa. No behavior change, given this still results in the same number of bytes being copied. This just makes corrects their usages. --- assetnameparser/main.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/assetnameparser/main.cpp b/assetnameparser/main.cpp index a4a2c6465..0a4d8d3a8 100644 --- a/assetnameparser/main.cpp +++ b/assetnameparser/main.cpp @@ -259,18 +259,18 @@ int main(int argc, const char* argv[]) uint32_t assetCount = SBig(uint32_t(assets.size())); FourCC sentinel(SBIG('AIDM')); - fwrite(&sentinel, 1, sizeof(sentinel), f.get()); - fwrite(&assetCount, 1, sizeof(assetCount), f.get()); + fwrite(&sentinel, sizeof(sentinel), 1, f.get()); + fwrite(&assetCount, sizeof(assetCount), 1, f.get()); for (const SAsset& asset : assets) { - fwrite(&asset.type, 1, sizeof(asset.type), f.get()); + fwrite(&asset.type, sizeof(asset.type), 1, f.get()); uint64_t id = SBig(asset.id); - fwrite(&id, 1, sizeof(id), f.get()); + fwrite(&id, sizeof(id), 1, f.get()); uint32_t tmp = SBig(uint32_t(asset.name.length())); - fwrite(&tmp, 1, sizeof(tmp), f.get()); + fwrite(&tmp, sizeof(tmp), 1, f.get()); fwrite(asset.name.c_str(), 1, SBig(tmp), f.get()); tmp = SBig(uint32_t(asset.dir.length())); - fwrite(&tmp, 1, sizeof(tmp), f.get()); - fwrite(asset.dir.c_str(), 1, SBig(tmp), f.get()); + fwrite(&tmp, sizeof(tmp), 1, f.get()); + fwrite(asset.dir.c_str(), SBig(tmp), 1, f.get()); } fflush(f.get()); return 0; From ab01cb5f1a383b65de727262ca7f948b82d6bed9 Mon Sep 17 00:00:00 2001 From: Luke Street Date: Sun, 31 May 2020 20:27:34 -0400 Subject: [PATCH 5/7] CFrontEndUI: Remove audio group on destruction --- Runtime/MP1/CFrontEndUI.cpp | 6 ++++++ Runtime/MP1/CFrontEndUI.hpp | 1 + 2 files changed, 7 insertions(+) diff --git a/Runtime/MP1/CFrontEndUI.cpp b/Runtime/MP1/CFrontEndUI.cpp index 4e73a5c31..6525104a1 100644 --- a/Runtime/MP1/CFrontEndUI.cpp +++ b/Runtime/MP1/CFrontEndUI.cpp @@ -1844,6 +1844,12 @@ CFrontEndUI::CFrontEndUI() : CIOWin("FrontEndUI") { m_touchBar->SetPhase(CFrontEndUITouchBar::EPhase::None); } +CFrontEndUI::~CFrontEndUI() { + if (x14_phase >= EPhase::DisplayFrontEnd) { + CAudioSys::RemoveAudioGroup(x44_frontendAudioGrp->GetAudioGroupData()); + } +} + void CFrontEndUI::StartSlideShow(CArchitectureQueue& queue) { xf4_curAudio->StopMixing(); queue.Push(MakeMsg::CreateCreateIOWin(EArchMsgTarget::IOWinManager, 12, 11, std::make_shared())); diff --git a/Runtime/MP1/CFrontEndUI.hpp b/Runtime/MP1/CFrontEndUI.hpp index c44e7ecd8..e708390f1 100644 --- a/Runtime/MP1/CFrontEndUI.hpp +++ b/Runtime/MP1/CFrontEndUI.hpp @@ -371,6 +371,7 @@ private: public: CFrontEndUI(); + ~CFrontEndUI(); void StartSlideShow(CArchitectureQueue& queue); std::string GetAttractMovieFileName(int idx); std::string GetNextAttractMovieFileName(); From 8d2d5ef5d5a2d18a6c367c374a5c6c82692327e7 Mon Sep 17 00:00:00 2001 From: Luke Street Date: Sun, 31 May 2020 22:05:48 -0400 Subject: [PATCH 6/7] COmegaPirate: MSVC runtime fix for skeleton asset IDs --- Runtime/MP1/World/COmegaPirate.cpp | 5 +++-- Runtime/MP1/World/COmegaPirate.hpp | 2 +- Runtime/World/ScriptLoader.cpp | 7 +++++-- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/Runtime/MP1/World/COmegaPirate.cpp b/Runtime/MP1/World/COmegaPirate.cpp index 2aec16342..110d23da2 100644 --- a/Runtime/MP1/World/COmegaPirate.cpp +++ b/Runtime/MP1/World/COmegaPirate.cpp @@ -120,10 +120,11 @@ void COmegaPirate::CFlash::Render(CStateManager& mgr) { COmegaPirate::COmegaPirate(TUniqueId uid, std::string_view name, const CEntityInfo& info, const zeus::CTransform& xf, CModelData&& mData, const CPatternedInfo& pInfo, const CActorParameters& actParms, - CElitePirateData data, CAssetId w1, CAssetId w2, CAssetId w3) + CElitePirateData data, CAssetId skeletonModelId, CAssetId skeletonSkinRulesId, + CAssetId skeletonLayoutInfoId) : CElitePirate(uid, name, info, xf, std::move(mData), pInfo, actParms, data) , x9d0_initialScale(GetModelData()->GetScale()) -, x9f0_skeletonModel(*g_SimplePool, w1, w2, w3, 0, 0) +, x9f0_skeletonModel(*g_SimplePool, skeletonModelId, skeletonSkinRulesId, skeletonLayoutInfoId, 0, 0) , xb70_thermalSpot(g_SimplePool->GetObj("Thermal_Spot_2"sv)) { x9a4_scriptWaypointPlatforms.reserve(3); x9b8_scriptEffects.reserve(24); diff --git a/Runtime/MP1/World/COmegaPirate.hpp b/Runtime/MP1/World/COmegaPirate.hpp index 21bb56280..eec2ec6cc 100644 --- a/Runtime/MP1/World/COmegaPirate.hpp +++ b/Runtime/MP1/World/COmegaPirate.hpp @@ -122,7 +122,7 @@ private: public: COmegaPirate(TUniqueId uid, std::string_view name, const CEntityInfo& info, const zeus::CTransform& xf, CModelData&& mData, const CPatternedInfo& pInfo, const CActorParameters& actParms, CElitePirateData data, - CAssetId w1, CAssetId w2, CAssetId w3); + CAssetId skeletonModelId, CAssetId skeletonSkinRulesId, CAssetId skeletonLayoutInfoId); void Think(float dt, CStateManager& mgr) override; void AcceptScriptMsg(EScriptObjectMessage msg, TUniqueId uid, CStateManager& mgr) override; diff --git a/Runtime/World/ScriptLoader.cpp b/Runtime/World/ScriptLoader.cpp index 6da251123..0aed06b85 100644 --- a/Runtime/World/ScriptLoader.cpp +++ b/Runtime/World/ScriptLoader.cpp @@ -3659,11 +3659,14 @@ CEntity* ScriptLoader::LoadOmegaPirate(CStateManager& mgr, CInputStream& in, int return nullptr; } + const CAssetId skeletonModelId{in}; + const CAssetId skeletonSkinRulesId{in}; + const CAssetId skeletonLayoutInfoId{in}; CModelData mData(CAnimRes(pInfo.GetAnimationParameters().GetACSFile(), pInfo.GetAnimationParameters().GetCharacter(), actHead.x40_scale, pInfo.GetAnimationParameters().GetInitialAnimation(), true)); - return new MP1::COmegaPirate(mgr.AllocateUniqueId(), actHead.x0_name, info, actHead.x10_transform, std::move(mData), - pInfo, actParms, elitePirateData, CAssetId(in), CAssetId(in), CAssetId(in)); + pInfo, actParms, elitePirateData, skeletonModelId, skeletonSkinRulesId, + skeletonLayoutInfoId); } CEntity* ScriptLoader::LoadPhazonPool(CStateManager& mgr, CInputStream& in, int propCount, const CEntityInfo& info) { From 4587632252ad00f481940cf18292964d66568b5a Mon Sep 17 00:00:00 2001 From: Luke Street Date: Mon, 1 Jun 2020 02:51:31 -0400 Subject: [PATCH 7/7] Check for AppleClang as well --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ab79451ae..832ffd855 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -173,7 +173,7 @@ else() -Wno-unused-variable -Wno-unused-result -Wno-unused-function -Wno-sign-compare -Wno-unknown-pragmas -Werror) # doesn't work with generator expression in add_compile_options? - if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang") + if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang" OR "${CMAKE_CXX_COMPILER_ID}" STREQUAL "AppleClang") add_compile_options(-Wno-unknown-warning-option -Wno-unused-private-field) elseif("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU") add_compile_options(-Wno-lto-type-mismatch)