[PATCH] Attack interval tooltip wrong

[elexis]

The heavy warship repeatedly fires a single shot if no catapults are garrisoned. But the tooltip shows that it fires 0 shots per second, which should be corrected.

[Sergey Kushnirenko]

See screenshot in attach which number of attack correct for this warship.

[elexis]

Where do you take that number 5 from? For an ungarrisoned ship, arrowCount and defaultArrowCount will be identical anyway.

If one reduces the trireme defaultArrowCount from 3 to 0, we can see that it still fires one arrow. So the arrowcount is wrong for all units that have both an Attack and BuildingAI component. For example the macedonian siege tower also has this problem.

The correct way to solve this issue is to add +1 in the tooltip function in this case.

[elexis]

Thanks leper and sanderd17 for explaining some background of the affected components.

Review: As mentioned before, this issue is not only affected the heavy warship but also other buildings:

• the macedonian siege tower (template_unit_mechanical_siege_tower). The tooltip says 0 arrows while it shoots one.
• trireme ship: it shoots 4 arrows instead of 3 if not garrisoned
• heavy warship: if no catapults garrisoned, it shoots 1 arrow while the tooltip says 0

So only changing the tooltip if there are catapults garrissoned is wrong.

---

Fixing the tooltip: According to sanderd17, the +1 arrow must be added to the tooltip if and only if the entity has a BuildingAI and a UnitAI component.

For example regular towers (template_structure_defense_defense_tower) actually shoot one arrow and the tooltip is correct (displays 1 arrow). This is because the tower doesn't have a UnitAI component (contrary to ships and siege).

If I understood correctly, the Attack component determines the attack stats and UnitAI and BuildingAI independently decide whether to attack.

Line 63 in tooltips.js is the right place to change, but you need to add the +1 if and only if the entity has a UnitAI component.

Since having this is one arrow is a weird behavior, we add this weird behavior to tooltips.js too. Therefore add a comment, both to tooltip.js that explains why we add it and a TODO comment for BuildingAI.prototype.FireArrows that informs other developers that the UnitAI component also shoots one arrow (if the entity has that component) and that this should not happen ideally.

---

(13:05:14) sanderd17: I've tried disableing that arrow before, but not really succeeded, as it's nested quite deep (and you should still have unitAI to task a unit to attack something, so you can't just ignore the attack from unitAI)

[elexis]

In 17783:

BuildingAI style fixes, refs #3763.
Remove a duplicate targetUnits-, some useless length-checks and unneeded comments.

[elexis]

In 17784:

Don't shoot an undocumented arrow from UnitAI if the entity also has a BuildingAI component shooting arrows. Fixes #3763.
Instead, add the arrow to the affected unit templates (except bireme and trireme, refs #3196).