Merge branch 'refactor/aisequence-2' into 'master'

#6091: Optimize isInCombat

See merge request OpenMW/openmw!1636

apps/openmw/mwlua/localscripts.cpp

@@ -113,15 +113,12 @@ namespace MWLua
         {
             const MWWorld::Ptr& ptr = self.ptr();
             MWMechanics::AiSequence& ai = ptr.getClass().getCreatureStats(ptr).getAiSequence();
-            std::list<std::shared_ptr<AiPackage>>& list = ai.getUnderlyingList();
-            for (auto it = list.begin(); it != list.end();)
+
+            ai.erasePackagesIf([&](auto& entry)
             {
-                bool keep = LuaUtil::call(callback, *it).get<bool>();
-                if (keep)
-                    ++it;
-                else
-                    it = list.erase(it);
-            }
+                bool keep = LuaUtil::call(callback, entry).template get<bool>();
+                return !keep;
+            });
         };
         selfAPI["_startAiCombat"] = [](SelfObject& self, const LObject& target)
         {

apps/openmw/mwmechanics/actors.cpp

@@ -73,23 +73,20 @@ bool isCommanded(const MWWorld::Ptr& actor)
 // Check for command effects having ended and remove package if necessary
 void adjustCommandedActor (const MWWorld::Ptr& actor)
 {
-    MWMechanics::CreatureStats& stats = actor.getClass().getCreatureStats(actor);
+    if (!isCommanded(actor))
+        return;
 
-    bool hasCommandPackage = false;
+    MWMechanics::CreatureStats& stats = actor.getClass().getCreatureStats(actor);
 
-    auto it = stats.getAiSequence().begin();
-    for (; it != stats.getAiSequence().end(); ++it)
+    stats.getAiSequence().erasePackageIf([](auto& entry)
     {
-        if ((*it)->getTypeId() == MWMechanics::AiPackageTypeId::Follow &&
-                static_cast<const MWMechanics::AiFollow*>(it->get())->isCommanded())
+        if (entry->getTypeId() == MWMechanics::AiPackageTypeId::Follow &&
+            static_cast<const MWMechanics::AiFollow*>(entry.get())->isCommanded())
         {
-            hasCommandPackage = true;
-            break;
+            return true;
         }
-    }
-
-    if (!isCommanded(actor) && hasCommandPackage)
-        stats.getAiSequence().erase(it);
+        return false;
+    });
 }
 
 void getRestorationPerHourOfSleep (const MWWorld::Ptr& ptr, float& health, float& magicka)

apps/openmw/mwmechanics/aisequence.cpp

@@ -1,6 +1,7 @@
 #include "aisequence.hpp"
 
 #include <limits>
+#include <algorithm>
 
 #include <components/debug/debuglog.hpp>
 #include <components/esm3/aisequence.hpp>
@@ -29,6 +30,9 @@ void AiSequence::copy (const AiSequence& sequence)
     // We need to keep an AiWander storage, if present - it has a state machine.
     // Not sure about another temporary storages
     sequence.mAiState.copy<AiWanderStorage>(mAiState);
+
+    mNumCombatPackages = sequence.mNumCombatPackages;
+    mNumPursuitPackages = sequence.mNumPursuitPackages;
 }
 
 AiSequence::AiSequence() : mDone (false), mLastAiPackage(AiPackageTypeId::None) {}
@@ -58,6 +62,28 @@ AiSequence::~AiSequence()
     clear();
 }
 
+void AiSequence::onPackageAdded(const AiPackage& package)
+{
+    if (package.getTypeId() == AiPackageTypeId::Combat)
+        mNumCombatPackages++;
+    else if (package.getTypeId() == AiPackageTypeId::Pursue)
+        mNumPursuitPackages++;
+
+    assert(mNumCombatPackages >= 0);
+    assert(mNumPursuitPackages >= 0);
+}
+
+void AiSequence::onPackageRemoved(const AiPackage& package)
+{
+    if (package.getTypeId() == AiPackageTypeId::Combat)
+        mNumCombatPackages--;
+    else if (package.getTypeId() == AiPackageTypeId::Pursue)
+        mNumPursuitPackages--;
+
+    assert(mNumCombatPackages >= 0);
+    assert(mNumPursuitPackages >= 0);
+}
+
 AiPackageTypeId AiSequence::getTypeId() const
 {
     if (mPackages.empty())
@@ -87,32 +113,30 @@ bool AiSequence::getCombatTargets(std::vector<MWWorld::Ptr> &targetActors) const
     return !targetActors.empty();
 }
 
-void AiSequence::erase(std::list<std::shared_ptr<AiPackage>>::const_iterator package)
+AiPackages::iterator AiSequence::erase(AiPackages::iterator package)
 {
     // Not sure if manually terminated packages should trigger mDone, probably not?
-    for(auto it = mPackages.begin(); it != mPackages.end(); ++it)
-    {
-        if (package == it)
-        {
-            mPackages.erase(it);
-            return;
-        }
-    }
-    throw std::runtime_error("can't find package to erase");
+    auto& ptr = *package;
+    onPackageRemoved(*ptr);
+
+    return mPackages.erase(package);
 }
 
 bool AiSequence::isInCombat() const
 {
-    for (auto it = mPackages.begin(); it != mPackages.end(); ++it)
-    {
-        if ((*it)->getTypeId() == AiPackageTypeId::Combat)
-            return true;
-    }
-    return false;
+    return mNumCombatPackages > 0;
+}
+
+bool AiSequence::isInPursuit() const
+{
+    return mNumPursuitPackages > 0;
 }
 
 bool AiSequence::isEngagedWithActor() const
 {
+    if (!isInCombat())
+        return false;
+
     for (auto it = mPackages.begin(); it != mPackages.end(); ++it)
     {
         if ((*it)->getTypeId() == AiPackageTypeId::Combat)
@@ -127,16 +151,18 @@ bool AiSequence::isEngagedWithActor() const
 
 bool AiSequence::hasPackage(AiPackageTypeId typeId) const
 {
-    for (auto it = mPackages.begin(); it != mPackages.end(); ++it)
+    auto it = std::find_if(mPackages.begin(), mPackages.end(), [typeId](const auto& package)
     {
-        if ((*it)->getTypeId() == typeId)
-            return true;
-    }
-    return false;
+        return package->getTypeId() == typeId;
+    });
+    return it != mPackages.end();
 }
 
 bool AiSequence::isInCombat(const MWWorld::Ptr &actor) const
 {
+    if (!isInCombat())
+        return false;
+
     for (auto it = mPackages.begin(); it != mPackages.end(); ++it)
     {
         if ((*it)->getTypeId() == AiPackageTypeId::Combat)
@@ -148,27 +174,31 @@ bool AiSequence::isInCombat(const MWWorld::Ptr &actor) const
     return false;
 }
 
-// TODO: use std::list::remove_if for all these methods when we switch to C++20
-void AiSequence::stopCombat()
+void AiSequence::removePackagesById(AiPackageTypeId id)
 {
-    for(auto it = mPackages.begin(); it != mPackages.end(); )
+    for (auto it = mPackages.begin(); it != mPackages.end(); )
     {
-        if ((*it)->getTypeId() == AiPackageTypeId::Combat)
+        if ((*it)->getTypeId() == id)
         {
-            it = mPackages.erase(it);
+            it = erase(it);
         }
         else
             ++it;
     }
 }
 
+void AiSequence::stopCombat()
+{
+    removePackagesById(AiPackageTypeId::Combat);
+}
+
 void AiSequence::stopCombat(const std::vector<MWWorld::Ptr>& targets)
 {
     for(auto it = mPackages.begin(); it != mPackages.end(); )
     {
         if ((*it)->getTypeId() == AiPackageTypeId::Combat && std::find(targets.begin(), targets.end(), (*it)->getTarget()) != targets.end())
         {
-            it = mPackages.erase(it);
+            it = erase(it);
         }
         else
             ++it;
@@ -177,15 +207,7 @@ void AiSequence::stopCombat(const std::vector<MWWorld::Ptr>& targets)
 
 void AiSequence::stopPursuit()
 {
-    for(auto it = mPackages.begin(); it != mPackages.end(); )
-    {
-        if ((*it)->getTypeId() == AiPackageTypeId::Pursue)
-        {
-            it = mPackages.erase(it);
-        }
-        else
-            ++it;
-    }
+    removePackagesById(AiPackageTypeId::Pursue);
 }
 
 bool AiSequence::isPackageDone() const
@@ -204,112 +226,117 @@ namespace
 
 void AiSequence::execute (const MWWorld::Ptr& actor, CharacterController& characterController, float duration, bool outOfRange)
 {
-    if(actor != getPlayer())
+    if (actor == getPlayer())
     {
-        if (mPackages.empty())
-        {
-            mLastAiPackage = AiPackageTypeId::None;
-            return;
-        }
+        // Players don't use this.
+        return;
+    }
 
-        auto packageIt = mPackages.begin();
-        MWMechanics::AiPackage* package = packageIt->get();
-        if (!package->alwaysActive() && outOfRange)
-            return;
+    if (mPackages.empty())
+    {
+        mLastAiPackage = AiPackageTypeId::None;
+        return;
+    }
 
-        auto packageTypeId = package->getTypeId();
-        // workaround ai packages not being handled as in the vanilla engine
-        if (isActualAiPackage(packageTypeId))
-            mLastAiPackage = packageTypeId;
-        // if active package is combat one, choose nearest target
-        if (packageTypeId == AiPackageTypeId::Combat)
-        {
-            auto itActualCombat = mPackages.end();
+    auto packageIt = mPackages.begin();
+    MWMechanics::AiPackage* package = packageIt->get();
+    if (!package->alwaysActive() && outOfRange)
+        return;
+
+    auto packageTypeId = package->getTypeId();
+    // workaround ai packages not being handled as in the vanilla engine
+    if (isActualAiPackage(packageTypeId))
+        mLastAiPackage = packageTypeId;
+    // if active package is combat one, choose nearest target
+    if (packageTypeId == AiPackageTypeId::Combat)
+    {
+        auto itActualCombat = mPackages.end();
 
-            float nearestDist = std::numeric_limits<float>::max();
-            osg::Vec3f vActorPos = actor.getRefData().getPosition().asVec3();
+        float nearestDist = std::numeric_limits<float>::max();
+        osg::Vec3f vActorPos = actor.getRefData().getPosition().asVec3();
 
-            float bestRating = 0.f;
+        float bestRating = 0.f;
 
-            for (auto it = mPackages.begin(); it != mPackages.end();)
-            {
-                if ((*it)->getTypeId() != AiPackageTypeId::Combat) break;
+        for (auto it = mPackages.begin(); it != mPackages.end();)
+        {
+            if ((*it)->getTypeId() != AiPackageTypeId::Combat) break;
 
-                MWWorld::Ptr target = (*it)->getTarget();
+            MWWorld::Ptr target = (*it)->getTarget();
 
-                // target disappeared (e.g. summoned creatures)
-                if (target.isEmpty())
-                {
-                    it = mPackages.erase(it);
-                }
-                else
-                {
-                    float rating = MWMechanics::getBestActionRating(actor, target);
+            // target disappeared (e.g. summoned creatures)
+            if (target.isEmpty())
+            {
+                it = erase(it);
+            }
+            else
+            {
+                float rating = MWMechanics::getBestActionRating(actor, target);
 
-                    const ESM::Position &targetPos = target.getRefData().getPosition();
+                const ESM::Position &targetPos = target.getRefData().getPosition();
 
-                    float distTo = (targetPos.asVec3() - vActorPos).length2();
+                float distTo = (targetPos.asVec3() - vActorPos).length2();
 
-                    // Small threshold for changing target
-                    if (it == mPackages.begin())
-                        distTo = std::max(0.f, distTo - 2500.f);
+                // Small threshold for changing target
+                if (it == mPackages.begin())
+                    distTo = std::max(0.f, distTo - 2500.f);
 
-                    // if a target has higher priority than current target or has same priority but closer
-                    if (rating > bestRating || ((distTo < nearestDist) && rating == bestRating))
-                    {
-                        nearestDist = distTo;
-                        itActualCombat = it;
-                        bestRating = rating;
-                    }
-                    ++it;
+                // if a target has higher priority than current target or has same priority but closer
+                if (rating > bestRating || ((distTo < nearestDist) && rating == bestRating))
+                {
+                    nearestDist = distTo;
+                    itActualCombat = it;
+                    bestRating = rating;
                 }
+                ++it;
             }
+        }
 
-            assert(!mPackages.empty());
+        assert(!mPackages.empty());
 
-            if (nearestDist < std::numeric_limits<float>::max() && mPackages.begin() != itActualCombat)
-            {
-                assert(itActualCombat != mPackages.end());
-                // move combat package with nearest target to the front
-                mPackages.splice(mPackages.begin(), mPackages, itActualCombat);
-            }
-
-            packageIt = mPackages.begin();
-            package = packageIt->get();
-            packageTypeId = package->getTypeId();
+        if (nearestDist < std::numeric_limits<float>::max() && mPackages.begin() != itActualCombat)
+        {
+            assert(itActualCombat != mPackages.end());
+            // move combat package with nearest target to the front
+            std::rotate(mPackages.begin(), itActualCombat, std::next(itActualCombat));
         }
 
-        try
+        packageIt = mPackages.begin();
+        package = packageIt->get();
+        packageTypeId = package->getTypeId();
+    }
+
+    try
+    {
+        if (package->execute(actor, characterController, mAiState, duration))
         {
-            if (package->execute(actor, characterController, mAiState, duration))
-            {
-                // Put repeating noncombat AI packages on the end of the stack so they can be used again
-                if (isActualAiPackage(packageTypeId) && package->getRepeat())
-                {
-                    package->reset();
-                    mPackages.push_back(package->clone());
-                }
-                // To account for the rare case where AiPackage::execute() queued another AI package
-                // (e.g. AiPursue executing a dialogue script that uses startCombat)
-                mPackages.erase(packageIt);
-                if (isActualAiPackage(packageTypeId))
-                    mDone = true;
-            }
-            else
+            // Put repeating noncombat AI packages on the end of the stack so they can be used again
+            if (isActualAiPackage(packageTypeId) && package->getRepeat())
             {
-                mDone = false;
+                package->reset();
+                mPackages.push_back(package->clone());
             }
+            // To account for the rare case where AiPackage::execute() queued another AI package
+            // (e.g. AiPursue executing a dialogue script that uses startCombat)
+            erase(packageIt);
+            if (isActualAiPackage(packageTypeId))
+                mDone = true;
         }
-        catch (std::exception& e)
+        else
         {
-            Log(Debug::Error) << "Error during AiSequence::execute: " << e.what();
+            mDone = false;
         }
     }
+    catch (std::exception& e)
+    {
+        Log(Debug::Error) << "Error during AiSequence::execute: " << e.what();
+    }
 }
 
 void AiSequence::clear()
 {
     mPackages.clear();
+    mNumCombatPackages = 0;
+    mNumPursuitPackages = 0;
 }
 
 void AiSequence::stack (const AiPackage& package, const MWWorld::Ptr& actor, bool cancelOther)
@@ -353,7 +380,7 @@ void AiSequence::stack (const AiPackage& package, const MWWorld::Ptr& actor, boo
         {
             if((*it)->canCancel())
             {
-                it = mPackages.erase(it);
+                it = erase(it);
             }
             else
                 ++it;
@@ -373,11 +400,13 @@ void AiSequence::stack (const AiPackage& package, const MWWorld::Ptr& actor, boo
 
         if((*it)->getPriority() <= package.getPriority())
         {
+            onPackageAdded(package);
             mPackages.insert(it, package.clone());
             return;
         }
     }
 
+    onPackageAdded(package);
     mPackages.push_back(package.clone());
 
     // Make sure that temporary storage is empty
@@ -435,6 +464,8 @@ void AiSequence::fill(const ESM::AIPackageList &list)
             ESM::AITarget data = esmPackage.mTarget;
             package = std::make_unique<MWMechanics::AiFollow>(data.mId.toStringView(), data.mDuration, data.mX, data.mY, data.mZ, data.mShouldRepeat != 0);
         }
+
+        onPackageAdded(*package);
         mPackages.push_back(std::move(package));
     }
 }
@@ -504,6 +535,7 @@ void AiSequence::readState(const ESM::AiSequence::AiSequence &sequence)
         if (!package.get())
             continue;
 
+        onPackageAdded(*package);
         mPackages.push_back(std::move(package));
     }
 

apps/openmw/mwmechanics/aisequence.hpp

@@ -1,8 +1,9 @@
 #ifndef GAME_MWMECHANICS_AISEQUENCE_H
 #define GAME_MWMECHANICS_AISEQUENCE_H
 
-#include <list>
 #include <memory>
+#include <vector>
+#include <algorithm>
 
 #include "aistate.hpp"
 #include "aipackagetypeid.hpp"
@@ -22,8 +23,6 @@ namespace ESM
     }
 }
 
-
-
 namespace MWMechanics
 {
     class AiPackage;
@@ -33,15 +32,20 @@ namespace MWMechanics
     struct AiTemporaryBase;
     typedef DerivedClassStorage<AiTemporaryBase> AiState;
 
+    using AiPackages = std::vector<std::shared_ptr<AiPackage>>;
+
     /// \brief Sequence of AI-packages for a single actor
     /** The top-most AI package is run each frame. When completed, it is removed from the stack. **/
     class AiSequence
     {
             ///AiPackages to run though
-            std::list<std::shared_ptr<AiPackage>> mPackages;
+            AiPackages mPackages;
 
             ///Finished with top AIPackage, set for one frame
-            bool mDone;
+            bool mDone{};
+
+            int mNumCombatPackages{};
+            int mNumPursuitPackages{};
 
             ///Copy AiSequence
             void copy (const AiSequence& sequence);
@@ -50,6 +54,11 @@ namespace MWMechanics
             AiPackageTypeId mLastAiPackage;
             AiState mAiState;
 
+            void onPackageAdded(const AiPackage& package);
+            void onPackageRemoved(const AiPackage& package);
+
+            AiPackages::iterator erase(AiPackages::iterator package);
+
         public:
             ///Default constructor
             AiSequence();
@@ -63,12 +72,31 @@ namespace MWMechanics
             virtual ~AiSequence();
 
             /// Iterator may be invalidated by any function calls other than begin() or end().
-            std::list<std::shared_ptr<AiPackage>>::const_iterator begin() const { return mPackages.begin(); }
-            std::list<std::shared_ptr<AiPackage>>::const_iterator end() const { return mPackages.end(); }
-
-            void erase(std::list<std::shared_ptr<AiPackage>>::const_iterator package);
-
-            std::list<std::shared_ptr<AiPackage>>& getUnderlyingList() { return mPackages; }
+            AiPackages::const_iterator begin() const { return mPackages.begin(); }
+            AiPackages::const_iterator end() const { return mPackages.end(); }
+
+            /// Removes all packages controlled by the predicate.
+            template<typename F>
+            void erasePackagesIf(const F&& pred)
+            {
+                mPackages.erase(std::remove_if(mPackages.begin(), mPackages.end(), [&](auto& entry)
+                {
+                    const bool doRemove = pred(entry);
+                    if (doRemove)
+                        onPackageRemoved(*entry);
+                    return doRemove;
+                }), mPackages.end());
+            }
+
+            /// Removes a single package controlled by the predicate.
+            template<typename F>
+            void erasePackageIf(const F&& pred)
+            {
+                auto it = std::find_if(mPackages.begin(), mPackages.end(), pred);
+                if (it == mPackages.end())
+                    return;
+                erase(it);
+            }
 
             /// Returns currently executing AiPackage type
             /** \see enum class AiPackageTypeId **/
@@ -89,6 +117,12 @@ namespace MWMechanics
             /// Is there any combat package?
             bool isInCombat () const;
 
+            /// Is there any pursuit package.
+            bool isInPursuit() const;
+
+            /// Removes all packages using the specified id.
+            void removePackagesById(AiPackageTypeId id);
+
             /// Are we in combat with any other actor, who's also engaging us?
             bool isEngagedWithActor () const;
 

apps/openmw/mwmechanics/mechanicsmanagerimp.cpp

@@ -1318,7 +1318,7 @@ namespace MWMechanics
                 // once the bounty has been paid.
                 actor.getClass().getNpcStats(actor).setCrimeId(id);
 
-                if (!actor.getClass().getCreatureStats(actor).getAiSequence().hasPackage(AiPackageTypeId::Pursue))
+                if (!actor.getClass().getCreatureStats(actor).getAiSequence().isInPursuit())
                 {
                     actor.getClass().getCreatureStats(actor).getAiSequence().stack(AiPursue(player), actor);
                 }
@@ -1396,7 +1396,7 @@ namespace MWMechanics
             {
                 // Attacker is in combat with us, but we are not in combat with the attacker yet. Time to fight back.
                 // Note: accidental or collateral damage attacks are ignored.
-                if (!victim.getClass().getCreatureStats(victim).getAiSequence().hasPackage(AiPackageTypeId::Pursue))
+                if (!victim.getClass().getCreatureStats(victim).getAiSequence().isInPursuit())
                     startCombat(victim, player);
 
                 // Set the crime ID, which we will use to calm down participants
@@ -1442,7 +1442,7 @@ namespace MWMechanics
         {
             // Attacker is in combat with us, but we are not in combat with the attacker yet. Time to fight back.
             // Note: accidental or collateral damage attacks are ignored.
-            if (!target.getClass().getCreatureStats(target).getAiSequence().hasPackage(AiPackageTypeId::Pursue))
+            if (!target.getClass().getCreatureStats(target).getAiSequence().isInPursuit())
             {
                 // If an actor has OnPCHitMe declared in his script, his Fight = 0 and the attacker is player,
                 // he will attack the player only if we will force him (e.g. via StartCombat console command)
@@ -1467,7 +1467,7 @@ namespace MWMechanics
         const MWMechanics::AiSequence& seq = target.getClass().getCreatureStats(target).getAiSequence();
         return target.getClass().isNpc() && !attacker.isEmpty() && !seq.isInCombat(attacker)
                 && !isAggressive(target, attacker) && !seq.isEngagedWithActor()
-                && !target.getClass().getCreatureStats(target).getAiSequence().hasPackage(AiPackageTypeId::Pursue);
+                && !target.getClass().getCreatureStats(target).getAiSequence().isInPursuit();
     }
 
     void MechanicsManager::actorKilled(const MWWorld::Ptr &victim, const MWWorld::Ptr &attacker)

apps/openmw/mwmechanics/spelleffects.cpp

@@ -979,12 +979,10 @@ void removeMagicEffect(const MWWorld::Ptr& target, ActiveSpells::ActiveSpellPara
             if(magnitudes.get(effect.mEffectId).getMagnitude() <= 0.f)
             {
                 auto& seq = target.getClass().getCreatureStats(target).getAiSequence();
-                auto it = std::find_if(seq.begin(), seq.end(), [&](const auto& package)
+                seq.erasePackageIf([&](const auto& package)
                 {
                     return package->getTypeId() == MWMechanics::AiPackageTypeId::Follow && static_cast<const MWMechanics::AiFollow*>(package.get())->isCommanded();
                 });
-                if(it != seq.end())
-                    seq.erase(it);
             }
             break;
         case ESM::MagicEffect::ExtraSpell: