search

yoanlcq / vek

Generic 2D-3D math swiss army knife for game engines, with SIMD support and focus on convenience.
https://docs.rs/vek
Apache License 2.0
282 stars 32 forks source link

Transform2 type #54

Open wackbyte opened 4 years ago

wackbyte commented 4 years ago

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

yoanlcq commented 4 years ago

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.

wackbyte commented 4 years ago

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

Koopa1018 commented 4 years ago

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...????"

yoanlcq commented 4 years ago

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.