From 4de67b5cdb2aee4e6961c75db0f3a0d147585421 Mon Sep 17 00:00:00 2001
From: Jan Hohenheim <jan@hohenheim.ch>
Date: Tue, 10 Sep 2024 05:53:32 +0200
Subject: [PATCH] Improve first person camera in example (#15109)

# Objective

- I've seen quite a few people on discord copy-paste the camera code of
the first-person example and then run into problems with the pitch.
- ~~Additionally, the code is framerate-dependent.~~ it's not, see
comment in PR

## Solution

- Make the code good enough to be copy-pasteable
- ~~Use `dt` to make the code framerate-independent~~ Add comment
explaining why we don't use `dt`
	- Clamp the pitch
- Move the camera sensitivity into a component for better
configurability

## Testing

Didn't run the example again, but the code is straight from another
project I have open, so I'm not worried.

---------

Co-authored-by: Antony <antony.m.3012@gmail.com>
---
 examples/camera/first_person_view_model.rs | 56 ++++++++++++++++++----
 1 file changed, 48 insertions(+), 8 deletions(-)

diff --git a/examples/camera/first_person_view_model.rs b/examples/camera/first_person_view_model.rs
index 9a91eb324b600..dcc3df459aa53 100644
--- a/examples/camera/first_person_view_model.rs
+++ b/examples/camera/first_person_view_model.rs
@@ -42,6 +42,8 @@
 //! | arrow up             | Decrease FOV  |
 //! | arrow down           | Increase FOV  |
 
+use std::f32::consts::FRAC_PI_2;
+
 use bevy::color::palettes::tailwind;
 use bevy::input::mouse::AccumulatedMouseMotion;
 use bevy::pbr::NotShadowCaster;
@@ -67,6 +69,22 @@ fn main() {
 #[derive(Debug, Component)]
 struct Player;
 
+#[derive(Debug, Component, Deref, DerefMut)]
+struct CameraSensitivity(Vec2);
+
+impl Default for CameraSensitivity {
+    fn default() -> Self {
+        Self(
+            // These factors are just arbitrary mouse sensitivity values.
+            // It's often nicer to have a faster horizontal sensitivity than vertical.
+            // We use a component for them so that we can make them user-configurable at runtime
+            // for accessibility reasons.
+            // It also allows you to inspect them in an editor if you `Reflect` the component.
+            Vec2::new(0.003, 0.002),
+        )
+    }
+}
+
 #[derive(Debug, Component)]
 struct WorldModelCamera;
 
@@ -90,6 +108,7 @@ fn spawn_view_model(
     commands
         .spawn((
             Player,
+            CameraSensitivity::default(),
             SpatialBundle {
                 transform: Transform::from_xyz(0.0, 1.0, 0.0),
                 ..default()
@@ -220,17 +239,36 @@ fn spawn_text(mut commands: Commands) {
 
 fn move_player(
     accumulated_mouse_motion: Res<AccumulatedMouseMotion>,
-    mut player: Query<&mut Transform, With<Player>>,
+    mut player: Query<(&mut Transform, &CameraSensitivity), With<Player>>,
 ) {
-    let mut transform = player.single_mut();
+    let Ok((mut transform, camera_sensitivity)) = player.get_single_mut() else {
+        return;
+    };
     let delta = accumulated_mouse_motion.delta;
 
     if delta != Vec2::ZERO {
-        let yaw = -delta.x * 0.003;
-        let pitch = -delta.y * 0.002;
-        // Order of rotations is important, see <https://gamedev.stackexchange.com/a/136175/103059>
-        transform.rotate_y(yaw);
-        transform.rotate_local_x(pitch);
+        // Note that we are not multiplying by delta_time here.
+        // The reason is that for mouse movement, we already get the full movement that happened since the last frame.
+        // This means that if we multiply by delta_time, we will get a smaller rotation than intended by the user.
+        // This situation is reversed when reading e.g. analog input from a gamepad however, where the same rules
+        // as for keyboard input apply. Such an input should be multiplied by delta_time to get the intended rotation
+        // independent of the framerate.
+        let delta_yaw = -delta.x * camera_sensitivity.x;
+        let delta_pitch = -delta.y * camera_sensitivity.y;
+
+        let (yaw, pitch, roll) = transform.rotation.to_euler(EulerRot::YXZ);
+        let yaw = yaw + delta_yaw;
+
+        // If the pitch was ±¹⁄₂ π, the camera would look straight up or down.
+        // When the user wants to move the camera back to the horizon, which way should the camera face?
+        // The camera has no way of knowing what direction was "forward" before landing in that extreme position,
+        // so the direction picked will for all intents and purposes be arbitrary.
+        // Another issue is that for mathematical reasons, the yaw will effectively be flipped when the pitch is at the extremes.
+        // To not run into these issues, we clamp the pitch to a safe range.
+        const PITCH_LIMIT: f32 = FRAC_PI_2 - 0.01;
+        let pitch = (pitch + delta_pitch).clamp(-PITCH_LIMIT, PITCH_LIMIT);
+
+        transform.rotation = Quat::from_euler(EulerRot::YXZ, yaw, pitch, roll);
     }
 }
 
@@ -238,7 +276,9 @@ fn change_fov(
     input: Res<ButtonInput<KeyCode>>,
     mut world_model_projection: Query<&mut Projection, With<WorldModelCamera>>,
 ) {
-    let mut projection = world_model_projection.single_mut();
+    let Ok(mut projection) = world_model_projection.get_single_mut() else {
+        return;
+    };
     let Projection::Perspective(ref mut perspective) = projection.as_mut() else {
         unreachable!(
             "The `Projection` component was explicitly built with `Projection::Perspective`"