bugfix: Prevent dead units from repeatedly dealing crash damage on collision with non-buildings

[Stubbjax]

Closes #2015

This change fixes an issue where dead vehicles repeatedly trigger VehicleCrashesIntoNonBuildingWeapon when colliding with mines, causing continuous damage to nearby units and structures.

This weapon is hardcoded into the physics update, and is fired when a vehicle falls onto a non-building object - the intent being that a vehicle deals some 'crush' damage to a victim it lands on. It is especially problematic in the case of mines as the mines cannot destroy the already-dead vehicle. The issue is most apparent with Technicals due to their frequent involvement in bombing runs and their tendency to fling themselves forwards upon death.

The fix prevents this weapon from firing if the vehicle is already dead.

There is also a building-specific crash block that fires VehicleCrashesIntoBuildingWeapon when a vehicle crashes into a building, however this results in the immediate destruction of the crashing vehicle, and so is not as problematic.

Before

The Propaganda Center dies due to dead Technicals firing VehicleCrashesIntoNonBuildingWeapon every frame

YES_DAMAGE.mp4

After

The Propaganda Center survives unscathed as dead Technicals no longer fire VehicleCrashesIntoNonBuildingWeapon

NO_DAMAGE.mp4

[greptile-apps]

Greptile Overview

Greptile Summary

Fixed a bug where dead vehicles repeatedly dealt crash damage every frame when colliding with non-building objects like mines. The fix adds an isEffectivelyDead() check before firing VehicleCrashesIntoNonBuildingWeapon, preventing already-dead vehicles from dealing damage. This was especially problematic with Technicals during bombing runs, as mines couldn't destroy already-dead vehicles, leading to continuous damage to nearby structures.

• Added !obj->isEffectivelyDead() condition to crash damage logic in both Generals and GeneralsMD versions
• Used RETAIL_COMPATIBLE_CRC preprocessor flag to maintain backwards compatibility
• Follows established codebase patterns for bug fixes and compatibility preservation

Confidence Score: 5/5

• This PR is safe to merge with minimal risk
• The fix is minimal, well-targeted, and uses an existing API (isEffectivelyDead()) that is widely used throughout the codebase. The change is protected by a compatibility flag and mirrors the established pattern of the building-crash path that already prevents dead vehicles from causing issues. The logic is sound and directly addresses the root cause.
• No files require special attention

Important Files Changed

Filename	Overview
Generals/Code/GameEngine/Source/GameLogic/Object/Update/PhysicsUpdate.cpp	Added dead unit check to prevent repeated crash damage when colliding with non-buildings
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/PhysicsUpdate.cpp	Added dead unit check to prevent repeated crash damage when colliding with non-buildings

Sequence Diagram

sequenceDiagram
    participant Physics as PhysicsBehavior::onCollide
    participant Vehicle as Dead Vehicle Object
    participant Mine as Mine/Non-Building
    participant Weapon as WeaponStore
    participant Nearby as Nearby Units/Structures
    
    Note over Physics,Mine: Frame N: Vehicle collides with mine
    Physics->>Vehicle: Check if KINDOF_VEHICLE
    Physics->>Vehicle: Check if !isEffectivelyDead() [NEW]
    alt Vehicle is dead (before fix)
        Physics->>Weapon: createAndFireTempWeapon(VehicleCrashesIntoNonBuildingWeapon)
        Weapon->>Nearby: Deal damage
        Note over Physics,Nearby: Every frame collision persists
    else Vehicle is dead (after fix)
        Physics-->>Physics: Skip weapon firing
        Note over Physics: No damage dealt
    end
    
    Note over Physics,Mine: Frame N+1: Still colliding
    alt Before fix
        Physics->>Weapon: Fire weapon AGAIN
        Weapon->>Nearby: Deal damage AGAIN
        Note over Nearby: Continuous damage until mine destroyed
    else After fix
        Physics-->>Physics: Skip - vehicle is dead
    end

Loading

[Mauller]

This doesn't properly resolve #2015, the issue with aircraft landing on mines is that the aircraftSlowDeathBehaviour module for the chinook is waiting to see when the unit hits the ground. But it never does and remains floating over the mines above the ground detection threshold.

The issue is that the mines block the slow death code from detecting when the chinook hits the ground properly and prevents the carcus from ever clearing itself.

[Stubbjax]

This doesn't properly resolve #2015, the issue with aircraft landing on mines is that the aircraftSlowDeathBehaviour module for the chinook is waiting to see when the unit hits the ground. But it never does and remains floating over the mines above the ground detection threshold.

The issue is that the mines block the slow death code from detecting when the chinook hits the ground properly and prevents the carcus from ever clearing itself.

That sounds like a separate issue? This fix targets the issue as described; "GLA Technical kills/damages building when dying by China Mines".

[Mauller]

This doesn't properly resolve #2015, the issue with aircraft landing on mines is that the aircraftSlowDeathBehaviour module for the chinook is waiting to see when the unit hits the ground. But it never does and remains floating over the mines above the ground detection threshold.
The issue is that the mines block the slow death code from detecting when the chinook hits the ground properly and prevents the carcus from ever clearing itself.

That sounds like a separate issue? This fix targets the issue as described; "GLA Technical kills/damages building when dying by China Mines".

Ah yeah, i glanced over it and read the bit about migs and chinooks in the issue as well.

The other week Fish and i were looking into the same issue but with aircraft and it is a problem with the death behaviour modules.

I don't think your change here will fix their behaviour since they are never properly set to being effectively dead since they remain stuck in their death crash handling.

[Stubbjax]

I don't think your change here will fix their behaviour since they are never properly set to being effectively dead since they remain stuck in their death crash handling.

Is effectively dead not exactly the status that should apply here? As far as I'm aware that status is applied as soon as health becomes depleted.

[Mauller]

I don't think your change here will fix their behaviour since they are never properly set to being effectively dead since they remain stuck in their death crash handling.

Is effectively dead not exactly the status that should apply here? As far as I'm aware that status is applied as soon as health becomes depleted.

I would need to check again, have you seen if this also prevents falling planes and helicopters applying continuous damage when they land on mines?

Or checked if falling planes and helicopters apply any damage when landing on things?

[Stubbjax]

I would need to check again, have you seen if this also prevents falling planes and helicopters applying continuous damage when they land on mines?

This will affect anything that would have previously repeatedly applied VehicleCrashesIntoNonBuildingWeapon on collision with mines. This should include aircraft as they all have the necessary VEHICLE KindOf, but I have not explicitly tested it.

[xezon]

I suspect this fix is not correct. I put a breakpoint into this branch and on the very first hit the Object is already dead, which means that this weapon now never fires once.

[Stubbjax]

Can you clarify what you mean? You have described the fix as intended, which is to prevent crash damage from triggering if the object is effectively dead.

[xezon]

Ok how do we know the intent is to not apply damage once when dead technical crashes?

[Stubbjax]

We do not. We can only say the new intent is to not have it deal any damage in order to improve gameplay.

[xezon]

I expect that a dead vehicle crashing into something can apply damage once. For example if a plane is shot and its rubble hits the ground, then it applies collision damage once. Otherwise only alive vehicles would cause crash damage, which may not be intuitive for players. This is also more in line with what the original code did (incorrectly).

[Stubbjax]

I agree that the behaviour could be more intuitive, especially in the case of crashed aircraft. However, this change isn't intended to add intuitive behaviour - it's meant to fix a bug.

This is also more in line with what the original code did (incorrectly).

In what way? Retail behaviour does not result in damage to units from crashed aircraft (unless it's the collateral from hitting a building). In fact, nothing mobile with an AIUpdateInterface seems to be affected as far as I can tell.

NO_DAM.mp4

If such behaviour is desirable, then a weapon needs to be fired upon collision with the ground via a dedicated data-driven module where the radius and damage amount can be controlled. Alternatively, aircraft can be assigned a CrusherLevel where necessary as an easy (though limited) solution.

[xezon]

Should the same test exist for this damage?

[Stubbjax]

I think it would probably be better. But as a separate change?

[xezon]

If the repro is unknown or very different for the other, then maybe make separate change.