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 interpolation of kinematic character controllers #463

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

Conversation

liquidev
Copy link

Instead of trying to detect changes in the Transform, rely on the user telling us when to teleport instead of interpolating positions. For that there is a new TransformInterpolation::teleport function, which resets the interpolation data manually.

Closes #341. Maybe also closes #379?

Instead of trying to detect changes in the Transform, rely on the user telling us when to teleport instead of interpolating positions.
For that there is a new TransformInterpolation::teleport function, which resets the interpolation data manually.
Comment on lines -252 to +253
interpolation.start = Some(*body.position());
interpolation.end = None;
interpolation.start = interpolation.end;
interpolation.end = Some(*body.position());
Copy link

Choose a reason for hiding this comment

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

From what I understanded of bevy_rapier's code this change is not valid, because this system execute whole Rapier's subupdates (I stick to name convention from #464), and this code block is executed only for last subupdate. This stores a value from previous subupdate, and next sets final subupdate position as end in lerp system:

if interpolation.end.is_none() {
interpolation.end = Some(*rb.position());
}

Maybe in case where you have only a one subupdate a code what are you change is invalid, but I must think about this. But otherwise in my opinion code which is writed earlier is correct.

Copy link

Choose a reason for hiding this comment

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

After implementation expected behavior from #464 I do not know how lerp should looks. When delta time of FixedUpdate is greater than delta time of Update @liquidev's lerp works good, but in situation when it is lower earlier lerp works good. For equals I do not know this at this time. Someone should investigate this.

@Vrixyz Vrixyz added D-Difficult Needs strong technical background, domain knowledge, or impacts are high, needs testing... P-Medium A-Integration very bevy specific bug Something isn't working labels May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Integration very bevy specific bug Something isn't working D-Difficult Needs strong technical background, domain knowledge, or impacts are high, needs testing... P-Medium
Projects
None yet
3 participants