Complete Garrisoning system functionality

[Kieran P]

With the recent addition of basic garrisoning, it'd be good to have this functionality finished off. Excluding artwork related items (e.g. propping) which will be handled later, here is some things left to do:

Enhancements

* [DONE] Infantry units which are garrisoned should be able to attack nearby enemies, within their normal range. Don't worry about being able to see the unit, just seeing the arrow from the building to the enemy and hitting it will be enough. The amount of arrows is equal to the amount of infantry units garrisoned. 10 infantry garrisoned == 10 arrows a second.

* [DONE] Need to be able to garrison units without using the CTRL key. A garrison button next to the kill button in the unit picture panel, which which clicked, makes the next left click garrison all selected units to it.

* [DONE] When a unit isn't able to be garrisoned into the building, then show a greyed out version of the cursor when holding down CTRL.

* [DONE] Garrisoning needs a new 'out' icon. It's currently pointing into the door, but the text says 'Unload-all'. The icon should have the arrow point away from the door.

* [MOVED TO #817] Need a way to tell if a building has garrisoned units, and how many, without having to click on it. Maybe a count next to the health bar, or a flag on the top of buildings?

Bug Fixes

* [DONE] Not all units are going to the rally flag for the building when they are all unloaded. Unloading one at a time works.

* [DONE] When more than one unit of a type is garrisoned, the garrison buttons may change position every time a unit of that type is unloaded. This occurs when units of the same type are spread throughout the garrison list. This list may need to be sorted to prevent this from happening. (Unit selections would also suffer from this problem if selections that contained multiple units of the same type allowed removing a single unit. However, the selection buttons remove the entire group so this problem is not seen there.)

* [DONE] The garrison icon when holding down CTRL (or using the garrison button in the units panel) doesn't grey out when that unit can be garrisoned, and you hover over a building which does support garrisoning, but the combination of the two (building and unit) is unsupported. Example: it shows coloured garrison icon when you try to put a horse into a tower, which is not allowed.

* [MOVED TO #818] When units are garrisoned in a ship, they do not increase the amount of arrows fired from it.

* [MOVED TO #818] When units are garrisoned in a ship, they can currently be ungarrisoned over deep water, which causes them to be stuck there.

[fabio]

Also garrisoning should be done without pressing the CTRL key. This would be more consistent with hunting/gathering. I wasn't able to try it until I read this ticket and noticed the mention of the CTRL key.

[fabio]

Also there should be a way to see if the building is garrisoned without clicking on it (maybe a number next to health bar). Or maybe just wait for the graphic feedback (show units inside towers, ...).

[Kieran P]

Good suggestions. I've added them to the main description.

[Michael D. Hafer]

I see no problem with the Ctrl key garrisoning. There will also be the UI icon available. A simple right-click should repair the building, while Ctrl-right click can garrison. Just my opinion.

[Michael D. Hafer]

Perhaps this needs a new ticket, but garrisoning for ships needs updated as well.

[Michael D. Hafer]

Just a random thought about siege garrisoning. It would be cool for civilizations with ranged siege (Greeks, Carthage, Rome) to have a technology that allows them to garrison towers and fortresses with ranged siege, thus providing extra defensive firepower.

[Philip Taylor]

Re r8636:

GarrisonHolder.prototype.AllowedToGarrison: classNotAllowed is declared but not used.

BuildingAI.prototype.SetupRangeQuery: Needs to do an "if (this.enemyUnitsQuery) cmpRangeManager.DestroyActiveQuery(this.enemyUnitsQuery);", else it will end up with two (or more) queries after changing ownership.

BuildingAI.prototype.OnRangeUpdate: You shouldn't always use ResetActiveQuery here, since that's inefficient (it'll be recomputing the range query twice whenever someone enters). OnRangeUpdate will be passed the list of newly-in-range entities in msg.added and newly-out-of-range in msg.removed. It should probably check if added is non-empty and there is no targetUnit yet, then pick the first added as targetUnit. Otherwise if removed is non-empty and targetUnit is inside that list, then it's lost the target, so do ResetActiveQuery and pick a new one. Otherwise just stick with the current target.

Timer can't guarantee executing at precisely the correct time - it only runs once per simulation turn so it might be a bit late. That means FireArrows can never be called more than once per turn, even if the requested interval is much shorter than that. I guess we ought to extend Timer so it can support true repeating timers, automatically running the handler multiple times if it's too fast.

[Kieran P]

Shifted some things around, marked some items as done, and added one new bug, which is:

• The garrison icon when holding down CTRL (or using the garrison button in the units panel) does doesn't grey out when that unit can be garrisoned, and you hover over a building which does support garrisoning, but the combination of the two (building and unit) is unsupported. Example: it shows coloured garrison icon when you try to put a horse into a tower, which is not allowed.

If you can get that fixed before Alpha 3 releases, then we can push the rest of the required fixed back to Alpha 4 :-)

[Kieran P]

Additionally, it'd also be good to show a greyed out garrison icon when the garrisonable building is full of units.

[Kieran P]

Added two more bugs related to ship garrisoning.

[historic_bruno]

What's the expected behavior for unloading ships? The button could simply be disabled if no land is near, but that seems clunky, and perhaps annoying if you have many ships about to invade an enemy island.

Another option is two step unloading: click the button first and get a special "unload here" cursor, then click the desired location for unloading. If you choose a bad location the command could be ignored.

[Kieran P]

The second way matches that of AOE2, which works well, and makes the most sense to new users.

[Erik Johansson]

I think at least EE2 did it the same way as well, so yeah, option two sounds good to me and is as far as I know familiar and works well as far as I can tell.

[Erik Johansson]

• Need a way to tell if a building has garrisoned units, and how many, without having to click on it. Maybe a count next to the health bar, or a flag on the top of buildings?

Not added for all buildings, but implemented already, at least some of the Hellenic buildings have the flag on them while units are garrisoned inside, so I think it would be fine to mark as done. New buildings will have to get it added as they are added, and I don't think it's worth holding off this ticket from being closed until all buildings are done/updated/etc.

That doesn't say how many, but is that really needed?

[Kieran P]

It's still pretty important to have the flag on existing buildings. None of the Iberian buildings will have the flag for example.

I'll assign this to Michael, maybe he can get onto it.

[fabio]

That doesn't say how many, but is that really needed?

What about changing the flag raising based on the number of garrisoned units? Something like:

• flag at base position: < 50% garrisoned units
• flag at mid: >= 50% garrisoned units
• flag at top: all units garrisoned

[michael]

Fabio has an awesome idea with the flags at half-mast, etc.

As far as the ship garrisoning, I think that is worth its own Ticket and can be moved out of this ticket. I think the half-mast flag thing can be its own as well. It feels like a much lower priority than the other features.

[Kieran P]

Agreed. I've split the remaining issues into new tickets.