Merge branch 'deserializedeez' into 'master'

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

See merge request OpenMW/openmw!3226

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,10 +21,15 @@ namespace ESM
 
         const std::string emptyString;
 
-        Misc::NotNullPtr<const std::string> getOrInsertString(std::string_view id)
+        Misc::ScopeGuarded<StringsSet>& getRefIds()
         {
             static Misc::ScopeGuarded<StringsSet> refIds;
-            const auto locked = refIds.lock();
+            return refIds;
+        }
+
+        Misc::NotNullPtr<const std::string> getOrInsertString(std::string_view id)
+        {
+            const auto locked = getRefIds().lock();
             auto it = locked->find(id);
             if (it == locked->end())
                 it = locked->emplace(id).first;
@@ -129,4 +134,15 @@ namespace ESM
     {
         return Misc::StringUtils::ciFind(*mValue, subString) != std::string_view::npos;
     }
+
+    std::optional<StringRefId> StringRefId::deserializeExisting(std::string_view value)
+    {
+        const auto locked = getRefIds().lock();
+        auto it = locked->find(value);
+        if (it == locked->end())
+            return {};
+        StringRefId id;
+        id.mValue = &*it;
+        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;
     };