Simplify push_box's camera height logic #3
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Problem: push_box.gd:_ready calls super() before setting the camera's position. super() modifies the camera's position by adding the dist_above_target offset, but setting the camera's position after makes this redundant. To fix this and apply the correct offset, the camera_controller_base class' _process function sets the camera's height to target.location.y + distance_above_target. However, this does not need to run in the _process function because neither the height of the target nor the distance_above_target change over time. To clean this up, do not override camera_controller_base:_ready in push_box.gd, make camera_controller_base:_ready set the position to the target's position + the vertical offset, and do not update the camera's vertical (y) position in the _process function. Then, the camera's height gets set correctly and only once (in push_box's inherited _ready function).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem: push_box's _ready() function calls super() before it sets the camera's position. However, setting the camera's position overwrites the height added to the camera in super(), making the call to super() useless.
This is currently solved by offsetting the camera's height to height_above_target + target.y in each call to _process().
Instead of this redundant logic, we can set the camera's height to height_above_target above the target in _ready() just once, remove the height logic from _process() entirely, and remove push_box's _ready() function entirely.
This makes the example camera implementation for the project slightly cleaner overall.