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.

Sign up for GitHub

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

Jump to bottom

Muse Dash: Option Groups and Options Rework #3434

Merged
merged 18 commits into from
Jun 4, 2024
Merged

Muse Dash: Option Groups and Options Rework #3434

merged 18 commits into from
Jun 4, 2024

Conversation

DeamonHunter
Copy link
Collaborator

@DeamonHunter DeamonHunter commented Jun 2, 2024 •
edited
Loading

What is this fixing or adding?

Does the following:

  • Adds options group.
  • Include Songs and Starter Songs must have their respective song dlc's enabled.
  • Traps now are selected individually rather than by group.
  • The big Muse Plus dlc has been merged into the rest of the dlcs.

There sadly was one option that I couldn't change, and that was the Allow [Muse Plus] DLC Songs. I would like this to be in DLC Packs, but as the Presets set this value, it currently cannot be in them.

How was this tested?

With the newly added tests, as well as test generations to confirm things are working as intended.

If this makes graphical changes, please attach screenshots.

image
firefox_1Pes18y6Um

@DeamonHunter DeamonHunter added is: enhancement Issues requesting new features or pull requests implementing new features. affects: webhost Issues/PRs that touch webhost and may need additional validation. labels Jun 2, 2024
@github-actions github-actions bot added the waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. label Jun 2, 2024
@DeamonHunter DeamonHunter removed the affects: webhost Issues/PRs that touch webhost and may need additional validation. label Jun 2, 2024
@Exempt-Medic Exempt-Medic added is: maintenance Regular updates to requirements and utilities that do not fix bugs or change/add features. is: refactor/cleanup Improvements to code/output readability or organizization. labels Jun 2, 2024
Exempt-Medic
Exempt-Medic suggested changes Jun 2, 2024
View reviewed changes
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.

A few minor comments/questions/corrections

worlds/musedash/Options.py Outdated Show resolved Hide resolved
worlds/musedash/Options.py
Copy link
Collaborator

@Exempt-Medic Exempt-Medic Jun 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Just checking that the way the tooltips display on the new WebHost are how you want them to display. Most games have changed their docstrings to display in a different format

Copy link
Collaborator Author

@DeamonHunter DeamonHunter Jun 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for reminding me. Its now set up so there isn't as much white space at the front of them. And added an extra line in front of the paragraphs so they read a bit easier.

worlds/musedash/Options.py Outdated Show resolved Hide resolved
worlds/musedash/Options.py Outdated Show resolved Hide resolved
worlds/musedash/test/TestCollection.py Outdated Show resolved Hide resolved
worlds/musedash/test/TestPlandoSettings.py Outdated Show resolved Hide resolved
@DeamonHunter
Copy link
Collaborator Author

After the fix on main, the old Just As Planned (Muse Plus) DLC has now been merged with the rest. As well as a bunch of smaller fixes.

@DeamonHunter
Copy link
Collaborator Author

I fixed the failing default options test. Current test failure is one on main.

Default Yaml Generation fails to put a # in front of a comment if it doesn't have more than 4 spaces. And I think the - in the docs on sets meant that it added invalid options to the sets.

@Exempt-Medic
Copy link
Collaborator

Exempt-Medic commented Jun 3, 2024 •
edited
Loading

The way to fix the spacing is this:

"""
Docstring start on the second line
More docstring

Even more docstring
"""

This will make it so the sections aren't indented

Exempt-Medic
Exempt-Medic approved these changes Jun 3, 2024
View reviewed changes
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.

Changes LGTM. The WebHost display all looks great, new option groups work, old options correctly throw errors from being Removed, docstrings look good grammar wise, trap adjustments work properly, new tests make sense (and pass). Just one tiny correction

worlds/musedash/Options.py Outdated Show resolved Hide resolved
ScipioWright
ScipioWright approved these changes Jun 3, 2024
View reviewed changes
Copy link
Collaborator

@ScipioWright ScipioWright left a comment

Choose a reason for hiding this comment

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

Test failure is from that Lingo keyerror thing, if tests are rerun this will pass.
Looks fine otherwise, gens fine.

@ScipioWright ScipioWright 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 Jun 3, 2024
@NewSoupVi NewSoupVi merged commit 1331675 into ArchipelagoMW:main Jun 4, 2024
16 checks passed
wu4 pushed a commit to wu4/Archipelago that referenced this pull request Jun 6, 2024
@DeamonHunter @NewSoupVi @wu4
Muse Dash: Option Groups and Options Rework (ArchipelagoMW#3434)
cfabe09 
* Ensure that included/starter songs only include those within enabled dlcs.

* Allow filtering traps by trap instead of by category.

* Add in the currently available limited time dlcs to the dlc list.

* Add the option group to the webhost and cleanup some errors.

* Fix trap list.

* Update tests. Add new ones to test correctness of new features.

* Remove the old Just As Planned option

* Make traps order alphabetically. Also adjust the title for traps.

* Adjust new lines to better fit the website.

* Style fixes.

* Test adjustments and a fix due to test no longer having just as planned dlc.

* Undo spacing changes as it breaks yaml generation.

* Fix indenting in webhost.

* Add the old options in as removed. Also clean up unused import.

* Remove references to the old allow_just_as_planned_dlc_songs option in Muse Dash tests.

* Add newline to end of file.

---------

Co-authored-by: NewSoupVi <[email protected]>
agilbert1412 pushed a commit to agilbert1412/Archipelago that referenced this pull request Jun 13, 2024
@DeamonHunter @NewSoupVi @agilbert1412
Muse Dash: Option Groups and Options Rework (ArchipelagoMW#3434)
3dc93d8 
* Ensure that included/starter songs only include those within enabled dlcs.

* Allow filtering traps by trap instead of by category.

* Add in the currently available limited time dlcs to the dlc list.

* Add the option group to the webhost and cleanup some errors.

* Fix trap list.

* Update tests. Add new ones to test correctness of new features.

* Remove the old Just As Planned option

* Make traps order alphabetically. Also adjust the title for traps.

* Adjust new lines to better fit the website.

* Style fixes.

* Test adjustments and a fix due to test no longer having just as planned dlc.

* Undo spacing changes as it breaks yaml generation.

* Fix indenting in webhost.

* Add the old options in as removed. Also clean up unused import.

* Remove references to the old allow_just_as_planned_dlc_songs option in Muse Dash tests.

* Add newline to end of file.

---------

Co-authored-by: NewSoupVi <[email protected]>
jnschurig pushed a commit to Tranquilite0/Archipelago-SoulBlazer that referenced this pull request Jun 13, 2024
@DeamonHunter @NewSoupVi
Muse Dash: Option Groups and Options Rework (ArchipelagoMW#3434)
32f367c 
* Ensure that included/starter songs only include those within enabled dlcs.

* Allow filtering traps by trap instead of by category.

* Add in the currently available limited time dlcs to the dlc list.

* Add the option group to the webhost and cleanup some errors.

* Fix trap list.

* Update tests. Add new ones to test correctness of new features.

* Remove the old Just As Planned option

* Make traps order alphabetically. Also adjust the title for traps.

* Adjust new lines to better fit the website.

* Style fixes.

* Test adjustments and a fix due to test no longer having just as planned dlc.

* Undo spacing changes as it breaks yaml generation.

* Fix indenting in webhost.

* Add the old options in as removed. Also clean up unused import.

* Remove references to the old allow_just_as_planned_dlc_songs option in Muse Dash tests.

* Add newline to end of file.

---------

Co-authored-by: NewSoupVi <[email protected]>
sflavelle pushed a commit to sflavelle/Archipelago-tgc that referenced this pull request Jun 20, 2024
@DeamonHunter @NewSoupVi @sflavelle
Muse Dash: Option Groups and Options Rework (ArchipelagoMW#3434)
283d427 
* Ensure that included/starter songs only include those within enabled dlcs.

* Allow filtering traps by trap instead of by category.

* Add in the currently available limited time dlcs to the dlc list.

* Add the option group to the webhost and cleanup some errors.

* Fix trap list.

* Update tests. Add new ones to test correctness of new features.

* Remove the old Just As Planned option

* Make traps order alphabetically. Also adjust the title for traps.

* Adjust new lines to better fit the website.

* Style fixes.

* Test adjustments and a fix due to test no longer having just as planned dlc.

* Undo spacing changes as it breaks yaml generation.

* Fix indenting in webhost.

* Add the old options in as removed. Also clean up unused import.

* Remove references to the old allow_just_as_planned_dlc_songs option in Muse Dash tests.

* Add newline to end of file.

---------

Co-authored-by: NewSoupVi <[email protected]>
qwint pushed a commit to qwint/Archipelago that referenced this pull request Jun 24, 2024
@DeamonHunter @NewSoupVi @qwint
Muse Dash: Option Groups and Options Rework (ArchipelagoMW#3434)
fc2a5be 
* Ensure that included/starter songs only include those within enabled dlcs.

* Allow filtering traps by trap instead of by category.

* Add in the currently available limited time dlcs to the dlc list.

* Add the option group to the webhost and cleanup some errors.

* Fix trap list.

* Update tests. Add new ones to test correctness of new features.

* Remove the old Just As Planned option

* Make traps order alphabetically. Also adjust the title for traps.

* Adjust new lines to better fit the website.

* Style fixes.

* Test adjustments and a fix due to test no longer having just as planned dlc.

* Undo spacing changes as it breaks yaml generation.

* Fix indenting in webhost.

* Add the old options in as removed. Also clean up unused import.

* Remove references to the old allow_just_as_planned_dlc_songs option in Muse Dash tests.

* Add newline to end of file.

---------

Co-authored-by: NewSoupVi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Exempt-Medic Exempt-Medic Exempt-Medic approved these changes

@NewSoupVi NewSoupVi NewSoupVi approved these changes

@ScipioWright ScipioWright ScipioWright approved these changes

Assignees
No one assigned
Labels
is: enhancement Issues requesting new features or pull requests implementing new features. is: maintenance Regular updates to requirements and utilities that do not fix bugs or change/add features. is: refactor/cleanup Improvements to code/output readability or organizization. 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@DeamonHunter @Exempt-Medic @ScipioWright @NewSoupVi