globals should be g_Foo (input.js has it wrong, but better add it correctly than having to change it later)

a sentence should end with a period

(I actually meant all 3 could be const, so only the upper one is a var)

We should find a unique catchy name like freehand selection, freehand formation, or input line and then make it g_Name_Property, so tht it is easily distinguished from other globals and so that the reader knows the connotation immediately even if there is no danger of confusion

to make code clearer, these could be const

Why not make it 2 then?

If we have comments, they should use JSdoc.
Globals should have one brief information (i.e. that isn't self evident) explaining what they are used for.

periods

(Why take the square but not the other function from that commit?)

Could use some bipartite weighted matching algorithm.
For example the Hungarian algorithm (https://en.wikipedia.org/wiki/Hungarian_algorithm). Though I don't know if that will be fast enough...

no

should become a Vector2D, can use Vector2D.sub and distanceTo then to simplify the code a bit

we do not put spaces after [ and before ]

compare the squares for performance, also sqrt takes only one argument,
we should check against maxDragDelta right?

(Always use let instead of var so that the reader only has to consider the scope that the variable is defined in. Also the variable is used only once so can be inlined.)

-1 tab

Why not:

if (action)
   return doAction...;

There is already a break on L1085

why not the euclidic distance?
like this it triggers at different limits depending on the direction...

MouseMove

Vector2D.from3D

wasn't the argument to change these lines in a previous iteration that it is a different distance for different directions?

I would have said target.distanceToSquared(FreehandSelection_InputLine[g_FreehandSelection_InputLine.length - 1]), but this one might be faster.
But then why does it actually square the number if we don't use euclidian distance anymore and just not use Math.abs?
This is only executed when sending a command and this only saves some microseconds I guess?

language

also could compare the squares for performance

also taking the square helps for performance (one could also store the resolutionSquared for even better performance)

return if !right

spaces around -

let

language

could be

let p0 = inputLine[0];
let entityDistribution = [p0];

no else after return

maybe freeDist

min => minimum

could use p1

That magic number 1 should become a global, so that the meaning is expressed in the variable name and it can be changed without reading the code.

!...length

missing points and is that comma correct?

(unneeded commment)

technically check unneeded, I am fine with keeping them (yellers may yell)

Could be merged like this?

let p = ...
if (...)
    g_InputLine.push(p);

(Besides the braces of that if not being needed, the if itself is not needed / implied by the loop either, right?

should work with for...of too

(comment implies there could be more that 1 point missing, but code doesn't handle that)

missing or too many
Isn't that one too many at most?

inline

inline that in L1186

I'd probably set this a lot higher.

use a cap

is there a real need for the minlength?

many points

spaces around - (same below)

If one wants to take into account multiple points missing, one could
entityDistribution = entityDistribution.concat(new Array(selection.length - entityDistribution.length).fill(inputLine[inputLine.length - 1])

(check theoretically unneeded that way)

else if
(Theoretically check isn't needed)

(could be slice aswel, no strong opinion)

we doesn't seem to need the index, so we should use for ...of
Also perhaps i=> pos

let reworkedLine = [inputLine[0]]

reworkedLine => entityDistribution

distanceBetweenEnts?

There are some edgy cases here still, f.e. allign the units on a line, and send them to a line orthagonal on the first, with one of the endpoints on an endpoint of the original line (So we turn the line 90deg), I would expect the unit on the endpoint of both lines to stay where it is, and swing the line over, However (lets assume that unit is the first on the first corner, argument is symmetrical) then firsttofirst < firsttoLast, but neither of the first conditions holds.
Maybe it would work to check firstToFirst+lasttoLast>firstTolast+lasttofirst (as that would minimise the straight line distance in such a case)
Testing with that looks ok to me

If you accidentally select a cc and drag the mouse when setting a rally point, you'll get:

ERROR: JavaScript error: uncaught exception: out of memory

Because if selection.length = 0, shouldDist is negative and the while loop on L1244 doesn't end.
Should probably be careful about dividing by zero too.

I think you want to use the reverse() method

no space needed after -

inline

return action && doAction

could be a forEach

unneeded braces

This will spam the replay files. Not sure if there is a performance overhead attached too.
It seems cleaner to extend Commands.js by a new command type and then do the unit commanding in there, would you agree @bb, @Imarok?
The computation of destination positions should likely remain in here, so that we pass as few information as possible over (also I guess screen coordinates only make sense in this file).

Engine.HotkeyIsPressed("session.queue")

shouldn't we multiply with shouldDist?

Could use type: hotkey ? "walk-line" : "attack-walk-line"

Shouldn't this support queueing too as usual?

(Perhaps its nicer to use Vector functions here to avoid the duplication of the computation for the second axis. Performance likely no issue as this only occurs on button up. In fact it looks like x0 and z0 are exactly the coordinates of that vector 3 lines above. Also some of the vectors used in here (x1-z1) could be computed above the while loop)

what if there are two missing points?

Vector2D.sub(vector1, vector2) seemed nicer than clone.sub. normalize() instead of division by dist

for of

vec1.distanceTo(vec2)

+1 tab

(comment redundant with the code)

That vector is created twice. Also doesn't the entity state have that vector already?

Some of this code looks duplicated. Could it be moved to a helper function?

This looks broken (as in missing property names and spaces)

== null is also true if == 0 if I'm not mistaken (see "falsey"). If you want that you can just use !minDist, otherwise === null

(probably better from a performance point of view to use the pythagorean distance function from Math rather than creating an object that is used only once)

(looks like it could use for (let ent in selection), or for...of?)

if reworkedLine.length> selection.length we will fail here, maybe change it around and let every unit go to the closest free spot on the line

min => minimum

instead of passing reworkedLine and i it could be reworkedLine[i]