New, smarter fast_fill function (#646)

Co-authored-by: Fabian Dill <Berserker66@users.noreply.github.com>

Fill.py

@@ -136,33 +136,98 @@ def fill_restrictive(world: MultiWorld, base_state: CollectionState, locations:
     itempool.extend(unplaced_items)
 
 
+def remaining_fill(world: MultiWorld,
+                   locations: typing.List[Location],
+                   itempool: typing.List[Item]) -> None:
+    unplaced_items: typing.List[Item] = []
+    placements: typing.List[Location] = []
+    swapped_items: typing.Counter[typing.Tuple[int, str]] = Counter()
+    while locations and itempool:
+        item_to_place = itempool.pop()
+        spot_to_fill: typing.Optional[Location] = None
+
+        for i, location in enumerate(locations):
+            if location.item_rule(item_to_place):
+                # popping 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.
+            # try swapping this item with previously placed items
+
+            for (i, location) in enumerate(placements):
+                placed_item = location.item
+                # Unplaceable items can sometimes be swapped infinitely. Limit the
+                # number of times we will swap an individual item to prevent this
+
+                if swapped_items[placed_item.player,
+                                 placed_item.name] > 1:
+                    continue
+
+                location.item = None
+                placed_item.location = None
+                if location.item_rule(item_to_place):
+                    # Add this item to the existing placement, and
+                    # add the old item to the back of the queue
+                    spot_to_fill = placements.pop(i)
+
+                    swapped_items[placed_item.player,
+                                  placed_item.name] += 1
+
+                    itempool.append(placed_item)
+
+                    break
+
+                # Item can't be placed here, restore original item
+                location.item = placed_item
+                placed_item.location = location
+
+            if spot_to_fill is None:
+                # Can't place this item, move on to the next
+                unplaced_items.append(item_to_place)
+                continue
+
+        world.push_item(spot_to_fill, item_to_place, False)
+        placements.append(spot_to_fill)
+
+    if unplaced_items and locations:
+        # There are leftover unplaceable items and locations that won't accept them
+        raise FillError(f'No more spots to place {unplaced_items}, locations {locations} are invalid. '
+                        f'Already placed {len(placements)}: {", ".join(str(place) for place in placements)}')
+
+    itempool.extend(unplaced_items)
+
+
+def fast_fill(world: MultiWorld,
+              item_pool: typing.List[Item],
+              fill_locations: typing.List[Location]) -> typing.Tuple[typing.List[Item], typing.List[Location]]:
+    placing = min(len(item_pool), len(fill_locations))
+    for item, location in zip(item_pool, fill_locations):
+        world.push_item(location, item, False)
+    return item_pool[placing:], fill_locations[placing:]
+
+
 def distribute_items_restrictive(world: MultiWorld) -> None:
     fill_locations = sorted(world.get_unfilled_locations())
     world.random.shuffle(fill_locations)
-
     # get items to distribute
     itempool = sorted(world.itempool)
     world.random.shuffle(itempool)
     progitempool: typing.List[Item] = []
-    nonexcludeditempool: typing.List[Item] = []
+    usefulitempool: typing.List[Item] = []
-    localrestitempool: typing.Dict[int, typing.List[Item]] = {player: [] for player in range(1, world.players + 1)}
+    filleritempool: typing.List[Item] = []
-    nonlocalrestitempool: typing.List[Item] = []
-    restitempool: typing.List[Item] = []
 
     for item in itempool:
         if item.advancement:
             progitempool.append(item)
-        elif item.useful:  # this only gets nonprogression items which should not appear in excluded locations
+        elif item.useful:
-            nonexcludeditempool.append(item)
+            usefulitempool.append(item)
-        elif item.name in world.local_items[item.player].value:
-            localrestitempool[item.player].append(item)
-        elif item.name in world.non_local_items[item.player].value:
-            nonlocalrestitempool.append(item)
         else:
-            restitempool.append(item)
+            filleritempool.append(item)
 
-    call_all(world, "fill_hook", progitempool, nonexcludeditempool,
+    call_all(world, "fill_hook", progitempool, usefulitempool, filleritempool, fill_locations)
-             localrestitempool, nonlocalrestitempool, restitempool, fill_locations)
 
     locations: typing.Dict[LocationProgressType, typing.List[Location]] = {
         loc_type: [] for loc_type in LocationProgressType}
@@ -184,50 +249,16 @@ def distribute_items_restrictive(world: MultiWorld) -> None:
             raise FillError(
                 f'Not enough locations for progress items. There are {len(progitempool)} more items than locations')
 
-    if nonexcludeditempool:
+    remaining_fill(world, excludedlocations, filleritempool)
-        world.random.shuffle(defaultlocations)
+    if excludedlocations:
-        # needs logical fill to not conflict with local items
+        raise FillError(
-        fill_restrictive(
+            f"Not enough filler items for excluded locations. There are {len(excludedlocations)} more locations than items")
-            world, world.state, defaultlocations, nonexcludeditempool)
-        if nonexcludeditempool:
-            raise FillError(
-                f'Not enough locations for non-excluded items. There are {len(nonexcludeditempool)} more items than locations')
 
-    defaultlocations = defaultlocations + excludedlocations
+    restitempool = usefulitempool + filleritempool
-    world.random.shuffle(defaultlocations)
 
-    if any(localrestitempool.values()):  # we need to make sure some fills are limited to certain worlds
+    remaining_fill(world, defaultlocations, restitempool)
-        local_locations: typing.Dict[int, typing.List[Location]] = {player: [] for player in world.player_ids}
-        for location in defaultlocations:
-            local_locations[location.player].append(location)
-        for player_locations in local_locations.values():
-            world.random.shuffle(player_locations)
 
-        for player, items in localrestitempool.items():  # items already shuffled
+    unplaced = restitempool
-            player_local_locations = local_locations[player]
-            for item_to_place in items:
-                if not player_local_locations:
-                    logging.warning(f"Ran out of local locations for player {player}, "
-                                    f"cannot place {item_to_place}.")
-                    break
-                spot_to_fill = player_local_locations.pop()
-                world.push_item(spot_to_fill, item_to_place, False)
-                defaultlocations.remove(spot_to_fill)
-
-    for item_to_place in nonlocalrestitempool:
-        for i, location in enumerate(defaultlocations):
-            if location.player != item_to_place.player:
-                world.push_item(defaultlocations.pop(i), item_to_place, False)
-                break
-        else:
-            raise Exception(f"Could not place non_local_item {item_to_place} among {defaultlocations}. "
-                            f"Too many non-local items for too few remaining locations.")
-
-    world.random.shuffle(defaultlocations)
-
-    restitempool, defaultlocations = fast_fill(
-        world, restitempool, defaultlocations)
-    unplaced = progitempool + restitempool
     unfilled = defaultlocations
 
     if unplaced or unfilled:
@@ -241,15 +272,6 @@ def distribute_items_restrictive(world: MultiWorld) -> None:
         logging.info(f'Per-Player counts: {print_data})')
 
 
-def fast_fill(world: MultiWorld,
-              item_pool: typing.List[Item],
-              fill_locations: typing.List[Location]) -> typing.Tuple[typing.List[Item], typing.List[Location]]:
-    placing = min(len(item_pool), len(fill_locations))
-    for item, location in zip(item_pool, fill_locations):
-        world.push_item(location, item, False)
-    return item_pool[placing:], fill_locations[placing:]
-
-
 def flood_items(world: MultiWorld) -> None:
     # get items to distribute
     world.random.shuffle(world.itempool)

test/general/TestFill.py

@@ -371,13 +371,13 @@ class TestDistributeItemsRestrictive(unittest.TestCase):
 
         distribute_items_restrictive(multi_world)
 
-        self.assertEqual(locations[0].item, basic_items[0])
+        self.assertEqual(locations[0].item, basic_items[1])
         self.assertFalse(locations[0].event)
         self.assertEqual(locations[1].item, prog_items[0])
         self.assertTrue(locations[1].event)
         self.assertEqual(locations[2].item, prog_items[1])
         self.assertTrue(locations[2].event)
-        self.assertEqual(locations[3].item, basic_items[1])
+        self.assertEqual(locations[3].item, basic_items[0])
         self.assertFalse(locations[3].event)
 
     def test_excluded_distribute(self):
@@ -500,8 +500,8 @@ class TestDistributeItemsRestrictive(unittest.TestCase):
         removed_item: list[Item] = []
         removed_location: list[Location] = []
 
-        def fill_hook(progitempool, nonexcludeditempool, localrestitempool, nonlocalrestitempool, restitempool, fill_locations):
+        def fill_hook(progitempool, usefulitempool, filleritempool, fill_locations):
-            removed_item.append(restitempool.pop(0))
+            removed_item.append(filleritempool.pop(0))
             removed_location.append(fill_locations.pop(0))
 
         multi_world.worlds[player1.id].fill_hook = fill_hook

worlds/AutoWorld.py

@@ -221,10 +221,8 @@ class World(metaclass=AutoWorldRegister):
     @classmethod
     def fill_hook(cls,
                   progitempool: List["Item"],
-                  nonexcludeditempool: List["Item"],
+                  usefulitempool: List["Item"],
-                  localrestitempool: Dict[int, List["Item"]],
+                  filleritempool: List["Item"],
-                  nonlocalrestitempool: Dict[int, List["Item"]],
-                  restitempool: List["Item"],
                   fill_locations: List["Location"]) -> None:
         """Special method that gets called as part of distribute_items_restrictive (main fill).
         This gets called once per present world type."""

worlds/alttp/__init__.py

@@ -424,8 +424,7 @@ class ALTTPWorld(World):
         return ALttPItem(name, self.player, **item_init_table[name])
 
     @classmethod
-    def stage_fill_hook(cls, world, progitempool, nonexcludeditempool, localrestitempool, nonlocalrestitempool,
+    def stage_fill_hook(cls, world, progitempool, usefulitempool, filleritempool, fill_locations):
-                        restitempool, fill_locations):
         trash_counts = {}
         standard_keyshuffle_players = set()
         for player in world.get_game_players("A Link to the Past"):
@@ -472,26 +471,15 @@ class ALTTPWorld(World):
             for player, trash_count in trash_counts.items():
                 gtower_locations = locations_mapping[player]
                 world.random.shuffle(gtower_locations)
-                localrest = localrestitempool[player]
-                if localrest:
-                    gt_item_pool = restitempool + localrest
-                    world.random.shuffle(gt_item_pool)
-                else:
-                    gt_item_pool = restitempool.copy()
 
-                while gtower_locations and gt_item_pool and trash_count > 0:
+                while gtower_locations and filleritempool and trash_count > 0:
                     spot_to_fill = gtower_locations.pop()
-                    item_to_place = gt_item_pool.pop()
+                    item_to_place = filleritempool.pop()
                     if spot_to_fill.item_rule(item_to_place):
-                        if item_to_place in localrest:
-                            localrest.remove(item_to_place)
-                        else:
-                            restitempool.remove(item_to_place)
                         world.push_item(spot_to_fill, item_to_place, False)
                         fill_locations.remove(spot_to_fill)  # very slow, unfortunately
                         trash_count -= 1
 
-
     def get_filler_item_name(self) -> str:
         if self.world.goal[self.player] == "icerodhunt":
             item = "Nothing"

worlds/sa2b/__init__.py

@@ -282,8 +282,7 @@ class SA2BWorld(World):
             spoiler_handle.writelines(text)
 
     @classmethod
-    def stage_fill_hook(cls, world, progitempool, nonexcludeditempool, localrestitempool, nonlocalrestitempool,
+    def stage_fill_hook(cls, world, progitempool, usefulitempool, filleritempool, fill_locations):
-                        restitempool, fill_locations):
         if world.get_game_players("Sonic Adventure 2 Battle"):
             progitempool.sort(
                 key=lambda item: 0 if (item.name != 'Emblem') else 1)

worlds/sm/__init__.py

@@ -660,8 +660,7 @@ class SMWorld(World):
                 loc.address = loc.item.code = None
 
     @classmethod
-    def stage_fill_hook(cls, world, progitempool, nonexcludeditempool, localrestitempool, nonlocalrestitempool,
+    def stage_fill_hook(cls, world, progitempool, usefulitempool, filleritempool, fill_locations):
-                        restitempool, fill_locations):      
         if world.get_game_players("Super Metroid"):
             progitempool.sort(
                 key=lambda item: 1 if (item.name == 'Morph Ball') else 0)