[PATCH] When a ship sinks/building destroys without space to unload units aren't lost in summary

[eternaf]

If you sink a ship, any unit garrisoned on it and the ship itself won't get counted as lost in the summary of the game.

[Vladislav Belov]

Summary doesn't contain units lost in building too (when no free space to place units before destruction).

But there is one moment: if delete a unit/builduing manually, then it won't be in the summary, or it shouldn't be?

[elexis]

The logic may or maynot be moved from GarrisonHolder or StatisticsTracker, not sure what's worse.

What if you garrison a garrisoned ram onto a ship which sinks? Are there actually further bugs than not counting those units with the statistics tracker, as those entities are not added to killedEntities? Could you do some philadelphia experiments to find out?

[Vladislav Belov]

Replying to elexis:

What if you garrison a garrisoned ram onto a ship which sinks? Are there actually further bugs than not counting those units with the statistics tracker, as those entities are not added to killedEntities? Could you do some philadelphia experiments to find out?

What's the ram? I could try to garrison another types of units. Logic of using StatisticsTracker is present in few other components.

[elexis]

Ram as in ​https://en.wikipedia.org/wiki/Battering_ram, as said above, one can garrison infantry onto a ram and garrison the ram onto a ship. Will the staticstracker be updated with these recursively garrisoned units too? Will cmpHealth.Kill() be called for those?

[Vladislav Belov]

Replying to elexis:

Ram as in ​https://en.wikipedia.org/wiki/Battering_ram, as said above, one can garrison infantry onto a ram and garrison the ram onto a ship. Will the staticstracker be updated with these recursively garrisoned units too? Will cmpHealth.Kill() be called for those?

It works for this case too. It will be recursively updated.

[elexis]

So far so good, but

• If a player deletes units, they won't count as "lost", but with your patch they would in case theywere garrisoned on a deleted ship. After reading Damage.TargetKilled it becomes clear that the stat should reflect how many units were killed by enemies
• cmpKillerPlayerStatisticsTracker.KilledEntity should therefore be called too
• You could try to call GarrisonHolder.EjectOrKill from Damage.TargetKilled with an additional argument giving the killer (and if the killer isn't set, it was likely deleted).
• Another edge case would be garrisoning units on an allied ship and then changing the diplomacy in the middle of the ocean. I guess in this case it should also count as actively killed units (killed by the ship).

[elexis]

Backlogging due to lack of progress.

[elexis]

If a player deletes something (Commands.js), the GarrisonHolder entities can be deleted manually (has to be done recursively since there can be ram garrisoned with units garrisoned in the ship).

The case of changing diplo in the middle of the sea can likely be ignored.

Also there are other cases where Kill is called but not the statistics tracker, for example extinct volcano, the Cheat or UnitMotionFlying.

[Mohamed Moanis]

Damage.TargetKilled is called after the target has already been killed. So the GarrisonHolder.OnHealthChange will be called earlier and therefore EjectOrKill,right?

Moving with a similar patch, it will be really hard to tell who killed the units garrisoned.

For alternatives, don't you think that the Health.kill should differentiate between an entity being deleted and being killed -for example Health.delete? Furthermore, should the Health component or atleast the OnHealthChange know what caused this change and who -take different actions according to who hurt or healed me, right?

Considering that the Health component can know what and who caused the change, this can be easily solved.

[elexis]

Statistics for killing and lost garrisoned units should work the same way like stats for ungarrisoned units.

Hm, EjectOrKill can also be called on ownershipchange and diplomacychange. I guess these messages would need extension too if we don't want to close this as won't fix. Should check that there isn't a performance problem as at least the healthchanged one can be sent often. But probably isn't.

[Imarok]

Move tickets to Simulation as UI & Simulation got some sub components.

[Mercury]

I made a fix for this. Addresses garrisonable entities being ejected due to damage, ownership change, diplomacy change or garisonable type change. calls KilledBy from EjectOrKill. Creates a new event named DamageTaken which fires after damage is done but before checking for death and includes attacker and attackOwner in the event message.

​https://code.wildfiregames.com/D4643

[Stan]

From #6675

​https://wildfiregames.com/forum/topic/96804-alpha26-feedback-and-suggested-changes/?do=findComment&comment=539271

When a ship is destroyed that had units garrisoned inside, these units disappear form the game (they've presumably died in the shipwreck) but they are not included in the lost unit counter in the stats. Is this intentional?

Screenshot from the stats of my playing Corsica and Sardinia for the first time in a while. You can see that the total of units lost from red and yellow are lower than the number of trained units.

Disclaimer: new to the forum (not new to the game, started playing in 2018). If this is not the right place for this kind of post, lmk. I sifted through the FAQ as well as this thread, and did a quick reddit search on r/0ad and did not see anything recent about this topic, but I'd be surprised if I was the first to bring it up. Game version: Alpha26

[Freagarach]

Pushing back.