From dbc12a2cf195abc6b201eb48d3a53e114cc3d6eb Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Wed, 6 Nov 2024 19:46:45 +0100 Subject: [PATCH 1/5] 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 c22ac65daf..d009f4872c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,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 401ba0ae86..1ead5b16bd 100644 --- a/apps/openmw/mwmechanics/actors.cpp +++ b/apps/openmw/mwmechanics/actors.cpp @@ -1248,7 +1248,7 @@ namespace MWMechanics { if (!keepActive) removeTemporaryEffects(iter->second->getPtr()); - mActors.erase(iter->second); + iter->second->invalidate(); mIndex.erase(iter); } } @@ -1300,16 +1300,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; } } @@ -1328,6 +1327,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. @@ -1392,6 +1393,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; @@ -1510,6 +1513,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 @@ -1571,6 +1576,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( @@ -1628,6 +1635,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()); @@ -1693,8 +1702,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()); @@ -1744,6 +1760,8 @@ namespace MWMechanics { for (Actor& actor : mActors) { + if (actor.isInvalid()) + continue; const MWWorld::Class& cls = actor.getPtr().getClass(); CreatureStats& stats = cls.getCreatureStats(actor.getPtr()); @@ -1831,6 +1849,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); @@ -1850,6 +1870,8 @@ namespace MWMechanics for (const Actor& actor : mActors) { + if (actor.isInvalid()) + continue; if (actor.getPtr().getClass().getCreatureStats(actor.getPtr()).isDead()) { adjustMagicEffects(actor.getPtr(), duration); @@ -2047,7 +2069,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) @@ -2061,6 +2086,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()); } @@ -2070,6 +2097,8 @@ namespace MWMechanics { for (const Actor& actor : mActors) { + if (actor.isInvalid()) + continue; if ((actor.getPtr().getRefData().getPosition().asVec3() - position).length2() <= radius * radius) return true; } @@ -2083,6 +2112,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; @@ -2353,10 +2384,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 46f6440ae6..c804cbad7c 100644 --- a/apps/openmw/mwmechanics/mechanicsmanagerimp.cpp +++ b/apps/openmw/mwmechanics/mechanicsmanagerimp.cpp @@ -1718,6 +1718,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 From ade762abc558fcdebc08d8eeafc3a98e8a76223d Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Wed, 6 Nov 2024 21:33:58 +0100 Subject: [PATCH 2/5] Run ~CharacterController when invalidating an Actor --- apps/openmw/mwmechanics/actor.hpp | 21 +++++++++++++-------- apps/openmw/mwmechanics/actors.cpp | 4 +++- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/apps/openmw/mwmechanics/actor.hpp b/apps/openmw/mwmechanics/actor.hpp index 1cafbe8253..6deaba3a95 100644 --- a/apps/openmw/mwmechanics/actor.hpp +++ b/apps/openmw/mwmechanics/actor.hpp @@ -2,6 +2,7 @@ #define OPENMW_MECHANICS_ACTOR_H #include +#include #include "character.hpp" #include "creaturestats.hpp" @@ -29,18 +30,18 @@ namespace MWMechanics { public: Actor(const MWWorld::Ptr& ptr, MWRender::Animation* animation) - : mCharacterController(ptr, animation) - , mPositionAdjusted(ptr.getClass().getCreatureStats(ptr).getFallHeight() > 0) + : mPositionAdjusted(ptr.getClass().getCreatureStats(ptr).getFallHeight() > 0) { + mCharacterController.emplace(ptr, animation); } - const MWWorld::Ptr& getPtr() const { return mCharacterController.getPtr(); } + const MWWorld::Ptr& getPtr() const { return mCharacterController->getPtr(); } /// Notify this actor of its new base object Ptr, use when the object changed cells - void updatePtr(const MWWorld::Ptr& newPtr) { mCharacterController.updatePtr(newPtr); } + void updatePtr(const MWWorld::Ptr& newPtr) { mCharacterController->updatePtr(newPtr); } - CharacterController& getCharacterController() { return mCharacterController; } - const CharacterController& getCharacterController() const { return mCharacterController; } + CharacterController& getCharacterController() { return *mCharacterController; } + const CharacterController& getCharacterController() const { return *mCharacterController; } int getGreetingTimer() const { return mGreetingTimer; } void setGreetingTimer(int timer) { mGreetingTimer = timer; } @@ -62,11 +63,15 @@ namespace MWMechanics void setPositionAdjusted(bool adjusted) { mPositionAdjusted = adjusted; } bool getPositionAdjusted() const { return mPositionAdjusted; } - void invalidate() { mInvalid = true; } + void invalidate() + { + mInvalid = true; + mCharacterController.reset(); + } bool isInvalid() const { return mInvalid; } private: - CharacterController mCharacterController; + std::optional mCharacterController; int mGreetingTimer{ 0 }; float mTargetAngleRadians{ 0.f }; GreetingState mGreetingState{ Greet_None }; diff --git a/apps/openmw/mwmechanics/actors.cpp b/apps/openmw/mwmechanics/actors.cpp index 1ead5b16bd..6498e24989 100644 --- a/apps/openmw/mwmechanics/actors.cpp +++ b/apps/openmw/mwmechanics/actors.cpp @@ -122,6 +122,8 @@ namespace { for (const MWMechanics::Actor& actor : actors) { + if (actor.isInvalid()) + continue; const MWWorld::Ptr& iteratedActor = actor.getPtr(); if (iteratedActor == player || iteratedActor == actorPtr) continue; @@ -345,7 +347,7 @@ namespace MWMechanics // Find something nearby. for (const Actor& otherActor : actors) { - if (otherActor.getPtr() == ptr) + if (otherActor.isInvalid() || otherActor.getPtr() == ptr) continue; updateHeadTracking( From 5eb5fd2a13e968a251e479cd83c7735e82337847 Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Thu, 7 Nov 2024 16:37:28 +0100 Subject: [PATCH 3/5] Allow unsetting a character controller's animation --- apps/openmw/mwmechanics/actor.hpp | 19 ++++--- apps/openmw/mwmechanics/actors.cpp | 2 +- apps/openmw/mwmechanics/character.cpp | 72 +++++++++++++++------------ apps/openmw/mwmechanics/character.hpp | 15 +++--- apps/openmw/mwmechanics/objects.cpp | 2 +- 5 files changed, 61 insertions(+), 49 deletions(-) diff --git a/apps/openmw/mwmechanics/actor.hpp b/apps/openmw/mwmechanics/actor.hpp index 6deaba3a95..69e370ec86 100644 --- a/apps/openmw/mwmechanics/actor.hpp +++ b/apps/openmw/mwmechanics/actor.hpp @@ -2,7 +2,6 @@ #define OPENMW_MECHANICS_ACTOR_H #include -#include #include "character.hpp" #include "creaturestats.hpp" @@ -29,19 +28,19 @@ namespace MWMechanics class Actor { public: - Actor(const MWWorld::Ptr& ptr, MWRender::Animation* animation) - : mPositionAdjusted(ptr.getClass().getCreatureStats(ptr).getFallHeight() > 0) + Actor(const MWWorld::Ptr& ptr, MWRender::Animation& animation) + : mCharacterController(ptr, animation) + , mPositionAdjusted(ptr.getClass().getCreatureStats(ptr).getFallHeight() > 0) { - mCharacterController.emplace(ptr, animation); } - const MWWorld::Ptr& getPtr() const { return mCharacterController->getPtr(); } + const MWWorld::Ptr& getPtr() const { return mCharacterController.getPtr(); } /// Notify this actor of its new base object Ptr, use when the object changed cells - void updatePtr(const MWWorld::Ptr& newPtr) { mCharacterController->updatePtr(newPtr); } + void updatePtr(const MWWorld::Ptr& newPtr) { mCharacterController.updatePtr(newPtr); } - CharacterController& getCharacterController() { return *mCharacterController; } - const CharacterController& getCharacterController() const { return *mCharacterController; } + CharacterController& getCharacterController() { return mCharacterController; } + const CharacterController& getCharacterController() const { return mCharacterController; } int getGreetingTimer() const { return mGreetingTimer; } void setGreetingTimer(int timer) { mGreetingTimer = timer; } @@ -66,12 +65,12 @@ namespace MWMechanics void invalidate() { mInvalid = true; - mCharacterController.reset(); + mCharacterController.detachAnimation(); } bool isInvalid() const { return mInvalid; } private: - std::optional mCharacterController; + CharacterController mCharacterController; int mGreetingTimer{ 0 }; float mTargetAngleRadians{ 0.f }; GreetingState mGreetingState{ Greet_None }; diff --git a/apps/openmw/mwmechanics/actors.cpp b/apps/openmw/mwmechanics/actors.cpp index 6498e24989..6913234f5c 100644 --- a/apps/openmw/mwmechanics/actors.cpp +++ b/apps/openmw/mwmechanics/actors.cpp @@ -1198,7 +1198,7 @@ namespace MWMechanics MWRender::Animation* anim = MWBase::Environment::get().getWorld()->getAnimation(ptr); if (!anim) return; - const auto it = mActors.emplace(mActors.end(), ptr, anim); + const auto it = mActors.emplace(mActors.end(), ptr, *anim); mIndex.emplace(ptr.mRef, it); if (updateImmediately) diff --git a/apps/openmw/mwmechanics/character.cpp b/apps/openmw/mwmechanics/character.cpp index ed4eb9e0ec..5ac5c07aa1 100644 --- a/apps/openmw/mwmechanics/character.cpp +++ b/apps/openmw/mwmechanics/character.cpp @@ -529,7 +529,7 @@ namespace MWMechanics bool CharacterController::onOpen() const { - if (mPtr.getType() == ESM::Container::sRecordId) + if (mPtr.getType() == ESM::Container::sRecordId && mAnimation) { if (!mAnimation->hasAnimation("containeropen")) return true; @@ -553,7 +553,7 @@ namespace MWMechanics { if (mPtr.getType() == ESM::Container::sRecordId) { - if (!mAnimation->hasAnimation("containerclose")) + if (!mAnimation || !mAnimation->hasAnimation("containerclose")) return; float complete, startPoint = 0.f; @@ -877,14 +877,16 @@ namespace MWMechanics } mDeathState = hitStateToDeathState(mHitState); - if (mDeathState == CharState_None && MWBase::Environment::get().getWorld()->isSwimming(mPtr)) - mDeathState = CharState_SwimDeath; - - if (mDeathState == CharState_None || !mAnimation->hasAnimation(deathStateToAnimGroup(mDeathState))) - mDeathState = chooseRandomDeathState(); + if (mDeathState == CharState_None) + { + if (MWBase::Environment::get().getWorld()->isSwimming(mPtr)) + mDeathState = CharState_SwimDeath; + else if (mAnimation && !mAnimation->hasAnimation(deathStateToAnimGroup(mDeathState))) + mDeathState = chooseRandomDeathState(); + } // Do not interrupt scripted animation by death - if (isScriptedAnimPlaying()) + if (!mAnimation || isScriptedAnimPlaying()) return; playDeath(startpoint, mDeathState); @@ -904,13 +906,10 @@ namespace MWMechanics return result; } - CharacterController::CharacterController(const MWWorld::Ptr& ptr, MWRender::Animation* anim) + CharacterController::CharacterController(const MWWorld::Ptr& ptr, MWRender::Animation& anim) : mPtr(ptr) - , mAnimation(anim) + , mAnimation(&anim) { - if (!mAnimation) - return; - mAnimation->setTextKeyListener(this); const MWWorld::Class& cls = mPtr.getClass(); @@ -986,11 +985,17 @@ namespace MWMechanics } CharacterController::~CharacterController() + { + detachAnimation(); + } + + void CharacterController::detachAnimation() { if (mAnimation) { persistAnimationState(); mAnimation->setTextKeyListener(nullptr); + mAnimation = nullptr; } } @@ -1060,7 +1065,7 @@ namespace MWMechanics std::string_view action = evt.substr(groupname.size() + 2); if (action == "equip attach") { - if (mUpperBodyState == UpperBodyState::Equipping) + if (mUpperBodyState == UpperBodyState::Equipping && mAnimation) { if (groupname == "shield") mAnimation->showCarriedLeft(true); @@ -1070,7 +1075,7 @@ namespace MWMechanics } else if (action == "unequip detach") { - if (mUpperBodyState == UpperBodyState::Unequipping) + if (mUpperBodyState == UpperBodyState::Unequipping && mAnimation) { if (groupname == "shield") mAnimation->showCarriedLeft(false); @@ -1142,18 +1147,18 @@ namespace MWMechanics mPtr, mAttackStrength, ESM::Weapon::AT_Thrust, mAttackVictim, mAttackHitPos, mAttackSuccess); } } - else if (action == "shoot attach") + else if (action == "shoot attach" && mAnimation) mAnimation->attachArrow(); else if (action == "shoot release") { // See notes for melee release above - if (mAttackStrength != -1.f) + if (mAttackStrength != -1.f && mAnimation) { mAnimation->releaseArrow(mAttackStrength); mAttackStrength = -1.f; } } - else if (action == "shoot follow attach") + else if (action == "shoot follow attach" && mAnimation) mAnimation->attachArrow(); // Make sure this key is actually for the RangeType we are casting. The flame atronach has // the same animation for all range types, so there are 3 "release" keys on the same time, one for each range @@ -1226,7 +1231,8 @@ namespace MWMechanics float CharacterController::calculateWindUp() const { - if (mCurrentWeapon.empty() || mWeaponType == ESM::Weapon::PickProbe || isRandomAttackAnimation(mCurrentWeapon)) + if (mCurrentWeapon.empty() || mWeaponType == ESM::Weapon::PickProbe || isRandomAttackAnimation(mCurrentWeapon) + || !mAnimation) return -1.f; float minAttackTime = mAnimation->getTextKeyTime(mCurrentWeapon + ": " + mAttackType + " min attack"); @@ -1935,6 +1941,8 @@ namespace MWMechanics void CharacterController::update(float duration) { + if (!mAnimation) + return; MWBase::World* world = MWBase::Environment::get().getWorld(); MWBase::SoundManager* sndMgr = MWBase::Environment::get().getSoundManager(); const MWWorld::Class& cls = mPtr.getClass(); @@ -2513,7 +2521,7 @@ namespace MWMechanics ESM::AnimationState::ScriptedAnimation anim; anim.mGroup = iter->mGroup; - if (iter == mAnimQueue.begin()) + if (iter == mAnimQueue.begin() && mAnimation) { float complete; size_t loopcount; @@ -2726,23 +2734,18 @@ namespace MWMechanics void CharacterController::clearAnimQueue(bool clearScriptedAnims) { // Do not interrupt scripted animations, if we want to keep them - if ((!isScriptedAnimPlaying() || clearScriptedAnims) && !mAnimQueue.empty()) + if (mAnimation && (!isScriptedAnimPlaying() || clearScriptedAnims) && !mAnimQueue.empty()) mAnimation->disable(mAnimQueue.front().mGroup); if (clearScriptedAnims) { - mAnimation->setPlayScriptedOnly(false); + if (mAnimation) + mAnimation->setPlayScriptedOnly(false); mAnimQueue.clear(); return; } - for (AnimationQueue::iterator it = mAnimQueue.begin(); it != mAnimQueue.end();) - { - if (!it->mScripted) - it = mAnimQueue.erase(it); - else - ++it; - } + std::erase_if(mAnimQueue, [](const AnimationQueueEntry& entry) { return !entry.mScripted; }); } void CharacterController::forceStateUpdate() @@ -2851,6 +2854,8 @@ namespace MWMechanics void CharacterController::setVisibility(float visibility) const { + if (!mAnimation) + return; // We should take actor's invisibility in account if (mPtr.getClass().isActor()) { @@ -2911,7 +2916,7 @@ namespace MWMechanics bool CharacterController::isReadyToBlock() const { - return updateCarriedLeftVisible(mWeaponType); + return mAnimation && updateCarriedLeftVisible(mWeaponType); } bool CharacterController::isKnockedDown() const @@ -3015,7 +3020,8 @@ namespace MWMechanics void CharacterController::setActive(int active) const { - mAnimation->setActive(active); + if (mAnimation) + mAnimation->setActive(active); } void CharacterController::setHeadTrackTarget(const MWWorld::ConstPtr& target) @@ -3046,6 +3052,8 @@ namespace MWMechanics float CharacterController::getAnimationMovementDirection() const { + if (!mAnimation) + return 0.f; switch (mMovementState) { case CharState_RunLeft: @@ -3140,6 +3148,8 @@ namespace MWMechanics MWWorld::MovementDirectionFlags CharacterController::getSupportedMovementDirections() const { + if (!mAnimation) + return 0; using namespace std::string_view_literals; // There are fallbacks in the CharacterController::refreshMovementAnims for certain animations. Arrays below // represent them. diff --git a/apps/openmw/mwmechanics/character.hpp b/apps/openmw/mwmechanics/character.hpp index f043419a81..c9744625e3 100644 --- a/apps/openmw/mwmechanics/character.hpp +++ b/apps/openmw/mwmechanics/character.hpp @@ -251,13 +251,21 @@ namespace MWMechanics void prepareHit(); + void unpersistAnimationState(); + + void playBlendedAnimation(const std::string& groupname, const MWRender::AnimPriority& priority, int blendMask, + bool autodisable, float speedmult, std::string_view start, std::string_view stop, float startpoint, + uint32_t loops, bool loopfallback = false) const; + public: - CharacterController(const MWWorld::Ptr& ptr, MWRender::Animation* anim); + CharacterController(const MWWorld::Ptr& ptr, MWRender::Animation& anim); virtual ~CharacterController(); CharacterController(const CharacterController&) = delete; CharacterController(CharacterController&&) = delete; + void detachAnimation(); + const MWWorld::Ptr& getPtr() const { return mPtr; } void handleTextKey(std::string_view groupname, SceneUtil::TextKeyMap::ConstIterator key, @@ -274,11 +282,6 @@ namespace MWMechanics void onClose() const; void persistAnimationState() const; - void unpersistAnimationState(); - - void playBlendedAnimation(const std::string& groupname, const MWRender::AnimPriority& priority, int blendMask, - bool autodisable, float speedmult, std::string_view start, std::string_view stop, float startpoint, - uint32_t loops, bool loopfallback = false) const; bool playGroup(std::string_view groupname, int mode, uint32_t count, bool scripted = false); bool playGroupLua(std::string_view groupname, float speed, std::string_view startKey, std::string_view stopKey, uint32_t loops, bool forceLoop); diff --git a/apps/openmw/mwmechanics/objects.cpp b/apps/openmw/mwmechanics/objects.cpp index 12d342666b..62f0df556d 100644 --- a/apps/openmw/mwmechanics/objects.cpp +++ b/apps/openmw/mwmechanics/objects.cpp @@ -20,7 +20,7 @@ namespace MWMechanics if (anim == nullptr) return; - const auto it = mObjects.emplace(mObjects.end(), ptr, anim); + const auto it = mObjects.emplace(mObjects.end(), ptr, *anim); mIndex.emplace(ptr.mRef, it); } From 16551052f6fda58cfb66429f34ece385bd5c5e7c Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Mon, 18 Nov 2024 17:26:43 +0100 Subject: [PATCH 4/5] Move null checks to address feedback --- apps/openmw/mwmechanics/character.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/apps/openmw/mwmechanics/character.cpp b/apps/openmw/mwmechanics/character.cpp index 5ac5c07aa1..c96fefeb33 100644 --- a/apps/openmw/mwmechanics/character.cpp +++ b/apps/openmw/mwmechanics/character.cpp @@ -1002,6 +1002,8 @@ namespace MWMechanics void CharacterController::handleTextKey( std::string_view groupname, SceneUtil::TextKeyMap::ConstIterator key, const SceneUtil::TextKeyMap& map) { + if (!mAnimation) + return; std::string_view evt = key->second; MWBase::Environment::get().getLuaManager()->animationTextKey(mPtr, key->second); @@ -1147,18 +1149,18 @@ namespace MWMechanics mPtr, mAttackStrength, ESM::Weapon::AT_Thrust, mAttackVictim, mAttackHitPos, mAttackSuccess); } } - else if (action == "shoot attach" && mAnimation) + else if (action == "shoot attach") mAnimation->attachArrow(); else if (action == "shoot release") { // See notes for melee release above - if (mAttackStrength != -1.f && mAnimation) + if (mAttackStrength != -1.f) { mAnimation->releaseArrow(mAttackStrength); mAttackStrength = -1.f; } } - else if (action == "shoot follow attach" && mAnimation) + else if (action == "shoot follow attach") mAnimation->attachArrow(); // Make sure this key is actually for the RangeType we are casting. The flame atronach has // the same animation for all range types, so there are 3 "release" keys on the same time, one for each range @@ -1231,8 +1233,8 @@ namespace MWMechanics float CharacterController::calculateWindUp() const { - if (mCurrentWeapon.empty() || mWeaponType == ESM::Weapon::PickProbe || isRandomAttackAnimation(mCurrentWeapon) - || !mAnimation) + if (!mAnimation || mCurrentWeapon.empty() || mWeaponType == ESM::Weapon::PickProbe + || isRandomAttackAnimation(mCurrentWeapon)) return -1.f; float minAttackTime = mAnimation->getTextKeyTime(mCurrentWeapon + ": " + mAttackType + " min attack"); From b6f9773f4359e9ecf33b1e167bf7c83baf71fad8 Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Mon, 18 Nov 2024 19:57:57 +0100 Subject: [PATCH 5/5] Remove more redundant checks --- apps/openmw/mwmechanics/character.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/openmw/mwmechanics/character.cpp b/apps/openmw/mwmechanics/character.cpp index c96fefeb33..4fbc30bc1a 100644 --- a/apps/openmw/mwmechanics/character.cpp +++ b/apps/openmw/mwmechanics/character.cpp @@ -1067,7 +1067,7 @@ namespace MWMechanics std::string_view action = evt.substr(groupname.size() + 2); if (action == "equip attach") { - if (mUpperBodyState == UpperBodyState::Equipping && mAnimation) + if (mUpperBodyState == UpperBodyState::Equipping) { if (groupname == "shield") mAnimation->showCarriedLeft(true); @@ -1077,7 +1077,7 @@ namespace MWMechanics } else if (action == "unequip detach") { - if (mUpperBodyState == UpperBodyState::Unequipping && mAnimation) + if (mUpperBodyState == UpperBodyState::Unequipping) { if (groupname == "shield") mAnimation->showCarriedLeft(false);