Core: Region connection helpers (ArchipelagoMW#1923)

* Region.create_exit and Region.connect helpers

* reduce code duplication and better naming in Region.connect

* thank you tests

* reorder class definition

* define entrance_type on Region

* document helpers

* drop __class_getitem__ for now

* review changes

Author: alwaysintreble

BaseClasses.py

@@ -9,7 +9,8 @@
 from argparse import Namespace
 from collections import ChainMap, Counter, deque
 from enum import IntEnum, IntFlag
-from typing import Any, Callable, Dict, Iterable, Iterator, List, NamedTuple, Optional, Set, Tuple, TypedDict, Union
+from typing import Any, Callable, Dict, Iterable, Iterator, List, NamedTuple, Optional, Set, Tuple, TypedDict, Union, \
+    Type, ClassVar
 
 import NetUtils
 import Options
@@ -788,6 +789,44 @@ def remove(self, item: Item):
             self.stale[item.player] = True
 
 
+class Entrance:
+    access_rule: Callable[[CollectionState], bool] = staticmethod(lambda state: True)
+    hide_path: bool = False
+    player: int
+    name: str
+    parent_region: Optional[Region]
+    connected_region: Optional[Region] = None
+    # LttP specific, TODO: should make a LttPEntrance
+    addresses = None
+    target = None
+
+    def __init__(self, player: int, name: str = '', parent: Region = None):
+        self.name = name
+        self.parent_region = parent
+        self.player = player
+
+    def can_reach(self, state: CollectionState) -> bool:
+        if self.parent_region.can_reach(state) and self.access_rule(state):
+            if not self.hide_path and not self in state.path:
+                state.path[self] = (self.name, state.path.get(self.parent_region, (self.parent_region.name, None)))
+            return True
+
+        return False
+
+    def connect(self, region: Region, addresses: Any = None, target: Any = None) -> None:
+        self.connected_region = region
+        self.target = target
+        self.addresses = addresses
+        region.entrances.append(self)
+
+    def __repr__(self):
+        return self.__str__()
+
+    def __str__(self):
+        world = self.parent_region.multiworld if self.parent_region else None
+        return world.get_name_string_for_object(self) if world else f'{self.name} (Player {self.player})'
+
+
 class Region:
     name: str
     _hint_text: str
@@ -796,6 +835,7 @@ class Region:
     entrances: List[Entrance]
     exits: List[Entrance]
     locations: List[Location]
+    entrance_type: ClassVar[Type[Entrance]] = Entrance
 
     def __init__(self, name: str, player: int, multiworld: MultiWorld, hint: Optional[str] = None):
         self.name = name
@@ -823,20 +863,48 @@ def can_reach_private(self, state: CollectionState) -> bool:
     def hint_text(self) -> str:
         return self._hint_text if self._hint_text else self.name
 
-    def get_connecting_entrance(self, is_main_entrance: typing.Callable[[Entrance], bool]) -> Entrance:
+    def get_connecting_entrance(self, is_main_entrance: Callable[[Entrance], bool]) -> Entrance:
         for entrance in self.entrances:
             if is_main_entrance(entrance):
                 return entrance
         for entrance in self.entrances:  # BFS might be better here, trying DFS for now.
             return entrance.parent_region.get_connecting_entrance(is_main_entrance)
 
-    def add_locations(self, locations: Dict[str, Optional[int]], location_type: Optional[typing.Type[Location]] = None) -> None:
-        """Adds locations to the Region object, where location_type is your Location class and locations is a dict of
-        location names to address."""
+    def add_locations(self, locations: Dict[str, Optional[int]],
+                      location_type: Optional[Type[Location]] = None) -> None:
+        """
+        Adds locations to the Region object, where location_type is your Location class and locations is a dict of
+        location names to address.
+        
+        :param locations: dictionary of locations to be created and added to this Region `{name: ID}`
+        :param location_type: Location class to be used to create the locations with"""
         if location_type is None:
             location_type = Location
         for location, address in locations.items():
             self.locations.append(location_type(self.player, location, address, self))
+    
+    def connect(self, connecting_region: Region, name: Optional[str] = None,
+                rule: Optional[Callable[[CollectionState], bool]] = None) -> None:
+        """
+        Connects this Region to another Region, placing the provided rule on the connection.
+        
+        :param connecting_region: Region object to connect to path is `self -> exiting_region`
+        :param name: name of the connection being created
+        :param rule: callable to determine access of this connection to go from self to the exiting_region"""
+        exit_ = self.create_exit(name if name else f"{self.name} -> {connecting_region.name}")
+        if rule:
+            exit_.access_rule = rule
+        exit_.connect(connecting_region)
+    
+    def create_exit(self, name: str) -> Entrance:
+        """
+        Creates and returns an Entrance object as an exit of this region.
+        
+        :param name: name of the Entrance being created
+        """
+        exit_ = self.entrance_type(self.player, name, self)
+        self.exits.append(exit_)
+        return exit_
 
     def add_exits(self, exits: Union[Iterable[str], Dict[str, Optional[str]]],
                   rules: Dict[str, Callable[[CollectionState], bool]] = None) -> None:
@@ -850,11 +918,9 @@ def add_exits(self, exits: Union[Iterable[str], Dict[str, Optional[str]]],
         if not isinstance(exits, Dict):
             exits = dict.fromkeys(exits)
         for connecting_region, name in exits.items():
-            entrance = Entrance(self.player, name if name else f"{self.name} -> {connecting_region}", self)
-            if rules and connecting_region in rules:
-                entrance.access_rule = rules[connecting_region]
-            self.exits.append(entrance)
-            entrance.connect(self.multiworld.get_region(connecting_region, self.player))
+            self.connect(self.multiworld.get_region(connecting_region, self.player),
+                         name,
+                         rules[connecting_region] if rules and connecting_region in rules else None)
 
     def __repr__(self):
         return self.__str__()
@@ -863,44 +929,6 @@ def __str__(self):
         return self.multiworld.get_name_string_for_object(self) if self.multiworld else f'{self.name} (Player {self.player})'
 
 
-class Entrance:
-    access_rule: Callable[[CollectionState], bool] = staticmethod(lambda state: True)
-    hide_path: bool = False
-    player: int
-    name: str
-    parent_region: Optional[Region]
-    connected_region: Optional[Region] = None
-    # LttP specific, TODO: should make a LttPEntrance
-    addresses = None
-    target = None
-
-    def __init__(self, player: int, name: str = '', parent: Region = None):
-        self.name = name
-        self.parent_region = parent
-        self.player = player
-
-    def can_reach(self, state: CollectionState) -> bool:
-        if self.parent_region.can_reach(state) and self.access_rule(state):
-            if not self.hide_path and not self in state.path:
-                state.path[self] = (self.name, state.path.get(self.parent_region, (self.parent_region.name, None)))
-            return True
-
-        return False
-
-    def connect(self, region: Region, addresses: Any = None, target: Any = None) -> None:
-        self.connected_region = region
-        self.target = target
-        self.addresses = addresses
-        region.entrances.append(self)
-
-    def __repr__(self):
-        return self.__str__()
-
-    def __str__(self):
-        world = self.parent_region.multiworld if self.parent_region else None
-        return world.get_name_string_for_object(self) if world else f'{self.name} (Player {self.player})'
-
-
 class LocationProgressType(IntEnum):
     DEFAULT = 1
     PRIORITY = 2

docs/world api.md

@@ -512,30 +512,28 @@ def create_items(self) -> None:
 def create_regions(self) -> None:
     # Add regions to the multiworld. "Menu" is the required starting point.
     # Arguments to Region() are name, player, world, and optionally hint_text
-    r = Region("Menu", self.player, self.multiworld)
-    # Set Region.exits to a list of entrances that are reachable from region
-    r.exits = [Entrance(self.player, "New game", r)]  # or use r.exits.append
-    # Append region to MultiWorld's regions
-    self.multiworld.regions.append(r)  # or use += [r...]
+    menu_region = Region("Menu", self.player, self.multiworld)
+    self.multiworld.regions.append(menu_region)  # or use += [menu_region...]
 
-    r = Region("Main Area", self.player, self.multiworld)
+    main_region = Region("Main Area", self.player, self.multiworld)
     # Add main area's locations to main area (all but final boss)
-    r.locations = [MyGameLocation(self.player, location.name,
-                   self.location_name_to_id[location.name], r)]
-    r.exits = [Entrance(self.player, "Boss Door", r)]
-    self.multiworld.regions.append(r)
+    main_region.add_locations(main_region_locations, MyGameLocation)
+    # or 
+    # main_region.locations = \
+    #   [MyGameLocation(self.player, location_name, self.location_name_to_id[location_name], main_region]
+    self.multiworld.regions.append(main_region)
 
-    r = Region("Boss Room", self.player, self.multiworld)
-    # add event to Boss Room
-    r.locations = [MyGameLocation(self.player, "Final Boss", None, r)]
-    self.multiworld.regions.append(r)
-    
-    # If entrances are not randomized, they should be connected here, otherwise
-    # they can also be connected at a later stage.
-    self.multiworld.get_entrance("New Game", self.player)
-        .connect(self.multiworld.get_region("Main Area", self.player))
-    self.multiworld.get_entrance("Boss Door", self.player)
-        .connect(self.multiworld.get_region("Boss Room", self.player))
+    boss_region = Region("Boss Room", self.player, self.multiworld)
+    # Add event to Boss Room
+    boss_region.locations.append(MyGameLocation(self.player, "Final Boss", None, boss_region))
+
+    # If entrances are not randomized, they should be connected here,
+    # otherwise they can also be connected at a later stage.
+    # Create Entrances and connect the Regions
+    menu_region.connect(main_region)  # connects the "Menu" and "Main Area", can also pass a rule
+    # or
+    main_region.add_exits({"Boss Room": "Boss Door"}, {"Boss Room": lambda state: state.has("Sword", self.player)})
+    # Connects the "Main Area" and "Boss Room" regions, and places a rule requiring the "Sword" item to traverse
 
     # If setting location access rules from data is easier here, set_rules can
     # possibly omitted.