diff --git a/Projects/Server.Tests/Tests/Maps/ItemByDistanceEnumeratorTests.cs b/Projects/Server.Tests/Tests/Maps/ItemByDistanceEnumeratorTests.cs index 458409705..fb12d0ab6 100644 --- a/Projects/Server.Tests/Tests/Maps/ItemByDistanceEnumeratorTests.cs +++ b/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(); + foreach (var (item, _) in map.GetItemsInRangeByDistance(center, range)) + { + byDistance.Add(item); + } + + var inRange = new List(); + 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); diff --git a/Projects/Server.Tests/Tests/Maps/MultiEnumeratorTests.cs b/Projects/Server.Tests/Tests/Maps/MultiEnumeratorTests.cs index cb06fe48d..c064623bf 100644 --- a/Projects/Server.Tests/Tests/Maps/MultiEnumeratorTests.cs +++ b/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(); + foreach (var multi in map.GetMultisInBounds(new Rectangle2D(1000, 990, 48, 20))) + { + found.Add(multi); + + foreach (var _ in map.GetMultisInBounds(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; + } } diff --git a/Projects/Server.Tests/Tests/Maps/StaticTileEnumeratorTests.cs b/Projects/Server.Tests/Tests/Maps/StaticTileEnumeratorTests.cs index 1ac335f7a..edce65c89 100644 --- a/Projects/Server.Tests/Tests/Maps/StaticTileEnumeratorTests.cs +++ b/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(); + 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( [ diff --git a/Projects/Server/Maps/Map.ClientByDistanceEnumerator.cs b/Projects/Server/Maps/Map.ClientByDistanceEnumerator.cs index eb79410ae..54d61797c 100644 --- a/Projects/Server/Maps/Map.ClientByDistanceEnumerator.cs +++ b/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) { diff --git a/Projects/Server/Maps/Map.ItemByDistanceEnumerator.cs b/Projects/Server/Maps/Map.ItemByDistanceEnumerator.cs index 207928f68..91da4969f 100644 --- a/Projects/Server/Maps/Map.ItemByDistanceEnumerator.cs +++ b/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) { diff --git a/Projects/Server/Maps/Map.MobileByDistanceEnumerator.cs b/Projects/Server/Maps/Map.MobileByDistanceEnumerator.cs index e5a8d0b91..92e9dcbb2 100644 --- a/Projects/Server/Maps/Map.MobileByDistanceEnumerator.cs +++ b/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) { diff --git a/Projects/Server/Maps/Map.MultiEnumerator.cs b/Projects/Server/Maps/Map.MultiEnumerator.cs index b0e069a75..bbd04a53c 100644 --- a/Projects/Server/Maps/Map.MultiEnumerator.cs +++ b/Projects/Server/Maps/Map.MultiEnumerator.cs @@ -85,8 +85,6 @@ public partial class Map public MultiBoundsEnumerable GetMultisInBounds(Rectangle2D bounds, bool makeBoundsInclusive = false) where T : BaseMulti => new(this, bounds, makeBoundsInclusive); - private static readonly HashSet _sharedDupes = []; - public ref struct MultiSectorEnumerable(Map map, Point2D loc) where T : BaseMulti { public static MultiSectorEnumerable Empty @@ -193,13 +191,11 @@ public partial class Map private int _currentVersion; private Sector _currentSector; private T _current; - + private HashSet _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)] diff --git a/Projects/Server/Maps/Map.StaticTileEnumerator.cs b/Projects/Server/Maps/Map.StaticTileEnumerator.cs index 27075b59d..73fec0684 100644 --- a/Projects/Server/Maps/Map.StaticTileEnumerator.cs +++ b/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; + } } } diff --git a/Projects/UOContent.Tests/Tests/Items/Doors/DoorChainTests.cs b/Projects/UOContent.Tests/Tests/Items/Doors/DoorChainTests.cs new file mode 100644 index 000000000..ed399714d --- /dev/null +++ b/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(); + 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(); + } + } +} diff --git a/Projects/UOContent/Items/Construction/Doors/BaseDoor.cs b/Projects/UOContent/Items/Construction/Doors/BaseDoor.cs index ef779760b..e95e87c13 100644 --- a/Projects/UOContent/Items/Construction/Doors/BaseDoor.cs +++ b/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)] diff --git a/Projects/UOContent/Multis/Boats/BaseBoat.cs b/Projects/UOContent/Multis/Boats/BaseBoat.cs index ed618b91a..743790a1e 100644 --- a/Projects/UOContent/Multis/Boats/BaseBoat.cs +++ b/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.Empty.GetEnumerator(); + _items = Map.ItemBoundsEnumerable.Empty.GetEnumerator(); + } + else + { + _mobiles = map.GetMobilesInBounds(bounds).GetEnumerator(); + _items = map.GetItemsInBounds(bounds).GetEnumerator(); + } _iterateMobiles = true; _iterateItems = true; }