Bug: After a trainer uses its 1st item, it'll start reading past the item list for items to use

[Rangi42]

https://twitter.com/pimanrules/status/1528137472680640513

https://youtu.be/Q6E6OaWb7LQ?t=1002

[Idain]

I'm testing with Blue, who has two Full Restores and he's able to use both items perfectly. I also checked the code and didn't see any irregularity.

[vulcandth]

Just looking at the code I found a potential issue... Althought i'd need to test to make sure.

AI_TryItem:
    ; items are not allowed in the Battle Tower
    ld a, [wInBattleTowerBattle]
    and a
    ret nz

    ld a, [wEnemyTrainerItem1]
    ld b, a
    ld a, [wEnemyTrainerItem2]
    or b
    ret z

    call .IsHighestLevel
    ret nc

    ld a, [wTrainerClass]
    dec a
    ld hl, TrainerClassAttributes + TRNATTR_AI_ITEM_SWITCH
    ld bc, NUM_TRAINER_ATTRIBUTES
    call AddNTimes
    ld b, h
    ld c, l
+ ; hl = AI_Items item script pointer table
    ld hl, AI_Items
+ ; de = the first item of two that the enemy trainer is using.
    ld de, wEnemyTrainerItem1
.loop
    ld a, [hl]
    and a
    inc a
+ ; if [hl] = -1 then return (end of table)
    ret z

    ld a, [de]
    cp [hl]
+ ; if current [hl] is equal to [wEnemyTrainerItem1] then we found our item, execute item script
    jr z, .has_item
+ ; increment de to point to wEnemyTrainerItem2
    inc de
    ld a, [de]
    cp [hl]
+ ; if current [hl] is equal to [wEnemyTrainerItem2] then we found our item, execute itme script
    jr z, .has_item

+ ; we did not found either items on current hl row, decrement de back to wEnemyTrainerItem1
    dec de
+ ; increment hl to the next row in AI_Items table
    inc hl
    inc hl
    inc hl
+ ; re-rerun the above on the next row.
    jr .loop

.has_item
+ ; we found a matching item, increment hl to the first byte of the script pointer.
    inc hl

+ ; we push hl & de onto the stack.. Note that if we jumped from a matching wEnemyTrainerItem2,
+ ; then de is still pointing to wEnemyTrainerItem2 and not wEnemyTrainerItem1.
    push hl
    push de
+ ; load the address of .callback into de
    ld de, .callback
+ ; push the address of .callback onto the stack, when we return after the jump, we will return
+ ; to this address!
    push de
+ ; jump to the script for this AI_Items row.
    ld a, [hli]
    ld h, [hl]
    ld l, a
    jp hl
.callback
+ ; We have returned from executing the script. Restore hl & de, remember,
+ ; that de will be either wEnemyTrainerItem1 or wEnemyTrainerItem2 depending on where we jumped from
+ ; before.
    pop de
    pop hl

+ ; increment past the pointer to the start of the next AI_Items row.
    inc hl
    inc hl
+ ; if the item script DID NOT use the item, for example if the item cures a status condition, and the pokemon
+ ; is not under any status conditions, then we loop again... This is a problem if we jumped from wEnemyTrainerItem2 as de
+ ; will still be set to the address of wEnemyTrainerItem2.. Which means that when it tries to increment de again to grab
+ ; what it thinks is wEnemytrainerItem2, It will be in fact wEnemyTrainerRewards address, ect.
    jr c, .loop

; used item
+ ; if the item was used.. then we set either wEnemyTrainerItem1 or wEnemyTrainerItem2 to 0.
    xor a
    ld [de], a

[vulcandth]

I think I would personally fix this by doing:

    ...
    ld hl, AI_Items
-   ld de, wEnemyTrainerItem1
.loop
+   ld de, wEnemyTrainerItem1
    ld a, [hl]
    and a
    inc a
    ret z
    ...

Hopefully the above makes sense; if not I can try to explain it better.. I haven't test any of this at all.. or traced it in bgb yet.. Just what I guessed from looking at the code.

[Idain]

Oh yes, you're right! There's a chance it might not use the item it finds which I didn't account for.