bugfix(ai): Do not rebuild the locomotor set of a dead aircraft

[wh1ter0se69]

• Fixes Airplane and helicopter wrecks can become stuck circling in air #62

What's wrong

JetSlowDeathBehavior::beginSlowDeath() grounds a dying aircraft by mutating the current
Locomotor instance:

Locomotor *locomotor = us->getAIUpdateInterface()->getCurLocomotor();
locomotor->setMaxLift( -TheGlobalData->m_gravity * (1.0f - modData->m_fallHowFast) );
locomotor->setMaxTurnRate( 0.0f );

A dead aircraft's AI keeps running, and when it changes locomotor set,
chooseLocomotorSetExplicit() throws that instance away and rebuilds from template:

m_locomotorSet.clear();          // deleteInstance() on every Locomotor in the set
m_curLocomotor = nullptr;
for (...) m_locomotorSet.addLocomotor(lt);   // TheLocomotorStore->newLocomotor(lt)

Locomotor::Locomotor(const LocomotorTemplate*) starts fresh — m_maxLift = BIGNUM,
m_maxTurnRate = BIGNUM. The wreck silently regains full lift and turn rate. It is no longer
falling; it is a flying aircraft with no pilot, and DestructionDelay = 99999999 means nothing
removes it. It circles until the match ends.

The fix refuses the rebuild for an effectively-dead aircraft, so the slow-death locomotor survives
and the wreck falls.

Why this layer

HelicopterSlowDeathUpdate mutates the current locomotor the same way
(setMaxLift + setMaxBraking), so the exposure is not jets-only. Counting retail templates that
carry a jet or helicopter slow-death module: 77 total, of which 45 never run JetAIUpdate —
27 use DeliverPayloadAIUpdate, 10 use ChinookAIUpdate (Chinook, Helix), 8 use plain
AIUpdateInterface. A guard in a single AI module cannot cover them; chooseLocomotorSetExplicit
is the one non-virtual funnel every module ends up in.

Retail already tried the per-call-site approach. ChinookAIUpdate.cpp refuses to restore lift on a
dead helicopter, with the right comment attached:

// don't restore lift if dead -- this may fight with JetSlowDeathBehavior!
if (!obj->isEffectivelyDead())
    loco->setMaxLift(BIGNUM);

...and then six lines later calls ai->chooseLocomotorSet(LOCOMOTORSET_TAXIING), which rebuilds the
set and restores BIGNUM anyway. Every direct lift restore is guarded; the indirect one through
the set rebuild is not. That is the bug.

Evidence

VC6 Release build (RTS_BUILD_OPTION_DEBUG=OFF), with a temporary probe logging any locomotor-set
rebuild on an effectively-dead aircraft. Captured live (USA Superweapon, Auroras guarded then killed
airborne):

[GXWRECK] frame=18807 obj=612 tmpl=SupW_AmericaJetAurora newset=7 hadLoco=1 airborne=1

newset=7 is LOCOMOTORSET_SLUGGISH; hadLoco=1 confirms the slow-death locomotor was present and
about to be discarded; airborne=1 confirms it was still in the air.

A/B on that recording, same binary, fix toggled by env var so playback is identical up to the
first block:

	events	outcome
fix off	1 rebuild allowed	stays airborne=1, circles indefinitely
fix on	87 rebuilds refused	airborne=0 at frame 18894 — falls in ~3 seconds

The CRC mismatch that follows in the fixed run is expected: the wreck now hits the ground, so the
simulation legitimately diverges from a recording of the broken behaviour.

No regression, against the full GeneralsReplays/GeneralsZH/1.04 CI corpus — 10 replays,
roughly four hours of game time:

	replays completed	CRC errors	wall clock
before	10/10	0	8:23
after	10/10	0	8:25

Both passes produced byte-identical stdout. The corpus never reaches the guarded state, which is why
it is unaffected either way.

Gating

The guard changes simulation state on a path retail reaches, so it sits behind
#if !RETAIL_COMPATIBLE_CRC like the other 85 blocks in the tree. That macro defaults to 1 and
nothing in the build system overrides it, so this ships disabled today and becomes live when the
project drops retail CRC compatibility. If maintainers would rather have it enabled sooner, the
paired PRESERVE_* form used elsewhere in GameDefines.h is the mechanism — that is a call for the
project, not for this PR.

Known limits

• Which call site produced newset=7 is not recoverable from this probe. It logs wst from
  inside chooseLocomotorSetExplicit, i.e. after JetAIUpdate::chooseLocomotorSet has already
  rewritten the requested set to m_returningLoco. LOCOMOTORSET_SLUGGISH appears in no .cpp at
  all, so the value can only have come from ReturnForAmmoLocomotorType = SET_SLUGGISH, which in
  retail is set on the five Aurora variants and nothing else. An earlier revision of this
  description attributed it to a Guard order; that attribution is not supported by the probe output
  and has been removed.
• JetAIUpdate calls setMaxLift() directly in several places for landing/takeoff sequencing.
  Those bypass the locomotor set change entirely, so this fix does not intercept them. Whether they
  are reachable on a dead jet is not established here.
• With the fix in place the AI retries the refused set change every frame (87 times in the capture
  above). Harmless and cheap, but it indicates the state machine is spinning on a transition it can
  never complete. Not driving a dead aircraft's AI at all may be the better long-term shape, and is
  a much bigger behavioural change than this one.

[Caball009]

Please update the PR description with only essential information.

It could be useful to have a VS22 replay for reproduction and testing. I've added a VS22 replay in the issue here.

[Caball009]

Try to keep the comment to two or three lines. It's fine if the comment doesn't hold all information, that's what the PR description (or later posts) are for.

Do not rebuild the locomotor set of a dead aircraft.

This is good. Please describe the rationale in one sentence as well.

[wh1ter0se69]

Cut to three lines in 18df0e2, with the rationale in the second sentence:

// TheSuperHackers @bugfix wh1ter0se69 10/08/2026 Do not rebuild the locomotor set of a dead
// aircraft. Rebuilding discards the Locomotor instance that a slow death module mutated to
// ground it, so the wreck regains full lift from template and keeps flying.

The mechanism detail moved into the PR description.

[Caball009]

How about fixing this higher up in the call stack?

if (!jet->isEffectivelyDead())
{
	chooseLocomotorSet(d->m_returningLoco);
}

[wh1ter0se69]

Checked this properly rather than answering from my own description, and the description turned out to be partly wrong. Corrected in 18df0e2.

Where you're right. SET_SLUGGISH can only reach chooseLocomotorSetExplicit through m_returningLoco: LOCOMOTORSET_SLUGGISH appears in no .cpp anywhere in the tree, and in retail ReturnForAmmoLocomotorType = SET_SLUGGISH is set on the five Aurora variants and nothing else. So the newset=7 in my capture came from the return-loco path — the "reached from a Guard order" line in the old description was not supported by the probe, which logs wst from inside chooseLocomotorSetExplicit, after JetAIUpdate::chooseLocomotorSet has already rewritten it. That claim is removed.

Why I'd still keep the guard at the funnel. Three reasons:

1. HelicopterSlowDeathUpdate mutates the current locomotor the same way (setMaxLift + setMaxBraking), so this is not jets-only. Of the 77 retail templates carrying a jet or helicopter slow-death module, 45 never run JetAIUpdate — 27 DeliverPayloadAIUpdate, 10 ChinookAIUpdate (Chinook and Helix, both named in Airplane and helicopter wrecks can become stuck circling in air #62), 8 plain AIUpdateInterface. A guard in JetAIUpdate::update() cannot reach any of them.

2. Even for jets it is one entry point of several. JetAIUpdate::chooseLocomotorSet() rewrites any caller's requested set into m_attackingLoco / m_returningLoco / TAXIING before delegating, so the same rebuild happens from callers that never go through update(). The sibling chooseLocomotorSet(d->m_attackingLoco) four lines above the line you linked has the identical exposure (AttackLocomotorType = SET_SUPERSONIC, same five Auroras). And ~20 lines above that, on current main, there is already a #if !RETAIL_COMPATIBLE_CRC block doing chooseLocomotorSet(LOCOMOTORSET_NORMAL) — arcticdolphin's Aurora supersonic-reset fix — which hits the same problem once the flag flips.

3. Retail already tried the per-call-site approach. ChinookAIUpdate.cpp has

   // don't restore lift if dead -- this may fight with JetSlowDeathBehavior!
   if (!obj->isEffectivelyDead())
       loco->setMaxLift(BIGNUM);

   and then, six lines later, ai->chooseLocomotorSet(LOCOMOTORSET_TAXIING); — which rebuilds the set and restores BIGNUM anyway. Every direct lift restore is guarded; the indirect one through the set rebuild is not. That is exactly this bug, already attempted, with the right comment attached.

chooseLocomotorSetExplicit is private, non-virtual and has exactly one caller, which is why I put it there. I do take the layering point — the header calls it the "jams it in, no sanity checking" primitive, and policy arguably does not belong in it. If you would rather the guard sat one level up in AIUpdateInterface::chooseLocomotorSet, that is identical coverage and I will move it; every override chains through there. The thing I would want to avoid is a jets-only guard.

One more change while I was in here: the guard now refuses rebuilds only, not the initial build. AIUpdateInterface::loadPostProcess() deliberately sets the current set to LOCOMOTORSET_INVALID and re-chooses it when loading a pre-version-4 save, and without that condition a dead aircraft in such a save would have been left with no locomotor set at all.