Do not duplicate send in-transit location checks

Archipelago.MultiClient.Net.Tests/LocationCheckHelperFixture.cs

@@ -394,7 +394,7 @@ public void Should_not_send_location_checks_already_confirmed_by_the_server()
         }
 
 		[Test]
-        public void Should_re_send_location_checks_already_checked_but_not_confirmed_by_server()
+		public void Should_not_re_send_location_checks_while_waiting_for_server_confirmation()
 		{
 			var socket = Substitute.For<IArchipelagoSocketHelper>();
 			var itemInfoResolver = Substitute.For<IItemInfoResolver>();
@@ -415,14 +415,48 @@ public void Should_re_send_location_checks_already_checked_but_not_confirmed_by_
 
 			socket.Received().SendPacket(Arg.Is<LocationChecksPacket>(p => p.Locations.Length == 2));
 
-			sut.CompleteLocationChecks(4);
-
-			socket.Received().SendPacket(Arg.Is<LocationChecksPacket>(p => p.Locations.Length == 3));
-
-			sut.CompleteLocationChecks(5, 6);
-
-			socket.Received().SendPacket(Arg.Is<LocationChecksPacket>(p => p.Locations.Length == 5));
-		}
+			sut.CompleteLocationChecks(4);
+
+			socket.Received().SendPacket(Arg.Is<LocationChecksPacket>(p => p.Locations.Length == 1 && p.Locations[0] == 4));
+
+			sut.CompleteLocationChecks(2, 3, 4);
+
+			socket.Received(2).SendPacket(Arg.Any<LocationChecksPacket>());
+
+			var updatePacket = new RoomUpdatePacket { CheckedLocations = new long[] { 2, 3 } };
+			socket.PacketReceived += Raise.Event<ArchipelagoSocketHelperDelagates.PacketReceivedHandler>(updatePacket);
+
+			sut.CompleteLocationChecks(2, 3, 5, 6);
+
+			socket.Received().SendPacket(Arg.Is<LocationChecksPacket>(p =>
+				p.Locations.Length == 2 && p.Locations.Contains(5) && p.Locations.Contains(6)));
+		}
+
+		[Test]
+		public void Should_clear_pending_location_checks_when_connected()
+		{
+			var socket = Substitute.For<IArchipelagoSocketHelper>();
+			var itemInfoResolver = Substitute.For<IItemInfoResolver>();
+			var connectionInfo = Substitute.For<IConnectionInfoProvider>();
+			var players = Substitute.For<IPlayerHelper>();
+
+			ILocationCheckHelper sut = new LocationCheckHelper(socket, itemInfoResolver, connectionInfo, players);
+
+			var connectedPacket = new ConnectedPacket
+			{
+				LocationsChecked = new long[0],
+				MissingChecks = new long[] { 1 }
+			};
+
+			socket.PacketReceived += Raise.Event<ArchipelagoSocketHelperDelagates.PacketReceivedHandler>(connectedPacket);
+			sut.CompleteLocationChecks(1);
+
+			socket.PacketReceived += Raise.Event<ArchipelagoSocketHelperDelagates.PacketReceivedHandler>(connectedPacket);
+			sut.CompleteLocationChecks(1);
+
+			socket.Received(2).SendPacket(Arg.Is<LocationChecksPacket>(p =>
+				p.Locations.Length == 1 && p.Locations[0] == 1));
+		}
 
         [Test]
         public void Should_not_send_check_if_no_new_locations_are_checked()

Archipelago.MultiClient.Net/ConcurrentCollection/ConcurrentHashSet.cs

@@ -10,9 +10,13 @@ class ConcurrentHashSet<T> : IConcurrentHashSet<T>
     {
         readonly ConcurrentDictionary<T, byte> set = new ConcurrentDictionary<T, byte>();
 
-        public bool TryAdd(T item) => set.TryAdd(item, 0);
-
-        public bool Contains(T item) => set.ContainsKey(item);
+        public bool TryAdd(T item) => set.TryAdd(item, 0);
+
+        public bool TryRemove(T item) => set.TryRemove(item, out _);
+
+        public void Clear() => set.Clear();
+
+        public bool Contains(T item) => set.ContainsKey(item);
 
         public void UnionWith(T[] otherSet)
         {

Archipelago.MultiClient.Net/ConcurrentCollection/ConcurrentHashSet_net35.cs

@@ -11,7 +11,7 @@ class ConcurrentHashSet<T> : IConcurrentHashSet<T>
 
         readonly object lockObject = new object();
 
-        public bool TryAdd(T item)
+        public bool TryAdd(T item)
         {
             lock (lockObject)
             {
@@ -23,9 +23,25 @@ public bool TryAdd(T item)
 
                 return false;
             }
-        }
-
-        public bool Contains(T item)
+        }
+
+        public bool TryRemove(T item)
+        {
+            lock (lockObject)
+            {
+                return set.Remove(item);
+            }
+        }
+
+        public void Clear()
+        {
+            lock (lockObject)
+            {
+                set.Clear();
+            }
+        }
+
+        public bool Contains(T item)
         {
             lock (lockObject)
             {

Archipelago.MultiClient.Net/ConcurrentCollection/IConcurrentHashSet.cs

@@ -2,10 +2,12 @@
 
 interface IConcurrentHashSet<T>
 {
-    bool TryAdd(T item);
-    bool Contains(T item);
+    bool TryAdd(T item);
+    bool TryRemove(T item);
+    void Clear();
+    bool Contains(T item);
     void UnionWith(T[] otherSet);
     T[] ToArray();
     ReadOnlyCollection<T> AsToReadOnlyCollection();
     ReadOnlyCollection<T> AsToReadOnlyCollectionExcept(IConcurrentHashSet<T> otherSet);
-}
+}

Archipelago.MultiClient.Net/Helpers/LocationCheckHelper.cs

@@ -243,8 +243,9 @@ public class LocationCheckHelper : ILocationCheckHelper
 		public event CheckedLocationsUpdatedHandler CheckedLocationsUpdated;
 
 		readonly IConcurrentHashSet<long> allLocations = new ConcurrentHashSet<long>();
-        readonly IConcurrentHashSet<long> locationsChecked = new ConcurrentHashSet<long>();
-        readonly IConcurrentHashSet<long> serverConfirmedChecks = new ConcurrentHashSet<long>();
+        readonly IConcurrentHashSet<long> locationsChecked = new ConcurrentHashSet<long>();
+        readonly IConcurrentHashSet<long> serverConfirmedChecks = new ConcurrentHashSet<long>();
+		readonly IConcurrentHashSet<long> pendingLocationChecks = new ConcurrentHashSet<long>();
 		ReadOnlyCollection<long> missingLocations = new ReadOnlyCollection<long>(new long[0]);
 
         readonly IArchipelagoSocketHelper socket;
@@ -280,8 +281,9 @@ void Socket_PacketReceived(ArchipelagoPacketBase packet)
         {
             switch (packet)
             {
-                case ConnectedPacket connectedPacket:
-                    allLocations.UnionWith(connectedPacket.LocationsChecked);
+                case ConnectedPacket connectedPacket:
+					pendingLocationChecks.Clear();
+                    allLocations.UnionWith(connectedPacket.LocationsChecked);
                     allLocations.UnionWith(connectedPacket.MissingChecks);
                     serverConfirmedChecks.UnionWith(connectedPacket.LocationsChecked);
 
@@ -292,8 +294,12 @@ void Socket_PacketReceived(ArchipelagoPacketBase packet)
                 case RoomUpdatePacket updatePacket:
                     CheckLocations(updatePacket.CheckedLocations);
 
-					if (updatePacket.CheckedLocations != null)
-						serverConfirmedChecks.UnionWith(updatePacket.CheckedLocations);
+					if (updatePacket.CheckedLocations != null)
+					{
+						serverConfirmedChecks.UnionWith(updatePacket.CheckedLocations);
+						foreach (var locationId in updatePacket.CheckedLocations)
+							pendingLocationChecks.TryRemove(locationId);
+					}
 					break;
 #if NET35
                 case LocationInfoPacket locationInfoPacket:
@@ -370,8 +376,8 @@ public void CompleteLocationChecksAsync(Action<bool> onComplete, params long[] i
 
             var packet = GetLocationChecksPacket();
 
-            if (packet.Locations.Any())
-				socket.SendPacketAsync(GetLocationChecksPacket(), onComplete);
+            if (packet.Locations.Any())
+				socket.SendPacketAsync(packet, onComplete);
 		}
 #elif NET40
 	    /// <inheritdoc/>
@@ -403,13 +409,15 @@ public Task CompleteLocationChecksAsync(params long[] ids)
 		}
 #endif
 
-		LocationChecksPacket GetLocationChecksPacket() =>
-		    new LocationChecksPacket
-		    {
-			    Locations = locationsChecked
-				    .AsToReadOnlyCollectionExcept(serverConfirmedChecks)
-				    .ToArray()
-		    };
+		LocationChecksPacket GetLocationChecksPacket()
+		{
+			var locations = locationsChecked
+				.AsToReadOnlyCollectionExcept(serverConfirmedChecks)
+				.Where(pendingLocationChecks.TryAdd)
+				.ToArray();
+
+			return new LocationChecksPacket { Locations = locations };
+		}
 
 #if NET35
 	    /// <inheritdoc/>