pret / pokecrystal

Disassembly of Pokémon Crystal
https://pret.github.io/pokecrystal/
2.06k stars 770 forks source link

`AI_Smart_Curse.approve`, `.greatly_encourage`, and `.encourage` look like wrong labels #1130

Open Grate-Oracle-Lewot opened 2 weeks ago

Grate-Oracle-Lewot commented 2 weeks ago

Generally, with the smart AI, dec [hl] causes the move to be encouraged, and inc [hl] for it to be discouraged. This even appears to be the case later in the same function, at AI_Smart_Curse.maybe_greatly_encourage. But under .approve, .greatly_encourage, and .encourage, we're given some inc [hl]s. Am I misinterpreting something, or are these labels backwards?

The fix would look something like this:

AI_Smart_Curse:
    ld a, [wEnemyMonType1]
    cp GHOST
    jr z, .ghost_curse
    ld a, [wEnemyMonType2]
    cp GHOST
    jr z, .ghost_curse

    call AICheckEnemyHalfHP
-   jr nc, .encourage
+   jr nc, .discourage

    ld a, [wEnemyAtkLevel]
    cp BASE_STAT_LEVEL + 4
-   jr nc, .encourage
+   jr nc, .discourage
    cp BASE_STAT_LEVEL + 2
    ret nc

    ld a, [wBattleMonType1]
    cp GHOST
-   jr z, .greatly_encourage
+   jr z, .greatly_discourage
    cp SPECIAL
    ret nc
    ld a, [wBattleMonType2]
    cp SPECIAL
    ret nc
    call AI_80_20
    ret c
    dec [hl]
    dec [hl]
    ret

-.approve
+.disapprove
    inc [hl]
    inc [hl]
-.greatly_encourage
+.greatly_discourage
    inc [hl]
-.encourage
+.discourage
    inc [hl]
    ret

.ghost_curse
    ld a, [wPlayerSubStatus1]
    bit SUBSTATUS_CURSE, a
    jp nz, AIDiscourageMove

    push hl
    farcall FindAliveEnemyMons
    pop hl
    jr nc, .notlastmon

    push hl
    call AICheckLastPlayerMon
    pop hl
-   jr nz, .approve
+   jr nz, .disapprove

    jr .ghost_continue

.notlastmon
    push hl
    call AICheckLastPlayerMon
    pop hl
    jr z, .maybe_greatly_encourage

.ghost_continue
    call AICheckEnemyQuarterHP
-   jp nc, .approve
+   jp nc, .disapprove

    call AICheckEnemyHalfHP
-   jr nc, .greatly_encourage
+   jr nc, .greatly_discourage

    call AICheckEnemyMaxHP
    ret nc

    ld a, [wPlayerTurnsTaken]
    and a
    ret nz

.maybe_greatly_encourage
    call AI_50_50
    ret c

    dec [hl]
    dec [hl]
    ret

I don't know what to rename "approve" to besides "disapprove." I was told to make a PR about this, but that looks complicated and I want to make sure I'm not mistaken first.

mid-kid commented 1 week ago

I'm not well-versed enough in the AI code to know whether this change is correct or not, but I'd suggest proposing changes through PRs, and getting used to the flow of making those, as it'll be easier to comment on things if you have any suggestions in the future.

"disapprove" sounds fine, though I'd look around the code in the same file to see if there's any similar functions with names you can borrow.