bugfix: Make healed units follow building rally point on exit

[tintinhamans]

Units will now follow building rally point after healing.

Closes #221

[greptile-apps]

Greptile Summary

This PR fixes a long-standing bug (closes #221) where healed units — ground troops exiting a HealContain building, Chinooks, and jet/helicopter aircraft — would not follow the building's rally point after healing completes. The fix is applied symmetrically across both Generals/ and GeneralsMD/ codebases.

Approach:

• In OpenContain::getRallyPoint() (both codebases), a #if !RETAIL_COMPATIBLE_CRC-guarded fallback is added: if the contain module itself has no rally point set, it delegates to the object's primary exit interface's rally point. A self-pointer guard (primaryExit != this) correctly prevents direct recursion. This allows HealContain — which derives from OpenContain — to transparently propagate the parent building's rally point when units exit via exitObjectViaDoor.
• exitObjectViaDoor is updated from a direct m_rallyPointExists check to using getRallyPoint(), wiring in the delegation chain with no change to retail-mode behavior.
• In ChinookAIUpdate and JetAIUpdate (both codebases), a #if !RETAIL_COMPATIBLE_CRC-guarded block queries the airfield's exit interface for a rally point immediately after initiating takeoff. aiMoveToPosition is called while the state machine has already entered the taking-off state (setting TAKEOFF_IN_PROGRESS/m_hasPendingCommand guards), so aiDoCommand correctly defers the move as a pending command to be dispatched once the aircraft reaches altitude — consistent with the established pattern used throughout both AIUpdate files.

Confidence Score: 4/5

• Safe to merge — all new paths are guarded by #if !RETAIL_COMPATIBLE_CRC, null-checked, and follow established patterns in the codebase.
• The implementation is well-structured and consistent with existing pending-command patterns in both AI update files. The null-safety guards are thorough. The one area of inherent complexity is the same-frame interaction in JetAIUpdate::update() where else if (HAS_PENDING_COMMAND) can fire immediately after aiMoveToPosition sets the flag — but this is harmless because TAKEOFF_IN_PROGRESS (set synchronously by the state machine's onEnter handler) causes aiDoCommand to simply re-queue the command as pending rather than executing it prematurely. The retail build is entirely unaffected by the non-retail guard. Score of 4 rather than 5 reflects the moderate state-machine interaction complexity that warrants in-game testing for both Chinooks and helipads with and without a rally point set.
• No files require special attention, though in-game testing of Chinook and helipad aircraft post-heal rally behavior is recommended.

Important Files Changed

Filename	Overview
Generals/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp	Refactors exitObjectViaDoor to use getRallyPoint() instead of checking m_rallyPointExists directly; adds a #if !RETAIL_COMPATIBLE_CRC-guarded fallback in getRallyPoint() to delegate to the primary exit interface's rally point, enabling HealContain to propagate the building's rally point. Self-recursion guard (primaryExit != this) is correct.
Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/ChinookAIUpdate.cpp	Adds #if !RETAIL_COMPATIBLE_CRC-guarded rally point move after healing is complete and takeoff begins. The pattern follows established aiDoCommand pending-command mechanics: calling aiMoveToPosition while m_flightStatus != CHINOOK_FLYING causes aiDoCommand to store the command and set m_hasPendingCommand = true, dispatched when the Chinook becomes idle post-takeoff. Note: aiDoCommand also clears m_airfieldForHealing (to INVALID_ID) via the non-evacuate path, but pp->setHealee(false) was already called so this is benign.
Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/JetAIUpdate.cpp	Adds #if !RETAIL_COMPATIBLE_CRC-guarded rally point move after helipad-produced aircraft finish healing. Since friend_setAllowAirLoco(true) and setState(TAKING_OFF_AWAIT_CLEARANCE) are called first (setting TAKEOFF_IN_PROGRESS = true via onEnter), the subsequent aiMoveToPosition correctly routes through the TAKEOFF_IN_PROGRESS guard in aiDoCommand, storing the command as pending for dispatch after takeoff completes. The same-frame else if (HAS_PENDING_COMMAND) re-dispatch re-queues the command safely.
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp	Mirror of the Generals/ counterpart — identical changes applied correctly to the Zero Hour codebase.
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/ChinookAIUpdate.cpp	Mirror of the Generals/ counterpart — identical healing rally point logic applied to the Zero Hour Chinook. Same behavioral analysis applies.
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/JetAIUpdate.cpp	Mirror of the Generals/ counterpart — identical healing rally point logic applied to the Zero Hour jet/helicopter. Same behavioral analysis applies.

Sequence Diagram

sequenceDiagram
    participant Update as ChinookAIUpdate / JetAIUpdate
    participant SM as StateMachine
    participant PP as ParkingPlace
    participant AI as AIInterface
    participant Exit as ExitInterface (Airfield)

    Update->>PP: setHealee(unit, false)
    Update->>SM: setState(TAKING_OFF / TAKING_OFF_AWAIT_CLEARANCE)
    Note over SM: onEnter() sets TAKEOFF_IN_PROGRESS=true
    Update->>Exit: getRallyPoint()
    Exit-->>Update: Coord3D* rallyPoint (if set)
    Update->>AI: aiMoveToPosition(rallyPoint)
    AI->>AI: aiDoCommand(AICMD_MOVE_TO_POSITION)
    Note over AI: TAKEOFF_IN_PROGRESS=true →<br/>store as m_mostRecentCommand,<br/>set HAS_PENDING_COMMAND=true, return
    Note over SM: Takeoff animation completes,<br/>TAKEOFF_IN_PROGRESS=false
    SM-->>Update: unit is now idle (flying)
    Update->>AI: dispatch m_mostRecentCommand (move to rally)
    AI->>AI: aiDoCommand(AICMD_MOVE_TO_POSITION)
    Note over AI: ALLOW_AIR_LOCO=true, not landing/takeoff →<br/>AIUpdateInterface::aiDoCommand executes move

Loading

_{Last reviewed commit: 7c656b2}

[greptile-apps]

_{6 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

_{4 files reviewed, 4 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Additional Comments (2)

GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/ChinookAIUpdate.cpp
airfield could be nullptr if the airfield was destroyed or doesn't exist. Need null check before calling getObjectExitInterface().

				Object *airfield = TheGameLogic->findObjectByID( m_airfieldForHealing );
				if (airfield)
				{
					const Coord3D *rp = airfield->getObjectExitInterface()->getRallyPoint();
					if( rp )
					{
						AICommandParms parms( AICMD_MOVE_TO_POSITION, CMD_FROM_AI );
						parms.m_pos = *rp;
						m_pendingCommand.store( parms );
						m_hasPendingCommand = true;
					}
				}

Prompt To Fix With AI

This is a comment left during a code review.
Path: GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/ChinookAIUpdate.cpp
Line: 1038:1039

Comment:
`airfield` could be `nullptr` if the airfield was destroyed or doesn't exist. Need null check before calling `getObjectExitInterface()`.

```suggestion
				Object *airfield = TheGameLogic->findObjectByID( m_airfieldForHealing );
				if (airfield)
				{
					const Coord3D *rp = airfield->getObjectExitInterface()->getRallyPoint();
					if( rp )
					{
						AICommandParms parms( AICMD_MOVE_TO_POSITION, CMD_FROM_AI );
						parms.m_pos = *rp;
						m_pendingCommand.store( parms );
						m_hasPendingCommand = true;
					}
				}
```

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

---

GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/JetAIUpdate.cpp
airfield could be nullptr if the airfield was destroyed or doesn't exist (see line 1717-1718 in same file for precedent). Need null check before calling getObjectExitInterface().

					Object *airfield = TheGameLogic->findObjectByID( jet->getProducerID() );
					if (airfield)
					{
						const Coord3D *rp = airfield->getObjectExitInterface()->getRallyPoint();
						if( rp )
						{
							AICommandParms parms( AICMD_MOVE_TO_POSITION, CMD_FROM_AI );
							parms.m_pos = *rp;
							m_mostRecentCommand.store( parms );
							setFlag(HAS_PENDING_COMMAND, true);
						}
					}

Prompt To Fix With AI

This is a comment left during a code review.
Path: GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/JetAIUpdate.cpp
Line: 1782:1783

Comment:
`airfield` could be `nullptr` if the airfield was destroyed or doesn't exist (see line 1717-1718 in same file for precedent). Need null check before calling `getObjectExitInterface()`.

```suggestion
					Object *airfield = TheGameLogic->findObjectByID( jet->getProducerID() );
					if (airfield)
					{
						const Coord3D *rp = airfield->getObjectExitInterface()->getRallyPoint();
						if( rp )
						{
							AICommandParms parms( AICMD_MOVE_TO_POSITION, CMD_FROM_AI );
							parms.m_pos = *rp;
							m_mostRecentCommand.store( parms );
							setFlag(HAS_PENDING_COMMAND, true);
						}
					}
```

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

[greptile-apps]

_{6 files reviewed, 3 comments}

_{Edit Code Review Agent Settings | Greptile}

[xezon]

This looks strange. Basically the Rally Point of the HealContain is now set/updated after every heal. Which begs the question, why is the Rally Point not updated for the Heal Contain when the Rally Point of the Object Exit Interface is updated? That would be the most logical and efficient way (and maybe that is what was supposed to happen but is broken somewhere).

[tintinhamans]

I changed approach on this and made fallback on OpenContain instead.

[xezon]

Is this the correct way to do the movement for this purpose?

For reference

OpenContain::exitObjectViaDoor uses ai->aiFollowPath( &exitPath, me, CMD_FROM_AI ) to go to the Rally Point.

DefaultProductionExitUpdate::exitObjectViaDoor uses ai->aiFollowExitProductionPath( &exitPath, creationObject, CMD_FROM_AI ) to go to the Rally Point.

Helicopter looks special, so perhaps that is fine. Unclear.

[tintinhamans]

I think this is correct. The Chinook/helicopter is landed when healing and needs to take off first - it cannot follow a path directly. This matches how ParkingPlaceBehavior::exitObjectViaDoor handles helipad-produced aircraft:

GeneralsGameCode/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Behavior/ParkingPlaceBehavior.cpp

Lines 857 to 865 in 83a51f8

	if( producedAtHelipad )
	{
	const Coord3D *rallyPoint = getRallyPoint();
	if( rallyPoint )
	{
	ai->aiMoveToPosition( rallyPoint, CMD_FROM_AI );
	movedToRallyPoint = TRUE;
	}
	}

I did see from above that I can use aiMoveToPosition instead of storing pending command manually since aiDoCommand already handles takeoff in-progress and queues move for us.

[Skyaero42]

This looks good to me

[xezon]

Can you explain the difference between the waypointing in OpenContain and in ChinookAIUpdate? Which one is used in what event?

[xezon]

!= static_cast<const ExitInterface *>(this)

[xezon]

Comment is redundant.

[tintinhamans]

Can you explain the difference between the waypointing in OpenContain and in ChinookAIUpdate? Which one is used in what event?

OpenContain::exitObjectViaDoor handles infrantry exiting barracks. HealContain inherits from OpenContain and doesn't override this - the rally point is appended to the exit path and units walk to it via aiFollowPath.

ChinookAIUpdate/JetAIUpdate handle helicopters healed at airfields. Helicopters can't follow a ground path while landed, so after takeoff they issue a separate aiMoveToPosition to the rally point. This matches how ParkingPlaceBehavior::exitObjectViaDoor handles newly produced aircraft.

War factory repair is unrelated - it uses the dock system (RepairDockUpdate), which has its own AI_DOCK_MOVE_TO_RALLY state that already queries getObjectExitInterface()->getRallyPoint() in AIDockMoveToRallyState::onEnter. Rally points already worked there in retail. The bug was only in the HealContain and helicopter healing paths.