idanarye / bevy-tnua

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

Character is pushed slightly when moving platform changes direction #28

Closed rparrett closed 1 year ago

rparrett commented 1 year ago

Tested main (e251483)

I think that maybe the previous fix for this was only a partial fix, because I recall it still being an issue. I think I was too deep in the game-jam to report it at the time.

This also seems to be behaving like a system ordering issue to me -- on some runs the character has this behavior and on other runs it seems fine (or maybe not as bad)

Here's a recording and a patch for the scene:

https://github.com/idanarye/bevy-tnua/assets/200550/4b151174-2f0c-45fa-96e2-54f219aa5ddd

diff --git a/examples/platformer_3d.rs b/examples/platformer_3d.rs
index 97dad5a3..84e011e2 100644
--- a/examples/platformer_3d.rs
+++ b/examples/platformer_3d.rs
@@ -2,6 +2,7 @@ mod common;

 use bevy::gltf::Gltf;
 use bevy::prelude::*;
+use bevy::render::camera::ScalingMode;
 use bevy::utils::HashMap;
 use bevy_egui::EguiContexts;
 use bevy_rapier3d::prelude::*;
@@ -20,7 +21,8 @@ use self::common::{FallingThroughControlScheme, MovingPlatform};

 fn main() {
     let mut app = App::new();
-    app.add_plugins(DefaultPlugins);
+    app.insert_resource(ClearColor(Color::rgb(0.8, 0.8, 0.8)))
+        .add_plugins(DefaultPlugins);
     app.add_plugins(RapierPhysicsPlugin::<NoUserData>::default());
     app.add_plugins(TnuaRapier3dPlugin);
     app.add_plugins(TnuaPlatformerPlugin);
@@ -60,8 +62,14 @@ fn update_plot_data(mut query: Query<(&mut PlotSource, &Transform, &Velocity)>)

 fn setup_camera(mut commands: Commands) {
     commands.spawn(Camera3dBundle {
-        transform: Transform::from_xyz(0.0, 16.0, 40.0)
-            .looking_at(Vec3::new(0.0, 10.0, 0.0), Vec3::Y),
+        projection: OrthographicProjection {
+            scale: 3.0,
+            scaling_mode: ScalingMode::FixedVertical(2.0),
+            ..default()
+        }
+        .into(),
+        transform: Transform::from_xyz(0.0, 11.0, 20.0)
+            .looking_at(Vec3::new(0.0, 11.0, 0.0), Vec3::Y),
         ..Default::default()
     });

@@ -179,22 +187,15 @@ fn setup_level(
         cmd.insert(PbrBundle {
             mesh: meshes.add(Mesh::from(shape::Box::new(4.0, 1.0, 4.0))),
             material: materials.add(Color::BLUE.into()),
-            transform: Transform::from_xyz(-4.0, 6.0, 0.0),
+            transform: Transform::from_xyz(-1.0, 8.0, 0.0),
             ..Default::default()
         });
-        cmd.insert(Collider::cuboid(2.0, 0.5, 2.0));
+        cmd.insert(Collider::cuboid(3.0, 0.5, 2.0));
         cmd.insert(Velocity::default());
         cmd.insert(RigidBody::KinematicVelocityBased);
         cmd.insert(MovingPlatform::new(
-            4.0,
-            &[
-                Vec3::new(-4.0, 6.0, 0.0),
-                Vec3::new(-8.0, 6.0, 0.0),
-                Vec3::new(-8.0, 10.0, 0.0),
-                Vec3::new(-8.0, 10.0, -4.0),
-                Vec3::new(-4.0, 10.0, -4.0),
-                Vec3::new(-4.0, 10.0, 0.0),
-            ],
+            3.0,
+            &[Vec3::new(3.0, 8.0, 0.0), Vec3::new(-1.0, 8.0, 0.0)],
         ));
     }

@@ -223,7 +224,7 @@ fn setup_player(mut commands: Commands, asset_server: Res<AssetServer>) {
     let mut cmd = commands.spawn_empty();
     cmd.insert(SceneBundle {
         scene: asset_server.load("player.glb#Scene0"),
-        transform: Transform::from_xyz(0.0, 10.0, 0.0),
+        transform: Transform::from_xyz(1.25, 11.0, 0.0),
         ..Default::default()
     });
     cmd.insert(GltfSceneHandler {
@@ -469,7 +470,9 @@ fn animate(
     mut animation_players_query: Query<&mut AnimationPlayer>,
 ) {
     for (mut animating_state, animating_output, handler) in animations_handlers_query.iter_mut() {
-        let Ok(mut player) = animation_players_query.get_mut(handler.player_entity) else { continue } ;
+        let Ok(mut player) = animation_players_query.get_mut(handler.player_entity) else {
+            continue;
+        };
         match animating_state.update_by_discriminant({
             if let Some(upward_velocity) = animating_output.jumping_velocity {
                 if 0.0 < upward_velocity {
@@ -507,7 +510,7 @@ fn animate(
                 AnimationState::Standing => {
                     player
                         .start(handler.animations["Standing"].clone_weak())
-                        .set_speed(1.0)
+                        .set_speed(0.0)
                         .repeat();
                 }
                 AnimationState::Running(speed) => {
idanarye commented 1 year ago

You are correct - this really is a matter of system order.

If I make the system that changes the platform's velocity run .after(bevy_tnua::TnuaPipelineStages::Sensors), the problem replicates on every run. If I make it run .before(bevy_tnua::TnuaPipelineStages::Sensors), it never happens.

I can fix the examples, but I'm not sure how I should document this. Maybe a Pitfalls section in the readme?

rparrett commented 1 year ago

For me, fixing the examples (with a comment explaining why the ordering is necessary) seems like enough. Thanks for looking into it.

I sort of wonder if tnua's systems should be running in PostUpdate, but it's all a bit awkward because I think rapier can be configured to run in any arbitrary schedule.

And in PostUpdate getting the ordering of rapier's systems and bevy's internal systems right is more involved.

rparrett commented 1 year ago

It also seems like there's no possible ordering that resolves this if the moving platform is a RigidBody::KinematicPositionBased.

idanarye commented 1 year ago

Makes sense. Tnua looks at the velocity, not the position, and with KinematicPositionBased that means the velocity only gets update once Rapier itself runs.