fix: better player movement handling (#831)

This PR aims to standardize player position handling notably
teleportations and velocity changes.
Right now every module does its own thing and sends packets directly to
the client so it is impossible for the server to keep track of the
player's state.
By normalizing the way to change player position/velocity it could allow
us to implement better movement verification and better on_ground
calculation in the future.

Author: GroobleDierne

Cargo.lock

@@ -6,14 +6,18 @@ use hyperion::{
         prelude::Module,
     },
     net::{ConnectionId, DataBundle},
-    protocol::{game_mode::OptGameMode, packets::play, ByteAngle, VarInt},
-    server::{ident, GameMode},
+    protocol::{
+        game_mode::OptGameMode,
+        packets::play::{self, PlayerAbilitiesS2c},
+        BlockPos, ByteAngle, GlobalPos, VarInt,
+    },
+    server::{abilities::PlayerAbilitiesFlags, ident, GameMode},
     simulation::{
         event::{ClientStatusCommand, ClientStatusEvent},
         handlers::PacketSwitchQuery,
         metadata::{entity::Pose, living_entity::Health},
         packet::HandlerRegistry,
-        Pitch, Position, Uuid, Xp, Yaw,
+        Flight, FlyingSpeed, Pitch, Position, Uuid, Xp, Yaw,
     },
 };
 use hyperion_utils::{EntityExt, LifetimeHandle};
@@ -43,8 +47,21 @@ impl Module for RespawnModule {
                         &Yaw,
                         &Pitch,
                         &Xp,
+                        &Flight,
+                        &FlyingSpeed,
                     )>(
-                        |(connection, health, pose, uuid, position, yaw, pitch, xp)| {
+                        |(
+                            connection,
+                            health,
+                            pose,
+                            uuid,
+                            position,
+                            yaw,
+                            pitch,
+                            xp,
+                            flight,
+                            flying_speed,
+                        )| {
                             health.heal(20.);
 
                             *pose = Pose::Standing;
@@ -65,7 +82,10 @@ impl Module for RespawnModule {
                                 is_debug: false,
                                 is_flat: false,
                                 copy_metadata: false,
-                                last_death_location: None,
+                                last_death_location: Option::from(GlobalPos {
+                                    dimension_name: ident!("minecraft:overworld").into(),
+                                    position: BlockPos::from(position.as_dvec3()),
+                                }),
                                 portal_cooldown: VarInt::default(),
                             };
 
@@ -75,6 +95,14 @@ impl Module for RespawnModule {
                                 total_xp: VarInt::default(),
                             };
 
+                            let pkt_abilities = PlayerAbilitiesS2c {
+                                flags: PlayerAbilitiesFlags::default()
+                                    .with_flying(flight.is_flying)
+                                    .with_allow_flying(flight.allow),
+                                flying_speed: flying_speed.speed,
+                                fov_modifier: 0.0,
+                            };
+
                             let pkt_add_player = play::PlayerSpawnS2c {
                                 entity_id: VarInt(client.minecraft_id()),
                                 player_uuid: uuid.0,
@@ -87,11 +115,13 @@ impl Module for RespawnModule {
                             bundle.add_packet(&pkt_health).unwrap();
                             bundle.add_packet(&pkt_respawn).unwrap();
                             bundle.add_packet(&pkt_xp).unwrap();
+                            bundle.add_packet(&pkt_abilities).unwrap();
 
                             bundle.unicast(*connection).unwrap();
                             query
                                 .compose
                                 .broadcast(&pkt_add_player, query.system)
+                                .exclude(*connection)
                                 .send()
                                 .unwrap();
                         },

crates/hyperion-respawn/src/lib.rs

@@ -2,6 +2,7 @@ use std::{borrow::Cow, collections::BTreeSet, ops::Index};
 
 use anyhow::Context;
 use flecs_ecs::prelude::*;
+use glam::DVec3;
 use hyperion_crafting::{Action, CraftingRegistry, RecipeBookState};
 use hyperion_utils::EntityExt;
 use rayon::iter::{IntoParallelIterator, ParallelIterator};
@@ -20,7 +21,7 @@ use valence_registry::{BiomeRegistry, RegistryCodec};
 use valence_server::entity::EntityKind;
 use valence_text::IntoText;
 
-use crate::simulation::{PacketState, Pitch};
+use crate::simulation::{MovementTracking, PacketState, Pitch};
 
 mod list;
 pub use list::*;
@@ -76,6 +77,16 @@ pub fn player_join_world(
 
     let id = entity.minecraft_id();
 
+    entity.set(MovementTracking {
+        received_movement_packets: 0,
+        last_tick_flying: false,
+        last_tick_position: **position,
+        fall_start_y: position.y,
+        server_velocity: DVec3::ZERO,
+        sprinting: false,
+        was_on_ground: false,
+    });
+
     let registry_codec = registry_codec_raw();
     let codec = RegistryCodec::default();
 
@@ -567,6 +578,7 @@ impl Module for PlayerJoinModule {
                         |(uuid, name, position, yaw, pitch, &stream_id)| {
                             let query = &query;
                             let query = &query.0;
+                            entity.set_name(name);
 
                             // if we get an error joining, we should kick the player
                             if let Err(e) = player_join_world(

crates/hyperion/src/egress/player_join/mod.rs

@@ -1,7 +1,7 @@
 use std::fmt::Debug;
 
 use flecs_ecs::prelude::*;
-use glam::Vec3;
+use glam::{IVec3, Vec3};
 use hyperion_utils::EntityExt;
 use itertools::Either;
 use valence_protocol::{
@@ -13,11 +13,11 @@ use crate::{
     Prev,
     net::{Compose, ConnectionId, DataBundle},
     simulation::{
-        Owner, Pitch, Position, Velocity, Xp, Yaw,
+        Flight, MovementTracking, Owner, PendingTeleportation, Pitch, Position, Velocity, Xp, Yaw,
         animation::ActiveAnimation,
         blocks::Blocks,
         entity_kind::EntityKind,
-        event,
+        event::{self, HitGroundEvent},
         handlers::is_grounded,
         metadata::{MetadataChanges, get_and_clear_metadata},
     },
@@ -143,8 +143,16 @@ impl Module for EntityStateSyncModule {
                         entity_id,
                         tracked_values: RawBytes(&view),
                     };
-
-                    // todo(perf): do so locally
+                    if entity.has::<Position>() {
+                        entity.get::<&Position>(|position| {
+                            compose
+                                .broadcast_local(&pkt, position.to_chunk(), system)
+                                .send()
+                                .unwrap();
+                        });
+                        return;
+                    }
+                    // Should never be reached but who knows
                     compose.broadcast(&pkt, system).send().unwrap();
                 }
             });
@@ -188,46 +196,92 @@ impl Module for EntityStateSyncModule {
             "sync_player_entity",
             world,
             &Compose($),
-            &mut (Prev, Position),
+            &mut Events($),
             &mut (Prev, Yaw),
             &mut (Prev, Pitch),
             &mut Position,
             &mut Velocity,
             &Yaw,
             &Pitch,
+            ?&mut PendingTeleportation,
+            &mut MovementTracking,
+            &Flight,
         )
-            .multi_threaded()
-            .kind::<flecs::pipeline::PreStore>()
-            .each_iter(
-                |it,
-                 row,
-                 (
-                     compose,
-                     prev_position,
-                     prev_yaw,
-                     prev_pitch,
-                     position,
-                     velocity,
-                     yaw,
-                     pitch,
-                 )| {
-                    let world = it.system().world();
-                    let system = it.system();
-                    let entity = it.entity(row);
-                    let entity_id = VarInt(entity.minecraft_id());
+        .multi_threaded()
+        .kind::<flecs::pipeline::PreStore>()
+        .each_iter(
+            |it,
+             row,
+             (
+                compose,
+                events,
+                prev_yaw,
+                prev_pitch,
+                position,
+                velocity,
+                yaw,
+                pitch,
+                pending_teleport,
+                tracking,
+                flight,
+            )| {
+                let world = it.system().world();
+                let system = it.system();
+                let entity = it.entity(row);
+                let entity_id = VarInt(entity.minecraft_id());
 
+                if let Some(pending_teleport) = pending_teleport {
+                    if pending_teleport.ttl == 0 {
+                        entity.set::<PendingTeleportation>(PendingTeleportation::new(
+                            pending_teleport.destination,
+                        ));
+                    }
+                    pending_teleport.ttl -= 1;
+                } else {
                     let chunk_pos = position.to_chunk();
 
-                    let position_delta = **position - **prev_position;
+                    let position_delta = **position - tracking.last_tick_position;
                     let needs_teleport = position_delta.abs().max_element() >= 8.0;
-                    let changed_position = **position != **prev_position;
+                    let changed_position = **position != tracking.last_tick_position;
 
-                    let look_changed = (**yaw - **prev_yaw).abs() >= 0.01 || (**pitch - **prev_pitch).abs() >= 0.01;
+                    let look_changed = (**yaw - **prev_yaw).abs() >= 0.01
+                        || (**pitch - **prev_pitch).abs() >= 0.01;
 
                     let mut bundle = DataBundle::new(compose, system);
 
+                    // Maximum number of movement packets allowed during 1 tick is 5
+                    if tracking.received_movement_packets > 5 {
+                        tracking.received_movement_packets = 1;
+                    }
+
+                    // Replace 100 by 300 if fall flying (aka elytra)
+                    if f64::from(position_delta.length_squared())
+                        - tracking.server_velocity.length_squared()
+                        > 100f64 * f64::from(tracking.received_movement_packets)
+                    {
+                        entity.set(PendingTeleportation::new(tracking.last_tick_position));
+                        tracking.received_movement_packets = 0;
+                        return;
+                    }
+
                     world.get::<&mut Blocks>(|blocks| {
                         let grounded = is_grounded(position, blocks);
+                        tracking.was_on_ground = grounded;
+                        if grounded
+                            && !tracking.last_tick_flying
+                            && tracking.fall_start_y - position.y > 3.
+                        {
+                            let event = HitGroundEvent {
+                                client: *entity,
+                                fall_distance: tracking.fall_start_y - position.y,
+                            };
+                            events.push(event, &world);
+                            tracking.fall_start_y = position.y;
+                        }
+
+                        if (tracking.last_tick_flying && flight.allow) || position_delta.y >= 0. {
+                            tracking.fall_start_y = position.y;
+                        }
 
                         if changed_position && !needs_teleport && look_changed {
                             let packet = play::RotateAndMoveRelativeS2c {
@@ -290,11 +344,51 @@ impl Module for EntityStateSyncModule {
                         };
 
                         bundle.add_packet(&packet).unwrap();
+                        velocity.0 = Vec3::ZERO;
                     }
 
                     bundle.broadcast_local(chunk_pos).unwrap();
-                },
-            );
+                }
+
+                tracking.received_movement_packets = 0;
+                tracking.last_tick_position = **position;
+                tracking.last_tick_flying = flight.is_flying;
+
+                let mut friction = 0.91;
+
+                if tracking.was_on_ground {
+                    tracking.server_velocity.y = 0.;
+                    #[allow(clippy::cast_possible_truncation)]
+                    world.get::<&mut Blocks>(|blocks| {
+                        let block_x = position.x as i32;
+                        let block_y = (position.y.ceil() - 1.0) as i32; // Check the block directly below
+                        let block_z = position.z as i32;
+
+                        if let Some(state) = blocks.get_block(IVec3::new(block_x, block_y, block_z))
+                        {
+                            let kind = state.to_kind();
+                            friction = f64::from(0.91 * kind.slipperiness() * kind.speed_factor());
+                        }
+                    });
+                }
+
+                tracking.server_velocity.x *= friction * 0.98;
+                tracking.server_velocity.y -= 0.08 * 0.980_000_019_073_486_3;
+                tracking.server_velocity.z *= friction * 0.98;
+
+                if tracking.server_velocity.x.abs() < 0.003 {
+                    tracking.server_velocity.x = 0.;
+                }
+
+                if tracking.server_velocity.y.abs() < 0.003 {
+                    tracking.server_velocity.y = 0.;
+                }
+
+                if tracking.server_velocity.z.abs() < 0.003 {
+                    tracking.server_velocity.z = 0.;
+                }
+            },
+        );
 
         system!(
             "update_projectile_positions",

crates/hyperion/src/egress/sync_entity_state.rs

@@ -359,12 +359,18 @@ impl Blocks {
     #[must_use]
     pub fn get_block(&self, position: IVec3) -> Option<BlockState> {
         const START_Y: i32 = -64;
+        const MAX_Y: i32 = 383;
 
+        // Todo should probably return none
         if position.y < START_Y {
             // This block is in the void.
             return Some(BlockState::VOID_AIR);
         }
 
+        if position.y > MAX_Y {
+            return Some(BlockState::VOID_AIR);
+        }
+
         let chunk_pos: IVec2 = IVec2::new(position.x, position.z) >> 4;
         let chunk_start_block: IVec2 = chunk_pos << 4;
 

crates/hyperion/src/simulation/blocks/mod.rs

@@ -1,6 +1,5 @@
 //! Flecs components which are used for events.
 
-use derive_more::Constructor;
 use flecs_ecs::{core::Entity, macros::Component};
 use glam::{IVec3, Vec3};
 use hyperion_utils::{Lifetime, RuntimeLifetime};
@@ -48,12 +47,6 @@ pub struct AttackEntity {
     pub damage: f32,
 }
 
-#[derive(Copy, Clone, Debug, PartialEq, Constructor)]
-pub struct HealthUpdate {
-    pub from: f32,
-    pub to: f32,
-}
-
 #[derive(Copy, Clone, Debug, PartialEq, Eq)]
 pub struct StartDestroyBlock {
     pub position: IVec3,
@@ -181,3 +174,10 @@ pub struct UpdateSelectedSlotEvent {
     pub client: Entity,
     pub slot: u8,
 }
+
+#[derive(Clone, Debug)]
+pub struct HitGroundEvent {
+    pub client: Entity,
+    /// This is at least 3
+    pub fall_distance: f32,
+}