From e52d54b7dacaadca3f4051e6a741c28ce19df1ee Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Tue, 1 Sep 2026 23:25:14 -0700 Subject: [PATCH] perf: keep damage entries in an inline intrusive list (#2605) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary `Mobile.DamageEntries` was a `List` allocated for every mobile, including the ~99% that never take damage. It is now an inline `ValueLinkList` (24 bytes in the `Mobile` object, no separate allocation) ordered least recent → most recent. - `DamageEntry` implements `IValueLinkListNode`. - `RegisterDamage` moves the entry to the tail in O(1) instead of `Remove` + `Add` on a list. - Expired entries are always a head prefix, so pruning walks from the head and stops at the first live entry. The `DamageEntries` getter prunes on access. - `DamageEntries` is exposed as `ref readonly`; enumerate with `foreach` or `.ByDescending()`. Mutation goes through `RegisterDamage` / `ClearDamageEntries`. - `BaseCreature.GetLootingRights` and `BaseCreature.ComputeBonusDamage` take `in ValueLinkList`; all callers compile unchanged. Files that `foreach` over `DamageEntries` need `using Server.Collections;` for the enumerator extension. - RunUO migration docs (`dev-docs/runuo-migration-docs/09`, `11`) and the `migrate-items-mobiles` skill document the change. Saves one object and 16 bytes per mobile (~8 MB and 500k gen2 objects on a 500k world). Second of three PRs from the lazy per-mobile collections design (first: #2604). Branched from `main`; the two diffs touch disjoint hunks of `Mobile.cs`. ## Breaking change - `Mobile.DamageEntries` is no longer a `List`. Indexing, `.Clear()`, `.Add()`, `.Remove()` no longer compile; use `foreach`, `.ByDescending()`, `.Count`, `ClearDamageEntries()`, and `RegisterDamage`. Calling a `ValueLinkList` mutator on the `ref readonly` property compiles but operates on a copy while still unlinking the real nodes; do not. - `BaseCreature.GetLootingRights` and `BaseCreature.ComputeBonusDamage` signatures changed to `(in ValueLinkList, …)`. Save format is untouched: damage entries are not serialized. ## Behavior Recency order, `allowSelf`, tie-breaking in `FindMostTotal`/`FindLeastTotal` (most recent wins), `Responsible` accounting, and loot-rights ordering are unchanged and covered by the new `DamageEntryTests` and `LootingRightsTests`. ## Testing - `dotnet build -c Release` clean. - New `DamageEntryTests` and `LootingRightsTests` plus full `Server.Tests` and `UOContent.Tests`. --- .../Tests/Mobiles/DamageEntryTests.cs | 311 ++++++++++++++++++ Projects/Server/Mobiles/Mobile.cs | 174 +++++----- .../Tests/Mobiles/LootingRightsTests.cs | 146 ++++++++ .../Engines/CannedEvil/ChampionSpawn.cs | 6 +- Projects/UOContent/Mobiles/BaseCreature.cs | 25 +- .../UOContent/Mobiles/Special/Harrower.cs | 1 + .../migrate-items-mobiles.md | 1 + .../09-items-mobiles-creatures.md | 39 +++ .../runuo-migration-docs/11-api-reference.md | 3 + 9 files changed, 603 insertions(+), 103 deletions(-) create mode 100644 Projects/Server.Tests/Tests/Mobiles/DamageEntryTests.cs create mode 100644 Projects/UOContent.Tests/Tests/Mobiles/LootingRightsTests.cs diff --git a/Projects/Server.Tests/Tests/Mobiles/DamageEntryTests.cs b/Projects/Server.Tests/Tests/Mobiles/DamageEntryTests.cs new file mode 100644 index 000000000..9c3cdfd14 --- /dev/null +++ b/Projects/Server.Tests/Tests/Mobiles/DamageEntryTests.cs @@ -0,0 +1,311 @@ +using System; +using System.Collections.Generic; +using Server.Collections; +using Xunit; + +namespace Server.Tests; + +[Collection("Sequential Server Tests")] +public class DamageEntryTests +{ + private class TestMobile : Mobile + { + } + + private class PetMobile : Mobile + { + public Mobile Master { get; set; } + + public override Mobile GetDamageMaster(Mobile damagee) => Master; + } + + private static List Damagers(Mobile victim) + { + var result = new List(); + foreach (var de in victim.DamageEntries) + { + result.Add(de.Damager); + } + + return result; + } + + [Fact] + public void FreshMobile_HasNoEntries() + { + var m = new TestMobile(); + + try + { + Assert.Equal(0, m.DamageEntries.Count); + Assert.Null(m.FindMostRecentDamageEntry(true)); + Assert.Null(m.FindLeastRecentDamageEntry(true)); + Assert.Null(m.FindMostTotalDamageEntry(true)); + Assert.Null(m.FindLeastTotalDamageEntry(true)); + Assert.Null(m.FindDamageEntryFor(m)); + } + finally + { + m.Delete(); + } + } + + [Fact] + public void RegisterDamage_OrdersLeastRecentToMostRecent() + { + var victim = new TestMobile(); + var a = new TestMobile(); + var b = new TestMobile(); + + try + { + victim.RegisterDamage(10, a); + victim.RegisterDamage(20, b); + victim.RegisterDamage(5, a); // a becomes most recent again + + Assert.Equal(2, victim.DamageEntries.Count); + Assert.Equal(new[] { b, a }, Damagers(victim)); + Assert.Equal(15, victim.FindDamageEntryFor(a).DamageGiven); + Assert.Same(a, victim.FindMostRecentDamager(true)); + Assert.Same(b, victim.FindLeastRecentDamager(true)); + } + finally + { + victim.Delete(); + a.Delete(); + b.Delete(); + } + } + + [Fact] + public void FindRecent_HonorsAllowSelf() + { + var victim = new TestMobile(); + var a = new TestMobile(); + + try + { + victim.RegisterDamage(10, a); + victim.RegisterDamage(10, victim); // self is most recent + + Assert.Same(victim, victim.FindMostRecentDamager(true)); + Assert.Same(a, victim.FindMostRecentDamager(false)); + Assert.Same(a, victim.FindLeastRecentDamager(false)); + } + finally + { + victim.Delete(); + a.Delete(); + } + } + + [Fact] + public void FindLeastRecent_HonorsAllowSelf() + { + var victim = new TestMobile(); + var a = new TestMobile(); + + try + { + victim.RegisterDamage(10, victim); // self is least recent, so the head is the one to skip + victim.RegisterDamage(10, a); + + Assert.Same(victim, victim.FindLeastRecentDamager(true)); + Assert.Same(a, victim.FindLeastRecentDamager(false)); + } + finally + { + victim.Delete(); + a.Delete(); + } + } + + [Fact] + public void FindTotal_PicksByDamage_MostRecentWinsTies() + { + var victim = new TestMobile(); + var a = new TestMobile(); + var b = new TestMobile(); + var c = new TestMobile(); + + try + { + victim.RegisterDamage(30, a); + victim.RegisterDamage(30, b); // ties a; b is more recent + victim.RegisterDamage(1, c); + + Assert.Same(b, victim.FindMostTotalDamager(true)); + Assert.Same(c, victim.FindLeastTotalDamager(true)); + } + finally + { + victim.Delete(); + a.Delete(); + b.Delete(); + c.Delete(); + } + } + + [Fact] + public void FindLeastTotal_MostRecentWinsTies() + { + var victim = new TestMobile(); + var a = new TestMobile(); + var b = new TestMobile(); + var c = new TestMobile(); + + try + { + victim.RegisterDamage(30, a); + victim.RegisterDamage(5, b); + victim.RegisterDamage(5, c); // ties b for the minimum; c is more recent + + Assert.Same(a, victim.FindMostTotalDamager(true)); + Assert.Same(c, victim.FindLeastTotalDamager(true)); + } + finally + { + victim.Delete(); + a.Delete(); + b.Delete(); + c.Delete(); + } + } + + [Fact] + public void Prune_RemovesExpiredPrefix_KeepsOrder() + { + var start = Core._now; + var victim = new TestMobile(); + var a = new TestMobile(); + var b = new TestMobile(); + + try + { + victim.RegisterDamage(10, a); + + Core._now = start + DamageEntry.ExpireDelay + TimeSpan.FromSeconds(1); + victim.RegisterDamage(10, b); // a is now expired, b is live + + Assert.Equal(new[] { b }, Damagers(victim)); + Assert.Null(victim.FindDamageEntryFor(a)); + } + finally + { + Core._now = start; + victim.Delete(); + a.Delete(); + b.Delete(); + } + } + + [Fact] + public void Prune_AllExpired_EmptiesList() + { + var start = Core._now; + var victim = new TestMobile(); + var a = new TestMobile(); + var b = new TestMobile(); + + try + { + victim.RegisterDamage(10, a); + victim.RegisterDamage(10, b); + + Core._now = start + DamageEntry.ExpireDelay + TimeSpan.FromSeconds(1); + + Assert.Equal(0, victim.DamageEntries.Count); + Assert.Null(victim.FindMostRecentDamageEntry(true)); + } + finally + { + Core._now = start; + victim.Delete(); + a.Delete(); + b.Delete(); + } + } + + [Fact] + public void ClearDamageEntries_UnlinksEveryNode() + { + var victim = new TestMobile(); + var a = new TestMobile(); + var b = new TestMobile(); + + try + { + var ea = victim.RegisterDamage(10, a); + var eb = victim.RegisterDamage(10, b); + + victim.ClearDamageEntries(); + + Assert.Equal(0, victim.DamageEntries.Count); + Assert.False(ea.OnLinkList); + Assert.False(eb.OnLinkList); + Assert.Null(ea.Next); + Assert.Null(ea.Previous); + Assert.Null(eb.Next); + Assert.Null(eb.Previous); + } + finally + { + victim.Delete(); + a.Delete(); + b.Delete(); + } + } + + [Fact] + public void FullHitPoints_ClearsEntries() + { + var victim = new TestMobile(); + var a = new TestMobile(); + + try + { + victim.RawStr = 50; // HitsMax follows Str for a base Mobile + victim.Hits = 10; + victim.RegisterDamage(10, a); + Assert.Equal(1, victim.DamageEntries.Count); + + // Also stops the HitsTimer the Hits = 10 write started, so the test leaves no timer behind. + victim.Hits = victim.HitsMax; + + Assert.Equal(0, victim.DamageEntries.Count); + } + finally + { + victim.Delete(); + a.Delete(); + } + } + + [Fact] + public void RegisterDamage_AccumulatesResponsibleMaster() + { + var victim = new TestMobile(); + var master = new TestMobile(); + var pet = new PetMobile { Master = master }; + + try + { + victim.RegisterDamage(10, pet); + var entry = victim.RegisterDamage(5, pet); + + Assert.Same(pet, entry.Damager); + Assert.Equal(15, entry.DamageGiven); + Assert.NotNull(entry.Responsible); + Assert.Single(entry.Responsible); + Assert.Same(master, entry.Responsible[0].Damager); + Assert.Equal(15, entry.Responsible[0].DamageGiven); + Assert.False(entry.Responsible[0].OnLinkList); // sub-entries never join the main list + } + finally + { + victim.Delete(); + master.Delete(); + pet.Delete(); + } + } +} diff --git a/Projects/Server/Mobiles/Mobile.cs b/Projects/Server/Mobiles/Mobile.cs index 0407d7f82..d1db8113b 100644 --- a/Projects/Server/Mobiles/Mobile.cs +++ b/Projects/Server/Mobiles/Mobile.cs @@ -42,7 +42,7 @@ public delegate void PromptCallback(Mobile from, string text); public delegate void PromptStateCallback(Mobile from, string text, T state); -public class DamageEntry +public class DamageEntry : IValueLinkListNode { public DamageEntry(Mobile damager) => Damager = damager; @@ -57,6 +57,11 @@ public class DamageEntry public List Responsible { get; set; } public static TimeSpan ExpireDelay { get; set; } = TimeSpan.FromMinutes(2.0); + + // Intrusive links for Mobile._damageEntries. Sub-entries in Responsible never join a list. + public DamageEntry Next { get; set; } + public DamageEntry Previous { get; set; } + public bool OnLinkList { get; set; } } [Flags] @@ -377,7 +382,6 @@ public partial class Mobile : IHued, IComparable, ISpawnable, IObjectPro Aggressors = new List(); Aggressed = new List(); NextSkillTime = Core.TickCount; - DamageEntries = new List(); } // Sectors @@ -958,7 +962,23 @@ public partial class Mobile : IHued, IComparable, ISpawnable, IObjectPro public static VisibleDamageType VisibleDamageType { get; set; } - public List DamageEntries { get; private set; } + private ValueLinkList _damageEntries; + + /// + /// Damage entries ordered least recent (head) to most recent (tail). Expired entries are + /// pruned on access. Enumerate with foreach (ascending) or .ByDescending(). + /// Mutate only through and . + /// Calling a ValueLinkList mutator on this reference compiles, but operates on a defensive copy + /// while still unlinking the real nodes — it silently corrupts the list. + /// + public ref readonly ValueLinkList DamageEntries + { + get + { + PruneExpiredDamageEntries(); + return ref _damageEntries; + } + } [CommandProperty(AccessLevel.GameMaster)] public Mobile LastKiller { get; set; } @@ -2020,10 +2040,7 @@ public partial class Mobile : IHued, IComparable, ISpawnable, IObjectPro Aggressors[i].CanReportMurder = false; } - if (DamageEntries.Count > 0) - { - DamageEntries.Clear(); // reset damage entries on full HP - } + ClearDamageEntries(); // reset damage entries on full HP } else if (CanRegenHits) { @@ -5745,24 +5762,54 @@ public partial class Mobile : IHued, IComparable, ISpawnable, IObjectPro } } + // Entries are kept in LastDamage order, so expired entries are always a head prefix. + private void PruneExpiredDamageEntries() + { +#if DEBUG + for (var node = _damageEntries._first; node != null; node = node.Next) + { + Debug.Assert( + node.Next == null || node.Next.LastDamage >= node.LastDamage, + "Damage entries must be ordered by LastDamage ascending." + ); + } +#endif + + var first = _damageEntries._first; + + if (first?.HasExpired != true) + { + return; + } + + var firstLive = first.Next; + + while (firstLive?.HasExpired == true) + { + firstLive = firstLive.Next; + } + + if (firstLive == null) + { + _damageEntries.RemoveAll(); + } + else + { + _damageEntries.RemoveAllBefore(firstLive); + } + } + + public void ClearDamageEntries() => _damageEntries.RemoveAll(); + public Mobile FindMostRecentDamager(bool allowSelf) => FindMostRecentDamageEntry(allowSelf)?.Damager; public DamageEntry FindMostRecentDamageEntry(bool allowSelf) { - for (var i = DamageEntries.Count - 1; i >= 0; --i) + PruneExpiredDamageEntries(); + + for (var de = _damageEntries._last; de != null; de = de.Previous) { - if (i >= DamageEntries.Count) - { - continue; - } - - var de = DamageEntries[i]; - - if (de.HasExpired) - { - DamageEntries.RemoveAt(i); - } - else if (allowSelf || de.Damager != this) + if (allowSelf || de.Damager != this) { return de; } @@ -5775,21 +5822,11 @@ public partial class Mobile : IHued, IComparable, ISpawnable, IObjectPro public DamageEntry FindLeastRecentDamageEntry(bool allowSelf) { - for (var i = 0; i < DamageEntries.Count; ++i) + PruneExpiredDamageEntries(); + + for (var de = _damageEntries._first; de != null; de = de.Next) { - if (i < 0) - { - continue; - } - - var de = DamageEntries[i]; - - if (de.HasExpired) - { - DamageEntries.RemoveAt(i); - --i; - } - else if (allowSelf || de.Damager != this) + if (allowSelf || de.Damager != this) { return de; } @@ -5800,24 +5837,17 @@ public partial class Mobile : IHued, IComparable, ISpawnable, IObjectPro public Mobile FindMostTotalDamager(bool allowSelf) => FindMostTotalDamageEntry(allowSelf)?.Damager; + // Walks most recent first with a strict comparison so the most recent entry wins ties, + // matching the previous reverse-indexed loop. public DamageEntry FindMostTotalDamageEntry(bool allowSelf) { + PruneExpiredDamageEntries(); + DamageEntry mostTotal = null; - for (var i = DamageEntries.Count - 1; i >= 0; --i) + for (var de = _damageEntries._last; de != null; de = de.Previous) { - if (i >= DamageEntries.Count) - { - continue; - } - - var de = DamageEntries[i]; - - if (de.HasExpired) - { - DamageEntries.RemoveAt(i); - } - else if ((allowSelf || de.Damager != this) && (mostTotal == null || de.DamageGiven > mostTotal.DamageGiven)) + if ((allowSelf || de.Damager != this) && (mostTotal == null || de.DamageGiven > mostTotal.DamageGiven)) { mostTotal = de; } @@ -5830,46 +5860,28 @@ public partial class Mobile : IHued, IComparable, ISpawnable, IObjectPro public DamageEntry FindLeastTotalDamageEntry(bool allowSelf) { - DamageEntry mostTotal = null; + PruneExpiredDamageEntries(); - for (var i = DamageEntries.Count - 1; i >= 0; --i) + DamageEntry leastTotal = null; + + for (var de = _damageEntries._last; de != null; de = de.Previous) { - if (i >= DamageEntries.Count) + if ((allowSelf || de.Damager != this) && (leastTotal == null || de.DamageGiven < leastTotal.DamageGiven)) { - continue; - } - - var de = DamageEntries[i]; - - if (de.HasExpired) - { - DamageEntries.RemoveAt(i); - } - else if ((allowSelf || de.Damager != this) && (mostTotal == null || de.DamageGiven < mostTotal.DamageGiven)) - { - mostTotal = de; + leastTotal = de; } } - return mostTotal; + return leastTotal; } public DamageEntry FindDamageEntryFor(Mobile m) { - for (var i = DamageEntries.Count - 1; i >= 0; --i) + PruneExpiredDamageEntries(); + + for (var de = _damageEntries._last; de != null; de = de.Previous) { - if (i >= DamageEntries.Count) - { - continue; - } - - var de = DamageEntries[i]; - - if (de.HasExpired) - { - DamageEntries.RemoveAt(i); - } - else if (de.Damager == m) + if (de.Damager == m) { return de; } @@ -5887,8 +5899,13 @@ public partial class Mobile : IHued, IComparable, ISpawnable, IObjectPro de.DamageGiven += amount; de.LastDamage = Core.Now; - DamageEntries.Remove(de); - DamageEntries.Add(de); + // Move to the tail so the list stays in LastDamage order. + if (de.OnLinkList) + { + _damageEntries.Remove(de); + } + + _damageEntries.AddLast(de); var master = from.GetDamageMaster(this); @@ -7814,7 +7831,6 @@ public partial class Mobile : IHued, IComparable, ISpawnable, IObjectPro AutoPageNotify = true; Aggressors = new List(); Aggressed = new List(); - DamageEntries = new List(); NextSkillTime = Core.TickCount; } diff --git a/Projects/UOContent.Tests/Tests/Mobiles/LootingRightsTests.cs b/Projects/UOContent.Tests/Tests/Mobiles/LootingRightsTests.cs new file mode 100644 index 000000000..e02abece7 --- /dev/null +++ b/Projects/UOContent.Tests/Tests/Mobiles/LootingRightsTests.cs @@ -0,0 +1,146 @@ +using System.Collections.Generic; +using Server.Mobiles; +using Xunit; + +namespace Server.Tests; + +/// +/// Pins the looting-rights rules that the inline damage entry list has to keep producing: the +/// returned stores are sorted by damage descending, the first (least recent) damager takes the +/// 1.25x bonus, the hitsMax band decides who clears the threshold, and a pet's damage is credited +/// to its damage master rather than to the pet. +/// +[Collection("Sequential UOContent Tests")] +public class LootingRightsTests +{ + private class TestMobile : Mobile + { + } + + private class PetMobile : Mobile + { + public Mobile Master { get; set; } + + public override Mobile GetDamageMaster(Mobile damagee) => Master; + } + + // GetLootingRights only ever credits mobiles flagged as players. + private static TestMobile NewPlayer() => new() { Player = true }; + + private static DamageStore FindStore(List rights, Mobile m) + { + for (var i = 0; i < rights.Count; i++) + { + if (rights[i].m_Mobile == m) + { + return rights[i]; + } + } + + return null; + } + + [Fact] + public void TwoPlayerDamagers_SortDescending_AndTheFirstDamagerTakesTheBonus() + { + var victim = new TestMobile(); + var first = NewPlayer(); + var second = NewPlayer(); + + try + { + victim.RegisterDamage(100, first); + victim.RegisterDamage(40, second); // second is the most recent, first is the "first damager" + + // hitsMax < 200 puts the bar at topDamage / 2. + var rights = BaseCreature.GetLootingRights(victim.DamageEntries, 100); + + Assert.Equal(2, rights.Count); + + // Sorted by damage descending. + Assert.True(rights[0].m_Damage >= rights[1].m_Damage); + Assert.Same(first, rights[0].m_Mobile); + Assert.Same(second, rights[1].m_Mobile); + + // The first damager - the least recent entry - gets the 1.25x bonus; nobody else does. + Assert.Equal(125, rights[0].m_Damage); + Assert.Equal(40, rights[1].m_Damage); + + // topDamage 125 / 2 = 62, so 40 is below the bar. + Assert.True(rights[0].m_HasRight); + Assert.False(rights[1].m_HasRight); + } + finally + { + victim.Delete(); + first.Delete(); + second.Delete(); + } + } + + [Fact] + public void HitsMaxBand_MovesTheRightsThreshold() + { + var victim = new TestMobile(); + var first = NewPlayer(); + var second = NewPlayer(); + + try + { + victim.RegisterDamage(100, first); + victim.RegisterDamage(40, second); + + // hitsMax >= 200 drops the bar to topDamage / 4 = 31, which 40 clears. + var rights = BaseCreature.GetLootingRights(victim.DamageEntries, 200); + + Assert.Equal(2, rights.Count); + Assert.True(rights[0].m_HasRight); + Assert.True(rights[1].m_HasRight); + Assert.Same(second, rights[1].m_Mobile); + } + finally + { + victim.Delete(); + first.Delete(); + second.Delete(); + } + } + + [Fact] + public void PetDamage_CreditsTheMaster_NotThePet() + { + var victim = new TestMobile(); + var master = NewPlayer(); + var pet = new PetMobile { Master = master }; + var wild = new TestMobile(); // no damage master, and not a player + + try + { + victim.RegisterDamage(50, pet); + victim.RegisterDamage(20, wild); + + var rights = BaseCreature.GetLootingRights(victim.DamageEntries, 100); + + // The master is credited through the entry's Responsible sub-entry, and is the only one. + Assert.Single(rights); + + var masterStore = FindStore(rights, master); + Assert.NotNull(masterStore); + Assert.Equal(62, masterStore.m_Damage); // 50, then the first-damager 1.25x bonus + Assert.True(masterStore.m_HasRight); + + // The pet's own damage was fully handed to the master, so it earns no store. + Assert.Null(FindStore(rights, pet)); + + // A non-player damager earns nothing even when its damage was never reassigned. + Assert.Null(FindStore(rights, wild)); + } + finally + { + victim.Delete(); + master.Delete(); + pet.Delete(); + wild.Delete(); + } + } +} diff --git a/Projects/UOContent/Engines/CannedEvil/ChampionSpawn.cs b/Projects/UOContent/Engines/CannedEvil/ChampionSpawn.cs index 9daea0ba3..3a024576b 100755 --- a/Projects/UOContent/Engines/CannedEvil/ChampionSpawn.cs +++ b/Projects/UOContent/Engines/CannedEvil/ChampionSpawn.cs @@ -18,6 +18,7 @@ using System.Net; using System.Collections.Generic; using System.Runtime.InteropServices; using ModernUO.Serialization; +using Server.Collections; using Server.Engines.Virtues; using Server.Gumps; using Server.Items; @@ -1181,11 +1182,6 @@ public partial class ChampionSpawn : Item foreach (var de in m.DamageEntries) { - if (de.HasExpired) - { - continue; - } - var damager = de.Damager; var master = damager.GetDamageMaster(m); diff --git a/Projects/UOContent/Mobiles/BaseCreature.cs b/Projects/UOContent/Mobiles/BaseCreature.cs index 581d472e3..404aaede8 100644 --- a/Projects/UOContent/Mobiles/BaseCreature.cs +++ b/Projects/UOContent/Mobiles/BaseCreature.cs @@ -3123,14 +3123,12 @@ namespace Server.Mobiles return base.OnBeforeDeath(); } - public int ComputeBonusDamage(List list, Mobile m) + public int ComputeBonusDamage(in ValueLinkList list, Mobile m) { var bonus = 0; - for (var i = list.Count - 1; i >= 0; --i) + foreach (var de in list.ByDescending()) { - var de = list[i]; - if (de.Damager == m || de.Damager is not BaseCreature bc) { continue; @@ -3167,26 +3165,15 @@ namespace Server.Mobiles Combatant is PlayerMobile || Combatant is BaseCreature { Controlled: true } bc && bc.GetMaster() is PlayerMobile; - public static List GetLootingRights(List damageEntries, int hitsMax) + // Iterates most recent first, matching the previous reverse-indexed loop. The list is + // already pruned of expired entries by the Mobile.DamageEntries getter. + public static List GetLootingRights(in ValueLinkList damageEntries, int hitsMax) { var rights = new List(); DamageStore firstDamager = null; - for (var i = damageEntries.Count - 1; i >= 0; --i) + foreach (var de in damageEntries.ByDescending()) { - if (i >= damageEntries.Count) - { - continue; - } - - var de = damageEntries[i]; - - if (de.HasExpired) - { - damageEntries.RemoveAt(i); - continue; - } - var damage = de.DamageGiven; var respList = de.Responsible; diff --git a/Projects/UOContent/Mobiles/Special/Harrower.cs b/Projects/UOContent/Mobiles/Special/Harrower.cs index ee3fa1873..8d87943bc 100644 --- a/Projects/UOContent/Mobiles/Special/Harrower.cs +++ b/Projects/UOContent/Mobiles/Special/Harrower.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using ModernUO.Serialization; +using Server.Collections; using Server.Engines.CannedEvil; using Server.Engines.Virtues; using Server.Items; diff --git a/dev-docs/claude-skills/migrate-from-runuo/migrate-items-mobiles.md b/dev-docs/claude-skills/migrate-from-runuo/migrate-items-mobiles.md index f12f92c55..7310c104e 100644 --- a/dev-docs/claude-skills/migrate-from-runuo/migrate-items-mobiles.md +++ b/dev-docs/claude-skills/migrate-from-runuo/migrate-items-mobiles.md @@ -31,6 +31,7 @@ description: > - Expression-bodied overrides: `public override int Meat { get { return 1; } }` -> `public override int Meat => 1;` - AI movement calls lose the `run` flag: `MoveTo(m, true, range)` -> `MoveTo(m, range)` (also `WalkMobileRange`, `ApproachTarget`, `MoveToPoint`, `PathFollower.Follow`); the Running bit is derived from step pace -> `dev-docs/runuo-migration-docs/09-items-mobiles-creatures.md` § AI Movement - `AcquireOnApproach` (bool) -> `AcquireOnApproachDelay` (TimeSpan; `Zero` = old instant behavior) -> same doc § Target Acquisition +- `DamageEntries` is an inline `ref readonly ValueLinkList`, not a `List`: indexer/`Add`/`Remove`/`Clear` -> `foreach` / `.ByDescending()` (needs `using Server.Collections;`) and `ClearDamageEntries()`; `GetLootingRights` takes it by `in` -> same doc § Damage Entries ## Anti-Patterns - Using `_field--` instead of `Property--` (bypasses MarkDirty tracking) diff --git a/dev-docs/runuo-migration-docs/09-items-mobiles-creatures.md b/dev-docs/runuo-migration-docs/09-items-mobiles-creatures.md index 5b30e7ef9..def2ecf48 100644 --- a/dev-docs/runuo-migration-docs/09-items-mobiles-creatures.md +++ b/dev-docs/runuo-migration-docs/09-items-mobiles-creatures.md @@ -517,6 +517,45 @@ periodic `ReacquireDelay` scan still sweeps the full `RangePerception`). The acquired target comes from the normal FightMode-ranked scan, not from whichever mobile happened to move. See `content-patterns.md` § Target Acquisition. +## Damage Entries: Inline `ValueLinkList`, Not `List` + +RunUO's `Mobile.DamageEntries` was a `List` allocated for every mobile. +ModernUO keeps damage entries in an inline `ValueLinkList` struct held by +the mobile itself, ordered least recent → most recent, so a mobile that never takes +damage owns no list object and `RegisterDamage` relinks in O(1). The property is +`ref readonly`; expired entries are pruned when it is read. + +```csharp +// RunUO +for (var i = m.DamageEntries.Count - 1; i >= 0; --i) +{ + var de = m.DamageEntries[i]; // indexer + ... +} +m.DamageEntries.Clear(); +var rights = BaseCreature.GetLootingRights(m.DamageEntries, m.HitsMax); // List + +// ModernUO — needs `using Server.Collections;` for the enumerator extensions +foreach (var de in m.DamageEntries.ByDescending()) // most recent first +{ + ... +} +foreach (var de in m.DamageEntries) // least recent first +{ + ... +} +m.ClearDamageEntries(); +var rights = BaseCreature.GetLootingRights(m.DamageEntries, m.HitsMax); // in ValueLinkList +``` + +What no longer compiles: the indexer, `.Add`, `.Remove`, `.RemoveAt`, `.Clear`, and +passing the property where a `List` is expected. `.Count`, +`FindDamageEntryFor`, `FindMostRecentDamager` and the other `Find*` methods, and +`RegisterDamage` are unchanged. `DamageEntry` now carries `Next`/`Previous`/`OnLinkList` +link fields; never set them yourself, and never call a `ValueLinkList` mutator on the +`ref readonly` property — it compiles against a copy and corrupts the node's link state. +Mutate only through `RegisterDamage` and `ClearDamageEntries`. + ## Item Name Changes ```csharp diff --git a/dev-docs/runuo-migration-docs/11-api-reference.md b/dev-docs/runuo-migration-docs/11-api-reference.md index 96db1590d..b6c12bfe8 100644 --- a/dev-docs/runuo-migration-docs/11-api-reference.md +++ b/dev-docs/runuo-migration-docs/11-api-reference.md @@ -134,6 +134,9 @@ Alphabetical by RunUO API name. Use Ctrl+F / Cmd+F to search. | `WalkMobileRange(m, steps, run, min, max)` | `WalkMobileRange(m, steps, min, max)` | Same | | `PathFollower.Follow(run, range)` | `Follow(range)` | Same | | `AcquireOnApproach` (bool) | `AcquireOnApproachDelay` (TimeSpan) | Reaction-time gradient; `Zero` = old instant behavior | +| `m.DamageEntries` (`List`) | `m.DamageEntries` (`ref readonly ValueLinkList`) | Inline, least→most recent; `foreach` / `.ByDescending()` only, needs `using Server.Collections;`; no indexer, `Add`, `Remove`, `Clear` | +| `m.DamageEntries.Clear()` | `m.ClearDamageEntries()` | | +| `GetLootingRights(List, int)` | `GetLootingRights(in ValueLinkList, int)` | Callers passing `m.DamageEntries` compile unchanged | ## Networking