Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged

Conversation

Mysteryem
Copy link
Contributor

@Mysteryem Mysteryem commented Nov 17, 2024

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

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.
@github-actions github-actions bot added the waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. label Nov 17, 2024
@Exempt-Medic Exempt-Medic added the is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. label Nov 17, 2024
@Mysteryem
Copy link
Contributor Author

@CookieCat45

@NewSoupVi NewSoupVi added the waiting-on: world-maintainer Issue/PR is waiting for feedback or approval by the maintainer of a world. label Nov 18, 2024
@Exempt-Medic Exempt-Medic removed the waiting-on: world-maintainer Issue/PR is waiting for feedback or approval by the maintainer of a world. label Nov 19, 2024
@NewSoupVi NewSoupVi merged commit ba50c94 into ArchipelagoMW:main Nov 22, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants