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

Timespinner: add indirect connections #3490

Merged
merged 1 commit into from
Jun 12, 2024

Conversation

Silvris
Copy link
Collaborator

@Silvris Silvris commented Jun 7, 2024

What is this fixing or adding?

Reviewing the spheres of an ongoing async showed that Timespinner is using can_reach(Region) in an Entrance rule without defining indirect connections, so they would often "miss spheres".

How was this tested?

Did some test generations, including with GyreArchives enabled. Ran unittests.

If this makes graphical changes, please attach screenshots.

@github-actions github-actions bot added the waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. label Jun 7, 2024
@Silvris
Copy link
Collaborator Author

Silvris commented Jun 7, 2024

@Jarno458

@Exempt-Medic Exempt-Medic added is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. waiting-on: world-maintainer Issue/PR is waiting for feedback or approval by the maintainer of a world. labels Jun 7, 2024
@Jarno458
Copy link
Collaborator

This seems to work, did a couple generations and checked the spoiler

@Jarno458 Jarno458 removed the waiting-on: world-maintainer Issue/PR is waiting for feedback or approval by the maintainer of a world. label Jun 10, 2024
@Jarno458
Copy link
Collaborator

Personally not sure if this should be labeled as bugfix, seems more like an refactor to me, but i respect your decision

@Silvris
Copy link
Collaborator Author

Silvris commented Jun 10, 2024

I'm pretty sure this prevents a chance at failed generation because of the possibility of a 0-location sphere if the only checks unlocked are Ifrit (which would skip a sphere due to this issue).

@NewSoupVi NewSoupVi merged commit b9e454a into ArchipelagoMW:main Jun 12, 2024
17 checks passed
agilbert1412 pushed a commit to agilbert1412/Archipelago that referenced this pull request Jun 13, 2024
jnschurig pushed a commit to Tranquilite0/Archipelago-SoulBlazer that referenced this pull request Jun 13, 2024
sflavelle pushed a commit to sflavelle/Archipelago-tgc that referenced this pull request Jun 20, 2024
qwint pushed a commit to qwint/Archipelago that referenced this pull request Jun 24, 2024
AustinSumigray pushed a commit to AustinSumigray/Archipelago that referenced this pull request Jan 4, 2025
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