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

The Witness: Local Laser Shuffle + Option Presets #2590

Merged
merged 15 commits into from
Jan 16, 2024

Conversation

NewSoupVi
Copy link
Member

@NewSoupVi NewSoupVi commented Dec 10, 2023

Added option presets.
One of the option presets relies on laser_shuffle: True with local lasers, which option presets don't support.
However, "shuffle_lasers: local" has actually been an option we have been considering adding for a while anyway, because it is a very popular setting.

Note: Every other place in the code should behave as expected in regards to the change in the laser shuffle option, as there is functionally no difference between "local" and "anywhere", and if self.options.laser_shuffle will resolve as True for both "local" and "anywhere" and as False for "off".

Tested:
Ran webhost locally and confirmed that the presets worked and no errors happened in the python console or browser console
Generated a couple of seeds to see whether local lasers works as intended

Btw: I know the comments don't show on the website, they're more for other ppl looking at the code

@NewSoupVi
Copy link
Member Author

NewSoupVi commented Dec 10, 2023

I would request a review from someone who plays The Witness on whether these presets make sense, before this PR is merged

As for the "Short & Dense" setting: I played this recently in a short sync on a whim and found it incredibly fun, because it's super fast but it feels packed with content because you're only solving non-trivial randomized puzzles and can usually fully clear areas. It's like Sigma any%/Sigma AL but forcing a different order. I personally played it with local laser shuffle as well but "local_items" is not support by presets I believe.

The other two should be self explanatory :)

@NewSoupVi NewSoupVi marked this pull request as draft December 10, 2023 11:00
@NewSoupVi NewSoupVi changed the title The Witness: Option presets The Witness: Local Laser Shuffle + Option Presets Dec 10, 2023
@NewSoupVi NewSoupVi marked this pull request as ready for review December 10, 2023 11:17
@NewSoupVi NewSoupVi added the is: enhancement Issues requesting new features or pull requests implementing new features. label Dec 10, 2023
@Exempt-Medic
Copy link
Member

The printout "[Player]'s Witness world doesn't have any progression items. Please turn on Symbol Shuffle, Door Shuffle or Laser Shuffle." can seem a little strange with shuffle_lasers: local given that there are progression items and laser shuffle isn't exactly "off". The printout also appears in a single-player world with shuffle_lasers: local but not with shuffle_lasers: anywhere which seem unusual/unintended.

@Silvris
Copy link
Collaborator

Silvris commented Dec 11, 2023

The printout "[Player]'s Witness world doesn't have any progression items. Please turn on Symbol Shuffle, Door Shuffle or Laser Shuffle." can seem a little strange with shuffle_lasers: local given that there are progression items and laser shuffle isn't exactly "off". The printout also appears in a single-player world with shuffle_lasers: local but not with shuffle_lasers: anywhere which seem unusual/unintended.

Maybe be more specific in the message stating you need items to place in other worlds? If we allow laser_shuffle: local to count for that, the Witness player will have all of their progression items be forced into their own world, which is effectively the same as off.

Copy link
Collaborator

@Silvris Silvris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM, not a Witness player so can't speak to the quality of the presets.

@NewSoupVi
Copy link
Member Author

Alright, I changed those messages to be a bit more sensible and specific and accurate :)

Copy link
Member

@Exempt-Medic Exempt-Medic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes all look good to me, including the presets

@NewSoupVi
Copy link
Member Author

NewSoupVi commented Dec 26, 2023

This PR depends on #2631 now

Edit: IT'S PASSING UNIT TESTS???
It really shouldn't without 2631 being merged
Please don't merge this if 2631 isn't merged lol
Here is a red x: ❌

@Berserker66 Berserker66 merged commit 5c7bae7 into ArchipelagoMW:main Jan 16, 2024
12 checks passed
Jouramie pushed a commit to Jouramie/Archipelago that referenced this pull request Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is: enhancement Issues requesting new features or pull requests implementing new features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants