fix: Fixes double calls with Target cancel and spell sequences (#1840)

### Summary - Cleans up target cancellation being called incorrectly. - An invalid target type (which should never happen), now calls `OnTargetUntargetable` instead of `OnTargetCanceled` - Removes double calls to `FinishSequence` in Spells.
modernuo · Jun 17, 2024 · b8ad5c6 · b8ad5c6
1 parent 9c0b572
commit b8ad5c6
Show file tree

Hide file tree

Showing 80 changed files with 193 additions and 398 deletions.
diff --git a/Projects/Server/Targeting/Target.cs b/Projects/Server/Targeting/Target.cs
@@ -38,7 +38,7 @@ protected Target(int range, bool allowGround, TargetFlags flags)
     public static void Cancel(Mobile m)
     {
         m.NetState.SendCancelTarget();
-        m.Target?.OnTargetCancel(m, TargetCancelType.Canceled);
+        m.Target?.Cancel(m, TargetCancelType.Canceled);
     }
 
     public void BeginTimeout(Mobile from, long delay)
@@ -57,15 +57,14 @@ public void CancelTimeout()
         m_TimeoutTimer = null;
     }
 
-    public void Timeout(Mobile from)
+    public void Timeout(Mobile m)
     {
+        m.NetState.SendCancelTarget();
         CancelTimeout();
-        from.ClearTarget();
+        m.ClearTarget();
 
-        Cancel(from);
-
-        OnTargetCancel(from, TargetCancelType.Timeout);
-        OnTargetFinish(from);
+        OnTargetCancel(m, TargetCancelType.Timeout);
+        OnTargetFinish(m);
     }
 
     public virtual void SendTargetTo(NetState ns) => ns.SendTargetReq(this);
@@ -84,7 +83,6 @@ protected virtual bool CanTarget(Mobile from, LandTarget landTarget, ref Point3D
         if (!AllowGround)
         {
             // We should actually never get here. If we do, it's probably a misbehaving client/macro.
-            OnTargetCancel(from, TargetCancelType.Canceled);
             return false;
         }
 
@@ -161,7 +159,7 @@ public void Invoke(Mobile from, object targeted)
         Map map = null;
         Item item = null;
         Mobile mobile = null;
-        bool isValidTargetType = true;
+        var isValidTargetType = true;
 
         bool valid = targeted switch
         {
@@ -176,13 +174,10 @@ public void Invoke(Mobile from, object targeted)
         {
             if (!isValidTargetType)
             {
-                OnTargetCancel(from, TargetCancelType.Canceled);
+                OnTargetUntargetable(from, targeted);
             }
-
-            OnTargetFinish(from);
         }
-
-        if (map == null || map != from.Map || Range >= 0 && !from.InRange(loc, Range))
+        else if (map == null || map != from.Map || Range >= 0 && !from.InRange(loc, Range))
         {
             OnTargetOutOfRange(from, targeted);
         }
@@ -206,7 +201,7 @@ public void Invoke(Mobile from, object targeted)
         {
             OnTargetUntargetable(from, targeted);
         }
-        else if (mobile?.CheckTarget(from, this, mobile) == false)
+        else if (mobile?.CheckTarget(from, this, targeted) == false)
         {
             OnTargetUntargetable(from, mobile);
         }
@@ -271,20 +266,20 @@ protected virtual void OnTargetFinish(Mobile from)
 
     private class TimeoutTimer : Timer
     {
-        private readonly Mobile m_Mobile;
-        private readonly Target m_Target;
+        private readonly Mobile _mobile;
+        private readonly Target _target;
 
         public TimeoutTimer(Target target, Mobile m, TimeSpan delay) : base(delay)
         {
-            m_Target = target;
-            m_Mobile = m;
+            _target = target;
+            _mobile = m;
         }
 
         protected override void OnTick()
         {
-            if (m_Mobile.Target == m_Target)
+            if (_mobile.Target == _target)
             {
-                m_Target.Timeout(m_Mobile);
+                _target.Timeout(_mobile);
             }
         }
     }

diff --git a/Projects/UOContent/Items/Skill Items/Tailor Items/Misc/Scissors.cs b/Projects/UOContent/Items/Skill Items/Tailor Items/Misc/Scissors.cs
@@ -59,19 +59,9 @@ protected override void OnTarget(Mobile from, object targeted)
                 // Didn't your parents ever tell you not to run with scissors in your hand?!
                 from.SendLocalizedMessage(1063305);
             }
-            else if (targeted is Item item && !item.Movable)
+            else if (targeted is Item item and IScissorable scissorable && (targeted is PlagueBeastInnard or PlagueBeastMutationCore || item.Movable))
             {
-                if (item is IScissorable obj && obj is PlagueBeastInnard or PlagueBeastMutationCore)
-                {
-                    if (CanScissor(from, obj) && obj.Scissor(from, m_Item))
-                    {
-                        from.PlaySound(0x248);
-                    }
-                }
-            }
-            else if (targeted is IScissorable obj)
-            {
-                if (CanScissor(from, obj) && obj.Scissor(from, m_Item))
+                if (CanScissor(from, scissorable) && scissorable.Scissor(from, m_Item))
                 {
                     from.PlaySound(0x248);
                 }
@@ -84,13 +74,15 @@ protected override void OnTarget(Mobile from, object targeted)
 
         protected override void OnNonlocalTarget(Mobile from, object targeted)
         {
-            if (targeted is not (IScissorable obj and (PlagueBeastInnard or PlagueBeastMutationCore)))
+            if (targeted is not PlagueBeastInnard and not PlagueBeastMutationCore)
             {
                 base.OnNonlocalTarget(from, targeted);
                 return;
             }
 
-            if (CanScissor(from, obj) && obj.Scissor(from, m_Item))
+            var scissorable = (IScissorable)targeted;
+
+            if (CanScissor(from, scissorable) && scissorable.Scissor(from, m_Item))
             {
                 from.PlaySound(0x248);
             }

diff --git a/Projects/UOContent/Spells/Chivalry/CleanseByFire.cs b/Projects/UOContent/Spells/Chivalry/CleanseByFire.cs
@@ -98,8 +98,6 @@ public void Target(Mobile m)
 
                 AOS.Damage(Caster, Caster, damage, 0, 100, 0, 0, 0, true);
             }
-
-            FinishSequence();
         }
 
         public override bool CheckCast()

diff --git a/Projects/UOContent/Spells/Chivalry/CloseWounds.cs b/Projects/UOContent/Spells/Chivalry/CloseWounds.cs
@@ -78,8 +78,6 @@ public void Target(Mobile m)
                 m.FixedParticles(0x376A, 1, 62, 9923, 3, 3, EffectLayer.Waist);
                 m.FixedParticles(0x3779, 1, 46, 9502, 5, 3, EffectLayer.Waist);
             }
-
-            FinishSequence();
         }
 
         public override bool CheckCast()

diff --git a/Projects/UOContent/Spells/Chivalry/RemoveCurse.cs b/Projects/UOContent/Spells/Chivalry/RemoveCurse.cs
@@ -124,8 +124,6 @@ public void Target(Mobile m)
                     m.PlaySound(0x1DF);
                 }
             }
-
-            FinishSequence();
         }
 
         public override bool CheckCast()

diff --git a/Projects/UOContent/Spells/Chivalry/SacredJourney.cs b/Projects/UOContent/Spells/Chivalry/SacredJourney.cs
@@ -109,8 +109,6 @@ public void Effect(Point3D loc, Map map, bool checkMulti)
                 Caster.MoveToWorld(loc, map);
                 Caster.PlaySound(0x1FC);
             }
-
-            FinishSequence();
         }
 
         public override void OnCast()
@@ -122,6 +120,7 @@ public override void OnCast()
             else
             {
                 Effect(m_Entry.Location, m_Entry.Map, true);
+                FinishSequence();
             }
         }
 

diff --git a/Projects/UOContent/Spells/Eighth/EnergyVortex.cs b/Projects/UOContent/Spells/Eighth/EnergyVortex.cs
@@ -40,8 +40,6 @@ public void Target(IPoint3D p)
 
                 BaseCreature.Summon(new EnergyVortex(), false, Caster, new Point3D(p), 0x212, duration);
             }
-
-            FinishSequence();
         }
 
         public override bool CheckCast()

diff --git a/Projects/UOContent/Spells/Eighth/Resurrection.cs b/Projects/UOContent/Spells/Eighth/Resurrection.cs
@@ -64,8 +64,6 @@ public void Target(Mobile m)
                 m.CloseGump<ResurrectGump>();
                 m.SendGump(new ResurrectGump(m, Caster));
             }
-
-            FinishSequence();
         }
 
         public override bool CheckCast()

diff --git a/Projects/UOContent/Spells/Fifth/BladeSpirits.cs b/Projects/UOContent/Spells/Fifth/BladeSpirits.cs
@@ -37,8 +37,6 @@ public void Target(IPoint3D p)
                 var duration = TimeSpan.FromSeconds(Core.AOS ? 120 : Utility.Random(80, 40));
                 BaseCreature.Summon(new BladeSpirits(), false, Caster, new Point3D(p), 0x212, duration);
             }
-
-            FinishSequence();
         }
 
         public override TimeSpan GetCastDelay()

diff --git a/Projects/UOContent/Spells/Fifth/DispelField.cs b/Projects/UOContent/Spells/Fifth/DispelField.cs
@@ -47,8 +47,6 @@ public void Target(Item item)
 
                 item.Delete();
             }
-
-            FinishSequence();
         }
 
         public override void OnCast()

diff --git a/Projects/UOContent/Spells/Fifth/MagicReflect.cs b/Projects/UOContent/Spells/Fifth/MagicReflect.cs
@@ -99,8 +99,6 @@ public override void OnCast()
                         BuffInfo.AddBuff(Caster, new BuffInfo(BuffIcon.MagicReflection, 1075817, buffFormat, true));
                     }
                 }
-
-                FinishSequence();
             }
             else
             {
@@ -129,9 +127,9 @@ public override void OnCast()
                         Caster.SendLocalizedMessage(1005385); // The spell will not adhere to you at this time.
                     }
                 }
-
-                FinishSequence();
             }
+
+            FinishSequence();
         }
 
         public static void EndReflect(Mobile m)

diff --git a/Projects/UOContent/Spells/Fifth/MindBlast.cs b/Projects/UOContent/Spells/Fifth/MindBlast.cs
@@ -106,8 +106,6 @@ public void Target(Mobile m)
 
                 SpellHelper.Damage(this, target, damage, 0, 0, 100, 0, 0);
             }
-
-            FinishSequence();
         }
 
         public override void OnCast()

diff --git a/Projects/UOContent/Spells/Fifth/Paralyze.cs b/Projects/UOContent/Spells/Fifth/Paralyze.cs
@@ -78,8 +78,6 @@ public void Target(Mobile m)
 
                 HarmfulSpell(m);
             }
-
-            FinishSequence();
         }
 
         public override void OnCast()

diff --git a/Projects/UOContent/Spells/Fifth/PoisonField.cs b/Projects/UOContent/Spells/Fifth/PoisonField.cs
@@ -53,8 +53,6 @@ public void Target(IPoint3D p)
                 new PoisonField(itemID, targetLoc, Caster, Caster.Map, duration, i);
             }
         }
-
-        FinishSequence();
     }
 
     public override void OnCast()

diff --git a/Projects/UOContent/Spells/First/Clumsy.cs b/Projects/UOContent/Spells/First/Clumsy.cs
@@ -43,8 +43,6 @@ public void Target(Mobile m)
 
                 HarmfulSpell(m);
             }
-
-            FinishSequence();
         }
 
         public override void OnCast()

diff --git a/Projects/UOContent/Spells/First/Feeblemind.cs b/Projects/UOContent/Spells/First/Feeblemind.cs
@@ -45,8 +45,6 @@ public void Target(Mobile m)
 
                 HarmfulSpell(m);
             }
-
-            FinishSequence();
         }
 
         public override void OnCast()

diff --git a/Projects/UOContent/Spells/First/Heal.cs b/Projects/UOContent/Spells/First/Heal.cs
@@ -69,8 +69,6 @@ public void Target(Mobile m)
                 m.FixedParticles(0x376A, 9, 32, 5005, EffectLayer.Waist);
                 m.PlaySound(0x1F2);
             }
-
-            FinishSequence();
         }
 
         public override bool CheckCast()

diff --git a/Projects/UOContent/Spells/First/MagicArrow.cs b/Projects/UOContent/Spells/First/MagicArrow.cs
@@ -64,8 +64,6 @@ public void Target(Mobile m)
 
                 SpellHelper.Damage(this, m, damage, 0, 100, 0, 0, 0);
             }
-
-            FinishSequence();
         }
 
         public override void OnCast()