Merge branch 'im_walking_on_water_whoa_oh' into 'master'

Move target checking code so it applies to all sources

Closes #6486

See merge request OpenMW/openmw!1472

apps/openmw/mwmechanics/spellcasting.cpp

@@ -128,9 +128,6 @@ namespace MWMechanics
             }
             canCastAnEffect = true;
 
-            if (!checkEffectTarget(effectIt->mEffectID, target, caster, castByPlayer))
-                continue;
-
             // caster needs to be an actor for linked effects (e.g. Absorb)
             if (magicEffect->mData.mFlags & ESM::MagicEffect::CasterLinked
                     && (caster.isEmpty() || !caster.getClass().isActor()))

apps/openmw/mwmechanics/spelleffects.cpp

@@ -774,6 +774,45 @@ void applyMagicEffect(const MWWorld::Ptr& target, const MWWorld::Ptr& caster, co
     }
 }
 
+bool shouldRemoveEffect(const MWWorld::Ptr& target, const ESM::ActiveEffect& effect)
+{
+    const auto world = MWBase::Environment::get().getWorld();
+    switch(effect.mEffectId)
+    {
+        case ESM::MagicEffect::Levitate:
+        {
+            if(!world->isLevitationEnabled())
+            {
+                if(target == getPlayer())
+                    MWBase::Environment::get().getWindowManager()->messageBox("#{sLevitateDisabled}");
+                return true;
+            }
+            break;
+        }
+        case ESM::MagicEffect::Recall:
+        case ESM::MagicEffect::DivineIntervention:
+        case ESM::MagicEffect::AlmsiviIntervention:
+        {
+            return effect.mFlags & ESM::ActiveEffect::Flag_Applied;
+        }
+        case ESM::MagicEffect::WaterWalking:
+        {
+            if (target.getClass().isPureWaterCreature(target) && world->isSwimming(target))
+                return true;
+            if (effect.mFlags & ESM::ActiveEffect::Flag_Applied)
+                break;
+            if (!world->isWaterWalkingCastableOnTarget(target))
+            {
+                if(target == getPlayer())
+                    MWBase::Environment::get().getWindowManager()->messageBox("#{sMagicInvalidEffect}");
+                return true;
+            }
+            break;
+        }
+    }
+    return false;
+}
+
 MagicApplicationResult applyMagicEffect(const MWWorld::Ptr& target, const MWWorld::Ptr& caster, ActiveSpells::ActiveSpellParams& spellParams, ESM::ActiveEffect& effect, float dt)
 {
     const auto world = MWBase::Environment::get().getWorld();
@@ -792,21 +831,11 @@ MagicApplicationResult applyMagicEffect(const MWWorld::Ptr& target, const MWWorl
             MWBase::Environment::get().getWindowManager()->messageBox("#{sMagicCorprusWorsens}");
         return MagicApplicationResult::APPLIED;
     }
-    else if(effect.mEffectId == ESM::MagicEffect::Levitate && !world->isLevitationEnabled())
+    else if(shouldRemoveEffect(target, effect))
     {
-        if(target == getPlayer())
-            MWBase::Environment::get().getWindowManager()->messageBox ("#{sLevitateDisabled}");
         onMagicEffectRemoved(target, spellParams, effect);
         return MagicApplicationResult::REMOVED;
     }
-    else if(effect.mEffectId == ESM::MagicEffect::AlmsiviIntervention || effect.mEffectId == ESM::MagicEffect::DivineIntervention || effect.mEffectId == ESM::MagicEffect::Recall)
-    {
-        if(effect.mFlags & ESM::ActiveEffect::Flag_Applied)
-        {
-            onMagicEffectRemoved(target, spellParams, effect);
-            return MagicApplicationResult::REMOVED;
-        }
-    }
     const auto* magicEffect = world->getStore().get<ESM::MagicEffect>().find(effect.mEffectId);
     if(effect.mFlags & ESM::ActiveEffect::Flag_Applied)
     {
@@ -883,8 +912,13 @@ MagicApplicationResult applyMagicEffect(const MWWorld::Ptr& target, const MWWorl
         float oldMagnitude = 0.f;
         if(effect.mFlags & ESM::ActiveEffect::Flag_Applied)
             oldMagnitude = effect.mMagnitude;
-        else if(spellParams.getType() == ESM::ActiveSpells::Type_Consumable || spellParams.getType() == ESM::ActiveSpells::Type_Temporary)
-            playEffects(target, *magicEffect);
+        else
+        {
+            if(spellParams.getType() == ESM::ActiveSpells::Type_Consumable || spellParams.getType() == ESM::ActiveSpells::Type_Temporary)
+                playEffects(target, *magicEffect);
+            if(effect.mEffectId == ESM::MagicEffect::Soultrap && !target.getClass().isNpc() && target.getType() == ESM::Creature::sRecordId && target.get<ESM::Creature>()->mBase->mData.mSoul == 0 && caster == getPlayer())
+                MWBase::Environment::get().getWindowManager()->messageBox("#{sMagicInvalidTarget}");
+        }
         float magnitude = roll(effect);
         //Note that there's an early out for Flag_Applied AppliedOnce effects so we don't have to exclude them here
         effect.mMagnitude = magnitude;

apps/openmw/mwmechanics/spellutil.cpp

@@ -199,48 +199,4 @@ namespace MWMechanics
         const auto spell = MWBase::Environment::get().getWorld()->getStore().get<ESM::Spell>().search(spellId);
         return spell && spellIncreasesSkill(spell);
     }
-
-    bool checkEffectTarget (int effectId, const MWWorld::Ptr& target, const MWWorld::Ptr& caster, bool castByPlayer)
-    {
-        switch (effectId)
-        {
-            case ESM::MagicEffect::Levitate:
-            {
-                if (!MWBase::Environment::get().getWorld()->isLevitationEnabled())
-                {
-                    if (castByPlayer)
-                        MWBase::Environment::get().getWindowManager()->messageBox("#{sLevitateDisabled}");
-                    return false;
-                }
-                break;
-            }
-            case ESM::MagicEffect::Soultrap:
-            {
-                if (!target.getClass().isNpc() // no messagebox for NPCs
-                     && (target.getType() == ESM::Creature::sRecordId && target.get<ESM::Creature>()->mBase->mData.mSoul == 0))
-                {
-                    if (castByPlayer)
-                        MWBase::Environment::get().getWindowManager()->messageBox("#{sMagicInvalidTarget}");
-                    return true; // must still apply to get visual effect and have target regard it as attack
-                }
-                break;
-            }
-            case ESM::MagicEffect::WaterWalking:
-            {
-                if (target.getClass().isPureWaterCreature(target) && MWBase::Environment::get().getWorld()->isSwimming(target))
-                    return false;
-
-                MWBase::World *world = MWBase::Environment::get().getWorld();
-
-                if (!world->isWaterWalkingCastableOnTarget(target))
-                {
-                    if (castByPlayer && caster == target)
-                        MWBase::Environment::get().getWindowManager()->messageBox ("#{sMagicInvalidEffect}");
-                    return false;
-                }
-                break;
-            }
-        }
-        return true;
-    }
 }

apps/openmw/mwmechanics/spellutil.hpp

@@ -48,9 +48,6 @@ namespace MWMechanics
     /// Get whether or not the given spell contributes to skill progress.
     bool spellIncreasesSkill(const ESM::Spell* spell);
     bool spellIncreasesSkill(const std::string& spellId);
-
-    /// Check if the given effect can be applied to the target. If \a castByPlayer, emits a message box on failure.
-    bool checkEffectTarget (int effectId, const MWWorld::Ptr& target, const MWWorld::Ptr& caster, bool castByPlayer);
 }
 
 #endif