From 7aac77288b9b07dc3923b16e2a5d0cf97f42d0be Mon Sep 17 00:00:00 2001
From: Avi Weinstock <aweinstock314@gmail.com>
Date: Sun, 30 May 2021 11:38:47 -0400
Subject: [PATCH] Address MR 2356 comments.

---
 common/src/comp/agent.rs             | 23 +++++++++++++++++++----
 common/src/comp/body/ship.rs         |  7 -------
 server/src/cmd.rs                    |  4 ++--
 server/src/events/entity_creation.rs | 13 ++++++++++---
 server/src/sys/agent.rs              |  4 ++--
 5 files changed, 33 insertions(+), 18 deletions(-)

diff --git a/common/src/comp/agent.rs b/common/src/comp/agent.rs
index 4b0a9150dd..3fade71b8c 100644
--- a/common/src/comp/agent.rs
+++ b/common/src/comp/agent.rs
@@ -1,5 +1,5 @@
 use crate::{
-    comp::{humanoid, quadruped_low, quadruped_medium, quadruped_small, Body},
+    comp::{humanoid, quadruped_low, quadruped_medium, quadruped_small, ship, Body},
     path::Chaser,
     rtsim::RtSimController,
     trade::{PendingTrade, ReducedInventory, SiteId, SitePrices, TradeId, TradeResult},
@@ -314,7 +314,7 @@ pub struct Agent {
     pub bearing: Vec2<f32>,
     pub sounds_heard: Vec<Sound>,
     pub awareness: f32,
-    pub pid_controller: Option<PidController<fn(Vec3<f32>, Vec3<f32>) -> f32, 16>>,
+    pub position_pid_controller: Option<PidController<fn(Vec3<f32>, Vec3<f32>) -> f32, 16>>,
 }
 
 #[derive(Clone, Debug, Default)]
@@ -339,11 +339,11 @@ impl Agent {
     }
 
     #[allow(clippy::type_complexity)]
-    pub fn with_pid_controller(
+    pub fn with_position_pid_controller(
         mut self,
         pid: PidController<fn(Vec3<f32>, Vec3<f32>) -> f32, 16>,
     ) -> Self {
-        self.pid_controller = Some(pid);
+        self.position_pid_controller = Some(pid);
         self
     }
 
@@ -503,3 +503,18 @@ impl<F: Fn(Vec3<f32>, Vec3<f32>) -> f32, const NUM_SAMPLES: usize> PidController
         (f(x1) - f(x0)) / h as f32
     }
 }
+
+/// Get the PID coefficients associated with some Body, since it will likely
+/// need to be tuned differently for each body type
+pub fn pid_coefficients(body: &Body) -> (f32, f32, f32) {
+    match body {
+        Body::Ship(ship::Body::DefaultAirship) => {
+            let kp = 1.0;
+            let ki = 1.0;
+            let kd = 1.0;
+            (kp, ki, kd)
+        },
+        // default to a pure-proportional controller, which is the first step when tuning
+        _ => (1.0, 0.0, 0.0),
+    }
+}
diff --git a/common/src/comp/body/ship.rs b/common/src/comp/body/ship.rs
index ccedd285b5..f880683d47 100644
--- a/common/src/comp/body/ship.rs
+++ b/common/src/comp/body/ship.rs
@@ -52,13 +52,6 @@ impl Body {
     pub fn density(&self) -> Density { Density(AIR_DENSITY) }
 
     pub fn mass(&self) -> Mass { Mass((self.hull_vol() + self.balloon_vol()) * self.density().0) }
-
-    pub fn pid_coefficients(&self) -> (f32, f32, f32) {
-        let kp = 1.0;
-        let ki = 1.0;
-        let kd = 1.0;
-        (kp, ki, kd)
-    }
 }
 
 /// Terrain is 11.0 scale relative to small-scale voxels,
diff --git a/server/src/cmd.rs b/server/src/cmd.rs
index b93550d1fe..b350359a88 100644
--- a/server/src/cmd.rs
+++ b/server/src/cmd.rs
@@ -1163,11 +1163,11 @@ fn handle_spawn_airship(
             animated: true,
         });
     if let Some(pos) = destination {
-        let (kp, ki, kd) = ship.pid_coefficients();
+        let (kp, ki, kd) = comp::agent::pid_coefficients(&comp::Body::Ship(ship));
         fn pure_z(sp: Vec3<f32>, pv: Vec3<f32>) -> f32 { (sp - pv).z }
         let agent = comp::Agent::default()
             .with_destination(pos)
-            .with_pid_controller(comp::PidController::new(
+            .with_position_pid_controller(comp::PidController::new(
                 kp,
                 ki,
                 kd,
diff --git a/server/src/events/entity_creation.rs b/server/src/events/entity_creation.rs
index 1e659477e7..c66317cf0e 100644
--- a/server/src/events/entity_creation.rs
+++ b/server/src/events/entity_creation.rs
@@ -3,6 +3,7 @@ use common::{
     character::CharacterId,
     comp::{
         self,
+        agent::pid_coefficients,
         aura::{Aura, AuraKind, AuraTarget},
         beam,
         buff::{BuffCategory, BuffData, BuffKind, BuffSource},
@@ -147,10 +148,16 @@ pub fn handle_create_ship(
 ) {
     let mut entity = server.state.create_ship(pos, ship, mountable);
     if let Some(mut agent) = agent {
-        let (kp, ki, kd) = ship.pid_coefficients();
+        let (kp, ki, kd) = pid_coefficients(&Body::Ship(ship));
         fn pure_z(sp: Vec3<f32>, pv: Vec3<f32>) -> f32 { (sp - pv).z }
-        agent =
-            agent.with_pid_controller(PidController::new(kp, ki, kd, Vec3::zero(), 0.0, pure_z));
+        agent = agent.with_position_pid_controller(PidController::new(
+            kp,
+            ki,
+            kd,
+            Vec3::zero(),
+            0.0,
+            pure_z,
+        ));
         entity = entity.with(agent);
     }
     if let Some(rtsim_entity) = rtsim_entity {
diff --git a/server/src/sys/agent.rs b/server/src/sys/agent.rs
index 1b407e4ee6..7f191c6405 100644
--- a/server/src/sys/agent.rs
+++ b/server/src/sys/agent.rs
@@ -271,7 +271,7 @@ impl<'a> System<'a> for Sys {
                         Some(CharacterState::GlideWield) | Some(CharacterState::Glide(_))
                     ) && !physics_state.on_ground;
 
-                    if let Some(pid) = agent.pid_controller.as_mut() {
+                    if let Some(pid) = agent.position_pid_controller.as_mut() {
                         pid.add_measurement(read_data.time.0, pos.0);
                     }
 
@@ -870,7 +870,7 @@ impl<'a> AgentData<'a> {
                     } else {
                         0.05 //normal land traveller offset
                     };
-                if let Some(pid) = agent.pid_controller.as_mut() {
+                if let Some(pid) = agent.position_pid_controller.as_mut() {
                     pid.sp = self.pos.0.z + height_offset * Vec3::unit_z();
                     controller.inputs.move_z = pid.calc_err();
                 } else {