Vec2.rotate(degrees), Vec2.angle, Vec2.angleTo methods

[Maksims]

Adds two methods to Vec2 to help with converting to radians and rotating it.

New APIs:

// pc.Vec2
vec.angle() // returns Euler angle of a vector
vec.angleTo(vector) // returns shortest Euler angle from one vector to another
vec.rotate(degrees) // rotate a vector by degrees (by Euler angle)

I confirm I have read the contributing guidelines and signed the Contributor License Agreement.

[mvaligursky]

I think rotate should take an angle in degrees, like all public facing functionality in math.
And the radians should be get angle that returns the angle in degrees as well.

[Maksims]

I think rotate should take an angle in degrees, like all public facing functionality in math. And the radians should be get angle that returns the angle in degrees as well.

Good point. I've updated the PR and added additional method: angleTo.

[mvaligursky]

Also, please update the PR's title.

[slimbuck]

Could you add a src = this param to rotate()? I did this for some vec2, vec3 and quat functions recently, eg https://github.com/playcanvas/engine/blob/main/src/core/math/vec2.js#L416. It makes the interface a little more consistent, a little more flexible and in some cases will result in one fewer vector copy.

[Maksims]

@slimbuck, not sure where would src argument would go in rotate method. As this method is within semantics of mulScalar or addScalar, where only modifier argument is provided.

[slimbuck]

It would go like this:

    rotate(degrees, src = this) {
        const angle = Math.atan2(src.x, src.y) + (degrees * math.DEG_TO_RAD);
        const len = Math.sqrt(src.x * src.x + src.y * src.y);
        this.x = Math.sin(angle) * len;
        this.y = Math.cos(angle) * len;
        return this;
    }

You're right rotate could be considered closer to mulScalar than normalize since rotate takes takes a parameter like mulScalar, but I'd say we could easily add src parameter to mulScalar one day too, why not?

[Maksims]

but I'd say we could easily add src parameter to mulScalar one day too, why not?

Sure, shall then this be left as is, to ensure consistency. And if src to be added to similar methods, that would come from a future PR?

[slimbuck]

Sure :)

[willeastcott]

Question: should this be a function or a getter?

[Maksims]

I believe it should be a function, to stay consistent with similar methods, such as: length()

[willeastcott]

That's a good and fair point. But I'm wondering whether length, for example, should also be a getter these days. If we do decide to leverage getters in the math API, that's for another PR... 😄