[ASM & Hex] Trouble with hooking

[CyberDork34]

I'm continuing my work to implement custom tag battles in Pokemon Emerald, and I've come across an issue with hooking.

The issue is as follows. So far, if I load Steven as the partner in the partner battle it works perfectly:

I've overwritten the seven partner trainer IDs before Steven to load other trainers. The backsprite table has 8 trainers in total. When I try to load a trainer that is NOT Steven with a backsprite, the game flips the hell out

It's not animated so you can't tell, but the battle menu is, like, completely unhinged from the screen. It scrolls up and down uncontrollably. In addition, when you try and start the battle, the game glitches beyond playability. If you try and switch your partner's pokemon with yours, the game will read a bad string and freeze. If you check your party pokemon and go back to the battle screen, the game will swap which pokemon are our, mix up your pokemon with your partners, change the battle type, and just in general screw everything.

So a lot of debugging and thinking later, I'm convinced this is because the game generates different graphics settings when Steven is your tag partner vs when a frontier trainer is. And trying to load a back sprite when the game expects something else breaks the whole thing.

My current guess is the following: in pokeemerald there's a function called CB2_InitBattleInternal. The if statement there does a hard check for Steven's trainer ID, and loads the graphics correctly for Steven. Since I'm extending the number of trainers that have backsprites, it seems these settings would be incorrect if backsprites were loaded unexpectedly.

So what I WISH the if statement at 647 looked like is something like this. And indeed, if I were just using the decomp proper, this is exactly what I'd write:

Spoiler:

// somwehere else
#define PARTNER_BATTLE_SLOT (gPartnerTrainerId <= TRAINER_STEVEN && gPartnerTrainerId >= TRAINER_MALE)
// ...
void CB2_InitBattleInternal(void) {
    // ...
    if (gBattleTypeFlags & BATTLE_TYPE_INGAME_PARTNER && !PARTNER_BATTLE_SLOT) {
        gBattle_WIN0V = 159;
        gBattle_WIN1H = 240;
        gBattle_WIN1V = 32;
    } else  {
        gBattle_WIN0V = 0x5051;
        ScanlineEffect_Clear();

        i = 0;
        while (i < 80)
        {
            gScanlineEffectRegBuffers[0][i] = 0xF0;
            gScanlineEffectRegBuffers[1][i] = 0xF0;
            i++;
        }

        while (i < 160)
        {
            gScanlineEffectRegBuffers[0][i] = 0xFF10;
            gScanlineEffectRegBuffers[1][i] = 0xFF10;
            i++;
        }

        ScanlineEffect_SetParams(sIntroScanlineParams16Bit);
    }
    // ...
}

Unfortunately, I can't just replace this if statement in code because this includes an extra check and presumably an extra branch instruction, which wouldn't fit where this function is in the ROM. So I have to branch out to my own function, run my own if statement, and continue execution afterwards.

It turns out this function is located at 0x80367d5 in Pokemon Emerald. Using a disassembler, I found it ends with

Spoiler:

MOVS            R1, #0
LDR             R0, =0x2024332
STRB            R1, [R0]
ADD             SP, SP, #4
POP             {R4,R5}
POP             {R0}
BX              R0

Or, in binary

Spoiler:

F8 DD 00 21 
0D 48 01 70 
01 B0 30 BC 
01 BC 00 47

Starting at 0x8036A1C. For hooking, I thought the aim was to replace the last few instructions with a branch to a different part of code. So something like

Spoiler:

MOVS            R1, #0
LDR             R0, =0x2024332
STRB            R1, [R0]
LDR             R0, =0x8XXYYZZ @ where ZZ is +1 for THUMB mode
BX              R0

or the following in binary, starting from the same place

Spoiler:

F8 DD 00 21 
0D 48 01 70 
00 48 00 47 
ZZ YY XX 08

When I try this, the battle freezes and never makes it to the intro. The music still plays but the screen is black forever. I'm guessing this is because the new routine never makes it back to where the first function was called.

The routine I'm hooking to looks like this in C

Spoiler:

void correct_scanlines(void) {
    // if this is true, the function above hasn't actually set
    // the scanlines right, redo that.
    if (battle_flags.player_ingame_partner && PARTNER_BATTLE_SLOT) {
        battle_WIN0V = 0x5051;
        scanline_effect_clear();

        uint8_t i = 0;
        while (i < 80) {
            scanline_effect_reg_buffers[0][i] = 0xF0;
            scanline_effect_reg_buffers[1][i] = 0xF0;
            i++;
        }

        while (i < 180) {
            scanline_effect_reg_buffers[0][i] = 0xFF10;
            scanline_effect_reg_buffers[1][i] = 0xFF10;
        }

        scanline_effect_set_params(intro_scanline_params16_bit);
    }
}

GCC generates the following assembly

Spoiler:

correct_scanlines:
	@ Function supports interworking.
	@ args = 0, pretend = 0, frame = 8
	@ frame_needed = 1, uses_anonymous_args = 0
	push	{fp, lr}
	add	fp, sp, #4
	sub	sp, sp, #8
	ldr	r3, .L64
	mov	r2, #0
	strb	r2, [r3]
	ldr	r3, .L64+4
	ldrb	r3, [r3, #2]	@ zero_extendqisi2
	and	r3, r3, #64
	and	r3, r3, #255
	cmp	r3, #0
	beq	.L63
	ldr	r3, .L64+8
	ldrh	r3, [r3]
	ldr	r2, .L64+12
	cmp	r3, r2
	bhi	.L63
	ldr	r3, .L64+8
	ldrh	r3, [r3]
	ldr	r2, .L64+16
	cmp	r3, r2
	bls	.L63
	ldr	r3, .L64+20
	ldr	r2, .L64+24
	strh	r2, [r3]	@ movhi
	bl	scanline_effect_clear
	mov	r3, #0
	strb	r3, [fp, #-5]
	b	.L59
.L60:
	ldrb	r3, [fp, #-5]	@ zero_extendqisi2
	ldr	r2, .L64+28
	lsl	r3, r3, #1
	add	r3, r2, r3
	mov	r2, #240
	strh	r2, [r3]	@ movhi
	ldrb	r3, [fp, #-5]	@ zero_extendqisi2
	ldr	r2, .L64+28
	add	r3, r3, #960
	lsl	r3, r3, #1
	add	r3, r2, r3
	mov	r2, #240
	strh	r2, [r3]	@ movhi
	ldrb	r3, [fp, #-5]	@ zero_extendqisi2
	add	r3, r3, #1
	strb	r3, [fp, #-5]
.L59:
	ldrb	r3, [fp, #-5]	@ zero_extendqisi2
	cmp	r3, #79
	bls	.L60
	b	.L61
.L62:
	ldrb	r3, [fp, #-5]	@ zero_extendqisi2
	ldr	r2, .L64+28
	lsl	r3, r3, #1
	add	r3, r2, r3
	mvn	r2, #239
	strh	r2, [r3]	@ movhi
	ldrb	r3, [fp, #-5]	@ zero_extendqisi2
	ldr	r2, .L64+28
	add	r3, r3, #960
	lsl	r3, r3, #1
	add	r3, r2, r3
	mvn	r2, #239
	strh	r2, [r3]	@ movhi
.L61:
	ldrb	r3, [fp, #-5]	@ zero_extendqisi2
	cmp	r3, #179
	bls	.L62
	ldr	r3, .L64+32
	ldm	r3, {r0, r1, r2}
	bl	scanline_effect_set_params
.L63:
	nop
	sub	sp, fp, #4
	@ sp needed
	pop	{fp, lr}
	bx	lr
.L65:
	.align	2
.L64:
	.word	battle_communication_struct @ ignore this, actually
	.word	battle_flags
	.word	partner_slot
	.word	3075
	.word	3067
	.word	battle_WIN0V
	.word	20561
	.word	scanline_effect_reg_buffers
	.word	intro_scanline_params16_bit
	.size	correct_scanlines, .-correct_scanlines
	.ident	"GCC: (devkitARM release 54) 10.1.0"

This is actually the first time I've done any hooking. Which, in hindsight seems really unfortunate given that this is not the easiest problem to solve. Please don't get distracted by the background information, my main concern here is that the hook freezes the game and not any of the stuff with loading backsprites. Even if you have no idea about the graphics stuff, if you've ever successfully written a hook I could use the help.