Update logic for supporting start_inventory and start_inventory_from_…

…pool

worlds/metroidprime/ItemPool.py

@@ -15,7 +15,7 @@
     from . import MetroidPrimeWorld
 
 
-def generate_start_inventory(world: "MetroidPrimeWorld") -> List[str]:
+def generate_base_start_inventory(world: "MetroidPrimeWorld") -> List[str]:
     assert world.starting_room_data.selected_loadout
     starting_items = [
         get_item_for_options(
@@ -30,15 +30,7 @@ def generate_start_inventory(world: "MetroidPrimeWorld") -> List[str]:
     if not world.options.shuffle_scan_visor:
         starting_items.append(SuitUpgrade.Scan_Visor.value)
 
-    already_collected_items = [
-        item
-        for item in [
-            *world.options.start_inventory_from_pool.value.keys(),
-            *world.options.start_inventory.value.keys(),
-        ]
-    ]
-
-    return [item for item in starting_items if item not in already_collected_items]
+    return starting_items
 
 
 def generate_item_pool(world: "MetroidPrimeWorld") -> List[MetroidPrimeItem]:
@@ -137,11 +129,10 @@ def generate_item_pool(world: "MetroidPrimeWorld") -> List[MetroidPrimeItem]:
         )
 
     assert world.starting_room_data.selected_loadout
-    items_to_remove: List[str] = [
-        # starting items
-        *[item for item in generate_start_inventory(world)],
-        # prefilled items
-        *[item for item in world.prefilled_item_map.values()],
+
+    items_to_remove = [
+        *world.prefilled_item_map.values(),
+        *generate_base_start_inventory(world),
     ]
 
     for item in items_to_remove:

worlds/metroidprime/__init__.py

@@ -33,7 +33,7 @@
 from .Config import make_config
 from .Regions import create_regions
 from .Locations import every_location
-from .ItemPool import generate_item_pool, generate_start_inventory
+from .ItemPool import generate_item_pool, generate_base_start_inventory
 from .PrimeOptions import (
     BlastShieldRandomization,
     DoorColorRandomization,
@@ -234,8 +234,15 @@ def generate_early(self) -> None:
             self.elevator_mapping = get_random_elevator_mapping(self)
 
         # Init starting inventory
-        starting_items = generate_start_inventory(self)
-        for item in starting_items:
+        starting_items = generate_base_start_inventory(self)
+        option_filled_items = [
+            *[item for item in self.options.start_inventory.value.keys()],
+            *[item for item in self.options.start_inventory_from_pool.value.keys()],
+        ]
+
+        for item in [
+            item for item in starting_items if item not in option_filled_items
+        ]:
             self.multiworld.push_precollected(
                 self.create_item(item, ItemClassification.progression)
             )
@@ -259,6 +266,18 @@ def create_item(
         )
 
     def create_items(self) -> None:
+        precollected_item_names = [
+            item.name for item in self.multiworld.precollected_items[self.player]
+        ]
+        new_map: Dict[str, str] = {}
+
+        for location, item in self.prefilled_item_map.items():
+            if item not in precollected_item_names:
+                # Prefilled items affect what goes into the item pool. If we already have collected something, we won't need to prefill it
+                new_map[location] = item
+
+        self.prefilled_item_map = new_map
+
         item_pool = generate_item_pool(self)
         self.multiworld.itempool += item_pool
 

worlds/metroidprime/data/StartRoomData.py

@@ -476,16 +476,17 @@ def init_starting_loadout(world: "MetroidPrimeWorld"):
 
     # If we are preventing bk then set a few items for prefill if available
     if not disable_bk_prevention:
-        for mapping in world.starting_room_data.selected_loadout.item_rules:
+        if len(world.starting_room_data.selected_loadout.item_rules):
+            mapping = world.random.choice(
+                world.starting_room_data.selected_loadout.item_rules
+            )
+
             for location_name, potential_items in mapping.items():
                 required_item = get_item_for_options(
                     world, world.random.choice(potential_items)
                 )
-                if (
-                    required_item.value
-                    not in world.options.start_inventory_from_pool.value.keys()
-                ):
-                    world.prefilled_item_map[location_name] = required_item.value
+                world.prefilled_item_map[location_name] = required_item.value
+
         if world.starting_room_data.local_early_items:
             for item in world.starting_room_data.local_early_items:
                 options_item = get_item_for_options(world, item)

worlds/metroidprime/test/TestShuffleScanVisor.py

@@ -19,7 +19,7 @@ def test_do_not_start_with_scan_visor(self):
             item.name for item in self.multiworld.precollected_items[self.player]
         ]
 
-    def test_scan_visor_not_in_item_pool(self):
+    def test_scan_visor_in_item_pool(self):
         assert SuitUpgrade.Scan_Visor.value in [
             item.name for item in self.multiworld.itempool
         ]
@@ -40,7 +40,7 @@ def test_start_with_scan_visor(self):
             item.name for item in self.multiworld.precollected_items[self.player]
         ]
 
-    def test_scan_visor_in_item_pool(self):
+    def test_scan_visor_not_in_item_pool(self):
         assert SuitUpgrade.Scan_Visor.value not in [
             item.name for item in self.multiworld.itempool
         ]

worlds/metroidprime/test/TestStartingRoom.py

@@ -99,18 +99,24 @@ def test_bk_prevention_should_not_add_items_when_in_start_inventory(
         )
 
 
-class TestPreCollectedItemsWithStartFromPool(MetroidPrimeWithOverridesTestBase):
+class TestPreCollectedItemsWithStartInventoryFromPool(
+    MetroidPrimeWithOverridesTestBase
+):
     run_default_tests = False  # type: ignore
-    options = {"start_inventory_from_pool": {SuitUpgrade.Missile_Expansion.value: 1}}
+    options = {"start_inventory": {SuitUpgrade.Missile_Expansion.value: 1}}
     overrides = {"starting_room_name": RoomName.Arboretum.value}
 
-    def test_should_not_double_collect_items_when_using_start_from_pool(
+    def test_should_not_double_collect_items_when_using_start_items(
         self,
     ):
-        self.assertNotIn(
-            SuitUpgrade.Missile_Expansion.value,
-            [item.name for item in self.multiworld.precollected_items[1]],
+        count = len(
+            [
+                item
+                for item in self.multiworld.precollected_items[self.player]
+                if item.name == SuitUpgrade.Missile_Expansion.value
+            ]
         )
+        self.assertEqual(count, 1, "Should only be one missile in precolelcted items")
 
 
 class TestPreCollectedItemsWithStartInventory(MetroidPrimeWithOverridesTestBase):
@@ -121,10 +127,14 @@ class TestPreCollectedItemsWithStartInventory(MetroidPrimeWithOverridesTestBase)
     def test_should_not_double_collect_items_when_using_start_items(
         self,
     ):
-        self.assertNotIn(
-            SuitUpgrade.Missile_Expansion.value,
-            [item.name for item in self.multiworld.precollected_items[1]],
+        count = len(
+            [
+                item
+                for item in self.multiworld.precollected_items[self.player]
+                if item.name == SuitUpgrade.Missile_Expansion.value
+            ]
         )
+        self.assertEqual(count, 1, "Should only be one missile in precolelcted items")
 
 
 class TestStartRoomBKPreventionEnabled(MetroidPrimeWithOverridesTestBase):
@@ -201,3 +211,21 @@ def test_starting_in_arboretum(self):
         self.assertTrue(
             self.can_reach_location("Chozo Ruins: Watery Hall - Scan Puzzle")
         )
+
+
+class TestPrefilledItemsNotPlaced(MetroidPrimeWithOverridesTestBase):
+    run_default_tests = False  # type: ignore
+    options = {"start_inventory_from_pool": {SuitUpgrade.Morph_Ball.value: 1}}
+    overrides = {"starting_room_name": RoomName.Save_Station_1.value}
+
+    def test_prefilled_items_should_not_be_placed_when_in_start_inventory(
+        self,
+    ):
+        self.assertNotIn(
+            SuitUpgrade.Morph_Ball.value,
+            self.world.prefilled_item_map.values(),
+        )
+        self.assertIn(
+            SuitUpgrade.Missile_Expansion.value,
+            self.world.prefilled_item_map.values(),
+        )

worlds/metroidprime/test/__init__.py

@@ -56,8 +56,17 @@ def world_setup(self, seed: typing.Optional[int] = None) -> None:
         self.world = self.multiworld.worlds[self.player]  # type: ignore
         self.pre_steps()
 
+        self.world.generate_early()
+
+        # A couple tests rely on this being set, it does not happen in the test base for some reason
+        if self.world.options.start_inventory_from_pool.value:
+            for item in self.world.options.start_inventory_from_pool.value.keys():
+                self.multiworld.push_precollected(self.world.create_item(item))
+        if self.world.options.start_inventory.value:
+            for item in self.world.options.start_inventory.value.keys():
+                self.multiworld.push_precollected(self.world.create_item(item))
+
         gen_steps = (
-            "generate_early",
             "create_regions",
             "create_items",
             "set_rules",