refactor(battleplan): Split data off of BattlePlanBonuses class

[xezon]

This change splits the data off of the BattlePlanBonuses class so that instances can be created on the stack for fast manipulations. Reason being, the destructor of BattlePlanBonuses is protected, and therefore it always required an allocation to create an instance. The new BattlePlanBonusesData does not have this restriction.

TODO

• Replicate to Generals

[greptile-apps]

Greptile Summary

Successfully refactors BattlePlanBonuses by extracting data members into a new BattlePlanBonusesData struct, enabling stack allocation for fast bonus manipulations.

Key Changes:

• Introduced BattlePlanBonusesData struct with default constructor that initializes scalar bonuses to 1.0f and counters to 0
• Made BattlePlanBonuses inherit from both BattlePlanBonusesData and MemoryPoolObject, maintaining memory pool allocation for persistent instances
• Updated method signatures throughout to accept BattlePlanBonusesData* where appropriate
• Eliminated heap allocation in Player::removeBattlePlanBonusesForObject() by creating inverted bonuses on the stack
• Applied appropriate static_cast operations when converting between base and derived types

Impact:
This is a clean refactoring that improves performance by allowing temporary bonus calculations to avoid heap allocation while maintaining the memory pool pattern for persistent bonus storage.

Confidence Score: 5/5

• This PR is safe to merge with minimal risk
• Clean refactoring with proper inheritance hierarchy, all casts are correct, default constructor properly initializes all members, and the change eliminates unnecessary heap allocations for temporary objects
• No files require special attention

Important Files Changed

Filename	Overview
GeneralsMD/Code/GameEngine/Include/GameLogic/Module/BattlePlanUpdate.h	Introduces BattlePlanBonusesData struct with default constructor, BattlePlanBonuses now inherits from it
GeneralsMD/Code/GameEngine/Include/Common/Player.h	Updates method signatures to use BattlePlanBonusesData* instead of BattlePlanBonuses*
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/BattlePlanUpdate.cpp	Simplified initialization to rely on BattlePlanBonusesData default constructor
GeneralsMD/Code/GameEngine/Source/Common/RTS/Player.cpp	Replaces heap allocation in removeBattlePlanBonusesForObject with stack allocation, updates casts to BattlePlanBonusesData

Class Diagram

%%{init: {'theme': 'neutral'}}%%
classDiagram
    class BattlePlanBonusesData {
        +Real m_armorScalar
        +Int m_bombardment
        +Int m_searchAndDestroy
        +Int m_holdTheLine
        +Real m_sightRangeScalar
        +KindOfMaskType m_validKindOf
        +KindOfMaskType m_invalidKindOf
        +BattlePlanBonusesData()
    }
    
    class MemoryPoolObject {
        <<interface>>
    }
    
    class BattlePlanBonuses {
    }
    
    BattlePlanBonusesData <|-- BattlePlanBonuses
    MemoryPoolObject <|-- BattlePlanBonuses
    
    note for BattlePlanBonusesData "Stack-allocatable struct\nwith default constructor"
    note for BattlePlanBonuses "Heap-allocated via\nmemory pool"

Loading

_{Last reviewed commit: 5b132a5}

[bobtista]

Looks good. I see that we don't have to, but could initialize m_validKindOf / m_invalidKindOf in the constructor. There are a couple of static_casts that are probably only needed for VC6 if anything.
Should probably replicate to Generals too, no?

[xezon]

Looks good. I see that we don't have to, but could initialize m_validKindOf / m_invalidKindOf in the constructor. There are a couple of static_casts that are probably only needed for VC6 if anything. Should probably replicate to Generals too, no?

• KindOfMaskType are default initialized to 0, so no problem.

• static_cast's are required because inherited classes can have pointer offsets that need to be explicitly communicated on cast to void*.

• Yes will be replicated to Generals after Approve.