-
Notifications
You must be signed in to change notification settings - Fork 706
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: more item groups, location groups, keysanity preset #3936
LADX: more item groups, location groups, keysanity preset #3936
Conversation
@@ -66,6 +68,15 @@ class LinksAwakeningWebWorld(WebWorld): | |||
)] | |||
theme = "dirt" | |||
option_groups = ladx_option_groups | |||
options_presets = { | |||
"Keysanity": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would probably change this. Maps, compasses, and stone beaks are not very "key" related. You could add in Instruments and call it "All Shuffle" or something maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I think upstream's preset is poorly named, then. "Shuffle Dungeon Items" maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with the points made by Medic and Scipio, HOWEVER: At the moment, presets always change every option. As a result, I think it's fine to add other options that seem to have a similar "spirit"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some cleanup, and some typing. While the rest of the world doesn't have good typing, that doesn't mean we shouldn't add some when new things are added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More logic options are always fun. Seeds with this implemented have generated as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR was tested as part of a Beta apworld that was used in three dedicated Test Asyncs, as well as dozen of normal Syncs. I have also personally run 200 test generations which succeeded.
We have also done polling on what users want, etc. which can be found here
The new feature has worked as expected and other stuff does not seem impacted.
Also let's be real, this is a very simple code change. It's just some design decision semantics.
What is this fixing or adding?
Adds a bunch more item groups, creates location groups by area, and adds a keysanity preset to the options page.
How was this tested?
Generated some seeds, tested preset in webhost, checked datapackage to see that groups were made correctly.