Make ingredient order affect effect order

CHANGELOG.md

@@ -103,6 +103,7 @@
     Bug #7660: Some inconsistencies regarding Invisibility breaking
     Bug #7665: Alchemy menu is missing the ability to deselect and choose different qualities of an apparatus
     Bug #7675: Successful lock spell doesn't produce a sound
+    Bug #7676: Incorrect magic effect order in alchemy
     Bug #7679: Scene luminance value flashes when toggling shaders
     Bug #7685: Corky sometimes doesn't follow Llovyn Andus
     Bug #7712: Casting doesn't support spells and enchantments with no effects

apps/openmw/mwgui/alchemywindow.cpp

@@ -433,7 +433,7 @@ namespace MWGui
 
         mItemView->update();
 
-        std::set<MWMechanics::EffectKey> effectIds = mAlchemy->listEffects();
+        std::vector<MWMechanics::EffectKey> effectIds = mAlchemy->listEffects();
         Widgets::SpellEffectList list;
         unsigned int effectIndex = 0;
         for (const MWMechanics::EffectKey& effectKey : effectIds)

apps/openmw/mwmechanics/alchemy.cpp

@@ -24,45 +24,64 @@
 #include "creaturestats.hpp"
 #include "magiceffects.hpp"
 
+namespace
+{
+    constexpr size_t sNumEffects = 4;
+
+    std::optional<MWMechanics::EffectKey> toKey(const ESM::Ingredient& ingredient, size_t i)
+    {
+        if (ingredient.mData.mEffectID[i] < 0)
+            return {};
+        ESM::RefId arg = ESM::Skill::indexToRefId(ingredient.mData.mSkills[i]);
+        if (arg.empty())
+            arg = ESM::Attribute::indexToRefId(ingredient.mData.mAttributes[i]);
+        return MWMechanics::EffectKey(ingredient.mData.mEffectID[i], arg);
+    }
+
+    bool containsEffect(const ESM::Ingredient& ingredient, const MWMechanics::EffectKey& effect)
+    {
+        for (size_t j = 0; j < sNumEffects; ++j)
+        {
+            if (toKey(ingredient, j) == effect)
+                return true;
+        }
+        return false;
+    }
+}
+
 MWMechanics::Alchemy::Alchemy()
     : mValue(0)
-    , mPotionName("")
 {
 }
 
-std::set<MWMechanics::EffectKey> MWMechanics::Alchemy::listEffects() const
+std::vector<MWMechanics::EffectKey> MWMechanics::Alchemy::listEffects() const
 {
-    std::map<EffectKey, int> effects;
+    // We care about the order of these effects as each effect can affect the next when applied.
-
+    // The player can affect effect order by placing ingredients into different slots
-    for (TIngredientsIterator iter(mIngredients.begin()); iter != mIngredients.end(); ++iter)
+    std::vector<EffectKey> effects;
+    for (size_t slotI = 0; slotI < mIngredients.size() - 1; ++slotI)
     {
-        if (!iter->isEmpty())
+        if (mIngredients[slotI].isEmpty())
+            continue;
+        const ESM::Ingredient* ingredient = mIngredients[slotI].get<ESM::Ingredient>()->mBase;
+        for (size_t slotJ = slotI + 1; slotJ < mIngredients.size(); ++slotJ)
         {
-            const MWWorld::LiveCellRef<ESM::Ingredient>* ingredient = iter->get<ESM::Ingredient>();
+            if (mIngredients[slotJ].isEmpty())
-
+                continue;
-            std::set<EffectKey> seenEffects;
+            const ESM::Ingredient* ingredient2 = mIngredients[slotJ].get<ESM::Ingredient>()->mBase;
-
+            for (size_t i = 0; i < sNumEffects; ++i)
-            for (int i = 0; i < 4; ++i)
+            {
-                if (ingredient->mBase->mData.mEffectID[i] != -1)
+                if (const auto key = toKey(*ingredient, i))
                 {
-                    ESM::RefId arg = ESM::Skill::indexToRefId(ingredient->mBase->mData.mSkills[i]);
+                    if (std::ranges::find(effects, *key) != effects.end())
-                    if (arg.empty())
+                        continue;
-                        arg = ESM::Attribute::indexToRefId(ingredient->mBase->mData.mAttributes[i]);
+                    if (containsEffect(*ingredient2, *key))
-                    EffectKey key(ingredient->mBase->mData.mEffectID[i], arg);
+                        effects.push_back(*key);
-
-                    if (seenEffects.insert(key).second)
-                        ++effects[key];
                 }
+            }
         }
     }
-
+    return effects;
-    std::set<EffectKey> effects2;
-
-    for (std::map<EffectKey, int>::const_iterator iter(effects.begin()); iter != effects.end(); ++iter)
-        if (iter->second > 1)
-            effects2.insert(iter->first);
-
-    return effects2;
 }
 
 void MWMechanics::Alchemy::applyTools(int flags, float& value) const
@@ -133,7 +152,7 @@ void MWMechanics::Alchemy::updateEffects()
         return;
 
     // find effects
-    std::set<EffectKey> effects(listEffects());
+    std::vector<EffectKey> effects = listEffects();
 
     // general alchemy factor
     float x = getAlchemyFactor();
@@ -150,14 +169,14 @@ void MWMechanics::Alchemy::updateEffects()
         x * MWBase::Environment::get().getESMStore()->get<ESM::GameSetting>().find("iAlchemyMod")->mValue.getFloat());
 
     // build quantified effect list
-    for (std::set<EffectKey>::const_iterator iter(effects.begin()); iter != effects.end(); ++iter)
+    for (const auto& effectKey : effects)
     {
         const ESM::MagicEffect* magicEffect
-            = MWBase::Environment::get().getESMStore()->get<ESM::MagicEffect>().find(iter->mId);
+            = MWBase::Environment::get().getESMStore()->get<ESM::MagicEffect>().find(effectKey.mId);
 
         if (magicEffect->mData.mBaseCost <= 0)
         {
-            const std::string os = "invalid base cost for magic effect " + std::to_string(iter->mId);
+            const std::string os = "invalid base cost for magic effect " + std::to_string(effectKey.mId);
             throw std::runtime_error(os);
         }
 
@@ -198,15 +217,15 @@ void MWMechanics::Alchemy::updateEffects()
         if (magnitude > 0 && duration > 0)
         {
             ESM::ENAMstruct effect;
-            effect.mEffectID = iter->mId;
+            effect.mEffectID = effectKey.mId;
 
             effect.mAttribute = -1;
             effect.mSkill = -1;
 
             if (magicEffect->mData.mFlags & ESM::MagicEffect::TargetSkill)
-                effect.mSkill = ESM::Skill::refIdToIndex(iter->mArg);
+                effect.mSkill = ESM::Skill::refIdToIndex(effectKey.mArg);
             else if (magicEffect->mData.mFlags & ESM::MagicEffect::TargetAttribute)
-                effect.mAttribute = ESM::Attribute::refIdToIndex(iter->mArg);
+                effect.mAttribute = ESM::Attribute::refIdToIndex(effectKey.mArg);
 
             effect.mRange = 0;
             effect.mArea = 0;
@@ -241,7 +260,7 @@ const ESM::Potion* MWMechanics::Alchemy::getRecord(const ESM::Potion& toFind) co
 
         bool mismatch = false;
 
-        for (int i = 0; i < static_cast<int>(iter->mEffects.mList.size()); ++i)
+        for (size_t i = 0; i < iter->mEffects.mList.size(); ++i)
         {
             const ESM::ENAMstruct& first = iter->mEffects.mList[i];
             const ESM::ENAMstruct& second = mEffects[i];
@@ -578,7 +597,7 @@ MWMechanics::Alchemy::Result MWMechanics::Alchemy::createSingle()
 
 std::string MWMechanics::Alchemy::suggestPotionName()
 {
-    std::set<MWMechanics::EffectKey> effects = listEffects();
+    std::vector<MWMechanics::EffectKey> effects = listEffects();
     if (effects.empty())
         return {};
 
@@ -595,11 +614,11 @@ std::vector<std::string> MWMechanics::Alchemy::effectsDescription(const MWWorld:
     const static auto fWortChanceValue = store->get<ESM::GameSetting>().find("fWortChanceValue")->mValue.getFloat();
     const auto& data = item->mData;
 
-    for (auto i = 0; i < 4; ++i)
+    for (size_t i = 0; i < sNumEffects; ++i)
     {
         const auto effectID = data.mEffectID[i];
 
-        if (alchemySkill < fWortChanceValue * (i + 1))
+        if (alchemySkill < fWortChanceValue * static_cast<int>(i + 1))
             break;
 
         if (effectID != -1)

apps/openmw/mwmechanics/alchemy.hpp

@@ -1,7 +1,6 @@
 #ifndef GAME_MWMECHANICS_ALCHEMY_H
 #define GAME_MWMECHANICS_ALCHEMY_H
 
-#include <set>
 #include <vector>
 
 #include <components/esm3/effectlist.hpp>
@@ -110,7 +109,7 @@ namespace MWMechanics
         void setPotionName(const std::string& name);
         ///< Set name of potion to create
 
-        std::set<EffectKey> listEffects() const;
+        std::vector<EffectKey> listEffects() const;
         ///< List all effects shared by at least two ingredients.
 
         int addIngredient(const MWWorld::Ptr& ingredient);

apps/openmw/mwmechanics/magiceffects.cpp

@@ -64,6 +64,11 @@ namespace MWMechanics
         return left.mArg < right.mArg;
     }
 
+    bool operator==(const EffectKey& left, const EffectKey& right)
+    {
+        return left.mId == right.mId && left.mArg == right.mArg;
+    }
+
     float EffectParam::getMagnitude() const
     {
         return mBase + mModifier;

apps/openmw/mwmechanics/magiceffects.hpp

@@ -38,6 +38,7 @@ namespace MWMechanics
     };
 
     bool operator<(const EffectKey& left, const EffectKey& right);
+    bool operator==(const EffectKey& left, const EffectKey& right);
 
     struct EffectParam
     {