Is defined as a static variables (shared in all instances)

like the functions. So might just as well make it consistent?
()I recall the this keyword is sometimes pointing to unexpected objects (different callers).

I write it as GameMap.types in an attempt not having to make another global g_GameMapTypes and thus no global scope pollution.
But if you want consistency I can also write it as g_GameMapTypes

That's a nice offer, but I was asking for .prototype though.
Read: GameMap.types -> GameMap.prototype.MapTypes.
Hypothetically speaking, since this variable is a copy from gamesetup that should be avoided, at least for vanilla.

Needs to expand inner array so won't work but seems [].concat.apply([], arrays) will

Deleting it given that I noticed is not used in the code.

return this.MapTypes.reduce((mapTypes, mapType) => ...concat..., []);

x = (x + step) % max is a trap

-1% 10 = -1 in javascirpt

x = ( x + max + step ) % max solves this

but I don't want it to be cyclic

Can't find how to make a one liner with Object.assign

I never knew about it until bb discovered it and added it in a gamesetup.js. (If I recall correctly theres still an unneeded {} in that call)

Is it really necessary? I looked at the gamesetup.js example and seem overly complex for this case

We might find out which implementation is nicer after having seen how the line would actually come out.
I think Object.assign just works like concat, but for objects instead of arrays, so quite easy.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/assign

Notice that this code is copy, it shouldn't exist. If it doesn't exist, we don't have to argue how it should be written. But I guess the copy would have to be moved, so can't escape the lint I suppose. Actually I rewrote the function too somewhere and moved it into a prototype.

So perhaps we can just exclude these copypasta functions from the review for now.

Does work tho.

eae pro elexis +1

What do you mean by that?

thought the same nani +1 eae

That "all" is already present unless it's an empty array?

eae

what a troll (nani), eae

For the record I copy-pasted some code given that i only needed some functionality of game setup but wasn't possible to just grab some function without the rest of the gamesetup.js code.

It should be possible to grab the mappreview and mapfilter ones without messing with the other code?

Needs a function not am arrow function.

function (map, filter = undefined) is synonymous with function (map, filter) and (map, filter) =>

Being the take on this what?

Done

Can't, given they all depend of each other (JS gives error at creation given that it doesn't know of the other keys)

Not really but linear doesn't mean better.

(Guess I was wondering if one could find a formula to compute the numbers rather than hardcoding the numbers)

I did it thinking of prepareForDropdown as is takes data from the same source structure and transforms it as a more useful one for this case.

Imho, in this case is much nicer to have two separate functions for each case.

Changed them as fractions 400/512 300/512

It does not. Is just stores last input the real input is unchanged. When pressed will check to compare the real caption with the this.lastCaption "copy".

Sounds weird and I didn't try to understand it, but why does this check for mouse presses when the caption doesn't change upon clicks; just subscribe to OnPress? You can find other available events in SendEvent lines in CInput.cpp. For example the new textedit that I already forgot about.

Oh, didn't know of textedit event. But either way, the map list can change while text still being on the text input (by the dropdowns) so it needs up update the list to the search list if the search box is selected. Maybe would be easier to understand if you try the patch search and use the drop downs at the same time.

Well I didn't even try to understand yet. Mostly wondering why this has to go through onTick rather than just calling an update function if the event of your needs is detected?

+1 eae

As is comparing strings I think is more clear the comparison.

elexis means its done his way everywhere in the code bc its shorter eae

elexis means its done his way everywhere in the code bc its shorter eae

• (not my way, I learnt it from others too)

As is comparing strings I think is more clear the comparison.

• What information does caption === "" carry that !caption does not carry? The stict equality test caption === "" also leads the reader to question whether it is important to distinguish between === "" and == "" and falsy values. Notice that caption === "" does actually not inform the reader whether caption is a string.

• the codebase should be consistent, we don't want to see a different way of writing code when opening a different file. So if the argument is in favor of === over == then this should be changed in all files, not only new code. If it doesn't really matter which one we chose, why not chose the more globally consistent variant over the personal preference variant?

Same is true for const in local scope btw.

(not my way, I learnt it from others too)

so it became ur way eae

Removed braces.
But I still think having each part separated makes it more digestible.

pro

:-)

total better but more to understand the code then

:-)

cant tooltips var be killed? eae