[PATCH] Allied units are not ejected upon player resignation.

[scythetwirler]

When a player resigns, allied units are not ejected. You will have a garrisoned Gaia structure.

[mimo]

Here is a fix to this bug. While testing it, I noticed that the rallyPoints were effective for allied units. As I don't think this is a wanted feature, I've also removed it in the patch. If you want to restore it, just remove the first lines of the patch (those about lines 278 to 289).

[sanderd17]

Your improvements work good, except for ships. But I see the existing also has problems with ships. When a ship changes ownership (or units in a ship change), they are forced to eject, and dropped to the bottom of the ocean. That would be a very nice way to cheat and attack ships with land units (imagine dropping rams to the ocean). It also happens in other parts (s.a. OnDiplomacyChanged).

Please see the OnHealthChanged about how it should be handled. It should be a forced eject for units in normal buildings, and just kill the units that are in entities (ships, planes ...) that kill their garrisoned entities when they die.

An alternative way would be ungarrisoning without being forced, and if that fails, kill the units. If you do it that way, also check the IsGarrisoningAllowed method, which is set by the plane when it's in the air. So although above valid ground, units should still be killed on ownership change.

It would be great if you can track down these issues.

Thanks for cleaning this up.

[mimo]

Thanks for the checks. I indeed did not tested the ships behaviour. I've just updated the patch.

[sanderd17]

Looks like a clean sollution with that extra throwAway function. I'll try it.

[leper]

I'd rename ThrowAway to EjectOrKill (which tells you what it does and doesn't have you look at the comment).

[sanderd17]

In 13810:

fix ejection of garrisoned units on resign. Fix ejection or killing on ownership and deplomacy changes. Patch by mimo. Fixes #2096

[sanderd17]

I modified the OnHealthChanged to also use your new method (and avoid code duplication).

Thanks for working on the patch.

[mimo]

Yes, I've hesitated to also do it, but haven't because the behaviour is a bit different : in this EjectOrKill, we do not go to the rallyPoint if there is any, while in the previous OnHealthChanged, we would go to it. I've no strong opinion about the preferred behaviour.

[mimo]

There are still two small corrections needed in the latest GarrisonHolder code: UpdateGarrisonFlag is needed after line 542. After this change, the UpdateGarrisonFlag from line 490 could be moved to line 486.

[sanderd17]

In 13815:

better garrison flag handling. Refs #2096.

[mimo]

There is still a problem in GarrisonHolder because the test on the number of already garrisoned units is done using this.spaceOccupied, which is not always synchronised. We could add the missing synchronizations, but I think it would be simpler and safer to directly test on this.entities.length. I've added the corresponding patch.

[sanderd17]

In 13857:

fix garrisoned units count for some special cases. Refs #2096. Patch by mimo

[sanderd17]

You're right, clean patch, I added it.