ArchipelagoMW / Archipelago

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

AHiT: Fix reconnecting rift access regions for starting and plando acts #4200

Closed Mysteryem closed 2 hours ago

Mysteryem commented 6 days ago

What is this fixing or adding?

Reconnecting an act in a telescope to a time rift removes the entrances to the time rift from its access regions because it will be accessible from the telescope instead.

By doing so early on, as a starting act with insanity act randomizer or as a plando-ed act, this can happen before the time rift itself has been reconnected to an act or other time rift. In which case, when later attempting to connect that time rift to an act or other time rift, the entrances from the rift access regions will no longer exist, so must be re-created. The original code was mistakenly re-creating the entrances from the time rift being reconnected, instead of from the rift access regions.

I'm not sure the entrances need to be removed in the first place and I think a better fix would be to not create the entrances until after act randomization is completed so that all the entrance reconnecting can be entirely avoided, but these changes would be larger, so I have made the simple fix for now.

How was this tested?

I have been writing logic tests as well as a fixing a number of minor/inconsequential logic issues that are not yet ready for review. The tests were what initially identified this issue.

To reproduce this issue with random seeds, either play with insanity act randomizer and a Chapter 1 or 2 start until a purple Time Rift gets chosen as one of the starting acts, or use act-plando to place Time Rifts on telescope acts, e.g.

  ActPlando:
    # Plando acts onto other acts. For example, "Train Rush": "Alpine Free Roam" will place Alpine Free Roam
    # at Train Rush.
    "Alpine Free Roam": "Time Rift - Rumbi Factory"
    "The Illness has Spread": "Time Rift - Sewers"
    "Down with the Mafia!": "Time Rift - Bazaar"

The playthrough from fully generating the same seed that failed the test contained a nonsensical path to reach Time Rift - Deep Sea because Time Rift levels do not contain Time Rift entrances (indicated by the entrance using the word "Portal"):

Deep Sea - Page: Urchin Ledge
        Menu -> Save File -> Spaceship
   =>   Spaceship -> Telescope -> Mafia Town
   =>   Mafia Town -> Mafia Town - Act 1
   =>   Time Rift - Dead Bird Studio -> Time Rift - Dead Bird Studio Portal - Entrance 2
   =>   Time Rift - Deep Sea

By visualising the regions, it could be seen that Time Rift - Dead Bird Studio (placed at Chapter 1 Act 1) had two unexpected connections to Time Rift - Deep Sea, which was actually placed where Time Rift - Dead Bird Studio would be in vanilla.

image


After this PR, the visualised regions show that Time Rift - Dead Bird Studio correctly connects to only the acts placed at Chapter 1 Act 2 and Chapter 1 Act 3, with Time Rift - Deep Sea being connected from Dead Bird Studio and Dead Bird Studio Basement as expected. image image

Mysteryem commented 5 days ago

@CookieCat45