Page MenuHomeWildfire Games

Standardize "Territory Influence" in tooltips
Needs ReviewPublic

Authored by Krinkle on Jun 22 2019, 9:33 PM.

Details

Reviewers
None
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Owners Package(Owns No Changed Paths)
Trac Tickets
#4259
Summary

Players can now find out which buildings provide territory rooting. Most buildings that provide rooting (such as Civic Centre, Military Colony, and Wonder) did not previously show this information to players in any way.

With the exception of two rare buildings (Iberian monument, and Maurian pillar), because these had this information hardcoded in their description.

  • Added parsing for the <TerritoryInfluence> element to the template loader.
  • Added a new tooltip function that can return a "Territory" heading with a brief explanation of this feature.
  • Removed the hardcoded text for the two buildings that had a similar explanation hardcoded in their description.

Fixes #4259.

Before (trunk)After (with this patch)
Test Plan
  • Start a single player match as Iberians.
  • With a citizen soldier selected, the construction panel for "Civic Centre" and "Fortress" now show the new "Territory" caption. The "Revered Monument" building no longer shows the old caption, and also features the new "Territory" caption.
  • The new "Territory" caption is also shown when hovering the Structure Tree icon, and in the right-click "More information" dialog.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
arcpatch_D2005 (branched from master)
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 10193
Build 17296: Vulcan BuildJenkins
Build 17295: Vulcan Build (Windows)Jenkins
Build 17294: arc lint + arc unit

Event Timeline

Krinkle created this revision.Jun 22 2019, 9:33 PM
Owners added a subscriber: Restricted Owners Package.Jun 22 2019, 9:33 PM
This comment was removed by Krinkle.

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (operator-linebreak):
|    | '||' should be placed at the end of the line.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/globalscripts/Templates.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/globalscripts/Templates.js
|  87|  87| 		// If the elements are still strings, split them by space or by '+'
|  88|  88| 		if (typeof sublist == "string")
|  89|  89| 			sublist = sublist.split(/[+\s]+/);
|  90|    |-		if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1)
|  91|    |-		                    || (c[0] != "!" && classes.indexOf(c) != -1)))
|    |  90|+		if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1) ||
|    |  91|+		                    (c[0] != "!" && classes.indexOf(c) != -1)))
|  92|  92| 			return true;
|  93|  93| 	}
|  94|  94| 
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/globalscripts/Templates.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/globalscripts/Templates.js
| 314| 314| 	}
| 315| 315| 
| 316| 316| 	if (template.TerritoryInfluence)
| 317|    |-	{
|    | 317|+	
| 318| 318| 		ret.territoryInfluence = {
| 319| 319| 			// false if undefined
| 320| 320| 			// Should match match logic of CCmpTerritoryInfluence.
| 321| 321| 			"root": template.TerritoryInfluence.Root === "true"
| 322| 322| 		};
| 323|    |-	}
|    | 323|+	
| 324| 324| 
| 325| 325| 	if (template.Heal)
| 326| 326| 		ret.heal = {

binaries/data/mods/public/globalscripts/Templates.js
|  91| »   »   ····················||·(c[0]·!=·"!"·&&·classes.indexOf(c)·!=·-1)))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
| 392| 392| function getRepairTimeTooltip(entState)
| 393| 393| {
| 394| 394| 	return sprintf(translate("%(label)s %(details)s"), {
| 395|    |-			"label": headerFont(translate("Number of repairers:")),
|    | 395|+		"label": headerFont(translate("Number of repairers:")),
| 396| 396| 			"details": entState.repairable.numBuilders
| 397| 397| 		}) + "\n" + (entState.repairable.numBuilders ?
| 398| 398| 		sprintf(translatePlural(
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
| 393| 393| {
| 394| 394| 	return sprintf(translate("%(label)s %(details)s"), {
| 395| 395| 			"label": headerFont(translate("Number of repairers:")),
| 396|    |-			"details": entState.repairable.numBuilders
|    | 396|+		"details": entState.repairable.numBuilders
| 397| 397| 		}) + "\n" + (entState.repairable.numBuilders ?
| 398| 398| 		sprintf(translatePlural(
| 399| 399| 			"Add another worker to speed up the repairs by %(second)s second.",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
| 394| 394| 	return sprintf(translate("%(label)s %(details)s"), {
| 395| 395| 			"label": headerFont(translate("Number of repairers:")),
| 396| 396| 			"details": entState.repairable.numBuilders
| 397|    |-		}) + "\n" + (entState.repairable.numBuilders ?
|    | 397|+	}) + "\n" + (entState.repairable.numBuilders ?
| 398| 398| 		sprintf(translatePlural(
| 399| 399| 			"Add another worker to speed up the repairs by %(second)s second.",
| 400| 400| 			"Add another worker to speed up the repairs by %(second)s seconds.",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
| 414| 414| function getBuildTimeTooltip(entState)
| 415| 415| {
| 416| 416| 	return sprintf(translate("%(label)s %(details)s"), {
| 417|    |-			"label": headerFont(translate("Number of builders:")),
|    | 417|+		"label": headerFont(translate("Number of builders:")),
| 418| 418| 			"details": entState.foundation.numBuilders
| 419| 419| 		}) + "\n" + (entState.foundation.numBuilders ?
| 420| 420| 		sprintf(translatePlural(
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
| 415| 415| {
| 416| 416| 	return sprintf(translate("%(label)s %(details)s"), {
| 417| 417| 			"label": headerFont(translate("Number of builders:")),
| 418|    |-			"details": entState.foundation.numBuilders
|    | 418|+		"details": entState.foundation.numBuilders
| 419| 419| 		}) + "\n" + (entState.foundation.numBuilders ?
| 420| 420| 		sprintf(translatePlural(
| 421| 421| 			"Add another worker to speed up the construction by %(second)s second.",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
| 416| 416| 	return sprintf(translate("%(label)s %(details)s"), {
| 417| 417| 			"label": headerFont(translate("Number of builders:")),
| 418| 418| 			"details": entState.foundation.numBuilders
| 419|    |-		}) + "\n" + (entState.foundation.numBuilders ?
|    | 419|+	}) + "\n" + (entState.foundation.numBuilders ?
| 420| 420| 		sprintf(translatePlural(
| 421| 421| 			"Add another worker to speed up the construction by %(second)s second.",
| 422| 422| 			"Add another worker to speed up the construction by %(second)s seconds.",
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
| 649| 649| function getTerritoryInfluenceTooltip(template)
| 650| 650| {
| 651| 651| 	let text = "";
| 652|    |-	if (template.territoryInfluence && template.territoryInfluence.root) {
|    | 652|+	if (template.territoryInfluence && template.territoryInfluence.root) 
| 653| 653| 		text += sprintf(translate("%(label)s %(text)s"), {
| 654| 654| 			"label": headerFont(translate("Territory Influence:")),
| 655| 655| 			"text": translate("Other buildings in this building's do not decay.")
| 656| 656| 		});
| 657|    |-	}
|    | 657|+	
| 658| 658| 	return text
| 659| 659| }
| 660| 660| 
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
| 655| 655| 			"text": translate("Other buildings in this building's do not decay.")
| 656| 656| 		});
| 657| 657| 	}
| 658|    |-	return text
|    | 658|+	return text;
| 659| 659| }
| 660| 660| 
| 661| 661| /**

binaries/data/mods/public/gui/common/tooltips.js
| 652| »   if·(template.territoryInfluence·&&·template.territoryInfluence.root)·{
|    | [NORMAL] ESLintBear (brace-rules/brace-on-same-line):
|    | Opening curly brace appears on the same line as controlling statement.

binaries/data/mods/public/gui/common/tooltips.js
| 658| »   return·text
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.
Executing section cli...

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

Krinkle updated this revision to Diff 8580.Jun 22 2019, 10:05 PM
Krinkle edited the summary of this revision. (Show Details)
Krinkle edited the test plan for this revision. (Show Details)

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (operator-linebreak):
|    | '||' should be placed at the end of the line.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/globalscripts/Templates.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/globalscripts/Templates.js
|  87|  87| 		// If the elements are still strings, split them by space or by '+'
|  88|  88| 		if (typeof sublist == "string")
|  89|  89| 			sublist = sublist.split(/[+\s]+/);
|  90|    |-		if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1)
|  91|    |-		                    || (c[0] != "!" && classes.indexOf(c) != -1)))
|    |  90|+		if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1) ||
|    |  91|+		                    (c[0] != "!" && classes.indexOf(c) != -1)))
|  92|  92| 			return true;
|  93|  93| 	}
|  94|  94| 
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/globalscripts/Templates.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/globalscripts/Templates.js
| 314| 314| 	}
| 315| 315| 
| 316| 316| 	if (template.TerritoryInfluence)
| 317|    |-	{
|    | 317|+	
| 318| 318| 		ret.territoryInfluence = {
| 319| 319| 			// false if undefined
| 320| 320| 			// Should match match logic of CCmpTerritoryInfluence.
| 321| 321| 			"root": template.TerritoryInfluence.Root === "true"
| 322| 322| 		};
| 323|    |-	}
|    | 323|+	
| 324| 324| 
| 325| 325| 	if (template.Heal)
| 326| 326| 		ret.heal = {

binaries/data/mods/public/globalscripts/Templates.js
|  91| »   »   ····················||·(c[0]·!=·"!"·&&·classes.indexOf(c)·!=·-1)))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
| 392| 392| function getRepairTimeTooltip(entState)
| 393| 393| {
| 394| 394| 	return sprintf(translate("%(label)s %(details)s"), {
| 395|    |-			"label": headerFont(translate("Number of repairers:")),
|    | 395|+		"label": headerFont(translate("Number of repairers:")),
| 396| 396| 			"details": entState.repairable.numBuilders
| 397| 397| 		}) + "\n" + (entState.repairable.numBuilders ?
| 398| 398| 		sprintf(translatePlural(
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
| 393| 393| {
| 394| 394| 	return sprintf(translate("%(label)s %(details)s"), {
| 395| 395| 			"label": headerFont(translate("Number of repairers:")),
| 396|    |-			"details": entState.repairable.numBuilders
|    | 396|+		"details": entState.repairable.numBuilders
| 397| 397| 		}) + "\n" + (entState.repairable.numBuilders ?
| 398| 398| 		sprintf(translatePlural(
| 399| 399| 			"Add another worker to speed up the repairs by %(second)s second.",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
| 394| 394| 	return sprintf(translate("%(label)s %(details)s"), {
| 395| 395| 			"label": headerFont(translate("Number of repairers:")),
| 396| 396| 			"details": entState.repairable.numBuilders
| 397|    |-		}) + "\n" + (entState.repairable.numBuilders ?
|    | 397|+	}) + "\n" + (entState.repairable.numBuilders ?
| 398| 398| 		sprintf(translatePlural(
| 399| 399| 			"Add another worker to speed up the repairs by %(second)s second.",
| 400| 400| 			"Add another worker to speed up the repairs by %(second)s seconds.",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
| 414| 414| function getBuildTimeTooltip(entState)
| 415| 415| {
| 416| 416| 	return sprintf(translate("%(label)s %(details)s"), {
| 417|    |-			"label": headerFont(translate("Number of builders:")),
|    | 417|+		"label": headerFont(translate("Number of builders:")),
| 418| 418| 			"details": entState.foundation.numBuilders
| 419| 419| 		}) + "\n" + (entState.foundation.numBuilders ?
| 420| 420| 		sprintf(translatePlural(
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
| 415| 415| {
| 416| 416| 	return sprintf(translate("%(label)s %(details)s"), {
| 417| 417| 			"label": headerFont(translate("Number of builders:")),
| 418|    |-			"details": entState.foundation.numBuilders
|    | 418|+		"details": entState.foundation.numBuilders
| 419| 419| 		}) + "\n" + (entState.foundation.numBuilders ?
| 420| 420| 		sprintf(translatePlural(
| 421| 421| 			"Add another worker to speed up the construction by %(second)s second.",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
| 416| 416| 	return sprintf(translate("%(label)s %(details)s"), {
| 417| 417| 			"label": headerFont(translate("Number of builders:")),
| 418| 418| 			"details": entState.foundation.numBuilders
| 419|    |-		}) + "\n" + (entState.foundation.numBuilders ?
|    | 419|+	}) + "\n" + (entState.foundation.numBuilders ?
| 420| 420| 		sprintf(translatePlural(
| 421| 421| 			"Add another worker to speed up the construction by %(second)s second.",
| 422| 422| 			"Add another worker to speed up the construction by %(second)s seconds.",
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
| 649| 649| function getTerritoryInfluenceTooltip(template)
| 650| 650| {
| 651| 651| 	let text = "";
| 652|    |-	if (template.territoryInfluence && template.territoryInfluence.root) {
|    | 652|+	if (template.territoryInfluence && template.territoryInfluence.root) 
| 653| 653| 		text += sprintf(translate("%(label)s %(text)s"), {
| 654| 654| 			"label": headerFont(translate("Territory:")),
| 655| 655| 			"text": translate("Buildings in this building's territory do not decay.")
| 656| 656| 		});
| 657|    |-	}
|    | 657|+	
| 658| 658| 	return text
| 659| 659| }
| 660| 660| 
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
| 655| 655| 			"text": translate("Buildings in this building's territory do not decay.")
| 656| 656| 		});
| 657| 657| 	}
| 658|    |-	return text
|    | 658|+	return text;
| 659| 659| }
| 660| 660| 
| 661| 661| /**

binaries/data/mods/public/gui/common/tooltips.js
| 652| »   if·(template.territoryInfluence·&&·template.territoryInfluence.root)·{
|    | [NORMAL] ESLintBear (brace-rules/brace-on-same-line):
|    | Opening curly brace appears on the same line as controlling statement.

binaries/data/mods/public/gui/common/tooltips.js
| 658| »   return·text
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/selection_panels.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/selection_panels.js
| 453| 453| 				continue;
| 454| 454| 
| 455| 455| 			if (state.pack.progress == 0)
| 456|    |-			{
|    | 456|+			
| 457| 457| 				if (state.pack.packed)
| 458| 458| 					checks.unpackButton = true;
| 459| 459| 				else
| 460| 460| 					checks.packButton = true;
| 461|    |-			}
|    | 461|+			
| 462| 462| 			else if (state.pack.packed)
| 463| 463| 				checks.unpackCancelButton = true;
| 464| 464| 			else
|    | [NORMAL] ESLintBear (space-before-function-paren):
|    | Unexpected space before function parentheses.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/selection_panels.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/selection_panels.js
| 764| 764| 				addResearchToQueue(data.item.researchFacilityId, t);
| 765| 765| 			})(tech);
| 766| 766| 
| 767|    |-			button.onPressRight = (t => function () {
|    | 767|+			button.onPressRight = (t => function() {
| 768| 768| 				showTemplateDetails(
| 769| 769| 					t,
| 770| 770| 					GetTemplateData(data.unitEntStates.find(state => state.id == data.item.researchFacilityId).template).nativeCiv);
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/selection_panels.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/selection_panels.js
| 942| 942| 			"player": data.player
| 943| 943| 		});
| 944| 944| 
| 945|    |-		let unitIds = data.unitEntStates.map(status => status.id)
|    | 945|+		let unitIds = data.unitEntStates.map(status => status.id);
| 946| 946| 		let [buildingsCountToTrainFullBatch, fullBatchSize, remainderBatch] =
| 947| 947| 			getTrainingStatus(unitIds, data.item, data.playerState);
| 948| 948| 

binaries/data/mods/public/gui/session/selection_panels.js
|  48| »   »   »   switch·(data.item)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/selection_panels.js
|  59| »   »   switch·(data.item)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/selection_panels.js
| 732| »   »   »   »   »   »   switch·(entity.check)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/selection_panels.js
| 945| »   »   let·unitIds·=·data.unitEntStates.map(status·=>·status.id)
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.
Executing section cli...

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

BeforeAfter

Note that I choose to not show territory information in tech tree tooltips (it shows only in the construction panel and building details tooltips).

This is for consistency with other primarily textual info such as "Auras" and "Classes". Those are also present in panel/details tooltips, but absent from tech tree tooltips. Instead, they are shown in the body info of the tech tree "Information" dialogs.

The tech tree has a hardcoded relation between what it shown crammed the upper area of "Information" dialogs and in tech tree tooltips. So the choices are:

  1. Have it be in all three kinds of tooltips, and in the upper area of the Information dialog. (this would be inconsistent with what we do for Auras and Classes.)
  2. Have it be in all three kinds of tooltips, and twice in the Information dialog. (Inconsistent, and would be the first time we do that for anything, also Ugh, duplicate info).
  3. Have it be in the tooltip for construction and building details, absent from tech tree tooltip, and in the body of Information dialog. (This is what we do for Auras and Classes).

The commit currently does the latter (Option 3).

Below is what Option 1 would look like:

Option 1 (alternate)
Krinkle updated this revision to Diff 8581.Jun 22 2019, 10:36 PM
Krinkle edited the summary of this revision. (Show Details)

(resolved TODO)

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
| 392| 392| function getRepairTimeTooltip(entState)
| 393| 393| {
| 394| 394| 	return sprintf(translate("%(label)s %(details)s"), {
| 395|    |-			"label": headerFont(translate("Number of repairers:")),
|    | 395|+		"label": headerFont(translate("Number of repairers:")),
| 396| 396| 			"details": entState.repairable.numBuilders
| 397| 397| 		}) + "\n" + (entState.repairable.numBuilders ?
| 398| 398| 		sprintf(translatePlural(
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
| 393| 393| {
| 394| 394| 	return sprintf(translate("%(label)s %(details)s"), {
| 395| 395| 			"label": headerFont(translate("Number of repairers:")),
| 396|    |-			"details": entState.repairable.numBuilders
|    | 396|+		"details": entState.repairable.numBuilders
| 397| 397| 		}) + "\n" + (entState.repairable.numBuilders ?
| 398| 398| 		sprintf(translatePlural(
| 399| 399| 			"Add another worker to speed up the repairs by %(second)s second.",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
| 394| 394| 	return sprintf(translate("%(label)s %(details)s"), {
| 395| 395| 			"label": headerFont(translate("Number of repairers:")),
| 396| 396| 			"details": entState.repairable.numBuilders
| 397|    |-		}) + "\n" + (entState.repairable.numBuilders ?
|    | 397|+	}) + "\n" + (entState.repairable.numBuilders ?
| 398| 398| 		sprintf(translatePlural(
| 399| 399| 			"Add another worker to speed up the repairs by %(second)s second.",
| 400| 400| 			"Add another worker to speed up the repairs by %(second)s seconds.",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
| 414| 414| function getBuildTimeTooltip(entState)
| 415| 415| {
| 416| 416| 	return sprintf(translate("%(label)s %(details)s"), {
| 417|    |-			"label": headerFont(translate("Number of builders:")),
|    | 417|+		"label": headerFont(translate("Number of builders:")),
| 418| 418| 			"details": entState.foundation.numBuilders
| 419| 419| 		}) + "\n" + (entState.foundation.numBuilders ?
| 420| 420| 		sprintf(translatePlural(
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
| 415| 415| {
| 416| 416| 	return sprintf(translate("%(label)s %(details)s"), {
| 417| 417| 			"label": headerFont(translate("Number of builders:")),
| 418|    |-			"details": entState.foundation.numBuilders
|    | 418|+		"details": entState.foundation.numBuilders
| 419| 419| 		}) + "\n" + (entState.foundation.numBuilders ?
| 420| 420| 		sprintf(translatePlural(
| 421| 421| 			"Add another worker to speed up the construction by %(second)s second.",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/tooltips.js
| 416| 416| 	return sprintf(translate("%(label)s %(details)s"), {
| 417| 417| 			"label": headerFont(translate("Number of builders:")),
| 418| 418| 			"details": entState.foundation.numBuilders
| 419|    |-		}) + "\n" + (entState.foundation.numBuilders ?
|    | 419|+	}) + "\n" + (entState.foundation.numBuilders ?
| 420| 420| 		sprintf(translatePlural(
| 421| 421| 			"Add another worker to speed up the construction by %(second)s second.",
| 422| 422| 			"Add another worker to speed up the construction by %(second)s seconds.",
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/selection_panels.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/selection_panels.js
| 453| 453| 				continue;
| 454| 454| 
| 455| 455| 			if (state.pack.progress == 0)
| 456|    |-			{
|    | 456|+			
| 457| 457| 				if (state.pack.packed)
| 458| 458| 					checks.unpackButton = true;
| 459| 459| 				else
| 460| 460| 					checks.packButton = true;
| 461|    |-			}
|    | 461|+			
| 462| 462| 			else if (state.pack.packed)
| 463| 463| 				checks.unpackCancelButton = true;
| 464| 464| 			else
|    | [NORMAL] ESLintBear (space-before-function-paren):
|    | Unexpected space before function parentheses.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/selection_panels.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/selection_panels.js
| 764| 764| 				addResearchToQueue(data.item.researchFacilityId, t);
| 765| 765| 			})(tech);
| 766| 766| 
| 767|    |-			button.onPressRight = (t => function () {
|    | 767|+			button.onPressRight = (t => function() {
| 768| 768| 				showTemplateDetails(
| 769| 769| 					t,
| 770| 770| 					GetTemplateData(data.unitEntStates.find(state => state.id == data.item.researchFacilityId).template).nativeCiv);
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/selection_panels.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/selection_panels.js
| 942| 942| 			"player": data.player
| 943| 943| 		});
| 944| 944| 
| 945|    |-		let unitIds = data.unitEntStates.map(status => status.id)
|    | 945|+		let unitIds = data.unitEntStates.map(status => status.id);
| 946| 946| 		let [buildingsCountToTrainFullBatch, fullBatchSize, remainderBatch] =
| 947| 947| 			getTrainingStatus(unitIds, data.item, data.playerState);
| 948| 948| 

binaries/data/mods/public/gui/session/selection_panels.js
|  48| »   »   »   switch·(data.item)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/selection_panels.js
|  59| »   »   switch·(data.item)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/selection_panels.js
| 732| »   »   »   »   »   »   switch·(entity.check)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/selection_panels.js
| 945| »   »   let·unitIds·=·data.unitEntStates.map(status·=>·status.id)
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.
|    | [NORMAL] ESLintBear (operator-linebreak):
|    | '||' should be placed at the end of the line.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/globalscripts/Templates.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/globalscripts/Templates.js
|  87|  87| 		// If the elements are still strings, split them by space or by '+'
|  88|  88| 		if (typeof sublist == "string")
|  89|  89| 			sublist = sublist.split(/[+\s]+/);
|  90|    |-		if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1)
|  91|    |-		                    || (c[0] != "!" && classes.indexOf(c) != -1)))
|    |  90|+		if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1) ||
|    |  91|+		                    (c[0] != "!" && classes.indexOf(c) != -1)))
|  92|  92| 			return true;
|  93|  93| 	}
|  94|  94| 

binaries/data/mods/public/globalscripts/Templates.js
|  91| »   »   ····················||·(c[0]·!=·"!"·&&·classes.indexOf(c)·!=·-1)))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.
Executing section cli...

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

Krinkle added inline comments.Jun 22 2019, 11:00 PM
binaries/data/mods/public/gui/reference/viewer/viewer.js
21

I swapped these for consistency with selection details and selection panel.

Nescio added a subscriber: Nescio.Jun 23 2019, 5:59 PM

Although I'm not opposed to the idea per se, I'm not convinced the proposed implementation would really be an improvement.

To start with, the proposed string seems unnecessarily verbose: "Territory: Buildings in this building's territory do not decay." What we're interested in is the fact it has a territory root, which can be conveyed with the much shorter "Territory root: true". And an entity could have territory influence but no root (the majority of structures). If there is a separate territory influence line, then perhaps display radius and weight as well, e.g. "Territory: root: true; radius: 38; weight: 40000."
However, we probably don't want the tooltip to clutter up with unimportant information. That's why we display <CapturePoints>, but not <Capturable/RegenRate> or <Capturable/GarrisonRegenRate>. So the question is whether territory influence is important enough for the typical player to warrant its own tooltip line.
Moreover, the fact a structure has a territory root can be dealt with by two words in the tooltip text (“Territory root.”), or perhaps the addition of a visible class (“TerritoryRoot”).

By the way, the first parts of the monument and pillar <Tooltip> can be removed, because auras now have their own tooltips displayed.
There are several other structures with a territory root (grep -r '<Root>true</Root>') you might want to check.

In D2005#83597, @Nescio wrote:

Although I'm not opposed to the idea per se, I'm not convinced the proposed implementation would really be an improvement.

We currently show this info for two seemingly random buildings only (one civ's monument, and one civ's pillar). For all other buildings with territory root, this information is not discoverable in-game right now. Having that available, as this commit does, I think is a clear improvement. Exactly how to show it, though, is something I'd like help with :)

Aside from making the info available for the first time on many buildings, having it standardised with a bold heading will also aid players to find it more quickly (even for those two random buildings). And it will aid in code maintenance and help translators focus their efforts onto this one single string only.

In D2005#83597, @Nescio wrote:

To start with, the proposed string seems unnecessarily verbose: "Territory: Buildings in this building's territory do not decay." What we're interested in is the fact it has a territory root, which can be conveyed with the much shorter "Territory root: true".

As starting point, I used the same text we already have today for the monument and pilar. Almost verbatim, but I made it slightly shorter. I'd like shorten it even more, but not sure how best to phrase it. "Territory root: true", I think would be too cryptic for the primary audience of this information (new players). The meaning of "root" is not clear (and difficult to translate). Use of "true" is a bit software-centric, I think.

Something like "Territory: Has territory root." could be a minimal version, but still has the "Root" problem. If we do go with something that short, I would propose that we display a different string in the Information dialog. I don't know if that would be controversial, as right now we always have the same text in both (for bits that are included in both).

In D2005#83597, @Nescio wrote:

If there is a separate territory influence line, then perhaps display radius and weight as well, e.g. "Territory: root: true; radius: 38; weight: 40000."

Yeah, this might be much for the tooltip, but could definitely work in the Information dialog. It would also provide space to use a little extra sentence to give it meaningful context.

If someone approves adding the other influence parameters as well now, I'd be happy to work on that. If not (yet), perhaps continue that on Trac or in the forums first?

The first parts of the monument and pillar <Tooltip> can be removed, because auras now have their own tooltips displayed.

I've removed the text about territory root from the descriptions in this commit. I'm not very familiar with auras yet, can you elaborate on that? Is there a Trac task for that?

We currently show this info for two seemingly random buildings only (one civ's monument, and one civ's pillar). For all other buildings with territory root, this information is not discoverable in-game right now. Having that available, as this commit does, I think is a clear improvement.

Yes, I fully agree with that, I'm just not convinced it should be on a separate line.
Wouldn't it be better to simply add "Territory root." at the beginning or end of the tooltip text string?
Have a look at the farmstead, where the tooltip informs us it's a dropsite for food, important information.

You're right “Territory root” might be unclear when encountered the first time, but I believe people can figure it out. “Buildings in the territory of the monument do not decay.” is also problematic (what is the “territory of the monument”? what does “decay” mean?).

I'm not very familiar with auras yet, can you elaborate on that? Is there a Trac task for that?

Have a look at the screenshots you posted. As you can see all relevant information of the aura is already displayed automatically (the "Religious Fervor" line). Which means the tooltips of iber_monument.xml, maur_pillar_ashoka.xml, and pers_ishtar_gate.xml can now be purged.

That's why we display <CapturePoints>

Actually it isn't, I thought it was. It probably should, perhaps on the same line as health. Something for a future patch.

Krinkle added a comment.EditedJun 24 2019, 11:25 PM
In D2005#83640, @Nescio wrote:
@Krinkle wrote:

I'm not very familiar with auras yet, can you elaborate on that? Is there a Trac task for that?

Have a look at the screenshots you posted. As you can see all relevant information of the aura is already displayed automatically (the "Religious Fervor" line). Which means the tooltips of iber_monument.xml, maur_pillar_ashoka.xml, and pers_ishtar_gate.xml can now be purged.

Ah, right. Yeah, that seems redundant indeed. I've filed #5466. I'll work on that in a separate patch alongside this one.

Krinkle edited the test plan for this revision. (Show Details)Jun 29 2019, 1:25 AM
wraitii added a subscriber: wraitii.EditedJun 29 2019, 12:19 PM

I agree that Territory root isn't immediately obvious, but we should have consistency in naming things so probably best to keep it: https://trac.wildfiregames.com/wiki/EnglishStyleGuide

It seems like having some data on influence could also be useful, but tooltip wise I think it makes sense to have them on separate lines.
If we go with just "Territory root" in bold white, players will ultimately understand what it means and that's quite enough imo.

We probably should have a glossary or something like the Civilopedia someday so that new players can understand such words though.

binaries/data/mods/public/globalscripts/Templates.js
325

By convention we use !!template.TerritoryInfluence.Root

Something similar is that <BuildRestrictions/Territory> (own, neutral, enemy, ally) is sometimes mentioned in a tooltip text (e.g. outpost), but certainly not always (e.g. civic centre). Perhaps we should move that information in this patch as well and display it on the same line as territory root, e.g.:
Territory: own, neutral; root: true
On the other hand, when you're attempting to place a structure in invalid territory, the previewed structure turns red and the tooltip informs you already why it can't be placed.
It's just something to contemplate, possibly though not necessarily a good idea.

Krinkle marked an inline comment as done.Jun 29 2019, 11:41 PM
Krinkle added inline comments.
binaries/data/mods/public/globalscripts/Templates.js
325

This is not a boolean comparison, it's a string comparison :)

Krinkle marked an inline comment as done.Jun 30 2019, 12:44 AM

A few options mentioned so far:

Buildings in the territory of the monument do not decay.(trunk)
Territory: Buildings in this building's territory do not decay.(current diff)
Territory: root = true.
Territory: Has territory root.
Territory: This building provides territory root.
Territory root: true.

The current diff is consistent with the old text, but is also not very good. Also, upon closer inspection, I find it's actually quite inconsistent with how other buildings are written. I believe, we tend to omit the context/subject from descriptions and tooltips. For example:

  • Barracks: Train citizen solders.
  • Temple / Medial Treatment: Heals nearby units.

A few more ideas with this taken into account:

Territory root: Territory of nearby buildings does not decay.
Territory root: Supporting territory for other buildings.
Territory root: Support the territory of nearby buildings.
Territory root: Strong territory that claims loyalty of nearby buildings.

It seems like having some data on influence could also be useful, [..]

Yeah, we should definitely add this data on the same line as well. The Trac ticket asked for this as well. Right now it is entirely undiscoverable in-game. If it's too much for the tooltip, it could also go to the Information overlay. Reducing the amount of text in the tooltip to only critical concepts, I think, helps players to identify and learn these fundamentals. Too much information overload in the tooltip can be discouraging and/or lead to "banner fatigue" where the tooltip is ignored entirely (I already see this problem today, something deserving of its own design improvements).

If it doesn't require too much extra code changes to expose, I can work that into this commit as well, but perhaps better to restrict to the Information overlay for now to keep the difference minimal. Then in a separate change we can work on finding a brief way to fit it in the tooltip, and decide whether it is worth the cost/overload to be in the tooltip. I'd like to leave that out for this change, which mainly focusses on standardising the existing information first, and making it available for many more buildings.

If we go with just "Territory root" in bold white, players will ultimately understand what it means and that's quite enough imo.
[..]
We probably should have a glossary or something like the Civilopedia someday so that new players can understand such words though.

I think once a concept is understood, we have quite a bit of liberty around what label to use to refer to it. "Territory root" is certainly good enough for that. But on its own, I don't think it help a player learn the concept itself. I don't think it's reasonable to expect that when a player needs to expand their territory, that they are first going to look through all possible buildings they can construct, find that none of them says it does what they find, collect all the unknown terms they encountered in the process (including "territory root"), and look them up in a terminology index to see if one of the missed terms does what they want. That's quite a lot of steps. I think this kind of workflow is good to allow, but better suited for something already less direct in nature. For example, perhaps if there was a special Healer that can defeat an Aura, it can be described "Disable the aura of nearby units". A player would only need this if they know what an aura is, so it doesn't need to explain it. Or if they're curious anyway, can look it up in such terminology page.

I think we can find a good middle ground by giving its own tooltip line (combined with the strength and radius of the territory root, but separate from what territory kind the building can be placed in), in which we give a brief definition of the concept that would help players understand it.

That gives us the best of both: We have a term that we can refer to elsewhere (so "Territory root", not "Territory"), and yet also explained in-context in-game.

Do you like one of the above ideas? Or perhaps have additional suggestions for the phrasing?

A few options mentioned so far:

  • Automatically insert “Territory root.” at the beginning of the tooltip text line
  • Automatically insert “Territory root.” at the end of the tooltip text line
  • have a separate territory root line (current diff), e.g. “Territory root: true”
  • have a separate territory influence line, e.g. “Territory: root: true; radius: 38; weight: 40000.”
  • have a combined <BuildRestrictions/Territory> and <Territory/Root> line, e.g. “Territory: own, neutral; root: true”
Krinkle marked an inline comment as not done.Jul 24 2019, 12:11 AM
elexis added a subscriber: elexis.Jul 24 2019, 12:51 AM

(I didn't read the conversation yet, just some inline comments from what I saw in the code, so I might say something redundant or stupid)

binaries/data/mods/public/globalscripts/Templates.js
325

comments unneeded, first one is true for all template objects, second one is true for all(?) data returned here.

Until more than 1 property exists, one can avoid creating the otherwise unused object for performance benefit.

binaries/data/mods/public/gui/common/tooltips.js
729
  • avoid the variable and return in both cases directly.
  • That's not a building, that's a structure.
  • It would be good to phrase "not decay" it positively, for example "This structure provides territory rooting to your city" or so. Reading the table you posted, Support the territory of nearby structures. sounded too ambiguous (what is support).
Krinkle updated this revision to Diff 9160.Jul 28 2019, 5:12 PM
Krinkle marked 2 inline comments as done.
Krinkle edited the summary of this revision. (Show Details)
Krinkle edited the test plan for this revision. (Show Details)

Appearance:

  • Change phrasing away from the status quo of "Buildings in the territory do not decay", to be like @elexis suggestion: "This structure provides territory rooting".
  • Make the Territory tooltip included when hovering icons in the Tech Tree, this should make it relatively easy for players to quickly find which buildings can provide territory rooting. This change was done by moving the function from viewer#g_InfoFunctions to draw#g_StatsFunctions *thus more like getPopulationBonusTooltip, and less like getAurasTooltip). This two side-effects:
    • In the "More information" dialog, it is now crammed in the top part, instead of in the more spaced out body area.
    • When selecting an existing structure on the map that has rooting and hovering the central panel on the screen, this tooltip is not included.

Code:

  • Removed an inline comment in globalscripts/Templates.js.
  • Removed the territoryInfluence wrapping object, in favour of assigning territoryInfluenceRoot directly to the template (for now; until other territory properties come along).
  • In getTerritoryInfluenceTooltip(), avoid the unused string, return directly instead (also fixed the function from which I copied this pattern).

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (operator-linebreak):
|    | '||' should be placed at the end of the line.
|----|    | /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|  87|  87| 		// If the elements are still strings, split them by space or by '+'
|  88|  88| 		if (typeof sublist == "string")
|  89|  89| 			sublist = sublist.split(/[+\s]+/);
|  90|    |-		if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1)
|  91|    |-		                    || (c[0] != "!" && classes.indexOf(c) != -1)))
|    |  90|+		if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1) ||
|    |  91|+		                    (c[0] != "!" && classes.indexOf(c) != -1)))
|  92|  92| 			return true;
|  93|  93| 	}
|  94|  94| 

binaries/data/mods/public/globalscripts/Templates.js
|  91| »   »   ····················||·(c[0]·!=·"!"·&&·classes.indexOf(c)·!=·-1)))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.
|    | [NORMAL] ESLintBear (space-before-function-paren):
|    | Unexpected space before function parentheses.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/session/selection_panels.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/session/selection_panels.js
| 764| 764| 				addResearchToQueue(data.item.researchFacilityId, t);
| 765| 765| 			})(tech);
| 766| 766| 
| 767|    |-			button.onPressRight = (t => function () {
|    | 767|+			button.onPressRight = (t => function() {
| 768| 768| 				showTemplateDetails(
| 769| 769| 					t,
| 770| 770| 					GetTemplateData(data.unitEntStates.find(state => state.id == data.item.researchFacilityId).template).nativeCiv);

binaries/data/mods/public/gui/session/selection_panels.js
|  48| »   »   »   switch·(data.item)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/selection_panels.js
|  59| »   »   switch·(data.item)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/selection_panels.js
| 732| »   »   »   »   »   »   switch·(entity.check)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 389| 389| function getRepairTimeTooltip(entState)
| 390| 390| {
| 391| 391| 	return sprintf(translate("%(label)s %(details)s"), {
| 392|    |-			"label": headerFont(translate("Number of repairers:")),
|    | 392|+		"label": headerFont(translate("Number of repairers:")),
| 393| 393| 			"details": entState.repairable.numBuilders
| 394| 394| 		}) + "\n" + (entState.repairable.numBuilders ?
| 395| 395| 		sprintf(translatePlural(
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 390| 390| {
| 391| 391| 	return sprintf(translate("%(label)s %(details)s"), {
| 392| 392| 			"label": headerFont(translate("Number of repairers:")),
| 393|    |-			"details": entState.repairable.numBuilders
|    | 393|+		"details": entState.repairable.numBuilders
| 394| 394| 		}) + "\n" + (entState.repairable.numBuilders ?
| 395| 395| 		sprintf(translatePlural(
| 396| 396| 			"Add another worker to speed up the repairs by %(second)s second.",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 2.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 391| 391| 	return sprintf(translate("%(label)s %(details)s"), {
| 392| 392| 			"label": headerFont(translate("Number of repairers:")),
| 393| 393| 			"details": entState.repairable.numBuilders
| 394|    |-		}) + "\n" + (entState.repairable.numBuilders ?
|    | 394|+	}) + "\n" + (entState.repairable.numBuilders ?
| 395| 395| 		sprintf(translatePlural(
| 396| 396| 			"Add another worker to speed up the repairs by %(second)s second.",
| 397| 397| 			"Add another worker to speed up the repairs by %(second)s seconds.",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 411| 411| function getBuildTimeTooltip(entState)
| 412| 412| {
| 413| 413| 	return sprintf(translate("%(label)s %(details)s"), {
| 414|    |-			"label": headerFont(translate("Number of builders:")),
|    | 414|+		"label": headerFont(translate("Number of builders:")),
| 415| 415| 			"details": entState.foundation.numBuilders
| 416| 416| 		}) + "\n" + (entState.foundation.numBuilders ?
| 417| 417| 		sprintf(translatePlural(
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 412| 412| {
| 413| 413| 	return sprintf(translate("%(label)s %(details)s"), {
| 414| 414| 			"label": headerFont(translate("Number of builders:")),
| 415|    |-			"details": entState.foundation.numBuilders
|    | 415|+		"details": entState.foundation.numBuilders
| 416| 416| 		}) + "\n" + (entState.foundation.numBuilders ?
| 417| 417| 		sprintf(translatePlural(
| 418| 418| 			"Add another worker to speed up the construction by %(second)s second.",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 2.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 413| 413| 	return sprintf(translate("%(label)s %(details)s"), {
| 414| 414| 			"label": headerFont(translate("Number of builders:")),
| 415| 415| 			"details": entState.foundation.numBuilders
| 416|    |-		}) + "\n" + (entState.foundation.numBuilders ?
|    | 416|+	}) + "\n" + (entState.foundation.numBuilders ?
| 417| 417| 		sprintf(translatePlural(
| 418| 418| 			"Add another worker to speed up the construction by %(second)s second.",
| 419| 419| 			"Add another worker to speed up the construction by %(second)s seconds.",
Executing section cli...

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

Krinkle updated this revision to Diff 9590.Sep 1 2019, 7:15 PM
Krinkle edited the summary of this revision. (Show Details)
Krinkle edited the test plan for this revision. (Show Details)

Rebased on latest trunk.

Vulcan added a comment.Sep 1 2019, 7:17 PM

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

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

Vulcan added a comment.Sep 1 2019, 7:19 PM

Build failure - The Moirai have given mortals hearts that can endure.

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (operator-linebreak):
|    | '||' should be placed at the end of the line.
|----|    | /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|  87|  87| 		// If the elements are still strings, split them by space or by '+'
|  88|  88| 		if (typeof sublist == "string")
|  89|  89| 			sublist = sublist.split(/[+\s]+/);
|  90|    |-		if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1)
|  91|    |-		                    || (c[0] != "!" && classes.indexOf(c) != -1)))
|    |  90|+		if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1) ||
|    |  91|+		                    (c[0] != "!" && classes.indexOf(c) != -1)))
|  92|  92| 			return true;
|  93|  93| 	}
|  94|  94| 

binaries/data/mods/public/globalscripts/Templates.js
|  91| »   »   ····················||·(c[0]·!=·"!"·&&·classes.indexOf(c)·!=·-1)))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 389| 389| function getRepairTimeTooltip(entState)
| 390| 390| {
| 391| 391| 	return sprintf(translate("%(label)s %(details)s"), {
| 392|    |-			"label": headerFont(translate("Number of repairers:")),
|    | 392|+		"label": headerFont(translate("Number of repairers:")),
| 393| 393| 			"details": entState.repairable.numBuilders
| 394| 394| 		}) + "\n" + (entState.repairable.numBuilders ?
| 395| 395| 		sprintf(translatePlural(
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 390| 390| {
| 391| 391| 	return sprintf(translate("%(label)s %(details)s"), {
| 392| 392| 			"label": headerFont(translate("Number of repairers:")),
| 393|    |-			"details": entState.repairable.numBuilders
|    | 393|+		"details": entState.repairable.numBuilders
| 394| 394| 		}) + "\n" + (entState.repairable.numBuilders ?
| 395| 395| 		sprintf(translatePlural(
| 396| 396| 			"Add another worker to speed up the repairs by %(second)s second.",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 2.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 391| 391| 	return sprintf(translate("%(label)s %(details)s"), {
| 392| 392| 			"label": headerFont(translate("Number of repairers:")),
| 393| 393| 			"details": entState.repairable.numBuilders
| 394|    |-		}) + "\n" + (entState.repairable.numBuilders ?
|    | 394|+	}) + "\n" + (entState.repairable.numBuilders ?
| 395| 395| 		sprintf(translatePlural(
| 396| 396| 			"Add another worker to speed up the repairs by %(second)s second.",
| 397| 397| 			"Add another worker to speed up the repairs by %(second)s seconds.",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 411| 411| function getBuildTimeTooltip(entState)
| 412| 412| {
| 413| 413| 	return sprintf(translate("%(label)s %(details)s"), {
| 414|    |-			"label": headerFont(translate("Number of builders:")),
|    | 414|+		"label": headerFont(translate("Number of builders:")),
| 415| 415| 			"details": entState.foundation.numBuilders
| 416| 416| 		}) + "\n" + (entState.foundation.numBuilders ?
| 417| 417| 		sprintf(translatePlural(
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 412| 412| {
| 413| 413| 	return sprintf(translate("%(label)s %(details)s"), {
| 414| 414| 			"label": headerFont(translate("Number of builders:")),
| 415|    |-			"details": entState.foundation.numBuilders
|    | 415|+		"details": entState.foundation.numBuilders
| 416| 416| 		}) + "\n" + (entState.foundation.numBuilders ?
| 417| 417| 		sprintf(translatePlural(
| 418| 418| 			"Add another worker to speed up the construction by %(second)s second.",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 2.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 413| 413| 	return sprintf(translate("%(label)s %(details)s"), {
| 414| 414| 			"label": headerFont(translate("Number of builders:")),
| 415| 415| 			"details": entState.foundation.numBuilders
| 416|    |-		}) + "\n" + (entState.foundation.numBuilders ?
|    | 416|+	}) + "\n" + (entState.foundation.numBuilders ?
| 417| 417| 		sprintf(translatePlural(
| 418| 418| 			"Add another worker to speed up the construction by %(second)s second.",
| 419| 419| 			"Add another worker to speed up the construction by %(second)s seconds.",
|    | [NORMAL] ESLintBear (space-before-function-paren):
|    | Unexpected space before function parentheses.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/session/selection_panels.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/session/selection_panels.js
| 764| 764| 				addResearchToQueue(data.item.researchFacilityId, t);
| 765| 765| 			})(tech);
| 766| 766| 
| 767|    |-			button.onPressRight = (t => function () {
|    | 767|+			button.onPressRight = (t => function() {
| 768| 768| 				showTemplateDetails(
| 769| 769| 					t,
| 770| 770| 					GetTemplateData(data.unitEntStates.find(state => state.id == data.item.researchFacilityId).template).nativeCiv);

binaries/data/mods/public/gui/session/selection_panels.js
|  48| »   »   »   switch·(data.item)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/selection_panels.js
|  59| »   »   switch·(data.item)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/selection_panels.js
| 732| »   »   »   »   »   »   switch·(entity.check)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.
Executing section cli...

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

Krinkle updated this revision to Diff 9747.Sep 14 2019, 5:28 AM

Rebased to re-trigger CI which had a false negative it seems.

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

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

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (operator-linebreak):
|    | '||' should be placed at the end of the line.
|----|    | /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|  87|  87| 		// If the elements are still strings, split them by space or by '+'
|  88|  88| 		if (typeof sublist == "string")
|  89|  89| 			sublist = sublist.split(/[+\s]+/);
|  90|    |-		if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1)
|  91|    |-		                    || (c[0] != "!" && classes.indexOf(c) != -1)))
|    |  90|+		if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1) ||
|    |  91|+		                    (c[0] != "!" && classes.indexOf(c) != -1)))
|  92|  92| 			return true;
|  93|  93| 	}
|  94|  94| 

binaries/data/mods/public/globalscripts/Templates.js
|  91| »   »   ····················||·(c[0]·!=·"!"·&&·classes.indexOf(c)·!=·-1)))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.
|    | [NORMAL] ESLintBear (space-before-function-paren):
|    | Unexpected space before function parentheses.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/session/selection_panels.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/session/selection_panels.js
| 764| 764| 				addResearchToQueue(data.item.researchFacilityId, t);
| 765| 765| 			})(tech);
| 766| 766| 
| 767|    |-			button.onPressRight = (t => function () {
|    | 767|+			button.onPressRight = (t => function() {
| 768| 768| 				showTemplateDetails(
| 769| 769| 					t,
| 770| 770| 					GetTemplateData(data.unitEntStates.find(state => state.id == data.item.researchFacilityId).template).nativeCiv);

binaries/data/mods/public/gui/session/selection_panels.js
|  48| »   »   »   switch·(data.item)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/selection_panels.js
|  59| »   »   switch·(data.item)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/selection_panels.js
| 732| »   »   »   »   »   »   switch·(entity.check)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.
Executing section cli...

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

Krinkle updated this revision to Diff 10481.Dec 5 2019, 11:53 PM

(Rebased.)

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

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

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (operator-linebreak):
|    | '||' should be placed at the end of the line.
|----|    | /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|  87|  87| 		// If the elements are still strings, split them by space or by '+'
|  88|  88| 		if (typeof sublist == "string")
|  89|  89| 			sublist = sublist.split(/[+\s]+/);
|  90|    |-		if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1)
|  91|    |-		                    || (c[0] != "!" && classes.indexOf(c) != -1)))
|    |  90|+		if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1) ||
|    |  91|+		                    (c[0] != "!" && classes.indexOf(c) != -1)))
|  92|  92| 			return true;
|  93|  93| 	}
|  94|  94| 

binaries/data/mods/public/globalscripts/Templates.js
|  91| »   »   ····················||·(c[0]·!=·"!"·&&·classes.indexOf(c)·!=·-1)))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.
|    | [NORMAL] ESLintBear (space-before-function-paren):
|    | Unexpected space before function parentheses.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/session/selection_panels.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/session/selection_panels.js
| 762| 762| 				addResearchToQueue(data.item.researchFacilityId, t);
| 763| 763| 			})(tech);
| 764| 764| 
| 765|    |-			button.onPressRight = (t => function () {
|    | 765|+			button.onPressRight = (t => function() {
| 766| 766| 				showTemplateDetails(
| 767| 767| 					t,
| 768| 768| 					GetTemplateData(data.unitEntStates.find(state => state.id == data.item.researchFacilityId).template).nativeCiv);

binaries/data/mods/public/gui/session/selection_panels.js
|  50| »   »   »   switch·(data.item)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/selection_panels.js
|  61| »   »   switch·(data.item)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/selection_panels.js
| 730| »   »   »   »   »   »   switch·(entity.check)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.
Executing section cli...

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

Krinkle edited the summary of this revision. (Show Details)Dec 7 2019, 10:07 PM
Krinkle edited the test plan for this revision. (Show Details)Dec 7 2019, 10:10 PM

@Nescio do you have an opinion on the current status? (I know it needs a rebase, but the images above will perhaps suffice?)

See my earlier remarks here, my opinion hasn't changed since June.