Add fallback item swap for unreachable items

Fill.py

@@ -3,7 +3,7 @@ import typing
 import collections
 import itertools
 
-from BaseClasses import CollectionState, Location, MultiWorld
+from BaseClasses import CollectionState, Location, MultiWorld, Item
 from worlds.generic import PlandoItem
 from worlds.AutoWorld import call_all
 
@@ -12,15 +12,16 @@ class FillError(RuntimeError):
     pass
 
 
-def fill_restrictive(world: MultiWorld, base_state: CollectionState, locations, itempool, single_player_placement=False,
-                     lock=False):
-    def sweep_from_pool():
-        new_state = base_state.copy()
-        for item in itempool:
-            new_state.collect(item, True)
-        new_state.sweep_for_events()
-        return new_state
+def sweep_from_pool(base_state: CollectionState, itempool: list[Item]):
+    new_state = base_state.copy()
+    for item in itempool:
+        new_state.collect(item, True)
+    new_state.sweep_for_events()
+    return new_state
 
+
+def fill_restrictive(world: MultiWorld, base_state: CollectionState, locations, itempool: list[Item], single_player_placement=False,
+                     lock=False):
     unplaced_items = []
     placements = []
 
@@ -29,13 +30,16 @@ def fill_restrictive(world: MultiWorld, base_state: CollectionState, locations,
         reachable_items.setdefault(item.player, []).append(item)
 
     while any(reachable_items.values()) and locations:
-        items_to_place = [items.pop() for items in reachable_items.values() if items]  # grab one item per player
+        # grab one item per player
+        items_to_place = [items.pop()
+                          for items in reachable_items.values() if items]
         for item in items_to_place:
             itempool.remove(item)
-        maximum_exploration_state = sweep_from_pool()
+        maximum_exploration_state = sweep_from_pool(base_state, itempool)
         has_beaten_game = world.has_beaten_game(maximum_exploration_state)
 
         for item_to_place in items_to_place:
+            spot_to_fill: Location = None
             if world.accessibility[item_to_place.player] == 'minimal':
                 perform_access_check = not world.has_beaten_game(maximum_exploration_state,
                                                                  item_to_place.player) if single_player_placement else not has_beaten_game
@@ -45,19 +49,41 @@ def fill_restrictive(world: MultiWorld, base_state: CollectionState, locations,
             for i, location in enumerate(locations):
                 if (not single_player_placement or location.player == item_to_place.player) \
                         and location.can_fill(maximum_exploration_state, item_to_place, perform_access_check):
-                    spot_to_fill = locations.pop(i) # poping by index is faster than removing by content,
+                    # poping by index is faster than removing by content,
+                    spot_to_fill = locations.pop(i)
                     # skipping a scan for the element
                     break
 
             else:
-                # we filled all reachable spots. Maybe the game can be beaten anyway?
-                unplaced_items.append(item_to_place)
-                if world.accessibility[item_to_place.player] != 'minimal' and world.can_beat_game():
-                    logging.warning(
-                        f'Not all items placed. Game beatable anyway. (Could not place {item_to_place})')
-                    continue
-                raise FillError(f'No more spots to place {item_to_place}, locations {locations} are invalid. '
-                                f'Already placed {len(placements)}: {", ".join(str(place) for place in placements)}')
+                # we filled all reachable spots.
+                # try swaping this item with previously placed items
+                for(i, location) in enumerate(placements):
+                    placed_item = location.item
+                    location.item = None
+                    placed_item.location = None
+                    swap_state = sweep_from_pool(base_state, itempool)
+                    if (not single_player_placement or location.player == item_to_place.player) \
+                            and location.can_fill(swap_state, item_to_place, perform_access_check):
+                        # Add this item to the exisiting placement, and 
+                        # add the old item to the back of the queue
+                        spot_to_fill = placements.pop(i)
+                        reachable_items.setdefault(placed_item.player, []).append(placed_item)
+                        itempool.append(placed_item)
+                        break
+                    else:
+                        # Item can't be placed here, restore original item
+                        location.item = placed_item
+                        placed_item.location = location
+
+                if spot_to_fill == None:
+                    # Maybe the game can be beaten anyway?
+                    unplaced_items.append(item_to_place)
+                    if world.accessibility[item_to_place.player] != 'minimal' and world.can_beat_game():
+                        logging.warning(
+                            f'Not all items placed. Game beatable anyway. (Could not place {item_to_place})')
+                        continue
+                    raise FillError(f'No more spots to place {item_to_place}, locations {locations} are invalid. '
+                                    f'Already placed {len(placements)}: {", ".join(str(place) for place in placements)}')
 
             world.push_item(spot_to_fill, item_to_place, False)
             spot_to_fill.locked = lock

test/base/TestFill.py

@@ -1,6 +1,7 @@
 import unittest
+import pytest
 from worlds.AutoWorld import World
-from Fill import fill_restrictive
+from Fill import FillError, fill_restrictive
 from BaseClasses import MultiWorld, Region, RegionType, Item, Location
 from worlds.generic.Rules import set_rule
 
@@ -105,4 +106,47 @@ class TestBase(unittest.TestCase):
         fill_restrictive(multi_world, multi_world.state, locations, items)
 
         self.assertEqual(loc0.item, item1)
-        self.assertEqual(loc1.item, item0)
+        self.assertEqual(loc1.item, item0)
+
+    def test_impossible_fill_restrictive(self):
+        multi_world = generate_multi_world()
+        player1_id = 1
+        player1_menu = multi_world.get_region("Menu", player1_id)
+
+        locations = generate_locations(2, player1_id, None, player1_menu)
+        items = generate_items(2, player1_id, True)
+
+        item0 = items[0]
+        item1 = items[1]
+        loc0 = locations[0]
+        loc1 = locations[1]
+
+        multi_world.completion_condition[player1_id] = lambda state: state.has(
+            item0.name, player1_id) and state.has(item1.name, player1_id)
+        set_rule(loc1, lambda state: state.has(item1.name, player1_id))
+        set_rule(loc0, lambda state: state.has(item0.name, player1_id))
+        with pytest.raises(FillError):
+            fill_restrictive(multi_world, multi_world.state, locations, items)
+
+    def test_circular_fill_restrictive(self):
+        multi_world = generate_multi_world()
+        player1_id = 1
+        player1_menu = multi_world.get_region("Menu", player1_id)
+
+        locations = generate_locations(3, player1_id, None, player1_menu)
+        items = generate_items(3, player1_id, True)
+
+        item0 = items[0]
+        item1 = items[1]
+        item2 = items[2]
+        loc0 = locations[0]
+        loc1 = locations[1]
+        loc2 = locations[2]
+
+        multi_world.completion_condition[player1_id] = lambda state: state.has(
+            item0.name, player1_id) and state.has(item1.name, player1_id) and state.has(item2.name, player1_id)
+        set_rule(loc1, lambda state: state.has(item0.name, player1_id))
+        set_rule(loc2, lambda state: state.has(item1.name, player1_id))
+        set_rule(loc0, lambda state: state.has(item2.name, player1_id))
+        with pytest.raises(FillError):
+            fill_restrictive(multi_world, multi_world.state, locations, items)