add tooltip to unit_attack dialogs

[newfrenchy83]

in tooltip, the special and weapon_abilities are listed by stats (damage, attacks and chance to hit) and owner of each is specified.
replace #6399 for readibility purpose.

[stevecotton]

I can't follow the logic of this function - it seems that weapon_abilities is a temporary string that's used for assembling parts of other strings called self_abilities etc, and if that's what it is then I don't see why weapon_abilities is in scope for the whole function. I also can't see why it's an input parameter to the weapon_specials_*_self functions, as it seems to be an output from those.

However, this line uses it purely as an output, and IIUC the value set here would be discarded, were it not for the first weapon_specials_impl_self call taking it as an input, and then possibly concatenating other values to it before copying it to self_abilities. However, that happens in a conditional block that only happens if self_ is non-null (which might always be true, but then why is there a conditional block)?

[newfrenchy83]

weapon_abilities used mike reference,in add_name, i cannot put other stinrg in input or i put the code itself. I will try ths option.

[newfrenchy83]

@stevecotton i removed only_active argument to attack_type::weapon_specials()

[newfrenchy83]

@stevecotton PR rebased

[newfrenchy83]

@stevecotton it is long time what PR not reviewed.

[newfrenchy83]

@Vultraz it is a long time what this PR not reviewed, could you give an advice, see what this PR is a UI work.

[Pentarctagon]

Can you post a screenshot of what it looks like with this PR's change applied?

[newfrenchy83]

Can you post a screenshot of what it looks like with this PR's change applied?

teached: (cantor encoded in Elvish Captain and leadership)

teached by an enemy(diversion ability)

used by opponent(formation ability)

[newfrenchy83]

@Pentarctagon @Vultraz , i have a question, with abilities/specials showed in tooltip, show the weapon teached by enemy or used by opponent in attack prediction windows is useful, or must i show in tooltip only for not give heavy the window itself, and show in window weapon owned or teached by allies?

[Pentarctagon]

I'd say to keep it just in the tooltip for now. Given how long this PR has already been open already, let's not add more to it.

[newfrenchy83]

I'd say to keep it just in the tooltip for now. Given how long this PR has already been open already, let's not add more to it.

except what 'diversion' and formation both in window in this PR, must becommit a remove of that or don't commit anymore?

[newfrenchy83]

@Pentarctagon i reformulate my question: 'diversion' and 'formation' name must appear in window(without mention of origin) or not? And if not, must commit that in this PR or i another PR after merging in master?

[Pentarctagon]

I think it'd be best to see what vultraz says first.

Just as far as wording goes though, for correctness:

• "teached" should be changed to "taught" in the above screenshots.
• "enemy" is misspelled as "ennemy".
• "others aspect" should be "other aspects".

[newfrenchy83]

I think it'd be best to see what vultraz says first.

Just as far as wording goes though, for correctness:

* "teached" should be changed to "taught" in the above screenshots.

* "enemy" is misspelled as "ennemy".

* "others aspect" should be "other aspects".

i post screenshot of modification in done in my local:
taught(leadership and cantor)

taught by enemy(divesion in tooltip only):

used by opponent(formation):

if @Vultraz is ok, then i commit change, or changes acceptable.

[newfrenchy83]

@Pentarctagon excuse my lack of patience but i would post remording in PR, and i let what weapon owned or taught by allies in window.

[stevecotton]

I apologise for how long it's taken me to review this, and thank @Vultraz for reviewing the UI.

My plan is to merge this evening, unless anyone has more to say. If I have any more ideas about possible refactoring, I'll create a new PR on top of this instead of blocking this PR.