Finalize "Update vector processing" (#5061)

[lL1l1]

Description of the proposed changes

1. Rebase Update vector processing #5061 to the latest develop version while fixing conflicts.
2. Fix structure alignment to terrain.
3. Test and code review the rest of the changes.

Testing done on the proposed changes

Testing structure orientation:

Put this command into the console then spawn units by pasting from the clipboard like normal.

UI_Lua 
local squareSize = 9
local outStr = ''
for x=0, squareSize do
   for z=0, squareSize do
      outStr = outStr .. string.format([[CreateUnitAtMouse('ueb0301', 0,    %f,  %f, %f)]]..'\n'
      , x * 8 - squareSize/2*8
      , z * 8 - squareSize/2*8
      , Random() * math.pi
   )
   end
end
CopyToClipboard(outStr)

Results (map is "Project Albus"):

The raw numbers when logged are very slightly different but I don't see a visual difference.

Generate a bunch of wreckages using ScenarioUtilities

SimLua 
local SUCreateWreckage = import('/lua/sim/ScenarioUtilities.lua').CreateWreckage
for i = 0, 50 do
   local unit = CreateUnit2('xsa0402', 1, 'Air', 100 * Random(), 100 * Random(), math.pi * Random())
   ForkThread(function()
      WaitTicks(1)
      SUCreateWreckage(unit, true)
   end)
end

Seems to still create random orientations. Unfortunately due to the TryCopyPose in Unit:CreateWreckageProp this doesn't work on units not in the air layer (land, navy, landed air units), so it must be tested with an air unit + 1 tick delay to get into the air layer (spawns on land layer).

Reviewed the utilities.lua and TerrainUtils.lua changes to make sure the logic stayed the same.

Unit rotation functions review:
Just spawned a tank unit (walkers always stay vertical) on a slope and saw the commands do what the annotations say:

SimLua 
ArmyBrains[1]:GetListOfUnits(categories.ALLUNITS)[1]:Rotate(45)

SimLua 
ArmyBrains[1]:GetListOfUnits(categories.ALLUNITS)[1]:SetRotation(45)

Ran a small m27AI skirmish with no errors.

Checklist

• Changes are annotated, including comments where useful
• Changes are documented in the changelog for the next game version

[lL1l1]

For what reason is the roll negated?
Also I'm not sure why it is important that here we do the terrain quaternion * orientation when other parts in this PR do orientation * quaternion just fine.

[Garanas]

That is probably best asked to @Hdt80bro , I do not recall myself. I think I negated it because otherwise it would not orientate properly.

[Hdt80bro]

You did, and I tried to extract the original mathematics that would interpret roll correctly without negation, but got stuck. Similarly, since I didn't quite figure it out, sometimes it needed to be multiplied on the left to rotate, others multiplied on the right (it is sensitive to the side its multiplied on). It's something I wasn't happy about and why this PR got stuck in limbo.

[lL1l1]

I spent some time figuring it out, the reason roll is negated is because the terrain calculation's roll is given with respect to the -z axis, and units use +z.

This is the x component of the basic form of the calculation for small structures (only 1 edge is calculated).
The number we get is atan2(dy, lx), where dy is the terrain height in the middle of the -x side edge. This number is the trigonometric representation (local perspective) in the bottom right.
But notice how in the diagram of the real world situation (global perspective) in the bottom left the triangle is flipped horizontally, and that when using right-handedness, the angle we get is on the -z rotation axis.

Averaging is done correctly by negating the second component, which is measuring the other side's angle which is flipping the axis of rotation 180 degrees.

fa/lua/sim/TerrainUtils.lua

Lines 24 to 25 in 81a95af

	local rightX = MathAtan2(GetTerrainHeight(posX + lenX, posZ) - posY, lenX)
	angleX = (angleX - rightX) * 0.5

equivalent to avg = (angle1 + -angle2) / 2

In comparison to Pitch, Roll is done incorrectly because it starts with pos - lengthX/2 towards the -x side edge which gives an angle around -z, but Pitch is correct because it also starts with pos - lengthZ/2, and that goes to the -z side edge, but that angle rotates around +x, which is the same as units.

[lL1l1]

Also I must've made a mistake in Projectile.lua OnTrackTargetGround because the game doesn't use y,z,x,w format quaternions but I haven't solved that yet, and the code is at least functional.

[lL1l1]

The utility tests fails because of

fa/lua/system/utils.lua

Line 885 in b93c634

local vector_metatable = getmetatable(Vector2(0, 0))

but this line works in-game, since Vector2 is a Core global.

[Garanas]

This is great @lL1l1 , it would be great to finalize this and get rid of all the magic mathematics that is in some of the Lua files 👍

[lL1l1]

@Hdt80bro @Garanas @BlackYps I think everything is figured out and this PR is finished. Please take some time to review it.

[Garanas]

I've reviewed the behavior in-game and all appears to be good. The relevant code changes where the new logic is applied look tremendously better.

An absolute high-quality contribution. Nothing but exceptional. This is the type of in-depth analysis at an almost professional level.

[Garanas]

As I am not that great at mathematics, I'll leave the review of this particular file to @BlackYps, @Hdt80bro or perhaps even @clyfordv .

[BlackYps]

I didn't trace every single operation, but that everything in the game works correctly gives me confidence that the math is correct too.
The files are much more readable now, very good work!
If I am just out of the loop about which functions are supposed to be deprecated, then you can merge this

[lL1l1]

A problem with the __add metamethod is that it has quite a few safety checks that slow down the operation. It can be faster than engine functions and even lua since it has the vector metatable cached for use with setmetatable (faster than Vector()).
Should the vector metatables be split for the different types? Then the only thing that needs to be compared is getmetatable(b) == vector/quat/vector2_metatable.

[4z0t]

All vector metatables can be expanded via bin patches and done with performance priority

[BlackYps]

I'm afraid I neither understand the question, nor your accompanying explanation...

[lL1l1]

Currently Vector, Vector2, and Quaternion use the same vector_metatable to do their operations. Because of this, a lot of the operations have quite a bit of type checking slowing them down. If each type had its own metatable, then over half the type checks wouldn't be needed as the type would already be known.
Should that be done here or leave it for later?

[BlackYps]

It does sound like a good idea, but I suggest to do that in a separate pull request. This one is already quite big and I'd like to have it merged. It's already a viable pull request.
Further improvements can be split out to make reviews easier.

[Garanas]

I agree, those changes would be worth while in my opinion. But let's do that in a separate, new pull request. That way we can merge this in. Other pull requests such as #6477 rely on it.

I'll wait for a response and then I'll merge it in.