Change disposition to work like vanilla

CHANGELOG.md

@@ -4,10 +4,12 @@
     Bug #3737: Scripts from The Underground 2 .esp do not play (all patched versions)
     Bug #3846: Strings starting with "-" fail to compile if not enclosed in quotes
     Bug #3905: Great House Dagoth issues
+    Bug #5100: Persuasion doesn't always clamp the resulting disposition
     Bug #5120: Scripted object spawning updates physics system
     Bug #5379: Wandering NPCs falling through cantons
     Bug #5453: Magic effect VFX are offset for creatures
     Bug #5483: AutoCalc flag is not used to calculate spells cost
+    Bug #5842: GetDisposition adds temporary disposition change from different actors
     Bug #6037: Morrowind Content Language Cannot be Set to English in OpenMW Launcher
     Bug #6066: addtopic "return" does not work from within script. No errors thrown
     Bug #6067: esp loader fails in for certain subrecord orders

apps/openmw/mwbase/dialoguemanager.hpp

@@ -94,7 +94,6 @@ namespace MWBase
             virtual bool checkServiceRefused (ResponseCallback* callback, ServiceType service = ServiceType::Any) = 0;
 
             virtual void persuade (int type, ResponseCallback* callback) = 0;
-            virtual int getTemporaryDispositionChange () const = 0;
 
             /// @note Controlled by an option, gets discarded when dialogue ends by default
             virtual void applyBarterDispositionChange (int delta) = 0;

apps/openmw/mwbase/mechanicsmanager.hpp

@@ -100,7 +100,7 @@ namespace MWBase
             virtual int getBarterOffer(const MWWorld::Ptr& ptr,int basePrice, bool buying) = 0;
             ///< This is used by every service to determine the price of objects given the trading skills of the player and NPC.
 
-            virtual int getDerivedDisposition(const MWWorld::Ptr& ptr, bool addTemporaryDispositionChange = true) = 0;
+            virtual int getDerivedDisposition(const MWWorld::Ptr& ptr, bool clamp = true) = 0;
             ///< Calculate the diposition of an NPC toward the player.
 
             virtual int countDeaths (const std::string& id) const = 0;
@@ -156,7 +156,7 @@ namespace MWBase
                 PT_Bribe100,
                 PT_Bribe1000
             };
-            virtual void getPersuasionDispositionChange (const MWWorld::Ptr& npc, PersuasionType type, bool& success, float& tempChange, float& permChange) = 0;
+            virtual void getPersuasionDispositionChange (const MWWorld::Ptr& npc, PersuasionType type, bool& success, int& tempChange, int& permChange) = 0;
             ///< Perform a persuasion action on NPC
 
             virtual void forceStateUpdate(const MWWorld::Ptr &ptr) = 0;

apps/openmw/mwdialogue/dialoguemanagerimp.cpp

@@ -52,8 +52,9 @@ namespace MWDialogue
       , mCompilerContext (MWScript::CompilerContext::Type_Dialogue)
       , mErrorHandler()
       , mTalkedTo(false)
-      , mTemporaryDispositionChange(0.f)
-      , mPermanentDispositionChange(0.f)
+      , mOriginalDisposition(0)
+      , mCurrentDisposition(0)
+      , mPermanentDispositionChange(0)
     {
         mChoice = -1;
         mIsInChoice = false;
@@ -65,7 +66,8 @@ namespace MWDialogue
     {
         mKnownTopics.clear();
         mTalkedTo = false;
-        mTemporaryDispositionChange = 0;
+        mOriginalDisposition = 0;
+        mCurrentDisposition = 0;
         mPermanentDispositionChange = 0;
     }
 
@@ -98,6 +100,20 @@ namespace MWDialogue
         }
     }
 
+    void DialogueManager::updateOriginalDisposition()
+    {
+        if(mActor.getClass().isNpc())
+        {
+            const auto& stats = mActor.getClass().getNpcStats(mActor);
+            // Disposition changed by script; discard our preconceived notions
+            if(stats.getBaseDisposition() != mCurrentDisposition)
+            {
+                mCurrentDisposition = stats.getBaseDisposition();
+                mOriginalDisposition = mCurrentDisposition;
+            }
+        }
+    }
+
     bool DialogueManager::startDialogue (const MWWorld::Ptr& actor, ResponseCallback* callback)
     {
         updateGlobals();
@@ -107,8 +123,7 @@ namespace MWDialogue
             return false;
 
         mLastTopic = "";
-        mPermanentDispositionChange = 0;
-        mTemporaryDispositionChange = 0;
+        // Note that we intentionally don't reset mPermanentDispositionChange
 
         mChoice = -1;
         mIsInChoice = false;
@@ -398,19 +413,21 @@ namespace MWDialogue
 
     void DialogueManager::goodbyeSelected()
     {
-        // Apply disposition change to NPC's base disposition
-        if (mActor.getClass().isNpc())
+        // Apply disposition change to NPC's base disposition if we **think** we need to change something
+        if ((mPermanentDispositionChange || mOriginalDisposition != mCurrentDisposition) && mActor.getClass().isNpc())
         {
-            // Clamp permanent disposition change so that final disposition doesn't go below 0 (could happen with intimidate)       
-            float curDisp = static_cast<float>(MWBase::Environment::get().getMechanicsManager()->getDerivedDisposition(mActor, false));
-            if (curDisp + mPermanentDispositionChange < 0)
-                mPermanentDispositionChange = -curDisp;
-
+            updateOriginalDisposition();
             MWMechanics::NpcStats& npcStats = mActor.getClass().getNpcStats(mActor);
-            npcStats.setBaseDisposition(static_cast<int>(npcStats.getBaseDisposition() + mPermanentDispositionChange));
+            // Clamp permanent disposition change so that final disposition doesn't go below 0 (could happen with intimidate)
+            npcStats.setBaseDisposition(0);
+            int zero = MWBase::Environment::get().getMechanicsManager()->getDerivedDisposition(mActor, false);
+            int disposition = std::min(100 - zero, std::max(mOriginalDisposition + mPermanentDispositionChange, -zero));
+
+            npcStats.setBaseDisposition(disposition);
         }
         mPermanentDispositionChange = 0;
-        mTemporaryDispositionChange = 0;
+        mOriginalDisposition = 0;
+        mCurrentDisposition = 0;
     }
 
     void DialogueManager::questionAnswered (int answer, ResponseCallback* callback)
@@ -490,20 +507,17 @@ namespace MWDialogue
     void DialogueManager::persuade(int type, ResponseCallback* callback)
     {
         bool success;
-        float temp, perm;
+        int temp, perm;
         MWBase::Environment::get().getMechanicsManager()->getPersuasionDispositionChange(
                     mActor, MWBase::MechanicsManager::PersuasionType(type),
                     success, temp, perm);
-        mTemporaryDispositionChange += temp;
+        updateOriginalDisposition();
+        if(temp > 0 && perm > 0 && mOriginalDisposition + perm + mPermanentDispositionChange < 0)
+            perm = -(mOriginalDisposition + mPermanentDispositionChange);
+        mCurrentDisposition += temp;
+        mActor.getClass().getNpcStats(mActor).setBaseDisposition(mCurrentDisposition);
         mPermanentDispositionChange += perm;
 
-        // change temp disposition so that final disposition is between 0...100
-        float curDisp = static_cast<float>(MWBase::Environment::get().getMechanicsManager()->getDerivedDisposition(mActor, false));
-        if (curDisp + mTemporaryDispositionChange < 0)
-            mTemporaryDispositionChange = -curDisp;
-        else if (curDisp + mTemporaryDispositionChange > 100)
-            mTemporaryDispositionChange = 100 - curDisp;
-
         MWWorld::Ptr player = MWMechanics::getPlayer();
         player.getClass().skillUsageSucceeded(player, ESM::Skill::Speechcraft, success ? 0 : 1);
 
@@ -539,16 +553,16 @@ namespace MWDialogue
         executeTopic (text + (success ? " Success" : " Fail"), callback);
     }
 
-    int DialogueManager::getTemporaryDispositionChange() const
-    {
-        return static_cast<int>(mTemporaryDispositionChange);
-    }
-
     void DialogueManager::applyBarterDispositionChange(int delta)
     {
-        mTemporaryDispositionChange += delta;
-        if (Settings::Manager::getBool("barter disposition change is permanent", "Game"))
-            mPermanentDispositionChange += delta;
+        if(mActor.getClass().isNpc())
+        {
+            updateOriginalDisposition();
+            mCurrentDisposition += delta;
+            mActor.getClass().getNpcStats(mActor).setBaseDisposition(mCurrentDisposition);
+            if (Settings::Manager::getBool("barter disposition change is permanent", "Game"))
+                mPermanentDispositionChange += delta;
+        }
     }
 
     bool DialogueManager::checkServiceRefused(ResponseCallback* callback, ServiceType service)

apps/openmw/mwdialogue/dialoguemanagerimp.hpp

@@ -47,8 +47,9 @@ namespace MWDialogue
 
             std::vector<std::pair<std::string, int> > mChoices;
 
-            float mTemporaryDispositionChange;
-            float mPermanentDispositionChange;
+            int mOriginalDisposition;
+            int mCurrentDisposition;
+            int mPermanentDispositionChange;
 
             void parseText (const std::string& text);
 
@@ -62,6 +63,8 @@ namespace MWDialogue
 
             const ESM::Dialogue* searchDialogue(const std::string& id);
 
+            void updateOriginalDisposition();
+
         public:
 
             DialogueManager (const Compiler::Extensions& extensions, Translation::Storage& translationDataStorage);
@@ -96,7 +99,6 @@ namespace MWDialogue
             void questionAnswered (int answer, ResponseCallback* callback) override;
 
             void persuade (int type, ResponseCallback* callback) override;
-            int getTemporaryDispositionChange () const override;
 
             /// @note Controlled by an option, gets discarded when dialogue ends by default
             void applyBarterDispositionChange (int delta) override;

apps/openmw/mwmechanics/mechanicsmanagerimp.cpp

@@ -483,7 +483,7 @@ namespace MWMechanics
         mUpdatePlayer = true;
     }
 
-    int MechanicsManager::getDerivedDisposition(const MWWorld::Ptr& ptr, bool addTemporaryDispositionChange)
+    int MechanicsManager::getDerivedDisposition(const MWWorld::Ptr& ptr, bool clamp)
     {
         const MWMechanics::NpcStats& npcSkill = ptr.getClass().getNpcStats(ptr);
         float x = static_cast<float>(npcSkill.getBaseDisposition());
@@ -562,11 +562,9 @@ namespace MWMechanics
 
         x += ptr.getClass().getCreatureStats(ptr).getMagicEffects().get(ESM::MagicEffect::Charm).getMagnitude();
 
-        if(addTemporaryDispositionChange)
-          x += MWBase::Environment::get().getDialogueManager()->getTemporaryDispositionChange();
-
-        int effective_disposition = std::max(0,std::min(int(x),100));//, normally clamped to [0..100] when used
-        return effective_disposition;
+        if(clamp)
+            return std::max(0,std::min(int(x),100));//, normally clamped to [0..100] when used
+        return int(x);
     }
 
     int MechanicsManager::getBarterOffer(const MWWorld::Ptr& ptr,int basePrice, bool buying)
@@ -603,7 +601,7 @@ namespace MWMechanics
         return mActors.countDeaths (id);
     }
 
-    void MechanicsManager::getPersuasionDispositionChange (const MWWorld::Ptr& npc, PersuasionType type, bool& success, float& tempChange, float& permChange)
+    void MechanicsManager::getPersuasionDispositionChange (const MWWorld::Ptr& npc, PersuasionType type, bool& success, int& tempChange, int& permChange)
     {
         const MWWorld::Store<ESM::GameSetting> &gmst =
             MWBase::Environment::get().getWorld()->getStore().get<ESM::GameSetting>();
@@ -727,19 +725,22 @@ namespace MWMechanics
             x = success ? std::max(iPerMinChange, c) : c;
         }
 
-        tempChange = type == PT_Intimidate ? x : int(x * fPerTempMult);
+        tempChange = type == PT_Intimidate ? int(x) : int(x * fPerTempMult);
 
 
-        float cappedDispositionChange = tempChange;
-        if (currentDisposition + tempChange > 100.f)
-            cappedDispositionChange = static_cast<float>(100 - currentDisposition);
-        if (currentDisposition + tempChange < 0.f)
-            cappedDispositionChange = static_cast<float>(-currentDisposition);
+        int cappedDispositionChange = tempChange;
+        if (currentDisposition + tempChange > 100)
+            cappedDispositionChange = 100 - currentDisposition;
+        if (currentDisposition + tempChange < 0)
+        {
+            cappedDispositionChange = -currentDisposition;
+            tempChange = 0;
+        }
 
         permChange = floor(cappedDispositionChange / fPerTempMult);
         if (type == PT_Intimidate)
         {
-            permChange = success ? -int(cappedDispositionChange/ fPerTempMult) : y;
+            permChange = success ? -int(cappedDispositionChange/ fPerTempMult) : int(y);
         }
     }
 
@@ -1722,7 +1723,7 @@ namespace MWMechanics
 
         int disposition = 50;
         if (ptr.getClass().isNpc())
-            disposition = getDerivedDisposition(ptr, true);
+            disposition = getDerivedDisposition(ptr);
 
         int fight = ptr.getClass().getCreatureStats(ptr).getAiSetting(CreatureStats::AI_Fight).getModified()
                 + static_cast<int>(getFightDistanceBias(ptr, target) + getFightDispositionBias(static_cast<float>(disposition)));

apps/openmw/mwmechanics/mechanicsmanagerimp.hpp

@@ -87,13 +87,13 @@ namespace MWMechanics
             int getBarterOffer(const MWWorld::Ptr& ptr,int basePrice, bool buying) override;
             ///< This is used by every service to determine the price of objects given the trading skills of the player and NPC.
 
-            int getDerivedDisposition(const MWWorld::Ptr& ptr, bool addTemporaryDispositionChange = true) override;
+            int getDerivedDisposition(const MWWorld::Ptr& ptr, bool clamp = true) override;
             ///< Calculate the diposition of an NPC toward the player.
 
             int countDeaths (const std::string& id) const override;
             ///< Return the number of deaths for actors with the given ID.
 
-            void getPersuasionDispositionChange (const MWWorld::Ptr& npc, PersuasionType type, bool& success, float& tempChange, float& permChange) override;
+            void getPersuasionDispositionChange (const MWWorld::Ptr& npc, PersuasionType type, bool& success, int& tempChange, int& permChange) override;
             ///< Perform a persuasion action on NPC
 
             /// Check if \a observer is potentially aware of \a ptr. Does not do a line of sight check!