From c098f2ccde424ea399034e43fa621ca0b70e8ace Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Wed, 6 Nov 2024 19:46:45 +0100 Subject: [PATCH] Prevent iterator invalidation during actor traversal --- CHANGELOG.md | 1 + apps/openmw/mwmechanics/actor.hpp | 6 +- apps/openmw/mwmechanics/actors.cpp | 58 ++++++++++++++----- .../mwmechanics/mechanicsmanagerimp.cpp | 2 + 4 files changed, 53 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5435835606..b4617936dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -80,6 +80,7 @@ Bug #4754: Stack of ammunition cannot be equipped partially Bug #4816: GetWeaponDrawn returns 1 before weapon is attached Bug #4822: Non-weapon equipment and body parts can't inherit time from parent animation + Bug #4885: Disable in dialogue result script causes a crash Bug #4898: Odd/Incorrect lighting on meshes Bug #5057: Weapon swing sound plays at same pitch whether it hits or misses Bug #5062: Root bone rotations for NPC animation don't work the same as for creature animation diff --git a/apps/openmw/mwmechanics/actor.hpp b/apps/openmw/mwmechanics/actor.hpp index d7438712d9..1cafbe8253 100644 --- a/apps/openmw/mwmechanics/actor.hpp +++ b/apps/openmw/mwmechanics/actor.hpp @@ -62,14 +62,18 @@ namespace MWMechanics void setPositionAdjusted(bool adjusted) { mPositionAdjusted = adjusted; } bool getPositionAdjusted() const { return mPositionAdjusted; } + void invalidate() { mInvalid = true; } + bool isInvalid() const { return mInvalid; } + private: CharacterController mCharacterController; int mGreetingTimer{ 0 }; float mTargetAngleRadians{ 0.f }; GreetingState mGreetingState{ Greet_None }; - bool mIsTurningToPlayer{ false }; Misc::DeviatingPeriodicTimer mEngageCombat{ 1.0f, 0.25f, Misc::Rng::deviate(0, 0.25f, MWBase::Environment::get().getWorld()->getPrng()) }; + bool mIsTurningToPlayer{ false }; + bool mInvalid{ false }; bool mPositionAdjusted; }; diff --git a/apps/openmw/mwmechanics/actors.cpp b/apps/openmw/mwmechanics/actors.cpp index 65eb71232f..8491cdcb7b 100644 --- a/apps/openmw/mwmechanics/actors.cpp +++ b/apps/openmw/mwmechanics/actors.cpp @@ -1247,7 +1247,7 @@ namespace MWMechanics { if (!keepActive) removeTemporaryEffects(iter->second->getPtr()); - mActors.erase(iter->second); + iter->second->invalidate(); mIndex.erase(iter); } } @@ -1299,16 +1299,15 @@ namespace MWMechanics void Actors::dropActors(const MWWorld::CellStore* cellStore, const MWWorld::Ptr& ignore) { - for (auto iter = mActors.begin(); iter != mActors.end();) + for (Actor& actor : mActors) { - if ((iter->getPtr().isInCell() && iter->getPtr().getCell() == cellStore) && iter->getPtr() != ignore) + if (!actor.isInvalid() && actor.getPtr().isInCell() && actor.getPtr().getCell() == cellStore + && actor.getPtr() != ignore) { - removeTemporaryEffects(iter->getPtr()); - mIndex.erase(iter->getPtr().mRef); - iter = mActors.erase(iter); + removeTemporaryEffects(actor.getPtr()); + mIndex.erase(actor.getPtr().mRef); + actor.invalidate(); } - else - ++iter; } } @@ -1327,6 +1326,8 @@ namespace MWMechanics const MWBase::World* const world = MWBase::Environment::get().getWorld(); for (const Actor& actor : mActors) { + if (actor.isInvalid()) + continue; const MWWorld::Ptr& ptr = actor.getPtr(); if (ptr == player) continue; // Don't interfere with player controls. @@ -1391,6 +1392,8 @@ namespace MWMechanics // Iterate through all other actors and predict collisions. for (const Actor& otherActor : mActors) { + if (otherActor.isInvalid()) + continue; const MWWorld::Ptr& otherPtr = otherActor.getPtr(); if (otherPtr == ptr || otherPtr == currentTarget) continue; @@ -1509,6 +1512,8 @@ namespace MWMechanics // AI and magic effects update for (Actor& actor : mActors) { + if (actor.isInvalid()) + continue; const bool isPlayer = actor.getPtr() == player; CharacterController& ctrl = actor.getCharacterController(); MWBase::LuaManager::ActorControls* luaControls @@ -1570,6 +1575,8 @@ namespace MWMechanics for (const Actor& otherActor : mActors) { + if (otherActor.isInvalid()) + continue; if (otherActor.getPtr() == actor.getPtr() || isPlayer) // player is not AI-controlled continue; engageCombat( @@ -1627,6 +1634,8 @@ namespace MWMechanics CharacterController* playerCharacter = nullptr; for (Actor& actor : mActors) { + if (actor.isInvalid()) + continue; const float dist = (playerPos - actor.getPtr().getRefData().getPosition().asVec3()).length(); const bool isPlayer = actor.getPtr() == player; CreatureStats& stats = actor.getPtr().getClass().getCreatureStats(actor.getPtr()); @@ -1692,8 +1701,15 @@ namespace MWMechanics luaControls->mJump = false; } - for (const Actor& actor : mActors) + for (auto it = mActors.begin(); it != mActors.end();) { + if (it->isInvalid()) + { + it = mActors.erase(it); + continue; + } + const Actor& actor = *it; + it++; const MWWorld::Class& cls = actor.getPtr().getClass(); CreatureStats& stats = cls.getCreatureStats(actor.getPtr()); @@ -1743,6 +1759,8 @@ namespace MWMechanics { for (Actor& actor : mActors) { + if (actor.isInvalid()) + continue; const MWWorld::Class& cls = actor.getPtr().getClass(); CreatureStats& stats = cls.getCreatureStats(actor.getPtr()); @@ -1830,6 +1848,8 @@ namespace MWMechanics { for (const Actor& actor : mActors) { + if (actor.isInvalid()) + continue; MWMechanics::ActiveSpells& spells = actor.getPtr().getClass().getCreatureStats(actor.getPtr()).getActiveSpells(); spells.purge(actor.getPtr(), casterActorId); @@ -1849,6 +1869,8 @@ namespace MWMechanics for (const Actor& actor : mActors) { + if (actor.isInvalid()) + continue; if (actor.getPtr().getClass().getCreatureStats(actor.getPtr()).isDead()) { adjustMagicEffects(actor.getPtr(), duration); @@ -2046,7 +2068,10 @@ namespace MWMechanics void Actors::persistAnimationStates() const { for (const Actor& actor : mActors) - actor.getCharacterController().persistAnimationState(); + { + if (!actor.isInvalid()) + actor.getCharacterController().persistAnimationState(); + } } void Actors::clearAnimationQueue(const MWWorld::Ptr& ptr, bool clearScripted) @@ -2060,6 +2085,8 @@ namespace MWMechanics { for (const Actor& actor : mActors) { + if (actor.isInvalid()) + continue; if ((actor.getPtr().getRefData().getPosition().asVec3() - position).length2() <= radius * radius) out.push_back(actor.getPtr()); } @@ -2069,6 +2096,8 @@ namespace MWMechanics { for (const Actor& actor : mActors) { + if (actor.isInvalid()) + continue; if ((actor.getPtr().getRefData().getPosition().asVec3() - position).length2() <= radius * radius) return true; } @@ -2082,6 +2111,8 @@ namespace MWMechanics list.push_back(actorPtr); for (const Actor& actor : mActors) { + if (actor.isInvalid()) + continue; const MWWorld::Ptr& iteratedActor = actor.getPtr(); if (iteratedActor == getPlayer()) continue; @@ -2352,10 +2383,11 @@ namespace MWMechanics if (!MWBase::Environment::get().getMechanicsManager()->isAIActive()) return; - for (auto it = mActors.begin(); it != mActors.end();) + for (const Actor& actor : mActors) { - const MWWorld::Ptr ptr = it->getPtr(); - ++it; + if (actor.isInvalid()) + continue; + const MWWorld::Ptr ptr = actor.getPtr(); if (ptr == getPlayer() || !isConscious(ptr) || ptr.getClass().getCreatureStats(ptr).isParalyzed()) continue; MWMechanics::AiSequence& seq = ptr.getClass().getCreatureStats(ptr).getAiSequence(); diff --git a/apps/openmw/mwmechanics/mechanicsmanagerimp.cpp b/apps/openmw/mwmechanics/mechanicsmanagerimp.cpp index 384c25953b..f0ba99b0ca 100644 --- a/apps/openmw/mwmechanics/mechanicsmanagerimp.cpp +++ b/apps/openmw/mwmechanics/mechanicsmanagerimp.cpp @@ -1725,6 +1725,8 @@ namespace MWMechanics .getActorId()); // Stops guard from ending combat if player is unreachable for (const Actor& actor : mActors) { + if (actor.isInvalid()) + continue; if (actor.getPtr().getClass().isClass(actor.getPtr(), "Guard")) { MWMechanics::AiSequence& aiSeq