Page MenuHomeWildfire Games
Differential D678

Autocomplete playernames first
ClosedPublic
Actions

Authored by elexis on Jun 26 2017, 4:40 PM.

Details

Reviewers
vladislavbelov
Commits
rP19837: Autocomplete playernames before everything else, as that is the most expected…
Summary

echotangoecho and mapkoc have complained about playernames not being autocompleted before other strings.
For example "map" is autocompleted to "Map Type", not "mapkoc" and "ver" is autocompleted to "Very Large", not "Vercingetorix".
Hence this changes the feature to use a priority value.

Test Plan

Apply the patch, press F9, type warn(g_Autocomplete); and make sure there's no "100" in it, because Population capacity values should not be autocompleted.
Host a game with the playername "numb", autocomplete "num" to "numb" and "numbe" to "Number Of Players".

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

elexis created this revision.Jun 26 2017, 4:40 PM
Harbormaster completed remote builds in B2414: Diff 2702.Jun 26 2017, 4:40 PM
Vulcan added a subscriber: Vulcan.Jun 26 2017, 6:21 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-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
|1144|1144| 
|1145|1145| function verticallyDistributeGUIObjects(parent, objectHeight, ignore)
|1146|1146| {
|1147|    |-	let yPos = undefined;
|    |1147|+	let yPos;
|1148|1148| 
|1149|1149| 	let parentObject = Engine.GetGUIObjectByName(parent);
|1150|1150| 	for (let child of parentObject.children)

binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 695| »   »   »   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
|1600| »   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
|2050| »   »   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
|2050| »   »   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
| 946| »   g_IsTutorial·=·attribs.tutorial·&&·attribs.tutorial·==·true;
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with 'true'.

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

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

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1634| »   »   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
|1635| »   »   »   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
|1812| »   »   »   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
|1826| »   »   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
|2262| »   if·(g_GameStanzaTimer·!=·undefined)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with 'undefined'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|2302| »   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/239/ for more details.

Vulcan added a comment.Jun 26 2017, 8:08 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/1632/ for more details.

Harbormaster completed remote builds in B2414: Diff 2702.Jun 26 2017, 8:08 PM
vladislavbelov added a subscriber: vladislavbelov.Jun 27 2017, 2:54 PM
vladislavbelov added inline comments.
binaries/data/mods/public/gui/gamesetup/gamesetup.js
2310 ↗(On Diff #2702)

Does it make sense to make it object:

let autocomplete = {0: []};

Since you use object keys later. Also what's better by performance?

2329 ↗(On Diff #2702)

Too long string, IMO it'd be better to add a comment here, what it does.

elexis updated this revision to Diff 2716.Jun 27 2017, 4:38 PM

Use { "0": [] }, which has the advantage of not leaving undefined array entries but the disadvantage of converting numerical keys to strings, which might be unexepcted, but works anyway.

Adds a .sort() just in case. Object.keys should use the order of for...in and for...in the insertion order. But I couldn't confirm this on our platform.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/keys
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for...in

Add JSdoc comment for that function.

Harbormaster completed remote builds in B2422: Diff 2716.Jun 27 2017, 4:38 PM
Vulcan added a comment.Jun 27 2017, 5:44 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/1639/ for more details.

Vulcan added a comment.Jun 27 2017, 5:46 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-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
|1144|1144| 
|1145|1145| function verticallyDistributeGUIObjects(parent, objectHeight, ignore)
|1146|1146| {
|1147|    |-	let yPos = undefined;
|    |1147|+	let yPos;
|1148|1148| 
|1149|1149| 	let parentObject = Engine.GetGUIObjectByName(parent);
|1150|1150| 	for (let child of parentObject.children)

binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 695| »   »   »   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
|1600| »   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
|2050| »   »   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
|2050| »   »   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
| 946| »   g_IsTutorial·=·attribs.tutorial·&&·attribs.tutorial·==·true;
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with 'true'.

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

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

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1634| »   »   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
|1635| »   »   »   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
|1812| »   »   »   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
|1826| »   »   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
|2262| »   if·(g_GameStanzaTimer·!=·undefined)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with 'undefined'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|2302| »   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/245/ for more details.

Harbormaster completed remote builds in B2422: Diff 2716.Jun 27 2017, 5:46 PM
elexis updated this revision to Diff 2726.Jun 27 2017, 11:10 PM

Fix typo autocompletes/d and use (x || []).concat instead of an if-statement

Harbormaster completed remote builds in B2430: Diff 2726.Jun 27 2017, 11:10 PM
vladislavbelov accepted this revision.Jun 27 2017, 11:14 PM
This revision is now accepted and ready to land.Jun 27 2017, 11:14 PM
Closed by commit rP19837: Autocomplete playernames before everything else, as that is the most expected… (authored by elexis). · Explain WhyJun 27 2017, 11:25 PM
This revision was automatically updated to reflect the committed changes.
Vulcan added a comment.Jun 28 2017, 1:04 AM
Executing section Default...
Executing section Source...
Executing section JS...
Executing section XML GUI...
Executing section Python...
Executing section Perl...

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

Vulcan added a comment.Jun 28 2017, 2:02 AM

Build has FAILED

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

Harbormaster failed remote builds in B2430: Diff 2726!Jun 28 2017, 2:03 AM

Revision Contents
Changeset List

PathSize
ps/
trunk/
binaries/
data/
mods/
public/
gui/
gamesetup/
40 lines

Diff 2728

View Options

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

Loading...
Wildfire Games · Phabricator