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

Hylics 2: Remove Random Start option and replace it with Start Location option #3289

Merged
merged 5 commits into from
May 14, 2024

Conversation

chandler05
Copy link
Contributor

@chandler05 chandler05 commented May 12, 2024

Removes the Random Start option from Hylics 2 and replaces it with the Start Location option, allowing the player to select a specific starting location in addition to having it randomly chosen.
@TRPG0

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

@TRPG0 TRPG0 left a comment

Choose a reason for hiding this comment

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

I actually had just been thinking about doing this pretty recently lol. There's no reason you shouldn't be allowed to choose, and I'm pretty sure forcing it to be random breaks Universal Tracker anyway. Just one small change before I approve

worlds/hylics2/Options.py Outdated Show resolved Hide resolved
worlds/hylics2/__init__.py Show resolved Hide resolved
@TRPG0 TRPG0 added the is: enhancement Issues requesting new features or pull requests implementing new features. label May 12, 2024
"random_start" : self.options.random_start.value,
"start_location" : self.start_location,
"random_start": int(self.options.start_location != "waynehouse"),
"start_location" : self.options.start_location.current_option_name,
Copy link
Collaborator

@Exempt-Medic Exempt-Medic May 13, 2024

Choose a reason for hiding this comment

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

I assume this is meant to use the capitalized forms, with spaces, and the other small changes? So waynehouse -> Waynehouse, viewaxs_edifice -> Viewax's Edifice, and so on? I just ask because the docstring says "For display purposes. Worlds should be using current_key."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's why I used current_option_name instead. I did the same thing when implementing an option for A Short Hike a while back.

Copy link
Collaborator

@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.

The back-compat line functions correctly. Code/changes LGTM. It generates just fine. Only suggestion, you may want to add random_start as a Removed option so that old yamls using it on newer AP will throw an error instead of silently using the default.
Update: The new removed option works properly

@chandler05
Copy link
Contributor Author

Random Overcooked 2! test failure...

@Exempt-Medic Exempt-Medic added waiting-on: core-review Issue/PR has been peer-reviewed and is ready to be merged or needs input from a core maintainer. and removed waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. labels May 13, 2024
@NewSoupVi NewSoupVi merged commit 6576b06 into ArchipelagoMW:main May 14, 2024
15 of 16 checks passed
jnschurig pushed a commit to Tranquilite0/Archipelago-SoulBlazer that referenced this pull request Jun 13, 2024
…on option (ArchipelagoMW#3289)

* Hylics 2: Remove Random Start option and replace it with Start Location option

* remove choice

* Readd random start to slot data

* newlines

* Add random_start as a Removed option
qwint pushed a commit to qwint/Archipelago that referenced this pull request Jun 24, 2024
…on option (ArchipelagoMW#3289)

* Hylics 2: Remove Random Start option and replace it with Start Location option

* remove choice

* Readd random start to slot data

* newlines

* Add random_start as a Removed option
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. waiting-on: core-review Issue/PR has been peer-reviewed and is ready to be merged or needs input from a core maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants