Perhaps move those two into the prototype

coding convention is to use ++i except where the return value matters

(spaces between operators)

(Wondering if the two if (type == cases should go to helper functions, but scope creep I guess)

(&& ... .length) if you will

{ } -> {} if it needs a value, same L730

If objects are written this way:

let notification = {
	 "players": [cmpPlayer.GetPlayerID()],
	"message": markForTranslation("Can't find free space to spawn trained units"),
	"translateMessage": true
};

it has the advantage that the reader can count and locate properties without reading all property values.

Also the notification variable can be inlined.

time can become negative time -= item.timeRemaining so time >0 is needed

does that not mean x times more variables where x is number of entities with production queue in game?

for...of if you want

-\n

perhaps for...of loop (I heard it was said it's slower, but what's the data for that actually, i.e. does that also include the cost for this.queue[i] lookup, and is it significant enough? I guess this function is only called per user click so its negligible here anyway but the iteration difference may still be relevant for code that is run often)

inline unitCategory

+1 tab should be enough

(I've always used +1 tab instead of tabs for object properties, but its not set in stone I guess)

Nope, its stored in the prototype of the object, just like the functions.

The component objects have owned properties and the prototype property which an object reference.
When accessing a JS object property it checks first if the object has that property, after that it checks if the prototype has that property, then it checks if the prototype of the prototype has that property etc.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Inheritance_and_the_prototype_chain

(Trivia: One could run some turns of a match with -ooslog and see the text representation of the savegame to see if these variables would end up in the savegame. The savegame only writes owned properties (see the JS_Enumerate call in CBinarySerializerScriptImpl::HandleScriptVal which https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/JSAPI_reference/JS_Enumerate If they were present in that file for each ProductionQueue entity component then it'd be stored per class instance. If they're not in the file, it wouldn't rule out that there is still one copy of the number per class instance in memory, so for that we have to either trust the specs or read the spidermonkey code)

(Other than that usually the first association of hardcoded literals in simulation components I have is to move them to the template files, since the purpose of the template files is to store all the values of simulation components to begin with
(I suppose a constant should be moved to a template if it makes sense to have at least 2 different templates with different values and in the prototype if we know for sure that the property or restriction is one of the code itself and thus applying equally to all instances. I guess it's still better to move them to the prototype than not moving them at all, considering the somewhat ugly letter cases, whatever)

no, we need i for L498

that are space not tabs :)
(For any alignment within a line of code (as opposed to indentation at the start), use spaces, not tabs.)
Or did I get that wrong ?

space after { and before }
supercede -> supersede

-a
-by

(ideally there should be no speculation in code)

(I suppose its slightly more performant to pass undefined than creating a garbage-collection tracked empty object, I suppose semantic changes are out of scope of this diff)

cmpPosition is only needed in one of the cases, so (unless the JIT optimizer is smart), it would become less performant for no benefit or did I miss something?

// so

1000msecs shouldnt be hardcoded in the comment

(Comment redundant with function name?)

(This is not a sentence, so the period would be grammatically wrong?)

// If
-.=

Its one convention people recommend, I personally didn't like nor use it (as 4 spaces require 4 more keypresses to insert and navigate and provide more surface area for too many or too few indentation). I don't care much, only problem is that if we dont use the same CC then we'll have inconsistent code, one patch adding it this way the next the other way, perhaps even some patches changing the same lines back and forth, so in theory it should become conventional or perhaps the consequences of different styles aren't so impactful, whatever)

^ This, not?

One of the cases is in for loop and it is when unit is not going to be auto-garrisoned what happens in most cases when spawning units

Variable? Dunno if it is smart enough to not compute the multiplication twice.

Like here.

delete this.timer?

It seems like a big waste to recreate the array and every item of it when it seems only one property of every object changes, or did I miss something?

I suppose it's cloning the thing to avoid modification. It could avoid doing so by letting the consumer of the return value make that one computation.

It also opens up the door for the consumer to unintentionally modify the object, but that probably should have lower precedence than copying it. From a quick glance, it doesn't seem to be used that rarely (AIProxy, GetEntityState).

(Its not a regression if its not addressed in this revision.)

timer is cancelled and what is in variable this.timer does not matter because this entity is going to be destroyed

Move below? Inline?