[PATCH] Improve FindIdleUnits of GUIInterface

[elexis]

hasIdleWorker of session.js (see r17674) calls the GuiInterface.js function FindIdleUnits for every type of g_WorkerTypes. Instead it should rather support the passing of the whole array. FindIdleUnits itself should not loop twice over playerEntities but only once.

[sanderd17]

A first review on code style (not yet on functionality): input.js

• line 1710: it's better to have a positive condition (currIdleClassIndex == 0) than a negative (currIdleClassIndex != 0). So you could swap the two around. Though I doubt that selector is needed, as AFAICS, when the index is 0, the slice and concat operation will do no work.
• line 1725-1737: try to avoid the extra indentation level. When you just return in a certain case, do an if (...) return;, and proceed with the other case on a lower indentation level.

GuiInterface.js

• Line 1666: for..each..in should be avoided in favour of for..of when possible.
• Line 1675: we normally don't use the bitwise OR but the logical OR to make sure something is defined (also with spaces around the operator, so it's obvious what's happening).
• Lines 1679-1685: those braces aren't needed, so should be left out according to our coding conventions.
• Line 1695: you don't need the else.
• If a function won't be used in the GUI itself (but only as a helper in the GuiInterface), it shouldn't be made public (a.k.a. included in the bottom list of public functions).
• Making a function-scoped variable to modify in an array.some is quite ugly code.
• A good Array should consist of objects of the same type (all booleans, all numbers, ...). If you want to return a boolean and a number, you should probably use an object to give meaning to the values.

[Stephen Imhoff]

sanderd17 - you're right about the `array.some` being lucky. I just hadn't seen `array.findIndex` yet.

[sanderd17]

I don't know if it's possible to use the MatchesClassList helper, but as that one also supports AND, OR and even NOT operations on class combinations, it might result in more versatile code. ​http://trac.wildfiregames.com/browser/ps/trunk/binaries/data/mods/public/globalscripts/Templates.js#L47

[elexis]

• if( -> if (
• L1719 early return if (!idleUnits.length)\n return; or similar
• L1678 -> if its not an array make it an array. always push the element (reduces the code slightly)

[Stephen Imhoff]

L1719's change is a little more involved; it needs to reset the cursor, as no idle units were found. There's also an included TODO about "nobody found" notifications should be added. Changed, but I can reverse it.

MatchesClassList is interesting, but unfortunately I don't think I can use it.  I'm using findIndex specifically to get the sorting bucket to put a given unit in, which MatchesClassList won't/can't give me.  I could use a modified version of the helper, I guess, but it'd need some pretty wide changes to return the information I need.

[sanderd17]

In 18139:

Improve findIdleUnit and hasIdleUnit code. Patch by Clockwork-Muse. Fixes #3826