Page MenuHomeWildfire Games

Gamesetup - Hide mapfilter option if there are no maps matching the filter
ClosedPublic

Authored by Imarok on Apr 20 2017, 5:03 PM.

Details

Summary

Hide mapfilter option if there are no maps matching the filter

Test Plan

select e.g. Scenario, New maps

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
https://svn.wildfiregames.com/public/ps/trunk
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 2203
Build 3619: Vulcan Build (Windows)Jenkins
Build 3618: Vulcan BuildJenkins
Build 3617: arc lint + arc unit

Event Timeline

rob-v created this revision.Apr 20 2017, 5:03 PM
rob-v updated this revision to Diff 1377.Apr 20 2017, 5:53 PM
rob-v retitled this revision from 4423 - Gamesetup - Hide mapfilter choices if there are no maps matching the filter to Gamesetup - Hide mapfilter choices if there are no maps matching the filter #4423.Apr 20 2017, 9:43 PM
Imarok added a subscriber: Imarok.Apr 21 2017, 12:51 AM
Imarok added inline comments.
binaries/data/mods/public/gui/credits/texts/programming.json
167 ↗(On Diff #1377)

The nicks should be sorted alphabetically

rob-v updated this revision to Diff 1391.Apr 21 2017, 9:52 AM

Fixed EOLN

rob-v added inline comments.Apr 21 2017, 9:54 AM
binaries/data/mods/public/gui/credits/texts/programming.json
167 ↗(On Diff #1377)

Ok. I removed it now. This diff is minor change. I can add it lately.

rob-v updated this revision to Diff 1405.Apr 21 2017, 5:15 PM
rob-v set the repository for this revision to rP 0 A.D. Public Repository.
elexis requested changes to this revision.Apr 21 2017, 5:55 PM
elexis added a subscriber: elexis.

Thanks for working on the release blocker ticket!

There are some concerns with this patch though:

  • The approach of the patch is not what I had in mind, see the title of that ticket. The mapfilter choice itself should become hidden if there are no maps matching that filter initMapNameList can be extended to test for that after selecting the maptype. Perhaps we can get D322 in, perhaps not. Might want to wait a bit. The mapfilter bug has to be fixed before the release and can be done after feature freeze, the option unification probably can't be added after feature freze. So might want to wait for feature freeze before starting to rewrite this patch.
  • The programming.json entry should always be part of the first patch.
This revision now requires changes to proceed.Apr 21 2017, 5:55 PM
Vulcan added a subscriber: Vulcan.Apr 21 2017, 7:41 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/843/ for more details.

Ping, D322 is complete.

When selecting a maptype, it should call initDropdown("mapFilter").

g_MapFilterList could be renamed to g_MapFilters and then we can have a new variable g_MapFilterList which is generated in a new function reloadMapFilters above reloadMapList which just sets g_MapFilterList = g_MapFilters.filter(mapFilter => { reloadMapList(mapFilter); return g_MapSelectionList.length ) (or close to that)

This is not a performance issue since we load each map XML / JSON when selecting a maptype already and since these things are cached both in the VFS (engine) and in g_MapData of gamesetup.js.

The mapFilter entry of g_Dropdowns should have

@rob-v are you still intending to work on this? Feature freeze is this sunday, so we will have to approach this in the next 1-3 weeks +/-

Imarok commandeered this revision.Jun 8 2017, 12:06 PM
Imarok added a reviewer: rob-v.

This is a release blocker, release is near and no activity since 1.5 months.

Build has FAILED

Link to build: http://jw:8080/job/phabricator/1504/
See console output for more information: http://jw:8080/job/phabricator/1504/console

Imarok updated this revision to Diff 2472.Jun 8 2017, 12:06 PM
Imarok edited edge metadata.

Only show valid map filter options

Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'g_GameStanzaTimer' to undefined.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 313| 313| /**
| 314| 314|  * Index of the GUI timer.
| 315| 315|  */
| 316|    |-var g_GameStanzaTimer = undefined;
|    | 316|+var g_GameStanzaTimer;
| 317| 317| 
| 318| 318| /**
| 319| 319|  * Only send a lobby update if something actually changed.
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before 'name'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 899| 899| 				name =
| 900| 900| 					'[color="' +
| 901| 901| 					g_ReadyData[assignedGUID ? g_PlayerAssignments[assignedGUID].status : 2].color +
| 902|    |-					'"]' +  name + '[/color]';
|    | 902|+					'"]' + name + '[/color]';
| 903| 903| 
| 904| 904| 			return name;
| 905| 905| 		},
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before 'attribs'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 937| 937| 
| 938| 938| 	g_IsNetworked = attribs.type != "offline";
| 939| 939| 	g_IsController = attribs.type != "client";
| 940|    |-	g_IsTutorial = attribs.tutorial &&  attribs.tutorial == true;
|    | 940|+	g_IsTutorial = attribs.tutorial && attribs.tutorial == true;
| 941| 941| 	g_ServerName = attribs.serverName;
| 942| 942| 	g_ServerPort = attribs.serverPort;
| 943| 943| 	g_StunEndpoint = attribs.stunEndpoint;
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'yPos' to undefined.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1138|1138| 
|1139|1139| function verticallyDistributeGUIObjects(parent, objectHeight, ignore)
|1140|1140| {
|1141|    |-	let yPos = undefined;
|    |1141|+	let yPos;
|1142|1142| 
|1143|1143| 	let parentObject = Engine.GetGUIObjectByName(parent);
|1144|1144| 	for (let child of parentObject.children)

binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 689| »   »   »   let·pData·=·playerData.find(pData·=>·sameColor(g_PlayerColorPickerList[selectedIdx],·pData.Color));
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'pData' is already declared in the upper scope.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1573| »   while·(g_IsNetworked)
|    | [NORMAL] ESLintBear (no-unmodified-loop-condition):
|    | 'g_IsNetworked' is not modified in this loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|2023| »   »   for·(let·guid·in·g_PlayerAssignments)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'guid' is already declared in the upper scope.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|2023| »   »   for·(let·guid·in·g_PlayerAssignments)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'guid' is already declared in the upper scope.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 316| var·g_GameStanzaTimer·=·undefined;
|    | [NORMAL] JSHintBear:
|    | It's not necessary to initialize 'g_GameStanzaTimer' to 'undefined'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 940| »   g_IsTutorial·=·attribs.tutorial·&&··attribs.tutorial·==·true;
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with 'true'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1141| »   let·yPos·=·undefined;
|    | [NORMAL] JSHintBear:
|    | It's not necessary to initialize 'yPos' to 'undefined'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1344| »   if·(map·==·"random"·||·map·==·"")
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with ''.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1386| »   »   let·filterID·=·g_MapFilterList.id.findIndex(id·=>·id·==·g_GameAttributes.mapFilter);
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1555| »   if·(g_LoadingState·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1607| »   »   if·(playerData.some((pData,·j)·=>·i·!=·j·&&·sameColor(playerData[i].Color,·pData.Color)))
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1608| »   »   »   playerData[i].Color·=·g_PlayerColorPickerList.find(color·=>·playerData.every(pData·=>·!sameColor(color,·pData.Color)));
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1785| »   »   »   chosenCiv·=·pickRandom(Object.keys(g_CivData).filter(civ·=>·g_CivData[civ].Culture·==·culture));
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1799| »   »   let·usedName·=·g_GameAttributes.settings.PlayerData.filter(pData·=>·pData.Name·&&·pData.Name.indexOf(chosenName)·!==·-1).length;
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1933| »   Engine.GetGUIObjectByName("mapInfoDescription").caption·=·g_GameAttributes.map·==·""·?·""·:·getGameDescription();
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with ''.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|2235| »   if·(g_GameStanzaTimer·!=·undefined)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with 'undefined'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|2275| »   if·(g_GameStanzaTimer·!=·undefined)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with 'undefined'.
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/151/ for more details.

Imarok retitled this revision from Gamesetup - Hide mapfilter choices if there are no maps matching the filter #4423 to Gamesetup - Hide mapfilter option if there are no maps matching the filter.Jun 8 2017, 12:07 PM
Imarok edited the summary of this revision. (Show Details)

Build is green

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

http://jw:8080/job/phabricator/1505/ for more details.

Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'g_GameStanzaTimer' to undefined.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 315| 315| /**
| 316| 316|  * Index of the GUI timer.
| 317| 317|  */
| 318|    |-var g_GameStanzaTimer = undefined;
|    | 318|+var g_GameStanzaTimer;
| 319| 319| 
| 320| 320| /**
| 321| 321|  * Only send a lobby update if something actually changed.
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before 'name'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 901| 901| 				name =
| 902| 902| 					'[color="' +
| 903| 903| 					g_ReadyData[assignedGUID ? g_PlayerAssignments[assignedGUID].status : 2].color +
| 904|    |-					'"]' +  name + '[/color]';
|    | 904|+					'"]' + name + '[/color]';
| 905| 905| 
| 906| 906| 			return name;
| 907| 907| 		},
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before 'attribs'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 939| 939| 
| 940| 940| 	g_IsNetworked = attribs.type != "offline";
| 941| 941| 	g_IsController = attribs.type != "client";
| 942|    |-	g_IsTutorial = attribs.tutorial &&  attribs.tutorial == true;
|    | 942|+	g_IsTutorial = attribs.tutorial && attribs.tutorial == true;
| 943| 943| 	g_ServerName = attribs.serverName;
| 944| 944| 	g_ServerPort = attribs.serverPort;
| 945| 945| 	g_StunEndpoint = attribs.stunEndpoint;
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'yPos' to undefined.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1140|1140| 
|1141|1141| function verticallyDistributeGUIObjects(parent, objectHeight, ignore)
|1142|1142| {
|1143|    |-	let yPos = undefined;
|    |1143|+	let yPos;
|1144|1144| 
|1145|1145| 	let parentObject = Engine.GetGUIObjectByName(parent);
|1146|1146| 	for (let child of parentObject.children)

binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 691| »   »   »   let·pData·=·playerData.find(pData·=>·sameColor(g_PlayerColorPickerList[selectedIdx],·pData.Color));
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'pData' is already declared in the upper scope.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1596| »   while·(g_IsNetworked)
|    | [NORMAL] ESLintBear (no-unmodified-loop-condition):
|    | 'g_IsNetworked' is not modified in this loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|2046| »   »   for·(let·guid·in·g_PlayerAssignments)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'guid' is already declared in the upper scope.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|2046| »   »   for·(let·guid·in·g_PlayerAssignments)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'guid' is already declared in the upper scope.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 318| var·g_GameStanzaTimer·=·undefined;
|    | [NORMAL] JSHintBear:
|    | It's not necessary to initialize 'g_GameStanzaTimer' to 'undefined'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 942| »   g_IsTutorial·=·attribs.tutorial·&&··attribs.tutorial·==·true;
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with 'true'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1143| »   let·yPos·=·undefined;
|    | [NORMAL] JSHintBear:
|    | It's not necessary to initialize 'yPos' to 'undefined'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1578| »   if·(g_LoadingState·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1630| »   »   if·(playerData.some((pData,·j)·=>·i·!=·j·&&·sameColor(playerData[i].Color,·pData.Color)))
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1631| »   »   »   playerData[i].Color·=·g_PlayerColorPickerList.find(color·=>·playerData.every(pData·=>·!sameColor(color,·pData.Color)));
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1808| »   »   »   chosenCiv·=·pickRandom(Object.keys(g_CivData).filter(civ·=>·g_CivData[civ].Culture·==·culture));
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1822| »   »   let·usedName·=·g_GameAttributes.settings.PlayerData.filter(pData·=>·pData.Name·&&·pData.Name.indexOf(chosenName)·!==·-1).length;
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|2258| »   if·(g_GameStanzaTimer·!=·undefined)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with 'undefined'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|2298| »   if·(g_GameStanzaTimer·!=·undefined)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with 'undefined'.
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/152/ for more details.

elexis added a comment.Jun 8 2017, 2:44 PM

Works marvelous, just requesting some JSdoc and variable renames.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
200

wraitii proposed to name all variables that hold dropdown values as g_FooList, so g_MapFilters and g_MapFilterList?

A JSdoc comment could explain that this only contains mapfilters for which we have at least one map matching this.

1366

JSdoc for filter

1368

This function is correct, in particular becaues it excludes the "Random" map choice.

1407

entry -> mapFilter?

This is doing too much work, since need to know only about one map. But fixing this is not worth the code IMO as the maps are already cached by the OS (#4072) and being re-read just few calls later.

1418

There's always one defined, comment not really needed

1423

could merge that with the line above, or remove the mapList = as sort sorts in place too

1882

Someone didn't update svn before uploading the patch.

The reloadMapList(); call that was inserted (restored) here doesn't need to be replaced with the reloadMapFilterList(), because the clients can't change the mapfilter setting and only see what the host set.

As mentioned somewhere, the rebuilding of dropdown values should be made generic too later.

Imarok planned changes to this revision.Jun 8 2017, 4:41 PM
Imarok marked 6 inline comments as done.

There is a slight bug in my patch: select random and new maps. Then set map type to scenario. The Map filter dropdown will be empty.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
1882

The reloadMapList(); call that was inserted (restored) here doesn't need to be replaced with the reloadMapFilterList(), because the clients can't change the mapfilter setting and only see what the host set.

I think it would be safer and saner to replace it, because in future clients may change the settings(D431).

Imarok updated this revision to Diff 2478.Jun 8 2017, 5:39 PM

Apply remarks and fix bug mentioned above.

Imarok updated this revision to Diff 2479.Jun 8 2017, 5:50 PM

Some renames and wording changes. Don't explicitly init g_MapFilterList.

Vulcan added a comment.Jun 8 2017, 6:54 PM

Build is green

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

http://jw:8080/job/phabricator/1507/ for more details.

Vulcan added a comment.Jun 8 2017, 6:55 PM
Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'g_GameStanzaTimer' to undefined.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 318| 318| /**
| 319| 319|  * Index of the GUI timer.
| 320| 320|  */
| 321|    |-var g_GameStanzaTimer = undefined;
|    | 321|+var g_GameStanzaTimer;
| 322| 322| 
| 323| 323| /**
| 324| 324|  * Only send a lobby update if something actually changed.
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before 'name'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 904| 904| 				name =
| 905| 905| 					'[color="' +
| 906| 906| 					g_ReadyData[assignedGUID ? g_PlayerAssignments[assignedGUID].status : 2].color +
| 907|    |-					'"]' +  name + '[/color]';
|    | 907|+					'"]' + name + '[/color]';
| 908| 908| 
| 909| 909| 			return name;
| 910| 910| 		},
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before 'attribs'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 942| 942| 
| 943| 943| 	g_IsNetworked = attribs.type != "offline";
| 944| 944| 	g_IsController = attribs.type != "client";
| 945|    |-	g_IsTutorial = attribs.tutorial &&  attribs.tutorial == true;
|    | 945|+	g_IsTutorial = attribs.tutorial && attribs.tutorial == true;
| 946| 946| 	g_ServerName = attribs.serverName;
| 947| 947| 	g_ServerPort = attribs.serverPort;
| 948| 948| 	g_StunEndpoint = attribs.stunEndpoint;
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'yPos' to undefined.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1143|1143| 
|1144|1144| function verticallyDistributeGUIObjects(parent, objectHeight, ignore)
|1145|1145| {
|1146|    |-	let yPos = undefined;
|    |1146|+	let yPos;
|1147|1147| 
|1148|1148| 	let parentObject = Engine.GetGUIObjectByName(parent);
|1149|1149| 	for (let child of parentObject.children)

binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 694| »   »   »   let·pData·=·playerData.find(pData·=>·sameColor(g_PlayerColorPickerList[selectedIdx],·pData.Color));
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'pData' is already declared in the upper scope.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1599| »   while·(g_IsNetworked)
|    | [NORMAL] ESLintBear (no-unmodified-loop-condition):
|    | 'g_IsNetworked' is not modified in this loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|2049| »   »   for·(let·guid·in·g_PlayerAssignments)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'guid' is already declared in the upper scope.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|2049| »   »   for·(let·guid·in·g_PlayerAssignments)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'guid' is already declared in the upper scope.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 321| var·g_GameStanzaTimer·=·undefined;
|    | [NORMAL] JSHintBear:
|    | It's not necessary to initialize 'g_GameStanzaTimer' to 'undefined'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 945| »   g_IsTutorial·=·attribs.tutorial·&&··attribs.tutorial·==·true;
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with 'true'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1146| »   let·yPos·=·undefined;
|    | [NORMAL] JSHintBear:
|    | It's not necessary to initialize 'yPos' to 'undefined'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1581| »   if·(g_LoadingState·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1633| »   »   if·(playerData.some((pData,·j)·=>·i·!=·j·&&·sameColor(playerData[i].Color,·pData.Color)))
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1634| »   »   »   playerData[i].Color·=·g_PlayerColorPickerList.find(color·=>·playerData.every(pData·=>·!sameColor(color,·pData.Color)));
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1811| »   »   »   chosenCiv·=·pickRandom(Object.keys(g_CivData).filter(civ·=>·g_CivData[civ].Culture·==·culture));
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1825| »   »   let·usedName·=·g_GameAttributes.settings.PlayerData.filter(pData·=>·pData.Name·&&·pData.Name.indexOf(chosenName)·!==·-1).length;
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|2261| »   if·(g_GameStanzaTimer·!=·undefined)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with 'undefined'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|2301| »   if·(g_GameStanzaTimer·!=·undefined)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with 'undefined'.
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/153/ for more details.

Vulcan added a comment.Jun 8 2017, 8:39 PM

Build is green

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

http://jw:8080/job/phabricator/1508/ for more details.

Vulcan added a comment.Jun 8 2017, 8:40 PM
Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'g_GameStanzaTimer' to undefined.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 318| 318| /**
| 319| 319|  * Index of the GUI timer.
| 320| 320|  */
| 321|    |-var g_GameStanzaTimer = undefined;
|    | 321|+var g_GameStanzaTimer;
| 322| 322| 
| 323| 323| /**
| 324| 324|  * Only send a lobby update if something actually changed.
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before 'name'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 904| 904| 				name =
| 905| 905| 					'[color="' +
| 906| 906| 					g_ReadyData[assignedGUID ? g_PlayerAssignments[assignedGUID].status : 2].color +
| 907|    |-					'"]' +  name + '[/color]';
|    | 907|+					'"]' + name + '[/color]';
| 908| 908| 
| 909| 909| 			return name;
| 910| 910| 		},
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before 'attribs'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 942| 942| 
| 943| 943| 	g_IsNetworked = attribs.type != "offline";
| 944| 944| 	g_IsController = attribs.type != "client";
| 945|    |-	g_IsTutorial = attribs.tutorial &&  attribs.tutorial == true;
|    | 945|+	g_IsTutorial = attribs.tutorial && attribs.tutorial == true;
| 946| 946| 	g_ServerName = attribs.serverName;
| 947| 947| 	g_ServerPort = attribs.serverPort;
| 948| 948| 	g_StunEndpoint = attribs.stunEndpoint;
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'yPos' to undefined.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1143|1143| 
|1144|1144| function verticallyDistributeGUIObjects(parent, objectHeight, ignore)
|1145|1145| {
|1146|    |-	let yPos = undefined;
|    |1146|+	let yPos;
|1147|1147| 
|1148|1148| 	let parentObject = Engine.GetGUIObjectByName(parent);
|1149|1149| 	for (let child of parentObject.children)

binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 694| »   »   »   let·pData·=·playerData.find(pData·=>·sameColor(g_PlayerColorPickerList[selectedIdx],·pData.Color));
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'pData' is already declared in the upper scope.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1599| »   while·(g_IsNetworked)
|    | [NORMAL] ESLintBear (no-unmodified-loop-condition):
|    | 'g_IsNetworked' is not modified in this loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|2049| »   »   for·(let·guid·in·g_PlayerAssignments)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'guid' is already declared in the upper scope.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|2049| »   »   for·(let·guid·in·g_PlayerAssignments)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'guid' is already declared in the upper scope.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 321| var·g_GameStanzaTimer·=·undefined;
|    | [NORMAL] JSHintBear:
|    | It's not necessary to initialize 'g_GameStanzaTimer' to 'undefined'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 945| »   g_IsTutorial·=·attribs.tutorial·&&··attribs.tutorial·==·true;
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with 'true'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1146| »   let·yPos·=·undefined;
|    | [NORMAL] JSHintBear:
|    | It's not necessary to initialize 'yPos' to 'undefined'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1581| »   if·(g_LoadingState·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1633| »   »   if·(playerData.some((pData,·j)·=>·i·!=·j·&&·sameColor(playerData[i].Color,·pData.Color)))
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1634| »   »   »   playerData[i].Color·=·g_PlayerColorPickerList.find(color·=>·playerData.every(pData·=>·!sameColor(color,·pData.Color)));
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1811| »   »   »   chosenCiv·=·pickRandom(Object.keys(g_CivData).filter(civ·=>·g_CivData[civ].Culture·==·culture));
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1825| »   »   let·usedName·=·g_GameAttributes.settings.PlayerData.filter(pData·=>·pData.Name·&&·pData.Name.indexOf(chosenName)·!==·-1).length;
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|2261| »   if·(g_GameStanzaTimer·!=·undefined)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with 'undefined'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|2301| »   if·(g_GameStanzaTimer·!=·undefined)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with 'undefined'.
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/154/ for more details.

Didn't check the bug yet you found

binaries/data/mods/public/gui/gamesetup/gamesetup.js
437–440

This implies that the default map filter contains maps. Could keep track of the working map filters, use the default if that passes and use any other working mapfilter otherwise, but I'm not sure if we care about that.

1369

@return could also mention the type (or both possibilities if we go with the second argument, x|y iirc)

1407

and loadMapData also caches the map data, so the OS cache is irrelevant even.

1430

Actually we only have to add a new argument to getFilteredMaps, use a ternaries in the existing return statements of getFilteredMaps and add an early return to the loop to avoid the useless work.

It would also inform the reader that we don't need the results in some cases of calling this function.
The downside would be adding 3 lines of code that aren't needed if we don't care about microseconds.

getFilteredMaps should be named so that it works with boolean returns too, filterMaps?

1430

Actually mapFilter should be filterFunc for global consistency (since it isn't an element of g_MapFilters`), right?

1882

That sounds true.

In D365#25504, @elexis wrote:

Didn't check the bug yet you found

I fixed the bug. (If we're talking bout the same bug...)

Imarok marked 2 inline comments as done.Jun 11 2017, 6:01 PM
Imarok added inline comments.
binaries/data/mods/public/gui/gamesetup/gamesetup.js
437–440

This implies that the default map filter contains maps.

Nope, if no default is given, prepareForDropdown sets the default to 0. (https://github.com/0ad/0ad/blob/ec75f37daf825880e19c9095bf06282a98c0ce1d/binaries/data/mods/public/gui/common/settings.js#L291)

1430

I don't think that is worth it. In the gamesetup code simple code is more important than performance.

1430

No, as this is a whole filter and not just the filter func.

Imarok updated this revision to Diff 2524.Jun 11 2017, 6:02 PM
Imarok marked an inline comment as done.

Add return type in JSDoc. And rename one mapFilter occurance to filterFunc.

Build is green

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

http://jw:8080/job/phabricator/1537/ for more details.

Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'g_GameStanzaTimer' to undefined.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 318| 318| /**
| 319| 319|  * Index of the GUI timer.
| 320| 320|  */
| 321|    |-var g_GameStanzaTimer = undefined;
|    | 321|+var g_GameStanzaTimer;
| 322| 322| 
| 323| 323| /**
| 324| 324|  * Only send a lobby update if something actually changed.
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'yPos' to undefined.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1143|1143| 
|1144|1144| function verticallyDistributeGUIObjects(parent, objectHeight, ignore)
|1145|1145| {
|1146|    |-	let yPos = undefined;
|    |1146|+	let yPos;
|1147|1147| 
|1148|1148| 	let parentObject = Engine.GetGUIObjectByName(parent);
|1149|1149| 	for (let child of parentObject.children)

binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 694| »   »   »   let·pData·=·playerData.find(pData·=>·sameColor(g_PlayerColorPickerList[selectedIdx],·pData.Color));
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'pData' is already declared in the upper scope.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1599| »   while·(g_IsNetworked)
|    | [NORMAL] ESLintBear (no-unmodified-loop-condition):
|    | 'g_IsNetworked' is not modified in this loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|2049| »   »   for·(let·guid·in·g_PlayerAssignments)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'guid' is already declared in the upper scope.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|2049| »   »   for·(let·guid·in·g_PlayerAssignments)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'guid' is already declared in the upper scope.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 321| var·g_GameStanzaTimer·=·undefined;
|    | [NORMAL] JSHintBear:
|    | It's not necessary to initialize 'g_GameStanzaTimer' to 'undefined'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 945| »   g_IsTutorial·=·attribs.tutorial·&&·attribs.tutorial·==·true;
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with 'true'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1146| »   let·yPos·=·undefined;
|    | [NORMAL] JSHintBear:
|    | It's not necessary to initialize 'yPos' to 'undefined'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1581| »   if·(g_LoadingState·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1633| »   »   if·(playerData.some((pData,·j)·=>·i·!=·j·&&·sameColor(playerData[i].Color,·pData.Color)))
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1634| »   »   »   playerData[i].Color·=·g_PlayerColorPickerList.find(color·=>·playerData.every(pData·=>·!sameColor(color,·pData.Color)));
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1811| »   »   »   chosenCiv·=·pickRandom(Object.keys(g_CivData).filter(civ·=>·g_CivData[civ].Culture·==·culture));
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1825| »   »   let·usedName·=·g_GameAttributes.settings.PlayerData.filter(pData·=>·pData.Name·&&·pData.Name.indexOf(chosenName)·!==·-1).length;
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|2261| »   if·(g_GameStanzaTimer·!=·undefined)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with 'undefined'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|2301| »   if·(g_GameStanzaTimer·!=·undefined)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with 'undefined'.
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/177/ for more details.

In D365#25672, @Imarok wrote:
In D365#25504, @elexis wrote:

Didn't check the bug yet you found

I fixed the bug. (If we're talking bout the same bug...)

Just wanted to state that I didn't review that part yet.

Tested now and it's been exactly the same I expected in the inline comment and fixed.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
437–440

Indeed, if the default "default" mapfilter doesn't contain any valid maps, it's going to be reset to the first mapfilter.
Might still bug in case there are no maps for every mapfilter, but that's too far out :-)

441

Can be shortened to g_MapFilterList.id.indexOf(g_GameAttributes.mapFilter || "") != -1

1430

It loops over g_MapFilters.filter which is an array of filter functions right?

Not fond of that boolean argument?

At least remove the unneeded > 0
\n can go too considering the length of the rest in the file (also not sure whether ); should be on a separate line. On the newest maps I had removed it)

Imarok marked 2 inline comments as done.Jun 12 2017, 8:55 PM
Imarok updated this revision to Diff 2538.Jun 12 2017, 8:55 PM

Style adaptions

Build is green

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

http://jw:8080/job/phabricator/1545/ for more details.

Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'g_GameStanzaTimer' to undefined.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 318| 318| /**
| 319| 319|  * Index of the GUI timer.
| 320| 320|  */
| 321|    |-var g_GameStanzaTimer = undefined;
|    | 321|+var g_GameStanzaTimer;
| 322| 322| 
| 323| 323| /**
| 324| 324|  * Only send a lobby update if something actually changed.
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'yPos' to undefined.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1143|1143| 
|1144|1144| function verticallyDistributeGUIObjects(parent, objectHeight, ignore)
|1145|1145| {
|1146|    |-	let yPos = undefined;
|    |1146|+	let yPos;
|1147|1147| 
|1148|1148| 	let parentObject = Engine.GetGUIObjectByName(parent);
|1149|1149| 	for (let child of parentObject.children)

binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 694| »   »   »   let·pData·=·playerData.find(pData·=>·sameColor(g_PlayerColorPickerList[selectedIdx],·pData.Color));
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'pData' is already declared in the upper scope.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1599| »   while·(g_IsNetworked)
|    | [NORMAL] ESLintBear (no-unmodified-loop-condition):
|    | 'g_IsNetworked' is not modified in this loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|2049| »   »   for·(let·guid·in·g_PlayerAssignments)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'guid' is already declared in the upper scope.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|2049| »   »   for·(let·guid·in·g_PlayerAssignments)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'guid' is already declared in the upper scope.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 321| var·g_GameStanzaTimer·=·undefined;
|    | [NORMAL] JSHintBear:
|    | It's not necessary to initialize 'g_GameStanzaTimer' to 'undefined'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 945| »   g_IsTutorial·=·attribs.tutorial·&&·attribs.tutorial·==·true;
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with 'true'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1146| »   let·yPos·=·undefined;
|    | [NORMAL] JSHintBear:
|    | It's not necessary to initialize 'yPos' to 'undefined'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1581| »   if·(g_LoadingState·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1633| »   »   if·(playerData.some((pData,·j)·=>·i·!=·j·&&·sameColor(playerData[i].Color,·pData.Color)))
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1634| »   »   »   playerData[i].Color·=·g_PlayerColorPickerList.find(color·=>·playerData.every(pData·=>·!sameColor(color,·pData.Color)));
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1811| »   »   »   chosenCiv·=·pickRandom(Object.keys(g_CivData).filter(civ·=>·g_CivData[civ].Culture·==·culture));
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1825| »   »   let·usedName·=·g_GameAttributes.settings.PlayerData.filter(pData·=>·pData.Name·&&·pData.Name.indexOf(chosenName)·!==·-1).length;
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|2261| »   if·(g_GameStanzaTimer·!=·undefined)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with 'undefined'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|2301| »   if·(g_GameStanzaTimer·!=·undefined)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with 'undefined'.
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/187/ for more details.

elexis accepted this revision.Jun 13 2017, 8:00 AM

Tested with MP again.
Thanks for the patch!

This revision is now accepted and ready to land.Jun 13 2017, 8:00 AM
This revision was automatically updated to reflect the committed changes.
Unknown Object (User) mentioned this in Unknown Object (User).Jan 12 2023, 7:49 AM