Make RefId::deserializeText return an empty RefId if no pre-existing StringRefId can be found

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

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

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 {};
     }
 }

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;
+    }
 }

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