Add environmental damage types for each kill type

[keelefi]

Closes #1638

[keelefi]

Draft PR because I'm not sure if I like the naming of the variables and I want to sleep on it.

I welcome any comments or feedback, now or later.

[sprunk]

Looks good! Seems to be missing some reasons for death:

1. out-of-bounds
   

   spring/rts/Sim/MoveTypes/Systems/GroundMoveSystem.cpp

   Lines 107 to 108 in 2ef0794

   	if (!unit->pos.IsInBounds() && (unit->speed.w > MAX_UNIT_SPEED))
   	unit->ForcedKillUnit(nullptr, false, true);

2. aircraft crash
   

   spring/rts/Sim/MoveTypes/StrafeAirMoveType.cpp

   Lines 525 to 526 in 2ef0794

   	if ((CGround::GetHeightAboveWater(owner->pos.x, owner->pos.z) + 5.0f + owner->radius) > owner->pos.y)
   	owner->ForcedKillUnit(nullptr, true, false);

3. turned into feature
   

   spring/rts/Sim/Units/Unit.cpp

   Line 450 in 2ef0794

   KillUnit(nullptr, false, true);

4. some of the missing ones above may be what you meant by "killed by game", but that is a pretty bad name. Every death is ultimately via game mechanics one way or another.
   

   spring/rts/Lua/LuaConstGame.cpp

   Line 278 in 9c7cefe

   LuaPushNamedNumber(L, "KilledByGame" , -CSolidObject::DAMAGE_KILLED_GAME );

5. the "unit script" death is COB, not LUS. LUS uses regular Lua interfaces.
   

   spring/rts/Sim/Units/Scripts/UnitScript.cpp

   Line 1258 in 2ef0794

   u->KillUnit(nullptr, p2 != 0, p3 != 0);

   
   

   spring/rts/Lua/LuaConstGame.cpp

   Line 277 in 9c7cefe

   LuaPushNamedNumber(L, "LuaUnitScript" , -CSolidObject::DAMAGE_LUS );

6. I'd rename Decay to ConstructionDecay because any custom timeout mechanic could also be called just "decay".
   

   spring/rts/Lua/LuaConstGame.cpp

   Line 272 in 9c7cefe

   LuaPushNamedNumber(L, "Decay" , -CSolidObject::DAMAGE_DECAY );

[keelefi]

(if you'd rather see full code, this is more or less what I'm working with: BAR105...keelefi:spring:kill-unit-add-damage-types)

1. out-of-bounds

I'm planning to use DAMAGE_KILLED_GAME for this and the likes of it. Although, now that I think of it, it should be ENGINE rather than GAME.

2. aircraft crash

DAMAGE_COLLISION_GROUND and DAMAGE_COLLISION_OBJECT already exist, I was planning to use those for the ForcedKillUnit() calls from the air move types.

3. turned into feature

Thanks a lot, this I really don't understand what this is and how it happens.

What should it be named?

5. the "unit script" death is COB, not LUS. LUS uses regular Lua interfaces.

Thanks, I did not know this. I'll change it to DAMAGE_COB? And the Lua variable to LuaCOB?

6. I'd rename `Decay` to `ConstructionDecay` because any custom timeout mechanic could also be called just "decay".

Good point, thanks a lot.

Btw I'm still lacking damage types for KillUnit() calls from

spring/rts/Sim/Units/Unit.cpp

Line 959 in 2ef0794

KillUnit(nullptr, false, true);

spring/rts/Sim/Units/UnitHandler.cpp

Line 279 in 2ef0794

unit->ForcedKillUnit(nullptr, false, true);

[sprunk]

2. I was planning to use existing DAMAGE_COLLISION_GROUND for crashing aircraft

I think using a separate constant is good for two reasons:

• it's trivial for a gamedev to make the existing value represent both aircrashes and collisions even if they have their own ID (if id == aircrash then id = collision) but it's much harder to go the other way around and tell crashes apart from collisions if they are under the same ID and you have to use external clues
• and there are plausible reasons for a gamedev to handle them separately. For example for an aircraft to crash it usually has to be shot down, so maybe a game already considers being shot down a "kill" and will want to ignore the real death according to various mechanics such as veterancy because it already processed the kill. While a kill via collision may be the first and the last time the game receives an event about the collidee so should not be ignored.

1. I'm planning to use DAMAGE_KILLED_GAME/ENGINE for this and the likes of it.

I'd go with a specific "out of bounds" value with similar reasoning as above. For example maybe going out of bounds is more of an error case than just being killed in one of the other ways so the game may want to do something about it (idk, produce logs and send an error report?) and it's nice to be able to distinguish that this specifically happened.

3. I really don't understand what "turned into feature" is and how it happens. What should it be named?

It happens when the unit is set to turn itself into a feature on completion via the isFeature unit def tag, that's for example how Dragon's Teeth used to work until recently in BAR. Being called "TurnedIntoFeature" sounds alright.

5. I'll change it to DAMAGE_COB? And the Lua variable to LuaCOB?

DAMAGE_COB sounds fine. There is no Lua involved in killing the unit so I'd go with just COB for the Lua var.

still lacking damage type for Unit.cpp : 959

A unit dies this way when health is directly set to less than 0 without any damage involved. Perhaps call it HealthUnder0 or SetNegativeHealth or something like that?

damage type for UnitHandler.cpp : 279

Currently this is reachable only from Lua, so will pass along whatever weaponID Lua code decided the unit was killed by. It doesn't need its own value.

[keelefi]

Thank you greatly for all the very thorough explanations!

I basically implemented all of your suggestions with the only exception that deaths from unit scripts are DAMAGE_UNIT_SCRIPT and UnitScript.

[keelefi]

How these are thought to be used can be seen here: d1df939

[sprunk]

One more minor thing (shouldn't cause conflicts in follow-up patches): keelefi#2

[keelefi]

One more minor thing (shouldn't cause conflicts in follow-up patches): keelefi#2

Done.

[lhog]

Looks to be included in aed398d#diff-85b82291d073a0f5bf8b0a4d29693156937cb4359544e2816acc61f3980a21e9R283 ?

[sprunk]

Yes, #1669 is based on this and wasn't really ready. I'll fix it up later.

[keelefi]

@sprunk you will make a separate PR for the fixup commit that closes #1638 am I right?

[sprunk]

Yes.