diff --git a/OpenRA.Game/Primitives/SpatiallyPartitioned.cs b/OpenRA.Game/Primitives/SpatiallyPartitioned.cs index 440d48bb43..85ce02c31e 100644 --- a/OpenRA.Game/Primitives/SpatiallyPartitioned.cs +++ b/OpenRA.Game/Primitives/SpatiallyPartitioned.cs @@ -10,17 +10,20 @@ #endregion using System; +using System.Collections; using System.Collections.Generic; +using System.Runtime.InteropServices; namespace OpenRA.Primitives { - public sealed class SpatiallyPartitioned + public sealed class SpatiallyPartitioned : IDictionary { + static readonly Action, T, Rectangle> AddItem = (bin, item, bounds) => bin.Add(item, bounds); + static readonly Action, T, Rectangle> RemoveItem = (bin, item, bounds) => bin.Remove(item); + readonly int rows, cols, binSize; readonly Dictionary[] itemBoundsBins; readonly Dictionary itemBounds = new(); - readonly Action, T, Rectangle> addItem = (bin, actor, bounds) => bin.Add(actor, bounds); - readonly Action, T, Rectangle> removeItem = (bin, actor, bounds) => bin.Remove(actor); public SpatiallyPartitioned(int width, int height, int binSize) { @@ -30,41 +33,43 @@ namespace OpenRA.Primitives itemBoundsBins = Exts.MakeArray(rows * cols, _ => new Dictionary()); } - static void ValidateBounds(T actor, Rectangle bounds) + static void ValidateBounds(T item, Rectangle bounds) { if (bounds.Width == 0 || bounds.Height == 0) - throw new ArgumentException($"Bounds of actor {actor} are empty.", nameof(bounds)); + throw new ArgumentException($"Bounds of {item} are empty.", nameof(bounds)); } public void Add(T item, Rectangle bounds) { ValidateBounds(item, bounds); itemBounds.Add(item, bounds); - MutateBins(item, bounds, addItem); + MutateBins(item, bounds, AddItem); } - public void Update(T item, Rectangle bounds) + public Rectangle this[T item] { - ValidateBounds(item, bounds); - MutateBins(item, itemBounds[item], removeItem); - MutateBins(item, itemBounds[item] = bounds, addItem); + get => itemBounds[item]; + set + { + ValidateBounds(item, value); + + // SAFETY: Dictionary cannot be modified whilst the ref is alive. + ref var bounds = ref CollectionsMarshal.GetValueRefOrAddDefault(itemBounds, item, out var exists); + if (exists) + MutateBins(item, bounds, RemoveItem); + MutateBins(item, bounds = value, AddItem); + } } public bool Remove(T item) { - if (!itemBounds.TryGetValue(item, out var bounds)) + if (!itemBounds.Remove(item, out var bounds)) return false; - MutateBins(item, bounds, removeItem); - itemBounds.Remove(item); + MutateBins(item, bounds, RemoveItem); return true; } - public bool Contains(T item) - { - return itemBounds.ContainsKey(item); - } - Dictionary BinAt(int row, int col) { return itemBoundsBins[row * cols + col]; @@ -88,13 +93,13 @@ namespace OpenRA.Primitives maxCol = Math.Min(cols, Exts.IntegerDivisionRoundingAwayFromZero(right, binSize)); } - void MutateBins(T actor, Rectangle bounds, Action, T, Rectangle> action) + void MutateBins(T item, Rectangle bounds, Action, T, Rectangle> action) { BoundsToBinRowsAndCols(bounds, out var minRow, out var maxRow, out var minCol, out var maxCol); for (var row = minRow; row < maxRow; row++) for (var col = minCol; col < maxCol; col++) - action(BinAt(row, col), actor, bounds); + action(BinAt(row, col), item, bounds); } public IEnumerable At(int2 location) @@ -135,8 +140,30 @@ namespace OpenRA.Primitives } } - public IEnumerable ItemBounds => itemBounds.Values; + public void Clear() + { + itemBounds.Clear(); + foreach (var bin in itemBoundsBins) + bin.Clear(); + } - public IEnumerable Items => itemBounds.Keys; + public ICollection Keys => itemBounds.Keys; + public ICollection Values => itemBounds.Values; + public int Count => itemBounds.Count; + public bool ContainsKey(T item) => itemBounds.ContainsKey(item); + public bool TryGetValue(T key, out Rectangle value) => itemBounds.TryGetValue(key, out value); + public IEnumerator> GetEnumerator() => itemBounds.GetEnumerator(); + + bool ICollection>.IsReadOnly => false; + void ICollection>.Add(KeyValuePair item) => + ((ICollection>)itemBounds).Add(item); + bool ICollection>.Contains(KeyValuePair item) => + ((ICollection>)itemBounds).Contains(item); + void ICollection>.CopyTo(KeyValuePair[] array, int arrayIndex) => + ((ICollection>)itemBounds).CopyTo(array, arrayIndex); + bool ICollection>.Remove(KeyValuePair item) => + ((ICollection>)itemBounds).Remove(item); + IEnumerator IEnumerable.GetEnumerator() => + ((IEnumerable)itemBounds).GetEnumerator(); } } diff --git a/OpenRA.Game/Traits/World/ScreenMap.cs b/OpenRA.Game/Traits/World/ScreenMap.cs index 46e3c2b092..6e19f092cf 100644 --- a/OpenRA.Game/Traits/World/ScreenMap.cs +++ b/OpenRA.Game/Traits/World/ScreenMap.cs @@ -217,11 +217,7 @@ namespace OpenRA.Traits var mouseBounds = a.MouseBounds(worldRenderer); if (!mouseBounds.IsEmpty) { - if (partitionedMouseActors.Contains(a)) - partitionedMouseActors.Update(a, mouseBounds.BoundingRect); - else - partitionedMouseActors.Add(a, mouseBounds.BoundingRect); - + partitionedMouseActors[a] = mouseBounds.BoundingRect; partitionedMouseActorBounds[a] = new ActorBoundsPair(a, mouseBounds); } else @@ -229,12 +225,7 @@ namespace OpenRA.Traits var screenBounds = a.ScreenBounds(worldRenderer).Union(); if (!screenBounds.Size.IsEmpty) - { - if (partitionedRenderableActors.Contains(a)) - partitionedRenderableActors.Update(a, screenBounds); - else - partitionedRenderableActors.Add(a, screenBounds); - } + partitionedRenderableActors[a] = screenBounds; else partitionedRenderableActors.Remove(a); } @@ -255,23 +246,13 @@ namespace OpenRA.Traits { var mouseBounds = fa.MouseBounds; if (!mouseBounds.IsEmpty) - { - if (partitionedMouseFrozenActors[kv.Key].Contains(fa)) - partitionedMouseFrozenActors[kv.Key].Update(fa, mouseBounds.BoundingRect); - else - partitionedMouseFrozenActors[kv.Key].Add(fa, mouseBounds.BoundingRect); - } + partitionedMouseFrozenActors[kv.Key][fa] = mouseBounds.BoundingRect; else partitionedMouseFrozenActors[kv.Key].Remove(fa); var screenBounds = fa.ScreenBounds.Union(); if (!screenBounds.Size.IsEmpty) - { - if (partitionedRenderableFrozenActors[kv.Key].Contains(fa)) - partitionedRenderableFrozenActors[kv.Key].Update(fa, screenBounds); - else - partitionedRenderableFrozenActors[kv.Key].Add(fa, screenBounds); - } + partitionedRenderableFrozenActors[kv.Key][fa] = screenBounds; else partitionedRenderableFrozenActors[kv.Key].Remove(fa); } @@ -293,16 +274,16 @@ namespace OpenRA.Traits public IEnumerable RenderBounds(Player viewer) { - var bounds = partitionedRenderableActors.ItemBounds - .Concat(partitionedRenderableEffects.ItemBounds); + var bounds = partitionedRenderableActors.Values + .Concat(partitionedRenderableEffects.Values); - return viewer != null ? bounds.Concat(partitionedRenderableFrozenActors[viewer].ItemBounds) : bounds; + return viewer != null ? bounds.Concat(partitionedRenderableFrozenActors[viewer].Values) : bounds; } public IEnumerable MouseBounds(Player viewer) { var bounds = partitionedMouseActorBounds.Values.Select(a => a.Bounds); - return viewer != null ? bounds.Concat(partitionedMouseFrozenActors[viewer].Items.Select(fa => fa.MouseBounds)) : bounds; + return viewer != null ? bounds.Concat(partitionedMouseFrozenActors[viewer].Keys.Select(fa => fa.MouseBounds)) : bounds; } } } diff --git a/OpenRA.Mods.Common/Traits/World/EditorActorLayer.cs b/OpenRA.Mods.Common/Traits/World/EditorActorLayer.cs index eb2dde17ae..eef1b53b88 100644 --- a/OpenRA.Mods.Common/Traits/World/EditorActorLayer.cs +++ b/OpenRA.Mods.Common/Traits/World/EditorActorLayer.cs @@ -340,7 +340,7 @@ namespace OpenRA.Mods.Common.Traits public void PopulateRadarSignatureCells(Actor self, List<(CPos Cell, Color Color)> destinationBuffer) { - foreach (var preview in cellMap.Items) + foreach (var preview in cellMap.Keys) foreach (var cell in Footprint(preview)) destinationBuffer.Add((cell, preview.RadarColor)); } diff --git a/OpenRA.Test/OpenRA.Game/SpatiallyPartitionedTest.cs b/OpenRA.Test/OpenRA.Game/SpatiallyPartitionedTest.cs index ae3d30f34f..b8f69384ad 100644 --- a/OpenRA.Test/OpenRA.Game/SpatiallyPartitionedTest.cs +++ b/OpenRA.Test/OpenRA.Game/SpatiallyPartitionedTest.cs @@ -38,7 +38,7 @@ namespace OpenRA.Test Assert.That(partition.At(new int2(3, 2)), Does.Not.Contain(b), "b is present in the wrong location"); Assert.That(partition.At(new int2(3, 3)), Does.Not.Contain(b), "b is present in the wrong location"); - partition.Update(b, new Rectangle(4, 4, 1, 1)); + partition[b] = new Rectangle(4, 4, 1, 1); Assert.That(partition.At(new int2(0, 0)), Does.Contain(a), "a wrongly changed location when b was updated"); Assert.That(partition.At(new int2(4, 4)), Does.Contain(b), "b is not present at the new location in the extreme corner of the partition"); Assert.That(partition.At(new int2(1, 1)), Does.Not.Contain(b), "b is still present at the old location after update"); @@ -79,7 +79,7 @@ namespace OpenRA.Test Assert.That(new[] { a, b }, Is.EquivalentTo(partition.InBox(new Rectangle(-10, -10, 25, 25))), "Searching an area larger than the partition did not return all items"); - partition.Update(b, new Rectangle(4, 4, 1, 1)); + partition[b] = new Rectangle(4, 4, 1, 1); Assert.That(partition.InBox(new Rectangle(0, 0, 1, 1)), Does.Contain(a), "a wrongly changed location when b was updated"); Assert.That(partition.InBox(new Rectangle(4, 4, 1, 1)), Does.Contain(b), "b is not present at the new location in the extreme corner of the partition"); Assert.That(partition.InBox(new Rectangle(1, 1, 1, 1)), Does.Not.Contain(b), "b is still present at the old location after update");