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

Magikarp length #1137

Merged
merged 4 commits into from
Mar 23, 2025
Merged

Magikarp length #1137

merged 4 commits into from
Mar 23, 2025

Conversation

itsdarsh
Copy link
Collaborator

@itsdarsh itsdarsh commented Mar 23, 2025

Making this pull request purely for posterity, as it could deserve a save patch. This would close #1131

Really, there are three issues this is resolving:

  1. When starting a new save file, the game initially stores the record Magikarp's length as $0306. This is a vestige from when the Polished codebase was in imperial units and the intended length was 3'6". However, the codebase is now in metric units, but the Magikarp length code hasn't been updated. Effectively this means the initial longest Magikarp is a pathetic 776mm, or about 2'6". The game now initializes to the initial record Magikarp being 1067mm, or 3'6". Note that this is longer than in other versions that used metric units (they initialized to 105.3mm in GSC and 106.5mm in HGSS), but preserving existing Polished balance will be a theme of this pull request.

  2. When the fishing guru checks your Magikarp length, the game calculates that length in millimeters and stores it in wMagikarpLengthMm. However, if the player is using imperial units, the game overwrites wMagikarpLengthMm with the length in feet in the high byte and inches in the low byte. This leads to the bizarre result of the check working correctly if the player is using metric units and incorrectly if the player is using imperial units. Storing the metric units on the stack before writing the imperial units and restoring them when done fixes this issue.

  3. Polished actually had the inverse problem of vanilla when determining the lengths of wild Magikarp - checks were done in imperial units when the data itself were in metric units. I should note that the millimeter thresholds I used are not identical to vanilla, which was 95% to reroll any Magikarp over 1616mm (5'4") and a total of 75% to reroll and Magikarp over 1600mm (5'3"). I instead used 1600mm (5'3") and 1575mm (5'2") respectively, as Polished had the reroll thresholds at 5'3" and 5'2". I don't know if that was intended or an oversight, but again, this is a bugfix and I'd like to avoid any sort of rebalancing. EDIT: Upon re-reading the initial comments, it seems the intention was indeed to match vanilla thresholds, but the code was off by one. Therefore, I decided to just use what vanilla already uses.

For the savepatch, since the name of the recordholder is initialized to Ralph, you could check if the name stored is indeed Ralph and check if the value of wMagikarpLengthMm is $0306, then update that to $042b. It's stored big-endian. It is of course possible that the player's name could also be Ralph, so I would strongly suggest checking for equality with $0306 rather than checking if it's less than $042b. I don't think the player gets a prize for tying the record, so it should be safe to update.

@itsdarsh itsdarsh added the minor-version Usually requires a save break, or other incompatibility label Mar 23, 2025
@itsdarsh itsdarsh merged commit 16d44d8 into Rangi42:master Mar 23, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor-version Usually requires a save break, or other incompatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lake of Rage Magikarp measurer not giving credit for big Magikarp
1 participant