Fix for place_item and local item not being done at the right time during gen

src/__init__.py

@@ -21,16 +21,16 @@
 from .Helpers import is_item_enabled, get_option_value, remove_specific_item, resolve_yaml_option, format_state_prog_items_key, convert_string_to_itemclassification, ProgItemsCat
 from .container import APManualFile
 
-from BaseClasses import CollectionState, ItemClassification, Item
+from BaseClasses import CollectionState, ItemClassification, Item, Location
 from Options import PerGameCommonOptions
 from worlds.AutoWorld import World
 
 from .hooks.World import \
     hook_get_filler_item_name, before_create_regions, after_create_regions, \
-    before_create_items_all, before_create_items_starting, before_create_items_filler, after_create_items, \
+    before_create_items_all, before_create_items_place_items, before_create_items_starting, before_create_items_filler, after_create_items, \
     before_create_item, after_create_item, \
     before_set_rules, after_set_rules, \
-    before_generate_basic, after_generate_basic, \
+    before_generate_basic, \
     before_fill_slot_data, after_fill_slot_data, before_write_spoiler, \
     before_extend_hint_information, after_extend_hint_information, \
     after_collect_item, after_remove_item, before_generate_early, hook_interpret_slot_data
@@ -91,6 +91,11 @@ def stage_assert_generate(cls, multiworld) -> None:
 
     def generate_early(self) -> None:
         before_generate_early(self, self.multiworld, self.player)
+        for item in item_table:
+            if item.get("local"):
+                if item.get("name") not in self.options.local_items.value:
+                    self.options.local_items.value.add(item["name"])
+
         if hasattr(self.multiworld, "re_gen_passthrough"):
             slot_data = self.multiworld.re_gen_passthrough.get(self.game, {})
             if slot_data:
@@ -186,10 +191,6 @@ def create_items(self):
                 else:
                     raise Exception(f"Item {name}'s 'early' has an invalid value of '{item['early']}'. \nA boolean or an integer was expected.")
 
-            if item.get("local"): # All local
-                if name not in self.options.local_items.value:
-                    self.options.local_items.value.add(name)
-
             if item.get("local_early"): # Some or all local and early
                 if isinstance(item["local_early"],int) or (isinstance(item["local_early"],str) and item["local_early"].isnumeric()):
                     self.multiworld.local_early_items[self.player][name] = int(item["local_early"])
@@ -200,7 +201,81 @@ def create_items(self):
                 else:
                     raise Exception(f"Item {name}'s 'local_early' has an invalid value of '{item['local_early']}'. \nA boolean or an integer was expected.")
 
+        # Handle item forbidding/placement
+        pool = before_create_items_place_items(pool, self, self.multiworld, self.player)
+        locations_with_forbid: list[Location] = []
+        locations_with_placements: list[Location] = []
+        for location in self.multiworld.get_unfilled_locations(player=self.player):
+            manual_location = self.location_name_to_location.get(location.name, {})
+            if manual_location.get("place_item") or manual_location.get("place_item_category"):
+                locations_with_placements.append(location)
+            elif manual_location.get("dont_place_item") or manual_location.get("dont_place_item_category"):
+                locations_with_forbid.append(location)
+
+        # Handle specific item forbidding using forbid_items_for_player
+        for location in locations_with_forbid:
+            manual_location = self.location_name_to_location.get(location.name, {})
+            forbidden_item_names: set[str] = set()
+
+            if manual_location.get("dont_place_item"):
+                forbidden_item_names |= set(manual_location["dont_place_item"])
+
+            if manual_location.get("dont_place_item_category"):
+                for cat in manual_location["dont_place_item_category"]:
+                    forbidden_item_names |= self.item_name_groups.get(cat, set())
+
+            if forbidden_item_names:
+                forbid_items_for_player(location, set(forbidden_item_names), self.player)
+
+        # Handle specific item placements using place_locked_item
+        for location in locations_with_placements:
+            manual_location = self.location_name_to_location.get(location.name, {})
+            eligible_items = []
+            eligible_item_names: set[str] = set()
+            forbidden_item_names: set[str] = set()
+            place_messages = []
+            forbid_messages = []
+
+            #First we get possible items names
+            if manual_location.get("place_item"):
+                eligible_item_names |= set(manual_location["place_item"])
+                place_messages.append('", "'.join(manual_location["place_item"]))
+
+            if manual_location.get("place_item_category"):
+                for cat in manual_location["place_item_category"]:
+                    eligible_item_names |= self.item_name_groups.get(cat, set())
+                place_messages.append('", "'.join(manual_location["place_item_category"]) + " category(ies)")
+
+            # Second we check for forbidden items names
+            if manual_location.get("dont_place_item"):
+                forbidden_item_names |= set(manual_location["dont_place_item"])
+                forbid_messages.append('", "'.join(manual_location["dont_place_item"]) + ' items')
+
+            if manual_location.get("dont_place_item_category"):
+                for cat in manual_location["dont_place_item_category"]:
+                    forbidden_item_names |= self.item_name_groups.get(cat, set())
+                forbid_messages.append('", "'.join(manual_location["dont_place_item_category"]) + ' category(ies)')
 
+            # If we forbid some names, check for those in the possible names and remove them
+            if forbidden_item_names:
+                eligible_item_names = {name for name in eligible_item_names if name not in forbidden_item_names}
+
+            if eligible_item_names:
+                eligible_items = [item for item in pool if item.name in eligible_item_names]
+
+            if len(eligible_items) == 0:
+                nl = "\n"
+                if forbidden_item_names:
+                    raise Exception(f'Could not find a suitable item to place at "{manual_location["name"]}".\n    No items that match "{f"{nl}     or ".join(place_messages)}"\n    Maybe because of forbidden "{f"{nl}     or ".join(forbid_messages)}"')
+                raise Exception(f'Could not find a suitable item to place at "{manual_location["name"]}". \n    No items that match "{f"{nl}     or ".join(place_messages)}"')
+
+            item_to_place = self.random.choice(eligible_items)
+            location.place_locked_item(item_to_place)
+
+            # remove the item we're about to place from the pool so it isn't placed twice
+            remove_specific_item(pool, item_to_place)
+
+        # Handle game.json's starting_items
         pool = before_create_items_starting(pool, self, self.multiworld, self.player)
 
         items_started: list[Item] = []
@@ -247,21 +322,26 @@ def create_items(self):
         pool = after_create_items(pool, self, self.multiworld, self.player)
 
         # need to put all of the items in the pool so we can have a full state for placement
-        # then will remove specific item placements below from the overall pool
         self.multiworld.itempool += pool
 
-        # Filter Precollected items for those not in logic aka created by start_inventory(_from_pool)
+        # Preparing to count the items:
         precollected_items = list(self.multiworld.precollected_items[self.player])
 
         # UT doesn't precollect the exceptions so this can be skipped
         if not getattr(self.multiworld, "generation_is_fake", False):
+            # Filter Precollected items for those not in logic aka created by start_inventory(_from_pool)
             precollected_exceptions = self.options.start_inventory.value + self.options.start_inventory_from_pool.value # type: ignore
             for item, count in precollected_exceptions.items():
                 items_iter = iter([i for i in precollected_items if i.name == item])
                 for _ in range(count):
                     precollected_items.remove(next(items_iter))
 
-        real_pool = pool + precollected_items
+        # Placed items detections:
+        placed_pool: list[Item] = []
+        for location in self.multiworld.get_filled_locations(self.player):
+            placed_pool.append(location.item)
+
+        real_pool = pool + precollected_items + placed_pool
         self.item_counts[self.player] = self.get_item_counts(pool=real_pool)
         self.item_counts_progression[self.player] = self.get_item_counts(pool=real_pool, only_progression=True)
 
@@ -337,73 +417,6 @@ def set_rules(self):
     def generate_basic(self):
         before_generate_basic(self, self.multiworld, self.player)
 
-        # Handle item forbidding
-        manual_locations_with_forbid = {location['name']: location for location in location_name_to_location.values() if "dont_place_item" in location or "dont_place_item_category" in location}
-        locations_with_forbid = [l for l in self.multiworld.get_unfilled_locations(player=self.player) if l.name in manual_locations_with_forbid.keys()]
-        for location in locations_with_forbid:
-            manual_location = manual_locations_with_forbid[location.name]
-            forbidden_item_names = []
-
-            if manual_location.get("dont_place_item"):
-                forbidden_item_names.extend([i["name"] for i in item_name_to_item.values() if i["name"] in manual_location["dont_place_item"]])
-
-            if manual_location.get("dont_place_item_category"):
-                forbidden_item_names.extend([i["name"] for i in item_name_to_item.values() if "category" in i and set(i["category"]).intersection(manual_location["dont_place_item_category"])])
-
-            if forbidden_item_names:
-                forbid_items_for_player(location, set(forbidden_item_names), self.player)
-
-        # Handle specific item placements using fill_restrictive
-        manual_locations_with_placements = {location['name']: location for location in location_name_to_location.values() if "place_item" in location or "place_item_category" in location}
-        locations_with_placements = [l for l in self.multiworld.get_unfilled_locations(player=self.player) if l.name in manual_locations_with_placements.keys()]
-        for location in locations_with_placements:
-            manual_location = manual_locations_with_placements[location.name]
-            eligible_items = []
-            eligible_item_names = []
-            forbidden_item_names = []
-            place_messages = []
-            forbid_messages = []
-
-            #First we get possible items names
-            if manual_location.get("place_item"):
-                eligible_item_names += manual_location["place_item"]
-                place_messages.append('", "'.join(manual_location["place_item"]))
-
-            if manual_location.get("place_item_category"):
-                eligible_item_names += [i["name"] for i in item_name_to_item.values() if "category" in i and set(i["category"]).intersection(manual_location["place_item_category"])]
-                place_messages.append('", "'.join(manual_location["place_item_category"]) + " category(ies)")
-
-            # Second we check for forbidden items names
-            if manual_location.get("dont_place_item"):
-                forbidden_item_names += manual_location["dont_place_item"]
-                forbid_messages.append('", "'.join(manual_location["dont_place_item"]) + ' items')
-
-            if manual_location.get("dont_place_item_category"):
-                forbidden_item_names += [i["name"] for i in item_name_to_item.values() if "category" in i and set(i["category"]).intersection(manual_location["dont_place_item_category"])]
-                forbid_messages.append('", "'.join(manual_location["dont_place_item_category"]) + ' category(ies)')
-
-            # If we forbid some names, check for those in the possible names and remove them
-            if forbidden_item_names:
-                eligible_item_names = [name for name in eligible_item_names if name not in forbidden_item_names]
-
-            if eligible_item_names:
-                eligible_items = [item for item in self.multiworld.itempool if item.player == self.player and item.name in eligible_item_names]
-
-            if len(eligible_items) == 0:
-                nl = "\n"
-                if forbidden_item_names:
-                    raise Exception(f'Could not find a suitable item to place at "{manual_location["name"]}".\n    No items that match "{f"{nl}     or ".join(place_messages)}"\n    Maybe because of forbidden "{f"{nl}     or ".join(forbid_messages)}"')
-                raise Exception(f'Could not find a suitable item to place at "{manual_location["name"]}". \n    No items that match "{f"{nl}     or ".join(place_messages)}"')
-
-            item_to_place = self.random.choice(eligible_items)
-            location.place_locked_item(item_to_place)
-
-            # remove the item we're about to place from the pool so it isn't placed twice
-            remove_specific_item(self.multiworld.itempool, item_to_place)
-
-
-        after_generate_basic(self, self.multiworld, self.player)
-
         # Enable this in Meta.json to generate a diagram of your manual.  Only works on 0.4.4+
         if get_option_value(self.multiworld, self.player, "generate_region_diagram"):
             from Utils import visualize_regions

src/data/categories.json

@@ -5,5 +5,15 @@
     },
     "Example Yaml-option category": {
         "yaml_option": ["DLC_enabled"]
+    },
+    "For_Tests_And_Examples": {
+         "hidden": true,
+         "yaml_option": ["Tests_content"]
+    },
+    "place_item_category_test": {
+         "hidden": true
+    },
+    "dont_place_item_category_test": {
+         "hidden": true
     }
 }

src/data/items.json

@@ -4,7 +4,8 @@
         {
             "name": "Item Name for OptionCount Example",
             "count": 5,
-            "progression": true
+            "progression": true,
+            "category": ["For_Tests_And_Examples"]
         },
         {
             "name": "Jill",
@@ -460,6 +461,59 @@
             ],
             "progression": true,
             "sort-key": "row-6-8"
+        },
+        {
+            "name": "Test item A",
+            "id": 200,
+            "_comment": "This and the following items are there to make sure we tests everything against Archipelago's Unit tests",
+            "progression": true,
+            "category": ["For_Tests_And_Examples", "dont_place_item_category_test"]
+        },
+        {
+            "name": "Test item B",
+            "progression": true,
+            "category": ["For_Tests_And_Examples", "place_item_category_test", "dont_place_item_category_test"]
+        },
+        {
+            "name": "Test item C",
+            "progression": true,
+            "category": ["For_Tests_And_Examples", "place_item_category_test"]
+        },
+        {
+            "name": "Test item D",
+            "progression": true,
+            "category": ["For_Tests_And_Examples", "place_item_category_test"]
+        },
+        {
+            "name": "Test Local item",
+            "local": true,
+            "progression": true,
+            "category": ["For_Tests_And_Examples", "place_item_category_test"]
+        },
+        {
+            "name": "Test early item A",
+            "early": true,
+            "progression": true,
+            "category": ["For_Tests_And_Examples", "place_item_category_test", "dont_place_item_category_test"]
+        },
+        {
+            "name": "Test early item B",
+            "early": 1,
+            "progression": true,
+            "category": ["For_Tests_And_Examples", "place_item_category_test", "dont_place_item_category_test"]
+        },
+        {
+            "name": "Test local-early item A",
+            "local_early": true,
+            "progression": true,
+            "category": ["For_Tests_And_Examples", "place_item_category_test", "dont_place_item_category_test"]
+        },
+        {
+            "name": "Test local-early item B",
+            "local_early": 1,
+            "progression": true,
+            "category": ["For_Tests_And_Examples", "place_item_category_test", "dont_place_item_category_test"]
         }
+
     ]
 }

src/data/locations.json

@@ -3,7 +3,8 @@
     "data": [
         {
             "name": "Example_Range decide the amount of items required for this location",
-            "requires": "{OptionCount(Item Name for OptionCount Example, Example_Range)}"
+            "requires": "{OptionCount(Item Name for OptionCount Example, Example_Range)}",
+            "category": ["For_Tests_And_Examples"]
         },
         {
             "name": "Beat the Game - Ryu",
@@ -283,6 +284,67 @@
             "victory": true,
             "category": ["All Characters Complete!"],
             "requires": "{ItemValue(star:10)} and {ItemValue(coins:10)}"
+        },
+
+        {
+            "name": "Test dont_place_item alone",
+            "id": 200,
+            "requires": "",
+            "dont_place_item": ["Test item A"],
+            "category": ["For_Tests_And_Examples"]
+        },
+        {
+            "name": "Test place_item alone",
+            "requires": "",
+            "place_item": ["Test item A"],
+            "category": ["For_Tests_And_Examples"]
+        },
+        {
+            "name": "Test dont_place_item_category alone",
+            "requires": "",
+            "dont_place_item_category": ["place_item_category_test"],
+            "category": ["For_Tests_And_Examples"]
+        },
+        {
+            "name": "Test place_item_category alone",
+            "requires": "",
+            "place_item_category": ["place_item_category_test"],
+            "category": ["For_Tests_And_Examples"]
+        },
+        {
+            "name": "Test place_item_category + dont_place_item",
+            "place_item_category": ["place_item_category_test"],
+            "dont_place_item": ["Test item B"],
+            "requires": "",
+            "category": ["For_Tests_And_Examples"]
+        },
+        {
+            "name": "Test place_item_category + dont_place_item_category",
+            "place_item_category": ["place_item_category_test"],
+            "dont_place_item_category": ["dont_place_item_category_test"],
+            "requires": "",
+            "category": ["For_Tests_And_Examples"]
+        },
+        {
+            "name": "Free location for tests items 1",
+            "_comment": "added those so we don't have more items than locations",
+            "requires": "",
+            "category": ["For_Tests_And_Examples"]
+        },
+        {
+            "name": "Free location for tests items 2",
+            "requires": "",
+            "category": ["For_Tests_And_Examples"]
+        },
+        {
+            "name": "Free location for tests items 3",
+            "requires": "",
+            "category": ["For_Tests_And_Examples"]
+        },
+        {
+            "name": "Free location for tests items 4",
+            "requires": "",
+            "category": ["For_Tests_And_Examples"]
         }
     ]
 }