Skip to content

Commit

Permalink
Fix occasional crash in CBuilderCAI::ExecuteBuildCmd() (#1947)
Browse files Browse the repository at this point in the history 
* * Fix occasional crash in CBuilderCAI::ExecuteBuildCmd() caused by a dangling other command, the reason for such commands to dangle is unknown for now
* Actually assign commands in CCommandAI::inCommand instead of specifying boolean flag, will help debug the above
* C++ pass on CMD_ and other related constants
  • Loading branch information
@lhog
lhog authored Feb 17, 2025
1 parent 1071a1b commit 53ef2b8
Show file tree
Hide file tree
Showing 7 changed files with 138 additions and 137 deletions.
2 changes: 1 addition & 1 deletion rts/Sim/MoveTypes/HoverAirMoveType.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ static const unsigned int FLOAT_MEMBER_HASHES[] = {

static bool UnitIsBusy(const CCommandAI* cai) {
// queued move-commands (or active build/repair/etc-commands) mean unit has to stay airborne
return (cai->inCommand || cai->HasMoreMoveCommands(false));
return (cai->inCommand != CMD_STOP || cai->HasMoreMoveCommands(false));
}

static bool UnitHasLoadCmd(const CCommandAI* cai) {
Expand Down
28 changes: 14 additions & 14 deletions rts/Sim/Units/CommandAI/AirCAI.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ bool CAirCAI::AirAutoGenerateTarget(AAirMoveType* myPlane) {
return false;

commandQue.push_front(Command(CMD_ATTACK, INTERNAL_ORDER, tgt->id));
inCommand = false;
inCommand = CMD_STOP;
return true;
}

Expand Down Expand Up @@ -290,7 +290,7 @@ void CAirCAI::ExecuteFight(Command& c)

if (tempOrder) {
tempOrder = false;
inCommand = true;
inCommand = CMD_FIGHT;
}

if (c.GetNumParams() < 3) {
Expand All @@ -299,7 +299,7 @@ void CAirCAI::ExecuteFight(Command& c)
}

if (c.GetNumParams() >= 6) {
if (!inCommand)
if (inCommand == CMD_STOP)
commandPos1 = c.GetPos(3);
} else {
// HACK to make sure the line (commandPos1,commandPos2) is NOT
Expand All @@ -315,8 +315,8 @@ void CAirCAI::ExecuteFight(Command& c)

float3 goalPos = c.GetPos(0);

if (!inCommand) {
inCommand = true;
if (inCommand == CMD_STOP) {
inCommand = CMD_FIGHT;
commandPos2 = goalPos;
}
if (c.GetNumParams() >= 6)
Expand Down Expand Up @@ -345,7 +345,7 @@ void CAirCAI::ExecuteFight(Command& c)
commandQue.push_front(Command(CMD_ATTACK, c.GetOpts(), enemy->id));

tempOrder = true;
inCommand = false;
inCommand = CMD_STOP;

if (lastPC1 != gs->frameNum) { // avoid infinite loops
lastPC1 = gs->frameNum;
Expand All @@ -366,7 +366,7 @@ void CAirCAI::ExecuteFight(Command& c)
commandQue.push_front(Command(CMD_ATTACK, c.GetOpts(), enemy->id));

tempOrder = true;
inCommand = false;
inCommand = CMD_STOP;

// avoid infinite loops (?)
if (lastPC2 != gs->frameNum) {
Expand Down Expand Up @@ -396,7 +396,7 @@ void CAirCAI::ExecuteAttack(Command& c)
}
}

if (inCommand) {
if (inCommand == CMD_ATTACK) {
if (targetDied || (c.GetNumParams() == 1 && UpdateTargetLostTimer(int(c.GetParam(0))) == 0)) {
StopMoveAndFinishCommand();
return;
Expand Down Expand Up @@ -436,12 +436,12 @@ void CAirCAI::ExecuteAttack(Command& c)
SetOrderTarget(targetUnit);
owner->AttackUnit(targetUnit, !c.IsInternalOrder(), false);

inCommand = true;
inCommand = CMD_ATTACK;
} else {
SetGoal(c.GetPos(0), owner->pos, cancelDistance);
owner->AttackGround(c.GetPos(0), !c.IsInternalOrder(), false);

inCommand = true;
inCommand = CMD_ATTACK;
}
}
}
Expand All @@ -456,15 +456,15 @@ void CAirCAI::ExecuteAreaAttack(Command& c)

if (targetDied) {
targetDied = false;
inCommand = false;
inCommand = CMD_STOP;
}

const float3& pos = c.GetPos(0);
const float radius = c.GetParam(3);

if (inCommand) {
if (inCommand == CMD_ATTACK) {
if (myPlane->aircraftState == AAirMoveType::AIRCRAFT_LANDED)
inCommand = false;
inCommand = CMD_STOP;

if (orderTarget && orderTarget->pos.SqDistance2D(pos) > Square(radius)) {
// target wandered out of the attack-area
Expand All @@ -473,7 +473,7 @@ void CAirCAI::ExecuteAreaAttack(Command& c)
}
} else {
if (myPlane->aircraftState != AAirMoveType::AIRCRAFT_LANDED) {
inCommand = true;
inCommand = CMD_ATTACK;

SelectNewAreaAttackTargetOrPos(c);
}
Expand Down
44 changes: 24 additions & 20 deletions rts/Sim/Units/CommandAI/BuilderCAI.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ void CBuilderCAI::ExecuteBuildCmd(Command& c)
if (buildOptions.find(c.GetID()) == buildOptions.end())
return;

if (!inCommand) {
if (inCommand == CMD_STOP) {
BuildInfo bi;

// note:
Expand Down Expand Up @@ -610,9 +610,13 @@ void CBuilderCAI::ExecuteBuildCmd(Command& c)

// <build> is never parsed (except in PostLoad) so just copy it
build = bi;
inCommand = true;
inCommand = c.GetID();
}

// guard against dangling non-build commands
if (inCommand >= CMD_STOP)
return;

assert(build.def != nullptr);
assert(build.def->id == -c.GetID() && build.def->id != 0);

Expand Down Expand Up @@ -671,7 +675,7 @@ void CBuilderCAI::ExecuteBuildCmd(Command& c)
}

if (f != nullptr && (!build.def->isFeature || build.def->wreckName != f->def->name)) {
inCommand = false;
inCommand = CMD_STOP;
ReclaimFeature(f);
return;
}
Expand Down Expand Up @@ -780,7 +784,7 @@ void CBuilderCAI::ExecuteRepair(Command& c)

ownerBuilder->StopBuild();
if (FindRepairTargetAndRepair(pos, radius, c.GetOpts(), false, (c.GetOpts() & META_KEY))) {
inCommand = false;
inCommand = CMD_STOP;
SlowUpdate();
return;
}
Expand Down Expand Up @@ -836,7 +840,7 @@ void CBuilderCAI::ExecuteCapture(Command& c)
ownerBuilder->StopBuild();

if (FindCaptureTargetAndCapture(pos, radius, c.GetOpts(), (c.GetOpts() & META_KEY))) {
inCommand = false;
inCommand = CMD_STOP;
SlowUpdate();
return;
}
Expand Down Expand Up @@ -912,7 +916,7 @@ void CBuilderCAI::ExecuteGuard(Command& c)
Command nc(CMD_REPAIR, c.GetOpts(), b->curBuild->id);

commandQue.push_front(nc);
inCommand = false;
inCommand = CMD_STOP;
SlowUpdate();
return;
}
Expand All @@ -929,7 +933,7 @@ void CBuilderCAI::ExecuteGuard(Command& c)
StopSlowGuard();

commandQue.push_front(Command(CMD_REPAIR, c.GetOpts(), fac->curBuild->id));
inCommand = false;
inCommand = CMD_STOP;
// SlowUpdate();
return;
}
Expand All @@ -954,7 +958,7 @@ void CBuilderCAI::ExecuteGuard(Command& c)
StopSlowGuard();

commandQue.push_front(Command(CMD_REPAIR, c.GetOpts(), guardee->id));
inCommand = false;
inCommand = CMD_STOP;
return;
}

Expand Down Expand Up @@ -1090,7 +1094,7 @@ void CBuilderCAI::ExecuteReclaim(Command& c)
if (recSpecial) recopt |= REC_SPECIAL;

if (FindReclaimTargetAndReclaim(pos, radius, c.GetOpts(), recopt)) {
inCommand = false;
inCommand = CMD_STOP;
SlowUpdate();
return;
}
Expand Down Expand Up @@ -1148,7 +1152,7 @@ void CBuilderCAI::ExecuteResurrect(Command& c)
c = Command(CMD_REPAIR, c.GetOpts() | INTERNAL_ORDER, ownerBuilder->lastResurrected);

ownerBuilder->lastResurrected = 0;
inCommand = false;
inCommand = CMD_STOP;
SlowUpdate();
return;
}
Expand All @@ -1165,7 +1169,7 @@ void CBuilderCAI::ExecuteResurrect(Command& c)
const float radius = c.GetParam(3);

if (FindResurrectableFeatureAndResurrect(pos, radius, c.GetOpts(), (c.GetOpts() & META_KEY))) {
inCommand = false;
inCommand = CMD_STOP;
SlowUpdate();
return;
}
Expand Down Expand Up @@ -1208,15 +1212,15 @@ void CBuilderCAI::ExecuteFight(Command& c)

if (tempOrder) {
tempOrder = false;
inCommand = true;
inCommand = CMD_FIGHT;
}
if (c.GetNumParams() < 3) {
LOG_L(L_ERROR, "[BuilderCAI::%s][f=%d][id=%d][#c.params=%d min=3]", __func__, gs->frameNum, owner->id, c.GetNumParams());
return;
}

if (c.GetNumParams() >= 6) {
if (!inCommand)
if (inCommand == CMD_STOP)
commandPos1 = c.GetPos(3);

} else {
Expand All @@ -1230,8 +1234,8 @@ void CBuilderCAI::ExecuteFight(Command& c)
}

float3 pos = c.GetPos(0);
if (!inCommand) {
inCommand = true;
if (inCommand == CMD_STOP) {
inCommand = CMD_FIGHT;
commandPos2 = pos;
}

Expand Down Expand Up @@ -1259,7 +1263,7 @@ void CBuilderCAI::ExecuteFight(Command& c)
// Priority 1: Repair
if (!reclaimEnemyOnlyMode && (ownerDef->canRepair || ownerDef->canAssist) && FindRepairTargetAndRepair(curPosOnLine, searchRadius, c.GetOpts(), true, resurrectMode)){
tempOrder = true;
inCommand = false;
inCommand = CMD_STOP;

if (lastPC1 != gs->frameNum) { //avoid infinite loops
lastPC1 = gs->frameNum;
Expand All @@ -1272,7 +1276,7 @@ void CBuilderCAI::ExecuteFight(Command& c)
// Priority 2: Resurrect (optional)
if (!reclaimEnemyOnlyMode && resurrectMode && ownerDef->canResurrect && FindResurrectableFeatureAndResurrect(curPosOnLine, searchRadius, c.GetOpts(), false)) {
tempOrder = true;
inCommand = false;
inCommand = CMD_STOP;

if (lastPC2 != gs->frameNum) { //avoid infinite loops
lastPC2 = gs->frameNum;
Expand All @@ -1285,7 +1289,7 @@ void CBuilderCAI::ExecuteFight(Command& c)
// Priority 3: Reclaim / reclaim non resurrectable (optional) / reclaim enemy units (optional)
if (ownerDef->canReclaim && FindReclaimTargetAndReclaim(curPosOnLine, searchRadius, c.GetOpts(), recopt)) {
tempOrder = true;
inCommand = false;
inCommand = CMD_STOP;

if (lastPC3 != gs->frameNum) { //avoid infinite loops
lastPC3 = gs->frameNum;
Expand Down Expand Up @@ -1314,7 +1318,7 @@ void CBuilderCAI::ExecuteRestore(Command& c)
if (!owner->unitDef->canRestore)
return;

if (inCommand) {
if (inCommand == CMD_RESTORE) {
if (!ownerBuilder->terraforming)
StopMoveAndFinishCommand();

Expand All @@ -1327,7 +1331,7 @@ void CBuilderCAI::ExecuteRestore(Command& c)

if (MoveInBuildRange(pos, radius * 0.7f)) {
ownerBuilder->StartRestore(pos, radius);
inCommand = true;
inCommand = CMD_RESTORE;
}
}
}
Expand Down
Loading

0 comments on commit 53ef2b8

Please sign in to comment.