Page MenuHomeWildfire Games

Minimum Distance Tooltip
ClosedPublic

Authored by elexis on Mar 27 2017, 1:49 PM.

Details

Summary

Displays the minimum range in the attack tooltips, if it exists. Split from D111. Fixes #4421.

Test Plan

Check it with towers and buildings that don't have a minimum range. Also test it with units.
Make sure the strings make sense, in particular the pluralization.
Check with elevation advantage too.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Vulcan added a subscriber: Vulcan.Mar 27 2017, 4:36 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/617/ for more details.

Gallaecio accepted this revision.Mar 28 2017, 7:15 PM
Gallaecio added a subscriber: Gallaecio.

Looks fine.

Some tips for you to consider, though:

  • It may be worth adding translation comments before those lines that are made of many placeholders providing simply an example sentence, similarly to what you do to translaate("meters").
  • It may make sense to use the plural function for both meter units, if for nothing else so that users can translate both in the same translation unit, just once.
This revision is now accepted and ready to land.Mar 28 2017, 7:15 PM
  • It may be worth adding translation comments before those lines that are made of many placeholders providing simply an example sentence, similarly to what you do to translaate("meters").

Done, indeed that will help people understanding these placeholder strings.

While at it, that "+" + relativeRange seems like it should use a translate+sprintf too.

  • It may make sense to use the plural function for both meter units, if for nothing else so that users can translate both in the same translation unit, just once.

Using translatePlural("meter", "meters", 2)), seems weird and I don't have a term to replce that 2 with (since those are two words like 5 to 6 meters).
Perhaps translators might want to use a different string here instead of reusing the plural form?

elexis updated this revision to Diff 990.Mar 29 2017, 3:27 AM

Add translation comments for the placeholder strings.
Add translate+sprintf for the +num.
Not sure about the "meters" string.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/630/ for more details.

Gallaecio: elexis: I agree about leaving meters separated after your comment, and the examples are perfect for translators.

This revision was automatically updated to reflect the committed changes.