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

MMBN3: Modernizations and Minor Bugfixes #2991

Merged
merged 21 commits into from
Apr 18, 2024

Conversation

digiholic
Copy link
Collaborator

What is this fixing or adding?

  • Updates MMBN3 to new Options API
  • Cleans up Lua script, removing unused functions and un-magics some addresses. No functionality changes
  • Fixes a Non-Deterministic Random check in deciding which filler items to place, which could lead to different runs of the same seed to have different filler items
  • Fixes an issue where the OilBody Program was not being received due to a Program Color Index error.
  • Updates some legacy text in the setup instructions that no longer apply to modern BizHawk versions

How was this tested?

Locally generated and connected to ensure everything still compiles and runs, gave OilBody program via commands to ensure it still sends

@github-actions github-actions bot added the waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. label Mar 19, 2024
@ScipioWright ScipioWright added is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. is: refactor/cleanup Improvements to code/output readability or organizization. labels Mar 21, 2024
Copy link
Member

@black-sliver black-sliver left a comment

Choose a reason for hiding this comment

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

I feel like it would've been better to split this PR in 2.

Either way getting a second set of eyes of someone who plays the game to review and/or test and approve asap would be appreciated so we can get the bugfix in.

class MMBN3Options(PerGameCommonOptions):
extra_ranks: ExtraRanks
include_jobs: IncludeJobs
trade_quest_hinting: TradeQuestHinting
Copy link
Member

Choose a reason for hiding this comment

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

Please add newline at end of file

@Exempt-Medic Exempt-Medic added the waiting-on: author Issue/PR is waiting for feedback or changes from its author. label Mar 28, 2024
@Exempt-Medic Exempt-Medic removed the waiting-on: author Issue/PR is waiting for feedback or changes from its author. label Apr 6, 2024
Copy link
Collaborator

@Silvris Silvris left a comment

Choose a reason for hiding this comment

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

Did not test, but the changes look correct to me.

@Berserker66 Berserker66 merged commit f89cee4 into ArchipelagoMW:main Apr 18, 2024
15 checks passed
qwint pushed a commit to qwint/Archipelago that referenced this pull request Jun 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. is: refactor/cleanup Improvements to code/output readability or organizization. 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.

6 participants