From c59032fd6663968e7bf3dc41b948c37e3d6705e4 Mon Sep 17 00:00:00 2001
From: Evil Eye <malusluminis@hotmail.com>
Date: Thu, 13 Jul 2023 16:49:23 +0200
Subject: [PATCH] Make RefId::deserializeText return an empty RefId if no
 pre-existing StringRefId can be found

---
 apps/openmw/mwlua/types/actor.cpp        |  5 +++--
 apps/openmw_test_suite/esm/testrefid.cpp |  6 ++++++
 components/esm/refid.cpp                 |  4 +++-
 components/esm/stringrefid.cpp           | 18 ++++++++++++++++--
 components/esm/stringrefid.hpp           |  4 ++++
 5 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/apps/openmw/mwlua/types/actor.cpp b/apps/openmw/mwlua/types/actor.cpp
index 42d25789f6..641723afe9 100644
--- a/apps/openmw/mwlua/types/actor.cpp
+++ b/apps/openmw/mwlua/types/actor.cpp
@@ -41,13 +41,14 @@ namespace MWLua
         }
         else
         {
-            ESM::RefId recordId = ESM::RefId::deserializeText(std::get<std::string>(item));
+            const auto& stringId = std::get<std::string>(item);
+            ESM::RefId recordId = ESM::RefId::deserializeText(stringId);
             if (old_it != store.end() && old_it->getCellRef().getRefId() == recordId)
                 return { old_it, true }; // already equipped
             itemPtr = store.search(recordId);
             if (itemPtr.isEmpty() || itemPtr.getRefData().getCount() == 0)
             {
-                Log(Debug::Warning) << "There is no object with recordId='" << recordId << "' in inventory";
+                Log(Debug::Warning) << "There is no object with recordId='" << stringId << "' in inventory";
                 return { store.end(), false };
             }
         }
diff --git a/apps/openmw_test_suite/esm/testrefid.cpp b/apps/openmw_test_suite/esm/testrefid.cpp
index e947aab4a9..6db0ef3907 100644
--- a/apps/openmw_test_suite/esm/testrefid.cpp
+++ b/apps/openmw_test_suite/esm/testrefid.cpp
@@ -124,6 +124,12 @@ namespace ESM
             EXPECT_LT(a, b);
         }
 
+        TEST(ESMRefIdTest, stringRefIdDeserializationReturnsEmptyRefIdForNonExistentValues)
+        {
+            RefId id = RefId::deserializeText("this stringrefid should not exist");
+            EXPECT_TRUE(id.empty());
+        }
+
         TEST(ESMRefIdTest, lessThanIsDefinedForStringRefIdAndRefId)
         {
             const StringRefId stringRefId("a");
diff --git a/components/esm/refid.cpp b/components/esm/refid.cpp
index 3d489b9b97..8fdcc043f1 100644
--- a/components/esm/refid.cpp
+++ b/components/esm/refid.cpp
@@ -254,6 +254,8 @@ namespace ESM
             return ESM::ESM3ExteriorCellRefId(x, y);
         }
 
-        return ESM::RefId::stringRefId(value);
+        if (auto id = ESM::StringRefId::deserializeExisting(value))
+            return *id;
+        return {};
     }
 }
diff --git a/components/esm/stringrefid.cpp b/components/esm/stringrefid.cpp
index 2572161853..2657734c7a 100644
--- a/components/esm/stringrefid.cpp
+++ b/components/esm/stringrefid.cpp
@@ -21,13 +21,17 @@ namespace ESM
 
         const std::string emptyString;
 
-        Misc::NotNullPtr<const std::string> getOrInsertString(std::string_view id)
+        Misc::NotNullPtr<const std::string> getOrInsertString(std::string_view id, bool insert)
         {
             static Misc::ScopeGuarded<StringsSet> refIds;
             const auto locked = refIds.lock();
             auto it = locked->find(id);
             if (it == locked->end())
+            {
+                if (!insert)
+                    return &emptyString;
                 it = locked->emplace(id).first;
+            }
             return &*it;
         }
 
@@ -50,7 +54,7 @@ namespace ESM
     }
 
     StringRefId::StringRefId(std::string_view value)
-        : mValue(getOrInsertString(value))
+        : mValue(getOrInsertString(value, true))
     {
     }
 
@@ -129,4 +133,14 @@ namespace ESM
     {
         return Misc::StringUtils::ciFind(*mValue, subString) != std::string_view::npos;
     }
+
+    std::optional<StringRefId> StringRefId::deserializeExisting(std::string_view value)
+    {
+        auto string = getOrInsertString(value, false);
+        if (string->empty())
+            return {};
+        StringRefId id;
+        id.mValue = string;
+        return id;
+    }
 }
diff --git a/components/esm/stringrefid.hpp b/components/esm/stringrefid.hpp
index 6679763d55..ec8e898bc7 100644
--- a/components/esm/stringrefid.hpp
+++ b/components/esm/stringrefid.hpp
@@ -3,6 +3,7 @@
 
 #include <functional>
 #include <iosfwd>
+#include <optional>
 #include <string>
 #include <string_view>
 #include <variant>
@@ -46,6 +47,9 @@ namespace ESM
 
         friend struct std::hash<StringRefId>;
 
+        // Similar to the constructor but only returns preexisting ids
+        static std::optional<StringRefId> deserializeExisting(std::string_view value);
+
     private:
         Misc::NotNullPtr<const std::string> mValue;
     };