Spellcasting timing fixes (bug #4227)

Play spellcasting animation and VFX (but not hand VFX) if spellcasting failed due to insufficient magicka
Apply spellcasting fatigue loss when the spellcasting starts instead of when the spell is applied

CHANGELOG.md

@@ -14,6 +14,7 @@
     Bug #3867: All followers attack player when one follower enters combat with player
     Bug #3905: Great House Dagoth issues
     Bug #4203: Resurrecting an actor doesn't close the loot GUI
+    Bug #4227: Spellcasting restrictions are checked before spellcasting animations are played
     Bug #4376: Moved actors don't respawn in their original cells
     Bug #4389: NPC's lips do not move if his head model has the NiBSAnimationNode root node
     Bug #4602: Robert's Bodies: crash inside createInstance()

apps/openmw/mwbase/world.hpp

@@ -17,6 +17,7 @@
 
 #include "../mwworld/ptr.hpp"
 #include "../mwworld/doorstate.hpp"
+#include "../mwworld/spellcaststate.hpp"
 
 #include "../mwrender/rendermode.hpp"
 
@@ -541,9 +542,9 @@ namespace MWBase
             /**
              * @brief startSpellCast attempt to start casting a spell. Might fail immediately if conditions are not met.
              * @param actor
-             * @return true if the spell can be casted (i.e. the animation should start)
+             * @return Success or the failure condition.
              */
-            virtual bool startSpellCast (const MWWorld::Ptr& actor) = 0;
+            virtual MWWorld::SpellCastState startSpellCast (const MWWorld::Ptr& actor) = 0;
 
             virtual void castSpell (const MWWorld::Ptr& actor, bool manualSpell=false) = 0;
 

apps/openmw/mwmechanics/character.cpp

@@ -42,6 +42,7 @@
 #include "../mwworld/inventorystore.hpp"
 #include "../mwworld/esmstore.hpp"
 #include "../mwworld/player.hpp"
+#include "../mwworld/spellcaststate.hpp"
 
 #include "aicombataction.hpp"
 #include "movement.hpp"
@@ -1052,8 +1053,10 @@ void CharacterController::handleTextKey(std::string_view groupname, SceneUtil::T
     // the same animation for all range types, so there are 3 "release" keys on the same time, one for each range type.
     else if (groupname == "spellcast" && action == mAttackType + " release")
     {
-        MWBase::Environment::get().getWorld()->castSpell(mPtr, mCastingManualSpell);
+        if (mCanCast)
+            MWBase::Environment::get().getWorld()->castSpell(mPtr, mCastingManualSpell);
         mCastingManualSpell = false;
+        mCanCast = false;
     }
     else if (groupname == "shield" && action == "block hit")
         charClass.block(mPtr);
@@ -1377,7 +1380,13 @@ bool CharacterController::updateState(CharacterState idle)
                 }
                 std::string spellid = stats.getSpells().getSelectedSpell();
                 bool isMagicItem = false;
-                bool canCast = mCastingManualSpell || world->startSpellCast(mPtr);
+
+                // Play hand VFX and allow castSpell use (assuming an animation is going to be played) if spellcasting is successful.
+                // Manual spellcasting bypasses restrictions.
+                MWWorld::SpellCastState spellCastResult = MWWorld::SpellCastState::Success;
+                if (!mCastingManualSpell)
+                    spellCastResult = world->startSpellCast(mPtr);
+                mCanCast = spellCastResult == MWWorld::SpellCastState::Success;
 
                 if (spellid.empty())
                 {
@@ -1402,7 +1411,9 @@ bool CharacterController::updateState(CharacterState idle)
                     resetIdle = false;
                     mUpperBodyState = UpperCharState_CastingSpell;
                 }
-                else if(!spellid.empty() && canCast)
+                // Play the spellcasting animation/VFX if the spellcasting was successful or failed due to insufficient magicka.
+                // Used up powers are exempt from this from some reason.
+                else if (!spellid.empty() && spellCastResult != MWWorld::SpellCastState::PowerAlreadyUsed)
                 {
                     world->breakInvisibility(mPtr);
                     MWMechanics::CastSpell cast(mPtr, nullptr, false, mCastingManualSpell);
@@ -1420,24 +1431,26 @@ bool CharacterController::updateState(CharacterState idle)
                         const ESM::Spell *spell = store.get<ESM::Spell>().find(spellid);
                         effects = spell->mEffects.mList;
                     }
-
+                    if (mCanCast)
-                    const ESM::MagicEffect *effect = store.get<ESM::MagicEffect>().find(effects.back().mEffectID); // use last effect of list for color of VFX_Hands
-
-                    const ESM::Static* castStatic = world->getStore().get<ESM::Static>().find ("VFX_Hands");
-
-                    const VFS::Manager* const vfs = MWBase::Environment::get().getResourceSystem()->getVFS();
-
-                    for (size_t iter = 0; iter < effects.size(); ++iter) // play hands vfx for each effect
                     {
-                        if (mAnimation->getNode("Bip01 L Hand"))
+                        const ESM::MagicEffect *effect = store.get<ESM::MagicEffect>().find(effects.back().mEffectID); // use last effect of list for color of VFX_Hands
-                            mAnimation->addEffect(
-                                Misc::ResourceHelpers::correctMeshPath(castStatic->mModel, vfs),
-                                -1, false, "Bip01 L Hand", effect->mParticle);
 
-                        if (mAnimation->getNode("Bip01 R Hand"))
+                        const ESM::Static* castStatic = world->getStore().get<ESM::Static>().find ("VFX_Hands");
-                            mAnimation->addEffect(
+
-                                Misc::ResourceHelpers::correctMeshPath(castStatic->mModel, vfs),
+                        const VFS::Manager* const vfs = MWBase::Environment::get().getResourceSystem()->getVFS();
-                                -1, false, "Bip01 R Hand", effect->mParticle);
+
+                        for (size_t iter = 0; iter < effects.size(); ++iter) // play hands vfx for each effect
+                        {
+                            if (mAnimation->getNode("Bip01 L Hand"))
+                                mAnimation->addEffect(
+                                    Misc::ResourceHelpers::correctMeshPath(castStatic->mModel, vfs),
+                                    -1, false, "Bip01 L Hand", effect->mParticle);
+
+                            if (mAnimation->getNode("Bip01 R Hand"))
+                                mAnimation->addEffect(
+                                    Misc::ResourceHelpers::correctMeshPath(castStatic->mModel, vfs),
+                                    -1, false, "Bip01 R Hand", effect->mParticle);
+                        }
                     }
 
                     const ESM::ENAMstruct &firstEffect = effects.at(0); // first effect used for casting animation
@@ -1448,8 +1461,10 @@ bool CharacterController::updateState(CharacterState idle)
                     {
                         startKey = "start";
                         stopKey = "stop";
-                        world->castSpell(mPtr, mCastingManualSpell); // No "release" text key to use, so cast immediately
+                        if (mCanCast)
+                            world->castSpell(mPtr, mCastingManualSpell); // No "release" text key to use, so cast immediately
                         mCastingManualSpell = false;
+                        mCanCast = false;
                     }
                     else
                     {
@@ -2547,6 +2562,7 @@ void CharacterController::forceStateUpdate()
 
     // Make sure we canceled the current attack or spellcasting,
     // because we disabled attack animations anyway.
+    mCanCast = false;
     mCastingManualSpell = false;
     setAttackingOrSpell(false);
     if (mUpperBodyState != UpperCharState_Nothing)

apps/openmw/mwmechanics/character.hpp

@@ -178,6 +178,8 @@ class CharacterController : public MWRender::Animation::TextKeyListener
 
     std::string mAttackType; // slash, chop or thrust
 
+    bool mCanCast{false};
+
     bool mCastingManualSpell{false};
 
     bool mIsMovingBackward{false};

apps/openmw/mwmechanics/spellcasting.cpp

@@ -313,8 +313,6 @@ namespace MWMechanics
         mSourceName = spell->mName;
         mId = spell->mId;
 
-        const MWWorld::ESMStore& store = MWBase::Environment::get().getWorld()->getStore();
-
         int school = 0;
 
         bool godmode = mCaster == MWMechanics::getPlayer() && MWBase::Environment::get().getWorld()->getGodModeState();
@@ -327,16 +325,6 @@ namespace MWMechanics
 
             if (!godmode)
             {
-                // Reduce fatigue (note that in the vanilla game, both GMSTs are 0, and there's no fatigue loss)
-                static const float fFatigueSpellBase = store.get<ESM::GameSetting>().find("fFatigueSpellBase")->mValue.getFloat();
-                static const float fFatigueSpellMult = store.get<ESM::GameSetting>().find("fFatigueSpellMult")->mValue.getFloat();
-                DynamicStat<float> fatigue = stats.getFatigue();
-                const float normalizedEncumbrance = mCaster.getClass().getNormalizedEncumbrance(mCaster);
-
-                float fatigueLoss = MWMechanics::calcSpellCost(*spell) * (fFatigueSpellBase + normalizedEncumbrance * fFatigueSpellMult);
-                fatigue.setCurrent(fatigue.getCurrent() - fatigueLoss); 
-                stats.setFatigue(fatigue);
-
                 bool fail = false;
 
                 // Check success

apps/openmw/mwworld/spellcaststate.hpp

@@ -0,0 +1,14 @@
+#ifndef GAME_MWWORLD_SPELLCASTSTATE_H
+#define GAME_MWWORLD_SPELLCASTSTATE_H
+
+namespace MWWorld
+{
+    enum class SpellCastState
+    {
+        Success = 0,
+        InsufficientMagicka = 1,
+        PowerAlreadyUsed = 2
+    };
+}
+
+#endif

apps/openmw/mwworld/worldimp.cpp

@@ -2969,12 +2969,12 @@ namespace MWWorld
         mGroundcoverStore.init(mStore.get<ESM::Static>(), fileCollections, groundcoverFiles, encoder, listener);
     }
 
-    bool World::startSpellCast(const Ptr &actor)
+    MWWorld::SpellCastState World::startSpellCast(const Ptr &actor)
     {
         MWMechanics::CreatureStats& stats = actor.getClass().getCreatureStats(actor);
 
         std::string message;
-        bool fail = false;
+        MWWorld::SpellCastState result = MWWorld::SpellCastState::Success;
         bool isPlayer = (actor == getPlayerPtr());
 
         std::string selectedSpell = stats.getSpells().getSelectedSpell();
@@ -2990,28 +2990,38 @@ namespace MWWorld
             if (spellCost > 0 && magicka.getCurrent() < spellCost && !godmode)
             {
                 message = "#{sMagicInsufficientSP}";
-                fail = true;
+                result = MWWorld::SpellCastState::InsufficientMagicka;
             }
 
             // If this is a power, check if it was already used in the last 24h
-            if (!fail && spell->mData.mType == ESM::Spell::ST_Power && !stats.getSpells().canUsePower(spell))
+            if (result == MWWorld::SpellCastState::Success && spell->mData.mType == ESM::Spell::ST_Power && !stats.getSpells().canUsePower(spell))
             {
                 message = "#{sPowerAlreadyUsed}";
-                fail = true;
+                result = MWWorld::SpellCastState::PowerAlreadyUsed;
             }
 
-            // Reduce mana
+            if (result ==  MWWorld::SpellCastState::Success && !godmode)
-            if (!fail && !godmode)
             {
+                // Reduce mana
                 magicka.setCurrent(magicka.getCurrent() - spellCost);
                 stats.setMagicka(magicka);
+
+                // Reduce fatigue (note that in the vanilla game, both GMSTs are 0, and there's no fatigue loss)
+                static const float fFatigueSpellBase = mStore.get<ESM::GameSetting>().find("fFatigueSpellBase")->mValue.getFloat();
+                static const float fFatigueSpellMult = mStore.get<ESM::GameSetting>().find("fFatigueSpellMult")->mValue.getFloat();
+                MWMechanics::DynamicStat<float> fatigue = stats.getFatigue();
+                const float normalizedEncumbrance = actor.getClass().getNormalizedEncumbrance(actor);
+
+                float fatigueLoss = spellCost * (fFatigueSpellBase + normalizedEncumbrance * fFatigueSpellMult);
+                fatigue.setCurrent(fatigue.getCurrent() - fatigueLoss);
+                stats.setFatigue(fatigue);
             }
         }
 
-        if (isPlayer && fail)
+        if (isPlayer && result != MWWorld::SpellCastState::Success)
             MWBase::Environment::get().getWindowManager()->messageBox(message);
 
-        return !fail;
+        return result;
     }
 
     void World::castSpell(const Ptr &actor, bool manualSpell)

apps/openmw/mwworld/worldimp.hpp

@@ -639,9 +639,9 @@ namespace MWWorld
             /**
              * @brief startSpellCast attempt to start casting a spell. Might fail immediately if conditions are not met.
              * @param actor
-             * @return true if the spell can be casted (i.e. the animation should start)
+             * @return Success or the failure condition.
              */
-            bool startSpellCast (const MWWorld::Ptr& actor) override;
+            MWWorld::SpellCastState startSpellCast (const MWWorld::Ptr& actor) override;
 
             /**
              * @brief Cast the actual spell, should be called mid-animation