ArchipelagoMW / Archipelago

Archipelago Multi-Game Randomizer and Server
https://archipelago.gg
Other
469 stars 624 forks source link

Bug: OOT entrance shuffle checks if the game is beatable before setting its completion_condition #3716

Open Mysteryem opened 1 month ago

Mysteryem commented 1 month ago

What happened?

At the very end of EntranceShuffle.shuffle_random_entrances(), OOT checks if it can beat the game with how the entrances were shuffled: https://github.com/ArchipelagoMW/Archipelago/blob/77e3f9fbefa32a51c6c6ce1c6a0aee584c0a72f1/worlds/oot/EntranceShuffle.py#L631

But, the completion condition is set in Rules.set_rules(), which is run after EntranceShuffle.shuffle_random_entrances() in OOTWorld.set_rules()

From a debugger, it can be seen that the completion_condition function is still set to the lambda state: True lambda that gets set by default in MultiWorld.__init__(). image


Modifying the code to set the completion condition first also results in the beatable check failing every time when the completion conditions requires getting the "Triforce" event item because the new_all_state returned by new_all_state = ootworld.get_state_with_complete_itempool() specifically does not collect event items: https://github.com/ArchipelagoMW/Archipelago/blob/77e3f9fbefa32a51c6c6ce1c6a0aee584c0a72f1/worlds/oot/__init__.py#L1383

The beatable check seems to be irrelevant in general because the state created by get_state_with_complete_itempool() is an empty state that has been made to collect all items in the OOT item pool and pre-fill items, which is unaffected by shuffling entrances.

What were the expected results?

The completion condition should be set before checking if the game is beatable.

If the intent is to check if the arrangement of entrances results in a beatable seed, the check for the seed being beatable shouldn't be entirely independent from how the entrances were shuffled.

Software

Local generation