Page MenuHomeWildfire Games

every technology modification on a new line
ClosedPublic

Authored by Nescio on Jun 5 2019, 11:37 AM.

Details

Summary

This patch puts every modification in every technology .json file on a new line, as requested by @Stan in D1950.
This is already the case for all aura files.

[EDIT] Furthermore:

  • corrects space/tab consistency.
  • standardize spaces between [, {, }, ]
  • remove unnecessary .0 from armour modifications
Test Plan

Check for mistakes.

Diff Detail

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

Event Timeline

Nescio created this revision.Jun 5 2019, 11:37 AM

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

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

Nescio updated this revision to Diff 8317.Jun 5 2019, 11:55 AM
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/differential/1610/display/redirect

bb added a subscriber: bb.Jun 5 2019, 8:37 PM

standardize { "value": } to {"value":}

Well no, { "key": value } is the convention in js, so also in the json...

Nescio added a comment.Jun 5 2019, 8:50 PM

Is it? Good to know. But shouldn't it then also for cost, requirements, and affects?
E.g. upgrade_rank_elite_infantry.json:

{
	"genericName": "Elite Citizen-Infantry",
	"specificName": {
		"mace": "Pentakosiomédimnoi",
		"spart": "Pentakosiomédimnoi",
		"athen": "Pentakosiomédimnoi"
	},
	"description": "Upgrade all of your citizen-soldier infantrymen to Elite rank.",
	"cost": {"food": 500, "wood": 0, "stone": 0, "metal": 500},
	"requirements": {"tech": "phase_city"},
	"requirementsTooltip": "Unlocked in City Phase.",
	"icon": "upgrade_elite.png",
	"researchTime": 40,
	"supersedes": "upgrade_rank_advanced_infantry",
	"tooltip": "Upgrade all of your citizen-soldier infantrymen to Elite rank. This increases their military prowess, but decreases their resource gathering rates another -25%.",
	"modifications": [
		{"value": "Promotion/RequiredXp", "replace": 0}
	],
	"affects": ["Infantry Advanced"],
	"soundComplete": "interface/alarm/alarm_upgradearmory.xml"
}

or should it be:

{
	"genericName": "Elite Citizen-Infantry",
	"specificName": {
		"mace": "Pentakosiomédimnoi",
		"spart": "Pentakosiomédimnoi",
		"athen": "Pentakosiomédimnoi"
	},
	"description": "Upgrade all of your citizen-soldier infantrymen to Elite rank.",
	"cost": { "food": 500, "wood": 0, "stone": 0, "metal": 500 },
	"requirements": { "tech": "phase_city" },
	"requirementsTooltip": "Unlocked in City Phase.",
	"icon": "upgrade_elite.png",
	"researchTime": 40,
	"supersedes": "upgrade_rank_advanced_infantry",
	"tooltip": "Upgrade all of your citizen-soldier infantrymen to Elite rank. This increases their military prowess, but decreases their resource gathering rates another -25%.",
	"modifications": [
		{ "value": "Promotion/RequiredXp", "replace": 0 }
	],
	"affects": [ "Infantry Advanced" ],
	"soundComplete": "interface/alarm/alarm_upgradearmory.xml"
}
bb added a comment.Jun 5 2019, 8:56 PM

it "should", but noone was ever crazy enough to spend some hours/days/weeks/month/years/centuries adding all the spaces

Nescio updated this revision to Diff 8339.Jun 5 2019, 11:27 PM
Nescio edited the summary of this revision. (Show Details)

Here you go, now with correct spacing.

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

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

Nescio updated this revision to Diff 8524.Jun 16 2019, 8:48 PM

Updated because of rP22379

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

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

Nescio edited reviewers, added: bb; removed: Stan.Jun 18 2019, 4:53 PM
bb accepted this revision.Jun 28 2019, 5:53 PM

Completeness check forced me to go through all the tech files, so fixed the array spacings and trailing 0's. Grepped for .0, found a couple more and fixed. Units_demo map and structree don't reveal any issues

binaries/data/mods/public/simulation/data/technologies/advanced_unit_bonus.json
28

arrays (stuff with [...]) does not have the spaces, only objects (stuff with {...})

binaries/data/mods/public/simulation/data/technologies/archery_tradition.json
4

got wondering why we add that "stone": 0 here, instead of having a 0 default

11

.0

binaries/data/mods/public/simulation/data/technologies/melee_inf_sidearms.json
14

this sounds wrong: any civ with the town phase can have this tech, should be { "all": [{ "tech": "phase_town" }, { "any": [{ "civ": "brit" }, { "civ": "gaul" }] }] } doesn't cause any harm now since the tech is unused, anyway out of scope

20

.0

binaries/data/mods/public/simulation/data/technologies/melee_inf_spearheads.json
18

.0

binaries/data/mods/public/simulation/data/technologies/trade_commercial_treaty.json
11

-0

binaries/data/mods/public/simulation/data/technologies/trade_gain_01.json
11

-0

binaries/data/mods/public/simulation/data/technologies/unlock_champion_units.json
11

we might want to add some newlines in this unreadable one-liners too see "upgrade_advanced_rank_mercenary"

This revision is now accepted and ready to land.Jun 28 2019, 5:53 PM
This revision was automatically updated to reflect the committed changes.
bb added a comment.Jun 28 2019, 6:05 PM

I see where the confusion about the array spacing came from, my bad, should have seen it in your code example, anyway thanks for the patch

Thank you for reviewing and committing this, I appreciate it! Are you interested in D2000 and D2025 as well?