Fix null pointer dereferences and code duplication in FakeTeamClass

[Copilot]

Fixed critical null pointer dereferences in _Coordinate_Attack and _TMission_GatherAtEnemy, along with an incorrect error message and several code quality issues in src/Ext/Team/Body.cpp.

Critical Fixes

• Null pointer dereference in _Coordinate_Attack: Added null checks before dereferencing teamLeader returned from _Fetch_A_Leader()
• Missing null check in _TMission_GatherAtEnemy: Added ternary operator for null leaderType (matching pattern in _TMission_GatherAtBase)
• Wrong error message in _TMission_Play_Animation: Changed BuildingTypeClass::Array->Count to AnimTypeClass::Array->Count

Code Quality

• Extracted comparator helper: Created EvaluateComparator() to eliminate 115+ lines of duplicated switch statements across 6 functions
• Named constants: Replaced magic numbers with RECRUIT_WRONG_GROUP_PENALTY, PATROL_THRESHOLD, FACING_DIRECTION_MULTIPLIER
• Removed unused variable: Deleted unused pCur in LoopThruMembers
• Simplified redundant logic: Cleaned up (!pEnemy || (pEnemy && !pHouse->IsAlliedWith(pEnemy))) → (!pEnemy || !pHouse->IsAlliedWith(pEnemy))
• Standardized aircraft multiplier: Changed stray distance from 2x to 3x in _CoordinateMove to match _Lagging_Units

Before:

switch (pThis->Conditions[0].ComparatorOperand) {
case 0: result = counter < pThis->Conditions[0].ComparatorType; break;
case 1: result = counter <= pThis->Conditions[0].ComparatorType; break;
// ... 4 more cases, repeated in 6 functions
}

After:

return EvaluateComparator(counter, pThis->Conditions[0].ComparatorType, pThis->Conditions[0].ComparatorOperand);

Original prompt

Summary

Analysis of FakeTeamClass in src/Ext/Team/Body.cpp and src/Ext/Team/Body.h revealed several bugs and code quality issues that need to be addressed.

Bugs to Fix

1. Null Pointer Dereference in _Coordinate_Attack (Critical)

Location: src/Ext/Team/Body.cpp around line 1140-1147
Problem: teamLeader can be null when returned from _Fetch_A_Leader(), but it's dereferenced without a null check.

FootClass* teamLeader = this->_Fetch_A_Leader();
// ...
if (targetCell
    && this->FirstUnit
    && teamLeader->WhatAmI() != AircraftClass::AbsID)  // BUG: teamLeader can be null!

Fix: Add null check for teamLeader before dereferencing.

2. Missing Null Check in _TMission_GatherAtEnemy (Critical)

Location: src/Ext/Team/Body.cpp around line 550-552
Problem: leaderType is used without null check, unlike the similar code in _TMission_GatherAtBase.

TechnoTypeClass* leaderType = bestLeader->GetTechnoType();
CellStruct finalCell = MapClass::Instance->NearByLocation(
    targetCell,
    leaderType->SpeedType,  // No null check like in _TMission_GatherAtBase

Fix: Add conditional null check like in _TMission_GatherAtBase: leaderType ? leaderType->SpeedType : SpeedType::Track

3. Wrong Error Message in _TMission_Play_Animation (Minor)

Location: src/Ext/Team/Body.cpp around line 3437
Problem: Error message references BuildingTypeClass::Array->Count instead of AnimTypeClass::Array->Count.

Debug::FatalErrorAndExit("Team [%s] TMission_Play_Animation: Invalid anim type index %d (max: %d)",
    this->Type->ID, animIndex, BuildingTypeClass::Array->Count - 1);  // Wrong array!

Fix: Change to AnimTypeClass::Array->Count - 1

4. Unused Variable in LoopThruMembers (Minor)

Location: src/Ext/Team/Body.cpp line 22
Problem: FootClass* pCur = nullptr; is declared but never used.
Fix: Remove the unused variable.

5. Redundant Condition in EnemyOwns (Minor)

Location: src/Ext/Team/Body.cpp around line 132-136
Problem: Redundant null check after OR condition.

if (pObject->Owner != pHouse
    && (!pEnemy || (pEnemy && !pHouse->IsAlliedWith(pEnemy)))  // Redundant check

Fix: Simplify to (!pEnemy || !pHouse->IsAlliedWith(pEnemy))

6. Inconsistent Aircraft Stray Distance Multiplier (Code Quality)

Location: src/Ext/Team/Body.cpp
Problem: _CoordinateMove uses strayDistance *= 2 for aircraft (line ~1349), but _Lagging_Units uses stray *= 3 (line ~2048). This inconsistency may cause unexpected behavior.
Fix: Standardize to use the same multiplier (recommend 3x to match _Lagging_Units).

Code Quality Improvements

7. Extract Comparator Logic to Helper Function

Problem: The switch statement for comparator operations (cases 0-5) is duplicated 6+ times across HouseOwns, EnemyOwns, NeutralOwns, HouseOwnsAll, EnemyOwnsAll, NeutralOwnsAll.
Fix: Extract to a helper function:

static bool EvaluateComparator(int counter, int comparatorOperand, int comparatorType) {
    switch (comparatorOperand) {
    case 0: return counter < comparatorType;
    case 1: return counter <= comparatorType;
    case 2: return counter == comparatorType;
    case 3: return counter >= comparatorType;
    case 4: return counter > comparatorType;
    case 5: return counter != comparatorType;
    default: return false;
    }
}

8. Replace Magic Numbers with Named Constants

Problem: Hard-coded values like 12800 (distance penalty in recruitment), 702 (patrol threshold), 8192 (facing conversion) are unclear.
Fix: Add named constants:

constexpr int RECRUIT_WRONG_GROUP_PENALTY = 12800;
constexpr int PATROL_THRESHOLD = 702;
constexpr int FACING_DIRECTION_MULTIPLIER = 8192;  // 65536 / 8 directions

Files to Modify

• src/Ext/Team/Body.cpp
• src/Ext/Team/Body.h (if adding helper functions as static members)

This pull request was created as a result of the following prompt from Copilot chat.

Summary

Analysis of FakeTeamClass in src/Ext/Team/Body.cpp and src/Ext/Team/Body.h revealed several bugs and code quality issues that need to be addressed.

Bugs to Fix

1. Null Pointer Dereference in _Coordinate_Attack (Critical)

Location: src/Ext/Team/Body.cpp around line 1140-1147
Problem: teamLeader can be null when returned from _Fetch_A_Leader(), but it's dereferenced without a null check.

FootClass* teamLeader = this->_Fetch_A_Leader();
// ...
if (targetCell
    && this->FirstUnit
    && teamLeader->WhatAmI() != AircraftClass::AbsID)  // BUG: teamLeader can be null!

Fix: Add null check for teamLeader before dereferencing.

2. Missing Null Check in _TMission_GatherAtEnemy (Critical)

Location: src/Ext/Team/Body.cpp around line 550-552
Problem: leaderType is used without null check, unlike the similar code in _TMission_GatherAtBase.

TechnoTypeClass* leaderType = bestLeader->GetTechnoType();
CellStruct finalCell = MapClass::Instance->NearByLocation(
    targetCell,
    leaderType->SpeedType,  // No null check like in _TMission_GatherAtBase

Fix: Add conditional null check like in _TMission_GatherAtBase: leaderType ? leaderType->SpeedType : SpeedType::Track

3. Wrong Error Message in _TMission_Play_Animation (Minor)

Location: src/Ext/Team/Body.cpp around line 3437
Problem: Error message references BuildingTypeClass::Array->Count instead of AnimTypeClass::Array->Count.

Debug::FatalErrorAndExit("Team [%s] TMission_Play_Animation: Invalid anim type index %d (max: %d)",
    this->Type->ID, animIndex, BuildingTypeClass::Array->Count - 1);  // Wrong array!

Fix: Change to AnimTypeClass::Array->Count - 1

4. Unused Variable in LoopThruMembers (Minor)

Location: src/Ext/Team/Body.cpp line 22
Problem: FootClass* pCur = nullptr; is declared but never used.
Fix: Remove the unused variable.

5. Redundant Condition in EnemyOwns (Minor)

Location: src/Ext/Team/Body.cpp around line 132-136
Problem: Redundant null check after OR condition.

if (pObject->Owner != pHouse
    && (!pEnemy || (pEnemy && !pHouse->IsAlliedWith(pEnemy)))  // Redundant check

Fix: Simplify to (!pEnemy || !pHouse->IsAlliedWith(pEnemy))

6. Inconsistent Aircraft Stray Distance Multiplier (Code Quality)

Location: src/Ext/Team/Body.cpp
Problem: _CoordinateMove uses strayDistance *= 2 for aircraft (line ~1349), but _Lagging_Units uses stray *= 3 (line ~2048). This inconsistency may cause unexpected behavior.
Fix: Standardize to use the same multiplier (recommend 3x to match _Lagging_Units).

Code Quality Improvements

7. Extract Comparator Logic to Helper Function

Problem: The switch statement for comparator operations (cases 0-5) is duplicated 6+ times across HouseOwns, EnemyOwns, NeutralOwns, HouseOwnsAll, EnemyOwnsAll, NeutralOwnsAll.
Fix: Extract to a helper function:

static bool EvaluateComparator(int counter, int comparatorOperand, int comparatorType) {
    switch (comparatorOperand) {
    case 0: return counter < comparatorType;
    case 1: return counter <= comparatorType;
    case 2: return counter == comparatorType;
    case 3: return counter >= comparatorType;
    case 4: return counter > comparatorType;
    case 5: return counter != comparatorType;
    default: return false;
    }
}

8. Replace Magic Numbers with Named Constants

Problem: Hard-coded values like 12800 (distance penalty in recruitment), 702 (patrol threshold), 8192 (facing conversion) are unclear.
Fix: Add named constants:

constexpr int RECRUIT_WRONG_GROUP_PENALTY = 12800;
constexpr int PATROL_THRESHOLD = 702;
constexpr int FACING_DIRECTION_MULTIPLIER = 8192;  // 65536 / 8 directions

Files to Modify

• src/Ext/Team/Body.cpp
• src/Ext/Team/Body.h (if adding helper functions as static members)

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.