[PATCH] Fix templates of non-garrisonable units

[elexis]

The original idea of this ticket was to hide the garrison-button for non-garrisonable units. But since that doesn't seem nicely achievable, we might want to fix those two related bugs identified in the course of this ticket:

Some units can't be garrissoned anywhere and have incoherent garrisson usage:

• Sheep Currently UnitAI can't deal with garrisoning animals. Hence it has a hardcoded disabling of animals. The GUI still draws the garrison cursor for sheep, which should not happen.

• The worker elephant can be garrisoned in temples, towers, outposts and other buildings that don't make sense (see #3611). It should be coherent with war-elephants, i.e. garrisonable only in buildings that make sense (#3135) or nowhere (#4030).

Due to these circumstances it seems appropriate to add a new boolean property IsGarrisonable to the templates which is true for all units except for those mentioned above.

[smitec]

So I have removed the garrison button from ships by checking if units have the class Ship but I feel this may be better handled in a way similar to guarding where <CanGarrison> becomes a tag in the templates.

[elexis]

Hi, thanks for your participation and looking at that big UnitAI maze!

After looking at your patch I added some more points to the ticket description on how this should be fixed.

With regards to your patch:

• Don't change UnitAI.canGarrison to work with and without argument. It would be more clear to add a new function IsGarrisonable instead that takes no argument. This should be implemented in the templates. Maybe UnitAI can be left untouched.

• Minor detail: it'd be more coherent if you use var cmpIdentity = ... and then check if (cmpIdentity.hasClass("Ship"))

[smitec]

Okay no worries I'll look into making those changes and moving the behaviour into the templates.

[smitec]

Added an updated diff that adds the CanGarrison template tag. I have added it to the applicable templates and testing seems to run okay. I disabled it for all fauna and decorative fauna for now. I'm not sure about the workings of the elephant situation but it should be easy enough to add the correct tags to the templates in question.

[smitec]

(not sure if I am supposed to add review back for the new patch, apologies if this isn't the right step, instructions didn't say)

[elexis]

Replying to smitec:

(not sure if I am supposed to add review back for the new patch, apologies if this isn't the right step, instructions didn't say)

Adding review is fine if you upload an updated patch.

---

Review: We discussed the patch and decided that adding a canGarrison flag to the templates is not ideal, since every unit might be garrisonable at some point and it also means having "canGarrison" logic in two complex components (UnitAI and GarrisonHolder). It is probably not nicely achievable to find out whether a given unit (ship, animal) is garrisonable in any of all buildings that can appear on a map. Hiding the button is likely not achievable. Sorry for changing the mind.

---

Todo: Updating the templates in those two places should still be done:

• Since animals can't be garrisoned due to UnitAI, the GarrisonHolder entry should be removed as well (I'll do that now).

• The worker elephant shouldn't be garrisonable in places that don't fit.

[elexis]

In 17648:

Adapt the template for corrals to the limitation in UnitAI.CanGarrison. Refs #3412.

[Imarok]

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

[Freagarach]

Following r23409, the UnitAI restriction to not allow animals to be garrisoned was lifted.