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

Pokemon Emerald: Convert to procedure patch #2995

Merged
merged 10 commits into from
May 5, 2024

Conversation

Zunawe
Copy link
Collaborator

@Zunawe Zunawe commented Mar 19, 2024

What is this fixing or adding?

Converts Emerald to use APProcedurePatch.

extracted_data.json was changed to include a battle type enum. Previously it was being read during patching, but now it's being read when extracted_data.json is created. That's its only change.

I know stuff like struct.pack("<B", 0) is sorta silly, but I like how explicit and consistent it is.

How was this tested?

Generated 9 seeds using a fully randomized yaml on main and on this branch and compared the md5 hashes of the patched ROM.

Community has had access to an apworld that includes this change for a couple weeks and have not reported any issues except #3161, which already existed and was only revealed by these changes.

@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
@Zunawe Zunawe marked this pull request as draft March 19, 2024 23:15
@Zunawe
Copy link
Collaborator Author

Zunawe commented Mar 19, 2024

Moved to draft waiting on #2996

Not dependent on it except to avoid the bug it fixes.

@Zunawe Zunawe marked this pull request as ready for review March 20, 2024 22:45
@Zunawe
Copy link
Collaborator Author

Zunawe commented Mar 20, 2024

Double-checked the output of a 2-player game now that #2996 is merged, files are identical.

@Zunawe Zunawe added the is: enhancement Issues requesting new features or pull requests implementing new features. label Mar 24, 2024
Copy link

@CoreZer0 CoreZer0 left a comment

Choose a reason for hiding this comment

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

I used Emerald as a guideline for converting Guardian Legend to APPP successfully. Read through of the code looks good to me.

@NewSoupVi NewSoupVi 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 May 3, 2024
Copy link
Member

@NewSoupVi NewSoupVi left a comment

Choose a reason for hiding this comment

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

I don't 100% understand rom stuff, but I checked the conversion and it seems to be 1:1, so in my estimation anything that that doesn't work here is a problem with APProcedurePatch itself, in which case I think it'd be good to have a big game in that uses it so we can expose any issues with it :) And the "it's been used by the community for a while as an .apworld" and "I compared rom hashes across 9 fully random multiworld seeds" is quite convincing :D

@NewSoupVi NewSoupVi merged commit 7e61211 into ArchipelagoMW:main May 5, 2024
16 checks passed
@Zunawe Zunawe deleted the emerald-appp branch May 23, 2024 08:42
jnschurig pushed a commit to Tranquilite0/Archipelago-SoulBlazer that referenced this pull request Jun 13, 2024
* Pokemon Emerald: Convert to procedure patch

* Pokemon Emerald: Remove assertion for vanilla rom's existence

* Pokemon Emerald: Add APPP implication to changelog

* Pokemon Emerald: Move procedure patch changelog line to new version

* Pokemon Emerald: Modify changelog versions

* Pokemon Emerald: Fix patch file download not appearing

---------

Co-authored-by: NewSoupVi <57900059+NewSoupVi@users.noreply.github.com>
qwint pushed a commit to qwint/Archipelago that referenced this pull request Jun 24, 2024
* Pokemon Emerald: Convert to procedure patch

* Pokemon Emerald: Remove assertion for vanilla rom's existence

* Pokemon Emerald: Add APPP implication to changelog

* Pokemon Emerald: Move procedure patch changelog line to new version

* Pokemon Emerald: Modify changelog versions

* Pokemon Emerald: Fix patch file download not appearing

---------

Co-authored-by: NewSoupVi <57900059+NewSoupVi@users.noreply.github.com>
AustinSumigray pushed a commit to AustinSumigray/Archipelago that referenced this pull request Jan 4, 2025
* Pokemon Emerald: Convert to procedure patch

* Pokemon Emerald: Remove assertion for vanilla rom's existence

* Pokemon Emerald: Add APPP implication to changelog

* Pokemon Emerald: Move procedure patch changelog line to new version

* Pokemon Emerald: Modify changelog versions

* Pokemon Emerald: Fix patch file download not appearing

---------

Co-authored-by: NewSoupVi <57900059+NewSoupVi@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is: enhancement Issues requesting new features or pull requests implementing new features. 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
Development

Successfully merging this pull request may close these issues.

4 participants