Addressed MR 2824 comments

common/src/combat.rs

@@ -231,7 +231,7 @@ impl Attack {
             let applied_damage = -change.amount as f32;
             accumulated_damage += applied_damage;
             emit_outcome(Outcome::Damage { pos: target.pos });
-            if change.amount.abs() > f32::EPSILON {
+            if change.amount.abs() > Health::HEALTH_EPSILON {
                 emit(ServerEvent::HealthChange {
                     entity: target.entity,
                     change,
@@ -277,7 +277,7 @@ impl Attack {
                                     by: attacker.map(|a| a.uid),
                                     cause: None,
                                 };
-                                if change.amount.abs() > f32::EPSILON {
+                                if change.amount.abs() > Health::HEALTH_EPSILON {
                                     emit(ServerEvent::HealthChange {
                                         entity: attacker_entity,
                                         change,
@@ -301,7 +301,7 @@ impl Attack {
                                 by: attacker.map(|a| a.uid),
                                 cause: None,
                             };
-                            if change.amount.abs() > f32::EPSILON {
+                            if change.amount.abs() > Health::HEALTH_EPSILON {
                                 emit(ServerEvent::HealthChange {
                                     entity: target.entity,
                                     change,
@@ -412,7 +412,7 @@ impl Attack {
                                 by: attacker.map(|a| a.uid),
                                 cause: None,
                             };
-                            if change.amount.abs() > f32::EPSILON {
+                            if change.amount.abs() > Health::HEALTH_EPSILON {
                                 emit(ServerEvent::HealthChange {
                                     entity: attacker_entity,
                                     change,
@@ -436,7 +436,7 @@ impl Attack {
                             by: attacker.map(|a| a.uid),
                             cause: None,
                         };
-                        if change.amount.abs() > f32::EPSILON {
+                        if change.amount.abs() > Health::HEALTH_EPSILON {
                             emit(ServerEvent::HealthChange {
                                 entity: target.entity,
                                 change,

common/src/comp/health.rs

@@ -27,9 +27,17 @@ impl HealthChange {
 // floats rather than integers.
 pub struct Health {
     // Current and base_max are scaled by 256 within this module compared to what is visible to
-    // outside this module
+    // outside this module. The scaling is done to allow health to function as a fixed point while
+    // still having the advantages of being an integer. The scaling of 256 was chosen so that max
+    // health could be u16::MAX - 1, and then the scaled health could fit inside an f32 with no
+    // precision loss
+    /// Current health is how much health the entity currently has
     current: u32,
+    /// Base max is the amount of health the entity has without considering
+    /// temporary modifiers such as buffs
     base_max: u32,
+    /// Maximum is the amount of health the entity has after temporary modifiers
+    /// are considered
     maximum: u32,
     // Time since last change and what the last change was
     // TODO: Remove the time since last change, either convert to time of last change or just emit
@@ -39,13 +47,20 @@ pub struct Health {
 }
 
 impl Health {
+    /// Used when comparisons to health are needed outside this module.
+    // This value is chosen as anything smaller than this is more precise than our
+    // units of health.
+    pub const HEALTH_EPSILON: f32 = 0.5 / Self::MAX_SCALED_HEALTH as f32;
+    /// Maximum value allowed for health before scaling
+    const MAX_HEALTH: u16 = u16::MAX - 1;
     /// The maximum value allowed for current and maximum health
-    /// Maximum value is u16:MAX - 1 * 256, which only requires 24 bits. This
+    /// Maximum value is (u16:MAX - 1) * 256, which only requires 24 bits. This
     /// can fit into an f32 with no loss to precision
-    const MAX_HEALTH: u32 = 16776960;
+    // Cast to u32 done as u32::from cannot be called inside constant
+    const MAX_SCALED_HEALTH: u32 = Self::MAX_HEALTH as u32 * Self::SCALING_FACTOR_INT;
     /// The amount health is scaled by within this module
     const SCALING_FACTOR_FLOAT: f32 = 256.;
-    const SCALING_FACTOR_INT: u32 = 256;
+    const SCALING_FACTOR_INT: u32 = Self::SCALING_FACTOR_FLOAT as u32;
 
     /// Returns the current value of health casted to a float
     pub fn current(&self) -> f32 { self.current as f32 / Self::SCALING_FACTOR_FLOAT }
@@ -63,14 +78,17 @@ impl Health {
     pub fn update_maximum(&mut self, modifiers: comp::stats::StatsModifier) {
         let maximum = modifiers
             .compute_maximum(self.base_max as f32)
-            .min(Self::MAX_HEALTH as f32) as u32;
+            // NaN does not need to be handled here as rust will automatically change to 0 when casting to u32
+            .clamp(0.0, Self::MAX_SCALED_HEALTH as f32) as u32;
         self.maximum = maximum;
     }
 
     #[cfg(not(target_arch = "wasm32"))]
     pub fn new(body: comp::Body, level: u16) -> Self {
-        let health = u32::from(body.base_health() + body.base_health_increase() * level)
+        let health = u32::from(
-            * Self::SCALING_FACTOR_INT;
+            body.base_health()
+                .saturating_add(body.base_health_increase().saturating_mul(level)),
+        ) * Self::SCALING_FACTOR_INT;
         Health {
             current: health,
             base_max: health,
@@ -88,15 +106,18 @@ impl Health {
     #[cfg(not(target_arch = "wasm32"))]
     pub fn update_max_hp(&mut self, body: comp::Body, level: u16) {
         let old_max = self.base_max;
-        self.base_max = u32::from(body.base_health() + body.base_health_increase() * level)
+        self.base_max = u32::from(
-            * Self::SCALING_FACTOR_INT;
+            body.base_health()
+                .saturating_add(body.base_health_increase().saturating_mul(level)),
+        ) * Self::SCALING_FACTOR_INT;
         self.current = (self.current + self.base_max - old_max).min(self.maximum);
     }
 
     #[cfg(not(target_arch = "wasm32"))]
     pub fn change_by(&mut self, change: HealthChange) {
-        self.current = (((self.current() + change.amount) as u32 * Self::SCALING_FACTOR_INT).max(0)
+        self.current = (((self.current() + change.amount).clamp(0.0, f32::from(Self::MAX_HEALTH))
-            as u32)
+            as u32
+            * Self::SCALING_FACTOR_INT) as u32)
             .min(self.maximum);
         self.last_change = (0.0, change);
     }

common/systems/src/stats.rs

@@ -114,7 +114,7 @@ impl<'a> System<'a> for Sys {
 
             let update_max_hp = {
                 stat.max_health_modifiers.update_maximum()
-                    || (health.base_max() - health.maximum()).abs() > f32::EPSILON
+                    || (health.base_max() - health.maximum()).abs() > Health::HEALTH_EPSILON
             };
 
             if update_max_hp {

server/src/events/entity_manipulation.rs

@@ -65,7 +65,8 @@ pub fn handle_health_change(server: &Server, entity: EcsEntity, change: HealthCh
     }
     // This if statement filters out anything under 5 damage, for DOT ticks
     // TODO: Find a better way to separate direct damage from DOT here
-    if change.amount < -5.0 {
+    let damage = -change.amount;
+    if damage > -5.0 {
         if let Some(agent) = ecs.write_storage::<Agent>().get_mut(entity) {
             agent.inbox.push_front(AgentEvent::Hurt);
         }

voxygen/src/ecs/sys/floater.rs

@@ -61,7 +61,7 @@ impl<'a> System<'a> for Sys {
 
             // Check if health has changed (won't work if damaged and then healed with
             // equivalently in the same frame)
-            if (hp_floater_list.last_hp - health.current()).abs() > f32::EPSILON {
+            if (hp_floater_list.last_hp - health.current()).abs() > Health::HEALTH_EPSILON {
                 hp_floater_list.last_hp = health.current();
                 // TODO: What if multiple health changes occurred since last check here
                 // Also, If we make health store a vec of the last_changes (from say the last
@@ -153,7 +153,7 @@ impl<'a> System<'a> for Sys {
                     } else {
                         MY_HP_SHOWTIME
                     }
-                    || last_hp.abs() < f32::EPSILON
+                    || last_hp.abs() < Health::HEALTH_EPSILON
             }) {
                 floaters.clear();
             }

voxygen/src/hud/buffs.rs

@@ -127,7 +127,8 @@ impl<'a> Widget for BuffsBar<'a> {
         .desc_text_color(TEXT_COLOR);
         if let BuffPosition::Bar = buff_position {
             let decayed_health = 1.0 - self.health.maximum() / self.health.base_max();
-            let show_health = (self.health.current() - self.health.maximum()).abs() > f32::EPSILON
+            let show_health = (self.health.current() - self.health.maximum()).abs()
+                > Health::HEALTH_EPSILON
                 || decayed_health > 0.0;
             let show_energy = self.energy.current() != self.energy.maximum();
             let offset = if show_energy && show_health {

voxygen/src/hud/mod.rs

@@ -1217,9 +1217,8 @@ impl Hud {
                         );
                         // Calculate total change
                         // Ignores healing
-                        let hp_damage = floaters
+                        let hp_damage: f32 = floaters.iter().map(|f| f.hp_change.min(0.0)).sum();
-                            .iter()
+                        // .fold(0.0, |acc, f| f.hp_change.min(0.0) + acc);
-                            .fold(0.0, |acc, f| f.hp_change.min(0.0) + acc);
                         let hp_dmg_rounded_abs = hp_damage.round().abs() as u32;
                         let max_hp_frac = hp_damage.abs() as f32 / health.maximum() as f32;
                         let timer = floaters

voxygen/src/hud/overhead.rs

@@ -67,7 +67,7 @@ pub struct Info<'a> {
 
 /// Determines whether to show the healthbar
 pub fn should_show_healthbar(health: &Health) -> bool {
-    (health.current() - health.maximum()).abs() > f32::EPSILON
+    (health.current() - health.maximum()).abs() > Health::HEALTH_EPSILON
         || health.current() < health.base_max()
 }
 /// Determines if there is decayed health being applied

voxygen/src/hud/skillbar.rs

@@ -377,7 +377,8 @@ impl<'a> Skillbar<'a> {
         let hp_ani = (self.pulse * 4.0/* speed factor */).cos() * 0.5 + 0.8;
         let crit_hp_color: Color = Color::Rgba(0.79, 0.19, 0.17, hp_ani);
         let bar_values = self.global_state.settings.interface.bar_numbers;
-        let show_health = (self.health.current() - self.health.maximum()).abs() > f32::EPSILON;
+        let show_health =
+            (self.health.current() - self.health.maximum()).abs() > Health::HEALTH_EPSILON;
         let show_energy = self.energy.current() != self.energy.maximum();
         let decayed_health = 1.0 - self.health.maximum() as f64 / self.health.base_max() as f64;