Fix engineers not dying when siege engine they are manning is killed

[patel-nikhil]

Works for player siege engine, may work for some AI siege weapons, but not all

[patel-nikhil]

ghost engineer build.zip

[Monsterfisch]

seems like the file is not working correctly (when trying to run or extract the file it gives me an errormessage)

[GRhin]

Install 7zip, that's how np zips files, and often i need 7zip installed to unzip properly

[patel-nikhil]

Ah sorry, did this one in bit of a rush. Yeah, 7zip should open it. I’ll try and set up a google drive folder for my PRs in the next few days with them zipped regularly

[Monsterfisch]

mhm so I didn't test to confirm engineers being counted towards the final roundup of the killed troops but I can confidently say that this doesn't seem to help with the raid siege engines not being build anymore. it also introduces other issues like ai lords increasingly spawning as "dead" sometimes even making it so that all lords spawn as dead =/
it also introduces the weird bug where you get a "million" random entities on the map that happened in the basic version of crusader when sh0wdown introduced the increased unit limit. (this could be related to the lords spawning as dead however as they keep producing troops and not attacking anyone so take it with a grain of salt.

the installation of the PR is also excessively slow compared to the other PRs this is ofc not a biggy per se but something to consider.

[GRhin]

@patel-nikhil do you know why this might cause the AI lords spawning as dead?
@Monsterfisch do you have a save, or a way to replicate that issue?

[GRhin]

Just did a bunch of testing in multiplayer Extreme.
This correctly decreases army count for engineers when seige units are destroyed. Works as advertised. No bugs found.
Tried with different seige units. no problems.
Only thing i think is missing is that killed engineers dont count towards units killed or units lost. Is this possible?
Incidentally, i found another bug during testing that may or may not be in the same area in case you know how to fix it;
Sometimes engineers "die" when manning a seige equipment. Not sure exactly the trigger, but it seems to be when creating new seige equipment while new units or workers are spawning, however the engineer that dies is not the one in the new equipment, but one that already exists in another seige equipment.
To replicate:

1. Open a two human player game. 
2. Player 1 builds granary, 2 houses, and a mercencaries camp.
3. Player 2 builds a granary, 2 houses, and an engineers guild.
4. Player 2 recruits a bunch of engineers (we had success with as little as 12, but sometimes required 30 or more)
5. Player 1 starts recruiting slaves, and continues to do so, so that new workers are constantly spawning.
6. Player 2 builds sheilds with their engineers.
7. Once all engineers has built their shield, check the army tab of the player 2's scribes book.
 You should notice that the engineer count is lower than you started with, and lower than the sheild count. 
All sheilds should be able to continue to move as if manned, but when you unman them the specific shield 
that has the "dead" engineer will not appear. These dead engineers count towards a players "units lost" value.

Note that this is present in vanilla game, not introduced by this PR (we specifically tested on another install that did not have this option installed) I bring it up here simply because it might be related in some way to the code NP has already worked on.

[patel-nikhil]

do you know why this might cause the AI lords spawning as dead?

So I'm not sure how my change could affect this. It works based on number of manning engineers, and when a siege engine dies. I have to revisit to give a more detailed answer.

Only thing i think is missing is that killed engineers dont count towards units killed or units lost. Is this possible?

When developing this I ensured the engineers get 'killed' and count updates accordingly, but the game stores a matrix of unit kills/losses and at the time I had not taken that into account. It'd be rather complex and time consuming to attribute it properly, but in theory possible - it assumes the siege engine is being counted properly towards kills/losses and there is some value we can get and use to figure out what killed it.

[GRhin]

The seige engine is not counted towards kill/lost units either.

[patel-nikhil]

Hmm okay then that makes it more complicated. Will have to investigate further to figure out if and how this can be done.

[Krarilotus]

https://youtu.be/XcvSr73ERnI this is a recording if Grhin and me investistiagting it in multiplayer, maybe it helps. But it could as well be not useful at all.

[patel-nikhil]

It seems this has been tested for multiplayer and works as expected.

Fixing this for single player (for AI) would require a set of different, independent bugfixes - is it worth doing in a separate pull request?

For making siege engines count towards lost/killed units, that may or may not be possible - because it would have to be attributed to the correct count (either PlayerX kill PlayerY, or death by map (lions etc)). Is that considered essential to this fix?

[GRhin]

The ai fix is a different bug, that can be easily confused with this one, in that it has the same result, but a different cause. Therfore it would be a different feature, not impacting on this.
As for the k/d. It should count as player x kills player y, and that should certainly be an extension of this feature, but if we describe it for now as "removing the engineers from the current unit count" then this can be released without that extension.

[Krarilotus]

how to resolve merge conflicts so people can test this?