From c1442aff3e15cb817a0b7ef5239d9b62e650697e Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Mon, 10 Aug 2026 09:01:29 -0700 Subject: [PATCH] fix: Stop treasure chest guardian spawn farming via stack splits (#2568) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### Summary Players reported an exploit: decipher a treasure map, then run a ClassicUO/Razor organizer agent that pulls the gold out of the chest in small amounts. Each pull spawned more monsters, turning one chest into an unbounded farmable spawn generator. ### Root cause `TreasureMapChest.OnItemLifted` grants a 10% guardian spawn roll per first-time-lifted item, deduplicated by the instance-keyed `_lifted` set. But a partial lift goes through `Mobile.LiftItemDupe`, which re-adds the stack remainder to the chest as a **brand-new item instance** (engine-side `AddItem`, bypassing the `CheckHold` block on refilling). Every subsequent pull lifts an instance the `_lifted` set has never seen, so each one re-rolls the 10% spawn chance: - A level 4 chest holds 4,000 gold → pulled coin by coin, ~400 spawned creatures (plus more from reagent stacks), hands-free, per chest. - Spawns use `guardian: false`, so nothing tracks or caps them. - Legit full-stack looting yields roughly 5–8 bonus spawns per chest for comparison. The code is inherited from RunUO, so descendant shards likely share the hole. ### Fix Mark every item that enters the chest **after the initial fill** as already lifted, via an `OnItemAdded` override gated by a non-serialized `_filled` flag (set at the end of the constructor and in `[AfterDeserialization]`). Ordering makes this exact: `LiftItemDupe` re-adds the remainder *before* the chest's `OnItemLifted` runs, so the lifted original still gets its one legitimate roll while the remainder is pre-marked. This also covers packing items *into* the chest (e.g., merging gold back in to lift it out again) and bounce-backs — anything not part of the original loot can never grant a spawn roll. ### Tests - `PartialLift_MarksSplitRemainderAsLifted` — drives the real `Mobile.Lift` path with a 1-coin pull and asserts the split remainder is marked (failed before the fix). - `ItemAddedAfterFill_IsMarkedLifted` — post-fill additions are marked (failed before the fix). - `OriginalFillLoot_IsNotMarkedLifted` — original loot keeps spawn-roll eligibility. Full `UOContent.Tests` suite: 701 passed. --- .../TreasureMapChestLiftTests.cs | 111 ++++++++++++++++++ .../Items/Containers/TreasureMapChest.cs | 64 ++++++++-- .../Server.Items.TreasureMapChest.v3.json | 57 +++++++++ 3 files changed, 219 insertions(+), 13 deletions(-) create mode 100644 Projects/UOContent.Tests/Tests/Items/TreasureChests/TreasureMapChestLiftTests.cs create mode 100644 Projects/UOContent/Migrations/Server.Items.TreasureMapChest.v3.json diff --git a/Projects/UOContent.Tests/Tests/Items/TreasureChests/TreasureMapChestLiftTests.cs b/Projects/UOContent.Tests/Tests/Items/TreasureChests/TreasureMapChestLiftTests.cs new file mode 100644 index 000000000..f9b8763e7 --- /dev/null +++ b/Projects/UOContent.Tests/Tests/Items/TreasureChests/TreasureMapChestLiftTests.cs @@ -0,0 +1,111 @@ +using Server; +using Server.Items; +using Server.Mobiles; +using Server.Tests; +using Xunit; + +namespace UOContent.Tests; + +[Collection("Sequential UOContent Tests")] +public class TreasureMapChestLiftTests +{ + // Coordinates chosen to avoid overlap with Tracking (1000-4000, 1000-4000) and + // DetectHidden (1000-2400, 500) test areas. + + [Fact] + public void PartialLift_MarksSplitRemainderAsLifted() + { + using var rng = new PredictableRandom(10); // RandomDouble() = 0.5, no spawn roll fires + var map = Map.Felucca; + var location = new Point3D(5000, 600, 0); + var player = CreatePlayerMobile(map, location); + var chest = new TreasureMapChest(1); + + try + { + chest.MoveToWorld(location, map); + chest.Locked = false; + + var gold = FindGold(chest, null); + Assert.NotNull(gold); + + player.Lift(gold, 1, out var rejected, out _); + Assert.False(rejected); + + // The stack split re-adds the remainder as a brand-new item. It must count as + // already lifted, otherwise every 1-coin pull grants a fresh guardian spawn roll. + var remainder = FindGold(chest, gold); + Assert.NotNull(remainder); + Assert.Contains(remainder, chest.Lifted); + Assert.Contains(gold, chest.Lifted); + } + finally + { + player.Holding?.Delete(); + player.Delete(); + chest.Delete(); + } + } + + [Fact] + public void ItemAddedAfterFill_IsMarkedLifted() + { + using var rng = new PredictableRandom(10); + var chest = new TreasureMapChest(1); + var packed = new Gold(500); + + try + { + // Anything entering the chest after the initial fill (packed-back gold, split + // remainders, GM drops) was never part of the original loot and must not + // grant spawn rolls when lifted back out. + chest.DropItem(packed); + + Assert.Contains(packed, chest.Lifted); + } + finally + { + chest.Delete(); + } + } + + [Fact] + public void OriginalFillLoot_IsNotMarkedLifted() + { + using var rng = new PredictableRandom(10); + var chest = new TreasureMapChest(1); + + try + { + // The original loot must stay roll-eligible for its first lift. + Assert.True(chest.Lifted == null || chest.Lifted.Count == 0); + } + finally + { + chest.Delete(); + } + } + + private static Gold FindGold(TreasureMapChest chest, Gold except) + { + var items = chest.Items; + + for (var i = 0; i < items.Count; i++) + { + if (items[i] is Gold gold && gold != except) + { + return gold; + } + } + + return null; + } + + private static PlayerMobile CreatePlayerMobile(Map map, Point3D location) + { + var mobile = new PlayerMobile(World.NewMobile); + mobile.DefaultMobileInit(); + mobile.MoveToWorld(location, map); + return mobile; + } +} diff --git a/Projects/UOContent/Items/Containers/TreasureMapChest.cs b/Projects/UOContent/Items/Containers/TreasureMapChest.cs index b59911a8a..94cbf7fad 100644 --- a/Projects/UOContent/Items/Containers/TreasureMapChest.cs +++ b/Projects/UOContent/Items/Containers/TreasureMapChest.cs @@ -9,10 +9,11 @@ using Server.Network; namespace Server.Items; -[SerializationGenerator(2, false)] +[SerializationGenerator(3, false)] public partial class TreasureMapChest : LockableContainer { [Tidy] + [CanBeNull] [SerializableField(0, setter: "private")] private List _guardians; @@ -43,10 +44,14 @@ public partial class TreasureMapChest : LockableContainer } [Tidy] + [CanBeNull] [SerializableField(5, setter: "private")] [SerializedCommandProperty(AccessLevel.GameMaster)] private HashSet _lifted; + // False only while the constructor fills the chest; deserialized chests are always filled. + private bool _filled; + [Constructible] public TreasureMapChest(int level) : this(null, level) { @@ -58,10 +63,10 @@ public partial class TreasureMapChest : LockableContainer _level = level; _temporary = temporary; - _guardians = []; _expireTimer = Timer.DelayCall(TimeSpan.FromHours(3.0), Delete); Fill(this, level); + _filled = true; } public override int LabelNumber => 3000541; @@ -182,7 +187,6 @@ public partial class TreasureMapChest : LockableContainer 2 => 76, 3 => 84, 4 => 92, - 5 => 100, _ => 100 }; @@ -300,14 +304,17 @@ public partial class TreasureMapChest : LockableContainer if (_level == 0 && from.AccessLevel < AccessLevel.GameMaster) { - for (var i = 0; i < _guardians.Count; i++) + if (_guardians.Count > 0) { - var m = _guardians[i]; - if (m.Alive) + for (var i = 0; i < _guardians.Count; i++) { - // You must first kill the guardians before you may open this chest. - from.SendLocalizedMessage(1046448); - return true; + var m = _guardians[i]; + if (m.Alive) + { + // You must first kill the guardians before you may open this chest. + from.SendLocalizedMessage(1046448); + return true; + } } } @@ -361,6 +368,17 @@ public partial class TreasureMapChest : LockableContainer public override bool CheckLift(Mobile from, Item item, ref LRReason reject) => CheckLoot(from, true) && base.CheckLift(from, item, ref reject); + public override void OnItemAdded(Item item) + { + base.OnItemAdded(item); + + if (_filled) + { + _lifted ??= []; + _lifted.Add(item); + } + } + public override void OnItemLifted(Mobile from, Item item) { var notYetLifted = _lifted?.Contains(item) != true; @@ -393,8 +411,6 @@ public partial class TreasureMapChest : LockableContainer private void Deserialize(IGenericReader reader, int version) { - _guardians = []; - _owner = reader.ReadEntity(); _level = reader.ReadInt(); var expireTimerNext = reader.ReadDeltaTime(); @@ -402,12 +418,34 @@ public partial class TreasureMapChest : LockableContainer _lifted = reader.ReadEntitySet(); } - [AfterDeserialization(false)] + private void MigrateFrom(V2Content content) + { + _guardians = content.Guardians; + if (_guardians.Count == 0) + { + _guardians = null; + } + _temporary = content.Temporary; + _owner = content.Owner; + _level = content.Level; + _lifted = content.Lifted; + if (_lifted.Count == 0) + { + _lifted = null; + } + + var expireTimerDelay = content.ExpireTimerDelay; + DeserializeExpireTimer(expireTimerDelay); + } + + [AfterDeserialization] private void AfterDeserialization() { + _filled = true; + if (_expireTimer == null) { - Delete(); + Timer.DelayCall(Delete); } } diff --git a/Projects/UOContent/Migrations/Server.Items.TreasureMapChest.v3.json b/Projects/UOContent/Migrations/Server.Items.TreasureMapChest.v3.json new file mode 100644 index 000000000..805bbb6d1 --- /dev/null +++ b/Projects/UOContent/Migrations/Server.Items.TreasureMapChest.v3.json @@ -0,0 +1,57 @@ +{ + "version": 3, + "type": "Server.Items.TreasureMapChest", + "properties": [ + { + "name": "Guardians", + "type": "System.Collections.Generic.List\u003CServer.Mobile\u003E", + "rule": "ListMigrationRule", + "ruleArguments": [ + "@Tidy", + "@CanBeNull", + "Server.Mobile", + "SerializableInterfaceMigrationRule" + ] + }, + { + "name": "Temporary", + "type": "bool", + "rule": "PrimitiveTypeMigrationRule", + "ruleArguments": [ + "" + ] + }, + { + "name": "Owner", + "type": "Server.Mobile", + "rule": "SerializableInterfaceMigrationRule" + }, + { + "name": "Level", + "type": "int", + "rule": "PrimitiveTypeMigrationRule", + "ruleArguments": [ + "" + ] + }, + { + "name": "ExpireTimer", + "type": "Server.Timer", + "rule": "TimerMigrationRule", + "ruleArguments": [ + "@TimerDrift" + ] + }, + { + "name": "Lifted", + "type": "System.Collections.Generic.HashSet\u003CServer.Item\u003E", + "rule": "HashSetMigrationRule", + "ruleArguments": [ + "@Tidy", + "@CanBeNull", + "Server.Item", + "SerializableInterfaceMigrationRule" + ] + } + ] +} \ No newline at end of file