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.

Sign up for GitHub

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
Open

Fix interpolation of kinematic character controllers #463

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/dynamics/rigid_body.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,16 @@ pub struct TransformInterpolation {
}

impl TransformInterpolation {
/// Skips interpolation, effectively causing the body to immediately teleport to its current
/// transform.
///
/// This is most useful with kinematic character controllers, which always interpolate and do
/// not perform transform change detection like rigid bodies do.
pub fn teleport(&mut self) {
self.start = None;
self.end = None;
}

/// Interpolates between the start and end positions with `t` in the range `[0..1]`.
pub fn lerp_slerp(&self, t: f32) -> Option<Isometry<f32>> {
if let (Some(start), Some(end)) = (self.start, self.end) {
Expand Down
4 changes: 2 additions & 2 deletions src/plugin/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,8 +249,8 @@ impl RapierContext {
// Update the previous state transforms
for (handle, mut interpolation) in interpolation_query.iter_mut() {
if let Some(body) = self.bodies.get(handle.0) {
interpolation.start = Some(*body.position());
interpolation.end = None;
interpolation.start = interpolation.end;
interpolation.end = Some(*body.position());
Comment on lines -252 to +253
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:

bevy_rapier/src/plugin/systems.rs

Lines 554 to 556 in c6bcce4

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.

}
}
}
Expand Down
37 changes: 22 additions & 15 deletions src/plugin/systems.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,7 @@ pub fn apply_rigid_body_user_changes(
&RapierRigidBodyHandle,
&GlobalTransform,
Option<&mut TransformInterpolation>,
Option<&KinematicCharacterController>,
),
Changed<GlobalTransform>,
>,
Expand Down Expand Up @@ -354,24 +355,30 @@ pub fn apply_rigid_body_user_changes(
}
};

for (handle, global_transform, mut interpolation) in changed_transforms.iter_mut() {
for (handle, global_transform, mut interpolation, kinematic_character_controller) in
changed_transforms.iter_mut()
{
// Use an Option<bool> to avoid running the check twice.
let mut transform_changed = None;

if let Some(interpolation) = interpolation.as_deref_mut() {
transform_changed = transform_changed.or_else(|| {
Some(transform_changed_fn(
&handle.0,
global_transform,
&context.last_body_transform_set,
))
});

if transform_changed == Some(true) {
// Reset the interpolation so we don’t overwrite
// the user’s input.
interpolation.start = None;
interpolation.end = None;
// Kinematic character controllers update outside of our tick loop, and therefore we must
// exclude them from interpolation.
if kinematic_character_controller.is_none() {
if let Some(interpolation) = interpolation.as_deref_mut() {
transform_changed = transform_changed.or_else(|| {
Some(transform_changed_fn(
&handle.0,
global_transform,
&context.last_body_transform_set,
))
});

if transform_changed == Some(true) {
// Reset the interpolation so we don’t overwrite
// the user’s input.
interpolation.start = None;
interpolation.end = None;
}
}
}

Expand Down