Skip to content

Fix null pointer dereferences and code duplication in FakeTeamClass #7

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
Copilot wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Fix null pointer dereferences and code duplication in FakeTeamClass #7

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2aaaa67
Initial plan
Copilot Dec 9, 2025
6e72c08
Fix critical and minor bugs in FakeTeamClass
Copilot Dec 9, 2025
1a09ee3
Extract comparator logic to helper function and add named constants
Copilot Dec 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
209 changes: 47 additions & 162 deletions src/Ext/Team/Body.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,17 @@
#include <TaskForceClass.h>
#include <TubeClass.h>

// Named constants for clarity
constexpr int RECRUIT_WRONG_GROUP_PENALTY = 12800; // Distance penalty for recruiting units from wrong group
constexpr int PATROL_THRESHOLD = 702; // Mission data threshold for patrol behavior
constexpr int FACING_DIRECTION_MULTIPLIER = 8192; // Convert 8-direction facing to 16-bit facing (65536 / 8 = 8192)

template<typename Func, typename... Args>
concept ReturnsBool = std::same_as<std::invoke_result_t<Func, Args...>, bool>;

template<typename Func>
void LoopThruMembers(TeamClass* pTeam, Func&& act)
{
FootClass* pCur = nullptr;
if (auto pFirst = pTeam->FirstUnit)
{
do
Expand All @@ -40,6 +44,28 @@ void LoopThruMembers(TeamClass* pTeam, Func&& act)
}
}

// Helper function for comparator operations
static bool EvaluateComparator(int counter, int comparatorType, int comparatorOperand)
{
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;
}
}

#pragma region ExtFuncs

bool TeamExtData::IsEligible(TechnoClass* pGoing, TechnoTypeClass* reinfocement)
Expand All @@ -66,7 +92,6 @@ void TeamExtData::InvalidatePointer(AbstractClass* ptr, bool bRemoved)

bool TeamExtData::HouseOwns(AITriggerTypeClass* pThis, HouseClass* pHouse, bool allies, const Iterator<TechnoTypeClass*>& list)
{
bool result = false;
int counter = 0;

// Count all objects of the list, like an OR operator
Expand All @@ -86,36 +111,11 @@ bool TeamExtData::HouseOwns(AITriggerTypeClass* pThis, HouseClass* pHouse, bool
}
}

switch (pThis->Conditions[0].ComparatorOperand)
{
case 0:
result = counter < pThis->Conditions[0].ComparatorType;
break;
case 1:
result = counter <= pThis->Conditions[0].ComparatorType;
break;
case 2:
result = counter == pThis->Conditions[0].ComparatorType;
break;
case 3:
result = counter >= pThis->Conditions[0].ComparatorType;
break;
case 4:
result = counter > pThis->Conditions[0].ComparatorType;
break;
case 5:
result = counter != pThis->Conditions[0].ComparatorType;
break;
default:
break;
}

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

bool TeamExtData::EnemyOwns(AITriggerTypeClass* pThis, HouseClass* pHouse, HouseClass* pEnemy, bool onlySelectedEnemy, const Iterator<TechnoTypeClass*>& list)
{
bool result = false;
int counter = 0;

if (pEnemy && pHouse->IsAlliedWith(pEnemy) && !onlySelectedEnemy)
Expand All @@ -130,7 +130,7 @@ bool TeamExtData::EnemyOwns(AITriggerTypeClass* pThis, HouseClass* pHouse, House
continue;

if (pObject->Owner != pHouse
&& (!pEnemy || (pEnemy && !pHouse->IsAlliedWith(pEnemy)))
&& (!pEnemy || !pHouse->IsAlliedWith(pEnemy))
&& !pObject->Owner->Type->MultiplayPassive
&& pObject->GetTechnoType() == pItem)
{
Expand All @@ -139,36 +139,11 @@ bool TeamExtData::EnemyOwns(AITriggerTypeClass* pThis, HouseClass* pHouse, House
}
}

switch (pThis->Conditions[0].ComparatorOperand)
{
case 0:
result = counter < pThis->Conditions[0].ComparatorType;
break;
case 1:
result = counter <= pThis->Conditions[0].ComparatorType;
break;
case 2:
result = counter == pThis->Conditions[0].ComparatorType;
break;
case 3:
result = counter >= pThis->Conditions[0].ComparatorType;
break;
case 4:
result = counter > pThis->Conditions[0].ComparatorType;
break;
case 5:
result = counter != pThis->Conditions[0].ComparatorType;
break;
default:
break;
}

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

bool TeamExtData::NeutralOwns(AITriggerTypeClass* pThis, const Iterator<TechnoTypeClass*>& list)
{
bool result = false;
int counter = 0;

for (auto pHouse : *HouseClass::Array)
Expand All @@ -193,31 +168,7 @@ bool TeamExtData::NeutralOwns(AITriggerTypeClass* pThis, const Iterator<TechnoTy
}
}

switch (pThis->Conditions[0].ComparatorOperand)
{
case 0:
result = counter < pThis->Conditions[0].ComparatorType;
break;
case 1:
result = counter <= pThis->Conditions[0].ComparatorType;
break;
case 2:
result = counter == pThis->Conditions[0].ComparatorType;
break;
case 3:
result = counter >= pThis->Conditions[0].ComparatorType;
break;
case 4:
result = counter > pThis->Conditions[0].ComparatorType;
break;
case 5:
result = counter != pThis->Conditions[0].ComparatorType;
break;
default:
break;
}

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

bool TeamExtData::HouseOwnsAll(AITriggerTypeClass* pThis, HouseClass* pHouse, const Iterator<TechnoTypeClass*>& list)
Expand Down Expand Up @@ -248,29 +199,7 @@ bool TeamExtData::HouseOwnsAll(AITriggerTypeClass* pThis, HouseClass* pHouse, co
}
}

switch (pThis->Conditions[0].ComparatorOperand)
{
case 0:
result = counter < pThis->Conditions[0].ComparatorType;
break;
case 1:
result = counter <= pThis->Conditions[0].ComparatorType;
break;
case 2:
result = counter == pThis->Conditions[0].ComparatorType;
break;
case 3:
result = counter >= pThis->Conditions[0].ComparatorType;
break;
case 4:
result = counter > pThis->Conditions[0].ComparatorType;
break;
case 5:
result = counter != pThis->Conditions[0].ComparatorType;
break;
default:
break;
}
result = EvaluateComparator(counter, pThis->Conditions[0].ComparatorType, pThis->Conditions[0].ComparatorOperand);
}

return result;
Expand Down Expand Up @@ -301,37 +230,15 @@ bool TeamExtData::EnemyOwnsAll(AITriggerTypeClass* pThis, HouseClass* pHouse, Ho
continue;

if (pObject->Owner != pHouse
&& (!pEnemy || (pEnemy && !pHouse->IsAlliedWith(pEnemy)))
&& (!pEnemy || !pHouse->IsAlliedWith(pEnemy))
&& !pObject->Owner->Type->MultiplayPassive
&& pObject->GetTechnoType() == pItem)
{
counter++;
}
}

switch (pThis->Conditions[0].ComparatorOperand)
{
case 0:
result = counter < pThis->Conditions[0].ComparatorType;
break;
case 1:
result = counter <= pThis->Conditions[0].ComparatorType;
break;
case 2:
result = counter == pThis->Conditions[0].ComparatorType;
break;
case 3:
result = counter >= pThis->Conditions[0].ComparatorType;
break;
case 4:
result = counter > pThis->Conditions[0].ComparatorType;
break;
case 5:
result = counter != pThis->Conditions[0].ComparatorType;
break;
default:
break;
}
result = EvaluateComparator(counter, pThis->Conditions[0].ComparatorType, pThis->Conditions[0].ComparatorOperand);
}

return result;
Expand Down Expand Up @@ -375,29 +282,7 @@ bool TeamExtData::NeutralOwnsAll(AITriggerTypeClass* pThis, const Iterator<Techn
}
}

switch (pThis->Conditions[0].ComparatorOperand)
{
case 0:
foundAll = counter < pThis->Conditions[0].ComparatorType;
break;
case 1:
foundAll = counter <= pThis->Conditions[0].ComparatorType;
break;
case 2:
foundAll = counter == pThis->Conditions[0].ComparatorType;
break;
case 3:
foundAll = counter >= pThis->Conditions[0].ComparatorType;
break;
case 4:
foundAll = counter > pThis->Conditions[0].ComparatorType;
break;
case 5:
foundAll = counter != pThis->Conditions[0].ComparatorType;
break;
default:
break;
}
foundAll = EvaluateComparator(counter, pThis->Conditions[0].ComparatorType, pThis->Conditions[0].ComparatorOperand);
}

if (!foundAll)
Expand Down Expand Up @@ -550,7 +435,7 @@ void FakeTeamClass::_TMission_GatherAtEnemy(ScriptActionNode* nNode, bool arg3)
TechnoTypeClass* leaderType = bestLeader->GetTechnoType();
CellStruct finalCell = MapClass::Instance->NearByLocation(
targetCell,
leaderType->SpeedType,
leaderType ? leaderType->SpeedType : SpeedType::Track,
ZoneType::None, MovementZone::Normal,
0, 3, 3, 0, 0, 0, 1, CellStruct::Empty, 0, 0
);
Expand Down Expand Up @@ -1144,6 +1029,7 @@ void FakeTeamClass::_Coordinate_Attack() {

if (targetCell
&& this->FirstUnit
&& teamLeader
&& teamLeader->WhatAmI() != AircraftClass::AbsID)
{

Expand All @@ -1153,7 +1039,7 @@ void FakeTeamClass::_Coordinate_Attack() {
}

// Check if team leader can fire at target (every 8 frames, on frame 4)
if (Unsorted::CurrentFrame % 8 == 4) {
if (teamLeader && Unsorted::CurrentFrame % 8 == 4) {
const int weaponIndex = teamLeader->SelectWeapon(this->ArchiveTarget);

if (teamLeader->GetFireError(this->ArchiveTarget, weaponIndex, true) == FireError::ILLEGAL) {
Expand Down Expand Up @@ -1345,9 +1231,9 @@ void FakeTeamClass::_CoordinateMove() {
{
int strayDistance = this->_Get_Stray();

// Double for airborne units
// Triple for airborne units
if (unit->IsInAir()){
strayDistance *= 2;
strayDistance *= 3;
}

found = true;
Expand Down Expand Up @@ -1800,7 +1686,7 @@ FootClass* FindClosestInfantry(TeamClass* team, int memberIndex, const CoordStru
// Calculate distance with penalty for wrong group
int distance = infantry->DistanceFromSquared(&location);
if (infantry->Group != targetGroup)
distance += 12800; // Penalty for wrong group
distance += RECRUIT_WRONG_GROUP_PENALTY;

// Check if this is a better candidate
if ((minDistance == -1 || distance < minDistance) &&
Expand Down Expand Up @@ -1831,7 +1717,7 @@ FootClass* FindClosestAircraft(TeamClass* team, int memberIndex, const CoordStru
// Calculate distance with penalty for wrong group
int distance = aircraft->DistanceFromSquared(&location);
if (aircraft->Group != targetGroup)
distance += 12800; // Penalty for wrong group
distance += RECRUIT_WRONG_GROUP_PENALTY;

// Check if this is a better candidate
if ((minDistance == -1 || distance < minDistance) &&
Expand Down Expand Up @@ -1862,7 +1748,7 @@ FootClass* FindClosestUnit(TeamClass* team, int memberIndex, const CoordStruct&
// Calculate distance with penalty for wrong group
int distance = unit->DistanceFromSquared(&location);
if (unit->Group != targetGroup)
distance += 12800; // Penalty for wrong group
distance += RECRUIT_WRONG_GROUP_PENALTY;

// Check if this unit matches requirements
if (unit->Owner == team->OwnerHouse && EligibleToRecruit(team->Type->TaskForce->Entries[memberIndex].Type , unit->Type) &&
Expand Down Expand Up @@ -3434,7 +3320,7 @@ void FakeTeamClass::_TMission_Play_Animation(ScriptActionNode* nNode, bool arg3)
// Validate anim index
if (animIndex >= 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);
this->Type->ID, animIndex, AnimTypeClass::Array->Count - 1);

this->StepCompleted = true;
return;
Expand Down Expand Up @@ -4197,8 +4083,7 @@ void FakeTeamClass::_AI()
}

// Target direction (0-7 facing) converted to internal facing units
// Multiply by 8192 (0x2000) to convert 8-direction facing to 16-bit facing (65536 / 8 = 8192)
DirType targetFacing = (DirType)(node.Argument << 13);
DirType targetFacing = (DirType)(node.Argument * FACING_DIRECTION_MULTIPLIER);

bool allMembersFacing = true;

Expand Down Expand Up @@ -4577,8 +4462,8 @@ void FakeTeamClass::_AI()
{
auto const& [miss, value] = this->CurrentScript->GetCurrentAction();

// Mission data < 702 is some threshold check (possibly frame-based)
if (value < 702) {
// Mission data threshold check (possibly frame-based)
if (value < PATROL_THRESHOLD) {
this->_AssignMissionTarget(waypointCell);
}
}
Expand Down