SM: fixed flawed and limited comeback check (#1398)

The issue at hand is fixing impossible seeds generated by a lack of properly checking if the player can come back to its previous region after reaching for a new location, as reported here: https://discord.com/channels/731205301247803413/1050529825212874763/1050529825212874763

The previous attempt at checking for comeback was only done against "Landing Site" and the custom start region which is a partial solution at best. For exemple, generating a single player plando seed with a custom starting location at "red_brinstar_elevator" with a forced red door at "RedTowerElevatorBottomLeft" and 2 Missiles set at "Morphing Ball" and "Energy Tank, Brinstar Ceiling" would generate an impossible seed where the player is expected to go through the green door "LandingSiteRight" with no Supers to go to the only possible next location "Power Bomb (red Brinstar spike room)". This is because the comeback check would pass because it would consider coming back to "Landing Site" enough.

The proposed solution is keeping a record of the last accessed region when collecting items. It would then be used as the source of the comeback check with the destination being the new location. This check had to be moved from can_fill() to can_reach() because the maximum_exploration_state of the AP filler only use can_reach().

Its still not perfect because collect() can be called in batch for many items at a time so the last accessed region will be set as the last collected item and will be used for the next comeback checks.

This was tested a bit with the given exemple above (its now failing generation) and by generating some 8 SM players seed with many door color rando, area rando and boss rando enabled.

worlds/sm/__init__.py

@@ -39,7 +39,7 @@ class SMCollectionState(metaclass=AutoLogicRegister):
         # for unit tests where MultiWorld is instantiated before worlds
         if hasattr(parent, "state"):
             self.smbm = {player: SMBoolManager(player, parent.state.smbm[player].maxDiff,
-                                    parent.state.smbm[player].onlyBossLeft) for player in
+                                    parent.state.smbm[player].onlyBossLeft, parent.state.smbm[player].lastAP) for player in
                                         parent.get_game_players("Super Metroid")}
             for player, group in parent.groups.items():
                 if (group["game"] == "Super Metroid"):
@@ -116,7 +116,7 @@ class SMWorld(World):
         Logic.factory('vanilla')
 
         self.variaRando = VariaRandomizer(self.multiworld, get_base_rom_path(), self.player)
-        self.multiworld.state.smbm[self.player] = SMBoolManager(self.player, self.variaRando.maxDifficulty)
+        self.multiworld.state.smbm[self.player] = SMBoolManager(self.player, self.variaRando.maxDifficulty, lastAP = self.variaRando.args.startLocation)
 
         # keeps Nothing items local so no player will ever pickup Nothing
         # doing so reduces contribution of this world to the Multiworld the more Nothing there is though
@@ -628,6 +628,11 @@ class SMWorld(World):
 
     def collect(self, state: CollectionState, item: Item) -> bool:
         state.smbm[self.player].addItem(item.type)
+        if (item.location != None and item.location.player == self.player):
+            for entrance in self.multiworld.get_region(item.location.parent_region.name, self.player).entrances:
+                if (entrance.parent_region.can_reach(state)):
+                    state.smbm[self.player].lastAP = entrance.parent_region.name
+                    break
         return super(SMWorld, self).collect(state, item)
 
     def remove(self, state: CollectionState, item: Item) -> bool:
@@ -736,15 +741,31 @@ class SMLocation(Location):
         super(SMLocation, self).__init__(player, name, address, parent)
 
     def can_fill(self, state: CollectionState, item: Item, check_access=True) -> bool:
-        return self.always_allow(state, item) or (self.item_rule(item) and (not check_access or (self.can_reach(state) and self.can_comeback(state, item))))
+        return self.always_allow(state, item) or (self.item_rule(item) and (not check_access or self.can_reach(state)))
+
+    def can_reach(self, state: CollectionState) -> bool:
+        # self.access_rule computes faster on average, so placing it first for faster abort
+        assert self.parent_region, "Can't reach location without region"
+        return self.access_rule(state) and self.parent_region.can_reach(state) and self.can_comeback(state)
+    
+    def can_comeback(self, state: CollectionState):
+        # some specific early/late game checks
+        if self.name == 'Bomb' or self.name == 'Mother Brain':
+            return True
 
-    def can_comeback(self, state: CollectionState, item: Item):
         randoExec = state.multiworld.worlds[self.player].variaRando.randoExec
+        n = 2 if GraphUtils.isStandardStart(randoExec.graphSettings.startAP) else 3
+        # is early game
+        if (len([loc for loc in state.locations_checked if loc.player == self.player]) <= n):
+            return True
+
         for key in locationsDict[self.name].AccessFrom.keys():
-            if (randoExec.areaGraph.canAccessList(  state.smbm[self.player], 
+            smbm = state.smbm[self.player]
+            if (randoExec.areaGraph.canAccess(  smbm, 
+                                                smbm.lastAP,
                                                 key,
-                                                    [randoExec.graphSettings.startAP, 'Landing Site'] if not GraphUtils.isStandardStart(randoExec.graphSettings.startAP) else ['Landing Site'],
+                                                smbm.maxDiff,
-                                                    state.smbm[self.player].maxDiff)):
+                                                None)):
                 return True
         return False
  

worlds/sm/variaRandomizer/graph/graph.py

@@ -367,22 +367,6 @@ class AccessGraph(object):
         #print("canAccess: {}".format(can))
         return can
 
-        # test access from an access point to a list of others, given an optional item
-    def canAccessList(self, smbm, srcAccessPointName, destAccessPointNameList, maxDiff, item=None):
-        if item is not None:
-            smbm.addItem(item)
-        #print("canAccess: item: {}, src: {}, dest: {}".format(item, srcAccessPointName, destAccessPointName))
-        destAccessPointList = [self.accessPoints[destAccessPointName] for destAccessPointName in destAccessPointNameList]
-        srcAccessPoint = self.accessPoints[srcAccessPointName]
-        availAccessPoints = self.getAvailableAccessPoints(srcAccessPoint, smbm, maxDiff, item)
-        can = any(ap in availAccessPoints for ap in destAccessPointList)
-        # if not can:
-        #     self.log.debug("canAccess KO: avail = {}".format([ap.Name for ap in availAccessPoints.keys()]))
-        if item is not None:
-            smbm.removeItem(item)
-        #print("canAccess: {}".format(can))
-        return can
-
     # returns a list of AccessPoint instances from srcAccessPointName to destAccessPointName
     # (not including source ap)
     # or None if no possible path

worlds/sm/variaRandomizer/logic/smboolmanager.py

@@ -13,7 +13,7 @@ class SMBoolManager(object):
     items = ['ETank', 'Missile', 'Super', 'PowerBomb', 'Bomb', 'Charge', 'Ice', 'HiJump', 'SpeedBooster', 'Wave', 'Spazer', 'SpringBall', 'Varia', 'Plasma', 'Grapple', 'Morph', 'Reserve', 'Gravity', 'XRayScope', 'SpaceJump', 'ScrewAttack', 'Nothing', 'NoEnergy', 'MotherBrain', 'Hyper'] + Bosses.Golden4()
     countItems = ['Missile', 'Super', 'PowerBomb', 'ETank', 'Reserve']
 
-    def __init__(self, player=0, maxDiff=sys.maxsize, onlyBossLeft = False):
+    def __init__(self, player=0, maxDiff=sys.maxsize, onlyBossLeft = False, lastAP = 'Landing Site'):
         self._items = { }
         self._counts = { }
 
@@ -21,6 +21,8 @@ class SMBoolManager(object):
         self.maxDiff = maxDiff
         self.onlyBossLeft = onlyBossLeft
 
+        self.lastAP = lastAP
+
         # cache related
         self.cacheKey = 0
         self.computeItemsPositions()