Page MenuHomeWildfire Games
Differential D1291

Put diplomacy colors in options
ClosedPublic
Actions

Authored by temple on Feb 10 2018, 5:01 AM.

Details

Reviewers
vladislavbelov
elexis
Commits
rP21527: Implement basic color editing in the options page and use it for the diplomacy…
Summary

Put diplomacy colors in options, so players can choose their own colors. Also use the regular rather than bright blue/green/yellow/red colors by default. See rP21107.

Test Plan

I still need to sanitize the values so that only valid rgb strings are used.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
elexis added inline comments.Feb 10 2018, 7:52 PM
binaries/data/mods/public/gui/options/options.json
453 ↗(On Diff #5746)

Where did the TODO go? It was right, the text input box should become red if the user typed something wrong.
Can look at the numeric input code.

Vulcan added a comment.Feb 10 2018, 8:27 PM

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

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (309 tests).....................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (309 tests).....................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

Link to build: https://jenkins.wildfiregames.com/job/phabricator/2720/display/redirect

Vulcan added a comment.Feb 10 2018, 8:29 PM
Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (object-curly-spacing):
|    | A space is required after '{'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/common/color.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/common/color.js
|   1|   1| /**
|   2|   2|  * Used to highlight hotkeys in tooltip descriptions.
|   3|   3|  */
|   4|    |-var g_HotkeyTags = {"color": "255 251 131" };
|    |   4|+var g_HotkeyTags = { "color": "255 251 131" };
|   5|   5| 
|   6|   6| /**
|   7|   7|  * Concatenate integer color values to a string (for use in GUI objects)

binaries/data/mods/public/gui/common/color.js
|  96| »   »   h·=·s·=·0;·//·achromatic
|    | [NORMAL] ESLintBear (no-multi-assign):
|    | Unexpected chained assignment.

binaries/data/mods/public/gui/common/color.js
| 101| »   »   switch·(max)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/common/color.js
| 147| »   if·(s·==·0)
|    | [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/color.js
| 148| »   »   r·=·g·=·b·=·l;·//·achromatic
|    | [NORMAL] ESLintBear (no-multi-assign):
|    | Unexpected chained assignment.

binaries/data/mods/public/gui/common/color.js
| 148| »   »   r·=·g·=·b·=·l;·//·achromatic
|    | [NORMAL] ESLintBear (no-multi-assign):
|    | Unexpected chained assignment.

binaries/data/mods/public/gui/session/menu.js
| 469| »   »   button.onPress·=·(function(player,·stance)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'stance' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 501| »   »   button.onPress·=·(function(i,·resCode,·button)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 501| »   »   button.onPress·=·(function(i,·resCode,·button)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'resCode' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 501| »   »   button.onPress·=·(function(i,·resCode,·button)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'button' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 544| »   button.onPress·=·(function(i)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 610| »   button.onPress·=·(function(i,·button)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 610| »   button.onPress·=·(function(i,·button)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'button' is already declared in the upper scope.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 6 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
| 418| 418| 				// Players see colors depending on diplomacy
| 419| 419| 				g_DisplayedPlayerColors[i] =
| 420| 420| 					g_ViewedPlayer == i ? sanitizeDiplomacyColor("self") :
| 421|    |-					g_Players[g_ViewedPlayer].isAlly[i] ? sanitizeDiplomacyColor("ally") :
|    | 421|+						g_Players[g_ViewedPlayer].isAlly[i] ? sanitizeDiplomacyColor("ally") :
| 422| 422| 					g_Players[g_ViewedPlayer].isNeutral[i] ? sanitizeDiplomacyColor("neutral") :
| 423| 423| 					sanitizeDiplomacyColor("enemy");
| 424| 424| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
| 419| 419| 				g_DisplayedPlayerColors[i] =
| 420| 420| 					g_ViewedPlayer == i ? sanitizeDiplomacyColor("self") :
| 421| 421| 					g_Players[g_ViewedPlayer].isAlly[i] ? sanitizeDiplomacyColor("ally") :
| 422|    |-					g_Players[g_ViewedPlayer].isNeutral[i] ? sanitizeDiplomacyColor("neutral") :
|    | 422|+							g_Players[g_ViewedPlayer].isNeutral[i] ? sanitizeDiplomacyColor("neutral") :
| 423| 423| 					sanitizeDiplomacyColor("enemy");
| 424| 424| 
| 425| 425| 		g_DisplayedPlayerColors[0] = g_Players[0].color;
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 8 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
| 420| 420| 					g_ViewedPlayer == i ? sanitizeDiplomacyColor("self") :
| 421| 421| 					g_Players[g_ViewedPlayer].isAlly[i] ? sanitizeDiplomacyColor("ally") :
| 422| 422| 					g_Players[g_ViewedPlayer].isNeutral[i] ? sanitizeDiplomacyColor("neutral") :
| 423|    |-					sanitizeDiplomacyColor("enemy");
|    | 423|+								sanitizeDiplomacyColor("enemy");
| 424| 424| 
| 425| 425| 		g_DisplayedPlayerColors[0] = g_Players[0].color;
| 426| 426| 	}
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|1249|1249| 
|1250|1250| 	let orderHotkeyTooltip = Object.keys(viewablePlayerStates).length <= 1 ? "" :
|1251|1251| 		"\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), {
|1252|    |-		"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|    |1252|+			"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|1253|1253| 		"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|1254|1254| 	});
|1255|1255| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|1250|1250| 	let orderHotkeyTooltip = Object.keys(viewablePlayerStates).length <= 1 ? "" :
|1251|1251| 		"\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), {
|1252|1252| 		"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|1253|    |-		"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|    |1253|+			"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|1254|1254| 	});
|1255|1255| 
|1256|1256| 	let resCodes = g_ResourceData.GetCodes();
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 1.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|1251|1251| 		"\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), {
|1252|1252| 		"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|1253|1253| 		"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|1254|    |-	});
|    |1254|+		});
|1255|1255| 
|1256|1256| 	let resCodes = g_ResourceData.GetCodes();
|1257|1257| 	for (let r = 0; r < resCodes.length; ++r)
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|1737|1737| 	for (let rct of resourcesCounterTypes)
|1738|1738| 		for (let rt of resourcesTypes)
|1739|1739| 			reportObject[rt + rct.substr(9)] = playerStatistics[rct][rt];
|1740|    |-			// eg. rt = food rct.substr = Gathered rct = resourcesGathered
|    |1740|+	// eg. rt = food rct.substr = Gathered rct = resourcesGathered
|1741|1741| 
|1742|1742| 	reportObject.vegetarianFoodGathered = playerStatistics.resourcesGathered.vegetarianFood;
|1743|1743| 	for (let type of unitsClasses)

binaries/data/mods/public/gui/session/session.js
|1077| »   let·getPanelEntNameTooltip·=·panelEntState·=>·"[font=\"sans-bold-16\"]"·+·template.name.specific·+·"[/font]";
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'panelEntState' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
|1152| »   »   button.onpress·=·(function(i)·{·return·function()·{·performGroup((Engine.HotkeyIsPressed("selection.add")·?·"add"·:·"select"),·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
|1153| »   »   button.ondoublepress·=·(function(i)·{·return·function()·{·performGroup("snap",·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
|1154| »   »   button.onpressright·=·(function(i)·{·return·function()·{·performGroup("breakUp",·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

Link to build: https://jenkins.wildfiregames.com/job/phabricator_lint/1016/display/redirect

Harbormaster completed remote builds in B4905: Diff 5746.Feb 10 2018, 8:29 PM
temple updated this revision to Diff 5747.Feb 10 2018, 9:04 PM
temple marked 2 inline comments as done.

Added back rgbString and included the color preview:

Made guiToRgbColor always return a valid color object. Not sure if that's the best option but it makes everything else easier.

Vulcan added a comment.Feb 10 2018, 10:15 PM

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

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (309 tests).....................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (309 tests).....................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

Link to build: https://jenkins.wildfiregames.com/job/phabricator/2721/display/redirect

Vulcan added a comment.Feb 10 2018, 10:16 PM
Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (object-curly-spacing):
|    | A space is required after '{'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/common/color.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/common/color.js
|   1|   1| /**
|   2|   2|  * Used to highlight hotkeys in tooltip descriptions.
|   3|   3|  */
|   4|    |-var g_HotkeyTags = {"color": "255 251 131" };
|    |   4|+var g_HotkeyTags = { "color": "255 251 131" };
|   5|   5| 
|   6|   6| /**
|   7|   7|  * Concatenate integer color values to a string (for use in GUI objects)

binaries/data/mods/public/gui/common/color.js
|  90| »   »   h·=·s·=·0;·//·achromatic
|    | [NORMAL] ESLintBear (no-multi-assign):
|    | Unexpected chained assignment.

binaries/data/mods/public/gui/common/color.js
|  95| »   »   switch·(max)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/common/color.js
| 141| »   if·(s·==·0)
|    | [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/color.js
| 142| »   »   r·=·g·=·b·=·l;·//·achromatic
|    | [NORMAL] ESLintBear (no-multi-assign):
|    | Unexpected chained assignment.

binaries/data/mods/public/gui/common/color.js
| 142| »   »   r·=·g·=·b·=·l;·//·achromatic
|    | [NORMAL] ESLintBear (no-multi-assign):
|    | Unexpected chained assignment.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 5 tabs but found 4.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/options/options.js
| 104| 104| 			sprintf(
| 105| 105| 				option.min !== undefined && option.max !== undefined ?
| 106| 106| 					translateWithContext("option number", "Min: %(min)s, Max: %(max)s") :
| 107|    |-				option.min !== undefined && option.max === undefined ?
|    | 107|+					option.min !== undefined && option.max === undefined ?
| 108| 108| 					translateWithContext("option number", "Min: %(min)s") :
| 109| 109| 				option.min === undefined && option.max !== undefined ?
| 110| 110| 					translateWithContext("option number", "Max: %(max)s") :
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 6 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/options/options.js
| 105| 105| 				option.min !== undefined && option.max !== undefined ?
| 106| 106| 					translateWithContext("option number", "Min: %(min)s, Max: %(max)s") :
| 107| 107| 				option.min !== undefined && option.max === undefined ?
| 108|    |-					translateWithContext("option number", "Min: %(min)s") :
|    | 108|+						translateWithContext("option number", "Min: %(min)s") :
| 109| 109| 				option.min === undefined && option.max !== undefined ?
| 110| 110| 					translateWithContext("option number", "Max: %(max)s") :
| 111| 111| 					"",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 6 tabs but found 4.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/options/options.js
| 106| 106| 					translateWithContext("option number", "Min: %(min)s, Max: %(max)s") :
| 107| 107| 				option.min !== undefined && option.max === undefined ?
| 108| 108| 					translateWithContext("option number", "Min: %(min)s") :
| 109|    |-				option.min === undefined && option.max !== undefined ?
|    | 109|+						option.min === undefined && option.max !== undefined ?
| 110| 110| 					translateWithContext("option number", "Max: %(max)s") :
| 111| 111| 					"",
| 112| 112| 				{
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/options/options.js
| 107| 107| 				option.min !== undefined && option.max === undefined ?
| 108| 108| 					translateWithContext("option number", "Min: %(min)s") :
| 109| 109| 				option.min === undefined && option.max !== undefined ?
| 110|    |-					translateWithContext("option number", "Max: %(max)s") :
|    | 110|+							translateWithContext("option number", "Max: %(max)s") :
| 111| 111| 					"",
| 112| 112| 				{
| 113| 113| 					"min": option.min,
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/options/options.js
| 108| 108| 					translateWithContext("option number", "Min: %(min)s") :
| 109| 109| 				option.min === undefined && option.max !== undefined ?
| 110| 110| 					translateWithContext("option number", "Max: %(max)s") :
| 111|    |-					"",
|    | 111|+							"",
| 112| 112| 				{
| 113| 113| 					"min": option.min,
| 114| 114| 					"max": option.max

binaries/data/mods/public/gui/options/options.js
| 220| »   »   »   let·value·=·optionType.guiToValue(control);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'value' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 469| »   »   button.onPress·=·(function(player,·stance)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'stance' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 501| »   »   button.onPress·=·(function(i,·resCode,·button)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 501| »   »   button.onPress·=·(function(i,·resCode,·button)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'resCode' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 501| »   »   button.onPress·=·(function(i,·resCode,·button)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'button' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 544| »   button.onPress·=·(function(i)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 610| »   button.onPress·=·(function(i,·button)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 610| »   button.onPress·=·(function(i,·button)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'button' is already declared in the upper scope.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 6 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
| 404| 404| 				// Players see colors depending on diplomacy
| 405| 405| 				g_DisplayedPlayerColors[i] =
| 406| 406| 					g_ViewedPlayer == i ? guiToRgbColor(Engine.ConfigDB_GetValue("user", "diplomacycolors.self")) :
| 407|    |-					g_Players[g_ViewedPlayer].isAlly[i] ? guiToRgbColor(Engine.ConfigDB_GetValue("user", "diplomacycolors.ally")) :
|    | 407|+						g_Players[g_ViewedPlayer].isAlly[i] ? guiToRgbColor(Engine.ConfigDB_GetValue("user", "diplomacycolors.ally")) :
| 408| 408| 					g_Players[g_ViewedPlayer].isNeutral[i] ? guiToRgbColor(Engine.ConfigDB_GetValue("user", "diplomacycolors.neutral")) :
| 409| 409| 					guiToRgbColor(Engine.ConfigDB_GetValue("user", "diplomacycolors.enemy"));
| 410| 410| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
| 405| 405| 				g_DisplayedPlayerColors[i] =
| 406| 406| 					g_ViewedPlayer == i ? guiToRgbColor(Engine.ConfigDB_GetValue("user", "diplomacycolors.self")) :
| 407| 407| 					g_Players[g_ViewedPlayer].isAlly[i] ? guiToRgbColor(Engine.ConfigDB_GetValue("user", "diplomacycolors.ally")) :
| 408|    |-					g_Players[g_ViewedPlayer].isNeutral[i] ? guiToRgbColor(Engine.ConfigDB_GetValue("user", "diplomacycolors.neutral")) :
|    | 408|+							g_Players[g_ViewedPlayer].isNeutral[i] ? guiToRgbColor(Engine.ConfigDB_GetValue("user", "diplomacycolors.neutral")) :
| 409| 409| 					guiToRgbColor(Engine.ConfigDB_GetValue("user", "diplomacycolors.enemy"));
| 410| 410| 
| 411| 411| 		g_DisplayedPlayerColors[0] = g_Players[0].color;
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 8 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
| 406| 406| 					g_ViewedPlayer == i ? guiToRgbColor(Engine.ConfigDB_GetValue("user", "diplomacycolors.self")) :
| 407| 407| 					g_Players[g_ViewedPlayer].isAlly[i] ? guiToRgbColor(Engine.ConfigDB_GetValue("user", "diplomacycolors.ally")) :
| 408| 408| 					g_Players[g_ViewedPlayer].isNeutral[i] ? guiToRgbColor(Engine.ConfigDB_GetValue("user", "diplomacycolors.neutral")) :
| 409|    |-					guiToRgbColor(Engine.ConfigDB_GetValue("user", "diplomacycolors.enemy"));
|    | 409|+								guiToRgbColor(Engine.ConfigDB_GetValue("user", "diplomacycolors.enemy"));
| 410| 410| 
| 411| 411| 		g_DisplayedPlayerColors[0] = g_Players[0].color;
| 412| 412| 	}
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|1235|1235| 
|1236|1236| 	let orderHotkeyTooltip = Object.keys(viewablePlayerStates).length <= 1 ? "" :
|1237|1237| 		"\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), {
|1238|    |-		"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|    |1238|+			"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|1239|1239| 		"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|1240|1240| 	});
|1241|1241| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|1236|1236| 	let orderHotkeyTooltip = Object.keys(viewablePlayerStates).length <= 1 ? "" :
|1237|1237| 		"\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), {
|1238|1238| 		"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|1239|    |-		"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|    |1239|+			"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|1240|1240| 	});
|1241|1241| 
|1242|1242| 	let resCodes = g_ResourceData.GetCodes();
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 1.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|1237|1237| 		"\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), {
|1238|1238| 		"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|1239|1239| 		"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|1240|    |-	});
|    |1240|+		});
|1241|1241| 
|1242|1242| 	let resCodes = g_ResourceData.GetCodes();
|1243|1243| 	for (let r = 0; r < resCodes.length; ++r)
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|1723|1723| 	for (let rct of resourcesCounterTypes)
|1724|1724| 		for (let rt of resourcesTypes)
|1725|1725| 			reportObject[rt + rct.substr(9)] = playerStatistics[rct][rt];
|1726|    |-			// eg. rt = food rct.substr = Gathered rct = resourcesGathered
|    |1726|+	// eg. rt = food rct.substr = Gathered rct = resourcesGathered
|1727|1727| 
|1728|1728| 	reportObject.vegetarianFoodGathered = playerStatistics.resourcesGathered.vegetarianFood;
|1729|1729| 	for (let type of unitsClasses)

binaries/data/mods/public/gui/session/session.js
|1063| »   let·getPanelEntNameTooltip·=·panelEntState·=>·"[font=\"sans-bold-16\"]"·+·template.name.specific·+·"[/font]";
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'panelEntState' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
|1138| »   »   button.onpress·=·(function(i)·{·return·function()·{·performGroup((Engine.HotkeyIsPressed("selection.add")·?·"add"·:·"select"),·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
|1139| »   »   button.ondoublepress·=·(function(i)·{·return·function()·{·performGroup("snap",·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
|1140| »   »   button.onpressright·=·(function(i)·{·return·function()·{·performGroup("breakUp",·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

Link to build: https://jenkins.wildfiregames.com/job/phabricator_lint/1017/display/redirect

Harbormaster completed remote builds in B4906: Diff 5747.Feb 10 2018, 10:16 PM
temple added inline comments.Feb 10 2018, 10:39 PM
binaries/data/mods/public/gui/common/color.js
32 ↗(On Diff #5746)

Should also consider values outside of the 8bit range.

What does this mean?

elexis added inline comments.Feb 11 2018, 12:04 AM
binaries/data/mods/public/gui/common/color.js
34 ↗(On Diff #5747)

See previous comment about accepting or rejecting rather than changing the input.
I'd prefer that returning undefined if there are more or less than 2 spaces, or a NaN or a number outside of the wished range.

binaries/data/mods/public/gui/session/session.js
409 ↗(On Diff #5747)

I guess those colors are then black in case the user set garbage in user.cfg. But loading the default.cfg value would be a better fallback I suppose.
It was possible, something like Engine.ConfigDB_GetValue("default", "diplomacycolors.enemy") I suppose.

With that undefined return, it could do user || default. Emitting a warning if the user malformed the user.cfg color can be considered optional, but probably not bad practice as long as it doesn't show that in a practical infinite loop).

temple marked an inline comment as done.Feb 11 2018, 1:56 AM
temple added inline comments.
binaries/data/mods/public/gui/session/session.js
409 ↗(On Diff #5747)

Okay, I see now, thanks.

temple updated this revision to Diff 5752.Feb 11 2018, 1:58 AM

return undefined for bad strings
gui.session.diplomacycolors, seems a good location

Still todo the callback stuff.

Vulcan added a comment.Feb 11 2018, 2:00 AM

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

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (309 tests).....................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (309 tests).....................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

Link to build: https://jenkins.wildfiregames.com/job/phabricator/2724/display/redirect

Vulcan added a comment.Feb 11 2018, 2:02 AM
Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 5 tabs but found 4.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/options/options.js
| 104| 104| 			sprintf(
| 105| 105| 				option.min !== undefined && option.max !== undefined ?
| 106| 106| 					translateWithContext("option number", "Min: %(min)s, Max: %(max)s") :
| 107|    |-				option.min !== undefined && option.max === undefined ?
|    | 107|+					option.min !== undefined && option.max === undefined ?
| 108| 108| 					translateWithContext("option number", "Min: %(min)s") :
| 109| 109| 				option.min === undefined && option.max !== undefined ?
| 110| 110| 					translateWithContext("option number", "Max: %(max)s") :
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 6 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/options/options.js
| 105| 105| 				option.min !== undefined && option.max !== undefined ?
| 106| 106| 					translateWithContext("option number", "Min: %(min)s, Max: %(max)s") :
| 107| 107| 				option.min !== undefined && option.max === undefined ?
| 108|    |-					translateWithContext("option number", "Min: %(min)s") :
|    | 108|+						translateWithContext("option number", "Min: %(min)s") :
| 109| 109| 				option.min === undefined && option.max !== undefined ?
| 110| 110| 					translateWithContext("option number", "Max: %(max)s") :
| 111| 111| 					"",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 6 tabs but found 4.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/options/options.js
| 106| 106| 					translateWithContext("option number", "Min: %(min)s, Max: %(max)s") :
| 107| 107| 				option.min !== undefined && option.max === undefined ?
| 108| 108| 					translateWithContext("option number", "Min: %(min)s") :
| 109|    |-				option.min === undefined && option.max !== undefined ?
|    | 109|+						option.min === undefined && option.max !== undefined ?
| 110| 110| 					translateWithContext("option number", "Max: %(max)s") :
| 111| 111| 					"",
| 112| 112| 				{
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/options/options.js
| 107| 107| 				option.min !== undefined && option.max === undefined ?
| 108| 108| 					translateWithContext("option number", "Min: %(min)s") :
| 109| 109| 				option.min === undefined && option.max !== undefined ?
| 110|    |-					translateWithContext("option number", "Max: %(max)s") :
|    | 110|+							translateWithContext("option number", "Max: %(max)s") :
| 111| 111| 					"",
| 112| 112| 				{
| 113| 113| 					"min": option.min,
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/options/options.js
| 108| 108| 					translateWithContext("option number", "Min: %(min)s") :
| 109| 109| 				option.min === undefined && option.max !== undefined ?
| 110| 110| 					translateWithContext("option number", "Max: %(max)s") :
| 111|    |-					"",
|    | 111|+							"",
| 112| 112| 				{
| 113| 113| 					"min": option.min,
| 114| 114| 					"max": option.max

binaries/data/mods/public/gui/options/options.js
| 220| »   »   »   let·value·=·optionType.guiToValue(control);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'value' is already declared in the upper scope.
|    | [NORMAL] ESLintBear (object-curly-spacing):
|    | A space is required after '{'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/common/color.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/common/color.js
|   1|   1| /**
|   2|   2|  * Used to highlight hotkeys in tooltip descriptions.
|   3|   3|  */
|   4|    |-var g_HotkeyTags = {"color": "255 251 131" };
|    |   4|+var g_HotkeyTags = { "color": "255 251 131" };
|   5|   5| 
|   6|   6| /**
|   7|   7|  * Concatenate integer color values to a string (for use in GUI objects)

binaries/data/mods/public/gui/common/color.js
|  93| »   »   h·=·s·=·0;·//·achromatic
|    | [NORMAL] ESLintBear (no-multi-assign):
|    | Unexpected chained assignment.

binaries/data/mods/public/gui/common/color.js
|  98| »   »   switch·(max)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/common/color.js
| 144| »   if·(s·==·0)
|    | [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/color.js
| 145| »   »   r·=·g·=·b·=·l;·//·achromatic
|    | [NORMAL] ESLintBear (no-multi-assign):
|    | Unexpected chained assignment.

binaries/data/mods/public/gui/common/color.js
| 145| »   »   r·=·g·=·b·=·l;·//·achromatic
|    | [NORMAL] ESLintBear (no-multi-assign):
|    | Unexpected chained assignment.

binaries/data/mods/public/gui/session/menu.js
| 469| »   »   button.onPress·=·(function(player,·stance)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'stance' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 501| »   »   button.onPress·=·(function(i,·resCode,·button)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 501| »   »   button.onPress·=·(function(i,·resCode,·button)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'resCode' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 501| »   »   button.onPress·=·(function(i,·resCode,·button)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'button' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 544| »   button.onPress·=·(function(i)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 610| »   button.onPress·=·(function(i,·button)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 610| »   button.onPress·=·(function(i,·button)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'button' is already declared in the upper scope.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 6 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
| 408| 408| 				// Players see colors depending on diplomacy
| 409| 409| 				g_DisplayedPlayerColors[i] =
| 410| 410| 					g_ViewedPlayer == i ? sanitizeDiplomacyColor("self") :
| 411|    |-					g_Players[g_ViewedPlayer].isAlly[i] ? sanitizeDiplomacyColor("ally") :
|    | 411|+						g_Players[g_ViewedPlayer].isAlly[i] ? sanitizeDiplomacyColor("ally") :
| 412| 412| 					g_Players[g_ViewedPlayer].isNeutral[i] ? sanitizeDiplomacyColor("neutral") :
| 413| 413| 					sanitizeDiplomacyColor("enemy");
| 414| 414| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
| 409| 409| 				g_DisplayedPlayerColors[i] =
| 410| 410| 					g_ViewedPlayer == i ? sanitizeDiplomacyColor("self") :
| 411| 411| 					g_Players[g_ViewedPlayer].isAlly[i] ? sanitizeDiplomacyColor("ally") :
| 412|    |-					g_Players[g_ViewedPlayer].isNeutral[i] ? sanitizeDiplomacyColor("neutral") :
|    | 412|+							g_Players[g_ViewedPlayer].isNeutral[i] ? sanitizeDiplomacyColor("neutral") :
| 413| 413| 					sanitizeDiplomacyColor("enemy");
| 414| 414| 
| 415| 415| 		g_DisplayedPlayerColors[0] = g_Players[0].color;
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 8 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
| 410| 410| 					g_ViewedPlayer == i ? sanitizeDiplomacyColor("self") :
| 411| 411| 					g_Players[g_ViewedPlayer].isAlly[i] ? sanitizeDiplomacyColor("ally") :
| 412| 412| 					g_Players[g_ViewedPlayer].isNeutral[i] ? sanitizeDiplomacyColor("neutral") :
| 413|    |-					sanitizeDiplomacyColor("enemy");
|    | 413|+								sanitizeDiplomacyColor("enemy");
| 414| 414| 
| 415| 415| 		g_DisplayedPlayerColors[0] = g_Players[0].color;
| 416| 416| 	}
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|1243|1243| 
|1244|1244| 	let orderHotkeyTooltip = Object.keys(viewablePlayerStates).length <= 1 ? "" :
|1245|1245| 		"\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), {
|1246|    |-		"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|    |1246|+			"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|1247|1247| 		"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|1248|1248| 	});
|1249|1249| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|1244|1244| 	let orderHotkeyTooltip = Object.keys(viewablePlayerStates).length <= 1 ? "" :
|1245|1245| 		"\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), {
|1246|1246| 		"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|1247|    |-		"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|    |1247|+			"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|1248|1248| 	});
|1249|1249| 
|1250|1250| 	let resCodes = g_ResourceData.GetCodes();
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 1.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|1245|1245| 		"\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), {
|1246|1246| 		"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|1247|1247| 		"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|1248|    |-	});
|    |1248|+		});
|1249|1249| 
|1250|1250| 	let resCodes = g_ResourceData.GetCodes();
|1251|1251| 	for (let r = 0; r < resCodes.length; ++r)
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|1731|1731| 	for (let rct of resourcesCounterTypes)
|1732|1732| 		for (let rt of resourcesTypes)
|1733|1733| 			reportObject[rt + rct.substr(9)] = playerStatistics[rct][rt];
|1734|    |-			// eg. rt = food rct.substr = Gathered rct = resourcesGathered
|    |1734|+	// eg. rt = food rct.substr = Gathered rct = resourcesGathered
|1735|1735| 
|1736|1736| 	reportObject.vegetarianFoodGathered = playerStatistics.resourcesGathered.vegetarianFood;
|1737|1737| 	for (let type of unitsClasses)

binaries/data/mods/public/gui/session/session.js
|1071| »   let·getPanelEntNameTooltip·=·panelEntState·=>·"[font=\"sans-bold-16\"]"·+·template.name.specific·+·"[/font]";
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'panelEntState' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
|1146| »   »   button.onpress·=·(function(i)·{·return·function()·{·performGroup((Engine.HotkeyIsPressed("selection.add")·?·"add"·:·"select"),·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
|1147| »   »   button.ondoublepress·=·(function(i)·{·return·function()·{·performGroup("snap",·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
|1148| »   »   button.onpressright·=·(function(i)·{·return·function()·{·performGroup("breakUp",·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

Link to build: https://jenkins.wildfiregames.com/job/phabricator_lint/1019/display/redirect

Harbormaster completed remote builds in B4911: Diff 5752.Feb 11 2018, 2:02 AM
elexis added a comment.Feb 11 2018, 2:07 AM

looks good

binaries/data/mods/public/gui/common/color.js
37 ↗(On Diff #5752)

good

binaries/data/mods/public/gui/options/options.js
71 ↗(On Diff #5752)

"color"

82 ↗(On Diff #5752)

We had changed the options dialog to only change user input when it was about capping a number. Otherwise it would just leave it as is and turn the background red. This way the user can fix a typo instead of having the input replaced altogether. Should do the same here so that the user doesn't lose the color he typed.

binaries/data/mods/public/gui/session/session.js
393 ↗(On Diff #5752)

getDiplomacyColor? (sanitization is repairing, but we don't do that)

temple marked 4 inline comments as done.Feb 11 2018, 5:54 AM
temple added inline comments.
binaries/data/mods/public/gui/session/menu.js
1147 ↗(On Diff #5744)

We don't close the options page often so it seems fine to just update everything.

temple updated this revision to Diff 5755.Feb 11 2018, 5:55 AM

Use the different callback as suggested by elexis.
Could go in a separate diff, but anyway..

temple added a comment.Feb 11 2018, 5:59 AM

rP19519 is the commit with the open concern.

Vulcan added a comment.Feb 11 2018, 6:22 AM

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

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (309 tests).....................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (309 tests).....................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

Link to build: https://jenkins.wildfiregames.com/job/phabricator/2726/display/redirect

Harbormaster completed remote builds in B4915: Diff 5755.Feb 11 2018, 6:25 AM
Vulcan added a comment.Feb 11 2018, 6:25 AM
Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (object-curly-spacing):
|    | A space is required after '{'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/common/color.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/common/color.js
|   1|   1| /**
|   2|   2|  * Used to highlight hotkeys in tooltip descriptions.
|   3|   3|  */
|   4|    |-var g_HotkeyTags = {"color": "255 251 131" };
|    |   4|+var g_HotkeyTags = { "color": "255 251 131" };
|   5|   5| 
|   6|   6| /**
|   7|   7|  * Concatenate integer color values to a string (for use in GUI objects)

binaries/data/mods/public/gui/common/color.js
|  93| »   »   h·=·s·=·0;·//·achromatic
|    | [NORMAL] ESLintBear (no-multi-assign):
|    | Unexpected chained assignment.

binaries/data/mods/public/gui/common/color.js
|  98| »   »   switch·(max)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/common/color.js
| 144| »   if·(s·==·0)
|    | [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/color.js
| 145| »   »   r·=·g·=·b·=·l;·//·achromatic
|    | [NORMAL] ESLintBear (no-multi-assign):
|    | Unexpected chained assignment.

binaries/data/mods/public/gui/common/color.js
| 145| »   »   r·=·g·=·b·=·l;·//·achromatic
|    | [NORMAL] ESLintBear (no-multi-assign):
|    | Unexpected chained assignment.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 5 tabs but found 4.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/options/options.js
| 104| 104| 			sprintf(
| 105| 105| 				option.min !== undefined && option.max !== undefined ?
| 106| 106| 					translateWithContext("option number", "Min: %(min)s, Max: %(max)s") :
| 107|    |-				option.min !== undefined && option.max === undefined ?
|    | 107|+					option.min !== undefined && option.max === undefined ?
| 108| 108| 					translateWithContext("option number", "Min: %(min)s") :
| 109| 109| 				option.min === undefined && option.max !== undefined ?
| 110| 110| 					translateWithContext("option number", "Max: %(max)s") :
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 6 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/options/options.js
| 105| 105| 				option.min !== undefined && option.max !== undefined ?
| 106| 106| 					translateWithContext("option number", "Min: %(min)s, Max: %(max)s") :
| 107| 107| 				option.min !== undefined && option.max === undefined ?
| 108|    |-					translateWithContext("option number", "Min: %(min)s") :
|    | 108|+						translateWithContext("option number", "Min: %(min)s") :
| 109| 109| 				option.min === undefined && option.max !== undefined ?
| 110| 110| 					translateWithContext("option number", "Max: %(max)s") :
| 111| 111| 					"",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 6 tabs but found 4.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/options/options.js
| 106| 106| 					translateWithContext("option number", "Min: %(min)s, Max: %(max)s") :
| 107| 107| 				option.min !== undefined && option.max === undefined ?
| 108| 108| 					translateWithContext("option number", "Min: %(min)s") :
| 109|    |-				option.min === undefined && option.max !== undefined ?
|    | 109|+						option.min === undefined && option.max !== undefined ?
| 110| 110| 					translateWithContext("option number", "Max: %(max)s") :
| 111| 111| 					"",
| 112| 112| 				{
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/options/options.js
| 107| 107| 				option.min !== undefined && option.max === undefined ?
| 108| 108| 					translateWithContext("option number", "Min: %(min)s") :
| 109| 109| 				option.min === undefined && option.max !== undefined ?
| 110|    |-					translateWithContext("option number", "Max: %(max)s") :
|    | 110|+							translateWithContext("option number", "Max: %(max)s") :
| 111| 111| 					"",
| 112| 112| 				{
| 113| 113| 					"min": option.min,
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/options/options.js
| 108| 108| 					translateWithContext("option number", "Min: %(min)s") :
| 109| 109| 				option.min === undefined && option.max !== undefined ?
| 110| 110| 					translateWithContext("option number", "Max: %(max)s") :
| 111|    |-					"",
|    | 111|+							"",
| 112| 112| 				{
| 113| 113| 					"min": option.min,
| 114| 114| 					"max": option.max

binaries/data/mods/public/gui/options/options.js
| 220| »   »   »   let·value·=·optionType.guiToValue(control);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'value' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 478| »   »   button.onPress·=·(function(player,·stance)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'stance' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 510| »   »   button.onPress·=·(function(i,·resCode,·button)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 510| »   »   button.onPress·=·(function(i,·resCode,·button)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'resCode' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 510| »   »   button.onPress·=·(function(i,·resCode,·button)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'button' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 553| »   button.onPress·=·(function(i)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 619| »   button.onPress·=·(function(i,·button)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 619| »   button.onPress·=·(function(i,·button)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'button' is already declared in the upper scope.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 6 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
| 408| 408| 				// Players see colors depending on diplomacy
| 409| 409| 				g_DisplayedPlayerColors[i] =
| 410| 410| 					g_ViewedPlayer == i ? getDiplomacyColor("self") :
| 411|    |-					g_Players[g_ViewedPlayer].isAlly[i] ? getDiplomacyColor("ally") :
|    | 411|+						g_Players[g_ViewedPlayer].isAlly[i] ? getDiplomacyColor("ally") :
| 412| 412| 					g_Players[g_ViewedPlayer].isNeutral[i] ? getDiplomacyColor("neutral") :
| 413| 413| 					getDiplomacyColor("enemy");
| 414| 414| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
| 409| 409| 				g_DisplayedPlayerColors[i] =
| 410| 410| 					g_ViewedPlayer == i ? getDiplomacyColor("self") :
| 411| 411| 					g_Players[g_ViewedPlayer].isAlly[i] ? getDiplomacyColor("ally") :
| 412|    |-					g_Players[g_ViewedPlayer].isNeutral[i] ? getDiplomacyColor("neutral") :
|    | 412|+							g_Players[g_ViewedPlayer].isNeutral[i] ? getDiplomacyColor("neutral") :
| 413| 413| 					getDiplomacyColor("enemy");
| 414| 414| 
| 415| 415| 		g_DisplayedPlayerColors[0] = g_Players[0].color;
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 8 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
| 410| 410| 					g_ViewedPlayer == i ? getDiplomacyColor("self") :
| 411| 411| 					g_Players[g_ViewedPlayer].isAlly[i] ? getDiplomacyColor("ally") :
| 412| 412| 					g_Players[g_ViewedPlayer].isNeutral[i] ? getDiplomacyColor("neutral") :
| 413|    |-					getDiplomacyColor("enemy");
|    | 413|+								getDiplomacyColor("enemy");
| 414| 414| 
| 415| 415| 		g_DisplayedPlayerColors[0] = g_Players[0].color;
| 416| 416| 	}
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|1237|1237| 
|1238|1238| 	let orderHotkeyTooltip = Object.keys(viewablePlayerStates).length <= 1 ? "" :
|1239|1239| 		"\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), {
|1240|    |-		"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|    |1240|+			"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|1241|1241| 		"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|1242|1242| 	});
|1243|1243| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|1238|1238| 	let orderHotkeyTooltip = Object.keys(viewablePlayerStates).length <= 1 ? "" :
|1239|1239| 		"\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), {
|1240|1240| 		"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|1241|    |-		"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|    |1241|+			"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|1242|1242| 	});
|1243|1243| 
|1244|1244| 	let resCodes = g_ResourceData.GetCodes();
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 1.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|1239|1239| 		"\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), {
|1240|1240| 		"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|1241|1241| 		"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|1242|    |-	});
|    |1242|+		});
|1243|1243| 
|1244|1244| 	let resCodes = g_ResourceData.GetCodes();
|1245|1245| 	for (let r = 0; r < resCodes.length; ++r)
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js
|1734|1734| 	for (let rct of resourcesCounterTypes)
|1735|1735| 		for (let rt of resourcesTypes)
|1736|1736| 			reportObject[rt + rct.substr(9)] = playerStatistics[rct][rt];
|1737|    |-			// eg. rt = food rct.substr = Gathered rct = resourcesGathered
|    |1737|+	// eg. rt = food rct.substr = Gathered rct = resourcesGathered
|1738|1738| 
|1739|1739| 	reportObject.vegetarianFoodGathered = playerStatistics.resourcesGathered.vegetarianFood;
|1740|1740| 	for (let type of unitsClasses)

binaries/data/mods/public/gui/session/session.js
|1065| »   let·getPanelEntNameTooltip·=·panelEntState·=>·"[font=\"sans-bold-16\"]"·+·template.name.specific·+·"[/font]";
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'panelEntState' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
|1140| »   »   button.onpress·=·(function(i)·{·return·function()·{·performGroup((Engine.HotkeyIsPressed("selection.add")·?·"add"·:·"select"),·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
|1141| »   »   button.ondoublepress·=·(function(i)·{·return·function()·{·performGroup("snap",·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
|1142| »   »   button.onpressright·=·(function(i)·{·return·function()·{·performGroup("breakUp",·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

Link to build: https://jenkins.wildfiregames.com/job/phabricator_lint/1021/display/redirect

ffffffff added a subscriber: ffffffff.Feb 11 2018, 6:37 PM
In D1291#52882, @temple wrote:

Added back rgbString and included the color preview:

Made guiToRgbColor always return a valid color object. Not sure if that's the best option but it makes everything else easier.

i guess they go Game Play Categorie

temple updated this revision to Diff 5884.Feb 22 2018, 9:03 PM
temple edited the test plan for this revision. (Show Details)

Moved options to the In-Game tab.
Use contrasting text color so we don't see e.g. white text on a white background.

In the screenshot, the first box "255 255 255w" is an invalid color so it uses the same ModernDarkBoxWhiteInvalid sprite that's used e.g. if you enter an invalid batch training size. Regular text entry uses white top and bottom outlines (as seen in batch training size), but if we used that here that we wouldn't be able to distinguish between an invalid color and a valid but maroon color (like the ally color in the screenshot). So I think it's best to leave the color boxes without the top and bottom outlines. (Also, I couldn't figure out a nice way to add the outlines anyway.)

Vulcan added a comment.Feb 23 2018, 12:59 AM

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

Linter detected issues:
Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (object-curly-spacing):
|    | A space is required after '{'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/color.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/color.js
|   1|   1| /**
|   2|   2|  * Used to highlight hotkeys in tooltip descriptions.
|   3|   3|  */
|   4|    |-var g_HotkeyTags = {"color": "255 251 131" };
|    |   4|+var g_HotkeyTags = { "color": "255 251 131" };
|   5|   5| 
|   6|   6| /**
|   7|   7|  * Concatenate integer color values to a string (for use in GUI objects)

binaries/data/mods/public/gui/common/color.js
|  43| function·isColorLight(color)·{
|    | [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/color.js
| 104| »   »   h·=·s·=·0;·//·achromatic
|    | [NORMAL] ESLintBear (no-multi-assign):
|    | Unexpected chained assignment.

binaries/data/mods/public/gui/common/color.js
| 109| »   »   switch·(max)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/common/color.js
| 155| »   if·(s·==·0)
|    | [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/color.js
| 156| »   »   r·=·g·=·b·=·l;·//·achromatic
|    | [NORMAL] ESLintBear (no-multi-assign):
|    | Unexpected chained assignment.

binaries/data/mods/public/gui/common/color.js
| 156| »   »   r·=·g·=·b·=·l;·//·achromatic
|    | [NORMAL] ESLintBear (no-multi-assign):
|    | Unexpected chained assignment.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 5 tabs but found 4.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
| 107| 107| 			sprintf(
| 108| 108| 				option.min !== undefined && option.max !== undefined ?
| 109| 109| 					translateWithContext("option number", "Min: %(min)s, Max: %(max)s") :
| 110|    |-				option.min !== undefined && option.max === undefined ?
|    | 110|+					option.min !== undefined && option.max === undefined ?
| 111| 111| 					translateWithContext("option number", "Min: %(min)s") :
| 112| 112| 				option.min === undefined && option.max !== undefined ?
| 113| 113| 					translateWithContext("option number", "Max: %(max)s") :
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 6 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
| 108| 108| 				option.min !== undefined && option.max !== undefined ?
| 109| 109| 					translateWithContext("option number", "Min: %(min)s, Max: %(max)s") :
| 110| 110| 				option.min !== undefined && option.max === undefined ?
| 111|    |-					translateWithContext("option number", "Min: %(min)s") :
|    | 111|+						translateWithContext("option number", "Min: %(min)s") :
| 112| 112| 				option.min === undefined && option.max !== undefined ?
| 113| 113| 					translateWithContext("option number", "Max: %(max)s") :
| 114| 114| 					"",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 6 tabs but found 4.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
| 109| 109| 					translateWithContext("option number", "Min: %(min)s, Max: %(max)s") :
| 110| 110| 				option.min !== undefined && option.max === undefined ?
| 111| 111| 					translateWithContext("option number", "Min: %(min)s") :
| 112|    |-				option.min === undefined && option.max !== undefined ?
|    | 112|+						option.min === undefined && option.max !== undefined ?
| 113| 113| 					translateWithContext("option number", "Max: %(max)s") :
| 114| 114| 					"",
| 115| 115| 				{
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
| 110| 110| 				option.min !== undefined && option.max === undefined ?
| 111| 111| 					translateWithContext("option number", "Min: %(min)s") :
| 112| 112| 				option.min === undefined && option.max !== undefined ?
| 113|    |-					translateWithContext("option number", "Max: %(max)s") :
|    | 113|+							translateWithContext("option number", "Max: %(max)s") :
| 114| 114| 					"",
| 115| 115| 				{
| 116| 116| 					"min": option.min,
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
| 111| 111| 					translateWithContext("option number", "Min: %(min)s") :
| 112| 112| 				option.min === undefined && option.max !== undefined ?
| 113| 113| 					translateWithContext("option number", "Max: %(max)s") :
| 114|    |-					"",
|    | 114|+							"",
| 115| 115| 				{
| 116| 116| 					"min": option.min,
| 117| 117| 					"max": option.max

binaries/data/mods/public/gui/options/options.js
| 223| »   »   »   let·value·=·optionType.guiToValue(control);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'value' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 478| »   »   button.onPress·=·(function(player,·stance)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'stance' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 510| »   »   button.onPress·=·(function(i,·resCode,·button)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 510| »   »   button.onPress·=·(function(i,·resCode,·button)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'resCode' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 510| »   »   button.onPress·=·(function(i,·resCode,·button)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'button' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 553| »   button.onPress·=·(function(i)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 619| »   button.onPress·=·(function(i,·button)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 619| »   button.onPress·=·(function(i,·button)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'button' is already declared in the upper scope.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 6 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
| 411| 411| 				// Players see colors depending on diplomacy
| 412| 412| 				g_DisplayedPlayerColors[i] =
| 413| 413| 					g_ViewedPlayer == i ? getDiplomacyColor("self") :
| 414|    |-					g_Players[g_ViewedPlayer].isAlly[i] ? getDiplomacyColor("ally") :
|    | 414|+						g_Players[g_ViewedPlayer].isAlly[i] ? getDiplomacyColor("ally") :
| 415| 415| 					g_Players[g_ViewedPlayer].isNeutral[i] ? getDiplomacyColor("neutral") :
| 416| 416| 					getDiplomacyColor("enemy");
| 417| 417| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
| 412| 412| 				g_DisplayedPlayerColors[i] =
| 413| 413| 					g_ViewedPlayer == i ? getDiplomacyColor("self") :
| 414| 414| 					g_Players[g_ViewedPlayer].isAlly[i] ? getDiplomacyColor("ally") :
| 415|    |-					g_Players[g_ViewedPlayer].isNeutral[i] ? getDiplomacyColor("neutral") :
|    | 415|+							g_Players[g_ViewedPlayer].isNeutral[i] ? getDiplomacyColor("neutral") :
| 416| 416| 					getDiplomacyColor("enemy");
| 417| 417| 
| 418| 418| 		g_DisplayedPlayerColors[0] = g_Players[0].color;
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 8 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
| 413| 413| 					g_ViewedPlayer == i ? getDiplomacyColor("self") :
| 414| 414| 					g_Players[g_ViewedPlayer].isAlly[i] ? getDiplomacyColor("ally") :
| 415| 415| 					g_Players[g_ViewedPlayer].isNeutral[i] ? getDiplomacyColor("neutral") :
| 416|    |-					getDiplomacyColor("enemy");
|    | 416|+								getDiplomacyColor("enemy");
| 417| 417| 
| 418| 418| 		g_DisplayedPlayerColors[0] = g_Players[0].color;
| 419| 419| 	}
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|1244|1244| 
|1245|1245| 	let orderHotkeyTooltip = Object.keys(viewablePlayerStates).length <= 1 ? "" :
|1246|1246| 		"\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), {
|1247|    |-		"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|    |1247|+			"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|1248|1248| 		"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|1249|1249| 	});
|1250|1250| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|1245|1245| 	let orderHotkeyTooltip = Object.keys(viewablePlayerStates).length <= 1 ? "" :
|1246|1246| 		"\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), {
|1247|1247| 		"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|1248|    |-		"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|    |1248|+			"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|1249|1249| 	});
|1250|1250| 
|1251|1251| 	let resCodes = g_ResourceData.GetCodes();
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 1.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|1246|1246| 		"\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), {
|1247|1247| 		"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|1248|1248| 		"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|1249|    |-	});
|    |1249|+		});
|1250|1250| 
|1251|1251| 	let resCodes = g_ResourceData.GetCodes();
|1252|1252| 	for (let r = 0; r < resCodes.length; ++r)
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|1744|1744| 	for (let rct of resourcesCounterTypes)
|1745|1745| 		for (let rt of resourcesTypes)
|1746|1746| 			reportObject[rt + rct.substr(9)] = playerStatistics[rct][rt];
|1747|    |-			// eg. rt = food rct.substr = Gathered rct = resourcesGathered
|    |1747|+	// eg. rt = food rct.substr = Gathered rct = resourcesGathered
|1748|1748| 
|1749|1749| 	reportObject.vegetarianFoodGathered = playerStatistics.resourcesGathered.vegetarianFood;
|1750|1750| 	for (let type of unitsClasses)

binaries/data/mods/public/gui/session/session.js
|1072| »   let·getPanelEntNameTooltip·=·panelEntState·=>·"[font=\"sans-bold-16\"]"·+·template.name.specific·+·"[/font]";
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'panelEntState' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
|1147| »   »   button.onpress·=·(function(i)·{·return·function()·{·performGroup((Engine.HotkeyIsPressed("selection.add")·?·"add"·:·"select"),·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
|1148| »   »   button.ondoublepress·=·(function(i)·{·return·function()·{·performGroup("snap",·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
|1149| »   »   button.onpressright·=·(function(i)·{·return·function()·{·performGroup("breakUp",·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

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

Harbormaster completed remote builds in B5134: Diff 5884.Feb 23 2018, 12:59 AM
Silier added a subscriber: Silier.Feb 24 2018, 4:21 PM

Save button is popping error.

temple updated this revision to Diff 5917.Feb 24 2018, 5:12 PM

if control

temple added inline comments.Feb 24 2018, 5:15 PM
binaries/data/mods/public/gui/options/options.js
87 ↗(On Diff #5917)

add this back

Vulcan added a comment.Feb 24 2018, 11:58 PM

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

Linter detected issues:
Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (object-curly-spacing):
|    | A space is required after '{'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/color.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/color.js
|   1|   1| /**
|   2|   2|  * Used to highlight hotkeys in tooltip descriptions.
|   3|   3|  */
|   4|    |-var g_HotkeyTags = {"color": "255 251 131" };
|    |   4|+var g_HotkeyTags = { "color": "255 251 131" };
|   5|   5| 
|   6|   6| /**
|   7|   7|  * Concatenate integer color values to a string (for use in GUI objects)

binaries/data/mods/public/gui/common/color.js
|  43| function·isColorLight(color)·{
|    | [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/color.js
| 104| »   »   h·=·s·=·0;·//·achromatic
|    | [NORMAL] ESLintBear (no-multi-assign):
|    | Unexpected chained assignment.

binaries/data/mods/public/gui/common/color.js
| 109| »   »   switch·(max)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/common/color.js
| 155| »   if·(s·==·0)
|    | [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/color.js
| 156| »   »   r·=·g·=·b·=·l;·//·achromatic
|    | [NORMAL] ESLintBear (no-multi-assign):
|    | Unexpected chained assignment.

binaries/data/mods/public/gui/common/color.js
| 156| »   »   r·=·g·=·b·=·l;·//·achromatic
|    | [NORMAL] ESLintBear (no-multi-assign):
|    | Unexpected chained assignment.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 5 tabs but found 4.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
| 109| 109| 			sprintf(
| 110| 110| 				option.min !== undefined && option.max !== undefined ?
| 111| 111| 					translateWithContext("option number", "Min: %(min)s, Max: %(max)s") :
| 112|    |-				option.min !== undefined && option.max === undefined ?
|    | 112|+					option.min !== undefined && option.max === undefined ?
| 113| 113| 					translateWithContext("option number", "Min: %(min)s") :
| 114| 114| 				option.min === undefined && option.max !== undefined ?
| 115| 115| 					translateWithContext("option number", "Max: %(max)s") :
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 6 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
| 110| 110| 				option.min !== undefined && option.max !== undefined ?
| 111| 111| 					translateWithContext("option number", "Min: %(min)s, Max: %(max)s") :
| 112| 112| 				option.min !== undefined && option.max === undefined ?
| 113|    |-					translateWithContext("option number", "Min: %(min)s") :
|    | 113|+						translateWithContext("option number", "Min: %(min)s") :
| 114| 114| 				option.min === undefined && option.max !== undefined ?
| 115| 115| 					translateWithContext("option number", "Max: %(max)s") :
| 116| 116| 					"",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 6 tabs but found 4.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
| 111| 111| 					translateWithContext("option number", "Min: %(min)s, Max: %(max)s") :
| 112| 112| 				option.min !== undefined && option.max === undefined ?
| 113| 113| 					translateWithContext("option number", "Min: %(min)s") :
| 114|    |-				option.min === undefined && option.max !== undefined ?
|    | 114|+						option.min === undefined && option.max !== undefined ?
| 115| 115| 					translateWithContext("option number", "Max: %(max)s") :
| 116| 116| 					"",
| 117| 117| 				{
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
| 112| 112| 				option.min !== undefined && option.max === undefined ?
| 113| 113| 					translateWithContext("option number", "Min: %(min)s") :
| 114| 114| 				option.min === undefined && option.max !== undefined ?
| 115|    |-					translateWithContext("option number", "Max: %(max)s") :
|    | 115|+							translateWithContext("option number", "Max: %(max)s") :
| 116| 116| 					"",
| 117| 117| 				{
| 118| 118| 					"min": option.min,
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
| 113| 113| 					translateWithContext("option number", "Min: %(min)s") :
| 114| 114| 				option.min === undefined && option.max !== undefined ?
| 115| 115| 					translateWithContext("option number", "Max: %(max)s") :
| 116|    |-					"",
|    | 116|+							"",
| 117| 117| 				{
| 118| 118| 					"min": option.min,
| 119| 119| 					"max": option.max

binaries/data/mods/public/gui/options/options.js
| 225| »   »   »   let·value·=·optionType.guiToValue(control);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'value' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 486| »   »   button.onPress·=·(function(player,·stance)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'stance' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 518| »   »   button.onPress·=·(function(i,·resCode,·button)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 518| »   »   button.onPress·=·(function(i,·resCode,·button)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'resCode' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 518| »   »   button.onPress·=·(function(i,·resCode,·button)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'button' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 561| »   button.onPress·=·(function(i)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 627| »   button.onPress·=·(function(i,·button)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 627| »   button.onPress·=·(function(i,·button)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'button' is already declared in the upper scope.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 6 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
| 411| 411| 				// Players see colors depending on diplomacy
| 412| 412| 				g_DisplayedPlayerColors[i] =
| 413| 413| 					g_ViewedPlayer == i ? getDiplomacyColor("self") :
| 414|    |-					g_Players[g_ViewedPlayer].isAlly[i] ? getDiplomacyColor("ally") :
|    | 414|+						g_Players[g_ViewedPlayer].isAlly[i] ? getDiplomacyColor("ally") :
| 415| 415| 					g_Players[g_ViewedPlayer].isNeutral[i] ? getDiplomacyColor("neutral") :
| 416| 416| 					getDiplomacyColor("enemy");
| 417| 417| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
| 412| 412| 				g_DisplayedPlayerColors[i] =
| 413| 413| 					g_ViewedPlayer == i ? getDiplomacyColor("self") :
| 414| 414| 					g_Players[g_ViewedPlayer].isAlly[i] ? getDiplomacyColor("ally") :
| 415|    |-					g_Players[g_ViewedPlayer].isNeutral[i] ? getDiplomacyColor("neutral") :
|    | 415|+							g_Players[g_ViewedPlayer].isNeutral[i] ? getDiplomacyColor("neutral") :
| 416| 416| 					getDiplomacyColor("enemy");
| 417| 417| 
| 418| 418| 		g_DisplayedPlayerColors[0] = g_Players[0].color;
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 8 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
| 413| 413| 					g_ViewedPlayer == i ? getDiplomacyColor("self") :
| 414| 414| 					g_Players[g_ViewedPlayer].isAlly[i] ? getDiplomacyColor("ally") :
| 415| 415| 					g_Players[g_ViewedPlayer].isNeutral[i] ? getDiplomacyColor("neutral") :
| 416|    |-					getDiplomacyColor("enemy");
|    | 416|+								getDiplomacyColor("enemy");
| 417| 417| 
| 418| 418| 		g_DisplayedPlayerColors[0] = g_Players[0].color;
| 419| 419| 	}
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 4 tabs but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
| 658| 658| 					"civ": setStringTags(g_CivData[g_Players[g_ViewedPlayer].civ].Name, { "font": "sans-bold-stroke-14" }),
| 659| 659| 					"hotkey_civinfo": colorizeHotkey("%(hotkey)s", "civinfo"),
| 660| 660| 					"hotkey_structree": colorizeHotkey("%(hotkey)s", "structree")
| 661|    |-			});
|    | 661|+				});
| 662| 662| 	}
| 663| 663| 
| 664| 664| 	Engine.GetGUIObjectByName("optionFollowPlayer").hidden = !g_IsObserver || !isPlayer;
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|1248|1248| 
|1249|1249| 	let orderHotkeyTooltip = Object.keys(viewablePlayerStates).length <= 1 ? "" :
|1250|1250| 		"\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), {
|1251|    |-		"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|    |1251|+			"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|1252|1252| 		"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|1253|1253| 	});
|1254|1254| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|1249|1249| 	let orderHotkeyTooltip = Object.keys(viewablePlayerStates).length <= 1 ? "" :
|1250|1250| 		"\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), {
|1251|1251| 		"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|1252|    |-		"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|    |1252|+			"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|1253|1253| 	});
|1254|1254| 
|1255|1255| 	let resCodes = g_ResourceData.GetCodes();
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 1.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|1250|1250| 		"\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), {
|1251|1251| 		"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|1252|1252| 		"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|1253|    |-	});
|    |1253|+		});
|1254|1254| 
|1255|1255| 	let resCodes = g_ResourceData.GetCodes();
|1256|1256| 	for (let r = 0; r < resCodes.length; ++r)
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|1748|1748| 	for (let rct of resourcesCounterTypes)
|1749|1749| 		for (let rt of resourcesTypes)
|1750|1750| 			reportObject[rt + rct.substr(9)] = playerStatistics[rct][rt];
|1751|    |-			// eg. rt = food rct.substr = Gathered rct = resourcesGathered
|    |1751|+	// eg. rt = food rct.substr = Gathered rct = resourcesGathered
|1752|1752| 
|1753|1753| 	reportObject.vegetarianFoodGathered = playerStatistics.resourcesGathered.vegetarianFood;
|1754|1754| 	for (let type of unitsClasses)

binaries/data/mods/public/gui/session/session.js
|1076| »   let·getPanelEntNameTooltip·=·panelEntState·=>·"[font=\"sans-bold-16\"]"·+·template.name.specific·+·"[/font]";
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'panelEntState' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
|1151| »   »   button.onpress·=·(function(i)·{·return·function()·{·performGroup((Engine.HotkeyIsPressed("selection.add")·?·"add"·:·"select"),·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
|1152| »   »   button.ondoublepress·=·(function(i)·{·return·function()·{·performGroup("snap",·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
|1153| »   »   button.onpressright·=·(function(i)·{·return·function()·{·performGroup("breakUp",·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

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

Harbormaster completed remote builds in B5185: Diff 5917.Feb 24 2018, 11:58 PM
temple updated this revision to Diff 5951.Feb 26 2018, 4:48 AM
temple marked an inline comment as done.

gui change

Vulcan added a comment.Feb 26 2018, 5:29 AM

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

Linter detected issues:
Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (object-curly-spacing):
|    | A space is required after '{'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/color.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/color.js
|   1|   1| /**
|   2|   2|  * Used to highlight hotkeys in tooltip descriptions.
|   3|   3|  */
|   4|    |-var g_HotkeyTags = {"color": "255 251 131" };
|    |   4|+var g_HotkeyTags = { "color": "255 251 131" };
|   5|   5| 
|   6|   6| /**
|   7|   7|  * Concatenate integer color values to a string (for use in GUI objects)

binaries/data/mods/public/gui/common/color.js
|  93| »   »   h·=·s·=·0;·//·achromatic
|    | [NORMAL] ESLintBear (no-multi-assign):
|    | Unexpected chained assignment.

binaries/data/mods/public/gui/common/color.js
|  98| »   »   switch·(max)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/common/color.js
| 144| »   if·(s·==·0)
|    | [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/color.js
| 145| »   »   r·=·g·=·b·=·l;·//·achromatic
|    | [NORMAL] ESLintBear (no-multi-assign):
|    | Unexpected chained assignment.

binaries/data/mods/public/gui/common/color.js
| 145| »   »   r·=·g·=·b·=·l;·//·achromatic
|    | [NORMAL] ESLintBear (no-multi-assign):
|    | Unexpected chained assignment.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 5 tabs but found 4.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
| 110| 110| 			sprintf(
| 111| 111| 				option.min !== undefined && option.max !== undefined ?
| 112| 112| 					translateWithContext("option number", "Min: %(min)s, Max: %(max)s") :
| 113|    |-				option.min !== undefined && option.max === undefined ?
|    | 113|+					option.min !== undefined && option.max === undefined ?
| 114| 114| 					translateWithContext("option number", "Min: %(min)s") :
| 115| 115| 				option.min === undefined && option.max !== undefined ?
| 116| 116| 					translateWithContext("option number", "Max: %(max)s") :
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 6 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
| 111| 111| 				option.min !== undefined && option.max !== undefined ?
| 112| 112| 					translateWithContext("option number", "Min: %(min)s, Max: %(max)s") :
| 113| 113| 				option.min !== undefined && option.max === undefined ?
| 114|    |-					translateWithContext("option number", "Min: %(min)s") :
|    | 114|+						translateWithContext("option number", "Min: %(min)s") :
| 115| 115| 				option.min === undefined && option.max !== undefined ?
| 116| 116| 					translateWithContext("option number", "Max: %(max)s") :
| 117| 117| 					"",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 6 tabs but found 4.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
| 112| 112| 					translateWithContext("option number", "Min: %(min)s, Max: %(max)s") :
| 113| 113| 				option.min !== undefined && option.max === undefined ?
| 114| 114| 					translateWithContext("option number", "Min: %(min)s") :
| 115|    |-				option.min === undefined && option.max !== undefined ?
|    | 115|+						option.min === undefined && option.max !== undefined ?
| 116| 116| 					translateWithContext("option number", "Max: %(max)s") :
| 117| 117| 					"",
| 118| 118| 				{
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
| 113| 113| 				option.min !== undefined && option.max === undefined ?
| 114| 114| 					translateWithContext("option number", "Min: %(min)s") :
| 115| 115| 				option.min === undefined && option.max !== undefined ?
| 116|    |-					translateWithContext("option number", "Max: %(max)s") :
|    | 116|+							translateWithContext("option number", "Max: %(max)s") :
| 117| 117| 					"",
| 118| 118| 				{
| 119| 119| 					"min": option.min,
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
| 114| 114| 					translateWithContext("option number", "Min: %(min)s") :
| 115| 115| 				option.min === undefined && option.max !== undefined ?
| 116| 116| 					translateWithContext("option number", "Max: %(max)s") :
| 117|    |-					"",
|    | 117|+							"",
| 118| 118| 				{
| 119| 119| 					"min": option.min,
| 120| 120| 					"max": option.max

binaries/data/mods/public/gui/options/options.js
| 226| »   »   »   let·value·=·optionType.guiToValue(control);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'value' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 486| »   »   button.onPress·=·(function(player,·stance)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'stance' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 518| »   »   button.onPress·=·(function(i,·resCode,·button)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 518| »   »   button.onPress·=·(function(i,·resCode,·button)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'resCode' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 518| »   »   button.onPress·=·(function(i,·resCode,·button)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'button' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 561| »   button.onPress·=·(function(i)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 627| »   button.onPress·=·(function(i,·button)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 627| »   button.onPress·=·(function(i,·button)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'button' is already declared in the upper scope.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 6 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
| 411| 411| 				// Players see colors depending on diplomacy
| 412| 412| 				g_DisplayedPlayerColors[i] =
| 413| 413| 					g_ViewedPlayer == i ? getDiplomacyColor("self") :
| 414|    |-					g_Players[g_ViewedPlayer].isAlly[i] ? getDiplomacyColor("ally") :
|    | 414|+						g_Players[g_ViewedPlayer].isAlly[i] ? getDiplomacyColor("ally") :
| 415| 415| 					g_Players[g_ViewedPlayer].isNeutral[i] ? getDiplomacyColor("neutral") :
| 416| 416| 					getDiplomacyColor("enemy");
| 417| 417| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
| 412| 412| 				g_DisplayedPlayerColors[i] =
| 413| 413| 					g_ViewedPlayer == i ? getDiplomacyColor("self") :
| 414| 414| 					g_Players[g_ViewedPlayer].isAlly[i] ? getDiplomacyColor("ally") :
| 415|    |-					g_Players[g_ViewedPlayer].isNeutral[i] ? getDiplomacyColor("neutral") :
|    | 415|+							g_Players[g_ViewedPlayer].isNeutral[i] ? getDiplomacyColor("neutral") :
| 416| 416| 					getDiplomacyColor("enemy");
| 417| 417| 
| 418| 418| 		g_DisplayedPlayerColors[0] = g_Players[0].color;
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 8 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
| 413| 413| 					g_ViewedPlayer == i ? getDiplomacyColor("self") :
| 414| 414| 					g_Players[g_ViewedPlayer].isAlly[i] ? getDiplomacyColor("ally") :
| 415| 415| 					g_Players[g_ViewedPlayer].isNeutral[i] ? getDiplomacyColor("neutral") :
| 416|    |-					getDiplomacyColor("enemy");
|    | 416|+								getDiplomacyColor("enemy");
| 417| 417| 
| 418| 418| 		g_DisplayedPlayerColors[0] = g_Players[0].color;
| 419| 419| 	}
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 4 tabs but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
| 658| 658| 					"civ": setStringTags(g_CivData[g_Players[g_ViewedPlayer].civ].Name, { "font": "sans-bold-stroke-14" }),
| 659| 659| 					"hotkey_civinfo": colorizeHotkey("%(hotkey)s", "civinfo"),
| 660| 660| 					"hotkey_structree": colorizeHotkey("%(hotkey)s", "structree")
| 661|    |-			});
|    | 661|+				});
| 662| 662| 	}
| 663| 663| 
| 664| 664| 	Engine.GetGUIObjectByName("optionFollowPlayer").hidden = !g_IsObserver || !isPlayer;
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|1248|1248| 
|1249|1249| 	let orderHotkeyTooltip = Object.keys(viewablePlayerStates).length <= 1 ? "" :
|1250|1250| 		"\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), {
|1251|    |-		"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|    |1251|+			"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|1252|1252| 		"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|1253|1253| 	});
|1254|1254| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|1249|1249| 	let orderHotkeyTooltip = Object.keys(viewablePlayerStates).length <= 1 ? "" :
|1250|1250| 		"\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), {
|1251|1251| 		"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|1252|    |-		"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|    |1252|+			"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|1253|1253| 	});
|1254|1254| 
|1255|1255| 	let resCodes = g_ResourceData.GetCodes();
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 1.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|1250|1250| 		"\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), {
|1251|1251| 		"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|1252|1252| 		"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|1253|    |-	});
|    |1253|+		});
|1254|1254| 
|1255|1255| 	let resCodes = g_ResourceData.GetCodes();
|1256|1256| 	for (let r = 0; r < resCodes.length; ++r)
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|1748|1748| 	for (let rct of resourcesCounterTypes)
|1749|1749| 		for (let rt of resourcesTypes)
|1750|1750| 			reportObject[rt + rct.substr(9)] = playerStatistics[rct][rt];
|1751|    |-			// eg. rt = food rct.substr = Gathered rct = resourcesGathered
|    |1751|+	// eg. rt = food rct.substr = Gathered rct = resourcesGathered
|1752|1752| 
|1753|1753| 	reportObject.vegetarianFoodGathered = playerStatistics.resourcesGathered.vegetarianFood;
|1754|1754| 	for (let type of unitsClasses)

binaries/data/mods/public/gui/session/session.js
|1076| »   let·getPanelEntNameTooltip·=·panelEntState·=>·"[font=\"sans-bold-16\"]"·+·template.name.specific·+·"[/font]";
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'panelEntState' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
|1151| »   »   button.onpress·=·(function(i)·{·return·function()·{·performGroup((Engine.HotkeyIsPressed("selection.add")·?·"add"·:·"select"),·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
|1152| »   »   button.ondoublepress·=·(function(i)·{·return·function()·{·performGroup("snap",·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
|1153| »   »   button.onpressright·=·(function(i)·{·return·function()·{·performGroup("breakUp",·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

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

Harbormaster completed remote builds in B5230: Diff 5951.Feb 26 2018, 5:29 AM
elexis added a comment.Feb 26 2018, 12:48 PM

02:55 < temple> so a lot easier than I thought

Good, that's how it's supposed to be!

Screenshot looks perfect!

But the first thought I had when this feature was proposed is the same I still have:
Having to let the user type the color is not the best, not even a conventional GUI.
It's certainly sufficient, but typically one can click on colors.
If we do a websearch for color picker we see many similar implementations.
Most often an area is visible, but google also has a slider.
So I'm wondering if we shouldn't replace the text input with a slider that only changes the hueness but enforces a default brightness and lightness?
We could prove this idea to be false if there is a good color we can't pick using that idea.

Also should check the 1024x768 aspect, possibly we could set a new category (which also reminds me that we wanted to change the order of categories).
Adding 3 sliders sounds like GUI space trouble and being a bit clumsy.
Adding one slider for lightness and one for saturation that applies to all diplo colors might be a workaround, but kind of unsatisfying too.
Maybe we can squeeze 3 sliders above each other on the same line as the label + preview box?

binaries/data/mods/public/gui/common/color.js
30 ↗(On Diff #5951)

Since it's a library function it should support alpha. I.e. check for length 3 || 4 and set alpha to color[3] || 255 would fix that.

vladislavbelov requested changes to this revision.Feb 26 2018, 2:32 PM
vladislavbelov added a subscriber: vladislavbelov.

We should use Hex values, because they're used in the Atlas. If you don't want to, then you should change the Atlas format too. We don't have to confuse the player.

It'd be good to support text colors, until we don't have a color picker.

This revision now requires changes to proceed.Feb 26 2018, 2:32 PM
elexis added a comment.Feb 26 2018, 2:36 PM

I dont share the point of view. Atlas displays numbers in both HSL and RGB and Hex too. Players and map editors are two different groups of people and there is no real proximity between the two features. Typing hex is also much harder than dec.

vladislavbelov added a comment.Feb 26 2018, 2:44 PM
In D1291#54797, @elexis wrote:

I dont share the point of view. Atlas displays numbers in both HSL and RGB and Hex too. Players and map editors are two different groups of people and there is no real proximity between the two features. Typing hex is also much harder than dec.

Then probably we need to add a color picker here too? Because there are 2 player groups: A) don't know rgb/hex, B) do know rgb/hex. And size of A is much bigger than size of B. So why we need to confuse the A group.

vladislavbelov added a comment.Feb 26 2018, 2:45 PM
In D1291#54797, @elexis wrote:

I dont share the point of view. Atlas displays numbers in both HSL and RGB and Hex too. Players and map editors are two different groups of people and there is no real proximity between the two features. Typing hex is also much harder than dec.

Where Atlas does display HSL or RGB?

elexis added a comment.Feb 26 2018, 2:51 PM
In D1291#54798, @vladislavbelov wrote:

Then probably we need to add a color picker here too?

You probably didn't read my first comment today then ;-) A color picker would be the best UI and a simple implementation could be 1 to 3 sliders per color.

Where Atlas does display HSL or RGB?

When you click on the sun/water color the color picker shows up

vladislavbelov added a comment.Feb 26 2018, 3:05 PM
In D1291#54800, @elexis wrote:

You probably didn't read my first comment today then ;-) A color picker would be the best UI and a simple implementation could be 1 to 3 sliders per color.

I did read, and I said exactly here, not somewhen and somewhere else.

When you click on the sun/water color the color picker shows up

It's a color picker, not a color field. All color fields contain Hex. Our in-game color picker can contain RGB, Hex, HSL, HSV together too.

elexis added a comment.Feb 26 2018, 3:26 PM

So ideally the options page could have a preview thing like it now does and an edit button opening a small GUI page with 3 sliders a preview box and hexcode input?
Sounds like work though.
To me this, the current iteration and the one-slider solution would likely be acceptable.

temple updated this revision to Diff 5967.Feb 27 2018, 10:49 PM

add alpha possibility to guiToRgbColor

temple added a comment.Feb 27 2018, 10:57 PM

Can't really reduce 3d color space to 1d slider, e.g. what if a player wants to use white or black?
I don't see the need to add hex at this point, better to be complete and make a proper color picker.
(Maybe add it to the game setup too? So players can choose beyond the eight given colors. If someone wants to be pink, why not?)

temple added a comment.Feb 27 2018, 10:59 PM

https://trac.wildfiregames.com/wiki/GUI_-_Scripting_Reference
Color-picker is/was planned as a GUI object.

Vulcan added a comment.Feb 27 2018, 11:01 PM

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

Linter detected issues:
Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (object-curly-spacing):
|    | A space is required after '{'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/color.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/color.js
|   1|   1| /**
|   2|   2|  * Used to highlight hotkeys in tooltip descriptions.
|   3|   3|  */
|   4|    |-var g_HotkeyTags = {"color": "255 251 131" };
|    |   4|+var g_HotkeyTags = { "color": "255 251 131" };
|   5|   5| 
|   6|   6| /**
|   7|   7|  * Concatenate integer color values to a string (for use in GUI objects)

binaries/data/mods/public/gui/common/color.js
|  95| »   »   h·=·s·=·0;·//·achromatic
|    | [NORMAL] ESLintBear (no-multi-assign):
|    | Unexpected chained assignment.

binaries/data/mods/public/gui/common/color.js
| 100| »   »   switch·(max)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/common/color.js
| 146| »   if·(s·==·0)
|    | [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/color.js
| 147| »   »   r·=·g·=·b·=·l;·//·achromatic
|    | [NORMAL] ESLintBear (no-multi-assign):
|    | Unexpected chained assignment.

binaries/data/mods/public/gui/common/color.js
| 147| »   »   r·=·g·=·b·=·l;·//·achromatic
|    | [NORMAL] ESLintBear (no-multi-assign):
|    | Unexpected chained assignment.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 5 tabs but found 4.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
| 110| 110| 			sprintf(
| 111| 111| 				option.min !== undefined && option.max !== undefined ?
| 112| 112| 					translateWithContext("option number", "Min: %(min)s, Max: %(max)s") :
| 113|    |-				option.min !== undefined && option.max === undefined ?
|    | 113|+					option.min !== undefined && option.max === undefined ?
| 114| 114| 					translateWithContext("option number", "Min: %(min)s") :
| 115| 115| 				option.min === undefined && option.max !== undefined ?
| 116| 116| 					translateWithContext("option number", "Max: %(max)s") :
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 6 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
| 111| 111| 				option.min !== undefined && option.max !== undefined ?
| 112| 112| 					translateWithContext("option number", "Min: %(min)s, Max: %(max)s") :
| 113| 113| 				option.min !== undefined && option.max === undefined ?
| 114|    |-					translateWithContext("option number", "Min: %(min)s") :
|    | 114|+						translateWithContext("option number", "Min: %(min)s") :
| 115| 115| 				option.min === undefined && option.max !== undefined ?
| 116| 116| 					translateWithContext("option number", "Max: %(max)s") :
| 117| 117| 					"",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 6 tabs but found 4.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
| 112| 112| 					translateWithContext("option number", "Min: %(min)s, Max: %(max)s") :
| 113| 113| 				option.min !== undefined && option.max === undefined ?
| 114| 114| 					translateWithContext("option number", "Min: %(min)s") :
| 115|    |-				option.min === undefined && option.max !== undefined ?
|    | 115|+						option.min === undefined && option.max !== undefined ?
| 116| 116| 					translateWithContext("option number", "Max: %(max)s") :
| 117| 117| 					"",
| 118| 118| 				{
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
| 113| 113| 				option.min !== undefined && option.max === undefined ?
| 114| 114| 					translateWithContext("option number", "Min: %(min)s") :
| 115| 115| 				option.min === undefined && option.max !== undefined ?
| 116|    |-					translateWithContext("option number", "Max: %(max)s") :
|    | 116|+							translateWithContext("option number", "Max: %(max)s") :
| 117| 117| 					"",
| 118| 118| 				{
| 119| 119| 					"min": option.min,
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
| 114| 114| 					translateWithContext("option number", "Min: %(min)s") :
| 115| 115| 				option.min === undefined && option.max !== undefined ?
| 116| 116| 					translateWithContext("option number", "Max: %(max)s") :
| 117|    |-					"",
|    | 117|+							"",
| 118| 118| 				{
| 119| 119| 					"min": option.min,
| 120| 120| 					"max": option.max

binaries/data/mods/public/gui/options/options.js
| 226| »   »   »   let·value·=·optionType.guiToValue(control);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'value' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 486| »   »   button.onPress·=·(function(player,·stance)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'stance' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 518| »   »   button.onPress·=·(function(i,·resCode,·button)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 518| »   »   button.onPress·=·(function(i,·resCode,·button)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'resCode' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 518| »   »   button.onPress·=·(function(i,·resCode,·button)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'button' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 561| »   button.onPress·=·(function(i)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 627| »   button.onPress·=·(function(i,·button)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 627| »   button.onPress·=·(function(i,·button)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'button' is already declared in the upper scope.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 6 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
| 411| 411| 				// Players see colors depending on diplomacy
| 412| 412| 				g_DisplayedPlayerColors[i] =
| 413| 413| 					g_ViewedPlayer == i ? getDiplomacyColor("self") :
| 414|    |-					g_Players[g_ViewedPlayer].isAlly[i] ? getDiplomacyColor("ally") :
|    | 414|+						g_Players[g_ViewedPlayer].isAlly[i] ? getDiplomacyColor("ally") :
| 415| 415| 					g_Players[g_ViewedPlayer].isNeutral[i] ? getDiplomacyColor("neutral") :
| 416| 416| 					getDiplomacyColor("enemy");
| 417| 417| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
| 412| 412| 				g_DisplayedPlayerColors[i] =
| 413| 413| 					g_ViewedPlayer == i ? getDiplomacyColor("self") :
| 414| 414| 					g_Players[g_ViewedPlayer].isAlly[i] ? getDiplomacyColor("ally") :
| 415|    |-					g_Players[g_ViewedPlayer].isNeutral[i] ? getDiplomacyColor("neutral") :
|    | 415|+							g_Players[g_ViewedPlayer].isNeutral[i] ? getDiplomacyColor("neutral") :
| 416| 416| 					getDiplomacyColor("enemy");
| 417| 417| 
| 418| 418| 		g_DisplayedPlayerColors[0] = g_Players[0].color;
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 8 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
| 413| 413| 					g_ViewedPlayer == i ? getDiplomacyColor("self") :
| 414| 414| 					g_Players[g_ViewedPlayer].isAlly[i] ? getDiplomacyColor("ally") :
| 415| 415| 					g_Players[g_ViewedPlayer].isNeutral[i] ? getDiplomacyColor("neutral") :
| 416|    |-					getDiplomacyColor("enemy");
|    | 416|+								getDiplomacyColor("enemy");
| 417| 417| 
| 418| 418| 		g_DisplayedPlayerColors[0] = g_Players[0].color;
| 419| 419| 	}
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 4 tabs but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
| 658| 658| 					"civ": setStringTags(g_CivData[g_Players[g_ViewedPlayer].civ].Name, { "font": "sans-bold-stroke-14" }),
| 659| 659| 					"hotkey_civinfo": colorizeHotkey("%(hotkey)s", "civinfo"),
| 660| 660| 					"hotkey_structree": colorizeHotkey("%(hotkey)s", "structree")
| 661|    |-			});
|    | 661|+				});
| 662| 662| 	}
| 663| 663| 
| 664| 664| 	Engine.GetGUIObjectByName("optionFollowPlayer").hidden = !g_IsObserver || !isPlayer;
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|1248|1248| 
|1249|1249| 	let orderHotkeyTooltip = Object.keys(viewablePlayerStates).length <= 1 ? "" :
|1250|1250| 		"\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), {
|1251|    |-		"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|    |1251|+			"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|1252|1252| 		"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|1253|1253| 	});
|1254|1254| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|1249|1249| 	let orderHotkeyTooltip = Object.keys(viewablePlayerStates).length <= 1 ? "" :
|1250|1250| 		"\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), {
|1251|1251| 		"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|1252|    |-		"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|    |1252|+			"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|1253|1253| 	});
|1254|1254| 
|1255|1255| 	let resCodes = g_ResourceData.GetCodes();
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 1.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|1250|1250| 		"\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), {
|1251|1251| 		"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|1252|1252| 		"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|1253|    |-	});
|    |1253|+		});
|1254|1254| 
|1255|1255| 	let resCodes = g_ResourceData.GetCodes();
|1256|1256| 	for (let r = 0; r < resCodes.length; ++r)
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|1748|1748| 	for (let rct of resourcesCounterTypes)
|1749|1749| 		for (let rt of resourcesTypes)
|1750|1750| 			reportObject[rt + rct.substr(9)] = playerStatistics[rct][rt];
|1751|    |-			// eg. rt = food rct.substr = Gathered rct = resourcesGathered
|    |1751|+	// eg. rt = food rct.substr = Gathered rct = resourcesGathered
|1752|1752| 
|1753|1753| 	reportObject.vegetarianFoodGathered = playerStatistics.resourcesGathered.vegetarianFood;
|1754|1754| 	for (let type of unitsClasses)

binaries/data/mods/public/gui/session/session.js
|1076| »   let·getPanelEntNameTooltip·=·panelEntState·=>·"[font=\"sans-bold-16\"]"·+·template.name.specific·+·"[/font]";
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'panelEntState' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
|1151| »   »   button.onpress·=·(function(i)·{·return·function()·{·performGroup((Engine.HotkeyIsPressed("selection.add")·?·"add"·:·"select"),·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
|1152| »   »   button.ondoublepress·=·(function(i)·{·return·function()·{·performGroup("snap",·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
|1153| »   »   button.onpressright·=·(function(i)·{·return·function()·{·performGroup("breakUp",·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

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

Harbormaster completed remote builds in B5255: Diff 5967.Feb 27 2018, 11:01 PM
elexis added a comment.Mar 10 2018, 1:10 PM
In D1291#55000, @temple wrote:

Can't really reduce 3d color space to 1d slider, e.g. what if a player wants to use white or black?

Correct

I don't see the need to add hex at this point, better to be complete and make a proper color picker.

Agree

Maybe add it to the game setup too? So players can choose beyond the eight given colors. If someone wants to be pink, why not?

It should ensure a minimum distance between colors, we already have colorDistance.
This is one of the catches to allowing more than 8 players (ticket out there).

binaries/data/mods/public/gui/common/color.js
37 ↗(On Diff #5967)

This silently replaces 0 with 255. Better test for color.length ==4 or color[3] !== undefined?
(Wasn't alpha 0 called "dirty transparency"? reminds me of my first patch #2823. )

39 ↗(On Diff #5967)

This function is committable if it works as advertized.

binaries/data/mods/public/gui/options/options.js
89 ↗(On Diff #5967)

(Only having to add one hunk to implement a new feature is only possible by object oriented programming. This implements something like an interface. We should use this much more in session/ and in general. Once everything would use almost exclusively prototypes, we could think about rearranging the objects themselves over there. gamesetup.js uses these objects too, but the individual options might become individual files and a map might want to define such a file (ticket exists). Anyway,..)

binaries/data/mods/public/gui/session/menu.js
261 ↗(On Diff #5967)

(It would be great to execute this conditionally. Not necessarily for the performance benefit, but to keep things less complex in the long term, as this function would become a load of hardcodings.
The options page could return the config names changed.

An onConfigDB_GetValueChanged(configName) sounds attractive, but would still require the modder to replace this function when wanting to extend it.
Even if options.json were a JS file and would allow functions, it shouldn't contain session logic either -.-

Perhaps the best split would be to let options.json define a function name that is called if the option changed?
That has the disadvantage of allowing non-options code execution, on the other hand mods can call any code they want anyway.
One could call "onOptionChanged_" + optionName, like setupBiome:

	let setupBiomeFunc = global["setupBiome_" + basename(biomeID)];
	if (setupBiomeFunc)
		setupBiomeFunc();

Due to the split, the textsearch becomes a bit more challenging, but seems worth it.
(IIRC options.json had a similar mechanism once.)

binaries/data/mods/public/gui/session/session.js
897 ↗(On Diff #5967)

This hunk move could be committed separately.

elexis added a reviewer: elexis.Mar 10 2018, 1:10 PM
temple added inline comments.Mar 11 2018, 2:04 AM
binaries/data/mods/public/gui/common/color.js
20 ↗(On Diff #5967)

same issue here

37 ↗(On Diff #5967)

oops

temple updated this revision to Diff 6129.Mar 11 2018, 2:05 AM

fix color functions
add callbacks

temple added inline comments.Mar 11 2018, 6:28 AM
binaries/data/mods/public/gui/session/menu.js
261 ↗(On Diff #5967)

I decided to use the names of the functions since both are called elsewhere. Otherwise we'd have two onOptionChanged_ functions that just call those other functions, which seems silly. But maybe that's the better way to go.

Vulcan added a comment.Mar 11 2018, 10:47 AM

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

Linter detected issues:
Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (object-curly-spacing):
|    | A space is required after '{'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/color.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/common/color.js
|   1|   1| /**
|   2|   2|  * Used to highlight hotkeys in tooltip descriptions.
|   3|   3|  */
|   4|    |-var g_HotkeyTags = {"color": "255 251 131" };
|    |   4|+var g_HotkeyTags = { "color": "255 251 131" };
|   5|   5| 
|   6|   6| /**
|   7|   7|  * Concatenate integer color values to a string (for use in GUI objects)

binaries/data/mods/public/gui/common/color.js
|  95| »   »   h·=·s·=·0;·//·achromatic
|    | [NORMAL] ESLintBear (no-multi-assign):
|    | Unexpected chained assignment.

binaries/data/mods/public/gui/common/color.js
| 100| »   »   switch·(max)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/common/color.js
| 146| »   if·(s·==·0)
|    | [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/color.js
| 147| »   »   r·=·g·=·b·=·l;·//·achromatic
|    | [NORMAL] ESLintBear (no-multi-assign):
|    | Unexpected chained assignment.

binaries/data/mods/public/gui/common/color.js
| 147| »   »   r·=·g·=·b·=·l;·//·achromatic
|    | [NORMAL] ESLintBear (no-multi-assign):
|    | Unexpected chained assignment.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 5 tabs but found 4.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
| 115| 115| 			sprintf(
| 116| 116| 				option.min !== undefined && option.max !== undefined ?
| 117| 117| 					translateWithContext("option number", "Min: %(min)s, Max: %(max)s") :
| 118|    |-				option.min !== undefined && option.max === undefined ?
|    | 118|+					option.min !== undefined && option.max === undefined ?
| 119| 119| 					translateWithContext("option number", "Min: %(min)s") :
| 120| 120| 				option.min === undefined && option.max !== undefined ?
| 121| 121| 					translateWithContext("option number", "Max: %(max)s") :
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 6 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
| 116| 116| 				option.min !== undefined && option.max !== undefined ?
| 117| 117| 					translateWithContext("option number", "Min: %(min)s, Max: %(max)s") :
| 118| 118| 				option.min !== undefined && option.max === undefined ?
| 119|    |-					translateWithContext("option number", "Min: %(min)s") :
|    | 119|+						translateWithContext("option number", "Min: %(min)s") :
| 120| 120| 				option.min === undefined && option.max !== undefined ?
| 121| 121| 					translateWithContext("option number", "Max: %(max)s") :
| 122| 122| 					"",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 6 tabs but found 4.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
| 117| 117| 					translateWithContext("option number", "Min: %(min)s, Max: %(max)s") :
| 118| 118| 				option.min !== undefined && option.max === undefined ?
| 119| 119| 					translateWithContext("option number", "Min: %(min)s") :
| 120|    |-				option.min === undefined && option.max !== undefined ?
|    | 120|+						option.min === undefined && option.max !== undefined ?
| 121| 121| 					translateWithContext("option number", "Max: %(max)s") :
| 122| 122| 					"",
| 123| 123| 				{
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
| 118| 118| 				option.min !== undefined && option.max === undefined ?
| 119| 119| 					translateWithContext("option number", "Min: %(min)s") :
| 120| 120| 				option.min === undefined && option.max !== undefined ?
| 121|    |-					translateWithContext("option number", "Max: %(max)s") :
|    | 121|+							translateWithContext("option number", "Max: %(max)s") :
| 122| 122| 					"",
| 123| 123| 				{
| 124| 124| 					"min": option.min,
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/options/options.js
| 119| 119| 					translateWithContext("option number", "Min: %(min)s") :
| 120| 120| 				option.min === undefined && option.max !== undefined ?
| 121| 121| 					translateWithContext("option number", "Max: %(max)s") :
| 122|    |-					"",
|    | 122|+							"",
| 123| 123| 				{
| 124| 124| 					"min": option.min,
| 125| 125| 					"max": option.max

binaries/data/mods/public/gui/options/options.js
| 232| »   »   »   let·value·=·optionType.guiToValue(control);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'value' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 486| »   »   button.onPress·=·(function(player,·stance)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'stance' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 518| »   »   button.onPress·=·(function(i,·resCode,·button)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 518| »   »   button.onPress·=·(function(i,·resCode,·button)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'resCode' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 518| »   »   button.onPress·=·(function(i,·resCode,·button)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'button' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 561| »   button.onPress·=·(function(i)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 627| »   button.onPress·=·(function(i,·button)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 627| »   button.onPress·=·(function(i,·button)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'button' is already declared in the upper scope.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 6 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
| 412| 412| 				// Players see colors depending on diplomacy
| 413| 413| 				g_DisplayedPlayerColors[i] =
| 414| 414| 					g_ViewedPlayer == i ? getDiplomacyColor("self") :
| 415|    |-					g_Players[g_ViewedPlayer].isAlly[i] ? getDiplomacyColor("ally") :
|    | 415|+						g_Players[g_ViewedPlayer].isAlly[i] ? getDiplomacyColor("ally") :
| 416| 416| 					g_Players[g_ViewedPlayer].isNeutral[i] ? getDiplomacyColor("neutral") :
| 417| 417| 					getDiplomacyColor("enemy");
| 418| 418| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
| 413| 413| 				g_DisplayedPlayerColors[i] =
| 414| 414| 					g_ViewedPlayer == i ? getDiplomacyColor("self") :
| 415| 415| 					g_Players[g_ViewedPlayer].isAlly[i] ? getDiplomacyColor("ally") :
| 416|    |-					g_Players[g_ViewedPlayer].isNeutral[i] ? getDiplomacyColor("neutral") :
|    | 416|+							g_Players[g_ViewedPlayer].isNeutral[i] ? getDiplomacyColor("neutral") :
| 417| 417| 					getDiplomacyColor("enemy");
| 418| 418| 
| 419| 419| 		g_DisplayedPlayerColors[0] = g_Players[0].color;
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 8 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
| 414| 414| 					g_ViewedPlayer == i ? getDiplomacyColor("self") :
| 415| 415| 					g_Players[g_ViewedPlayer].isAlly[i] ? getDiplomacyColor("ally") :
| 416| 416| 					g_Players[g_ViewedPlayer].isNeutral[i] ? getDiplomacyColor("neutral") :
| 417|    |-					getDiplomacyColor("enemy");
|    | 417|+								getDiplomacyColor("enemy");
| 418| 418| 
| 419| 419| 		g_DisplayedPlayerColors[0] = g_Players[0].color;
| 420| 420| 	}
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 4 tabs but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
| 659| 659| 					"civ": setStringTags(g_CivData[g_Players[g_ViewedPlayer].civ].Name, { "font": "sans-bold-stroke-14" }),
| 660| 660| 					"hotkey_civinfo": colorizeHotkey("%(hotkey)s", "civinfo"),
| 661| 661| 					"hotkey_structree": colorizeHotkey("%(hotkey)s", "structree")
| 662|    |-			});
|    | 662|+				});
| 663| 663| 	}
| 664| 664| 
| 665| 665| 	Engine.GetGUIObjectByName("optionFollowPlayer").hidden = !g_IsObserver ||
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|1250|1250| 
|1251|1251| 	let orderHotkeyTooltip = Object.keys(viewablePlayerStates).length <= 1 ? "" :
|1252|1252| 		"\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), {
|1253|    |-		"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|    |1253|+			"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|1254|1254| 		"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|1255|1255| 	});
|1256|1256| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|1251|1251| 	let orderHotkeyTooltip = Object.keys(viewablePlayerStates).length <= 1 ? "" :
|1252|1252| 		"\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), {
|1253|1253| 		"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|1254|    |-		"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|    |1254|+			"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|1255|1255| 	});
|1256|1256| 
|1257|1257| 	let resCodes = g_ResourceData.GetCodes();
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 1.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|1252|1252| 		"\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), {
|1253|1253| 		"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|1254|1254| 		"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|1255|    |-	});
|    |1255|+		});
|1256|1256| 
|1257|1257| 	let resCodes = g_ResourceData.GetCodes();
|1258|1258| 	for (let r = 0; r < resCodes.length; ++r)
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|1750|1750| 	for (let rct of resourcesCounterTypes)
|1751|1751| 		for (let rt of resourcesTypes)
|1752|1752| 			reportObject[rt + rct.substr(9)] = playerStatistics[rct][rt];
|1753|    |-			// eg. rt = food rct.substr = Gathered rct = resourcesGathered
|    |1753|+	// eg. rt = food rct.substr = Gathered rct = resourcesGathered
|1754|1754| 
|1755|1755| 	reportObject.vegetarianFoodGathered = playerStatistics.resourcesGathered.vegetarianFood;
|1756|1756| 	for (let type of unitsClasses)

binaries/data/mods/public/gui/session/session.js
|1078| »   let·getPanelEntNameTooltip·=·panelEntState·=>·"[font=\"sans-bold-16\"]"·+·template.name.specific·+·"[/font]";
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'panelEntState' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
|1153| »   »   button.onpress·=·(function(i)·{·return·function()·{·performGroup((Engine.HotkeyIsPressed("selection.add")·?·"add"·:·"select"),·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
|1154| »   »   button.ondoublepress·=·(function(i)·{·return·function()·{·performGroup("snap",·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
|1155| »   »   button.onpressright·=·(function(i)·{·return·function()·{·performGroup("breakUp",·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

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

Harbormaster completed remote builds in B5475: Diff 6129.Mar 11 2018, 10:47 AM
elexis accepted this revision.Mar 12 2018, 5:19 AM

Thanks for the patch. The preview has come out really great!
I will commit it just before feature freeze as you are offline now, so we can enjoy it in alpha 23 in the same bundle with the diplomacy color feature.

binaries/data/mods/public/gui/session/menu.js
262 ↗(On Diff #6129)

Should we restrict this by enforcing the function name to .startsWith("onOptionChanged_")?
Given what you have in options.json: no.
But it had the advantage that people can encounter these update functions only in JS files and only in session/.
I'm expecting that someone will rename a function and break the options with the current approach.

(There is one drawback to these proxy functions, which is mods adding new options that only want to call existing functions being required to add a new session file. But it seems improbable and if it occurs, doesnt hurt much).

We could also have g_OptionCallbacks = { "diplomacyColor": updateDiplomacyColor, .... }. But then again stacking options logic in session that should be in options/!?.. ¯\_(ツ)_/¯

-> meh

binaries/data/mods/public/gui/session/session.js
1424 ↗(On Diff #6129)

(The current code doesnt seem to work on attack range btw)

elexis added inline comments.Mar 12 2018, 5:32 AM
binaries/data/mods/public/gui/options/options.json
500 ↗(On Diff #6129)

Default: \"21 55 149\"", is bad actually.

It can be loaded from default.cfg automatically and appended in the JS code.

This revision was not accepted when it landed; it landed in state Needs Review.Mar 12 2018, 5:48 AM
Closed by commit rP21527: Implement basic color editing in the options page and use it for the diplomacy… (authored by elexis). · Explain Why
This revision was automatically updated to reflect the committed changes.
Owners added a subscriber: Restricted Owners Package.Mar 12 2018, 5:48 AM
temple added a comment.Mar 12 2018, 5:04 PM

Thanks for the review and commit, elexis.

binaries/data/mods/public/gui/session/session.js
1424 ↗(On Diff #6129)

What do you mean? I had noticed that the overlays wouldn't update for selected units until you deselected then reselected them, but didn't get around to investigating that. (Seemed maybe not worth the hassle.)

elexis added inline comments.Mar 12 2018, 7:31 PM
binaries/data/mods/public/gui/session/session.js
308 ↗(On Diff #6129)

Thought it's not needed. It is.

1424 ↗(On Diff #6129)

Yes

elexis mentioned this in D2389: Make options page agnostic of session page, RangeOverlayManager GUI class.Oct 25 2019, 1:23 PM

Revision Contents
Changeset List

PathSize
ps/
trunk/
binaries/
data/
config/
6 lines
mods/
public/
gui/
common/
17 lines
options/
34 lines
37 lines
4 lines
session/
11 lines
35 lines
simulation/
data/
settings/
6 lines

Diff 6156

View Options

ps/trunk/binaries/data/config/default.cfg

Loading...
View Options

ps/trunk/binaries/data/mods/public/gui/common/color.js

Loading...
View Options

ps/trunk/binaries/data/mods/public/gui/options/options.js

Loading...
View Options

ps/trunk/binaries/data/mods/public/gui/options/options.json

Loading...
View Options

ps/trunk/binaries/data/mods/public/gui/options/options.xml

Loading...
View Options

ps/trunk/binaries/data/mods/public/gui/session/menu.js

Loading...
View Options

ps/trunk/binaries/data/mods/public/gui/session/session.js

Loading...
View Options

ps/trunk/binaries/data/mods/public/simulation/data/settings/diplomacy_colors.json

Loading...
Wildfire Games · Phabricator