Fix documentation about Magikarp length calculation and bugfix about "Magikarp lengths can be miscalculated"

[Idain]

This pull request tackles two documentation topics about Magikarp's length:

1. The maximum length of a Magikarp when judged by the Guru Fisher in Lake of Rage.
2. A badly documented fix in "Magikarp lengths can be miscalculated".

1. Wrong comment about maximum Magikarp's length in CalcMagikarpLength

First of all, let's talk about CalcMagikarpLength. This function first calculates the length in millimeters by using the Magikarp's DVs and the Trainer ID, and then convert it to feet and inches. The maximum length a Magikarp can have (in millimeters) is 1625mm (bugged version)/1755mm (fixed version), but the comment inside the function says the maximum is 1786mm (the bug makes it so that only b is used when calculating the length, instead of bc). To check this, I've run various calculations, both accounting for the bug in CalcMagikarpLength.BCLessThanDE and with the fixed version.

To prove this, let's assume bc has the maximum possible value ($ffff). According to CalcMagikarpLength, if bc ≥ $ff00: [wMagikarpLength] = c + 1370, whose code is located here, so let's start calculating:

; If bc = $ffff, then c = $ff = 255
length = c + 1370
length = 255 + 1370
length = 1625

Q: "According to the comment inside CalcMagikarpLength, the formula is different if bc < $ff00, so maybe the result could be 1786mm?"

A: I also accounted for this, and no, the max possible result is either 1590mm (with the bug) or 1755mm (fixed version). If you fix the function, the last threshold in the MagikarpLength table is used, which is 65510. Let's make two calculations, one for each version (the formula is z * 100 + (bc - x) / y):

Bugged version

; The highest threshold is x = 65410, where y = 2 and z = 14. We'll use bc = $ff00 - $1 = 65279

length = z * 100 + (bc - x) / y
length = 14 * 100 + (65279 - 65410) / 2
length = 1400 + (65405) / 2  ; The result is stored in bc and becomes -131 = 65405
length = 1400 + 190  ; The result is $7FBE = 32702, but the code only takes the low byte, which is $BE = 190
length = 1590

Fixed version

; With the code fixed, the highest threshold is x = 65510, where y = 1 and z = 15. We'll use bc = 65509

length = z * 100 + (bc - x) / y
length = 15 * 100 + (65509 - 65510) / 1
length = 1500 + (65535) / 1  ; The result is stored in bc and becomes -1 = 65535
length = 1500 + 255  ; The code only takes the low byte of the result, which is $FF = 255
length = 1755

2. Wrong fix in "Magikarp lengths can be miscalculated"

The bugfix doesn't account for cases where b > d, so instead of returning, it tries to compare the low bytes bc and de as if the high bytes were equal, which will give the wrong result. A simple fix is to ret nz to discard all cases where b ≠ d.

[Rangi42]

Nicely explained, thank you!

[Idain]

@Rangi42, done. Let me know if there's something else to address.

[mid-kid]

While we're at it, I'd add a ; BUG: marker to this, replacing some redundant bits of explanation if possible. https://github.com/pret/pokecrystal/wiki/Code-cleanup#refer-to-docsbugs_and_glitchesmd-instead-of-explaining-a-bug-inline

[Idain]

@mid-kid, how is it now?