idanarye / bevy-tnua

A floating character controller for Bevy
https://crates.io/crates/bevy-tnua
Apache License 2.0
180 stars 12 forks source link

Tilt correction wrong if `up != Vec3::Y` #40

Open unpairedbracket opened 5 months ago

unpairedbracket commented 5 months ago

I think there's an error here: the tilted_up vector should be calculated as the tracked rotation applied to the y-axis rather than to self.up:

https://github.com/idanarye/bevy-tnua/blob/5fdc7c284109880a6e29dfeacb6eee07ac5c184f/src/builtins/walk.rs#L282-L284

The rotation_required_to_fix_tilt line makes sense - the desired "equilibrium" position is that where tilted_up == self.up. However, computing tilted_up as tracker.rotation * self.up means this condition becomes self.up = tracker_rotation * self.up, so the resulting equilibrium character transform is a rotation around self.up (in world-space) rather than a rotation of the "character-space" up vector (the y-axis) onto the desired world-space up vector self.up.

This leads to some odd-looking effects in-game when rotating the desired_forward direction of a character with self.up set to the normal of an angled floor.

Changing line 282 to

let tilted_up = ctx.tracker.rotation.mul_vec3(Vec3::Y);

solved these issues for me in local testing.

Hopefully this makes sense and I'm not just misinterpreting the intended usage of the up vector or something. Happy to open a PR if you'd prefer but that felt a bit overkill for a +/- 7-character proposed change

idanarye commented 5 months ago

Originally up was accompanied by another field - forward - and together they decided the orientation - which directions are considered up and the forward. They were both part of the configuration struct - unlike desired_forward, which was part of the controls struct.

When I did the big refactor of version 0.10, I did many changes - two of them I think are relevant to the confusion:

So if previously it was clearer that up as a similar purpose to forward - now it looks like it has a similar purpose to desired_forward. I think what you need is a desired_up field - which Tnua does not currently have.

The current up is not "where the head should be facing" - it's "which direction is considered as 'up'". For example - that's the direction opposite to the gravity. I think (didn't actually test this) that if you change it it'll cause problem because the spring force will no longer be parallel to the gravity.

Maybe I really should replace up with a proper desired_up that has the correct semantics...

idanarye commented 5 months ago

Now that I think about it - maybe the up direction should be set from the gravity? Instead of actions getting it from the basis, I can add it to the context and both basis and action will get it from there.

As for the desired_up - currently the ray's cast direction is affected by the rotation, which is not that noticeable but a desired_up will make it noticeable. I assume that even if the character is tilted by desired_up we still want to cast the ray straight down?