bugfix: Battle plans are now transferred to the new owner on capture

[Stubbjax]

Closes #2211

This change fixes an issue where battle plans are not transferred from the old owner to the new owner when a Strategy Center is captured. In addition, bonuses in effect upon surrender in team games are also now transferred to the new owner, if applicable.

In retail, capturing a Strategy Center leaves any existing bonus intact for the opponent, while giving the capturing player no bonus.

[greptile-apps]

Greptile Overview

Greptile Summary

This PR fixes battle plan transfer on Strategy Center capture and resolves a related bug in bonus removal logic.

Key Changes:

• Implemented BattlePlanUpdate::onCapture() to transfer active battle plans from the old owner to the new owner when a Strategy Center is captured
• Fixed a critical bug in Player::changeBattlePlan() where bonus removal was modifying the original bonus structure in-place instead of creating an inverted copy, which corrupted the stored bonus values
• Changes are wrapped in !RETAIL_COMPATIBLE_CRC guards to preserve original game behavior compatibility
• Applied identically to both Generals and GeneralsMD codebases

Impact:
The fix ensures that when a Strategy Center with an active battle plan (Bombardment, Hold the Line, or Search and Destroy) is captured, the bonuses are properly removed from the old owner's army and applied to the new owner's army, matching expected behavior for team game surrenders.

Confidence Score: 4/5

• Safe to merge with minor optimization opportunity
• The implementation correctly transfers battle plans on capture and fixes the bonus removal bug. The logic follows existing patterns and is guarded appropriately. Score reduced to 4 due to a minor inefficiency where changeBattlePlan is called even when no plan is active (PLANSTATUS_NONE), though this is harmless.
• No files require special attention - the implementation is straightforward and correct

Important Files Changed

Filename	Overview
Generals/Code/GameEngine/Source/Common/RTS/Player.cpp	Fixed bonus removal bug by creating separate invertedBonus copy instead of modifying original bonus in-place
GeneralsMD/Code/GameEngine/Source/Common/RTS/Player.cpp	Fixed bonus removal bug (identical fix to Generals version)
Generals/Code/GameEngine/Source/GameLogic/Object/Update/BattlePlanUpdate.cpp	Implemented onCapture to transfer battle plans from old to new owner; wrapped in !RETAIL_COMPATIBLE_CRC guard
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/BattlePlanUpdate.cpp	Implemented onCapture (identical to Generals version)

Sequence Diagram

sequenceDiagram
    participant Team
    participant Object as Strategy Center
    participant BattlePlanUpdate
    participant OldOwner as Old Player
    participant NewOwner as New Player
    
    Team->>Object: onCapture(oldOwner, newOwner)
    Note over Object: Object ownership already changed
    Object->>Object: Update AI state, score, etc.
    
    loop For each behavior module
        Object->>BattlePlanUpdate: onCapture(oldOwner, newOwner)
        alt m_planAffectingArmy != PLANSTATUS_NONE
            BattlePlanUpdate->>OldOwner: changeBattlePlan(plan, -1, bonuses)
            Note over OldOwner: Remove plan from old owner<br/>Decrement counter<br/>If counter reaches 0, remove bonuses
            OldOwner->>OldOwner: applyBattlePlanBonusesForPlayerObjects(invertedBonus)
            Note over OldOwner: Apply inverted bonuses to undo effects
            
            BattlePlanUpdate->>NewOwner: changeBattlePlan(plan, 1, bonuses)
            Note over NewOwner: Add plan to new owner<br/>Increment counter<br/>If counter reaches 1, add bonuses
            NewOwner->>NewOwner: applyBattlePlanBonusesForPlayerObjects(bonus)
            Note over NewOwner: Apply bonuses to all qualifying units
        end
    end
    
    Object->>Object: handlePartitionCellMaintenance()

Loading

[greptile-apps]

_{4 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

_{4 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

calling changeBattlePlan with PLANSTATUS_NONE will do nothing (no bonuses applied/removed), so capturing a Strategy Center with no active plan is safe but will call changeBattlePlan unnecessarily

Suggested change

	//-------------------------------------------------------------------------------------------------
	void BattlePlanUpdate::onCapture(Player* oldOwner, Player* newOwner)
	{
	#if !RETAIL_COMPATIBLE_CRC
	// TheSuperHackers @bugfix Stubbjax 04/11/2025 Transfer battle plan bonuses on capture.
	oldOwner->changeBattlePlan(m_planAffectingArmy, -1, m_bonuses);
	newOwner->changeBattlePlan(m_planAffectingArmy, 1, m_bonuses);
	#endif
	void BattlePlanUpdate::onCapture(Player* oldOwner, Player* newOwner)
	{
	#if !RETAIL_COMPATIBLE_CRC
	// TheSuperHackers @bugfix Stubbjax 04/11/2025 Transfer battle plan bonuses on capture.
	if (m_planAffectingArmy != PLANSTATUS_NONE)
	{
	oldOwner->changeBattlePlan(m_planAffectingArmy, -1, m_bonuses);
	newOwner->changeBattlePlan(m_planAffectingArmy, 1, m_bonuses);
	}
	#endif
	}

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

Prompt To Fix With AI

This is a comment left during a code review.
Path: Generals/Code/GameEngine/Source/GameLogic/Object/Update/BattlePlanUpdate.cpp
Line: 256:263

Comment:
calling `changeBattlePlan` with `PLANSTATUS_NONE` will do nothing (no bonuses applied/removed), so capturing a Strategy Center with no active plan is safe but will call `changeBattlePlan` unnecessarily

```suggestion
void BattlePlanUpdate::onCapture(Player* oldOwner, Player* newOwner)
{
#if !RETAIL_COMPATIBLE_CRC
	// TheSuperHackers @bugfix Stubbjax 04/11/2025 Transfer battle plan bonuses on capture.
	if (m_planAffectingArmy != PLANSTATUS_NONE)
	{
		oldOwner->changeBattlePlan(m_planAffectingArmy, -1, m_bonuses);
		newOwner->changeBattlePlan(m_planAffectingArmy, 1, m_bonuses);
	}
#endif
}
```

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

calling changeBattlePlan with PLANSTATUS_NONE will do nothing (no bonuses applied/removed), so capturing a Strategy Center with no active plan is safe but will call changeBattlePlan unnecessarily

Suggested change

	//-------------------------------------------------------------------------------------------------
	void BattlePlanUpdate::onCapture(Player* oldOwner, Player* newOwner)
	{
	#if !RETAIL_COMPATIBLE_CRC
	// TheSuperHackers @bugfix Stubbjax 04/11/2025 Transfer battle plan bonuses on capture.
	oldOwner->changeBattlePlan(m_planAffectingArmy, -1, m_bonuses);
	newOwner->changeBattlePlan(m_planAffectingArmy, 1, m_bonuses);
	#endif
	void BattlePlanUpdate::onCapture(Player* oldOwner, Player* newOwner)
	{
	#if !RETAIL_COMPATIBLE_CRC
	// TheSuperHackers @bugfix Stubbjax 04/11/2025 Transfer battle plan bonuses on capture.
	if (m_planAffectingArmy != PLANSTATUS_NONE)
	{
	oldOwner->changeBattlePlan(m_planAffectingArmy, -1, m_bonuses);
	newOwner->changeBattlePlan(m_planAffectingArmy, 1, m_bonuses);
	}
	#endif
	}

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

Prompt To Fix With AI

This is a comment left during a code review.
Path: GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/BattlePlanUpdate.cpp
Line: 256:263

Comment:
calling `changeBattlePlan` with `PLANSTATUS_NONE` will do nothing (no bonuses applied/removed), so capturing a Strategy Center with no active plan is safe but will call `changeBattlePlan` unnecessarily

```suggestion
void BattlePlanUpdate::onCapture(Player* oldOwner, Player* newOwner)
{
#if !RETAIL_COMPATIBLE_CRC
	// TheSuperHackers @bugfix Stubbjax 04/11/2025 Transfer battle plan bonuses on capture.
	if (m_planAffectingArmy != PLANSTATUS_NONE)
	{
		oldOwner->changeBattlePlan(m_planAffectingArmy, -1, m_bonuses);
		newOwner->changeBattlePlan(m_planAffectingArmy, 1, m_bonuses);
	}
#endif
}
```

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.