Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix safari run/mud/bait function names and variable names #363

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

Cyniikal
Copy link

@Cyniikal Cyniikal commented Dec 29, 2024

Noticed a mismatch in naming when doing a deep dive on Safari mechanics, figured I'd try to merge my updated and (hopefully) correct changes.

Rename SafariRun to SafariThrowBait
Rename SafariWatching to SafariRun
Rename safariRunAttempts to safariFleeStage
Rename unk_311C to safariCatchStage

@adrienntindall
Copy link
Collaborator

This all looks good and I don't doubt we've made mistakes, but if it's alright I would like some evidence this renaming is correct since it's your first PR

@Cyniikal
Copy link
Author

Cyniikal commented Dec 29, 2024

This all looks good and I don't doubt we've made mistakes, but if it's alright I would like some evidence this renaming is correct since it's your first PR

From Bulbapedia

At the start of an encounter, both the catch rate and escape rate are set to their species defaults. When Bait is thrown, it lowers the escape rate by one stage, but has a 90% chance of also lowering the catch rate by one stage. When Mud is thrown, it raises the catch rate by one stage, but has a 90% chance of raising the escape rate by one stage.

BattleControllerPlayer_SafariRun -> BattleControllerPlayer_SafariThrowMud

static void BattleControllerPlayer_SafariRun(BattleSystem *bsys, BattleContext *ctx) {
    /* REDACTED */
    ctx->tempData = BattleSystem_Random(bsys) % 10;
    if (ctx->unk_311C < 12) {
        ctx->unk_311C++;
    }
    if (ctx->tempData != 0 && ctx->safariRunAttempts < 12) {
        ctx->safariRunAttempts++;
    }
}

This above function always increasing unk_311C and has a 90% chance of increasing safariRunAttempts, so it's a perfect match for what throwing mud does: Raise catch rate by 1 stage and have a 90% chance of raising the flee rate by 1 stage.

BattleControllerPlayer_SafariThrowMud -> BattleControllerPlayer_SafariThrowBait

Throwing bait always decreases the escape/flee stage and has a 90% chance of decreasing the catch stage. BattleControllerPlayer_SafariThrowBait now matches that. Previously this was named "throw mud" but was decreasing the flee/catch stages, which is what bait throwing does.

unk_311C -> safariCatchStage

Name created based on how throwing mud and throwing bait affect it. It is almost certainly the catch rate/stage.

safariRunAttempts -> safariFleeStage

This isn't technically the amount of times the pokemon has tried to run away, it's a modifier on the flee chance in the assembly routine ov12_0225E154 in overlay_12_battle_controller_opponent.s. When it's larger, it increases the chance that the pokemon flees.

(extra comments mine)

ov12_0225E154: ; 0x0225E154 This is the safari flee check function maybe worth renaming?
	push {r4, r5, r6, lr}
	add r4, r1, #0
	add r6, r0, #0
	ldr r0, [r4]
	bl ov12_0223B694
	add r5, r0, #0
	ldr r0, [r4]
	bl BattleSystem_GetBattleContext
	mov r2, #0
	ldrb r1, [r4, #9]
	add r3, r2, #0
	bl GetBattlerVar
	lsl r0, r0, #0x10
	lsr r0, r0, #0x10
	mov r1, #0x1a
	bl GetMonBaseStat ;1a = 26 = BaseMarshRate
	ldr r2, _0225E1CC ; =ov12_0226D140
	lsl r1, r5, #1 ; Left shift r5 by 1 and store in r1 (multiply r5 by 2)
	ldrb r2, [r2, r1]
	mul r2, r0 ; Multiply BaseMarshRate by 10
	add r0, r2, #0
	ldr r2, _0225E1D0 ; =ov12_0226D141
					  ; ov12_0226D141 = [_, 10, _, 10, _, 10, _, 10, _, 10, _, 10, _, 15, _, 20, _, 25, _, 30, _, 35, _, 40, _]
	ldrb r1, [r2, r1] ; Load ov12_0226D141 + (safariFleeStage * 2) (Array above)
	bl _s32_div_f ; Divide (BaseMarshRate*10)/chosen_value
			  ; For example, at the base modifier of 6, you'd have (BaseMarshRate*10)/10 = BaseMarshRate
	add r5, r0, #0 ; r5 = r0 + 0
	ldr r0, [r4]
	bl BattleSystem_Random ; Get a random 16 bit number [0, 65535]
	mov r1, #0xff ; Move 255 into R1
	bl _s32_div_f ; Divide r0 by r1: random_number  %= 255 (r1 is modulo)
	cmp r1, r5 ; r1 > r5 Compare (BaseMarshRate*10)/(ov12_0226D141[safariFleeStage*2]) to random number in [0, 254]
	ldr r0, [r4]
	bgt _0225E1AC ; If (random_number % 255) > (BaseMarshRate*10)/(ov12_0226D141[safariFleeStage*2]), then pokemon flees
	ldrb r1, [r4, #9] 
	mov r2, #4 ; 4 is probably the flag for 'opponent is still here' or something?
	bl ov12_02262F24 ; End turn without oponent fleeing
	b _0225E1B41 ; Unconditional branch to battle loop start?

BattleControllerPlayer_SafariWatching -> BattleControllerPlayer_SafariRun

This plays the BATTLE_SUBSCRIPT_SAFARI_ESCAPE message and ends the battle, this is the player running, not watching right?

@adrienntindall
Copy link
Collaborator

That does make things a lot clearer, thanks! I'll merge on build- and as a side note were you interested in disassembling the file containing ov12_0225E154? If not, I'd be able to do that file within the next few days to make things easier

@Cyniikal
Copy link
Author

I do sort of wonder if it's correct to have 5 switch cases in the safari part of BattleControllerPlayer_SelectionScreenInput

Like, is the safari run special or something? It being case 5 makes it impossible to use right? Idk.

switch (BattleBuffer_GetNext(ctx, battlerId)) {
case 1:
    ctx->unk_0[battlerId] = SSI_STATE_END;
    ctx->unk_4[battlerId] = SSI_STATE_13;
    ctx->playerActions[battlerId].command = CONTROLLER_COMMAND_SAFARI_THROW_BALL;
    break;
case 2:
    ctx->unk_0[battlerId] = SSI_STATE_END;
    ctx->unk_4[battlerId] = SSI_STATE_13;
    ctx->playerActions[battlerId].command = CONTROLLER_COMMAND_SAFARI_THROW_MUD;
    break;
case 3:
    ctx->unk_0[battlerId] = SSI_STATE_END;
    ctx->unk_4[battlerId] = SSI_STATE_13;
    ctx->playerActions[battlerId].command = CONTROLLER_COMMAND_SAFARI_THROW_BAIT;
    break;
case 4:
    ctx->unk_0[battlerId] = SSI_STATE_END;
    ctx->unk_4[battlerId] = SSI_STATE_13;
    ctx->playerActions[battlerId].command = CONTROLLER_COMMAND_RUN_INPUT;
    break;
case 5:
    ctx->unk_0[battlerId] = SSI_STATE_END;
    ctx->unk_0[battlerId] = SSI_STATE_13;
    ctx->playerActions[battlerId].command = CONTROLLER_COMMAND_SAFARI_RUN;
    break;
}

That does make things a lot clearer, thanks! I'll merge on build- and as a side note were you interested in disassembling the file containing ov12_0225E154? If not, I'd be able to do that file within the next few days to make things easier

Don't have much experience doing that, I could give it a go but no promises lol

@adrienntindall
Copy link
Collaborator

Like, is the safari run special or something? It being case 5 makes it impossible to use right? Idk.

Player chooses to run vs mon attempts to flee

Don't have much experience doing that, I could give it a go but no promises lol

You're welcome to join our discord if you'd like to give it a go, but it can be a steep learning curve so no pressure (though if you're able to read asm then you'll probably be fine), and again I can totally decompile that file sometime tomorrow

@adrienntindall
Copy link
Collaborator

Build failed, please ensure you replaced everything properly (ideally using replace.sh)

@Cyniikal
Copy link
Author

Cyniikal commented Dec 29, 2024

Build failed, please ensure you replaced everything properly (ideally using replace.sh)

Global find/replace with VSCode and a clean version with replace.sh yielded the same results, so not sure what's going on. Maybe due to enums changing? I'll mark this as a draft for now.

@Cyniikal Cyniikal marked this pull request as draft December 29, 2024 22:09
include/constants/battle.h Outdated Show resolved Hide resolved
Rename SafariRun to SafariThrowMud
Rename SafariThrowMud to SafariThrowBait
Rename SafariWatching to SafariRun
Rename safariRunAttempts to safariFleeStage
Rename unk_311C to safariCatchStage
Renaming safari battle enums caused the enums to be mismatched in the
battle.h and battle_controller_player.c files. This should realign them.
@Cyniikal Cyniikal force-pushed the fix/rename_safari_vars branch from 93393d3 to a468117 Compare December 30, 2024 00:55
@Cyniikal Cyniikal marked this pull request as ready for review December 30, 2024 01:15
@Cyniikal Cyniikal marked this pull request as draft December 30, 2024 23:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants