Addressed more comments. Changed how buffs were sorted so that duration was also taken into account.

common/src/comp/buff.rs

@@ -2,7 +2,7 @@ use crate::sync::Uid;
 use serde::{Deserialize, Serialize};
 use specs::{Component, FlaggedStorage};
 use specs_idvs::IdvStorage;
-use std::{collections::HashMap, time::Duration};
+use std::{cmp::Ordering, collections::HashMap, time::Duration};
 
 /// De/buff Kind.
 /// This is used to determine what effects a buff will have, as well as
@@ -132,7 +132,7 @@ impl Buff {
             ),
             BuffKind::Cursed => (
                 vec![BuffEffect::MaxHealthModifier {
-                    value: -100.,
+                    value: -100. * data.strength,
                     kind: ModifierKind::Additive,
                 }],
                 data.duration,
@@ -149,6 +149,34 @@ impl Buff {
     }
 }
 
+impl PartialOrd for Buff {
+    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+        if self.data.strength > other.data.strength {
+            Some(Ordering::Greater)
+        } else if self.data.strength < other.data.strength {
+            Some(Ordering::Less)
+        } else if compare_duration(self.time, other.time) {
+            Some(Ordering::Greater)
+        } else if compare_duration(other.time, self.time) {
+            Some(Ordering::Less)
+        } else if self == other {
+            Some(Ordering::Equal)
+        } else {
+            None
+        }
+    }
+}
+
+fn compare_duration(a: Option<Duration>, b: Option<Duration>) -> bool {
+    a.map_or(true, |dur_a| b.map_or(false, |dur_b| dur_a > dur_b))
+}
+
+impl PartialEq for Buff {
+    fn eq(&self, other: &Self) -> bool {
+        self.data.strength == other.data.strength || self.time == other.time
+    }
+}
+
 /// Source of the de/buff
 #[derive(Clone, Copy, PartialEq, Debug, Serialize, Deserialize)]
 pub enum BuffSource {
@@ -190,17 +218,13 @@ pub struct Buffs {
 impl Buffs {
     fn sort_kind(&mut self, kind: BuffKind) {
         if let Some(buff_order) = self.kinds.get_mut(&kind) {
-            if buff_order.len() == 0 {
+            if buff_order.is_empty() {
                 self.kinds.remove(&kind);
             } else {
                 let buffs = &self.buffs;
-                buff_order.sort_by(|a, b| {
+                // Intentionally sorted in reverse so that the strongest buffs are earlier in
-                    buffs[&b]
+                // the vector
-                        .data
+                buff_order.sort_by(|a, b| buffs[&b].partial_cmp(&buffs[&a]).unwrap());
-                        .strength
-                        .partial_cmp(&buffs[&a].data.strength)
-                        .unwrap()
-                });
             }
         }
     }
@@ -233,7 +257,7 @@ impl Buffs {
         self.kinds
             .get(&kind)
             .map(|ids| ids.iter())
-            .unwrap_or((&[]).iter())
+            .unwrap_or_else(|| (&[]).iter())
             .map(move |id| (*id, &self.buffs[id]))
     }
 

common/src/event.rs

@@ -107,7 +107,7 @@ pub enum ServerEvent {
     /// Send a chat message to the player from an npc or other player
     Chat(comp::UnresolvedChatMsg),
     Buff {
-        uid: Uid,
+        entity: EcsEntity,
         buff_change: comp::BuffChange,
     },
 }

common/src/sys/buff.rs

@@ -7,13 +7,14 @@ use crate::{
     state::DeltaTime,
     sync::Uid,
 };
-use specs::{Join, Read, ReadStorage, System, WriteStorage};
+use specs::{Entities, Join, Read, ReadStorage, System, WriteStorage};
 use std::time::Duration;
 
 pub struct Sys;
 impl<'a> System<'a> for Sys {
     #[allow(clippy::type_complexity)]
     type SystemData = (
+        Entities<'a>,
         Read<'a, DeltaTime>,
         Read<'a, EventBus<ServerEvent>>,
         ReadStorage<'a, Uid>,
@@ -21,11 +22,11 @@ impl<'a> System<'a> for Sys {
         WriteStorage<'a, Buffs>,
     );
 
-    fn run(&mut self, (dt, server_bus, uids, mut stats, mut buffs): Self::SystemData) {
+    fn run(&mut self, (entities, dt, server_bus, uids, mut stats, mut buffs): Self::SystemData) {
         let mut server_emitter = server_bus.emitter();
         // Set to false to avoid spamming server
-        // buffs.set_event_emission(false);
+        buffs.set_event_emission(false);
-        for (buff_comp, uid, stat) in (&mut buffs, &uids, &mut stats).join() {
+        for (entity, buff_comp, uid, stat) in (&entities, &mut buffs, &uids, &mut stats).join() {
             let mut expired_buffs = Vec::<BuffId>::new();
             for (id, buff) in buff_comp.buffs.iter_mut() {
                 // Tick the buff and subtract delta from it
@@ -63,7 +64,6 @@ impl<'a> System<'a> for Sys {
                     // Now, execute the buff, based on it's delta
                     for effect in &mut buff.effects {
                         match effect {
-                            // Only add an effect here if it is continuous or it is not immediate
                             BuffEffect::HealthChangeOverTime { rate, accumulated } => {
                                 *accumulated += *rate * dt.0;
                                 // Apply damage only once a second (with a minimum of 1 damage), or
@@ -106,7 +106,7 @@ impl<'a> System<'a> for Sys {
             // Remove buffs that expire
             if !expired_buffs.is_empty() {
                 server_emitter.emit(ServerEvent::Buff {
-                    uid: *uid,
+                    entity,
                     buff_change: BuffChange::RemoveById(expired_buffs),
                 });
             }
@@ -114,7 +114,7 @@ impl<'a> System<'a> for Sys {
             // Remove stats that don't persist on death
             if stat.is_dead {
                 server_emitter.emit(ServerEvent::Buff {
-                    uid: *uid,
+                    entity,
                     buff_change: BuffChange::RemoveByCategory {
                         all_required: vec![],
                         any_required: vec![],
@@ -123,6 +123,6 @@ impl<'a> System<'a> for Sys {
                 });
             }
         }
-        // buffs.set_event_emission(true);
+        buffs.set_event_emission(true);
     }
 }

common/src/sys/combat.rs

@@ -153,25 +153,12 @@ impl<'a> System<'a> for Sys {
                             },
                         });
 
-                        use buff::*;
-                        server_emitter.emit(ServerEvent::Buff {
-                            uid: *uid_b,
-                            buff_change: BuffChange::Add(Buff::new(
-                                BuffKind::Cursed,
-                                BuffData {
-                                    strength: attack.base_damage as f32 / 10.0,
-                                    duration: Some(Duration::from_secs(10)),
-                                },
-                                vec![BuffCategory::Physical],
-                                BuffSource::Character { by: *uid },
-                            )),
-                        });
-
                         // Apply bleeding buff on melee hits with 10% chance
                         // TODO: Don't have buff uniformly applied on all melee attacks
                         if thread_rng().gen::<f32>() < 0.1 {
+                            use buff::*;
                             server_emitter.emit(ServerEvent::Buff {
-                                uid: *uid_b,
+                                entity: b,
                                 buff_change: BuffChange::Add(Buff::new(
                                     BuffKind::Bleeding,
                                     BuffData {

common/src/sys/controller.rs

@@ -51,7 +51,7 @@ impl<'a> System<'a> for Sys {
         span!(_guard, "run", "controller::Sys::run");
         let mut server_emitter = server_bus.emitter();
 
-        for (entity, uid, controller, character_state) in
+        for (entity, _uid, controller, character_state) in
             (&entities, &uids, &mut controllers, &mut character_states).join()
         {
             let mut inputs = &mut controller.inputs;
@@ -85,7 +85,7 @@ impl<'a> System<'a> for Sys {
                     },
                     ControlEvent::RemoveBuff(buff_id) => {
                         server_emitter.emit(ServerEvent::Buff {
-                            uid: *uid,
+                            entity,
                             buff_change: BuffChange::RemoveFromController(buff_id),
                         });
                     },

server/src/events/entity_manipulation.rs

@@ -699,66 +699,64 @@ pub fn handle_level_up(server: &mut Server, entity: EcsEntity, new_level: u32) {
         ));
 }
 
-pub fn handle_buff(server: &mut Server, uid: Uid, buff_change: buff::BuffChange) {
+pub fn handle_buff(server: &mut Server, entity: EcsEntity, buff_change: buff::BuffChange) {
     let ecs = &server.state.ecs();
     let mut buffs_all = ecs.write_storage::<comp::Buffs>();
-    if let Some(entity) = ecs.entity_from_uid(uid.into()) {
+    if let Some(buffs) = buffs_all.get_mut(entity) {
-        if let Some(buffs) = buffs_all.get_mut(entity) {
+        use buff::BuffChange;
-            use buff::BuffChange;
+        match buff_change {
-            match buff_change {
+            BuffChange::Add(new_buff) => {
-                BuffChange::Add(new_buff) => {
+                buffs.insert(new_buff);
-                    buffs.insert(new_buff);
+            },
-                },
+            BuffChange::RemoveById(ids) => {
-                BuffChange::RemoveById(ids) => {
+                for id in ids {
-                    for id in ids {
+                    buffs.remove(id);
-                        buffs.remove(id);
+                }
-                    }
+            },
-                },
+            BuffChange::RemoveByKind(kind) => {
-                BuffChange::RemoveByKind(kind) => {
+                buffs.remove_kind(kind);
+            },
+            BuffChange::RemoveFromController(kind) => {
+                if kind.is_buff() {
                     buffs.remove_kind(kind);
-                },
+                }
-                BuffChange::RemoveFromController(kind) => {
+            },
-                    if kind.is_buff() {
+            BuffChange::RemoveByCategory {
-                        buffs.remove_kind(kind);
+                all_required,
-                    }
+                any_required,
-                },
+                none_required,
-                BuffChange::RemoveByCategory {
+            } => {
-                    all_required,
+                let mut ids_to_remove = Vec::new();
-                    any_required,
+                for (id, buff) in buffs.buffs.iter() {
-                    none_required,
+                    let mut required_met = true;
-                } => {
+                    for required in &all_required {
-                    let mut ids_to_remove = Vec::new();
+                        if !buff.cat_ids.iter().any(|cat| cat == required) {
-                    for (id, buff) in buffs.buffs.iter() {
+                            required_met = false;
-                        let mut required_met = true;
+                            break;
-                        for required in &all_required {
-                            if !buff.cat_ids.iter().any(|cat| cat == required) {
-                                required_met = false;
-                                break;
-                            }
-                        }
-                        let mut any_met = any_required.is_empty();
-                        for any in &any_required {
-                            if buff.cat_ids.iter().any(|cat| cat == any) {
-                                any_met = true;
-                                break;
-                            }
-                        }
-                        let mut none_met = true;
-                        for none in &none_required {
-                            if buff.cat_ids.iter().any(|cat| cat == none) {
-                                none_met = false;
-                                break;
-                            }
-                        }
-                        if required_met && any_met && none_met {
-                            ids_to_remove.push(*id);
                         }
                     }
-                    for id in ids_to_remove {
+                    let mut any_met = any_required.is_empty();
-                        buffs.remove(id);
+                    for any in &any_required {
+                        if buff.cat_ids.iter().any(|cat| cat == any) {
+                            any_met = true;
+                            break;
+                        }
                     }
-                },
+                    let mut none_met = true;
-            }
+                    for none in &none_required {
+                        if buff.cat_ids.iter().any(|cat| cat == none) {
+                            none_met = false;
+                            break;
+                        }
+                    }
+                    if required_met && any_met && none_met {
+                        ids_to_remove.push(*id);
+                    }
+                }
+                for id in ids_to_remove {
+                    buffs.remove(id);
+                }
+            },
         }
     }
 }

server/src/events/mod.rs

@@ -133,7 +133,10 @@ impl Server {
                 ServerEvent::Chat(msg) => {
                     chat_messages.push(msg);
                 },
-                ServerEvent::Buff { uid, buff_change } => handle_buff(self, uid, buff_change),
+                ServerEvent::Buff {
+                    entity,
+                    buff_change,
+                } => handle_buff(self, entity, buff_change),
             }
         }