Make enchanting make more sense (bug #5038)

CHANGELOG.md

@@ -89,6 +89,7 @@
     Bug #5004: Werewolves shield their eyes during storm
     Bug #5018: Spell tooltips don't support purely negative magnitudes
     Bug #5028: Offered price caps are not trading-specific
+    Bug #5038: Enchanting success chance calculations are blatantly wrong
     Feature #1774: Handle AvoidNode
     Feature #2229: Improve pathfinding AI
     Feature #3025: Analogue gamepad movement controls

apps/openmw/mwgui/enchantingdialog.cpp

@@ -107,20 +107,11 @@ namespace MWGui
 
     void EnchantingDialog::updateLabels()
     {
-        std::stringstream enchantCost;
+        mEnchantmentPoints->setCaption(std::to_string(static_cast<int>(mEnchanting.getEnchantPoints(false))) + " / " + std::to_string(mEnchanting.getMaxEnchantValue()));
-        enchantCost << std::setprecision(1) << std::fixed << mEnchanting.getEnchantPoints();
+        mCharge->setCaption(std::to_string(mEnchanting.getGemCharge()));
-        mEnchantmentPoints->setCaption(enchantCost.str() + " / " + MyGUI::utility::toString(mEnchanting.getMaxEnchantValue()));
+        mSuccessChance->setCaption(std::to_string(std::max(0, std::min(100, mEnchanting.getEnchantChance()))));
-
+        mCastCost->setCaption(std::to_string(mEnchanting.getEffectiveCastCost()));
-        mCharge->setCaption(MyGUI::utility::toString(mEnchanting.getGemCharge()));
+        mPrice->setCaption(std::to_string(mEnchanting.getEnchantPrice()));
-
-        int successChance = int(mEnchanting.getEnchantChance());
-        mSuccessChance->setCaption(MyGUI::utility::toString(std::max(0, successChance)));
-
-        std::stringstream castCost;
-        castCost << mEnchanting.getEffectiveCastCost();
-        mCastCost->setCaption(castCost.str());
-
-        mPrice->setCaption(MyGUI::utility::toString(mEnchanting.getEnchantPrice()));
 
         switch(mEnchanting.getCastStyle())
         {
@@ -322,7 +313,7 @@ namespace MWGui
             return;
         }
 
-        if (mEnchanting.getEnchantPoints() > mEnchanting.getMaxEnchantValue())
+        if (static_cast<int>(mEnchanting.getEnchantPoints(false)) > mEnchanting.getMaxEnchantValue())
         {
             MWBase::Environment::get().getWindowManager()->messageBox ("#{sNotifyMessage29}");
             return;

apps/openmw/mwmechanics/enchanting.cpp

@@ -164,46 +164,36 @@ namespace MWMechanics
      *
      *  Formula on UESPWiki is not entirely correct.
      */
-    int Enchanting::getEnchantPoints() const
+    float Enchanting::getEnchantPoints(bool precise) const
     {
         if (mEffectList.mList.empty())
             // No effects added, cost = 0
             return 0;
 
         const MWWorld::ESMStore &store = MWBase::Environment::get().getWorld()->getStore();
-        std::vector<ESM::ENAMstruct> mEffects = mEffectList.mList;
+        const float fEffectCostMult = store.get<ESM::GameSetting>().find("fEffectCostMult")->mValue.getFloat();
+        const float fEnchantmentConstantDurationMult = store.get<ESM::GameSetting>().find("fEnchantmentConstantDurationMult")->mValue.getFloat();
 
-        int enchantmentCost = 0;
+        float enchantmentCost = 0.f;
-        float cost = 0;
+        float cost = 0.f;
-        for (std::vector<ESM::ENAMstruct>::const_iterator it = mEffects.begin(); it != mEffects.end(); ++it)
+        for (const ESM::ENAMstruct& effect : mEffectList.mList)
         {
-            float baseCost = (store.get<ESM::MagicEffect>().find(it->mEffectID))->mData.mBaseCost;
+            float baseCost = (store.get<ESM::MagicEffect>().find(effect.mEffectID))->mData.mBaseCost;
-            int magMin = std::max(1, it->mMagnMin);
+            int magMin = std::max(1, effect.mMagnMin);
-            int magMax = std::max(1, it->mMagnMax);
+            int magMax = std::max(1, effect.mMagnMax);
-            int area = std::max(1, it->mArea);
+            int area = std::max(1, effect.mArea);
-
+            float duration = static_cast<float>(effect.mDuration);
-            float magnitudeCost = (magMin + magMax) * baseCost * 0.05f;
             if (mCastStyle == ESM::Enchantment::ConstantEffect)
-            {
+                duration = fEnchantmentConstantDurationMult;
-                magnitudeCost *= store.get<ESM::GameSetting>().find("fEnchantmentConstantDurationMult")->mValue.getFloat();
-            }
-            else
-            {
-                magnitudeCost *= it->mDuration;
-            }
 
-            float areaCost = area * 0.05f * baseCost;
+            cost += ((magMin + magMax) * duration + area) * baseCost * fEffectCostMult * 0.05f;
-
-            const float fEffectCostMult = store.get<ESM::GameSetting>().find("fEffectCostMult")->mValue.getFloat();
-
-            cost += (magnitudeCost + areaCost) * fEffectCostMult;
 
             cost = std::max(1.f, cost);
 
-            if (it->mRange == ESM::RT_Target)
+            if (effect.mRange == ESM::RT_Target)
-                cost *= 1.5;
+                cost *= 1.5f;
 
-            enchantmentCost += static_cast<int>(cost);
+            enchantmentCost += precise ? cost : std::floor(cost);
         }
 
         return enchantmentCost;
@@ -215,7 +205,7 @@ namespace MWMechanics
         if (mCastStyle == ESM::Enchantment::ConstantEffect)
             return 0;
 
-        return getEnchantPoints();
+        return static_cast<int>(getEnchantPoints(false));
     }
 
     int Enchanting::getEffectiveCastCost() const
@@ -279,21 +269,21 @@ namespace MWMechanics
         mEnchanter = enchanter;
     }
 
-    float Enchanting::getEnchantChance() const
+    int Enchanting::getEnchantChance() const
     {
         const CreatureStats& stats = mEnchanter.getClass().getCreatureStats(mEnchanter);
-
-        float chance1 = (mEnchanter.getClass().getSkill(mEnchanter, ESM::Skill::Enchant) + 
-        (0.25f * stats.getAttribute (ESM::Attribute::Intelligence).getModified())
-        + (0.125f * stats.getAttribute (ESM::Attribute::Luck).getModified()));
-
         const MWWorld::Store<ESM::GameSetting>& gmst = MWBase::Environment::get().getWorld()->getStore().get<ESM::GameSetting>();
+        const float a = static_cast<float>(mEnchanter.getClass().getSkill(mEnchanter, ESM::Skill::Enchant));
+        const float b = static_cast<float>(stats.getAttribute (ESM::Attribute::Intelligence).getModified());
+        const float c = static_cast<float>(stats.getAttribute (ESM::Attribute::Luck).getModified());
+        const float fEnchantmentChanceMult = gmst.find("fEnchantmentChanceMult")->mValue.getFloat();
+        const float fEnchantmentConstantChanceMult = gmst.find("fEnchantmentConstantChanceMult")->mValue.getFloat();
 
-        float chance2 = 7.5f / (gmst.find("fEnchantmentChanceMult")->mValue.getFloat() * ((mCastStyle == ESM::Enchantment::ConstantEffect) ?
+        float x = (a - getEnchantPoints()*fEnchantmentChanceMult + 0.2f * b + 0.1f * c) * stats.getFatigueTerm();
-                                                                          gmst.find("fEnchantmentConstantChanceMult")->mValue.getFloat() : 1.0f ))
+        if (mCastStyle == ESM::Enchantment::ConstantEffect)
-                * getEnchantPoints();
+            x *= fEnchantmentConstantChanceMult;
 
-        return (chance1-chance2);
+        return static_cast<int>(x);
     }
 
     void Enchanting::payForEnchantment() const

apps/openmw/mwmechanics/enchanting.hpp

@@ -37,13 +37,13 @@ namespace MWMechanics
             bool create(); //Return true if created, false if failed.
             void nextCastStyle(); //Set enchant type to next possible type (for mOldItemPtr object)
             int getCastStyle() const;
-            int getEnchantPoints() const;
+            float getEnchantPoints(bool precise = true) const;
             int getBaseCastCost() const; // To be saved in the enchantment's record
             int getEffectiveCastCost() const; // Effective cost taking player Enchant skill into account, used for preview purposes in the UI
             int getEnchantPrice() const;
             int getMaxEnchantValue() const;
             int getGemCharge() const;
-            float getEnchantChance() const;
+            int getEnchantChance() const;
             bool soulEmpty() const; //Return true if empty
             bool itemEmpty() const; //Return true if empty
             void payForEnchantment() const;