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

Add optional `az` dependency, implement its traits #89

Closed Patiga closed 1 year ago

Patiga commented 1 year ago

az provides a variety of conversion traits which number types can implement. In particular, fixed implements those traits.

We could consider introducing a method for each of the conversions. Not to avoid bringing the traits into scope. To annotate the destination type of casts, you need to do vec.checked_as::<Vec2<u16>>()?. With a method, we could remove the Vec2<_> and condense it down to vec.checked_as<u16>()?>().

I wasn't sure which version I should use, so I used the first one which offers all of its traits, which is also compatible with the newest release version-wise.

More context in #88

yoanlcq commented 1 year ago

That looks good to me. I think it could benefit from the introduction of more ergonomic methods as you describe.

Any specific reason for not using az's latest version?

Patiga commented 1 year ago

Any specific reason for not using az's latest version?

Not really, I just wasn't sure which version such a general crate should use and thought that lower might be better.

Would you see it as problematic if I call the methods the same as they are called in the trait?

yoanlcq commented 1 year ago

Not really, I just wasn't sure which version such a general crate should use and thought that lower might be better.

Ok, that's fair.

Would you see it as problematic if I call the methods the same as they are called in the trait?

I think it's fine; it's already the case for as_, numcast, mul_add.

Patiga commented 1 year ago

I wasn't sure if the methods would collide with the traits if they were in scope, but methods seem to take precedence (playground).

First I did the impl block for the methods under the implementations of the traits, however, that made them appear at the very top of the documentation. That is why those two code blocks are that far apart.

I also added some tests to verify that I didn't mess up with the macros.

yoanlcq commented 1 year ago

Sorry for the delay, all of that looks great! I'm ok with merging this.

Before I do though, feel free to add yourself as a contributor in Cargo.toml and README.md; I'll wait for your final green light.

Patiga commented 1 year ago

I'm glad that you approve this! :)