Merge branch 'itsakindofskillorattributeaffectingmagic' into 'master'

Unify string construction of skill/attribute affecting effect names

Closes #7307

See merge request OpenMW/openmw!2894

CHANGELOG.md

@@ -51,6 +51,7 @@
     Bug #7229: Error marker loading failure is not handled
     Bug #7243: Get Skyrim.esm loading
     Bug #7298: Water ripples from projectiles sometimes are not spawned
+    Bug #7307: Alchemy "Magic Effect" search string does not match on tool tip for effects related to attributes
     Feature #3537: Shader-based water ripples
     Feature #5492: Let rain and snow collide with statics
     Feature #6447: Add LOD support to Object Paging

apps/openmw/mwgui/spellmodel.cpp

@@ -45,7 +45,6 @@ namespace MWGui
 
     bool SpellModel::matchingEffectExists(std::string filter, const ESM::EffectList& effects)
     {
-        auto wm = MWBase::Environment::get().getWindowManager();
         const MWWorld::ESMStore& store = MWBase::Environment::get().getWorld()->getStore();
 
         for (const auto& effect : effects.mList)
@@ -55,22 +54,9 @@ namespace MWGui
             if (effectId != -1)
             {
                 const ESM::MagicEffect* magicEffect = store.get<ESM::MagicEffect>().find(effectId);
-                const std::string& effectIDStr = ESM::MagicEffect::effectIdToString(effectId);
-                std::string fullEffectName{ wm->getGameSettingString(effectIDStr, {}) };
-
-                if (magicEffect->mData.mFlags & ESM::MagicEffect::TargetSkill && effect.mSkill != -1)
-                {
-                    fullEffectName += ' ';
-                    fullEffectName += wm->getGameSettingString(ESM::Skill::sSkillNameIds[effect.mSkill], {});
-                }
-
-                if (magicEffect->mData.mFlags & ESM::MagicEffect::TargetAttribute && effect.mAttribute != -1)
-                {
-                    fullEffectName += ' ';
-                    fullEffectName
-                        += wm->getGameSettingString(ESM::Attribute::sGmstAttributeIds[effect.mAttribute], {});
-                }
 
+                std::string fullEffectName
+                    = MWMechanics::getMagicEffectString(*magicEffect, effect.mAttribute, effect.mSkill);
                 std::string convert = Utf8Stream::lowerCaseUtf8(fullEffectName);
                 if (convert.find(filter) != std::string::npos)
                 {

apps/openmw/mwgui/widgets.cpp

@@ -17,6 +17,8 @@
 #include "../mwbase/windowmanager.hpp"
 #include "../mwbase/world.hpp"
 
+#include "../mwmechanics/magiceffects.hpp"
+
 #include "../mwworld/esmstore.hpp"
 
 #include "ustring.hpp"
@@ -391,57 +393,8 @@ namespace MWGui::Widgets
         std::string sec = " " + std::string{ windowManager->getGameSettingString("ssecond", {}) };
         std::string secs = " " + std::string{ windowManager->getGameSettingString("sseconds", {}) };
 
-        const bool targetsSkill
+        std::string spellLine
-            = magicEffect->mData.mFlags & ESM::MagicEffect::TargetSkill && mEffectParams.mSkill != -1;
+            = MWMechanics::getMagicEffectString(*magicEffect, mEffectParams.mAttribute, mEffectParams.mSkill);
-        const bool targetsAttribute
-            = magicEffect->mData.mFlags & ESM::MagicEffect::TargetAttribute && mEffectParams.mAttribute != -1;
-
-        std::string spellLine;
-
-        if (targetsSkill || targetsAttribute)
-        {
-            switch (magicEffect->mIndex)
-            {
-                case ESM::MagicEffect::AbsorbAttribute:
-                case ESM::MagicEffect::AbsorbSkill:
-                    spellLine = windowManager->getGameSettingString("sAbsorb", {});
-                    break;
-                case ESM::MagicEffect::DamageAttribute:
-                case ESM::MagicEffect::DamageSkill:
-                    spellLine = windowManager->getGameSettingString("sDamage", {});
-                    break;
-                case ESM::MagicEffect::DrainAttribute:
-                case ESM::MagicEffect::DrainSkill:
-                    spellLine = windowManager->getGameSettingString("sDrain", {});
-                    break;
-                case ESM::MagicEffect::FortifyAttribute:
-                case ESM::MagicEffect::FortifySkill:
-                    spellLine = windowManager->getGameSettingString("sFortify", {});
-                    break;
-                case ESM::MagicEffect::RestoreAttribute:
-                case ESM::MagicEffect::RestoreSkill:
-                    spellLine = windowManager->getGameSettingString("sRestore", {});
-                    break;
-            }
-        }
-
-        if (spellLine.empty())
-        {
-            const std::string& effectIDStr = ESM::MagicEffect::effectIdToString(mEffectParams.mEffectID);
-            spellLine = windowManager->getGameSettingString(effectIDStr, {});
-        }
-
-        if (targetsSkill)
-        {
-            spellLine += ' ';
-            spellLine += windowManager->getGameSettingString(ESM::Skill::sSkillNameIds[mEffectParams.mSkill], {});
-        }
-        if (targetsAttribute)
-        {
-            spellLine += ' ';
-            spellLine
-                += windowManager->getGameSettingString(ESM::Attribute::sGmstAttributeIds[mEffectParams.mAttribute], {});
-        }
 
         if (mEffectParams.mMagnMin || mEffectParams.mMagnMax)
         {

apps/openmw/mwmechanics/alchemy.cpp

@@ -563,15 +563,9 @@ std::string MWMechanics::Alchemy::suggestPotionName()
 {
     std::set<MWMechanics::EffectKey> effects = listEffects();
     if (effects.empty())
-        return "";
+        return {};
 
-    int effectId = effects.begin()->mId;
+    return effects.begin()->toString();
-    return MWBase::Environment::get()
-        .getWorld()
-        ->getStore()
-        .get<ESM::GameSetting>()
-        .find(ESM::MagicEffect::effectIdToString(effectId))
-        ->mValue.getString();
 }
 
 std::vector<std::string> MWMechanics::Alchemy::effectsDescription(const MWWorld::ConstPtr& ptr, const int alchemySkill)
@@ -580,6 +574,7 @@ std::vector<std::string> MWMechanics::Alchemy::effectsDescription(const MWWorld:
 
     const auto& item = ptr.get<ESM::Ingredient>()->mBase;
     const auto& gmst = MWBase::Environment::get().getWorld()->getStore().get<ESM::GameSetting>();
+    const auto& mgef = MWBase::Environment::get().getWorld()->getStore().get<ESM::MagicEffect>();
     const static auto fWortChanceValue = gmst.find("fWortChanceValue")->mValue.getFloat();
     const auto& data = item->mData;
 
@@ -594,12 +589,7 @@ std::vector<std::string> MWMechanics::Alchemy::effectsDescription(const MWWorld:
 
         if (effectID != -1)
         {
-            std::string effect = gmst.find(ESM::MagicEffect::effectIdToString(effectID))->mValue.getString();
+            std::string effect = getMagicEffectString(*mgef.find(effectID), attributeID, skillID);
-
-            if (skillID != -1)
-                effect += " " + gmst.find(ESM::Skill::sSkillNameIds[skillID])->mValue.getString();
-            else if (attributeID != -1)
-                effect += " " + gmst.find(ESM::Attribute::sGmstAttributeIds[attributeID])->mValue.getString();
 
             effects.push_back(effect);
         }

apps/openmw/mwmechanics/magiceffects.cpp

@@ -3,9 +3,18 @@
 #include <cmath>
 #include <stdexcept>
 
+#include <components/esm/attr.hpp>
 #include <components/esm3/effectlist.hpp>
+#include <components/esm3/loadmgef.hpp>
+#include <components/esm3/loadskil.hpp>
 #include <components/esm3/magiceffects.hpp>
 
+#include "../mwbase/environment.hpp"
+#include "../mwbase/windowmanager.hpp"
+#include "../mwbase/world.hpp"
+
+#include "../mwworld/esmstore.hpp"
+
 namespace
 {
     // Round value to prevent precision issues
@@ -40,6 +49,13 @@ namespace MWMechanics
         }
     }
 
+    std::string EffectKey::toString() const
+    {
+        const ESM::MagicEffect* magicEffect
+            = MWBase::Environment::get().getWorld()->getStore().get<ESM::MagicEffect>().search(mId);
+        return getMagicEffectString(*magicEffect, mArg, mArg);
+    }
+
     bool operator<(const EffectKey& left, const EffectKey& right)
     {
         if (left.mId < right.mId)
@@ -208,4 +224,59 @@ namespace MWMechanics
             mCollection[EffectKey(key)].setModifier(params.second);
         }
     }
+
+    std::string getMagicEffectString(const ESM::MagicEffect& effect, int attributeArg, int skillArg)
+    {
+        const bool targetsSkill = effect.mData.mFlags & ESM::MagicEffect::TargetSkill && skillArg != -1;
+        const bool targetsAttribute = effect.mData.mFlags & ESM::MagicEffect::TargetAttribute && attributeArg != -1;
+
+        std::string spellLine;
+
+        auto windowManager = MWBase::Environment::get().getWindowManager();
+
+        if (targetsSkill || targetsAttribute)
+        {
+            switch (effect.mIndex)
+            {
+                case ESM::MagicEffect::AbsorbAttribute:
+                case ESM::MagicEffect::AbsorbSkill:
+                    spellLine = windowManager->getGameSettingString("sAbsorb", {});
+                    break;
+                case ESM::MagicEffect::DamageAttribute:
+                case ESM::MagicEffect::DamageSkill:
+                    spellLine = windowManager->getGameSettingString("sDamage", {});
+                    break;
+                case ESM::MagicEffect::DrainAttribute:
+                case ESM::MagicEffect::DrainSkill:
+                    spellLine = windowManager->getGameSettingString("sDrain", {});
+                    break;
+                case ESM::MagicEffect::FortifyAttribute:
+                case ESM::MagicEffect::FortifySkill:
+                    spellLine = windowManager->getGameSettingString("sFortify", {});
+                    break;
+                case ESM::MagicEffect::RestoreAttribute:
+                case ESM::MagicEffect::RestoreSkill:
+                    spellLine = windowManager->getGameSettingString("sRestore", {});
+                    break;
+            }
+        }
+
+        if (spellLine.empty())
+        {
+            const std::string& effectIDStr = ESM::MagicEffect::effectIdToString(effect.mIndex);
+            spellLine = windowManager->getGameSettingString(effectIDStr, {});
+        }
+
+        if (targetsSkill)
+        {
+            spellLine += ' ';
+            spellLine += windowManager->getGameSettingString(ESM::Skill::sSkillNameIds[skillArg], {});
+        }
+        else if (targetsAttribute)
+        {
+            spellLine += ' ';
+            spellLine += windowManager->getGameSettingString(ESM::Attribute::sGmstAttributeIds[attributeArg], {});
+        }
+        return spellLine;
+    }
 }

apps/openmw/mwmechanics/magiceffects.hpp

@@ -8,7 +8,7 @@ namespace ESM
 {
     struct ENAMstruct;
     struct EffectList;
-
+    struct MagicEffect;
     struct MagicEffects;
 }
 
@@ -28,6 +28,8 @@ namespace MWMechanics
         }
 
         EffectKey(const ESM::ENAMstruct& effect);
+
+        std::string toString() const;
     };
 
     bool operator<(const EffectKey& left, const EffectKey& right);
@@ -108,6 +110,8 @@ namespace MWMechanics
         static MagicEffects diff(const MagicEffects& prev, const MagicEffects& now);
         ///< Return changes from \a prev to \a now.
     };
+
+    std::string getMagicEffectString(const ESM::MagicEffect& effect, int attributeArg, int skillArg);
 }
 
 #endif