-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP rcamera redesign vector #2563
Conversation
36215aa
to
5883b86
Compare
@Crydsch Thanks for the redesign, I like this approach more than previous one, less breaking changes and a code simplification. Adding the I had problems to switch between camera modes without keeping some of those globals but I think it should be possible to do so without them... I just was not able to do it in the past. I've been also doing some changes to As always, I prioritize code simplicity, in case someone in the future needs to review this API. |
@raysan5 I am Glad to hear you like this approach better :) The different camera modes will be interesting. Now that you mention FrameTime based movements... View bobbing is another topic. If you have some simplifications for this, The camera rotation functions are not perfect yet - I agree. |
131bc83
to
aaebbaa
Compare
Camera modesAll remaining camera modes have been added! Switching between the different modes is now supported! Further, I added a I noticed a small compilation issue with the As in the previous PR, you can see and test everything in action in the New Input systemIf you approve of the current state, I would like to continue with the To stay backwards compatible and keep the simple API with
Version 1 makes more sense I think, in terms of where the camera logic is expected. If used in another engine the function Also we then have to think about which keys to use in And lastly, what should we do with the funtions |
@Crydsch Amazing work! I need some time to review it properly and give you a better answer. About the functions |
aaebbaa
to
5d88068
Compare
@raysan5 Sorry for pushing so quickly. Please take your time for the review, I just wanted to keep working on other parts in the meantime. View BobbingWhile trying to design the new input system I noticed that the view bobbing Previously this was done with the The view bobbing is disabled if I kept the core behaviour as I am not to familiar with view bobbing. Adjusting all examplesThe previous change from Currently I adjusted the camera_first_person example to showcase all camera modes. Some minor fixes were/are necessary in:
New Input System ?This comment got quite long, but I felt like explaining my thought process. |
Please consider using delta time between frames for camera position calculation. For now camera speed depends on target FPS, which may be a problem if FPS is adjustable or not stable. |
@GoodNike @raysan5 |
@Crydsch Excuse me for the late response! I couldn't check it yet, I've been out for a week and it's a big PR, I'll answer it properly by tomorrow! :) |
Reviewed some camera functionality: - Reviewed camera swinging (up-down movement) - Reviewed camera tilting (left-right movement) - Make movement independent of frame-rate - removed unneeded variables NOTE: Camera rotation has some speed issues on first person when fixed 60 fps are used: it moves too fast. Independent framerate movement is not properly implemented.
@Crydsch Just pushed the latest camera changes I was working on:
It's been about a month but I didn't push them because there is still some issue on the camera rotation for the first-person view, it moves too fast when fixed to 60 fps, the framerate independant movement is not compretely right... in any case I pushed those changes in case they can be helpful for you as a reference. About all your changes: New signature for
|
@Crydsch I'm checking the code more carefully and I see no quaternions are used for camera rotations, it keeps using the same previous Euler maths but now through the raymath functions... |
@raysan5 The quaternion approach is fundamentally different and would have required a different camera struct (as per the first PR). But we can still improve the camera module with the low-level API, general API improvements, removing the static data, enabling the stand alone mode, ... |
Agree. |
I had a look at the change regarding
Should we continue with the change anyway? |
@Crydsch I see the issue with the Which low-level functions depend on the |
The low-level functions requiring
As you can see, there are some dependencies on |
@Crydsch I think those functions shouldn't depend on For example: void CameraMoveForward(Camera3D *camera, float distance, Vector3 axisLock);
void CameraPitch(Camera3D *camera, float angle, bool lockView); |
@raysan5 That might be a possible fix. I'll have a look into those functions. With that change we can then remove That would also mean that |
1a9e83f
to
866d474
Compare
Hey @raysan5 Sorry it has been a while since I worked on this. The semester ended and I was on vacation. The low-level functions now have additional parameters and no longer rely on While adjusting the examples to use the new Looking at your review again, you mentioned removing the Also I rebased onto master again. PS: I found a new example missing from the VS2022 project ;) |
@Crydsch Excuse me for this super-late response, it's a big change and it requires time to review but I can't find the moment to jump into it... Taking a quick look, it seems and amazing improvement! It does not only simplify the API a lot but also opens a door for future improvements. I'll try to review it more carefully as soon as possible. Thank you very much for all the work put into it! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the general direction of the changes. I have some issues with a few of the names. There is also some inconsistent newline useage.
I am also not a fan of having the view bob data burned into the camera structure itself and the speed data hardcoded. I wonder if there may be benefit in having a camera state structure that keeps track of all the internal state data that the update method needs, like the bob, any mouse positions, and speeds, and have the user pass that in if they are using the built in camera update function.
Hello, just reading through and finding this PR interesting because I've found the default camera behavior to be wanting for many of the reasons this PR exists. I would just like to throw in my 2 cents that the entire view bobbing feature should just be removed or at least explicitly be an opt-in option for the camera. I would never expect in any engine or toolkit to switch to a default first person camera and get a view bobbing as part of that, especially a view bob that isn't readily configurable or quick to disable. |
Just to leave a note here: In the previous predefined cameras, the baked-in mouse buttons assigned to the different motions made it impossible e.g. to pan on macbooks with a touchpad. the new implementation should make sure to 1. choose compatible defaults 2. make buttons and modifier keys configurable in the predefined cameras. |
Merged new camera redesign, reviewing it carefully in following commits... |
- update UpdateCamera function and remove SetCameraMode - SetCameraMode was removed in raysan5/raylib#2563
rcamera re-design
This PR addresses the
rcamera.h
redesign as discussed in #2507.This PR is very similar to #2542.
There we evaluated a quaternion-based design, but noticed a breaking change in
the camera struct. The new design was also more confusing to new users.
As raylib prioritizes simplicity for less-experienced users this was problematic.
This PR aims to keep the camera struct as close as possible to the old one
and not introduce breaking changes.
It therefore also keeps the more intuitive vector based implementation (
position, target, up
).The discussed improvements can still be applied though!
Such as the low-level functions for easy custom camera control.
Note: Some changes may still be necessary to improve the camera module and implement the set requirements.
Key-Design notes
All static variables will be removed and the camera struct contains the entire camera state.
It can be directly manipulated at any time and the camera will function accordingly.
This gives users full control over the camera and allows customized modes.
All functions are available in all modes, there are no restrictions.
It may make no sense to use them though - this is at the users discretion.
I did this deliberately to allow for a maximum of freedom for the user.
For example, while at first glance a
CameraRoll(..)
makes no sense in theCAMERA_FIRST_PERSON
mode,it may be used to implement a "looking around the corner" feature.
The functions
CameraYaw(..)
,CameraPitch(..)
andCameraRoll(..)
take an angle parameter (in radians) which allow precise manipulation.A
CameraYaw(camera, 90 * DEG2RAD);
call results precisely in a 90 degree turn.Behavioural differences to the old rcamera.h
API and usage
API
Usage
TODO
Feedback
I'd much appreciate feedback!
If you want to have a look at the code or the demo, you can checkout my feature branch rcamera_redesign_vector or this PR..
The
core_3d_camera_first_person
example is functional and can be used for testing.Note: The camera struct was extended by the camera
mode
andswingCounter
(for view bobbing).This should not be a breaking change, as the struct is initialized by
Camera camera = { 0 };
and the camera mode is set with
SetCameraMode(..);
, both which always produce a valid state.Edit: Updated API and usage to reflect current iteration (for quick overview)