Simplify/Clean battle controllers code

[DizzyEggg]

As we are all aware of, battle controllers' code is one of GF's worst creations. The amount of duplicated code over the controllers files is truly unmatched.

Goals of this PR:

• remove all of the duplicated code
• simplify to make the code easier to follow
• get rid of unused data and code
• remove usage of gActiveBattler in the controller files. The functions now take a battler argument.

This PR should be thoroughly tested as I might have accidentally omitted something while copying the functions. Things to keep an eye on are link battles, multi battles, recorded battles and multi link battles as things may break the most there.

Suggestions and reviews are welcome.

Note:
NONE of the functionality has changed, everything works exactly the same way as before. Compatibility with vanilla games is preserved(at least in the state it was before this PR).

Note 2:
I did the math and this PR saves approx whooping 80 064 bytes of space.

[AsparagusEduardo]

On one hand, I'm glad we're able to untangle this mess :D
On the other, I wonder if this will break the potential vanilla compatibility, as we've found out that it's actually surprisingly compatible due to all the logic and data being handled by a single game 👀

[DizzyEggg]

On one hand, I'm glad we're able to untangle this mess :D On the other, I wonder if this will break the potential vanilla compatibility, as we've found out that it's actually surprisingly compatible due to all the logic and data being handled by a single game 👀

I see no reason why the compatibility would be broken. I want the flow to remain the same, the difference being there is 1 function doing 1 thing, instead of 5 functions doing the same thing :D

[AsparagusEduardo]

I see no reason why the compatibility would be broken. I want the flow to remain the same, the difference being there is 1 function doing 1 thing, instead of 5 functions doing the same thing :D

Awesome, we're on the same league then :D

[DizzyEggg]

Ready for review. I also updated the first post with some more information.

[AsparagusEduardo]

If it's not breaking, why does it have the BREAKING tag? 👀

[DizzyEggg]

If it's not breaking, why does it have the BREAKING tag? 👀

You're right, I removed the tag.

[ghoulslash]

I love this PR :)

[ghoulslash]

Actually I think we can just consolidate all of these GetBattlerParty and gBattlerPartyIndexes into just GetBattlerPartyData calls. Sorry I should have noticed earlier

[ghoulslash]

It might also make sense to replace the gActiveBattler check in PrepareBufferDataTransfer with a battler argument, but that would require changing all battle controller emit function calls. But currently calling any of those without properly setting gActiveBattler can be problematic. especially since gActiveBattler is used a loop index in some places so can be set to gBattlersCount and therefore emitting data to that battler Id would result in bad UB

edit: actually does this not break some implementations of calling BtlController_x inside the battle controller files since gActiveBattler is not explicitly set?

[DizzyEggg]

It might also make sense to replace the gActiveBattler check in PrepareBufferDataTransfer with a battler argument, but that would require changing all battle controller emit function calls. But currently calling any of those without properly setting gActiveBattler can be problematic. especially since gActiveBattler is used a loop index in some places so can be set to gBattlersCount and therefore emitting data to that battler Id would result in bad UB

edit: actually does this not break some implementations of calling BtlController_x inside the battle controller files since gActiveBattler is not explicitly set?

This should also be done, but I think this PR is big enough as it is.
I don't think it should break anything, if the battler is provided via an argument instead of gActiveBattler.

[DizzyEggg]

Conflicts fixed. Ready to review & merge

[ghoulslash]

Been testing this in my own game and seems pretty solid. Will do some basic fresh expansion tests soon but should be good to go

[ghoulslash]

Alex confirmed link battles work, singles and doubles work as expected locally as well