Phaser: Add Up/Down/Left/Right Shorthand (Like the current ZERO) to Vector classes

[Aedalus]

I'd like to propose the following static fields get added to the Vector2/3 classes as shorthand for common vectors.

UP: new Vector(0,1,0)
DOWN: new Vector(0,-1,0)
RIGHT: new Vector(1,0,0)
LEFT: new Vector(-1,0,0)
BACK: new Vector(0,0,-1) // Vector3 Only
FORWARD: new Vector(0,0,1) // Vector3 Only
ONE: new Vector(1,1,1)

I would be happy to make a PR to add this functionality to both classes, but wanted to confirm it was a welcome addition first.

[Aedalus]

Also, looking at the current implementation, it looks like its possible to screw up the constant if you do something like Vector2.ZERO.add({x:1,y:1})

Based on this documentation, it looks like if we changed it to use a getter, it would functionally become a constant. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/get

Object.defineProperty(Vector2, 'ZERO', { get: function() { return new Vector2() } });

[photonstorm]

Vector2.ZERO is meant to be used as a comparison constant. Or I guess you could clone it, but that'd be pointless as it's the default anyway!

I could add the constants outlined above, but again they'd need to be treated as something you'd use for comparisons, rather than instantiation. Although, you could clone one too I guess.

[Aedalus]

My two main use cases are comparisons and a quick shorthand for vector operations.

Pretty much just being able to do the following shorthand:

const myVector = new Vector2()

myVector.add({x: 1, y: 0})
myVector.add(Vector2.RIGHT)

myVector.normalize().equals({x:1,y:0})
myVector.normalize().equals(Vector2.RIGHT)

My game is really heavy on grids and tiles, so I found myself creating new Vector classes inherited from their Phaser3 equivalents, and add the constants on just for easy use.

My follow up comment wasn't suggesting that ZERO necessarily be used as a constructor, just that it seemed like odd behavior that the current implementation allows you to mutate the value of ZERO and possibly mess up future comparisons on accident!

[photonstorm]

To be fair, you could mess up _any_ of the constants if you're not careful. Replacing the value of Phaser.Math.PI2, for example, would have some pretty nasty internal impacts.

Changing them to be a getter would create a brand new instance for every single comparison in your code, which sounds like heck of a overhead to me. We could always pass them through Object.freeze, but browser support isn't universal.

[Aedalus]

Those are good points. If you were using it in every game loop tick you're just slowing it down a lot. I could still see some (very minor) utility in just providing the additional constants LEFT/RIGHT/etc, but just having to be careful not to mutate them, same as ZERO.

If it seems like it's just unnecessary fields for Phaser3 and wouldn't be commonly used, I'd definitely understand and we can mark the issue as closed.

[photonstorm]

I'm happy to have them as constants if you'd like to submit a PR? As I think the concept is sound. I just wouldn't like them as getters. We can comment in the docs for them that you should avoid mutation. Or, we could do a quick feature test for freeze support and then apply that?

[Aedalus]

I've created a PR here: https://github.com/photonstorm/phaser/pull/4290

Please let me know if I need to change anything!

[photonstorm]

Thanks for opening this issue, and for submitting a PR to fix it. We have merged your PR into the master branch and attributed the work to you in the Change Log. If you need to tweak the code for whatever reason please submit a new PR.