Page MenuHomeWildfire Games

team bonus descriptions consistency
Needs RevisionPublic

Authored by Nescio on Dec 30 2018, 5:46 PM.

Details

Reviewers
Gallaecio
Summary

The descriptions of team bonuses were sometimes incorrect and sentences were often longer than necessary. This patch corrects mistakes and implements a standard format: [player] [class] [change] [attributes]. E.g.: "Allied warships −25% construction time." instead of "Ships construct 25% faster."
It also updates the team bonus descriptions listed in the civ.json files.

Test Plan

Check for typos.

Diff Detail

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

Event Timeline

Nescio created this revision.Dec 30 2018, 5:46 PM
Nescio updated this revision to Diff 7149.Dec 30 2018, 5:55 PM

BuildTime according to https://trac.wildfiregames.com/wiki/EnglishStyleGuide#Actions

  • ships and siege engines are constructed
  • structures are built
  • technologies are researched
  • units are trained
Nescio edited the summary of this revision. (Show Details)Dec 30 2018, 5:56 PM
Vulcan added a subscriber: Vulcan.Dec 30 2018, 5:56 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/857/

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/859/

Stan added a subscriber: Gallaecio.EditedDec 30 2018, 6:08 PM

Sounds pretty cool. Pinging @Gallaecio, @bb, and @elexis as it's still a big enough patch :)

Stan added subscribers: bb, elexis.Dec 30 2018, 6:09 PM
Nescio added a comment.Jan 5 2019, 8:50 PM

Big enough? One line for each civ, two files each.

Nescio updated this revision to Diff 7595.Tue, Mar 19, 12:37 PM

Updated because of rP22133.

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1121/display/redirect

Gallaecio requested changes to this revision.Sun, Mar 24, 2:26 PM

I really like the change, I just have some minor feedback for specific lines.

binaries/data/mods/public/simulation/data/auras/teambonuses/mace_player_teambonus.json
12

Shouldn’t it be ‘Allied markets +20% sell prices.’?

binaries/data/mods/public/simulation/data/auras/teambonuses/maur_player_teambonus.json
10

I would prefer this unrelated change not to be part of the patch, but if it does, I think this line should be last, assuming the new order is alphabetical.

binaries/data/mods/public/simulation/data/auras/teambonuses/sele_player_teambonus.json
12

Centers

binaries/data/mods/public/simulation/data/civs/mace.json
85

Same as above.

binaries/data/mods/public/simulation/data/civs/sele.json
84

Same as above.

This revision now requires changes to proceed.Sun, Mar 24, 2:26 PM
Nescio added inline comments.Sun, Mar 24, 3:27 PM
binaries/data/mods/public/simulation/data/auras/teambonuses/mace_player_teambonus.json
12

No, that would imply this aura modifies the market, which isn't the case; barter prices are independent from specific structures.
Perhaps it would be better to omit the "at market", e.g. "Allies +20% barter sell prices." or something. What do you think?

binaries/data/mods/public/simulation/data/auras/teambonuses/maur_player_teambonus.json
10

Attributes are typically listed in alphabetical order, but resources have are listed in the way they are numbered and displayed in-game (f, w, m, s, t).

It is included in this patch because it's related. To start with,the current order of modifications is inconsistent with the practice of other teambonuses (e.g. brit), auras, technologies, and templates.
More importantly, what this patch does is make the description follow the modifications; in this case first the temple cost, then the tech cost multiplier.

binaries/data/mods/public/simulation/data/auras/teambonuses/sele_player_teambonus.json
12

You're right, I'll correct this.