Many of the range strings are reported on transifex and automatic ones could be good.
That information is now less relevant as we have range visualization but it still relevant (at least for structure tree).
I'll nuke the related string in another diff.
Details
Details
- Reviewers
elexis - Commits
- rP19544: Aura Range tooltip. Reviewed by elexis.
entity panel
struct tree
training panel
hero panel
Check if translation is done correctly.
I was getting headache with the arrow function so I write it as such but could be written without the explicit loop.
Gui formatting is bad too I guess.
Diff Detail
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
Looks good. But similarly to that old spread tooltip proposal, we run into the issue that the line (for example for Boudica) becomes so long that the line will split to 2 lines.
So we end up with:
John Doe Hero Aura: all champions in range +3 attack damage. Range: 30 meters Attack: 40 pierce, Range: 72 meters
but that is already the case with languages whose translation of the string is longer than the english one.
An alternative (which might avoid repeating this issue here) might be "Hero Aura: All champions within %(auraRange)s +3 attack damage".
This has the disadvantage that the number is embedded.
So at some point I guess we want to use sprintf for all modifyable template attributes.
Like "Hero Aura: +3 Pierce, Range: 30 meters, Affects: Champion" or "Some Hero: +40 Minimum Distance, Affects: Fortress".
But the proposal works on its own and won't have to be rewritten from scratch if we would implement the latter.
So only requesting indentation, Range, auraID, " ", unless someone else has some opinion too.
| binaries/data/mods/public/gui/common/tooltips.js | ||
|---|---|---|
| 626 ↗ | (On Diff #1771) | Should use the same identifier as in the GuiInterface / Templates.js. 3x auraID would be better than 1x aura 1x name and 1x auraID |
| 634 ↗ | (On Diff #1771) | Agree with this simple check, because the user won't be interested in auras with 0 meters effect range until we would start doing things like changing the aura range with a tech or aura (which we can legitimately exclude at this point). Missing indentation 1 tab for these 8 lines |
| 636 ↗ | (On Diff #1771) | tooltip += " " + sprintf |
| 637 ↗ | (On Diff #1771) | Just translateWithContext("aura", "Range:") ? |
Comment Actions
Build is green
Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
http://jw:8080/job/phabricator/1092/ for more details.
Comment Actions
Thanks for this great patch!
Tested again and works as it should and is complete.
We should remove cunobelin, catafalque, monument range strings after this patch.
| binaries/data/mods/public/gui/common/tooltips.js | ||
|---|---|---|
| 636 ↗ | (On Diff #1787) | Was wondering whether we should end with a period, but since the other tooltips don't do that, won't do it here either I guess |
| 637 ↗ | (On Diff #1787) | Didn't mean to remove the font, but that's actually ok and addresses also that issue where the label appears in the next line. |
Comment Actions
Build is green
Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
http://jw:8080/job/phabricator/1102/ for more details.