Get rid of the gActiveBattler variable #3262
Conversation
|
Now this we truly have to make sure that doesn't break linking between versions ^^; |
|
I think this is ready for testing and review. I did some initial plays and everything seemed to be working correctly 👀 |
| shouldSwitch = ShouldSwitch(); | ||
| battlerToSwitch = *(gBattleStruct->AI_monToSwitchIntoId + gActiveBattler); | ||
| gActiveBattler = backupBattler; | ||
| shouldSwitch = ShouldSwitch(battlerAtk); |
There was a problem hiding this comment.
somewhat unrelated to this PR, but is it a problem to call ShouldSwitch and run through the EmitReturnValues inside an AI calculation? Couldnt that potentially mess up other data transfers?
There was a problem hiding this comment.
Hm, you may be right. This should be investigated and fixed if it's an issue.
| @@ -240,21 +240,21 @@ static bool8 FindMonThatAbsorbsOpponentsMove(void) | |||
| if (absorbingTypeAbilities[j] == monAbility && Random() & 1) | |||
| { | |||
| // we found a mon. | |||
| *(gBattleStruct->AI_monToSwitchIntoId + gActiveBattler) = i; | |||
There was a problem hiding this comment.
not necessarily in this PR, but we should change *(gBattleStruct->AI_monToSwitchIntoId + battler); to gBattleStruct->AI_monToSwitchIntoId[battler]; and other similar lines
There was a problem hiding this comment.
Yeah, I agree, these should be changed.
| @ FD 3C - preiously gActiveBattler | ||
| @ FD 3D - preiously gActiveBattler without Illusion Check |
There was a problem hiding this comment.
| @ FD 3C - preiously gActiveBattler | |
| @ FD 3D - preiously gActiveBattler without Illusion Check | |
| @ FD 3C - preiously B_ACTIVE_NAME (gActiveBattler) | |
| @ FD 3D - preiously B_ACTIVE_NAME2 (gActiveBattler without Illusion Check) |
| @@ -367,7 +367,7 @@ B_ATK_PARTNER_NAME = FD 0E | |||
| B_ATK_NAME_WITH_PREFIX = FD 0F | |||
| B_DEF_NAME_WITH_PREFIX = FD 10 | |||
| B_EFF_NAME_WITH_PREFIX = FD 11 @ EFF = short for gEffectBattler | |||
| B_ACTIVE_NAME_WITH_PREFIX = FD 12 | |||
| @ FD 12 - preiously gActiveBattler with prefix | |||
There was a problem hiding this comment.
| @ FD 12 - preiously gActiveBattler with prefix | |
| @ FD 12 - preiously B_ACTIVE_NAME_WITH_PREFIX (gActiveBattler with prefix) |
| @@ -112,6 +112,7 @@ BattleScript_ItemSetFocusEnergy:: | |||
| setfocusenergy | |||
| playmoveanimation BS_ATTACKER, MOVE_FOCUS_ENERGY | |||
| waitanimation | |||
| copybyte sBATTLER, gBattlerAttacker | |||
There was a problem hiding this comment.
Spaces instead of tab.
gActiveBattler is one of the worst global variables used in the battle's code. It's caused many problems before, because there are functions which expect that value to be correctly set when called. I laid the groundwork in the controllers clean-up PR #3202, and this will be a follow-up.
Goals:
Additional things:
I moved sBattleBuffersTransferData to gBattleResources, so it's allocated dynamically which frees 256 bytes in EWRAM.
I renamed instances of battlerId to battler for consistency
It doesn't add/remove any features, so all the compatibility should be preserved
gActiveBattler was used in some strings, so I created an additional gSelectionBattler variable in its place, so that the messages display correct battler. I could change it to sth else if anyone has a better idea.