-
Notifications
You must be signed in to change notification settings - Fork 671
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
Muse Dash: Add filler items and rework generation balance #2809
Muse Dash: Add filler items and rework generation balance #2809
Conversation
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.
Options and presets look good except for a small typo in the tooltip marked in the review.
Only reviewed Options and Presets
Co-authored-by: Nicholas Saylor <[email protected]>
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.
Reviewed code updates and everything I was concerned about turned out to be already covered, (like create_items() seemingly being ok with adding more items than locations, but not being mathematically possible)
Also ran generate on different song count, trap settings, and music sheet percents (making sure to confirm both none trap types and 0 trap percent don't add any traps to the pool) to confirm no fill errors are weird values,
but did not play any games as I don't own it.
One thing that shouldn't stop this PR at all but is good to note is the TestDifficultyRanges test file is still using multiworld.option, even though the rest of the world is up to date.
That and i'm assuming the Unit test failure is a random failure, nothing failed on my machine (but I'm not running the same profile that failed)
Failure is from MMBN3 and is unrelated to the PR (for those concerned with the failure) |
Thanks for catching the old options usage. And yes the old test failure was unrelated to the PR. It should be impossible for the create items func to create more items than locations. It does seem to not care at first, but that is because all worlds will have enough space for a copy of every song, plus all the music sheets required. (As every song has 2 locations now.) And then every step after that checks to ensure it doesn't add too much. |
…temsTry2 # Conflicts: # worlds/musedash/MuseDashCollection.py
Resolved the merge conflict. Bringing this to core-review. Special note if merging before the big async. This does end up removing the option |
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.
Changes I requested were fixed
What is this fixing or adding?
This PR aims to fix a problem within Muse Dash. The total lack of filler items. This ends up changing the balance of the Muse Dash rando so other changes have been made.
Full list of changes:
How was this tested?
Using multiple Beta Test games using discord. As well as existing tests to ensure no old functionality was broken.