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

LADX: combine warp options #4325

Merged

Conversation

threeandthreee
Copy link
Contributor

What is this fixing or adding?

LADX has two QoL warp options as toggles, but one only works if the other is enabled. This makes a single option combining both.

How was this tested?

Was added very early in the ladx beta process, has had a lot of playtesting.

@github-actions github-actions bot added the waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. label Dec 3, 2024
@Exempt-Medic Exempt-Medic added the is: enhancement Issues requesting new features or pull requests implementing new features. label Dec 3, 2024
@Exempt-Medic
Copy link
Member

Exempt-Medic commented Dec 3, 2024

This should probably mark both WarpImprovements and AdditionalWarpPoints as Removed. For an example https://github.com/ArchipelagoMW/Archipelago/blob/main/worlds/dark_souls_3/Options.py#L420

Copy link
Contributor

@palex00 palex00 left a comment

Choose a reason for hiding this comment

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

Tested in the Asyncs, done the poll, had lots of Syncs, you know the drill.

One thing I'd love is if the old options could be converted by YAMLs into the new ones (to make old YAMLs forward compatible) but then when would it stop I guess. The current Beta apworld v9.1 has this fwiw.

@NewSoupVi NewSoupVi merged commit ac8a206 into ArchipelagoMW:main Dec 3, 2024
16 checks passed
@NewSoupVi
Copy link
Member

NewSoupVi commented Dec 3, 2024

For the record, I think "yaml backwards compatibility" is one of those QoL features that we should as devs draw a strict boundary against - Unless it can be solved by aliases, or a one-or-two-line from_any override, it just makes our code worse and more complicated

@threeandthreee threeandthreee deleted the ladx/combine-warp-options branch December 10, 2024 07:25
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: 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