fu5ha / ultraviolet

A wide linear algebra crate for games and graphics.
https://crates.io/crates/ultraviolet
750 stars 81 forks source link

intersect_ray/intersect_plane returns an Option for possibly no reason? #59

Closed Veykril closed 4 years ago

Veykril commented 4 years ago

The family of functions intersect_ray/line/plane on Plane/Ray all return an Option that is actually always Some while the comment(that is not present on Ray::intersect_plane) states that the callee has to check prior to calling the given function whether the ray actually intersects the plane or not. The return type in general made me at first assume that the operation is actually checking whether the intersection occured or not. Was the Option forgotten to be removed or was it planned to actually be a checked operation at some point?

The code in question: https://github.com/termhn/ultraviolet/blob/0dfc114deda9e8ed94c964de027bd74a206cd672/src/geometry.rs#L177-L182

fu5ha commented 4 years ago

Yeah I think this is an oversight of something that was originally meant to get checked... I'm actually thinking I'm going to rip out the geometry module from ultraviolet proper for the next release and have it as its own addon crate, i.e. ultraviolet_geometry or something as it's a little bit of an afterthought at the moment.

fu5ha commented 4 years ago

oh also cc @jaynus

Veykril commented 4 years ago

Seperating the geometry part from this crate seems like a good call in general since not everyone needs it either I imagine.

fu5ha commented 4 years ago

Indeed! Closing this here; thanks for the report :)