Skip to content
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

Fix 'create_snap' desync #686

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix 'create_snap' desync #686

wants to merge 1 commit into from

Conversation

squidt
Copy link

@squidt squidt commented Oct 3, 2024

If the player drops then picks up a 'SnapZone' with a 'PickableObject' in it the 'PickableObject' will lag behind during Player or SnapZone movement due to calling 'create_snap'. Adding the driver as a direct child of the grabber prevents any desync. 'create_lerp' function is similar but has been left out until further testing.

Tested on Godot 4.3.stable

Before, while moving (the small sphere on the green rail is where the SnapZone is):
image
After, while moving:
image

If the player drops then picks up a 'SnapZone' with a 'PickableObject' in it the 'PickableObject' will lag behind during player or SnapZone movement due to calling 'create_snap'. Adding the driver as a direct child of the grabber prevents any desync. 'create_lerp' function is similar but has been left out until further testing.

Tested on Godot 4.3.stable
@BastiaanOlij BastiaanOlij added the bug Something isn't working label Oct 4, 2024
@BastiaanOlij BastiaanOlij added this to the 4.4.0 milestone Oct 21, 2024
@BastiaanOlij BastiaanOlij added the needstesting Needs testing label Oct 21, 2024
Copy link
Member

@BastiaanOlij BastiaanOlij left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would like to get some feedback from others to make sure this doesn't have any unexpected side-effects. Other than that, I think this is safe to merge.

Copy link
Contributor

@DigitalN8m4r3 DigitalN8m4r3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

finally got around, lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needstesting Needs testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants