Merge branch 'avoid_get_ref_id_string_for_spell' into 'master'

Do not use ESM::RefId::getRefIdString for spell id (#7291)

Closes #7291

See merge request OpenMW/openmw!2856

apps/openmw/mwgui/hud.cpp

@@ -409,7 +409,7 @@ namespace MWGui
         mSpellStatus->setProgressPosition(successChancePercent);
 
         mSpellBox->setUserString("ToolTipType", "Spell");
-        mSpellBox->setUserString("Spell", spellId.getRefIdString());
+        mSpellBox->setUserString("Spell", spellId.serialize());
 
         // use the icon of the first effect
         const ESM::MagicEffect* effect = MWBase::Environment::get().getWorld()->getStore().get<ESM::MagicEffect>().find(

apps/openmw/mwgui/quickkeysmenu.cpp

@@ -296,7 +296,7 @@ namespace MWGui
 
         mSelected->button->setItem(MWWorld::Ptr());
         mSelected->button->setUserString("ToolTipType", "Spell");
-        mSelected->button->setUserString("Spell", spellId.getRefIdString());
+        mSelected->button->setUserString("Spell", spellId.serialize());
 
         // use the icon of the first effect
         const ESM::MagicEffect* effect = esmStore.get<ESM::MagicEffect>().find(spell->mEffects.mList.front().mEffectID);
@@ -517,7 +517,7 @@ namespace MWGui
                     break;
                 }
                 case ESM::QuickKeys::Type::Magic:
-                    key.mId = ESM::RefId::stringRefId(button->getUserString("Spell"));
+                    key.mId = ESM::RefId::deserialize(button->getUserString("Spell"));
                     break;
             }
 

apps/openmw/mwgui/race.cpp

@@ -452,13 +452,13 @@ namespace MWGui
         const ESM::Race* race = store.get<ESM::Race>().find(mCurrentRaceId);
 
         int i = 0;
-        for (const auto& spellpower : race->mPowers.mList)
+        for (const ESM::RefId& spellpower : race->mPowers.mList)
         {
             Widgets::MWSpellPtr spellPowerWidget = mSpellPowerList->createWidget<Widgets::MWSpell>(
                 "MW_StatName", coord, MyGUI::Align::Default, std::string("SpellPower") + MyGUI::utility::toString(i));
             spellPowerWidget->setSpellId(spellpower);
             spellPowerWidget->setUserString("ToolTipType", "Spell");
-            spellPowerWidget->setUserString("Spell", spellpower.getRefIdString());
+            spellPowerWidget->setUserString("Spell", spellpower.serialize());
 
             mSpellPowerItems.push_back(spellPowerWidget);
 

apps/openmw/mwgui/review.cpp

@@ -315,7 +315,7 @@ namespace MWGui
             "MW_StatName", coord1 + MyGUI::IntSize(coord2.width, 0), MyGUI::Align::Default);
         widget->setSpellId(spell->mId);
         widget->setUserString("ToolTipType", "Spell");
-        widget->setUserString("Spell", spell->mId.getRefIdString());
+        widget->setUserString("Spell", spell->mId.serialize());
         widget->eventMouseWheel += MyGUI::newDelegate(this, &ReviewDialog::onMouseWheel);
 
         mSkillWidgets.push_back(widget);

apps/openmw/mwgui/spellbuyingwindow.cpp

@@ -66,7 +66,7 @@ namespace MWGui
         toAdd->setSize(mSpellsView->getWidth(), lineHeight);
         toAdd->eventMouseWheel += MyGUI::newDelegate(this, &SpellBuyingWindow::onMouseWheel);
         toAdd->setUserString("ToolTipType", "Spell");
-        toAdd->setUserString("Spell", spell.mId.getRefIdString());
+        toAdd->setUserString("Spell", spell.mId.serialize());
         toAdd->setUserString("SpellCost", std::to_string(spell.mData.mCost));
         toAdd->eventMouseButtonClick += MyGUI::newDelegate(this, &SpellBuyingWindow::onSpellButtonClick);
         mSpellsWidgetMap.insert(std::make_pair(toAdd, spell.mId));

apps/openmw/mwgui/spellview.cpp

@@ -286,7 +286,7 @@ namespace MWGui
         else
         {
             widget->setUserString("ToolTipType", "Spell");
-            widget->setUserString("Spell", spell.mId.getRefIdString());
+            widget->setUserString("Spell", spell.mId.serialize());
         }
 
         widget->setUserString(sSpellModelIndex, MyGUI::utility::toString(index));

apps/openmw/mwgui/tooltips.cpp

@@ -227,7 +227,7 @@ namespace MWGui
                     ToolTipInfo info;
 
                     const ESM::Spell* spell = MWBase::Environment::get().getWorld()->getStore().get<ESM::Spell>().find(
-                        ESM::RefId::stringRefId(focus->getUserString("Spell")));
+                        ESM::RefId::deserialize(focus->getUserString("Spell")));
                     info.caption = spell->mName;
                     Widgets::SpellEffectList effects;
                     for (const ESM::ENAMstruct& spellEffect : spell->mEffects.mList)

apps/openmw_test_suite/esm/testrefid.cpp

@@ -179,14 +179,14 @@ namespace ESM
 
         TEST(ESMRefIdTest, canBeUsedAsMapKeyWithLookupByStringView)
         {
-            const std::map<ESM::RefId, int, std::less<>> map({ { ESM::RefId::stringRefId("a"), 42 } });
+            const std::map<RefId, int, std::less<>> map({ { RefId::stringRefId("a"), 42 } });
             EXPECT_EQ(map.count("A"), 1);
         }
 
         TEST(ESMRefIdTest, canBeUsedAsLookupKeyForMapWithStringKey)
         {
             const std::map<std::string, int, std::less<>> map({ { "a", 42 } });
-            EXPECT_EQ(map.count(ESM::RefId::stringRefId("A")), 1);
+            EXPECT_EQ(map.count(RefId::stringRefId("A")), 1);
         }
 
         TEST(ESMRefIdTest, stringRefIdIsNotEqualToFormId)
@@ -196,7 +196,7 @@ namespace ESM
             EXPECT_NE(stringRefId, formIdRefId);
         }
 
-        struct ESMRefIdToStringTest : TestWithParam<std::pair<ESM::RefId, std::string>>
+        struct ESMRefIdToStringTest : TestWithParam<std::pair<RefId, std::string>>
         {
         };
 
@@ -213,12 +213,12 @@ namespace ESM
             { RefId::stringRefId(std::string({ 'a', 0, -1, '\n', '\t' })), { 'a', 0, -1, '\n', '\t' } },
             { RefId::formIdRefId(42), "42" },
             { RefId::generated(42), "42" },
-            { RefId::index(ESM::REC_ARMO, 42), "ARMO, 42" },
+            { RefId::index(REC_ARMO, 42), "ARMO, 42" },
         };
 
         INSTANTIATE_TEST_SUITE_P(ESMRefIdToString, ESMRefIdToStringTest, ValuesIn(toStringParams));
 
-        struct ESMRefIdToDebugStringTest : TestWithParam<std::pair<ESM::RefId, std::string>>
+        struct ESMRefIdToDebugStringTest : TestWithParam<std::pair<RefId, std::string>>
         {
         };
 
@@ -244,9 +244,61 @@ namespace ESM
             { RefId::stringRefId(std::string({ 'a', 0, -1, '\n', '\t' })), "String{a\\x0\\xFF\\xA\\x9}" },
             { RefId::formIdRefId(42), "FormId{42}" },
             { RefId::generated(42), "Generated{42}" },
-            { RefId::index(ESM::REC_ARMO, 42), "Index{ARMO, 42}" },
+            { RefId::index(REC_ARMO, 42), "Index{ARMO, 42}" },
         };
 
         INSTANTIATE_TEST_SUITE_P(ESMRefIdToDebugString, ESMRefIdToDebugStringTest, ValuesIn(toDebugStringParams));
+
+        template <class T>
+        RefId generateRefId();
+
+        template <>
+        RefId generateRefId<EmptyRefId>()
+        {
+            return RefId();
+        }
+
+        template <>
+        RefId generateRefId<StringRefId>()
+        {
+            return RefId::stringRefId("StringRefId");
+        }
+
+        template <>
+        RefId generateRefId<FormIdRefId>()
+        {
+            return RefId::formIdRefId(42);
+        }
+
+        template <>
+        RefId generateRefId<GeneratedRefId>()
+        {
+            return RefId::generated(13);
+        }
+
+        template <>
+        RefId generateRefId<IndexRefId>()
+        {
+            return RefId::index(REC_BOOK, 7);
+        }
+
+        template <class T>
+        struct ESMRefIdSerializeDeserializeTest : Test
+        {
+        };
+
+        TYPED_TEST_SUITE_P(ESMRefIdSerializeDeserializeTest);
+
+        TYPED_TEST_P(ESMRefIdSerializeDeserializeTest, serializeThenDeserializeShouldProduceSameValue)
+        {
+            const RefId refId = generateRefId<TypeParam>();
+            EXPECT_EQ(RefId::deserialize(refId.serialize()), refId);
+        }
+
+        REGISTER_TYPED_TEST_SUITE_P(ESMRefIdSerializeDeserializeTest, serializeThenDeserializeShouldProduceSameValue);
+
+        using RefIdTypeParams = Types<EmptyRefId, StringRefId, FormIdRefId, GeneratedRefId, IndexRefId>;
+
+        INSTANTIATE_TYPED_TEST_SUITE_P(RefIdTypes, ESMRefIdSerializeDeserializeTest, RefIdTypeParams);
     }
 }

components/esm/refid.cpp

@@ -177,4 +177,20 @@ namespace ESM
     {
         return std::visit(Contains{ subString }, mValue);
     }
+
+    std::string RefId::serialize() const
+    {
+        std::string result(sizeof(mValue), '\0');
+        std::memcpy(result.data(), &mValue, sizeof(mValue));
+        return result;
+    }
+
+    ESM::RefId RefId::deserialize(std::string_view value)
+    {
+        if (value.size() != sizeof(ESM::RefId::mValue))
+            throw std::runtime_error("Invalid value size to deserialize: " + std::to_string(value.size()));
+        ESM::RefId result;
+        std::memcpy(&result.mValue, value.data(), sizeof(result.mValue));
+        return result;
+    }
 }

components/esm/refid.hpp

@@ -48,6 +48,9 @@ namespace ESM
     public:
         const static RefId sEmpty;
 
+        // Constructs RefId from a string containing byte by byte copy of RefId::mValue.
+        static ESM::RefId deserialize(std::string_view value);
+
         // Constructs RefId from a string using a pointer to a static set of strings.
         static RefId stringRefId(std::string_view value);
 
@@ -112,6 +115,9 @@ namespace ESM
         // Otherwise returns false.
         bool contains(std::string_view subString) const;
 
+        // Copy mValue byte by byte into a string. Use result only during within the same process.
+        std::string serialize() const;
+
         friend constexpr bool operator==(const RefId& l, const RefId& r) { return l.mValue == r.mValue; }
 
         bool operator==(std::string_view rhs) const;