Make KinematicCharacterController move along obstacles, respect offset more and fix false positives in grounded detection

[janhohenheim]

• Fixes #426
• Fixes https://github.com/dimforge/bevy_rapier/issues/301 modulo the documentation aspect
• Touched #418 and fixed it, but reverted because of jittering issues, see discussion.
• Fixes snapping to ground not triggering sometimes as described in https://github.com/dimforge/bevy_rapier/issues/301

• Fixes horizontal offset being mostly ignored and some instances of vertical offset being ignored

  PR contents:

• Move stairs check before slope check to deal with some stopping along stairs
• Move along surface after stairs and slope checks
• Clean up some code, remove duplications
• Take offset into account in some places it was left out
• Fix ground being erroneously detected when jumping next to a wall composed of multiple triangles in 3D.

[janhohenheim]

Don't merge this quite yet, there's a tiny bug with the offset / snapping to ground. Update: Done

[janhohenheim]

Update: done done now :)

[janhohenheim]

@sebcrozet PR should pass checks now

[janhohenheim]

Thanks for the review, I'll fix the issues and ping you once I'm done :) Nearly all changes in the stairs code are early returns to remove indentation and mental overhead.

[janhohenheim]

The revert c9462e0 (#446) fixes the jittering, but causes the character to violate the offset on the platform:

This is not a regression since the main branch suffers from the same issue (see https://github.com/dimforge/rapier/issues/418). I guess this might be a conceptual problem: the platform might try to fill the gap left by the offset, thus moving the character up. The character then tries to move down again, causing the jitter. This is just a guess, but if this is really the cause, I'm really unsure how one could fix this in the future.

There is some jittering left when near the center of the platform, but that was also present already.

As for the autostopping: I think one of the reverts fixed it because I cannot reproduce it.

[janhohenheim]

@sebcrozet I addressed your concerns, ready for another review 🚀

[morbidbark]

I just checked out your branch to see if it would fix some issues I was having with the KinematicCharacterController(offset not being respected and getting stuck to walls). It did fix those problems, but introduced a few others. Lot's of jitter and very odd interactions with moving platforms.

[janhohenheim]

@morbidbark that's a bummer. I did see some jitter as well, but it's hard to tell how much of that was already present and not noticeable because before the character didn't move at all in many of these situations. I tried looking further into it, but I'm at a loss here.

[morbidbark]

So I played around with it a little more today and found that when standing still on solid ground my CharacterController is constantly switching between the grounded and falling state. However, when I ran the character_controller2 example and monitored the grounded attribute on EffectiveCharacterMovement it appeared to be stable. It looks like my problems are probably just user error.

[morbidbark]

I was able to verify that my jittering problems were due to user error. I had frames where my "gravity" was not acting on my controller causing it to appear ungrounded. The issues I was having with the moving platforms is still there, but this also happens on the latest release so not a problem with this PR. Sorry for any confusion.

[janhohenheim]

Glad to hear 😄

[sebcrozet]

Hey @janhohenheim! Sorry for taking so long for getting back to this PR, I’ve been on vacation. Thank you for your latest changes.

Taking a closer look at handle_stairs reminded me of why it was run after handle_slopes. If stairs are handled first, climbing slopes (the ones with an angle smaller than the max_slope_climb_angle) will actually be handled partially (or completely if moving at low speed) by the stair handling code. Which I guess is generally fine, just something to keep in mind.

Now, the new slope handling code does the following:

        if climbing {
            // Are we allowed to climb?
            (angle_with_floor <= self.max_slope_climb_angle).then_some(horizontal_translation)
        }

the comparison is incorrect here as it reads "if the floor is angles by less than max_slope_climb_angle, prevent climbing the slope", which is the opposite of what max_slope_climb_angle means. This has two consequences:

• When auto-step is enabled, climbing will always be false at low speed when angle_with_floor <= self.max_slope_climb_angle because that climbing has already been dealt with by the stair handling. The reason why the user cannot climb steep slopes in this PR is only because of gravity: if there isn’t enough movement uphill to overcome gravity, the player won’t be able to go up.
• When auto-step is disabled, then the player will no longer be able to climb small slopes because that test is reversed.

What if we fixed that check with a >= instead of a <=? Well, in that case, sliding against walls no longer works because it’s prevented by this threshold.

I think we can have the >= case return vertical_translation, and None otherwise. This will prevent strong horizontal motion push through the slope, letting the user climb due to the indirect vertical movement that emanate from this horizontal motion minus the diagonal contact normal.

In addition, having handle_slopes return Some(slope_translation) is equivalent to returning None because of the None case results in a subtract_hit equivalent to the ones applied to get slope_translation. This lets us simplify the code slightly.

I don’t want to delay this PR further so I’ll make a commit for these observations and merge the PR. We can still improve it further later if these suggestions still have issues.

[sebcrozet]

Fantastic work @janhohenheim, thanks again!

[janhohenheim]

@sebcrozet no worries, hope you enjoyed your vacation :) Thanks for bringing these points up, I'll see what I can do about them next week