Calculating distance only in 1 place

Author: artemijan

game/src/movement.rs

@@ -1,6 +1,20 @@
 use std::time::Instant;
 use tokio::task::JoinHandle;
 
+/// Calculate Euclidean distance between two 3D points (x1,y1,z1) -> (x2,y2,z2)
+/// Returns None if coordinate subtraction overflows
+pub fn calculate_distance(x1: i32, y1: i32, z1: i32, x2: i32, y2: i32, z2: i32) -> Option<f64> {
+    let dx = x2.checked_sub(x1)?;
+    let dy = y2.checked_sub(y1)?;
+    let dz = z2.checked_sub(z1)?;
+
+    let dx = f64::from(dx);
+    let dy = f64::from(dy);
+    let dz = f64::from(dz);
+
+    Some((dx * dx + dy * dy + dz * dz).sqrt())
+}
+
 /// Represents the current movement state of a player
 #[derive(Debug)]
 pub struct MovementState {
@@ -49,16 +63,22 @@ impl MovementState {
     }
 
     /// Calculate the total distance to travel
-    pub fn calculate_distance(&self) -> f64 {
-        let dx = f64::from(self.dest_x) - f64::from(self.source_x);
-        let dy = f64::from(self.dest_y) - f64::from(self.source_y);
-        let dz = f64::from(self.dest_z) - f64::from(self.source_z);
-        (dx * dx + dy * dy + dz * dz).sqrt()
+    pub fn total_distance(&self) -> Option<f64> {
+        calculate_distance(
+            self.source_x,
+            self.source_y,
+            self.source_z,
+            self.dest_x,
+            self.dest_y,
+            self.dest_z,
+        )
     }
 
     /// Calculate how long the entire journey should take (in seconds)
     pub fn calculate_travel_duration(&self) -> f64 {
-        let distance = self.calculate_distance();
+        let Some(distance) = self.total_distance() else {
+            return 0.0;
+        };
         if self.speed > 0.0 {
             distance / self.speed
         } else {
@@ -78,9 +98,12 @@ impl MovementState {
 
         let progress = (elapsed / duration).min(1.0);
 
-        let current_x = self.source_x + ((f64::from(self.dest_x) - f64::from(self.source_x)) * progress) as i32;
-        let current_y = self.source_y + ((f64::from(self.dest_y) - f64::from(self.source_y)) * progress) as i32;
-        let current_z = self.source_z + ((f64::from(self.dest_z) - f64::from(self.source_z)) * progress) as i32;
+        let current_x =
+            self.source_x + ((f64::from(self.dest_x) - f64::from(self.source_x)) * progress) as i32;
+        let current_y =
+            self.source_y + ((f64::from(self.dest_y) - f64::from(self.source_y)) * progress) as i32;
+        let current_z =
+            self.source_z + ((f64::from(self.dest_z) - f64::from(self.source_z)) * progress) as i32;
 
         (current_x, current_y, current_z)
     }
@@ -113,23 +136,25 @@ mod tests {
     #[test]
     fn test_calculate_distance() {
         let state = MovementState::new(0, 0, 0, 300, 400, 0, 100);
-        let distance = state.calculate_distance();
-        assert_eq!(distance, 500.0); // 3-4-5 triangle
+        let distance = state.total_distance().unwrap();
+        //float values can give 0.00000001 error, so we check for a range of 0.001
+        assert!((distance - 500.0).abs() < 0.001); // 3-4-5 triangle
     }
 
     #[test]
     fn test_calculate_travel_duration() {
         let state = MovementState::new(0, 0, 0, 500, 0, 0, 100);
         let duration = state.calculate_travel_duration();
-        assert_eq!(duration, 5.0); // 500 units at 100 units/sec = 5 seconds
+        //float values can give 0.00000001 error, so we check for a range of 0.001
+        assert!((duration - 5.0).abs() < 0.1); // 500 units at 100 units/sec = 5 seconds
     }
 
     #[test]
     fn test_calculate_current_position_at_start() {
         let state = MovementState::new(0, 0, 0, 1000, 0, 0, 100);
         let (x, y, z) = state.calculate_current_position();
         // Should be at or very near start position
-        assert!(x >= 0 && x < 50); // Allow small movement
+        assert!((0..10).contains(&x)); // Allow small movement
         assert_eq!(y, 0);
         assert_eq!(z, 0);
     }

game/src/packets/from_client/move_to_location.rs

@@ -1,23 +1,9 @@
-use anyhow::bail;
 use bytes::BytesMut;
 use kameo::message::{Context, Message};
 use l2_core::shared_packets::common::ReadablePacket;
 use l2_core::shared_packets::read::ReadablePacketBuffer;
-use tracing::{info, instrument, warn};
-
-/// Helper function to calculate distance between two 3D points
-/// Returns None if coordinate subtraction overflows
-fn calculate_distance(x1: i32, y1: i32, z1: i32, x2: i32, y2: i32, z2: i32) -> Option<f64> {
-    let dx = x2.checked_sub(x1)?;
-    let dy = y2.checked_sub(y1)?;
-    let dz = z2.checked_sub(z1)?;
-    
-    let dx = f64::from(dx);
-    let dy = f64::from(dy);
-    let dz = f64::from(dz);
-    
-    Some((dx * dx + dy * dy + dz * dz).sqrt())
-}
+use tracing::{instrument, warn};
+use crate::movement::calculate_distance;
 
 use crate::pl_client::PlayerClient;
 
@@ -56,52 +42,41 @@ impl Message<RequestMoveToLocation> for PlayerClient {
         msg: RequestMoveToLocation,
         ctx: &mut Context<Self, Self::Reply>,
     ) -> anyhow::Result<()> {
-        info!("Received MoveToLocation packet {:?}", msg);
-        
         //TODO check with geodata if the location is valid.
-        
-        // Get current position for distance validation
+
+        // Get the current position for distance validation
         let player = self.try_get_selected_char()?;
         let (current_x, current_y, current_z) = (player.get_x(), player.get_y(), player.get_z());
-        
+
         // Calculate distance
-        let distance = match calculate_distance(current_x, current_y, current_z, msg.x_to, msg.y_to, msg.z_to) {
-            Some(d) => d,
-            None => {
-                warn!(
-                    "Player {} attempted movement causing coordinate overflow. Pos: ({},{},{}) -> Dest: ({},{},{})",
-                    player.char_model.name,
-                    current_x, current_y, current_z,
-                    msg.x_to, msg.y_to, msg.z_to
-                );
-                bail!("Movement coordinates cause overflow");
-            }
+        let Some(distance) = calculate_distance(
+            current_x, current_y, current_z, msg.x_to, msg.y_to, msg.z_to,
+        ) else {
+            warn!(
+                "Player {} attempted movement causing coordinate overflow. Pos: ({},{},{}) -> Dest: ({},{},{})",
+                player.char_model.name,
+                current_x,
+                current_y,
+                current_z,
+                msg.x_to,
+                msg.y_to,
+                msg.z_to
+            );
+            return Ok(());
         };
-        
+
         // Check against max distance from config
         let cfg = self.controller.get_cfg();
         if cfg.max_movement_distance > 0 && distance > f64::from(cfg.max_movement_distance) {
             warn!(
                 "Player {} attempted to move excessive distance: {:.2} (max: {})",
-                player.char_model.name,
-                distance,
-                cfg.max_movement_distance
+                player.char_model.name, distance, cfg.max_movement_distance
             );
             return Ok(());
         }
-        
-        // Start or restart movement
-        let (source_x, source_y, source_z) = self.start_movement(msg.x_to, msg.y_to, msg.z_to, ctx.actor_ref().clone())?;
-        
-        // Send initial movement packet immediately (optional, but good for responsiveness if task has delay)
-        // Note: The task sends update immediately, but we might want to send the "start" packet here?
-        // Actually, the previous logic removed the explicit broadcast.
-        // But wait, the task sends `CharMoveToLocation`.
-        // If I remove the explicit broadcast here, the task will send it.
-        // So I just need to call start_movement.
+
+        self.start_movement(msg.x_to, msg.y_to, msg.z_to, ctx.actor_ref().clone())?;
 
         Ok(())
     }
 }
-
-

game/src/pl_client.rs

@@ -1,13 +1,13 @@
 use crate::controller::GameController;
 use crate::cp_factory::build_client_packet;
 use crate::movement::MovementState;
+use crate::packets::to_client::CharMoveToLocation;
 use anyhow::{anyhow, bail};
 use bytes::BytesMut;
 use entities::entities::{character, user};
 use entities::DBPool;
 use kameo::actor::{ActorId, ActorRef, Spawn, WeakActorRef};
 use kameo::error::{ActorStopReason, PanicError};
-use crate::packets::to_client::CharMoveToLocation;
 use kameo::message::{Context, Message};
 use kameo::Actor;
 use l2_core::crypt::game::GameClientEncryption;
@@ -327,15 +327,16 @@ impl PlayerClient {
         dest_z: i32,
         actor_ref: ActorRef<PlayerClient>,
     ) -> anyhow::Result<(i32, i32, i32)> {
-        // Cancel existing movement if any and get current position
-        let (current_x, current_y, current_z) = if let Some(mut existing_movement) = self.movement_state.take() {
-            existing_movement.cancel_task();
-            existing_movement.calculate_current_position()
-        } else {
-            // Use player's stored position
-            let player = self.try_get_selected_char()?;
-            (player.get_x(), player.get_y(), player.get_z())
-        };
+        // Cancel existing movement if any and get the current position
+        let (current_x, current_y, current_z) =
+            if let Some(mut existing_movement) = self.movement_state.take() {
+                existing_movement.cancel_task();
+                existing_movement.calculate_current_position()
+            } else {
+                // Use a player's stored position
+                let player = self.try_get_selected_char()?;
+                (player.get_x(), player.get_y(), player.get_z())
+            };
 
         // Get player speed
         let player = self.try_get_selected_char()?;
@@ -345,53 +346,50 @@ impl PlayerClient {
             player.get_walk_speed()
         };
 
-        // Create new movement state
+        // Create a new movement state
         let mut movement = MovementState::new(
-            current_x,
-            current_y,
-            current_z,
-            dest_x,
-            dest_y,
-            dest_z,
-            speed,
+            current_x, current_y, current_z, dest_x, dest_y, dest_z, speed,
         );
 
         // Spawn periodic broadcast task
         let controller = self.controller.clone();
-        
+
         let task_handle = tokio::spawn(async move {
             let mut interval = tokio::time::interval(std::time::Duration::from_millis(300));
-            
+
             loop {
                 // Check movement state BEFORE waiting
-                let result = actor_ref
-                    .ask(GetMovementPosition)
-                    .await;
+                let result = actor_ref.ask(GetMovementPosition).await;
 
                 if let Ok(Some((_x, _y, _z, has_arrived))) = result {
                     if has_arrived {
                         // Arrived at destination, stop broadcasting
-                        info!("Player arrived at destination ({}, {}, {})", dest_x, dest_y, dest_z);
+                        info!(
+                            "Player arrived at destination ({}, {}, {})",
+                            dest_x, dest_y, dest_z
+                        );
                         break;
                     }
 
                     // Broadcast current position
-                    if let Ok(player) = actor_ref.ask(crate::pl_client::GetCharInfo).await {
-                        if let Ok(packet) = CharMoveToLocation::new(&player, dest_x, dest_y, dest_z) {
-                            controller.broadcast_packet(packet);
-                        }
+                    if let Ok(player) = actor_ref.ask(GetCharInfo).await
+                        && let Ok(packet) = CharMoveToLocation::new(&player, dest_x, dest_y, dest_z)
+                    {
+                        controller.broadcast_packet(packet);
+                    } else {
+                        error!("Error while broadcasting movement packet");
                     }
                 } else {
-                    // Error or no movement state, stop task
+                    // Error or no movement state, stop a task
                     break;
                 }
 
-                // Wait for next tick
+                // Wait for the next tick
                 interval.tick().await;
             }
         });
 
-        // Store the task handle in movement state and set it in client
+        // Store the task handle in a movement state and set it in a client
         movement.task_handle = Some(task_handle);
         self.movement_state = Some(movement);
 
@@ -409,7 +407,6 @@ impl PlayerClient {
     }
 }
 
-
 impl Actor for PlayerClient {
     type Args = (
         Self,
@@ -569,12 +566,12 @@ impl Message<GetMovementPosition> for PlayerClient {
         if let Some(movement) = self.movement_state.as_ref() {
             let (x, y, z) = movement.calculate_current_position();
             let has_arrived = movement.has_arrived();
-            
+
             // Update player's actual position
             if let Ok(player) = self.try_get_selected_char_mut() {
                 let _ = player.set_location(x, y, z);
             }
-            
+
             Ok(Some((x, y, z, has_arrived)))
         } else {
             Ok(None)