search

CesiumGS / cesium-unreal

Bringing the 3D geospatial ecosystem to Unreal Engine
https://cesium.com/platform/cesium-for-unreal/
Apache License 2.0
885 stars 286 forks source link

Add DynamicPawn deceleration #1390

Closed csciguy8 closed 1 month ago

csciguy8 commented 3 months ago

This PR changes the dynamic camera to smoothly decelerate and come to a stop once you've stopped giving player input. This is a nicety that lets the camera feel like it has some mass as it moves around the world.

Start any level and navigate with the keyboard and mouse and you'll immediately notice it.

Feedback welcome. Getting the feel just right may take some iterations.

Blueprint changes...

Set deceleration based on speed image

Simplified use of Speed Line Trace node This block previously only allowed the pawn's speed to increase except in certain cases. I removed this limitation because it was causing a bug with deceleration when flying straight towards the ground. It would seem to fall and slide much faster than expected. In this case we need the speed to be updated continuously. I'm not exactly sure why it was needed. To smooth speed changes when flying over buildings maybe? image

Simplified internal implementation of Speed Line Trace Related to before, this node previously used "Dynamic Speed Min Height" to determine whether to override height. Not seeing why we need this anymore. We always use height as speed, and now we always update speed. Removing the related logic simplified the internals quite a bit. image

Added internal speed scaling factor to tweak default feel I always thought the default speed felt a little sluggish. This param lets us tweak it independent of user speed scaling image

csciguy8 commented 2 months ago

@j9liu Hold on before reviewing this, need to merge.

csciguy8 commented 2 months ago

Ok @j9liu , this is ready for a look now.

Adding deceleration sounds easy enough, but it led to more changes than I anticipated. I've screen captured all the changes in the description, with explanations.

Open to any thoughts on the changes. It's hard to know the history of some of the logic that exists.

azrogers commented 1 month ago

Looks good to me! Made a few small formatting changes in the blueprint. This shouldn't have any effect on the build but I'll let the checks complete before I merge this one.

csciguy8 commented 1 month ago

Hey @azrogers , thanks for the review.

I noticed that you made some misc formatting changes, and now I'm no longer able to open the dynamic pawn blueprint in UE 5.2.

Did you happen to save this asset in a later version? (because we need to keep support for older versions, all the way to UE 5.2.)

kring commented 1 month ago

I reverted the "minor formatting changes" commit in b04a3892 to restore UE 5.2 compatibility. @azrogers please open a new PR with these changes, made with the correct UE version.

j9liu commented 1 month ago

I'm really late to this, but I'm not a fan of the deceleration at much higher speeds. I'm not opposed to deceleration entirely, but I think it needs to scale better with the velocity of the camera. When you're flying around at high speeds, it takes a long time to slow down so you can really overshoot your target.

It's really hard to capture this in a gif, but basically I'm just flying at around this height above the earth.

image

I can increase my speed with the scroll wheel, and when I try to stop I sometimes end up bouncing off the terrain because the camera hit it, because it was too fast. In fact... when I'm not accidentally headbutting the Earth, I end up underground again 🙃

too fast

@csciguy8 I hate to say this, but I would really like to revert this change and test it further before we release it. The DynamicPawn is our sole example of movement across the globe, and I don't think it reflects well if a user can't traverse the globe faster without ending up underground, or bouncing off of the Earth. Please let me know ASAP if that's fine with you. Definitely feel free to remake this PR for future discussion / testing but I just don't agree with shipping the plugin with the camera as it exists now.

csciguy8 commented 1 month ago

@csciguy8 I hate to say this, but I would really like to revert this change and test it further before we release it.

No worries there. I think getting "feel" right is an iterative process anyway. Even if this PR got released, there's probably going to be more PRs to tweak movement in the future.

I'll remake this PR, and we can try again with the "ending up underground" issues in mind.