Page MenuHomeWildfire Games

{civ}.json style corrections
ClosedPublic

Authored by Nescio on Aug 24 2019, 7:44 PM.

Details

Reviewers
wraitii
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP23896: Civilisation .json style corrections
Summary

This patch corrects the indentation and format style of the {civ}.json files, per https://trac.wildfiregames.com/wiki/Coding_Conventions#JSON via this script by @fatherbushido:


See also D2000/rP23556.

Test Plan

Ought to be unproblematic

Event Timeline

Nescio created this revision.Aug 24 2019, 7:44 PM
Nescio edited the summary of this revision. (Show Details)Aug 24 2019, 7:47 PM
Nescio edited the summary of this revision. (Show Details)

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

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/479/display/redirect

Nescio updated this revision to Diff 10004.Sep 29 2019, 10:02 AM

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

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/349/display/redirect

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

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/865/display/redirect

Freagarach added inline comments.
binaries/data/mods/public/simulation/data/civs/mace.json
168–169

-

binaries/data/mods/public/simulation/data/civs/pers.json
168

-

binaries/data/mods/public/simulation/data/civs/rome.json
154–155

-

157

-

binaries/data/mods/public/simulation/data/civs/sele.json
190–191

-

binaries/data/mods/public/simulation/data/civs/spart.json
166–167

-

Nescio updated this revision to Diff 10009.Sep 29 2019, 12:52 PM
Nescio edited the summary of this revision. (Show Details)

" :":

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

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/353/display/redirect

Perhaps specify in the title/summary that you are not correcting text style?

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

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/869/display/redirect

elexis added a subscriber: elexis.Sep 29 2019, 6:53 PM
elexis added inline comments.
binaries/data/mods/public/simulation/data/civs/kush.json
136

Keeping each property on a separate line has the advantage that one can add additional properties easily and the data layout resembles a table, (i.e. it allows the reader to read one column at a time, i.e. only the property names, then find the property name interested in, and only then lookup the value of that property, as opposed to having to pass all keys and all values until the pair in one wanted to find is located).
This becomes more relevant with greater amounts of properties, but it's true for n=2 already and doing it so for n=2 means that it will be consistent with the n>=3 cases and allows to insert the 3rd property without changing the other lines.

Nescio added inline comments.Sep 29 2019, 7:26 PM
binaries/data/mods/public/simulation/data/civs/kush.json
136

With “property” you mean what's inside the braces and separated by commas?
I agree for n>=3, but these starting entities have only 1 or 2; moreover, the music (see above) have 2 and are on a single line each and modifications in aura and technology files have 2 or 3 properties and are on one line too, e.g.: https://trac.wildfiregames.com/browser/ps/trunk/binaries/data/mods/public/simulation/data/technologies/advanced_unit_bonus.json
The question is how likely it is that StartEntities will get additional properties.

Nescio removed a reviewer: bb.Jan 7 2020, 6:31 PM
Nescio updated this revision to Diff 11170.Jan 24 2020, 12:28 AM
Nescio edited the summary of this revision. (Show Details)

"SB1", "SB2"

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

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1647/display/redirect

Stan added a subscriber: Stan.Jan 24 2020, 9:50 AM

I assume there is no class for special buildings? (SB1 & SB2)

"SB1" and "SB2" appeared exactly once, in the maur.json file, and were obviously placeholders; other civs leave this string empty ("").
Inserting SpecialBuilding is rather meaningless, because the fact that they're “special” (i.e. not available to all civilizations) is the very reason they're listed in the civ files in the first place.

Stan added a comment.Jan 24 2020, 10:30 AM

Fair enough :)

Nescio planned changes to this revision.Jul 14 2020, 2:58 PM
Nescio updated this revision to Diff 12913.Jul 25 2020, 10:56 AM
Nescio edited the summary of this revision. (Show Details)
Nescio edited the test plan for this revision. (Show Details)
Nescio added a subscriber: fatherbushido.
Owners added a subscriber: Restricted Owners Package.Jul 25 2020, 10:56 AM

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

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2777/display/redirect

wraitii accepted this revision.Jul 25 2020, 11:16 AM

Loads, rather unproblematic files, and a common format seems a good idea.
Sure :) .

This revision is now accepted and ready to land.Jul 25 2020, 11:16 AM
This revision was automatically updated to reflect the committed changes.