Merge branch openmw:master into master

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.writeHNRefId("ID__", id);
-                esm.writeHNT("COUN", it->second);
+                esm.writeHNT("COUN", count);
             }
             esm.endRecord(ESM::REC_DCOU);
         }
 
     private:
-        std::map<std::string, int> mKillCounter;
+        std::map<ESM::RefId, int> mKillCounter;
     };
 
     class ConvertFACT : public Converter

apps/essimporter/importacdt.hpp

@@ -75,8 +75,8 @@ namespace ESSImport
         // to change them ingame
         int mCombatStats[3][2];
 
-        std::string mSelectedSpell;
+        ESM::RefId mSelectedSpell;
-        std::string mSelectedEnchantItem;
+        ESM::RefId mSelectedEnchantItem;
 
         SCRI mSCRI;
 

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

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");
     }
 

apps/essimporter/importinfo.hpp

@@ -3,6 +3,8 @@
 
 #include <string>
 
+#include <components/esm/refid.hpp>
+
 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);

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;

apps/essimporter/importklst.hpp

@@ -3,7 +3,8 @@
 
 #include <cstdint>
 #include <map>
-#include <string>
+
+#include <components/esm/refid.hpp>
 
 namespace ESM
 {
@@ -18,8 +19,7 @@ namespace ESSImport
     {
         void load(ESM::ESMReader& esm);
 
-        /// RefId, kill count
+        std::map<ESM::RefId, int32_t> mKillCounter;
-        std::map<std::string, int32_t> mKillCounter;
 
         int32_t mWerewolfKills;
     };

components/config/gamesettings.cpp

@@ -189,19 +189,16 @@ bool Config::GameSettings::readFile(
             if (ignoreContent && (key == QLatin1String("content") || key == QLatin1String("data")))
                 continue;
 
-            QList<SettingValue> values = cache.values(key);
+            auto containsValue = [&](const QMultiMap<QString, SettingValue>& map) {
-            values.append(settings.values(key));
+                for (auto [itr, end] = map.equal_range(key); itr != end; ++itr)
+                {
+                    if (itr->value == value.value)
+                        return true;
+                }
+                return false;
+            };
 
-            bool exists = false;
+            if (!containsValue(cache) && !containsValue(settings))
-            for (const auto& existingValue : values)
-            {
-                if (existingValue.value == value.value)
-                {
-                    exists = true;
-                    break;
-                }
-            }
-            if (!exists)
             {
                 cache.insert(key, value);
             }

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
+        // give monitor a chance to start dumping
-        waitMonitor();
+        // as we're suspended, this might time out even if it's successful, so mMonitorStatus is the source of truth
+        waitMonitorNoThrow();
 
+        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

components/crashcatcher/windows_crashcatcher.hpp

@@ -66,6 +66,7 @@ namespace Crash
             const std::filesystem::path& freezeDumpName);
 
         void waitMonitor();
+        bool waitMonitorNoThrow();
 
         void signalMonitor();
 

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<MINIDUMP_TYPE>(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");
         }
     }
 

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;

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.