shop cleanup and correctly backreference swapped items' locations

Also fixes a false reference in progression balancing from 2019 (swapped Location.item.location was not updated)

Fill.py

@@ -1,7 +1,7 @@
 import logging
 import typing
 
-from BaseClasses import CollectionState, PlandoItem
+from BaseClasses import CollectionState, PlandoItem, Location
 from Items import ItemFactory
 from Regions import key_drop_data
 
@@ -336,7 +336,8 @@ def balance_multiworld_progression(world):
                             replacement_locations.insert(0, new_location)
                             new_location = replacement_locations.pop()
 
-                        new_location.item, old_location.item = old_location.item, new_location.item
+                        swap_location_item(old_location, new_location)
+
                         new_location.event, old_location.event = True, False
                         logging.debug(f"Progression balancing moved {new_location.item} to {new_location}, "
                                       f"displacing {old_location.item} in {old_location}")
@@ -361,6 +362,18 @@ def balance_multiworld_progression(world):
                 raise RuntimeError('Not all required items reachable. Something went terribly wrong here.')
 
 
+def swap_location_item(location_1: Location, location_2: Location, check_locked=True):
+    """Swaps Items of locations. Does NOT swap flags like event, shop_slot or locked"""
+    if check_locked:
+        if location_1.locked:
+            logging.warning(f"Swapping {location_1}, which is marked as locked.")
+        if location_2.locked:
+            logging.warning(f"Swapping {location_2}, which is marked as locked.")
+    location_2.item, location_1.item = location_1.item, location_2.item
+    location_1.item.location = location_1
+    location_2.item.location = location_2
+
+
 def distribute_planned(world):
     world_name_lookup = {world.player_names[player_id][0]: player_id for player_id in world.player_ids}
 
@@ -368,7 +381,8 @@ def distribute_planned(world):
         placement: PlandoItem
         for placement in world.plando_items[player]:
             if placement.location in key_drop_data:
-                placement.warn(f"Can't place '{placement.item}' at '{placement.location}', as key drop shuffle locations are not supported yet.")
+                placement.warn(
+                    f"Can't place '{placement.item}' at '{placement.location}', as key drop shuffle locations are not supported yet.")
                 continue
             item = ItemFactory(placement.item, player)
             target_world: int = placement.world

Main.py

@@ -8,16 +8,19 @@ import random
 import time
 import zlib
 import concurrent.futures
+import typing
 
 from BaseClasses import World, CollectionState, Item, Region, Location, Shop
 from Items import ItemFactory, item_table, item_name_groups
-from Regions import create_regions, create_shops, mark_light_world_regions, lookup_vanilla_location_to_entrance, SHOP_ID_START
+from Regions import create_regions, create_shops, mark_light_world_regions, lookup_vanilla_location_to_entrance, \
+    SHOP_ID_START
 from InvertedRegions import create_inverted_regions, mark_dark_world_regions
 from EntranceShuffle import link_entrances, link_inverted_entrances, plando_connect
 from Rom import patch_rom, patch_race_rom, patch_enemizer, apply_rom_settings, LocalRom, get_hash_string
 from Rules import set_rules
 from Dungeons import create_dungeons, fill_dungeons, fill_dungeons_restrictive
-from Fill import distribute_items_restrictive, flood_items, balance_multiworld_progression, distribute_planned
+from Fill import distribute_items_restrictive, flood_items, balance_multiworld_progression, distribute_planned, \
+    swap_location_item
 from ItemPool import generate_itempool, difficulties, fill_prizes
 from Utils import output_path, parse_player_names, get_options, __version__, _version_tuple
 import Patch
@@ -210,18 +213,24 @@ def main(args, seed=None):
     if world.players > 1:
         balance_multiworld_progression(world)
 
+    shop_slots: typing.List[Location] = [location for shop_locations in (shop.region.locations for shop in world.shops)
+                                         for location in shop_locations if location.shop_slot]
+
+    if shop_slots:
+        # TODO: allow each game to register a blacklist to be used here?
         blacklist_words = {"Rupee", "Pendant", "Crystal"}
         blacklist_words = {item_name for item_name in item_table if any(
             blacklist_word in item_name for blacklist_word in blacklist_words)}
         blacklist_words.add("Bee")
-    candidates = [location for location in world.get_locations() if
+        candidates: typing.List[Location] = [location for location in world.get_locations() if
                                              not location.locked and
                                              not location.shop_slot and
                                              not location.item.name in blacklist_words]
 
         world.random.shuffle(candidates)
-    shop_slots = [item for sublist in (shop.region.locations for shop in world.shops) for item in sublist if
+
-                  item.shop_slot]
+        # currently special care needs to be taken so that Shop.region.locations.item is identical to Shop.inventory
+        # Potentially create Locations as needed and make inventory the only source, to prevent divergence
 
         for location in shop_slots:
             slot_num = int(location.name[-1]) - 1
@@ -230,9 +239,14 @@ def main(args, seed=None):
                 for c in candidates:  # chosen item locations
                     if c.item_rule(location.item):  # if rule is good...
                         logging.debug('Swapping {} with {}:: {} ||| {}'.format(c, location, c.item, location.item))
-                    c.item, location.item = location.item, c.item
+                        swap_location_item(c, location, check_locked=False)
+                        # TODO: should likely be (all_state-c.item).can_reach(shop.region)
+                        # can still be can_beat_game when beatable-only for the item-owning player
+                        # appears to be the main source of "progression items unreachable Exception"
+                        # in door-rando + multishop
                         if not world.can_beat_game():
-                        c.item, location.item = location.item, c.item
+                            # swap back
+                            swap_location_item(c, location, check_locked=False)
                         else:
                             # we use this candidate
                             candidates.remove(c)
@@ -243,7 +257,6 @@ def main(args, seed=None):
                     shop.region.locations.remove(location)
                     continue
 
-            # update table to location data
                 item_name = location.item.name
                 if any(x in item_name for x in ['Single Bomb', 'Single Arrow']):
                     price = world.random.randrange(1, 7)
@@ -302,7 +315,6 @@ def main(args, seed=None):
                            args.fastmenu[player], args.disablemusic[player], args.sprite[player],
                            palettes_options, world, player, True)
 
-
         mcsb_name = ''
         if all([world.mapshuffle[player], world.compassshuffle[player], world.keyshuffle[player],
                 world.bigkeyshuffle[player]]):