pmmp / Math

PHP library containing math related code used in PocketMine-MP
GNU Lesser General Public License v3.0
41 stars 20 forks source link

Make Vector[23] immutable #4

Open SOF3 opened 5 years ago

SOF3 commented 5 years ago

Vector2 and Vector3 mutability is not self-documented. This is unclear in program design.

For performance reasons, it is possible to want to make a vector mutable. For API design reasons, it is possible to want to make a vector immutable.

This issue proposes the separation of mutability from the vector classes.

See https://medium.com/@elye.project/read-only-collection-in-kotlin-leads-to-better-coding-40cdfa4c6359 for a similar discussion about kotlin collections.

dktapps commented 5 years ago

This can be solved by moving Vector3 setters into a MutableVector3 class and making Vector3's fields protected. However, this has some problems with relation to PM since there's a vast array of things which depend on Vector3 being mutable (blocks, entities). We also have PM Position and Location to consider.

dktapps commented 3 years ago

My preferred direction for this is to just make Vector3 entirely immutable.

In addition, my previous recommendation for MutableVector3 would have been a bad idea, because it would have also made Vector3 implicitly mutable (anything that accepted Vector3 wouldn't be able to rely on the immutability of whatever got passed).

dktapps commented 2 years ago

Might be a good idea to wait until after 8.1 minimum is imposed for this. 8.1 has readonly for properties, which would allow us to continue to allow direct read access to the x/y/z fields without imposing the performance penalty that results from hiding them behind getters.