From b98b0e296439c4df8da63db0987e06c4b922780c Mon Sep 17 00:00:00 2001 From: Jack Date: Wed, 7 Oct 2026 13:39:04 +1300 Subject: [PATCH] fix: digging up a treasure map chest no longer crashes when guardians spawn (#2692) ## Summary Digging up a decoded treasure map of level 0, or of level 2 and above, throws inside `DigTimer.OnTick` once the chest surfaces. The exception escapes the timer, so the server goes down: ``` System.NullReferenceException: Object reference not set to an instance of an object. at Server.Items.TreasureMap.DigTimer.OnTick() in ...\Items\Maps\TreasureMap.cs:line 809 ``` #2568 made `TreasureMapChest._guardians` lazily created: the constructor no longer initialises it, and migration nulls an empty list. #2569 updated the read in `CheckLocked` to match, but the dig timer still adds each spawned guardian directly: ```csharp if (bc != null) { m_Chest.Guardians.Add(bc); // null on a freshly dug chest } ``` Level 1 maps survive only because they spawn no guardians. I confirmed the regression by running the dig test below at #2568's parent, where it passes, and at #2568, where it fails. The dig timer now calls the generated `AddToGuardians`, which creates the list on first use. ## Testing - `dotnet build`: 0 warnings, 0 errors. - New `TreasureMapChestGuardianTests`: - `Dig_AddsSpawnedGuardiansToTheChest` (levels 0 and 2) drives the real `DigTimer` tick by tick until the chest surfaces. Before the fix both cases fail with the `NullReferenceException` above. It needs client map data, so it calls `TileDataRequirement.SkipIfMissing()` and skips on CI. - `AddToGuardians_OnNewChest_KeepsLevelZeroChestShut` needs no client data, so it runs on CI: the list starts null, the first guardian creates it, and a level 0 chest stays shut to players until that guardian is dead. - Full `UOContent.Tests`: 1196 passed and 2 skipped with client data; 1105 passed and 93 skipped without. --- .../TreasureMapChestGuardianTests.cs | 139 ++++++++++++++++++ Projects/UOContent/Items/Maps/TreasureMap.cs | 2 +- 2 files changed, 140 insertions(+), 1 deletion(-) create mode 100644 Projects/UOContent.Tests/Tests/Items/TreasureChests/TreasureMapChestGuardianTests.cs diff --git a/Projects/UOContent.Tests/Tests/Items/TreasureChests/TreasureMapChestGuardianTests.cs b/Projects/UOContent.Tests/Tests/Items/TreasureChests/TreasureMapChestGuardianTests.cs new file mode 100644 index 000000000..633025042 --- /dev/null +++ b/Projects/UOContent.Tests/Tests/Items/TreasureChests/TreasureMapChestGuardianTests.cs @@ -0,0 +1,139 @@ +using System; +using System.Collections.Generic; +using System.Reflection; +using Server; +using Server.Items; +using Server.Mobiles; +using Server.Tests; +using Xunit; + +namespace UOContent.Tests; + +[Collection("Sequential UOContent Tests")] +public class TreasureMapChestGuardianTests +{ + // A treasure.cfg chest site with room to dig the chest up and spawn its guardians. + private const int SiteX = 2643; + private const int SiteY = 851; + + // The guardian list starts null and is created by the first guardian added. A level 0 chest + // stays shut to players until its guardians are dead. + [Fact] + public void AddToGuardians_OnNewChest_KeepsLevelZeroChestShut() + { + var map = Map.Felucca; + var location = new Point3D(5000, 610, 0); + var player = CreatePlayerMobile(map, location); + var guardian = new Mobile(World.NewMobile); + guardian.DefaultMobileInit(); + var chest = new TreasureMapChest(0); + + try + { + Assert.Null(chest.Guardians); + + chest.AddToGuardians(guardian); + + Assert.Same(guardian, Assert.Single(chest.Guardians)); + Assert.True(chest.CheckLocked(player)); + + guardian.Delete(); + + Assert.False(chest.CheckLocked(player)); + } + finally + { + chest.Delete(); + guardian.Delete(); + player.Delete(); + } + } + + // Drives the real dig, one DigTimer tick per call, until the chest surfaces and its guardians + // spawn. Levels 0 and 2+ spawn guardians; level 1 spawns none. + [SkippableTheory] + [InlineData(0)] + [InlineData(2)] + public void Dig_AddsSpawnedGuardiansToTheChest(int level) + { + TileDataRequirement.SkipIfMissing(); + + var map = Map.Felucca; + var site = new Point3D(SiteX, SiteY, map.GetAverageZ(SiteX, SiteY)); + var player = CreatePlayerMobile(map, new Point3D(site.X + 1, site.Y, site.Z)); + var treasureMap = new TreasureMap(level, map); + + try + { + Assert.True(map.CanFit(site.X, site.Y, site.Z, 16, true)); + Assert.True(player.BeginAction()); + + var timerType = typeof(TreasureMap).GetNestedType("DigTimer", BindingFlags.NonPublic)!; + var onTick = timerType.GetMethod("OnTick", BindingFlags.NonPublic | BindingFlags.Instance)!; + var digTimer = Activator.CreateInstance(timerType, player, treasureMap, site, map); + + for (var i = 0; i < 30 && !treasureMap.Completed; i++) + { + onTick.Invoke(digTimer, BindingFlags.DoNotWrapExceptions, null, null, null); + } + + var chest = FindChest(map, site); + + Assert.True(treasureMap.Completed); + Assert.NotNull(chest); + Assert.NotNull(chest.Guardians); + Assert.NotEmpty(chest.Guardians); + } + finally + { + Cleanup(map, site); + treasureMap.Delete(); + player.Delete(); + } + } + + private static TreasureMapChest FindChest(Map map, Point3D site) + { + foreach (var chest in map.GetItemsInRange(site, 0)) + { + return chest; + } + + return null; + } + + // Sweeps the site rather than trusting references, so a dig that throws partway still leaves + // nothing behind for the next case: the chest, both dirt piles and any guardian spawned so far. + private static void Cleanup(Map map, Point3D site) + { + var leftovers = new List(); + + foreach (var m in map.GetMobilesInRange(site, 3)) + { + leftovers.Add(m); + } + + foreach (var item in map.GetItemsInRange(site, 1)) + { + leftovers.Add(item); + } + + foreach (var item in map.GetItemsInRange(site, 1)) + { + leftovers.Add(item); + } + + foreach (var entity in leftovers) + { + entity.Delete(); + } + } + + 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/Maps/TreasureMap.cs b/Projects/UOContent/Items/Maps/TreasureMap.cs index 25711e120..fd2a9d75d 100644 --- a/Projects/UOContent/Items/Maps/TreasureMap.cs +++ b/Projects/UOContent/Items/Maps/TreasureMap.cs @@ -806,7 +806,7 @@ public partial class TreasureMap : MapItem if (bc != null) { - m_Chest.Guardians.Add(bc); + m_Chest.AddToGuardians(bc); } } }