fix: enumerators that end early, repeat entries, or never end (#2685)

Fixes #2682. Thanks @DatMoshu for the detailed report and repro.

While fixing it I audited the other map, item and multi enumerators for the same class of defect. None repeats #2682 itself, but four others turned up. All are fixed here.

## Fixes

- **`StaticTileEnumerator` (#2682)**: `SetMulti` returned `SetTile()` for the first multi whose bounds contained the point. If that multi had no tile at the cell, the enumeration ended and every later multi in the sector was skipped. Movement and line of sight lost a floor that stood inside another multi's bounding box. It now moves on to the next multi.
- **`MultiBoundsEnumerator`**: deduplication used one static `HashSet` that every new query cleared. A query run inside another made the outer query skip multis or return them twice. Each multi is now yielded only from the sector that holds its `Location`; the bounds filter guarantees that sector is visited. A per-enumerator set covers a multi that isn't registered in that sector (components changed after placement). Normal queries allocate nothing.
- **`Item`/`Mobile`/`ClientDistanceEnumerator`**: ring sectors were bounded only by `sx >= _sectorStartX`. Ring cells past the map edge resolved to the shared invalid sector, once per cell. Entities at off-map coordinates were returned more than once, and sectors outside the query were created lazily. The check now covers all four sides. Results match the bounds enumerators, which never returned off-map entities.
- **`BaseBoat.MovingEntitiesEnumerator`**: `GetMovingEntities` handles a null map, but the enumerator constructor still called `boat.Map.GetMobilesInBounds` and threw. A boat with no map now gets empty enumerators.
- **`BaseDoor.ChainEnumerator`**: the walk stopped only on returning to the start door. Links are set in pairs, so `[ChainLink A B C` followed by `[Link B C` leaves A→B→C→B, and enumerating that chain never ended. The walk now stops on any door it has already yielded. Only reachable when `UseChainedFunctionality` is true, which is off in base and in all shipped content.

## Testing

- A regression test for each fix except the boat null guard. Each failed before its fix and passes after.
- Server.Tests 910/910. UOContent.Tests 1185 passed, 2 skipped.
- Test note: the `MultiComponentList(List<MultiTileEntry>)` constructor drops entries after the first whose `Flags` are 0. The new test multis use non-zero flags.

Projects/Server.Tests/Tests/Maps/ItemByDistanceEnumeratorTests.cs

@@ -436,6 +436,39 @@ public class ItemByDistanceEnumeratorTests
         }
     }
 
+    [Fact]
+    public void ItemByDistanceEnumerator_MatchesBoundsEnumeratorAtMapEdge()
+    {
+        var map = Map.Felucca;
+        var center = new Point3D(5, 5, 0);
+        const int range = 18;
+
+        // Off-map items live in the shared invalid sector, which ring cells past the map edge resolve to
+        var items = new TestItem[1];
+        try
+        {
+            items[0] = CreateItem(map, new Point3D(3, -2, 0));
+
+            var byDistance = new List<Item>();
+            foreach (var (item, _) in map.GetItemsInRangeByDistance(center, range))
+            {
+                byDistance.Add(item);
+            }
+
+            var inRange = new List<Item>();
+            foreach (var item in map.GetItemsInRange(center, range))
+            {
+                inRange.Add(item);
+            }
+
+            Assert.Equal(inRange, byDistance);
+        }
+        finally
+        {
+            DeleteAll(items);
+        }
+    }
+
     private static TestItem CreateItem(Map map, Point3D location)
     {
         var Item = new TestItem(World.NewItem);

Projects/Server.Tests/Tests/Maps/MultiEnumeratorTests.cs

@@ -386,6 +386,35 @@ public class MultiEnumeratorTests
         }
     }
 
+    [Fact]
+    public void MultiEnumerator_NestedQueryDoesNotDisturbOuterDeduplication()
+    {
+        var map = Map.Felucca;
+
+        // Components reach two tiles either side, so at x = 1024 the multi spans sectors 63 and 64
+        var spanning = new SpanningMulti();
+        try
+        {
+            spanning.MoveToWorld(new Point3D(1024, 1000, 0), map);
+
+            var found = new List<BaseMulti>();
+            foreach (var multi in map.GetMultisInBounds<BaseMulti>(new Rectangle2D(1000, 990, 48, 20)))
+            {
+                found.Add(multi);
+
+                foreach (var _ in map.GetMultisInBounds<BaseMulti>(new Rectangle2D(3000, 3000, 4, 4)))
+                {
+                }
+            }
+
+            Assert.Equal([spanning], found);
+        }
+        finally
+        {
+            spanning.Delete();
+        }
+    }
+
     private static TestMulti CreateMulti(Map map, Point3D location)
     {
         var multi = new TestMulti();
@@ -408,5 +437,22 @@ public class MultiEnumeratorTests
         {
         }
     }
+
+    private class SpanningMulti : BaseMulti
+    {
+        // Entries after the first need non-zero flags or MultiComponentList drops them
+        private static readonly MultiComponentList _components = new(
+            [
+                new MultiTileEntry(0x1, -2, 0, 0, TileFlag.Background),
+                new MultiTileEntry(0x1, 2, 0, 0, TileFlag.Background)
+            ]
+        );
+
+        public SpanningMulti() : base(0x1)
+        {
+        }
+
+        public override MultiComponentList Components => _components;
+    }
 }
 

Projects/Server.Tests/Tests/Maps/StaticTileEnumeratorTests.cs

@@ -293,6 +293,55 @@ public class StaticTileEnumeratorTests
         }
     }
 
+    [Fact]
+    public void StaticTileEnumerator_MultiWithEmptyCellDoesNotHideLaterMultis()
+    {
+        var map = Map.Felucca;
+        var location = new Point2D(1101, 1100);
+
+        TestMulti outer = null;
+        TestMulti inner = null;
+        try
+        {
+            // Outer's bounds cover the probe cell but it has no tile there; placing it first puts it
+            // ahead of inner in the sector's multi list
+            outer = new TestMulti(World.NewItem, GappedComponents);
+            outer.MoveToWorld(new Point3D(1100, 1100, 0), map);
+
+            inner = new TestMulti(World.NewItem, SingleTileComponents);
+            inner.MoveToWorld(new Point3D(location.X, location.Y, 20), map);
+
+            var tiles = new List<StaticTile>();
+            foreach (var tile in new Map.StaticTileEnumerable(map, location, includeStatics: false, includeMultis: true))
+            {
+                tiles.Add(tile);
+            }
+
+            // Only inner sits at z 20; item ids are masked by tiledata, which CI does not have
+            var single = Assert.Single(tiles);
+            Assert.Equal(20, single.Z);
+        }
+        finally
+        {
+            outer?.Delete();
+            inner?.Delete();
+        }
+    }
+
+    // Entries after the first need non-zero flags or MultiComponentList drops them
+    private static readonly MultiComponentList GappedComponents = new(
+        [
+            new MultiTileEntry(0x1, 0, 0, 0, TileFlag.Background),
+            new MultiTileEntry(0x2, 2, 0, 0, TileFlag.Background)
+        ]
+    );
+
+    private static readonly MultiComponentList SingleTileComponents = new(
+        [
+            new MultiTileEntry(0x3, 0, 0, 0, TileFlag.Background)
+        ]
+    );
+
     private static TestMulti CreateMultiWithComponents(Map map, Point3D location)
     {
         var multi = new TestMulti(World.NewItem);
@@ -302,11 +351,12 @@ public class StaticTileEnumeratorTests
 
     private class TestMulti : BaseMulti
     {
-        public TestMulti(Serial serial) : base(serial)
-        {
-        }
+        private readonly MultiComponentList _components;
 
-        public override MultiComponentList Components => DefaultComponents;
+        public TestMulti(Serial serial, MultiComponentList components = null) : base(serial) =>
+            _components = components ?? DefaultComponents;
+
+        public override MultiComponentList Components => _components;
 
         private static readonly MultiComponentList DefaultComponents = new(
             [

Projects/Server/Maps/Map.ClientByDistanceEnumerator.cs

@@ -85,6 +85,9 @@ public partial class Map
         private Rectangle2D _bounds;
 
         private int _sectorStartX;
+        private int _sectorStartY;
+        private int _sectorEndX;
+        private int _sectorEndY;
         private int _maxRing;
 
         private int _ring;   // -1 = uninitialized, then 0.._maxRing
@@ -115,11 +118,11 @@ public partial class Map
                 var centerSectorX = center.m_X / SectorSize;
                 var centerSectorY = center.m_Y / SectorSize;
 
-                map.CalculateSectors(_bounds, out _sectorStartX, out var sectorStartY, out var sectorEndX, out var sectorEndY);
+                map.CalculateSectors(_bounds, out _sectorStartX, out _sectorStartY, out _sectorEndX, out _sectorEndY);
 
                 // Calculate max ring based on bounds
-                var dx = Math.Max(centerSectorX - _sectorStartX, sectorEndX - centerSectorX);
-                var dy = Math.Max(centerSectorY - sectorStartY, sectorEndY - centerSectorY);
+                var dx = Math.Max(centerSectorX - _sectorStartX, _sectorEndX - centerSectorX);
+                var dy = Math.Max(centerSectorY - _sectorStartY, _sectorEndY - centerSectorY);
                 _maxRing = Math.Max(dx, dy);
             }
 
@@ -202,7 +205,7 @@ public partial class Map
                     _ringIndex = 0;
                     sx = _center.m_X / SectorSize;
                     sy = _center.m_Y / SectorSize;
-                    return sx >= _sectorStartX;
+                    return IsSectorInBounds(sx, sy);
                 }
 
                 sx = sy = 0;
@@ -225,7 +228,7 @@ public partial class Map
                 _ringIndex = nextIndex;
                 CalculatePositionFromIndex(nextIndex, out sx, out sy);
 
-                if (sx >= _sectorStartX)
+                if (IsSectorInBounds(sx, sy))
                 {
                     return true;
                 }
@@ -233,6 +236,11 @@ public partial class Map
         }
 
 
+        // Ring cells outside the clamped sector range would resolve to the shared invalid sector, once per cell
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        private bool IsSectorInBounds(int sx, int sy) =>
+            sx >= _sectorStartX && sx <= _sectorEndX && sy >= _sectorStartY && sy <= _sectorEndY;
+
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         private void CalculatePositionFromIndex(int index, out int x, out int y)
         {

Projects/Server/Maps/Map.ItemByDistanceEnumerator.cs

@@ -113,6 +113,9 @@ public partial class Map
         private Rectangle2D _bounds;
 
         private int _sectorStartX;
+        private int _sectorStartY;
+        private int _sectorEndX;
+        private int _sectorEndY;
         private int _maxRing;
 
         private int _ring;   // -1 = uninitialized, then 0.._maxRing
@@ -143,11 +146,11 @@ public partial class Map
                 var centerSectorX = center.m_X / SectorSize;
                 var centerSectorY = center.m_Y / SectorSize;
 
-                map.CalculateSectors(_bounds, out _sectorStartX, out var sectorStartY, out var sectorEndX, out var sectorEndY);
+                map.CalculateSectors(_bounds, out _sectorStartX, out _sectorStartY, out _sectorEndX, out _sectorEndY);
 
                 // Calculate max ring based on bounds
-                var dx = Math.Max(centerSectorX - _sectorStartX, sectorEndX - centerSectorX);
-                var dy = Math.Max(centerSectorY - sectorStartY, sectorEndY - centerSectorY);
+                var dx = Math.Max(centerSectorX - _sectorStartX, _sectorEndX - centerSectorX);
+                var dy = Math.Max(centerSectorY - _sectorStartY, _sectorEndY - centerSectorY);
                 _maxRing = Math.Max(dx, dy);
             }
 
@@ -228,7 +231,7 @@ public partial class Map
                     _ringIndex = 0;
                     sx = _center.m_X / SectorSize;
                     sy = _center.m_Y / SectorSize;
-                    return sx >= _sectorStartX;
+                    return IsSectorInBounds(sx, sy);
                 }
 
                 sx = sy = 0;
@@ -251,7 +254,7 @@ public partial class Map
                 _ringIndex = nextIndex;
                 CalculatePositionFromIndex(nextIndex, out sx, out sy);
 
-                if (sx >= _sectorStartX)
+                if (IsSectorInBounds(sx, sy))
                 {
                     return true;
                 }
@@ -259,6 +262,11 @@ public partial class Map
         }
 
 
+        // Ring cells outside the clamped sector range would resolve to the shared invalid sector, once per cell
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        private bool IsSectorInBounds(int sx, int sy) =>
+            sx >= _sectorStartX && sx <= _sectorEndX && sy >= _sectorStartY && sy <= _sectorEndY;
+
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         private void CalculatePositionFromIndex(int index, out int x, out int y)
         {

Projects/Server/Maps/Map.MobileByDistanceEnumerator.cs

@@ -108,6 +108,9 @@ public partial class Map
         private Rectangle2D _bounds;
 
         private int _sectorStartX;
+        private int _sectorStartY;
+        private int _sectorEndX;
+        private int _sectorEndY;
         private int _maxRing;
 
         private int _ring;   // -1 = uninitialized, then 0.._maxRing
@@ -138,11 +141,11 @@ public partial class Map
                 var centerSectorX = center.m_X / SectorSize;
                 var centerSectorY = center.m_Y / SectorSize;
 
-                map.CalculateSectors(_bounds, out _sectorStartX, out var sectorStartY, out var sectorEndX, out var sectorEndY);
+                map.CalculateSectors(_bounds, out _sectorStartX, out _sectorStartY, out _sectorEndX, out _sectorEndY);
 
                 // Calculate max ring based on bounds
-                var dx = Math.Max(centerSectorX - _sectorStartX, sectorEndX - centerSectorX);
-                var dy = Math.Max(centerSectorY - sectorStartY, sectorEndY - centerSectorY);
+                var dx = Math.Max(centerSectorX - _sectorStartX, _sectorEndX - centerSectorX);
+                var dy = Math.Max(centerSectorY - _sectorStartY, _sectorEndY - centerSectorY);
                 _maxRing = Math.Max(dx, dy);
             }
 
@@ -223,7 +226,7 @@ public partial class Map
                     _ringIndex = 0;
                     sx = _center.m_X / SectorSize;
                     sy = _center.m_Y / SectorSize;
-                    return sx >= _sectorStartX;
+                    return IsSectorInBounds(sx, sy);
                 }
 
                 sx = sy = 0;
@@ -246,7 +249,7 @@ public partial class Map
                 _ringIndex = nextIndex;
                 CalculatePositionFromIndex(nextIndex, out sx, out sy);
 
-                if (sx >= _sectorStartX)
+                if (IsSectorInBounds(sx, sy))
                 {
                     return true;
                 }
@@ -254,6 +257,11 @@ public partial class Map
         }
 
 
+        // Ring cells outside the clamped sector range would resolve to the shared invalid sector, once per cell
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        private bool IsSectorInBounds(int sx, int sy) =>
+            sx >= _sectorStartX && sx <= _sectorEndX && sy >= _sectorStartY && sy <= _sectorEndY;
+
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         private void CalculatePositionFromIndex(int index, out int x, out int y)
         {

Projects/Server/Maps/Map.MultiEnumerator.cs

@@ -85,8 +85,6 @@ public partial class Map
     public MultiBoundsEnumerable<T> GetMultisInBounds<T>(Rectangle2D bounds, bool makeBoundsInclusive = false) where T : BaseMulti =>
         new(this, bounds, makeBoundsInclusive);
 
-    private static readonly HashSet<Serial> _sharedDupes = [];
-
     public ref struct MultiSectorEnumerable<T>(Map map, Point2D loc) where T : BaseMulti
     {
         public static MultiSectorEnumerable<T> Empty
@@ -193,13 +191,11 @@ public partial class Map
         private int _currentVersion;
         private Sector _currentSector;
         private T _current;
-
+        private HashSet<Serial> _unanchoredSeen;
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public MultiBoundsEnumerator(Map map, Rectangle2D bounds, bool makeBoundsInclusive)
         {
-            _sharedDupes.Clear();
-
             _map = map;
             _bounds = bounds;
 
@@ -255,14 +251,10 @@ public partial class Map
                     while (++_currentIndex < _currentList.Length)
                     {
                         var item = _currentList[_currentIndex];
-                        if (item is T { Deleted: false } o && bounds.Contains(o.Location))
+                        if (item is T { Deleted: false } o && bounds.Contains(o.Location) && ShouldYieldFromCurrentSector(map, o))
                         {
-                            // Multis can span multiple sectors, so we need to deduplicate
-                            if (_sharedDupes.Add(o.Serial))
-                            {
-                                _current = o;
-                                return true;
-                            }
+                            _current = o;
+                            return true;
                         }
                     }
                 }
@@ -290,6 +282,27 @@ public partial class Map
             }
         }
 
+        // A multi is listed in every sector it spans, but its location is in exactly one sector, which the bounds
+        // filter guarantees is visited. Yield it from that sector only. A multi missing from its location's sector
+        // (components changed after it was placed) falls back to a per-enumerator set, so nested queries stay independent.
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        private bool ShouldYieldFromCurrentSector(Map map, T multi)
+        {
+            var home = map.GetSector(map.Bound(multi.Location));
+
+            if (home == _currentSector)
+            {
+                return true;
+            }
+
+            if (home.Multis.Contains(multi))
+            {
+                return false;
+            }
+
+            return (_unanchoredSeen ??= []).Add(multi.Serial);
+        }
+
         public T Current
         {
             [MethodImpl(MethodImplOptions.AggressiveInlining)]

Projects/Server/Maps/Map.StaticTileEnumerator.cs

@@ -99,7 +99,12 @@ public partial class Map
                 {
                     _tiles = multi.Components.Tiles[offsetX][offsetY];
                     _index = -1;
-                    return SetTile();
+
+                    // A multi can have no tile at this cell; later multis in the sector still may
+                    if (SetTile())
+                    {
+                        return true;
+                    }
                 }
             }
 

Projects/UOContent.Tests/Tests/Items/Doors/DoorChainTests.cs

@@ -0,0 +1,40 @@
+using System.Collections.Generic;
+using Server.Items;
+using Xunit;
+
+namespace UOContent.Tests;
+
+[Collection("Sequential UOContent Tests")]
+public class DoorChainTests
+{
+    [Fact]
+    public void GetChain_LoopNotThroughStart_YieldsEachDoorOnce()
+    {
+        var a = new MetalDoor(DoorFacing.WestCW);
+        var b = new MetalDoor(DoorFacing.WestCW);
+        var c = new MetalDoor(DoorFacing.WestCW);
+
+        try
+        {
+            // A -> B -> C -> B: the chain never returns to A
+            a.Link = b;
+            b.Link = c;
+            c.Link = b;
+
+            var chain = new List<BaseDoor>();
+            foreach (var door in a.GetChain())
+            {
+                chain.Add(door);
+                Assert.True(chain.Count <= 3, "Chain enumeration did not terminate");
+            }
+
+            Assert.Equal([a, b, c], chain);
+        }
+        finally
+        {
+            a.Delete();
+            b.Delete();
+            c.Delete();
+        }
+    }
+}

Projects/UOContent/Items/Construction/Doors/BaseDoor.cs

@@ -484,8 +484,9 @@ public abstract partial class BaseDoor : Item, ILockable, ITelekinesisable
             }
             else
             {
-                _current = _current.Link;
-                valid = _current?.Deleted == false && _current != _door;
+                var next = _current.Link;
+                valid = next?.Deleted == false && !IsVisited(next);
+                _current = next;
             }
 
             if (!valid)
@@ -497,6 +498,27 @@ public abstract partial class BaseDoor : Item, ILockable, ITelekinesisable
             return valid;
         }
 
+        // Links are set pairwise by GMs, so a chain can loop back to a door other than the start
+        private readonly bool IsVisited(BaseDoor door)
+        {
+            var visited = _door;
+
+            while (true)
+            {
+                if (visited == door)
+                {
+                    return true;
+                }
+
+                if (visited == _current || visited == null)
+                {
+                    return false;
+                }
+
+                visited = visited.Link;
+            }
+        }
+
         public BaseDoor Current
         {
             [MethodImpl(MethodImplOptions.AggressiveInlining)]

Projects/UOContent/Multis/Boats/BaseBoat.cs

@@ -1924,8 +1924,18 @@ namespace Server.Multis
                 _includeBoat = includeBoat;
                 _boat = boat;
 
-                _mobiles = boat.Map.GetMobilesInBounds(bounds).GetEnumerator();
-                _items = boat.Map.GetItemsInBounds(bounds).GetEnumerator();
+                var map = boat.Map;
+
+                if (map == null)
+                {
+                    _mobiles = Map.MobileBoundsEnumerable<Mobile>.Empty.GetEnumerator();
+                    _items = Map.ItemBoundsEnumerable<Item>.Empty.GetEnumerator();
+                }
+                else
+                {
+                    _mobiles = map.GetMobilesInBounds(bounds).GetEnumerator();
+                    _items = map.GetItemsInBounds(bounds).GetEnumerator();
+                }
                 _iterateMobiles = true;
                 _iterateItems = true;
             }