Evaluate melee hits on weapon release (bug #5057)

CHANGELOG.md

@@ -2,6 +2,7 @@
 ------
 
     Bug #4127: Weapon animation looks choppy
+    Bug #5057: Weapon swing sound plays at same pitch whether it hits or misses
 
 0.48.0
 ------

apps/openmw/mwclass/creature.cpp

@@ -226,16 +226,10 @@ namespace MWClass
         return ptr.getRefData().getCustomData()->asCreatureCustomData().mCreatureStats;
     }
 
-
-    void Creature::hit(const MWWorld::Ptr& ptr, float attackStrength, int type) const
+    bool Creature::evaluateHit(const MWWorld::Ptr& ptr, MWWorld::Ptr& victim, osg::Vec3f& hitPosition) const
     {
-        MWWorld::LiveCellRef<ESM::Creature> *ref =
-            ptr.get<ESM::Creature>();
-        const MWWorld::Store<ESM::GameSetting> &gmst = MWBase::Environment::get().getWorld()->getStore().get<ESM::GameSetting>();
-        MWMechanics::CreatureStats &stats = getCreatureStats(ptr);
-
-        if (stats.getDrawState() != MWMechanics::DrawState::Weapon)
-            return;
+        victim = MWWorld::Ptr();
+        hitPosition = osg::Vec3f();
 
         // Get the weapon used (if hand-to-hand, weapon = inv.end())
         MWWorld::Ptr weapon;
@@ -247,36 +241,71 @@ namespace MWClass
                 weapon = *weaponslot;
         }
 
-        MWMechanics::applyFatigueLoss(ptr, weapon, attackStrength);
-
-        float dist = gmst.find("fCombatDistance")->mValue.getFloat();
+        MWBase::World *world = MWBase::Environment::get().getWorld();
+        const MWWorld::Store<ESM::GameSetting> &store = world->getStore().get<ESM::GameSetting>();
+        float dist = store.find("fCombatDistance")->mValue.getFloat();
         if (!weapon.isEmpty())
             dist *= weapon.get<ESM::Weapon>()->mBase->mData.mReach;
 
         // For AI actors, get combat targets to use in the ray cast. Only those targets will return a positive hit result.
         std::vector<MWWorld::Ptr> targetActors;
-        stats.getAiSequence().getCombatTargets(targetActors);
+        getCreatureStats(ptr).getAiSequence().getCombatTargets(targetActors);
 
         std::pair<MWWorld::Ptr, osg::Vec3f> result = MWBase::Environment::get().getWorld()->getHitContact(ptr, dist, targetActors);
-        if (result.first.isEmpty())
-            return; // Didn't hit anything
+        if (result.first.isEmpty()) // Didn't hit anything
+            return true;
 
-        MWWorld::Ptr victim = result.first;
+        const MWWorld::Class &othercls = result.first.getClass();
+        if (!othercls.isActor()) // Can't hit non-actors
+            return true;
 
-        if (!victim.getClass().isActor())
-            return; // Can't hit non-actors
+        MWMechanics::CreatureStats &otherstats = othercls.getCreatureStats(result.first);
+        if (otherstats.isDead()) // Can't hit dead actors
+            return true;
 
-        osg::Vec3f hitPosition (result.second);
+        // Note that earlier we returned true in spite of an apparent failure to hit anything alive.
+        // This is because hitting nothing is not a "miss" and should be handled as such character controller-side.
+        victim = result.first;
+        hitPosition = result.second;
 
-        float hitchance = MWMechanics::getHitChance(ptr, victim, ref->mBase->mData.mCombat);
-        auto& prng = MWBase::Environment::get().getWorld()->getPrng();
-        if(Misc::Rng::roll0to99(prng) >= hitchance)
+        float hitchance = MWMechanics::getHitChance(ptr, victim, ptr.get<ESM::Creature>()->mBase->mData.mCombat);
+        return Misc::Rng::roll0to99(world->getPrng()) < hitchance;
+    }
+
+    void Creature::hit(const MWWorld::Ptr& ptr, float attackStrength, int type, const MWWorld::Ptr& victim, const osg::Vec3f& hitPosition, bool success) const
+    {
+        MWMechanics::CreatureStats &stats = getCreatureStats(ptr);
+
+        if (stats.getDrawState() != MWMechanics::DrawState::Weapon)
+            return;
+
+        MWWorld::Ptr weapon;
+        if (hasInventoryStore(ptr))
+        {
+            MWWorld::InventoryStore &inv = getInventoryStore(ptr);
+            MWWorld::ContainerStoreIterator weaponslot = inv.getSlot(MWWorld::InventoryStore::Slot_CarriedRight);
+            if (weaponslot != inv.end() && weaponslot->getType() == ESM::Weapon::sRecordId)
+                weapon = *weaponslot;
+        }
+
+        MWMechanics::applyFatigueLoss(ptr, weapon, attackStrength);
+
+        if (victim.isEmpty())
+            return; // Didn't hit anything
+
+        const MWWorld::Class &othercls = victim.getClass();
+        MWMechanics::CreatureStats &otherstats = othercls.getCreatureStats(victim);
+        if (otherstats.isDead()) // Can't hit dead actors
+            return;
+
+        if (!success)
         {
             victim.getClass().onHit(victim, 0.0f, false, MWWorld::Ptr(), ptr, osg::Vec3f(), false);
             MWMechanics::reduceWeaponCondition(0.f, false, weapon, ptr);
             return;
         }
 
+        MWWorld::LiveCellRef<ESM::Creature> *ref = ptr.get<ESM::Creature>();
         int min,max;
         switch (type)
         {

apps/openmw/mwclass/creature.hpp

@@ -63,7 +63,9 @@ namespace MWClass
             MWMechanics::CreatureStats& getCreatureStats (const MWWorld::Ptr& ptr) const override;
             ///< Return creature stats
 
-            void hit(const MWWorld::Ptr& ptr, float attackStrength, int type) const override;
+            bool evaluateHit(const MWWorld::Ptr& ptr, MWWorld::Ptr& victim, osg::Vec3f& hitPosition) const override;
+
+            void hit(const MWWorld::Ptr& ptr, float attackStrength, int type, const MWWorld::Ptr& victim, const osg::Vec3f& hitPosition, bool success) const override;
 
             void onHit(const MWWorld::Ptr &ptr, float damage, bool ishealth, const MWWorld::Ptr &object, const MWWorld::Ptr &attacker, const osg::Vec3f &hitPosition, bool successful) const override;
 

apps/openmw/mwclass/npc.cpp

@@ -554,21 +554,20 @@ namespace MWClass
     }
 
 
-    void Npc::hit(const MWWorld::Ptr& ptr, float attackStrength, int type) const
+    bool Npc::evaluateHit(const MWWorld::Ptr& ptr, MWWorld::Ptr& victim, osg::Vec3f& hitPosition) const
     {
-        MWBase::World *world = MWBase::Environment::get().getWorld();
-
-        const MWWorld::Store<ESM::GameSetting> &store = world->getStore().get<ESM::GameSetting>();
+        victim = MWWorld::Ptr();
+        hitPosition = osg::Vec3f();
 
         // Get the weapon used (if hand-to-hand, weapon = inv.end())
         MWWorld::InventoryStore &inv = getInventoryStore(ptr);
         MWWorld::ContainerStoreIterator weaponslot = inv.getSlot(MWWorld::InventoryStore::Slot_CarriedRight);
-        MWWorld::Ptr weapon = ((weaponslot != inv.end()) ? *weaponslot : MWWorld::Ptr());
-        if(!weapon.isEmpty() && weapon.getType() != ESM::Weapon::sRecordId)
-            weapon = MWWorld::Ptr();
-
-        MWMechanics::applyFatigueLoss(ptr, weapon, attackStrength);
+        MWWorld::Ptr weapon;
+        if (weaponslot != inv.end() && weaponslot->getType() == ESM::Weapon::sRecordId)
+            weapon = *weaponslot;
 
+        MWBase::World *world = MWBase::Environment::get().getWorld();
+        const MWWorld::Store<ESM::GameSetting> &store = world->getStore().get<ESM::GameSetting>();
         const float fCombatDistance = store.find("fCombatDistance")->mValue.getFloat();
         float dist = fCombatDistance * (!weapon.isEmpty() ?
                                weapon.get<ESM::Weapon>()->mBase->mData.mReach :
@@ -581,28 +580,53 @@ namespace MWClass
 
         // TODO: Use second to work out the hit angle
         std::pair<MWWorld::Ptr, osg::Vec3f> result = world->getHitContact(ptr, dist, targetActors);
-        MWWorld::Ptr victim = result.first;
-        osg::Vec3f hitPosition (result.second);
+        if (result.first.isEmpty()) // Didn't hit anything
+            return true;
+
+        const MWWorld::Class &othercls = result.first.getClass();
+        if (!othercls.isActor()) // Can't hit non-actors
+            return true;
+
+        MWMechanics::CreatureStats &otherstats = othercls.getCreatureStats(result.first);
+        if (otherstats.isDead()) // Can't hit dead actors
+            return true;
+
+        // Note that earlier we returned true in spite of an apparent failure to hit anything alive.
+        // This is because hitting nothing is not a "miss" and should be handled as such character controller-side.
+        victim = result.first;
+        hitPosition = result.second;
+
+        int weapskill = ESM::Skill::HandToHand;
+        if (!weapon.isEmpty())
+            weapskill = weapon.getClass().getEquipmentSkill(weapon);
+
+        float hitchance = MWMechanics::getHitChance(ptr, victim, getSkill(ptr, weapskill));
+
+        return Misc::Rng::roll0to99(world->getPrng()) < hitchance;
+    }
+
+    void Npc::hit(const MWWorld::Ptr& ptr, float attackStrength, int type, const MWWorld::Ptr& victim, const osg::Vec3f& hitPosition, bool success) const
+    {
+        MWWorld::InventoryStore &inv = getInventoryStore(ptr);
+        MWWorld::ContainerStoreIterator weaponslot = inv.getSlot(MWWorld::InventoryStore::Slot_CarriedRight);
+        MWWorld::Ptr weapon;
+        if (weaponslot != inv.end() && weaponslot->getType() == ESM::Weapon::sRecordId)
+            weapon = *weaponslot;
+
+        MWMechanics::applyFatigueLoss(ptr, weapon, attackStrength);
+
         if(victim.isEmpty()) // Didn't hit anything
             return;
 
         const MWWorld::Class &othercls = victim.getClass();
-        if(!othercls.isActor()) // Can't hit non-actors
-            return;
         MWMechanics::CreatureStats &otherstats = othercls.getCreatureStats(victim);
-        if(otherstats.isDead()) // Can't hit dead actors
+        if (otherstats.isDead()) // Can't hit dead actors
             return;
 
         if(ptr == MWMechanics::getPlayer())
             MWBase::Environment::get().getWindowManager()->setEnemy(victim);
 
-        int weapskill = ESM::Skill::HandToHand;
-        if(!weapon.isEmpty())
-            weapskill = weapon.getClass().getEquipmentSkill(weapon);
-
-        float hitchance = MWMechanics::getHitChance(ptr, victim, getSkill(ptr, weapskill));
-
-        if (Misc::Rng::roll0to99(world->getPrng()) >= hitchance)
+        if (!success)
         {
             othercls.onHit(victim, 0.0f, false, weapon, ptr, osg::Vec3f(), false);
             MWMechanics::reduceWeaponCondition(0.f, false, weapon, ptr);
@@ -634,8 +658,15 @@ namespace MWClass
         {
             MWMechanics::getHandToHandDamage(ptr, victim, damage, healthdmg, attackStrength);
         }
+
+        MWBase::World *world = MWBase::Environment::get().getWorld();
+        const MWWorld::Store<ESM::GameSetting> &store = world->getStore().get<ESM::GameSetting>();
+
         if(ptr == MWMechanics::getPlayer())
         {
+            int weapskill = ESM::Skill::HandToHand;
+            if(!weapon.isEmpty())
+                weapskill = weapon.getClass().getEquipmentSkill(weapon);
             skillUsageSucceeded(ptr, weapskill, 0);
 
             const MWMechanics::AiSequence& seq = victim.getClass().getCreatureStats(victim).getAiSequence();

apps/openmw/mwclass/npc.hpp

@@ -76,7 +76,9 @@ namespace MWClass
 
             bool hasInventoryStore(const MWWorld::Ptr &ptr) const override { return true; }
 
-            void hit(const MWWorld::Ptr& ptr, float attackStrength, int type) const override;
+            bool evaluateHit(const MWWorld::Ptr& ptr, MWWorld::Ptr& victim, osg::Vec3f& hitPosition) const override;
+
+            void hit(const MWWorld::Ptr& ptr, float attackStrength, int type, const MWWorld::Ptr& victim, const osg::Vec3f& hitPosition, bool success) const override;
 
             void onHit(const MWWorld::Ptr &ptr, float damage, bool ishealth, const MWWorld::Ptr &object, const MWWorld::Ptr &attacker, const osg::Vec3f &hitPosition, bool successful) const override;
 

apps/openmw/mwmechanics/character.cpp

@@ -998,21 +998,21 @@ void CharacterController::handleTextKey(std::string_view groupname, SceneUtil::T
             mAnimation->showWeapons(false);
     }
     else if (action == "chop hit")
-        charClass.hit(mPtr, mAttackStrength, ESM::Weapon::AT_Chop);
+        charClass.hit(mPtr, mAttackStrength, ESM::Weapon::AT_Chop, mAttackVictim, mAttackHitPos, mAttackSuccess);
     else if (action == "slash hit")
-        charClass.hit(mPtr, mAttackStrength, ESM::Weapon::AT_Slash);
+        charClass.hit(mPtr, mAttackStrength, ESM::Weapon::AT_Slash, mAttackVictim, mAttackHitPos, mAttackSuccess);
     else if (action == "thrust hit")
-        charClass.hit(mPtr, mAttackStrength, ESM::Weapon::AT_Thrust);
+        charClass.hit(mPtr, mAttackStrength, ESM::Weapon::AT_Thrust, mAttackVictim, mAttackHitPos, mAttackSuccess);
     else if (action == "hit")
     {
         if (groupname == "attack1" || groupname == "swimattack1")
-            charClass.hit(mPtr, mAttackStrength, ESM::Weapon::AT_Chop);
+            charClass.hit(mPtr, mAttackStrength, ESM::Weapon::AT_Chop, mAttackVictim, mAttackHitPos, mAttackSuccess);
         else if (groupname == "attack2" || groupname == "swimattack2")
-            charClass.hit(mPtr, mAttackStrength, ESM::Weapon::AT_Slash);
+            charClass.hit(mPtr, mAttackStrength, ESM::Weapon::AT_Slash, mAttackVictim, mAttackHitPos, mAttackSuccess);
         else if (groupname == "attack3" || groupname == "swimattack3")
-            charClass.hit(mPtr, mAttackStrength, ESM::Weapon::AT_Thrust);
+            charClass.hit(mPtr, mAttackStrength, ESM::Weapon::AT_Thrust, mAttackVictim, mAttackHitPos, mAttackSuccess);
         else
-            charClass.hit(mPtr, mAttackStrength);
+            charClass.hit(mPtr, mAttackStrength, -1, mAttackVictim, mAttackHitPos, mAttackSuccess);
     }
     else if (isRandomAttackAnimation(groupname) && action == "start")
     {
@@ -1036,11 +1036,11 @@ void CharacterController::handleTextKey(std::string_view groupname, SceneUtil::T
         if (!hasHitKey)
         {
             if (groupname == "attack1" || groupname == "swimattack1")
-                charClass.hit(mPtr, mAttackStrength, ESM::Weapon::AT_Chop);
+                charClass.hit(mPtr, mAttackStrength, ESM::Weapon::AT_Chop, mAttackVictim, mAttackHitPos, mAttackSuccess);
             else if (groupname == "attack2" || groupname == "swimattack2")
-                charClass.hit(mPtr, mAttackStrength, ESM::Weapon::AT_Slash);
+                charClass.hit(mPtr, mAttackStrength, ESM::Weapon::AT_Slash, mAttackVictim, mAttackHitPos, mAttackSuccess);
             else if (groupname == "attack3" || groupname == "swimattack3")
-                charClass.hit(mPtr, mAttackStrength, ESM::Weapon::AT_Thrust);
+                charClass.hit(mPtr, mAttackStrength, ESM::Weapon::AT_Thrust, mAttackVictim, mAttackHitPos, mAttackSuccess);
         }
     }
     else if (action == "shoot attach")
@@ -1544,6 +1544,13 @@ bool CharacterController::updateWeaponState()
                                  weapSpeed, startKey, stopKey,
                                  0.0f, 0);
                 mUpperBodyState = UpperBodyState::AttackWindUp;
+
+                // Reset the attack results when the attack starts.
+                // Strictly speaking this should probably be done when the attack ends,
+                // but the attack animation might be cancelled in a myriad different ways.
+                mAttackSuccess = false;
+                mAttackVictim = MWWorld::Ptr();
+                mAttackHitPos = osg::Vec3f();
             }
         }
 
@@ -1583,7 +1590,13 @@ bool CharacterController::updateWeaponState()
             mAttackStrength = calculateWindUp();
             if (mAttackStrength == -1.f)
                 mAttackStrength = std::min(1.f, 0.1f + Misc::Rng::rollClosedProbability(prng));
-            playSwishSound();
+            if (weapclass != ESM::WeaponType::Ranged && weapclass != ESM::WeaponType::Thrown)
+            {
+                mAttackSuccess = cls.evaluateHit(mPtr, mAttackVictim, mAttackHitPos);
+                if (!mAttackSuccess)
+                    mAttackStrength = 0.f;
+                playSwishSound();
+            }
         }
 
         if (mWeaponType == ESM::Weapon::PickProbe || isRandomAttackAnimation(mCurrentWeapon))
@@ -2706,10 +2719,6 @@ void CharacterController::setHeadTrackTarget(const MWWorld::ConstPtr &target)
 
 void CharacterController::playSwishSound() const
 {
-    ESM::WeaponType::Class weapclass = getWeaponType(mWeaponType)->mWeaponClass;
-    if (weapclass == ESM::WeaponType::Ranged || weapclass == ESM::WeaponType::Thrown)
-        return;
-
     std::string soundId = "Weapon Swish";
     float volume = 0.98f + mAttackStrength * 0.02f;
     float pitch = 0.75f + mAttackStrength * 0.4f;

apps/openmw/mwmechanics/character.hpp

@@ -164,6 +164,9 @@ class CharacterController : public MWRender::Animation::TextKeyListener
     std::string mCurrentWeapon;
 
     float mAttackStrength{0.f};
+    MWWorld::Ptr mAttackVictim;
+    osg::Vec3f mAttackHitPos;
+    bool mAttackSuccess{false};
 
     bool mSkipAnim{false};
 

apps/openmw/mwworld/class.cpp

@@ -100,7 +100,12 @@ namespace MWWorld
         throw std::runtime_error ("class does not have item health");
     }
 
-    void Class::hit(const Ptr& ptr, float attackStrength, int type) const
+    bool Class::evaluateHit(const Ptr& ptr, Ptr& victim, osg::Vec3f& hitPosition) const
+    {
+        throw std::runtime_error("class cannot hit");
+    }
+
+    void Class::hit(const Ptr& ptr, float attackStrength, int type, const Ptr& victim, const osg::Vec3f& hitPosition, bool success) const
     {
         throw std::runtime_error("class cannot hit");
     }

apps/openmw/mwworld/class.hpp

@@ -123,9 +123,12 @@ namespace MWWorld
             ///< Return item max health or throw an exception, if class does not have item health
             /// (default implementation: throw an exception)
 
-            virtual void hit(const Ptr& ptr, float attackStrength, int type=-1) const;
-            ///< Execute a melee hit, using the current weapon. This will check the relevant skills
-            /// of the given attacker, and whoever is hit.
+            virtual bool evaluateHit(const Ptr& ptr, Ptr& victim, osg::Vec3f& hitPosition) const;
+            ///< Evaluate the victim of a melee hit produced by ptr in the current circumstances and return dice roll success.
+            /// (default implementation: throw an exception)
+
+            virtual void hit(const Ptr& ptr, float attackStrength, int type=-1, const Ptr& victim = Ptr(), const osg::Vec3f& hitPosition = osg::Vec3f(), bool success = false) const;
+            ///< Execute a melee hit on the victim at hitPosition, using the current weapon. If the hit was successful, apply damage and process corresponding events.
             /// \param attackStrength how long the attack was charged for, a value in 0-1 range.
             /// \param type - type of attack, one of the MWMechanics::CreatureStats::AttackType
             ///               enums. ignored for creature attacks.