Improve first person camera in example#15109
Improve first person camera in example#15109alice-i-cecile merged 18 commits intobevyengine:mainfrom janhohenheim:patch-1
Conversation
|
Multiplying mouse motion by delta time actually has the opposite effect. The mouse motion already matches the amount you moved it during the frame, so multipling it by delta time makes your mouse movement FPS-dependant, with a higher FPS meaning slower movement. However, if the input was a gamepad stick it would need to be multiplied by delta, which might be worth including in the example for that reason. Ideally we would of course upstream LWIM and have it handle those differences instead. I do like the limit for the angle, tho the comment could be better. Not only is rotating too far not particularly useful, it also makes the other axis function like it's reversed. |
| let mut transform = player.single_mut(); | ||
| let delta = accumulated_mouse_motion.delta; | ||
| let dt = time.delta_seconds(); | ||
| // The factors are just arbitrary mouse sensitivity values. |
There was a problem hiding this comment.
Pull these out into constants please :)
There was a problem hiding this comment.
I disagree. Pulling them into MOUSE_SENSITIVITY_X and MOUSE_SENSITIVITY_Y
- makes them less nicely grouped than now, and
- is semantically wrong.
PIis a constant, an aspect ratio is a constant. Mouse sensitivity is a value that should be configurable for accessibility.
As a compromise, I could extract them into a component if you want. Or add a component mentioning that this should be configurable.
There was a problem hiding this comment.
Component is nice: it makes it easy for the values to be twiddled via inspectors.
|
@richchurcher, if you're interested in helping out by reviewing, I'd welcome your opinions here :) Bevy uses community reviews to vet when things are ready to merge, so please leave an approval when you think this work is ready. |
|
@NiseVoid thanks for pointing out that |
alice-i-cecile
left a comment
There was a problem hiding this comment.
Sticking data on the Player component like this feels messy, but it does keep the example simpler. I'd prefer a dedicated CameraSensitivity component.
|
@alice-i-cecile yeah I was considering that as well and used the |
Co-authored-by: Antony <antony.m.3012@gmail.com>
| // 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. |
There was a problem hiding this comment.
| // To not run into these issues, we clamp the pitch to a safe range. | |
| // To not run into these issues, we clamp the pitch to a safe range. (FRAC_PI_2 is just a constant representing half the value of π). |
Feel free to ignore this, but my first glimpse of this value gave me a bit of a WTF moment 😆
There was a problem hiding this comment.
Good point :) I changed the wording a bit: 2bc51d1 (#15109)
Objective
Additionally, the code is framerate-dependent.it's not, see comment in PRSolution
UseAdd comment explaining why we don't usedtto make the code framerate-independentdtTesting
Didn't run the example again, but the code is straight from another project I have open, so I'm not worried.