vek icon indicating copy to clipboard operation
vek copied to clipboard

Transform2 type

Open wackbyte opened this issue 5 years ago • 4 comments

I added a Transform2 type and moved Transform to Transform3. Fixes #4.

wackbyte avatar May 17 '20 01:05 wackbyte

Hi,

Thanks for the PR. I'll happily merge it once you fix the errors reported by AppVeyor CI.

Renaming Transform to Transform3, although a good naming choice for the sake of consistency with Transform2, is a breaking change I am somewhat reluctant to accept. I think it is better to just leave it as it is, especially since 3D is the default, common case.

Users can always use renaming imports if they wish. On our end, however, I do not think pub use Transform3 = Transform; is a good thing to do either, because then users would wonder which they should use and why there is an alias at all.

In any case, this is a welcome addition. Just please rename Transform3 back to Transform, the benefits largely outweight the disadvantages. Thanks in advance!

While I'm at it, would you be opposed to renaming Transform2 to Transform2D? Both names are fine to me, but I prefer the second one.

yoanlcq avatar May 17 '20 09:05 yoanlcq

I prefer Transform2 as it uses the same naming scheme as the other types, like Vec3 or Mat4.

wackbyte avatar May 17 '20 13:05 wackbyte

I would argue against Transform2 as well, because it doesn't adequately communicate the intent of the the type. Having it named like Vec3 and Mat4 suggests that it is like Vec3 and Mat4--a vector type of some kind--which isn't what Transform is for at all!

If we additionally assume users will look for Transform2d when they need a 2D Transform, suddenly we've got two layers of confusion: "I need a 2D Transform type but there's only this Transform2 thing; is that what I need, or is that a vector, or...????"

Koopa1018 avatar Aug 13 '20 21:08 Koopa1018

Just my 2 cents, while I'd be in favor of naming it Transform2D or Transform2d, I don't think naming it Transform2 is that big a deal either. The struct can be quickly accessed and figured out, and in some IDEs or editors, the doc comment is shown when hovering over the type.
I don't think it can be confused with a vector, indeed because there is Vec2 already, so there is not much else a Transform2 can be.

So I don't think the name is that ambiguous, and in rare cases where it would be, it's easy to access the documentation and figure it out quickly.

I get that this is fairly subjective; for this kind of stuff, it's good to see how other well-known libraries do it.

yoanlcq avatar Aug 14 '20 10:08 yoanlcq