diff --git a/Main.py b/Main.py index d105bd4a..d0e7a7f8 100644 --- a/Main.py +++ b/Main.py @@ -148,7 +148,8 @@ def main(args, seed=None, baked_server_options: Optional[Dict[str, object]] = No else: multiworld.worlds[1].options.non_local_items.value = set() multiworld.worlds[1].options.local_items.value = set() - + + AutoWorld.call_all(multiworld, "connect_entrances") AutoWorld.call_all(multiworld, "generate_basic") # remove starting inventory from pool items. diff --git a/docs/entrance randomization.md b/docs/entrance randomization.md index 9e3e281b..0f9d7647 100644 --- a/docs/entrance randomization.md +++ b/docs/entrance randomization.md @@ -370,19 +370,13 @@ target_group_lookup = bake_target_group_lookup(world, get_target_groups) #### When to call `randomize_entrances` -The short answer is that you will almost always want to do ER in `pre_fill`. For more information why, continue reading. +The correct step for this is `World.connect_entrances`. -ER begins by collecting the entire item pool and then uses your access rules to try and prevent some kinds of failures. -This means 2 things about when you can call ER: -1. You must supply your item pool before calling ER, or call ER before setting any rules which require items. -2. If you have rules dependent on anything other than items (e.g. `Entrance`s or events), you must set your rules - and create your events before you call ER if you want to guarantee a correct output. - -If the conditions above are met, you could theoretically do ER as early as `create_regions`. However, plando is also -a consideration. Since item plando happens between `set_rules` and `pre_fill` and modifies the item pool, doing ER -in `pre_fill` is the only way to account for placements made by item plando, otherwise you risk impossible seeds or -generation failures. Obviously, if your world implements entrance plando, you will likely want to do that before ER as -well. +Currently, you could theoretically do it as early as `World.create_regions` or as late as `pre_fill`. +However, there are upcoming changes to Item Plando and Generic Entrance Randomizer to make the two features work better +together. +These changes necessitate that entrance randomization is done exactly in `World.connect_entrances`. +It is fine for your Entrances to be connected differently or not at all before this step. #### Informing your client about randomized entrances diff --git a/docs/world api.md b/docs/world api.md index 762189a9..90fe446d 100644 --- a/docs/world api.md +++ b/docs/world api.md @@ -490,6 +490,9 @@ In addition, the following methods can be implemented and are called in this ord after this step. Locations cannot be moved to different regions after this step. * `set_rules(self)` called to set access and item rules on locations and entrances. +* `connect_entrances(self)` + by the end of this step, all entrances must exist and be connected to their source and target regions. + Entrance randomization should be done here. * `generate_basic(self)` player-specific randomization that does not affect logic can be done here. * `pre_fill(self)`, `fill_hook(self)` and `post_fill(self)` diff --git a/test/benchmark/locations.py b/test/benchmark/locations.py index f2209eb6..857e1882 100644 --- a/test/benchmark/locations.py +++ b/test/benchmark/locations.py @@ -18,7 +18,15 @@ def run_locations_benchmark(): class BenchmarkRunner: gen_steps: typing.Tuple[str, ...] = ( - "generate_early", "create_regions", "create_items", "set_rules", "generate_basic", "pre_fill") + "generate_early", + "create_regions", + "create_items", + "set_rules", + "connect_entrances", + "generate_basic", + "pre_fill", + ) + rule_iterations: int = 100_000 if sys.version_info >= (3, 9): diff --git a/test/general/__init__.py b/test/general/__init__.py index 8afd8497..6c4d5092 100644 --- a/test/general/__init__.py +++ b/test/general/__init__.py @@ -5,7 +5,15 @@ from BaseClasses import CollectionState, Item, ItemClassification, Location, Mul from worlds import network_data_package from worlds.AutoWorld import World, call_all -gen_steps = ("generate_early", "create_regions", "create_items", "set_rules", "generate_basic", "pre_fill") +gen_steps = ( + "generate_early", + "create_regions", + "create_items", + "set_rules", + "connect_entrances", + "generate_basic", + "pre_fill", +) def setup_solo_multiworld( diff --git a/test/general/test_entrances.py b/test/general/test_entrances.py new file mode 100644 index 00000000..72161dfb --- /dev/null +++ b/test/general/test_entrances.py @@ -0,0 +1,36 @@ +import unittest +from worlds.AutoWorld import AutoWorldRegister, call_all, World +from . import setup_solo_multiworld + + +class TestBase(unittest.TestCase): + def test_entrance_connection_steps(self): + """Tests that Entrances are connected and not changed after connect_entrances.""" + def get_entrance_name_to_source_and_target_dict(world: World): + return [ + (entrance.name, entrance.parent_region, entrance.connected_region) + for entrance in world.get_entrances() + ] + + gen_steps = ("generate_early", "create_regions", "create_items", "set_rules", "connect_entrances") + additional_steps = ("generate_basic", "pre_fill") + + for game_name, world_type in AutoWorldRegister.world_types.items(): + with self.subTest("Game", game_name=game_name): + multiworld = setup_solo_multiworld(world_type, gen_steps) + + original_entrances = get_entrance_name_to_source_and_target_dict(multiworld.worlds[1]) + + self.assertTrue( + all(entrance[1] is not None and entrance[2] is not None for entrance in original_entrances), + f"{game_name} had unconnected entrances after connect_entrances" + ) + + for step in additional_steps: + with self.subTest("Step", step=step): + call_all(multiworld, step) + step_entrances = get_entrance_name_to_source_and_target_dict(multiworld.worlds[1]) + + self.assertEqual( + original_entrances, step_entrances, f"{game_name} modified entrances during {step}" + ) diff --git a/test/general/test_items.py b/test/general/test_items.py index 64ce1b69..91d334e9 100644 --- a/test/general/test_items.py +++ b/test/general/test_items.py @@ -67,7 +67,7 @@ class TestBase(unittest.TestCase): def test_itempool_not_modified(self): """Test that worlds don't modify the itempool after `create_items`""" gen_steps = ("generate_early", "create_regions", "create_items") - additional_steps = ("set_rules", "generate_basic", "pre_fill") + additional_steps = ("set_rules", "connect_entrances", "generate_basic", "pre_fill") excluded_games = ("Links Awakening DX", "Ocarina of Time", "SMZ3") worlds_to_test = {game: world for game, world in AutoWorldRegister.world_types.items() if game not in excluded_games} @@ -84,7 +84,7 @@ class TestBase(unittest.TestCase): def test_locality_not_modified(self): """Test that worlds don't modify the locality of items after duplicates are resolved""" gen_steps = ("generate_early", "create_regions", "create_items") - additional_steps = ("set_rules", "generate_basic", "pre_fill") + additional_steps = ("set_rules", "connect_entrances", "generate_basic", "pre_fill") worlds_to_test = {game: world for game, world in AutoWorldRegister.world_types.items()} for game_name, world_type in worlds_to_test.items(): with self.subTest("Game", game=game_name): diff --git a/test/general/test_locations.py b/test/general/test_locations.py index 4b95ebd2..37ae94e0 100644 --- a/test/general/test_locations.py +++ b/test/general/test_locations.py @@ -45,6 +45,12 @@ class TestBase(unittest.TestCase): self.assertEqual(location_count, len(multiworld.get_locations()), f"{game_name} modified locations count during rule creation") + call_all(multiworld, "connect_entrances") + self.assertEqual(region_count, len(multiworld.get_regions()), + f"{game_name} modified region count during rule creation") + self.assertEqual(location_count, len(multiworld.get_locations()), + f"{game_name} modified locations count during rule creation") + call_all(multiworld, "generate_basic") self.assertEqual(region_count, len(multiworld.get_regions()), f"{game_name} modified region count during generate_basic") diff --git a/test/general/test_reachability.py b/test/general/test_reachability.py index fafa7023..b45a2bdf 100644 --- a/test/general/test_reachability.py +++ b/test/general/test_reachability.py @@ -2,11 +2,11 @@ import unittest from BaseClasses import CollectionState from worlds.AutoWorld import AutoWorldRegister -from . import setup_solo_multiworld +from . import setup_solo_multiworld, gen_steps class TestBase(unittest.TestCase): - gen_steps = ["generate_early", "create_regions", "create_items", "set_rules", "generate_basic", "pre_fill"] + gen_steps = gen_steps default_settings_unreachable_regions = { "A Link to the Past": { diff --git a/worlds/AutoWorld.py b/worlds/AutoWorld.py index a5107179..0fcacc8a 100644 --- a/worlds/AutoWorld.py +++ b/worlds/AutoWorld.py @@ -378,6 +378,10 @@ class World(metaclass=AutoWorldRegister): """Method for setting the rules on the World's regions and locations.""" pass + def connect_entrances(self) -> None: + """Method to finalize the source and target regions of the World's entrances""" + pass + def generate_basic(self) -> None: """ Useful for randomizing things that don't affect logic but are better to be determined before the output stage. diff --git a/worlds/kh1/Regions.py b/worlds/kh1/Regions.py index a6f85fe6..6189adf2 100644 --- a/worlds/kh1/Regions.py +++ b/worlds/kh1/Regions.py @@ -483,6 +483,8 @@ def create_regions(multiworld: MultiWorld, player: int, options): for name, data in regions.items(): multiworld.regions.append(create_region(multiworld, player, name, data)) + +def connect_entrances(multiworld: MultiWorld, player: int): multiworld.get_entrance("Awakening", player).connect(multiworld.get_region("Awakening", player)) multiworld.get_entrance("Destiny Islands", player).connect(multiworld.get_region("Destiny Islands", player)) multiworld.get_entrance("Traverse Town", player).connect(multiworld.get_region("Traverse Town", player)) @@ -500,6 +502,7 @@ def create_regions(multiworld: MultiWorld, player: int, options): multiworld.get_entrance("World Map", player).connect(multiworld.get_region("World Map", player)) multiworld.get_entrance("Levels", player).connect(multiworld.get_region("Levels", player)) + def create_region(multiworld: MultiWorld, player: int, name: str, data: KH1RegionData): region = Region(name, player, multiworld) if data.locations: diff --git a/worlds/kh1/__init__.py b/worlds/kh1/__init__.py index 63b45755..3b498acf 100644 --- a/worlds/kh1/__init__.py +++ b/worlds/kh1/__init__.py @@ -6,7 +6,7 @@ from worlds.AutoWorld import WebWorld, World from .Items import KH1Item, KH1ItemData, event_item_table, get_items_by_category, item_table, item_name_groups from .Locations import KH1Location, location_table, get_locations_by_category, location_name_groups from .Options import KH1Options, kh1_option_groups -from .Regions import create_regions +from .Regions import connect_entrances, create_regions from .Rules import set_rules from .Presets import kh1_option_presets from worlds.LauncherComponents import Component, components, Type, launch_subprocess @@ -242,6 +242,9 @@ class KH1World(World): def create_regions(self): create_regions(self.multiworld, self.player, self.options) + + def connect_entrances(self): + connect_entrances(self.multiworld, self.player) def generate_early(self): value_names = ["Reports to Open End of the World", "Reports to Open Final Rest Door", "Reports in Pool"]