From f6cd6a487459b35e7a0ac9a2ccdceacfabf40272 Mon Sep 17 00:00:00 2001
From: Joona Aalto <jondolf.dev@gmail.com>
Date: Tue, 8 Oct 2024 19:45:03 +0300
Subject: [PATCH] Use `Dir2`/`Dir3` instead of `Vec2`/`Vec3` for
 `Ray2d::new`/`Ray3d::new` (#15735)

# Objective

The `new` constructors for our ray types currently take a `Vec2`/`Vec3`
instead of a `Dir2`/`Dir3`. This is confusing and footgunny for several
reasons.

- Which one of these is the direction? You can't see it from the type.

```rust
let ray = Ray2d::new(Vec2::X, Vec2::X);
```

- Many engines allow unnormalized rays, and this can affect ray cast
results by scaling the time of impact. However, in Bevy, rays are
*always* normalized despite what the input argument in this case
implies, and ray cast results are *not* scaled.

```rust
// The true ray direction is still normalized, unlike what you'd expect.
let ray = Ray2d::new(Vec2::X, Vec2::new(5.0, 0.0, 0.0)));
```

These cases are what the direction types are intended for, and we should
use them as such.

## Solution

Use `Dir2`/`Dir3` in the constructors.

```rust
let ray = Ray2d::new(Vec2::X, Dir2::X);
```

We *could* also use `impl TryInto<DirN>`, which would allow both vectors
and direction types, and then panic if the input is not normalized. This
could be fine for ergonomics in some cases, but especially for rays, I
think it's better to take an explicit direction type here.

---

## Migration Guide

`Ray2d::new` and `Ray3d::new` now take a `Dir2` and `Dir3` instead of
`Vec2` and `Vec3` respectively for the ray direction.
---
 crates/bevy_math/src/ray.rs | 26 ++++++--------------------
 1 file changed, 6 insertions(+), 20 deletions(-)

diff --git a/crates/bevy_math/src/ray.rs b/crates/bevy_math/src/ray.rs
index df490a506cf49..e083d7b4a1b3d 100644
--- a/crates/bevy_math/src/ray.rs
+++ b/crates/bevy_math/src/ray.rs
@@ -25,16 +25,9 @@ pub struct Ray2d {
 
 impl Ray2d {
     /// Create a new `Ray2d` from a given origin and direction
-    ///
-    /// # Panics
-    ///
-    /// Panics if the given `direction` is zero (or very close to zero), or non-finite.
     #[inline]
-    pub fn new(origin: Vec2, direction: Vec2) -> Self {
-        Self {
-            origin,
-            direction: Dir2::new(direction).expect("ray direction must be nonzero and finite"),
-        }
+    pub const fn new(origin: Vec2, direction: Dir2) -> Self {
+        Self { origin, direction }
     }
 
     /// Get a point at a given distance along the ray
@@ -74,16 +67,9 @@ pub struct Ray3d {
 
 impl Ray3d {
     /// Create a new `Ray3d` from a given origin and direction
-    ///
-    /// # Panics
-    ///
-    /// Panics if the given `direction` is zero (or very close to zero), or non-finite.
     #[inline]
-    pub fn new(origin: Vec3, direction: Vec3) -> Self {
-        Self {
-            origin,
-            direction: Dir3::new(direction).expect("ray direction must be nonzero and finite"),
-        }
+    pub const fn new(origin: Vec3, direction: Dir3) -> Self {
+        Self { origin, direction }
     }
 
     /// Get a point at a given distance along the ray
@@ -112,7 +98,7 @@ mod tests {
 
     #[test]
     fn intersect_plane_2d() {
-        let ray = Ray2d::new(Vec2::ZERO, Vec2::Y);
+        let ray = Ray2d::new(Vec2::ZERO, Dir2::Y);
 
         // Orthogonal, and test that an inverse plane_normal has the same result
         assert_eq!(
@@ -152,7 +138,7 @@ mod tests {
 
     #[test]
     fn intersect_plane_3d() {
-        let ray = Ray3d::new(Vec3::ZERO, Vec3::Z);
+        let ray = Ray3d::new(Vec3::ZERO, Dir3::Z);
 
         // Orthogonal, and test that an inverse plane_normal has the same result
         assert_eq!(