fix: Fixes spell mechanics and misc bugs (#1118)

- [X] Fixes NPE from account tags.
- [X] Fixes bad skill check due to missing cast to double.
- [X] Fixes water elemental duration.
- [X] Standardizes spell summon duration by expansion.

Projects/Server.Tests/Tests/Network/Packets/Outgoing/MobilePackets.cs

@@ -540,7 +540,7 @@ namespace Server.Tests.Network
 
                 var layer = (byte)item.Layer;
 
-                if (!item.Deleted && beholder.CanSee(item) && m_DupedLayers[layer] != m_Version)
+                if (!item.Deleted && beholder.CanSee(item) && m_DupedLayers![layer] != m_Version)
                 {
                     m_DupedLayers[layer] = m_Version;
 
@@ -572,7 +572,7 @@ namespace Server.Tests.Network
 
             if (beheld.HairItemID > 0)
             {
-                if (m_DupedLayers[(int)Layer.Hair] != m_Version)
+                if (m_DupedLayers![(int)Layer.Hair] != m_Version)
                 {
                     m_DupedLayers[(int)Layer.Hair] = m_Version;
                     hue = beheld.HairHue;
@@ -603,7 +603,7 @@ namespace Server.Tests.Network
 
             if (beheld.FacialHairItemID > 0)
             {
-                if (m_DupedLayers[(int)Layer.FacialHair] != m_Version)
+                if (m_DupedLayers![(int)Layer.FacialHair] != m_Version)
                 {
                     m_DupedLayers[(int)Layer.FacialHair] = m_Version;
                     hue = beheld.FacialHairHue;

Projects/Server/Buffers/ValueStringBuilderExtensions.cs

@@ -21,6 +21,7 @@ public static class ValueStringBuilderExtensions
 {
     // Compiler generated
     public static void Append(
+        // ReSharper disable once RedundantAssignment
         this ref ValueStringBuilder stringBuilder,
         [InterpolatedStringHandlerArgument("stringBuilder")]
         ref ValueStringBuilder.AppendInterpolatedStringHandler handler)

Projects/UOContent/Accounting/Account.cs

@@ -635,6 +635,11 @@ namespace Server.Accounting
         /// <param name="name">Tag name to remove.</param>
         public void RemoveTag(string name)
         {
+            if (_tags == null)
+            {
+                return;
+            }
+
             for (var i = _tags.Count - 1; i >= 0; --i)
             {
                 if (i >= _tags.Count)
@@ -646,7 +651,7 @@ namespace Server.Accounting
 
                 if (tag.Name == name)
                 {
-                    _tags?.RemoveAt(i);
+                    _tags.RemoveAt(i);
                     this.MarkDirty();
                 }
             }

Projects/UOContent/Engines/Craft/Core/CraftSystem.cs

@@ -1,6 +1,5 @@
 using System;
 using System.Collections.Generic;
-using System.Diagnostics;
 using Server.Items;
 
 namespace Server.Engines.Craft

Projects/UOContent/Gumps/AdminGump.cs

@@ -1329,9 +1329,7 @@ namespace Server.Gumps
 
                                 var loginList = acct.LoginIPs;
 
-                                var contains = false;
+                                for (var i = 0; i < loginList.Length; ++i)
-
-                                for (var i = 0; !contains && i < loginList.Length; ++i)
                                 {
                                     if (firewallEntry.IsBlocked(loginList[i]))
                                     {
@@ -2653,7 +2651,7 @@ namespace Server.Gumps
                                 {
                                     index -= 2;
 
-                                    if (m_List != null && index >= 0 && index < m_List.Count)
+                                    if (index < m_List?.Count)
                                     {
                                         if (m_List[index] is not NetState ns)
                                         {

Projects/UOContent/Items/New Haven Quest Rewards/HammerOfHephaestus.cs

@@ -33,8 +33,8 @@ namespace Server.Items
 
         public override void OnDoubleClick(Mobile from)
         {
-            if (!IsChildOf(from.Backpack) && Parent != from
+            // TODO: These checks don't match EA, but they match BaseTool for now
-            )                                       // TODO: These checks don't match EA, but they match BaseTool for now
+            if (!IsChildOf(from.Backpack) && Parent != from)
             {
                 from.SendLocalizedMessage(1042001); // That must be in your pack for you to use it.
             }

Projects/UOContent/Items/Weapons/Abilities/ForceArrow.cs

@@ -61,7 +61,7 @@ namespace Server.Items
             }
             else
             {
-                _table.Add(attacker, new List<ForceArrowInfo>() { info });
+                _table.Add(attacker, new List<ForceArrowInfo> { info });
             }
 
             BuffInfo.AddBuff(defender, new BuffInfo(BuffIcon.ForceArrow, 1151285, 1151286, info.DefenseChanceMalus.ToString()));

Projects/UOContent/Items/Weapons/Abilities/WeaponAbilityPackets.cs

@@ -11,7 +11,7 @@ namespace Server.Items
             IncomingPackets.RegisterEncoded(0x19, true, &SetAbility);
         }
 
-        public static unsafe void SetAbility(NetState state, IEntity e, EncodedReader reader)
+        public static void SetAbility(NetState state, IEntity e, EncodedReader reader)
         {
             var m = state.Mobile;
             var index = reader.ReadInt32();

Projects/UOContent/Misc/SkillCheck.cs

@@ -282,7 +282,7 @@ public static class SkillCheck
 
             var skills = from.Skills;
 
-            if (from.Player && skills.Total / skills.Cap >= Utility.RandomDouble())
+            if (from.Player && skills.Total / (double)skills.Cap >= Utility.RandomDouble())
             {
                 for (var i = 0; i < skills.Length; ++i)
                 {

Projects/UOContent/Mobiles/Animals/Mounts/Kirin.cs

@@ -76,22 +76,17 @@ namespace Server.Mobiles
                 return false;
             }
 
-            if (Rider.Hits - damage < 30 && Rider.Map == attacker.Map && Rider.InRange(attacker, 18)
+            // Range and map checked here instead of other base fuction because of abiliites that don't need to check this
-            ) // Range and map checked here instead of other base fuction because of abiliites that don't need to check this
+            if (Rider.Hits - damage < 30 && Rider.Map == attacker.Map && Rider.InRange(attacker, 18))
             {
                 attacker.BoltEffect(0);
                 // 35~100 damage, unresistable, by the Ki-rin.
-                attacker.Damage(
+                // Don't inform mount about this damage, Still unsure wether or not it's flagged as the mount doing damage or the player.
-                    Utility.RandomMinMax(35, 100),
+                // If changed to player, without the extra bool it'd be an infinite loop
-                    this,
+                attacker.Damage(Utility.RandomMinMax(35, 100), this, false);
-                    false
-                ); // Don't inform mount about this damage, Still unsure wether or not it's flagged as the mount doing damage or the player.  If changed to player, without the extra bool it'd be an infinite loop
 
-                Rider.LocalOverheadMessage(
+                // Your mount calls down the forces of nature on your opponent.
-                    MessageType.Regular,
+                Rider.LocalOverheadMessage(MessageType.Regular, 0x3B2, 1042534);
-                    0x3B2,
-                    1042534
-                ); // Your mount calls down the forces of nature on your opponent.
                 Rider.FixedParticles(0, 0, 0, 0x13A7, EffectLayer.Waist);
                 Rider.PlaySound(0xA9); // Ki-rin's whinny.
                 return true;

Projects/UOContent/Mobiles/BaseCreature.cs

@@ -1881,7 +1881,7 @@ namespace Server.Mobiles
                 if (_summoned)
                 {
                     SummonEnd = reader.ReadDeltaTime();
-                    new UnsummonTimer(m_ControlMaster, this, SummonEnd - Core.Now).Start();
+                    new UnsummonTimer(this, SummonEnd - Core.Now).Start();
                 }
 
                 ControlSlots = reader.ReadInt();
@@ -3443,7 +3443,7 @@ namespace Server.Mobiles
                 }
             }
 
-            new UnsummonTimer(caster, creature, duration).Start();
+            new UnsummonTimer(creature, duration).Start();
             creature.SummonEnd = Core.Now + duration;
 
             creature.MoveToWorld(p, caster.Map);

Projects/UOContent/Mobiles/Healers/BaseHealer.cs

@@ -135,6 +135,7 @@ namespace Server.Mobiles
             }
         }
 
+        // ReSharper disable once RedundantOverriddenMember
         public override void Serialize(IGenericWriter writer)
         {
             base.Serialize(writer);

Projects/UOContent/Mobiles/Monsters/Humanoid/Melee/RestlessSoul.cs

@@ -79,8 +79,8 @@ namespace Server.Mobiles
 
         public override bool IsEnemy(Mobile m)
         {
-            if (m is PlayerMobile player && Map == Map.Trammel && X >= 5199 && X <= 5271 && Y >= 1812 && Y <= 1865
+            // Schmendrick's cave
-            ) // Schmendrick's cave
+            if (m is PlayerMobile player && Map == Map.Trammel && X is >= 5199 and <= 5271 && Y is >= 1812 and <= 1865)
             {
                 var qs = player.Quest;
 

Projects/UOContent/Multis/Houses/HouseSign.cs

@@ -163,7 +163,7 @@ namespace Server.Multis
             if (m.AccessLevel < AccessLevel.GameMaster && Owner.Owner == null &&
                 Owner.DecayLevel != DecayLevel.DemolitionPending)
             {
-                var canClaim = Owner?.CoOwners.Count > 0 && Owner.IsCoOwner(m) || Owner.IsFriend(m);
+                var canClaim = Owner.IsCoOwner(m) || Owner.IsFriend(m);
 
                 if (canClaim && !BaseHouse.HasAccountHouse(m))
                 {

Projects/UOContent/Multis/Houses/PreviewHouse.cs

@@ -21,7 +21,7 @@ namespace Server.Multis
 
                 if (entry.Flags == 0)
                 {
-                    Item item = new Static((int)entry.ItemId);
+                    Item item = new Static(entry.ItemId);
 
                     item.MoveToWorld(new Point3D(X + entry.OffsetX, Y + entry.OffsetY, Z + entry.OffsetZ), Map);
 

Projects/UOContent/Skills/Stealing.cs

@@ -154,9 +154,8 @@ namespace Server.SkillHandlers
                         }
                         else if (pl.IsLeaving)
                         {
-                            m_Thief.SendLocalizedMessage(
+                            // You are currently quitting a faction and cannot steal the town sigil
-                                1005589
+                            m_Thief.SendLocalizedMessage(1005589);
-                            ); // You are currently quitting a faction and cannot steal the town sigil
                         }
                         else if (sig.IsBeingCorrupted && sig.LastMonolith.Faction == faction)
                         {
@@ -170,15 +169,13 @@ namespace Server.SkillHandlers
                         {
                             if (Sigil.ExistsOn(m_Thief))
                             {
-                                m_Thief.SendLocalizedMessage(
+                                // The sigil has gone back to its home location because you already have a sigil.
-                                    1010258
+                                m_Thief.SendLocalizedMessage(1010258);
-                                ); // The sigil has gone back to its home location because you already have a sigil.
                             }
-                            else if (m_Thief?.Backpack.CheckHold(m_Thief, sig, false, true) != true)
+                            else if (m_Thief.Backpack?.CheckHold(m_Thief, sig, false, true) != true)
                             {
-                                m_Thief.SendLocalizedMessage(
+                                // The sigil has gone home because your backpack is full
-                                    1010259
+                                m_Thief.SendLocalizedMessage(1010259);
-                                ); // The sigil has gone home because your backpack is full
                             }
                             else
                             {
@@ -226,11 +223,8 @@ namespace Server.SkillHandlers
                 }
                 else if (si != null && m_Thief.Skills.Stealing.Value < 100.0)
                 {
-                    m_Thief.SendLocalizedMessage(
+                    // You're not skilled enough to attempt the theft of this item.
-                        1060025,
+                    m_Thief.SendLocalizedMessage(1060025, "", 0x66D);
-                        "",
-                        0x66D
-                    ); // You're not skilled enough to attempt the theft of this item.
                 }
                 else if (toSteal.Parent is Mobile)
                 {

Projects/UOContent/Spells/Eighth/AirElemental.cs

@@ -42,7 +42,12 @@ namespace Server.Spells.Eighth
         {
             if (CheckSequence())
             {
-                var duration = TimeSpan.FromSeconds(2 * Caster.Skills.Magery.Fixed / 5.0);
+                var duration = Core.Expansion switch
+                {
+                    Expansion.None => TimeSpan.FromSeconds(Caster.Skills.Magery.Value),
+                    // T2A -> Current
+                    _ => TimeSpan.FromSeconds(4 * Math.Min(5, Caster.Skills.Magery.Value)),
+                };
 
                 if (Core.AOS)
                 {

Projects/UOContent/Spells/Eighth/EarthElemental.cs

@@ -42,7 +42,12 @@ namespace Server.Spells.Eighth
         {
             if (CheckSequence())
             {
-                var duration = TimeSpan.FromSeconds(2 * Caster.Skills.Magery.Fixed / 5.0);
+                var duration = Core.Expansion switch
+                {
+                    Expansion.None => TimeSpan.FromSeconds(Caster.Skills.Magery.Value),
+                    // T2A -> Current
+                    _ => TimeSpan.FromSeconds(4 * Math.Min(5, Caster.Skills.Magery.Value)),
+                };
 
                 if (Core.AOS)
                 {

Projects/UOContent/Spells/Eighth/EnergyVortex.cs

@@ -35,16 +35,8 @@ namespace Server.Spells.Eighth
             }
             else if (SpellHelper.CheckTown(p, Caster) && CheckSequence())
             {
-                TimeSpan duration;
+                // TODO: Check Demo for pre-T2A.
-
+                var duration = Core.T2A ? TimeSpan.FromSeconds(90.0) : TimeSpan.FromSeconds(Utility.Random(80, 40));
-                if (Core.AOS)
-                {
-                    duration = TimeSpan.FromSeconds(90.0);
-                }
-                else
-                {
-                    duration = TimeSpan.FromSeconds(Utility.Random(80, 40));
-                }
 
                 BaseCreature.Summon(new EnergyVortex(), false, Caster, new Point3D(p), 0x212, duration);
             }

Projects/UOContent/Spells/Eighth/FireElemental.cs

@@ -43,7 +43,12 @@ namespace Server.Spells.Eighth
         {
             if (CheckSequence())
             {
-                var duration = TimeSpan.FromSeconds(2 * Caster.Skills.Magery.Fixed / 5);
+                var duration = Core.Expansion switch
+                {
+                    Expansion.None => TimeSpan.FromSeconds(Caster.Skills.Magery.Value),
+                    // T2A -> Current
+                    _ => TimeSpan.FromSeconds(4 * Math.Min(5, Caster.Skills.Magery.Value)),
+                };
 
                 if (Core.AOS)
                 {

Projects/UOContent/Spells/Eighth/SummonDaemon.cs

@@ -43,7 +43,12 @@ namespace Server.Spells.Eighth
         {
             if (CheckSequence())
             {
-                var duration = TimeSpan.FromSeconds(2 * Caster.Skills.Magery.Fixed / 5);
+                var duration = Core.Expansion switch
+                {
+                    Expansion.None => TimeSpan.FromSeconds(Caster.Skills.Magery.Value),
+                    // T2A -> Current
+                    _ => TimeSpan.FromSeconds(4 * Math.Min(5, Caster.Skills.Magery.Value)),
+                };
 
                 if (Core.AOS) /* Why two diff daemons? TODO: solve this */
                 {

Projects/UOContent/Spells/Eighth/WaterElemental.cs

@@ -42,7 +42,12 @@ namespace Server.Spells.Eighth
         {
             if (CheckSequence())
             {
-                var duration = TimeSpan.FromSeconds(2 * Caster.Skills.Magery.Fixed / 5);
+                var duration = Core.Expansion switch
+                {
+                    Expansion.None => TimeSpan.FromSeconds(Caster.Skills.Magery.Value),
+                    // T2A -> Current
+                    _  => TimeSpan.FromSeconds(4 * Math.Min(5, Caster.Skills.Magery.Value)),
+                };
 
                 if (Core.AOS)
                 {

Projects/UOContent/Spells/Ninjitsu/MirrorImage.cs

@@ -170,7 +170,7 @@ namespace Server.Mobiles
 
             var duration = TimeSpan.FromSeconds(30 + caster.Skills.Ninjitsu.Fixed / 40);
 
-            new UnsummonTimer(caster, this, duration).Start();
+            new UnsummonTimer(this, duration).Start();
             SummonEnd = Core.Now + duration;
 
             MirrorImage.AddClone(m_Caster);

Projects/UOContent/Spells/UnsummonTimer.cs

@@ -6,11 +6,9 @@ namespace Server.Spells
     internal class UnsummonTimer : Timer
     {
         private readonly BaseCreature m_Creature;
-        private Mobile m_Caster;
 
-        public UnsummonTimer(Mobile caster, BaseCreature creature, TimeSpan delay) : base(delay)
+        public UnsummonTimer(BaseCreature creature, TimeSpan delay) : base(delay)
         {
-            m_Caster = caster;
             m_Creature = creature;
         }