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?

Minimum

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

Sure dragStart isn't better off as a new Vector2D if we're already creating a new object?

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?

let

could be

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

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

inline

vector1.distanceTo(vector2)

I'd probably set this a lot higher.

use a cap

is there a real need for the minlength?

unneeded parentheses, && binds stronge than || like * and +, in doubt lookup "operator precedence"

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

let reworkedLine = [inputLine[0]]

reworkedLine => entityDistribution

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.

(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)

+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?)

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

return action && doAction

no space needed after -

distanceBetweenEnts?

Don't think this is really required, units will sometimes still run around a bit, but that is ok with me and this code doesn't help too much in that I guess, since the area of the line is rather small most of the time
(for me even first unit => first spot would be ok, or for gameplay it could be last spot too depending on what is closer)

shouldn't we multiply with shouldDist?

what if there are two missing points?

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]

for of

vec1.distanceTo(vec2)

also could compare the squares for performance

inline that in L1186

spaces around -

spaces around - (same below)

I think you want to use the reverse() method

unneeded braces

could be a forEach

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

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

language

language

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

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).

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

Shouldn't this support queueing too as usual?

return if !right

no else after return

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

else if
(Theoretically check isn't needed)

inline

(could be slice aswel, no strong opinion)

many points

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)

Engine.HotkeyIsPressed("session.queue")