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

MLSS: General bugfixes + Add patch extension to inno_setup.iss #3286

Merged
merged 12 commits into from
May 18, 2024

Conversation

jamesbrq
Copy link
Contributor

What is this fixing or adding?

This fixes a GBA bios softlock due to the header of the game being edited.

How was this tested?

The ROM was able to be verified in a fresh game and I could send checks out.

If this makes graphical changes, please attach screenshots.

@github-actions github-actions bot added the waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. label May 11, 2024
@Exempt-Medic Exempt-Medic added the is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. label May 11, 2024
Copy link
Contributor

@nicholassaylor nicholassaylor left a comment

Choose a reason for hiding this comment

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

Tested old and new verification and changes are fixed like described, LGTM

@github-actions github-actions bot added the affects: core Issues/PRs that touch core and may need additional validation. label May 17, 2024
@jamesbrq jamesbrq changed the title MLSS: Remove outdated header change for ROM verification MLSS: General bugfixes + Add patch extension to inno_setup.iss May 17, 2024
@jamesbrq
Copy link
Contributor Author

jamesbrq commented May 17, 2024

I accidentally used my main branch for this PR when I initially uploaded it, so I just updated the PR title since 2 other small other changes we're made. Nicholas' review is now out of date on this PR.

inno_setup.iss Outdated Show resolved Hide resolved
Copy link
Contributor

@nicholassaylor nicholassaylor left a comment

Choose a reason for hiding this comment

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

Additional changes also look good, renewing my review for approval

@black-sliver
Copy link
Member

Can someone test the setup changes? Easiest is probably running it from the CI build ( https://github.com/ArchipelagoMW/Archipelago/actions/runs/9134849817 ) , which would also test frozen 3.8, which I think we also never test 😓

Copy link
Collaborator

@beauxq beauxq left a comment

Choose a reason for hiding this comment

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

  • downloaded the Windows Python 3.8 build produced by CI
  • installed it
  • made the template yamls
  • moved Mario & Luigi Superstar Saga.yaml to Players
  • generated
  • double-clicked the patch, and it asked me for my rom

I stopped there because I don't have the game, but it seems to be working to that point.

also skimmed over the code, and don't see any problems

@NewSoupVi NewSoupVi merged commit 0e89388 into ArchipelagoMW:main May 18, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects: core Issues/PRs that touch core and may need additional validation. is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. 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.

None yet

7 participants