diff --git a/apps/essimporter/converter.hpp b/apps/essimporter/converter.hpp index 93b4e2c810..095e36c7ee 100644 --- a/apps/essimporter/converter.hpp +++ b/apps/essimporter/converter.hpp @@ -252,7 +252,7 @@ namespace ESSImport for (size_t i = 0; i < invState.mItems.size(); ++i) { // FIXME: in case of conflict (multiple items with this refID) use the already equipped one? - if (invState.mItems[i].mRef.mRefID == ESM::RefId::stringRefId(refr.mActorData.mSelectedEnchantItem)) + if (invState.mItems[i].mRef.mRefID == refr.mActorData.mSelectedEnchantItem) invState.mSelectedEnchantItem = i; } } @@ -260,12 +260,12 @@ namespace ESSImport void write(ESM::ESMWriter& esm) override { esm.startRecord(ESM::REC_ASPL); - esm.writeHNString("ID__", mSelectedSpell); + esm.writeHNRefId("ID__", mSelectedSpell); esm.endRecord(ESM::REC_ASPL); } private: - std::string mSelectedSpell; + ESM::RefId mSelectedSpell; }; class ConvertPCDT : public Converter @@ -374,16 +374,16 @@ namespace ESSImport void write(ESM::ESMWriter& esm) override { esm.startRecord(ESM::REC_DCOU); - for (auto it = mKillCounter.begin(); it != mKillCounter.end(); ++it) + for (const auto& [id, count] : mKillCounter) { - esm.writeHNString("ID__", it->first); - esm.writeHNT("COUN", it->second); + esm.writeHNRefId("ID__", id); + esm.writeHNT("COUN", count); } esm.endRecord(ESM::REC_DCOU); } private: - std::map mKillCounter; + std::map mKillCounter; }; class ConvertFACT : public Converter diff --git a/apps/essimporter/importacdt.hpp b/apps/essimporter/importacdt.hpp index 65519c6a6c..e961725e75 100644 --- a/apps/essimporter/importacdt.hpp +++ b/apps/essimporter/importacdt.hpp @@ -75,8 +75,8 @@ namespace ESSImport // to change them ingame int mCombatStats[3][2]; - std::string mSelectedSpell; - std::string mSelectedEnchantItem; + ESM::RefId mSelectedSpell; + ESM::RefId mSelectedEnchantItem; SCRI mSCRI; diff --git a/apps/essimporter/importcellref.cpp b/apps/essimporter/importcellref.cpp index 9e8e9a6948..c49c876e37 100644 --- a/apps/essimporter/importcellref.cpp +++ b/apps/essimporter/importcellref.cpp @@ -107,12 +107,12 @@ namespace ESSImport if (esm.isNextSub("WNAM")) { - std::string id = esm.getHString(); + ESM::RefId spellRefId = esm.getRefId(); if (esm.isNextSub("XNAM")) - mActorData.mSelectedEnchantItem = esm.getHString(); + mActorData.mSelectedEnchantItem = esm.getRefId(); else - mActorData.mSelectedSpell = std::move(id); + mActorData.mSelectedSpell = std::move(spellRefId); if (esm.isNextSub("YNAM")) esm.skipHSub(); // 4 byte, 0 diff --git a/apps/essimporter/importinfo.cpp b/apps/essimporter/importinfo.cpp index 66902f6ff4..d902ff1823 100644 --- a/apps/essimporter/importinfo.cpp +++ b/apps/essimporter/importinfo.cpp @@ -7,7 +7,19 @@ namespace ESSImport void INFO::load(ESM::ESMReader& esm) { - mInfo = esm.getHNString("INAM"); + if (esm.peekNextSub("XNAM")) + { + // TODO: Support older saves by turning XNAM into a RefId. + // XNAM is probably the number of the topic response within the topic record's linked list. + // Resolving this value will likely require loading Morrowind.esm. + + esm.getSubName(); + esm.skipHSub(); + mInfo = ESM::RefId(); + } + else + mInfo = esm.getHNRefId("INAM"); + mActorRefId = esm.getHNString("ACDT"); } diff --git a/apps/essimporter/importinfo.hpp b/apps/essimporter/importinfo.hpp index f4d616786f..e0d1de4e92 100644 --- a/apps/essimporter/importinfo.hpp +++ b/apps/essimporter/importinfo.hpp @@ -3,6 +3,8 @@ #include +#include + namespace ESM { class ESMReader; @@ -13,7 +15,7 @@ namespace ESSImport struct INFO { - std::string mInfo; + ESM::RefId mInfo; std::string mActorRefId; void load(ESM::ESMReader& esm); diff --git a/apps/essimporter/importklst.cpp b/apps/essimporter/importklst.cpp index 2d5e09e913..12721f81bd 100644 --- a/apps/essimporter/importklst.cpp +++ b/apps/essimporter/importklst.cpp @@ -9,7 +9,7 @@ namespace ESSImport { while (esm.isNextSub("KNAM")) { - std::string refId = esm.getHString(); + ESM::RefId refId = esm.getRefId(); int32_t count; esm.getHNT(count, "CNAM"); mKillCounter[refId] = count; diff --git a/apps/essimporter/importklst.hpp b/apps/essimporter/importklst.hpp index 9cdb2d701b..57c271c95a 100644 --- a/apps/essimporter/importklst.hpp +++ b/apps/essimporter/importklst.hpp @@ -3,7 +3,8 @@ #include #include -#include + +#include namespace ESM { @@ -18,8 +19,7 @@ namespace ESSImport { void load(ESM::ESMReader& esm); - /// RefId, kill count - std::map mKillCounter; + std::map mKillCounter; int32_t mWerewolfKills; }; diff --git a/components/config/gamesettings.cpp b/components/config/gamesettings.cpp index b70fe2db4b..7f022bb653 100644 --- a/components/config/gamesettings.cpp +++ b/components/config/gamesettings.cpp @@ -189,19 +189,16 @@ bool Config::GameSettings::readFile( if (ignoreContent && (key == QLatin1String("content") || key == QLatin1String("data"))) continue; - QList values = cache.values(key); - values.append(settings.values(key)); - - bool exists = false; - for (const auto& existingValue : values) - { - if (existingValue.value == value.value) + auto containsValue = [&](const QMultiMap& map) { + for (auto [itr, end] = map.equal_range(key); itr != end; ++itr) { - exists = true; - break; + if (itr->value == value.value) + return true; } - } - if (!exists) + return false; + }; + + if (!containsValue(cache) && !containsValue(settings)) { cache.insert(key, value); } diff --git a/components/crashcatcher/windows_crashcatcher.cpp b/components/crashcatcher/windows_crashcatcher.cpp index d9ef00ef77..bd5e94a6aa 100644 --- a/components/crashcatcher/windows_crashcatcher.cpp +++ b/components/crashcatcher/windows_crashcatcher.cpp @@ -129,6 +129,8 @@ namespace Crash if (mShm == nullptr) throw std::runtime_error("Failed to map crash catcher shared memory"); + mShm->mMonitorStatus = CrashSHM::Status::Uninitialised; + mShmMutex = CreateMutexW(&attributes, FALSE, NULL); if (mShmMutex == nullptr) throw std::runtime_error("Failed to create crash catcher shared memory mutex"); @@ -147,10 +149,15 @@ namespace Crash void CrashCatcher::waitMonitor() { - if (WaitForSingleObject(mSignalAppEvent, CrashCatcherTimeout) != WAIT_OBJECT_0) + if (!waitMonitorNoThrow()) throw std::runtime_error("Waiting for monitor failed"); } + bool CrashCatcher::waitMonitorNoThrow() + { + return WaitForSingleObject(mSignalAppEvent, CrashCatcherTimeout) == WAIT_OBJECT_0; + } + void CrashCatcher::signalMonitor() { SetEvent(mSignalMonitorEvent); @@ -234,13 +241,23 @@ namespace Crash signalMonitor(); - // must remain until monitor has finished - waitMonitor(); + // give monitor a chance to start dumping + // as we're suspended, this might time out even if it's successful, so mMonitorStatus is the source of truth + waitMonitorNoThrow(); - std::string message = "OpenMW has encountered a fatal error.\nCrash dump saved to '" - + Misc::StringUtils::u8StringToString(getCrashDumpPath(*mShm).u8string()) - + "'.\nPlease report this to https://gitlab.com/OpenMW/openmw/issues !"; - SDL_ShowSimpleMessageBox(0, "Fatal Error", message.c_str(), nullptr); + shmLock(); + CrashSHM::Status monitorStatus = mShm->mMonitorStatus; + shmUnlock(); + + if (monitorStatus == CrashSHM::Status::DumpedSuccessfully) + { + std::string message = "OpenMW has encountered a fatal error.\nCrash dump saved to '" + + Misc::StringUtils::u8StringToString(getCrashDumpPath(*mShm).u8string()) + + "'.\nPlease report this to https://gitlab.com/OpenMW/openmw/issues !"; + SDL_ShowSimpleMessageBox(0, "Fatal Error", message.c_str(), nullptr); + } + else if (monitorStatus == CrashSHM::Status::Dumping) + SDL_ShowSimpleMessageBox(0, "Fatal Error", "Timed out while creating crash dump", nullptr); } } // namespace Crash diff --git a/components/crashcatcher/windows_crashcatcher.hpp b/components/crashcatcher/windows_crashcatcher.hpp index bcf1ed688d..5bc78b19fa 100644 --- a/components/crashcatcher/windows_crashcatcher.hpp +++ b/components/crashcatcher/windows_crashcatcher.hpp @@ -66,6 +66,7 @@ namespace Crash const std::filesystem::path& freezeDumpName); void waitMonitor(); + bool waitMonitorNoThrow(); void signalMonitor(); diff --git a/components/crashcatcher/windows_crashmonitor.cpp b/components/crashcatcher/windows_crashmonitor.cpp index 8398528ec9..116a678fef 100644 --- a/components/crashcatcher/windows_crashmonitor.cpp +++ b/components/crashcatcher/windows_crashmonitor.cpp @@ -163,6 +163,7 @@ namespace Crash void CrashMonitor::run() { + mShm->mMonitorStatus = CrashSHM::Status::Monitoring; try { // app waits for monitor start up, let it continue @@ -231,6 +232,18 @@ namespace Crash void CrashMonitor::handleCrash(bool isFreeze) { + shmLock(); + mShm->mMonitorStatus = CrashSHM::Status::Dumping; + shmUnlock(); + + auto failedDumping = [this](const std::string& message) { + Log(Debug::Error) << "CrashMonitor: " << message; + shmLock(); + mShm->mMonitorStatus = CrashSHM::Status::FailedDumping; + shmUnlock(); + SDL_ShowSimpleMessageBox(0, "Failed to create crash dump", message.c_str(), nullptr); + }; + DWORD processId = GetProcessId(mAppProcessHandle); const char* env = getenv("OPENMW_FULL_MEMDUMP"); @@ -238,7 +251,10 @@ namespace Crash { HMODULE dbghelp = LoadLibraryA("dbghelp.dll"); if (dbghelp == NULL) + { + failedDumping("Could not load dbghelp.dll"); return; + } using MiniDumpWirteDumpFn = BOOL(WINAPI*)(HANDLE hProcess, DWORD ProcessId, HANDLE hFile, MINIDUMP_TYPE DumpType, PMINIDUMP_EXCEPTION_INFORMATION ExceptionParam, @@ -246,11 +262,17 @@ namespace Crash MiniDumpWirteDumpFn miniDumpWriteDump = (MiniDumpWirteDumpFn)GetProcAddress(dbghelp, "MiniDumpWriteDump"); if (miniDumpWriteDump == NULL) + { + failedDumping("Could not get MiniDumpWriteDump address"); return; + } std::wstring utf16Path = (isFreeze ? getFreezeDumpPath(*mShm) : getCrashDumpPath(*mShm)).native(); if (utf16Path.empty()) + { + failedDumping("Empty dump path"); return; + } if (utf16Path.length() > MAX_PATH) utf16Path = LR"(\\?\)" + utf16Path; @@ -258,9 +280,17 @@ namespace Crash HANDLE hCrashLog = CreateFileW(utf16Path.c_str(), GENERIC_READ | GENERIC_WRITE, 0, nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr); if (hCrashLog == NULL || hCrashLog == INVALID_HANDLE_VALUE) + { + failedDumping("Bad dump file handle"); return; + } if (auto err = GetLastError(); err != ERROR_ALREADY_EXISTS && err != 0) + { + std::stringstream ss; + ss << "Error opening dump file: " << std::hex << err; + failedDumping(ss.str()); return; + } EXCEPTION_POINTERS exp; exp.ContextRecord = &mShm->mCrashed.mContext; @@ -274,15 +304,26 @@ namespace Crash if (env) type = static_cast(type | MiniDumpWithFullMemory); - miniDumpWriteDump(mAppProcessHandle, processId, hCrashLog, type, &infos, 0, 0); + if (!miniDumpWriteDump(mAppProcessHandle, processId, hCrashLog, type, &infos, 0, 0)) + { + auto err = GetLastError(); + std::stringstream ss; + ss << "Error while writing dump: " << std::hex << err; + failedDumping(ss.str()); + return; + } + + shmLock(); + mShm->mMonitorStatus = CrashSHM::Status::DumpedSuccessfully; + shmUnlock(); } catch (const std::exception& e) { - Log(Debug::Error) << "CrashMonitor: " << e.what(); + failedDumping(e.what()); } catch (...) { - Log(Debug::Error) << "CrashMonitor: unknown exception"; + failedDumping("Unknown exception"); } } diff --git a/components/crashcatcher/windows_crashshm.hpp b/components/crashcatcher/windows_crashshm.hpp index b919757890..2cad54b17a 100644 --- a/components/crashcatcher/windows_crashshm.hpp +++ b/components/crashcatcher/windows_crashshm.hpp @@ -22,6 +22,17 @@ namespace Crash Event mEvent; + enum class Status + { + Uninitialised, + Monitoring, + Dumping, + DumpedSuccessfully, + FailedDumping + }; + + Status mMonitorStatus; + struct Startup { HANDLE mAppProcessHandle; diff --git a/docs/source/reference/modding/paths.rst b/docs/source/reference/modding/paths.rst index 26c3ccbb65..4ea5219e74 100644 --- a/docs/source/reference/modding/paths.rst +++ b/docs/source/reference/modding/paths.rst @@ -75,6 +75,7 @@ Configuration sources Configuration for OpenMW is composed from several sources. From lowest to highest priority, these are: + * The local or global ``openmw.cfg``. * Any ``openmw.cfg`` files in any other directories specified with the ``config`` option. * Options specified on the command line.