Fix no fewer than two issues with the "closest reachable unchecked lo…

…cation" solver:

1. The first location in each region was being double-counted for the purposes of path weight calculations.
2. For unknown and, perhaps, unknowable reasons, paths that go backward through a region were getting inappropriately adjusted too low.
3. Paths that enter a filler region only to turn right back around and exit to a different region in the direction that we just came from would count as walking through the entire filler region.

The third one is dubious because ALL filler regions in our map connect EXACTLY two landmark regions, and ALWAYS on opposite sides, so this probably would have never been seen.

Resolves #92

src/Autopelago/RouteCalculator.cs

@@ -141,7 +141,7 @@ public bool CanReach(LocationDefinitionModel location)
                 {
                     Direction.TowardsGoal => forwardLocationsInCurrentRegion,
                     _ => backwardLocationsInCurrentRegion,
-                } + 1);
+                } + 1); // +1 gets us to the first location in the next region.
             }
         }
 
@@ -158,8 +158,7 @@ public bool CanReach(LocationDefinitionModel location)
             {
                 if (direction == Direction.TowardsGoal)
                 {
-                    int max = Math.Min(checkedLocationsInConnectedRegion.Length, bestDistance - extraDistance);
-                    for (int i = 0; i < max; i++)
+                    for (int i = 0; i < checkedLocationsInConnectedRegion.Length && extraDistance + i < bestDistance; i++)
                     {
                         if (checkedLocationsInConnectedRegion[i])
                         {
@@ -168,40 +167,59 @@ public bool CanReach(LocationDefinitionModel location)
 
                         bestDistance = extraDistance + i;
                         bestLocationKey = new() { RegionKey = connectedRegion.Key, N = i };
-                        break;
                     }
                 }
                 else
                 {
-                    int min = bestDistance == int.MaxValue ? 0 : Math.Max(0, extraDistance - bestDistance);
-                    for (int i = checkedLocationsInConnectedRegion.Length - 1; i >= min; --i)
+                    for (int i = 0; i < checkedLocationsInConnectedRegion.Length && extraDistance + i < bestDistance; i++)
                     {
-                        if (checkedLocationsInConnectedRegion[i])
+                        if (checkedLocationsInConnectedRegion[^(i + 1)])
                         {
                             continue;
                         }
 
-                        bestDistance = checkedLocationsInConnectedRegion.Length - i - i + extraDistance;
-                        bestLocationKey = new() { RegionKey = connectedRegion.Key, N = i };
+                        bestDistance = extraDistance + i;
+                        bestLocationKey = new() { RegionKey = connectedRegion.Key, N = (^(i + 1)).GetOffset(checkedLocationsInConnectedRegion.Length) };
                         break;
                     }
                 }
             }
 
-            extraDistance += checkedLocationsInConnectedRegion.Length + 1;
-            if (extraDistance >= bestDistance)
-            {
-                continue;
-            }
-
+            // routes from here will take at least one step into the next region, so add that here.
+            // the exact details of how far we will need to walk through the current region to get
+            // to the next one will depend on the direction we face when entering the next region.
+            ++extraDistance;
             foreach ((RegionDefinitionModel nextConnectedRegion, Direction nextDirection) in _connectedRegions[connectedRegion])
             {
+                // at the time of writing, we technically don't need this to be conditional: only
+                // landmark regions can connect to more than one other region in a given direction,
+                // and it costs the same to route through them regardless of which direction a path
+                // turns through them. it costs practically nothing to do this check, though, so I'm
+                // going to do it if for no other reason than to make it possible to fix a few odd
+                // cases in the sewer level that look different on the map than the actual path that
+                // the game implements by creating new filler regions that connect to other filler
+                // regions to make everything visually consistent.
+                //
+                // the -1 part below is because the path up to this point has already gotten us to
+                // the first location in the region we'd be coming from, so the only thing that we
+                // need to do is add on the number of ADDITIONAL locations within our region that we
+                // need to follow to get to the location in the current region that exits to the
+                // connected region.
+                int nextExtraDistance = nextDirection == direction
+                    ? extraDistance + checkedLocationsInConnectedRegion.Length - 1
+                    : extraDistance;
+
+                if (nextExtraDistance >= bestDistance)
+                {
+                    continue;
+                }
+
                 if (_visitedRegions.Add(nextConnectedRegion.Key) &&
                     (_fillerRegions.ContainsKey(nextConnectedRegion.Key) ||
                      _checkedLocationsBitmap[nextConnectedRegion.Key][0] ||
                      _clearableLandmarks.Contains(nextConnectedRegion.Key)))
                 {
-                    _pq.Enqueue((nextConnectedRegion, nextDirection), extraDistance);
+                    _pq.Enqueue((nextConnectedRegion, nextDirection), nextExtraDistance);
                 }
             }
         }

tests/Autopelago.Test/GameTests.cs

@@ -729,6 +729,57 @@ public void ReceiveItemsShouldApplyAuras()
         });
     }
 
+    [Test]
+    [Property("Regression", 92)]
+    public void RegressionTestPathingErrors()
+    {
+        Game game = new(s_highRolls);
+        game.InitializeCheckedLocations([
+            GameDefinitions.Instance.LocationsByName["Basketball"],
+            GameDefinitions.Instance.LocationsByName["Angry Turtles"],
+            GameDefinitions.Instance.LocationsByName["Restaurant"],
+            GameDefinitions.Instance.LocationsByName["Bowling Ball Door"],
+            GameDefinitions.Instance.LocationsByName["Captured Goldfish"],
+            .. GameDefinitions.Instance.FillerRegions["Menu"].Locations, // "Before Basketball"
+            .. GameDefinitions.Instance.FillerRegions["before_prawn_stars"].Locations,
+            .. GameDefinitions.Instance.FillerRegions["before_angry_turtles"].Locations,
+            .. GameDefinitions.Instance.FillerRegions["after_restaurant"].Locations,
+            .. GameDefinitions.Instance.FillerRegions["before_captured_goldfish"].Locations,
+
+            .. GameDefinitions.Instance.FillerRegions["after_pirate_bake_sale"].Locations.AsSpan(3..),
+            .. GameDefinitions.Instance.FillerRegions["before_computer_interface"].Locations.AsSpan(..3),
+        ]);
+        game.InitializeReceivedItems([
+            GameDefinitions.Instance.ItemsByName["Giant Novelty Scissors"],
+            GameDefinitions.Instance.ItemsByName["Ninja Rat"],
+            GameDefinitions.Instance.ItemsByName["Chef Rat"],
+            GameDefinitions.Instance.ItemsByName["Computer Rat"],
+            GameDefinitions.Instance.ItemsByName["Notorious R.A.T."],
+            .. Enumerable.Repeat(s_normalRat, 14),
+        ]);
+
+        game.ArbitrarilyModifyState(g => g.CurrentLocation, GameDefinitions.Instance.LocationsByName["Before Goldfish #2"]);
+        game.ArbitrarilyModifyState(g => g.TargetLocation, GameDefinitions.Instance.LocationsByName["Before Computer Interface #4"]);
+
+        // the rat has everything it needs to make a few location checks. make sure it does that and
+        // doesn't instead go into a loop like it was seen doing before.
+        int initialCheckedLocationCount = game.CheckedLocations.Count;
+        HashSet<LocationKey> locationsVisited = [];
+        while (true)
+        {
+            game.Advance();
+            if (game.CheckedLocations.Count > initialCheckedLocationCount)
+            {
+                break;
+            }
+
+            // this part ensures that the test will not loop forever: a LocationDefinitionModel does
+            // exist for CurrentLocation, and only <1000 of those ever get created.
+            Assert.That(locationsVisited, Does.Not.Contain(game.CurrentLocation.Key));
+            locationsVisited.Add(game.CurrentLocation.Key);
+        }
+    }
+
     private static FrozenDictionary<LocationDefinitionModel, ArchipelagoItemFlags> CreateSpoiler(ReadOnlySpan<(LocationDefinitionModel Location, ArchipelagoItemFlags Flags)> defined)
     {
         Dictionary<LocationDefinitionModel, ArchipelagoItemFlags> result = GameDefinitions.Instance.LocationsByName.Values.ToDictionary(l => l, _ => ArchipelagoItemFlags.None);