search

Rust-SDL2 / rust-sdl2

SDL2 bindings for Rust
MIT License
2.65k stars 465 forks source link

Support converting arrays of integers to Rect via TryFrom #1403

Open lolbinarycat opened 3 weeks ago

Cobrand commented 3 weeks ago

Something tells me this will fail fmt

antonilol commented 3 weeks ago

You should put the doc comments on try_from, not on the impl block (like the standard lib), this way compiler frontends like rust-analyzer can show the doc on hover.

Accepting all types that are TryInto<i32> (and the same for u32) will give inconsistent behavior on overflowing i32::MAX/2, because when it is out of bounds for i32, it will error, but when it is only out of bounds for the rect, it will clamp (existing behavior).

Also, using an array does not make sense to me because a Rect is not some sort of list, each value has a distinct meaning (namely x, y, width and height).

lolbinarycat commented 3 weeks ago

Accepting all types that are TryInto (and the same for u32) will give inconsistent behavior on overflowing i32::MAX/2, because when it is out of bounds for i32, it will error, but when it is only out of bounds for the rect, it will clamp (existing behavior).

do you have a better idea? i don't think rust provides a generic way to do a clamping type conversion.

Also, using an array does not make sense to me because a Rect is not some sort of list, each value has a distinct meaning (namely x, y, width and height).

yet it still supports casts from tuples, which are basically just heterogeneous lists.

antonilol commented 3 weeks ago

do you have a better idea? i don't think rust provides a generic way to do a clamping type conversion.

The standard library does not provide this, but you can make a custom trait to do saturating conversion to i32 and u32. (To me this does seem overkill for one conversion function.)

yet it still supports casts from tuples, which are basically just heterogeneous lists.

A tuple gets closer to Rect than an array (type per field), but fields are still unnamed.

Cobrand commented 2 weeks ago

I do agree that tuples are better, but maybe there is a specific usecase like matrix computing or something. @lolbinarycat why do you need this specifically?