Update MapItems to use multi-key collection. Fixes rendering performa…

…nce issues when trying to look up map items by map coordinate

EOBot/TrainerBot.cs

@@ -116,6 +116,7 @@ protected override async Task DoWorkAsync(CancellationToken ct)
                         ConsoleHelper.WriteMessage(ConsoleHelper.Type.Chat, $"{message.Who}: {message.Message}", ConsoleColor.Cyan);
 
                     _cachedChat = _chatProvider.AllChat[ChatTab.Local].ToHashSet();
+                    Console.Beep(261, 1500);
                 }
 
                 var character = _characterRepository.MainCharacter;
@@ -129,6 +130,9 @@ protected override async Task DoWorkAsync(CancellationToken ct)
                     if (cachedPlayerCount > 0)
                     {
                         ConsoleHelper.WriteMessage(ConsoleHelper.Type.Warning, $"{cachedPlayerCount,7} - Players on map - You may not be able to train here", ConsoleColor.DarkYellow);
+                        Console.Beep(220, 500);
+                        Console.Beep(247, 500);
+                        Console.Beep(220, 500);
                     }
                 }
 

EOLib/Domain/Login/LoginActions.cs

@@ -185,7 +185,7 @@ public async Task<CharacterLoginReply> CompleteCharacterLogin(int sessionID)
 
             _currentMapStateRepository.Characters = data.MapCharacters.Except(new[] { mainCharacter }).ToDictionary(k => k.ID, v => v);
             _currentMapStateRepository.NPCs = new HashSet<NPC.NPC>(data.MapNPCs);
-            _currentMapStateRepository.MapItems = new HashSet<MapItem>(data.MapItems);
+            _currentMapStateRepository.MapItems = new MapEntityCollectionHashSet<MapItem>(item => item.UniqueID, item => new MapCoordinate(item.X, item.Y), data.MapItems);
 
             _playerInfoRepository.PlayerIsInGame = true;
             _characterSessionRepository.ResetState();

EOLib/Domain/Map/CurrentMapStateRepository.cs

@@ -19,7 +19,7 @@ public interface ICurrentMapStateRepository
 
         HashSet<NPC.NPC> NPCs { get; set; }
 
-        HashSet<MapItem> MapItems { get; set; }
+        MapEntityCollectionHashSet<MapItem> MapItems { get; set; }
 
         HashSet<Warp> OpenDoors { get; set;  }
 
@@ -54,7 +54,7 @@ public interface ICurrentMapStateProvider
 
         IReadOnlyCollection<NPC.NPC> NPCs { get; }
 
-        IReadOnlyCollection<MapItem> MapItems { get; }
+        IReadOnlyMapEntityCollection<MapItem> MapItems { get; }
 
         IReadOnlyCollection<Warp> OpenDoors { get; }
 
@@ -90,7 +90,7 @@ public class CurrentMapStateRepository : ICurrentMapStateRepository, ICurrentMap
 
         public HashSet<NPC.NPC> NPCs { get; set; }
 
-        public HashSet<MapItem> MapItems { get; set; }
+        public MapEntityCollectionHashSet<MapItem> MapItems { get; set; }
 
         public HashSet<Warp> OpenDoors { get; set; }
 
@@ -114,7 +114,7 @@ public class CurrentMapStateRepository : ICurrentMapStateRepository, ICurrentMap
 
         IReadOnlyCollection<NPC.NPC> ICurrentMapStateProvider.NPCs => NPCs;
 
-        IReadOnlyCollection<MapItem> ICurrentMapStateProvider.MapItems => MapItems;
+        IReadOnlyMapEntityCollection<MapItem> ICurrentMapStateProvider.MapItems => MapItems;
 
         IReadOnlyCollection<Warp> ICurrentMapStateProvider.OpenDoors => OpenDoors;
 
@@ -135,7 +135,7 @@ public void ResetState()
 
             Characters = new Dictionary<int, Character.Character>();
             NPCs = new HashSet<NPC.NPC>();
-            MapItems = new HashSet<MapItem>();
+            MapItems = new MapEntityCollectionHashSet<MapItem>(x => x.UniqueID, x => new MapCoordinate(x.X, x.Y));
             OpenDoors = new HashSet<Warp>();
             PendingDoors = new HashSet<Warp>();
             VisibleSpikeTraps = new HashSet<MapCoordinate>();

EOLib/Domain/Map/MapCellStateProvider.cs

@@ -6,6 +6,7 @@
 using EOLib.IO.Repositories;
 using Optional;
 using Optional.Collections;
+using System.Collections.Generic;
 using System.Linq;
 
 namespace EOLib.Domain.Map
@@ -43,7 +44,9 @@ public IMapCellState GetCellStateAt(int x, int y)
                 .Where(c => CharacterAtCoordinates(c, x, y))
                 .ToList();
             var npc = _mapStateProvider.NPCs.FirstOrNone(n => NPCAtCoordinates(n, x, y));
-            var items = _mapStateProvider.MapItems.Where(i => i.X == x && i.Y == y).OrderByDescending(i => i.UniqueID);
+            var items = _mapStateProvider.MapItems.TryGetValues(new MapCoordinate(x, y), out var mapItems)
+                ? mapItems.OrderByDescending(i => i.UniqueID)
+                : Enumerable.Empty<MapItem>();
 
             return new MapCellState
             {

EOLib/Domain/Map/MapEntityCollectionHashSet.cs

@@ -0,0 +1,118 @@
+using System;
+using System.Collections;
+using System.Collections.Generic;
+using System.Linq;
+
+namespace EOLib.Domain.Map
+{
+    public class MapEntityCollectionHashSet<TValue> : IReadOnlyMapEntityCollection<TValue>
+    {
+        private readonly Dictionary<int, int> _uniqueIdToHash;
+        private readonly Dictionary<MapCoordinate, HashSet<int>> _mapCoordinateToHashList;
+
+        private readonly Dictionary<int, TValue> _valueSet;
+
+        private readonly Func<TValue, int> _uniqueIdSelector;
+        private readonly Func<TValue, MapCoordinate> _mapCoordinateSelector;
+
+        public MapEntityCollectionHashSet(Func<TValue, int> uniqueIdSelector,
+                                          Func<TValue, MapCoordinate> mapCoordinateSelector)
+        {
+            _uniqueIdToHash = new Dictionary<int, int>();
+            _mapCoordinateToHashList = new Dictionary<MapCoordinate, HashSet<int>>();
+            _valueSet = new Dictionary<int, TValue>();
+
+            _uniqueIdSelector = uniqueIdSelector;
+            _mapCoordinateSelector = mapCoordinateSelector;
+        }
+
+        public MapEntityCollectionHashSet(Func<TValue, int> uniqueIdSelector,
+                                          Func<TValue, MapCoordinate> mapCoordinateSelector,
+                                          IEnumerable<TValue> values)
+            : this(uniqueIdSelector, mapCoordinateSelector)
+        {
+            foreach (var value in values)
+            {
+                Add(value);
+            }
+        }
+
+        public TValue this[int key1] => _valueSet[_uniqueIdToHash[key1]];
+
+        public HashSet<TValue> this[MapCoordinate key2] => new HashSet<TValue>(_mapCoordinateToHashList[key2].Select(x => _valueSet[x]));
+
+        public void Add(TValue value)
+        {
+            var key1 = _uniqueIdSelector.Invoke(value);
+
+            var hash = value.GetHashCode();
+            _uniqueIdToHash[key1] = hash;
+
+            var key2 = _mapCoordinateSelector.Invoke(value);
+            if (!_mapCoordinateToHashList.ContainsKey(key2))
+                _mapCoordinateToHashList.Add(key2, new HashSet<int>());
+
+            _mapCoordinateToHashList[key2].Add(hash);
+            _valueSet[hash] = value;
+        }
+
+        public IEnumerator<TValue> GetEnumerator() => _valueSet.Values.GetEnumerator();
+
+        IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
+
+        public void Remove(TValue value)
+        {
+            var key1 = _uniqueIdSelector.Invoke(value);
+            var key2 = _mapCoordinateSelector.Invoke(value);
+            _uniqueIdToHash.Remove(key1);
+
+            var hash = value.GetHashCode();
+            _mapCoordinateToHashList[key2].Remove(hash);
+            if (_mapCoordinateToHashList[key2].Count == 0)
+                _mapCoordinateToHashList.Remove(key2);
+
+            _valueSet.Remove(hash);
+        }
+
+        public bool TryGetValue(int uniqueId, out TValue value)
+        {
+            value = default;
+
+            if (!_uniqueIdToHash.ContainsKey(uniqueId))
+                return false;
+
+            var hash = _uniqueIdToHash[uniqueId];
+            if (!_valueSet.ContainsKey(hash))
+                return false;
+
+            value = _valueSet[hash];
+
+            return true;
+        }
+
+        public bool TryGetValues(MapCoordinate mapCoordinate, out HashSet<TValue> values)
+        {
+            values = default;
+
+            if (!_mapCoordinateToHashList.ContainsKey(mapCoordinate))
+                return false;
+
+            var hashes = _mapCoordinateToHashList[mapCoordinate];
+            if (!_valueSet.Any(x => hashes.Contains(x.Key)))
+                return false;
+
+            values = new HashSet<TValue>(_mapCoordinateToHashList[mapCoordinate].Select(x => _valueSet[x]));
+
+            return true;
+        }
+    }
+
+    public interface IReadOnlyMapEntityCollection<TValue> : IEnumerable<TValue>
+    {
+        TValue this[int key1] { get; }
+        HashSet<TValue> this[MapCoordinate key2] { get; }
+
+        bool TryGetValue(int key1, out TValue value);
+        bool TryGetValues(MapCoordinate key2, out HashSet<TValue> values);
+    }
+}

EOLib/Domain/Map/MapItem.cs

@@ -1,11 +1,12 @@
 using Amadevus.RecordGenerator;
+using EOLib.IO.Map;
 using Optional;
 using System;
 
 namespace EOLib.Domain.Map
 {
     [Record]
-    public sealed partial class MapItem
+    public sealed partial class MapItem : IMapEntity
     {
         public int UniqueID { get; }
 

EOLib/PacketHandlers/Items/ItemGetHandler.cs

@@ -46,10 +46,10 @@ public override bool HandlePacket(IPacket packet)
             var weight = packet.ReadChar();
             var maxWeight = packet.ReadChar();
 
-            var existing = _characterInventoryRepository.ItemInventory.SingleOrNone(x => x.ItemID == id);
-            existing.MatchSome(x => _characterInventoryRepository.ItemInventory.Remove(x));
+            var existingInventoryItem = _characterInventoryRepository.ItemInventory.SingleOrNone(x => x.ItemID == id);
+            existingInventoryItem.MatchSome(x => _characterInventoryRepository.ItemInventory.Remove(x));
 
-            existing.Map(x => x.WithAmount(x.Amount + amountTaken))
+            existingInventoryItem.Map(x => x.WithAmount(x.Amount + amountTaken))
                 .Match(some: _characterInventoryRepository.ItemInventory.Add,
                        none: () => _characterInventoryRepository.ItemInventory.Add(new InventoryItem(id, amountTaken)));
 
@@ -58,7 +58,7 @@ public override bool HandlePacket(IPacket packet)
                 .WithNewStat(CharacterStat.MaxWeight, maxWeight);
             _characterRepository.MainCharacter = _characterRepository.MainCharacter.WithStats(newStats);
 
-            _mapStateRepository.MapItems.RemoveWhere(x => x.UniqueID == uid);
+            _mapStateRepository.MapItems.Remove(_mapStateRepository.MapItems[uid]);
 
             foreach (var notifier in _mainCharacterEventNotifiers)
             {

EOLib/PacketHandlers/Items/ItemRemoveHandler.cs

@@ -28,7 +28,7 @@ public class ItemRemoveHandler : InGameOnlyPacketHandler
         public override bool HandlePacket(IPacket packet)
         {
             var uid = packet.ReadShort();
-            _currentMapStateRepository.MapItems.RemoveWhere(x => x.UniqueID == uid);
+            _currentMapStateRepository.MapItems.Remove(_currentMapStateRepository.MapItems[uid]);
             return true;
         }
     }

EOLib/PacketHandlers/NPC/NPCSpecHandler.cs

@@ -8,6 +8,7 @@
 using Optional;
 using System;
 using System.Collections.Generic;
+using System.Security.Cryptography;
 
 namespace EOLib.PacketHandlers.NPC
 {
@@ -152,7 +153,9 @@ private void ShowDroppedItem(int playerID, int droppedItemUID, int droppedItemID
                 .WithDropTime(Option.Some(DateTime.Now))
                 .WithOwningPlayerID(Option.Some(playerID));
 
-            _currentMapStateRepository.MapItems.RemoveWhere(item => item.UniqueID == droppedItemUID);
+            if (_currentMapStateRepository.MapItems.TryGetValue(droppedItemID, out var oldItem))
+                _currentMapStateRepository.MapItems.Remove(oldItem);
+
             _currentMapStateRepository.MapItems.Add(mapItem);
 
             foreach (var notifier in _npcActionNotifiers)

EOLib/PacketHandlers/Refresh/RefreshReplyHandler.cs

@@ -58,7 +58,7 @@ public override bool HandlePacket(IPacket packet)
 
             _currentMapStateRepository.Characters = withoutMainCharacter.ToDictionary(k => k.ID, v => v);
             _currentMapStateRepository.NPCs = new HashSet<DomainNPC>(data.NPCs);
-            _currentMapStateRepository.MapItems = new HashSet<MapItem>(data.Items);
+            _currentMapStateRepository.MapItems = new MapEntityCollectionHashSet<MapItem>(item => item.UniqueID, item => new MapCoordinate(item.X, item.Y), data.Items);
 
             _currentMapStateRepository.OpenDoors.Clear();
             _currentMapStateRepository.PendingDoors.Clear();

EOLib/PacketHandlers/Warp/WarpAgreeHandler.cs

@@ -75,7 +75,7 @@ public override bool HandlePacket(IPacket packet)
 
             _currentMapStateRepository.Characters = warpAgreePacketData.Characters.ToDictionary(k => k.ID, v => v);
             _currentMapStateRepository.NPCs = new HashSet<DomainNPC>(warpAgreePacketData.NPCs);
-            _currentMapStateRepository.MapItems = new HashSet<MapItem>(warpAgreePacketData.Items);
+            _currentMapStateRepository.MapItems = new MapEntityCollectionHashSet<MapItem>(item => item.UniqueID, item => new MapCoordinate(item.X, item.Y), warpAgreePacketData.Items);
             _currentMapStateRepository.OpenDoors.Clear();
             _currentMapStateRepository.VisibleSpikeTraps.Clear();
             _currentMapStateRepository.ShowMiniMap = _currentMapStateRepository.ShowMiniMap &&

EndlessClient/Network/UnknownEntitiesRequester.cs

@@ -3,16 +3,22 @@
 using EOLib.Domain.Character;
 using EOLib.Domain.Map;
 using EOLib.Domain.NPC;
+using EOLib.IO.Map;
 using EOLib.Net;
 using EOLib.Net.Communication;
 using Microsoft.Xna.Framework;
 using System;
 using System.Collections.Generic;
+using System.Linq;
+using System.Reflection.Metadata.Ecma335;
 
 namespace EndlessClient.Network
 {
     public class UnknownEntitiesRequester : GameComponent
     {
+        private const int UPPER_SEE_DISTANCE = 11;
+        private const int LOWER_SEE_DISTANCE = 14;
+
         private const double REQUEST_INTERVAL_SECONDS = 1.0;
 
         private readonly IClientWindowSizeProvider _clientWindowSizeProvider;
@@ -119,55 +125,41 @@ private IPacket CreateRequestForPlayers()
 
         private void ClearOutOfRangeActors()
         {
-            // todo: the server should communicate the "seedistance" to clients
-            // for now, disable auto remove of entities in Resizable mode
-            if (_clientWindowSizeProvider.Resizable)
-            {
-                return;
-            }
-
             var mc = _characterProvider.MainCharacter;
 
-            var idsToRemove = new List<int>();
-            foreach (var id in _currentMapStateRepository.Characters.Keys)
-            {
-                var c = _currentMapStateRepository.Characters[id];
-
-                var xDiff = Math.Abs(mc.X - c.X);
-                var yDiff = Math.Abs(mc.Y - c.Y);
-
-                if (c.X < mc.X || c.Y < mc.Y)
-                {
-                    if (xDiff + yDiff > 11)
-                        idsToRemove.Add(id);
-                }
-                else if (xDiff + yDiff > 14)
-                {
-                    idsToRemove.Add(id);
-                }
-            }
+            var entities = _currentMapStateRepository.MapItems.Cast<IMapEntity>()
+                .Concat(_currentMapStateRepository.NPCs)
+                .Concat(_currentMapStateRepository.Characters.Values);
 
-            foreach (var id in idsToRemove)
-                _currentMapStateRepository.Characters.Remove(id);
+            var seeDistanceUpper = (int)((_clientWindowSizeProvider.Height / 480.0) * UPPER_SEE_DISTANCE);
+            var seeDistanceLower = (int)((_clientWindowSizeProvider.Height / 480.0) * LOWER_SEE_DISTANCE);
 
-            var npcsToRemove = new List<NPC>();
-            foreach (var npc in _currentMapStateRepository.NPCs)
+            var entitiesToRemove = new List<IMapEntity>();
+            foreach (var entity in entities)
             {
-                var xDiff = Math.Abs(mc.X - npc.X);
-                var yDiff = Math.Abs(mc.Y - npc.Y);
+                var xDiff = Math.Abs(mc.X - entity.X);
+                var yDiff = Math.Abs(mc.Y - entity.Y);
 
-                if (npc.X < mc.X || npc.Y < mc.Y)
+                if (entity.X < mc.X || entity.Y < mc.Y)
                 {
-                    if (xDiff + yDiff > 11)
-                        npcsToRemove.Add(npc);
+                    if (xDiff + yDiff > seeDistanceUpper)
+                        entitiesToRemove.Add(entity);
                 }
-                else if (xDiff + yDiff > 14)
+                else if (xDiff + yDiff > seeDistanceLower)
                 {
-                    npcsToRemove.Add(npc);
+                    entitiesToRemove.Add(entity);
                 }
             }
 
-            _currentMapStateRepository.NPCs.RemoveWhere(npcsToRemove.Contains);
+            foreach (var entity in entitiesToRemove)
+            {
+                if (entity is Character c)
+                    _currentMapStateRepository.Characters.Remove(c.ID);
+                else if (entity is NPC n)
+                    _currentMapStateRepository.NPCs.Remove(n);
+                else if (entity is MapItem i)
+                    _currentMapStateRepository.MapItems.Remove(i);
+            }
         }
     }
 }