Page MenuHomeWildfire Games

Remove reference to range in the aura description as it's misleading or redundant with range tooltip.
ClosedPublic

Authored by Grugnas on May 9 2017, 3:23 PM.

Details

Summary

Some aura description string like "in his range" are reported on transifex (misleading between aura or vision...).
Moreover with the range tooltip (and also the visualization range) the numerical information is to remove but even without any number reference it appears to be redundant.

Test Plan

Check. I don't want to spend time on this, so feel free to commandeer.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 1647
Build 2603: Vulcan BuildJenkins
Build 2602: arc lint + arc unit

Event Timeline

fatherbushido created this revision.May 9 2017, 3:23 PM
Vulcan added a subscriber: Vulcan.May 9 2017, 4:52 PM

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/1103/ for more details.

Grugnas added inline comments.
binaries/data/mods/public/simulation/data/auras/units/heroes/athen_hero_pericles_1.json
9

same "+15% building rate and repair rate for workers."

binaries/data/mods/public/simulation/data/auras/units/heroes/cart_hero_hannibal.json
16

I'd use "20% attack and +1 capture for soldiers, siege engines and allied units."

binaries/data/mods/public/simulation/data/auras/units/heroes/cart_hero_maharbal.json
11

I'd use "+20% attack for cavalry melee units."

binaries/data/mods/public/simulation/data/auras/units/heroes/ptol_hero_cleopatra_1.json
10

remove "Egyptian"

binaries/data/mods/public/simulation/data/auras/units/heroes/ptol_hero_ptolemy_I_1.json
9

I'd use "+10% repair rate and build rate for workers."

binaries/data/mods/public/simulation/data/auras/units/heroes/rome_hero_marcellus.json
15

I'd remove the "Roman" word as it could be misleading

Grugnas: you can commandeer it and do the said modifications

binaries/data/mods/public/simulation/data/auras/units/heroes/cart_hero_hannibal.json
16

we can think it's for all allied units and not all our own ones

binaries/data/mods/public/simulation/data/auras/units/heroes/cart_hero_maharbal.json
11

ok

binaries/data/mods/public/simulation/data/auras/units/heroes/ptol_hero_cleopatra_1.json
10

yes

binaries/data/mods/public/simulation/data/auras/units/heroes/ptol_hero_ptolemy_I_1.json
9

yes

binaries/data/mods/public/simulation/data/auras/units/heroes/rome_hero_marcellus.json
15

yes

bb added a subscriber: bb.May 11 2017, 12:14 AM
bb added inline comments.
binaries/data/mods/public/simulation/data/auras/structures/iber_monument.json
14

"in aura"??? nuke

fatherbushido added inline comments.May 11 2017, 6:48 AM
binaries/data/mods/public/simulation/data/auras/structures/iber_monument.json
14

yes

Check. I don't want to spend time on this, so feel free to commandeer.
-> already two candidates :)

fatherbushido added inline comments.May 11 2017, 8:48 AM
binaries/data/mods/public/simulation/data/auras/structures/iber_monument.json
14

Remove "all iberian" too.
(else Iberian)

Grugnas commandeered this revision.May 11 2017, 2:53 PM
Grugnas added a reviewer: fatherbushido.
Grugnas updated this revision to Diff 1846.May 11 2017, 2:56 PM

i thught that using uppercases for classes and entries affected would make the reading easier. Attack Damage is used now instead of Attack to specify the only attack type value affected and not a possible Attack Repeat Time aswell.

Owners added a subscriber: Restricted Owners Package.May 11 2017, 2:56 PM
elexis added a subscriber: elexis.May 11 2017, 4:20 PM

The capitalization should be kept in sync with the rest of the xml and josn strings though.
It uses common nouns almost everywhere. If we want to use proper nouns in some cases okay, but then it should be globally consistent.
(And then we still should figure out why soldier isn't a proper noun but Attack Damage is)

fatherbushido edited edge metadata.May 11 2017, 6:02 PM
In D464#19192, @Grugnas wrote:

i thught that using uppercases for classes and entries affected would make the reading easier. Attack Damage is used now instead of Attack to specify the only attack type value affected and not a possible Attack Repeat Time aswell.

Could you revert the last diff and change only the relevant things?

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/1142/ for more details.

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/1143/ for more details.

Grugnas updated this revision to Diff 1852.May 11 2017, 9:10 PM

reupload

fatherbushido added inline comments.May 11 2017, 9:57 PM
binaries/data/mods/public/simulation/data/auras/units/heroes/cart_hero_hannibal.json
16

extra space

binaries/data/mods/public/simulation/data/auras/units/heroes/spart_hero_leonidas.json
12

extra spaces
you could let spearmen, spear soldiers ok too

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/1148/ for more details.

Grugnas updated this revision to Diff 1865.May 12 2017, 10:07 AM

spaces fix

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/1157/ for more details.

fatherbushido accepted this revision.May 12 2017, 7:07 PM
This revision is now accepted and ready to land.May 12 2017, 7:07 PM
This revision was automatically updated to reflect the committed changes.