Quoting myself from a post in another thread I made a few days ago:
Battle mode code changes are not going well. I hate to say it, but this is turning into a major re-write. I didn't want to do it and I really tried hard to avoid doing it, but things were just too messed up in there its very difficult and frustrating to try to fix one thing without fixing them all. I'll be continuing to work on this over the next few days but you won't see any commits from me until I have something stable in there again with almost all of our old functionality back.
So that's how it is. I know some of you may be rolling your eyes at me right now thinking "damnit, Roots is doing another unnecessary re-write" but trust me, we need
this and the sooner the better. We need it because when I went in to try to make a quick fix to the display location of the stamina icons, I found myself having to make non-minor modifications in 3 different source files. When it takes that much work to fix a freaking draw position, something is very wrong.
Anyway, with all the changes I'm making I doubt this is going to be ready for the internal release coming in less than a week. So unless I fix everything and have ample time for testing, I won't be committing any of my changes for at least another week. I'm writing down a list of changes that I'm working on here for review. If you don't like something or you think something should be done a different/better way, let me know.
1) State-based processing
One problem we have in the battle code, especially in the BattleMode class, is that we have a clumsy handful of members for representing various states of operation. For example, there's a boolean for whether or not a dialogue is taking place. There's a boolean for if the battle is over or not. There's a boolean for if the characters were victorious or not, etc. This is hard to manage and error prone.
My solution is to have two state enums. One is for the BattleMode class and basically represents the general state of the battle. The other is for BattleActors and helps to determine things like if they are dead/paralyzed, where to draw their stamina icon, etc. All actor objects also have their own timer available for the states to use. This is helpful for each state to manage when it is to transition to the next state.
2) "Battle windows" and battle mode split
Right now we have a somewhat awkward file pair, battle_windows.* which contains two classes. One for the "action window" (where the player selects actions for their characters) and one for the "finish window" (which displays the victory/defeat menus). There's also a heap of members in the BattleMode class that more or less only need to be used by these two classes. I've given this some thought and here's a general list of things I'm going to do with this code, with the aim of: (1) reducing the clutter in the BattleMode class, (2) implementing cursor memory in the action window (remembering the selected targets/actions for each character individually), (3) separating the action/finish window code because they really nothing in common other than displaying menu windows.
- Create new files: battle_command.h, battle_command.cpp, battle_finish.h, battle_finish.cpp. Remove files: battle_windows.h, battle_windows.cpp
- battle_command.* will contain the code that is currently in the ActionWindow class. The code in these files handle anything and everything related to the player selecting commands for their characters.
- battle_finish.* will contain the code that is currently in the FinishWindow class. The code here will handle anything and everything required to do after the battle has finished with either a victory or defeat
- Move the victory/defeat music from BattleMode class to some class in battle_finish.* (and allow an option to set custom victory/defeat music)
- Move the target selection graphics to the battle_command code from BattleMode.
- Move all members of BattleMode related to current targets/attack points to battle_command code.
3) BattleTarget class
One small utility class that I think could come in handy is a class for representing battle targets. It would basically be a container class holding a target type (GLOBAL_TARGET_TYPE), pointer to a BattleActor, attack point index integer, and a pointer to a std:: container of BattleActor pointers. Only certain members of this class are valid depending upon the target type. This makes it rather simple to pass around target information where it is needed. I'll probably stick this class in battle_utils.*
4) Damage text
Right now the BattleMode has a single container of all damage text. I am changing this so that instead each actor has a container of damage text. This makes sense because actor's can automatically register damage (or healing) when their stats are modified. A new damage display manager class can also ensure that two damage text objects are not drawn on top of one another as they are currently. I think in the future this class can also be expanded to indicate additional damage or resistance due to elemental effects and changes in status effects.