3

I'm trying to understand a strange bug that happens in Pokemon Red/Blue where if you try using the move Recover when you're exactly 255 hp below your max HP, the move will fail.

The code I am looking at is a disassembly that can be found here. The specific code in question is under the .healEffect label.

I think I've figured out, code-wise, why this is happening. Let's say your max HP is 703, and you are currently at 448. The machine compares your HP values to make sure you aren't trying to heal at full HP. However, the programmers used the "cp" instruction, which is only for 8-bit numbers, cutting off the highest bit. I assume this was just simply programmer error.

703 = 00000010 10111111
448 = 00000001 11000000
Removing the highest bit, your max becomes 191 and current becomes 192. This isn't possible, so a carry is triggered.

Now where I am confused is why the sbc instruction is used. sbc is called, and the machine performs current HP - max HP - carry. Using the example above, this results in 0, meaning you're at full HP in the machine's eyes, and the move fails.

The cp instruction I can explain as just programmer error. But why on earth use sbc here? sbc is meant to be used for multi-word arithmetic, but HP is a 16-bit number. I'm struggling to think of a situation where sbc is called for in this situation.

Any ideas?

Gummysaur
  • 96
  • 7
  • 2
    `sbc` isn't necessarily only for multi-word use. You're operating on the bytes of a word in memory. The intent of the code seems to be to try to check both bytes of the words to check overall equality. The carry flag from the `cp` seems to be intended to be used by `sbc`. – Thomas Jager Sep 15 '21 at 15:42
  • 2
    _"sbc is meant to be used for multi-word arithmetic"_. You mean multi-byte? Perhaps the author intended to do a 16-bit comparison, but implemented it incorrectly, or implemented it correctly but then did some refactoring and broke it. – Michael Sep 15 '21 at 15:46
  • 3
    The `cp` performs a subtraction like `sub`, but without storing the result in `a`, and the following `sbc` is the correct second step to subtract two 16 bit values. I cannot see the error as you describe it, did you check this in an emulator? – the busybee Sep 15 '21 at 18:34
  • Are you sure the disassembly is correct about the variables of maximum and current HP? As shown, the difference is calculated by "current HP - maximum HP", which is only non-negative if both are equal; given that current HP are never more than maximum HP. On the other hand this makes sense because of the `jr z` later... Hm. The error might be somewhere else. Better check with an emulator! – the busybee Sep 15 '21 at 19:02
  • @thebusybee I believe the point of the operation is simply to see if the HP values are equal. That's what the jp z is there; jump (if the zero flag is set) to the "failed" label. It simply checks if the answer is 0, because if it is, the numbers are the same, meaning you're at full HP; if it's anything else at all, the numbers are different and the move can work. I have not tried this myself, but it's a well-documented glitch. I linked a video of it happening in the OP. – Gummysaur Sep 15 '21 at 23:31
  • I watch the video, but it is quite fast and the glitch can hardly be noticed. Anyway, you are confirming my analysis. So the error is somewhere else, perhaps later where Recover is handled specifically. – the busybee Sep 16 '21 at 05:46

2 Answers2

3

To check for equality you don't need the carry, you can simply check the two bytes separately. The carry is useful if you want to do "less/greater" which this code clearly intends to. As such the bug is the jp z which should be jp nc to mean "if you can subtract the max from the current without producing a carry that means current is at least as big as max so the operation is not allowed"

Jester
  • 56,577
  • 4
  • 81
  • 125
  • 1
    At first sight I thought this, too. But since the subtraction is done correctly, the MSB is only zero if current HP are equal to maximum HP. There is no need to check for the carry flag. So it seems as if the error is somewhere else. A bit later in the source there is a division by 2 for this special case, but this also looks OK. – the busybee Sep 16 '21 at 05:45
  • No, because the low byte is not checked. In the example, it's 255 but is ignored so the to values are not equal. – Jester Sep 16 '21 at 09:52
  • 2
    No. Since maximum HP are subtracted from current HP, the LSB can never be non-zero if the MSB is zero. The result is zero or negative. – the busybee Sep 16 '21 at 11:09
  • I see what you mean. – Jester Sep 16 '21 at 11:26
1

I'm not really an assembly wizard, but I think the issue is that HP might be big-endian, but in this case it's being treated as little-endian. As others have said, it seems that sbc is used to "borrow" from one 8-bit subtraction to another, allowing 16-bit arithmetic.

In the battle code, when damage is applied, we have damage stored as two bytes at wDamage and HP stored as two bytes at wBattleMonHP. Around line 4929 (I call it out below) we subtract the higher memory addresses first and the lower memory addresses second, using the carry flag the second time. This implies that we are "borrowing" from the lower memory addresses, so they should be the ones with the most significant bytes.

ApplyDamageToPlayerPokemon:
    ld hl, wDamage
    ld a, [hli]
    ld b, a
    ld a, [hl]
    or b
    jr z, ApplyAttackToPlayerPokemonDone ; we're done if damage is 0
    ld a, [wPlayerBattleStatus2]
    bit HAS_SUBSTITUTE_UP, a ; does the player have a substitute?
    jp nz, AttackSubstitute
; subtract the damage from the pokemon's current HP
; also, save the current HP at wHPBarOldHP and the new HP at wHPBarNewHP
    ld a, [hld] ; <<<<<<< line 4929 >>>>>
    ld b, a
    ld a, [wBattleMonHP + 1]
    ld [wHPBarOldHP], a
    sub b
    ld [wBattleMonHP + 1], a
    ld [wHPBarNewHP], a
    ld b, [hl]
    ld a, [wBattleMonHP]
    ld [wHPBarOldHP+1], a
    sbc b
    ld [wBattleMonHP], a
    ld [wHPBarNewHP+1], a

So if I'm right (and this code is working properly, which I don't know - it's Gen 1), our most significant byte for battle HP is at wBattleMonHP, and the least significant byte at wBattleMonHP + 1.

The .healEffect code compares current HP to max HP in the other order - it cps wBattleMonHP first, then "borrows" any necessary carry bit from wBattleMonHP + 1.

That would mean that when we need to carry, we subtract one more from the least significant byte with sbc, and then compare to zero. So if the subtraction would have been 1 (e.g. the difference is 255 or "-1"), it becomes zero and the heal is .failed

And interesting consequence if I'm understanding correctly, is that there's an edge case when you don't have to carry, where healing works as expected:

Max HP:     00000001 11111111 # 511
Current HP: 00000001 00000000 # 256
  cp "first" byte results in no carry (1-1 is possible without borrowing)
  sbc "second" byte is 0-255 = 1 and borrow
But 1 is not 0, so we heal even though the delta is 255!

I kind of want to test this now, if I can find my old pokemon game...

Edit:

I tested this in Pokemon Red and it works! A level 80 Chansey with 511 max HP and 256 current HP does heal with Softboiled, even though the difference in HP is 255. Since the current HP has the same most significant byte (0x01) as its max HP, there is no carry flag from the first cp, and the subtraction correctly identifies that the hp is not equal to the max hp.

Nigel Robbins
  • 26
  • 3