Added: Function GetOrthogonal for easier othogonalization of a 2 ve…

[dvdvideo1234]

Utilize: vector/angle method Unpack in TypeToString finction

[dvdvideo1234]

This date commit also reduces 6 string concatenations and one table extraction.

[Kefta]

1. You should not bundle unrelated changes together.
2. You should mention that this PR is reliant on your other Vector PR.
3. You should probably make the name GetOrghogonalVector or something.

[thegrb93]

Garrysmod is not Matlab. Don't expect generic math function to be added to the library. They are just code bloat.

[GrahamBest]

Garrysmod is not Matlab. Don't expect generic math function to be added to the library. They are just code bloat.

I mean, not really. The GMod Lua library has tons of "generic math functions"

Its a good change. Makes things easier, that is a fine update for me.

[thegrb93]

generic was the wrong adjective. I meant very specific use case and won't be used often.

[dvdvideo1234]

@thegrb93 @Kefta

Is the date change fine then ? I am just using OS.date and it is 6 tail concatenation less ;)

[dvdvideo1234]

To me atleast orthogonalizing vectors happens quite often and I have to code stuff like this. instead of just calling a function :)

[GrahamBest]

To me atleast orthogonalizing vectors happens quite often and I have to code stuff like this. instead of just calling a function :)

Some things I noticed that you should update:

Such as Returns hour, minute, secon in a formatted string.'
fix secon to second

Also, I don't necessarily believe that GetOrthogonal should be in the util library, thats more of just misc functions. The math library would be better, in my opinion.

[GrahamBest]

generic was the wrong adjective. I meant very specific use case and won't be used often.

Yea, I figured that's what you meant.
His push req isn't bloating code or anything (at least not to my eye). It seems fine.

[Kefta]

You still need to remove unrelated changes. This PR should only contain the GetOrthogonalVector addition, nothing else.

[Kefta]

Also, you need to fix the code styling to match the rest of the codebase.

[thegrb93]

I'm not saying it's not useful. I'm saying its not useful enough to be a core function. If it is, then it still shouldn't be in the util lib.

[Kefta]

The util library is the best place for it. math is currently for numbers, and it doesn't make sense in the Vector metatable since it modifies arguments and it's ambiguous which is the forward and up vector. Vector orthogonality is useful enough to be a core function.

[dvdvideo1234]

I am still going to let function util.TypeToString( v ) exist because currently this is the right way to do it using unpack being pushed to PROD.

[Kefta]

Put it into a separate PR. Also, you still need to fix the style.

[dvdvideo1234]

@Kefta I am using Tabs. An I missing something else ?

[Kefta]

Look around the file. There should be spaces surrounding parentheses (if ( bN ) then), and there should be empty lines surrounding control-statements, in this case, if and end.

[dvdvideo1234]

Others are requested in the PRs below:
#1621
#1622

[dvdvideo1234]

@Kefta Done, Found it. thank you!

[Kefta]

You still need spaces around control statements, and a space between if and the parenthesis. Ex.

function foo()
   somefunc()

   if ( condition ) then
      someotherfunc()
   end

   evenmorefunc()
end

Notice the spaces surrounding the if-statement.

[Kefta]

You still need a space between if and the parenthesis...

[dvdvideo1234]

@Kefta Done, thank you !

[dvdvideo1234]

I wonder is there a way for this to utilize VectorVectors?

[Kefta]

Yes, I've requested it to be pushed.

[dvdvideo1234]

@Kefta

I see it includes the normalization of the right and up vectors when only forward Z != 0, but does not normalize the forward vector as it is a constant reference. It means that the normalization of the unit vectors is not optional.

[Kefta]

The right and up vectors are the returns: they aren't provided to the function.

[dvdvideo1234]

But how does it decide which plane the second vector lays in. I mean if you try to create a three-unit vector coordinate system without an UP vector and only FORWARD one , this can have infinite solutions, which will result in a different output angle depending on the chosen UP direction.

[Kefta]

0,0,1 is used as the absolute up vector.

[dvdvideo1234]

@Kefta

Is it good then for [0,0,1] to be a default fallback value when a dedicated up direction is not provided?

[robotboy655]

I think this is too niche to be included with the base game.

Not every math function you use in your addon(s) needs including in the base game.