Core: CollectionState - Never pick up advancements

BaseClasses.py

@@ -425,11 +425,11 @@ def get_all_state(self, use_cache: bool) -> CollectionState:
         ret = CollectionState(self)
 
         for item in self.itempool:
-            self.worlds[item.player].collect(ret, item)
+            ret.collect(item, prevent_sweep=True)
         for player in self.player_ids:
             subworld = self.worlds[player]
             for item in subworld.get_pre_fill_items():
-                subworld.collect(ret, item)
+                ret.collect(item, prevent_sweep=True)
         ret.sweep_for_events()
 
         if use_cache:
@@ -867,14 +867,16 @@ def collect(self, item: Item, prevent_sweep: bool = False, location: Optional[Lo
         if location:
             self.locations_checked.add(location)
 
-        changed = self.multiworld.worlds[item.player].collect(self, item)
+        if not item.advancement:
+            return False
 
+        self.multiworld.worlds[item.player].collect(self, item)
         self.stale[item.player] = True
 
-        if changed and not prevent_sweep:
+        if not prevent_sweep:
             self.sweep_for_events()
 
-        return changed
+        return True
 
     def remove(self, item: Item):
         changed = self.multiworld.worlds[item.player].remove(self, item)

worlds/AutoWorld.py

@@ -476,17 +476,15 @@ def create_group(cls, multiworld: "MultiWorld", new_player_id: int, players: Set
         return group
 
     # decent place to implement progressive items, in most cases can stay as-is
-    def collect_item(self, state: "CollectionState", item: "Item", remove: bool = False) -> Optional[str]:
+    def collect_item(self, state: "CollectionState", item: "Item", remove: bool = False) -> str:
         """
-        Collect an item name into state. For speed reasons items that aren't logically useful get skipped.
+        Collect an item name into state.
         Collect None to skip item.
         :param state: CollectionState to collect into
         :param item: Item to decide on if it should be collected into state
         :param remove: indicate if this is meant to remove from state instead of adding.
         """
-        if item.advancement:
-            return item.name
-        return None
+        return item.name
 
     def get_pre_fill_items(self) -> List["Item"]:
         """
@@ -497,22 +495,20 @@ def get_pre_fill_items(self) -> List["Item"]:
 
     # these two methods can be extended for pseudo-items on state
     def collect(self, state: "CollectionState", item: "Item") -> bool:
+        assert item.advancement, "Collect should no longer be called with non-advancements."
+
         """Called when an item is collected in to state. Useful for things such as progressive items or currency."""
         name = self.collect_item(state, item)
-        if name:
-            state.prog_items[self.player][name] += 1
-            return True
-        return False
+        state.prog_items[self.player][name] += 1
+        return True  # Backwards compatibility, this used to denote whether a progression item was picked up
 
     def remove(self, state: "CollectionState", item: "Item") -> bool:
         """Called when an item is removed from to state. Useful for things such as progressive items or currency."""
         name = self.collect_item(state, item, True)
-        if name:
-            state.prog_items[self.player][name] -= 1
-            if state.prog_items[self.player][name] < 1:
-                del (state.prog_items[self.player][name])
-            return True
-        return False
+        state.prog_items[self.player][name] -= 1
+        if state.prog_items[self.player][name] < 1:
+            del (state.prog_items[self.player][name])
+        return True  # Backwards compatibility, this used to denote whether a progression item was picked up
 
     # following methods should not need to be overridden.
     def create_filler(self) -> "Item":

worlds/alttp/Dungeons.py

@@ -236,7 +236,7 @@ def fill_dungeons_restrictive(multiworld: MultiWorld):
             in_dungeon_player_ids = {item.player for item in in_dungeon_items}
             all_state_base = CollectionState(multiworld)
             for item in multiworld.itempool:
-                multiworld.worlds[item.player].collect(all_state_base, item)
+                all_state_base.collect(item, prevent_sweep=True)
             pre_fill_items = []
             for player in in_dungeon_player_ids:
                 pre_fill_items += multiworld.worlds[player].get_pre_fill_items()

worlds/factorio/__init__.py

@@ -294,7 +294,7 @@ def generate_basic(self):
                 start_location_hints.add(loc.name)
 
     def collect_item(self, state, item, remove=False):
-        if item.advancement and item.name in progressive_technology_table:
+        if item.name in progressive_technology_table:
             prog_table = progressive_technology_table[item.name].progressive
             if remove:
                 for item_name in reversed(prog_table):

worlds/oot/__init__.py

@@ -874,7 +874,7 @@ def pre_fill(self):
         def prefill_state(base_state):
             state = base_state.copy()
             for item in self.get_pre_fill_items():
-                self.collect(state, item)
+                state.collect(item, prevent_sweep=True)
             state.sweep_for_events(locations=self.get_locations())
             return state
 
@@ -886,7 +886,7 @@ def prefill_state(base_state):
         # Set up initial state
         state = CollectionState(self.multiworld)
         for item in self.itempool:
-            self.collect(state, item)
+            state.collect(item, prevent_sweep=True)
         state.sweep_for_events(locations=self.get_locations())
 
         # Place dungeon items
@@ -1385,7 +1385,7 @@ def is_major_item(self, item: OOTItem):
     def get_state_with_complete_itempool(self):
         all_state = CollectionState(self.multiworld)
         for item in self.itempool + self.pre_fill_items:
-            self.multiworld.worlds[item.player].collect(all_state, item)
+            all_state.collect(item, prevent_sweep=True)
         # If free_scarecrow give Scarecrow Song
         if self.free_scarecrow:
             all_state.collect(self.create_item("Scarecrow Song"), prevent_sweep=True)

worlds/raft/__init__.py

@@ -137,8 +137,6 @@ def create_resourcePack(self, rpName: str) -> Item:
         return RaftItem(rpName, ItemClassification.filler, self.item_name_to_id[rpName], player=self.player)
 
     def collect_item(self, state, item, remove=False):
-        if item.advancement is False:
-            return None
         if item.name in progressive_item_list:
             prog_table = progressive_item_list[item.name]
             if remove:

worlds/smz3/__init__.py

@@ -224,7 +224,7 @@ def create_items(self):
             # Dungeons items here are not in the itempool and will be prefilled locally so they must stay local
             self.multiworld.non_local_items[self.player].value -= frozenset(item_name for item_name in self.item_names if TotalSMZ3Item.Item.IsNameDungeonItem(item_name))
             for item in self.keyCardsItems:
-                self.multiworld.push_precollected(SMZ3Item(item.Type.name, ItemClassification.filler, item.Type, self.item_name_to_id[item.Type.name], self.player, item))
+                self.multiworld.push_precollected(SMZ3Item(item.Type.name, ItemClassification.progression, item.Type, self.item_name_to_id[item.Type.name], self.player, item))
 
         itemPool = [SMZ3Item(item.Type.name, ItemClassification.progression, item.Type, self.item_name_to_id[item.Type.name], self.player, item) for item in progressionItems] + \
                     [SMZ3Item(item.Type.name, ItemClassification.useful, item.Type, self.item_name_to_id[item.Type.name], self.player, item) for item in niceItems] + \