Merge branch 'crabman/unstack-buffs' into 'master'

Don't stack buffs of the same kind with equal attributes

See merge request veloren/veloren!4251

CHANGELOG.md

@@ -96,6 +96,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Fixed Perforate icon not displaying
 - Make cave entrances easier to follow
 - Renamed Twiggy Shoulders to match the Twig Armor set 
+- No longer stack buffs of the same kind with equal attributes, this could lead to a DoS if ie. an entity stayed long enough in lava.
 
 ## [0.15.0] - 2023-07-01
 

common/src/combat.rs

@@ -791,7 +791,7 @@ impl AttackDamage {
     }
 }
 
-#[derive(Clone, Debug, Serialize, Deserialize)]
+#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
 pub struct AttackEffect {
     target: Option<GroupTarget>,
     effect: CombatEffect,
@@ -890,7 +890,7 @@ impl CombatEffect {
     }
 }
 
-#[derive(Clone, Debug, Serialize, Deserialize)]
+#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
 pub enum CombatRequirement {
     AnyDamage,
     Energy(f32),
@@ -898,7 +898,7 @@ pub enum CombatRequirement {
     TargetHasBuff(BuffKind),
 }
 
-#[derive(Clone, Debug, Serialize, Deserialize)]
+#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
 pub enum DamagedEffect {
     Combo(i32),
 }

common/src/comp/buff.rs

@@ -494,9 +494,6 @@ pub struct BuffData {
     pub strength: f32,
     pub duration: Option<Secs>,
     pub delay: Option<Secs>,
-    /// Force the buff effects to be applied each tick, ignoring num_ticks
-    #[serde(default)]
-    pub force_immediate: bool,
     /// Used for buffs that have rider buffs (e.g. Flame, Frigid)
     pub secondary_duration: Option<Secs>,
     /// Used to add random data to buffs if needed (e.g. polymorphed)
@@ -513,7 +510,6 @@ impl BuffData {
         Self {
             strength,
             duration,
-            force_immediate: false,
             delay: None,
             secondary_duration: None,
             misc_data: None,
@@ -530,12 +526,6 @@ impl BuffData {
         self
     }
 
-    /// Force the buff effects to be applied each tick, ignoring num_ticks
-    pub fn with_force_immediate(mut self, force_immediate: bool) -> Self {
-        self.force_immediate = force_immediate;
-        self
-    }
-
     pub fn with_misc_data(mut self, misc_data: MiscBuffData) -> Self {
         self.misc_data = Some(misc_data);
         self
@@ -558,14 +548,14 @@ pub enum BuffCategory {
     SelfBuff,
 }
 
-#[derive(Clone, Debug, Serialize, Deserialize)]
+#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
 pub enum ModifierKind {
     Additive,
     Multiplicative,
 }
 
 /// Data indicating and configuring behaviour of a de/buff.
-#[derive(Clone, Debug, Serialize, Deserialize)]
+#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
 pub enum BuffEffect {
     /// Periodically damages or heals entity
     HealthChangeOverTime {
@@ -816,11 +806,44 @@ impl Buffs {
 
     pub fn insert(&mut self, buff: Buff, current_time: Time) -> BuffKey {
         let kind = buff.kind;
-        let key = self.buffs.insert(buff);
-        self.kinds[kind]
-            .get_or_insert_with(|| (Vec::new(), current_time))
-            .0
-            .push(key);
+        // Try to find another overlaping non-queueable buff with same data, cat_ids and
+        // source.
+        let other_key = if kind.queues() {
+            None
+        } else {
+            self.kinds[kind].as_ref().and_then(|(keys, _)| {
+                keys.iter()
+                    .find(|key| {
+                        self.buffs.get(**key).map_or(false, |other_buff| {
+                            other_buff.data == buff.data
+                                && other_buff.cat_ids == buff.cat_ids
+                                && other_buff.source == buff.source
+                                && other_buff
+                                    .end_time
+                                    .map_or(true, |end_time| end_time.0 >= buff.start_time.0)
+                        })
+                    })
+                    .copied()
+            })
+        };
+
+        // If another buff with the same fields is found, update end_time and effects
+        let key = if let Some((other_buff, key)) =
+            other_key.and_then(|key| Some((self.buffs.get_mut(key)?, key)))
+        {
+            other_buff.end_time = buff.end_time;
+            other_buff.effects = buff.effects;
+            key
+        // Otherwise, insert a new buff
+        } else {
+            let key = self.buffs.insert(buff);
+            self.kinds[kind]
+                .get_or_insert_with(|| (Vec::new(), current_time))
+                .0
+                .push(key);
+            key
+        };
+
         self.sort_kind(kind);
         if kind.queues() {
             self.delay_queueable_buffs(kind, current_time);

common/systems/src/buff.rs

@@ -161,7 +161,7 @@ impl<'a> System<'a> for Sys {
                         entity,
                         buff_change: BuffChange::Add(Buff::new(
                             BuffKind::Bleeding,
-                            BuffData::new(1.0, Some(Secs(6.0))).with_force_immediate(true),
+                            BuffData::new(1.0, Some(Secs(6.0))),
                             Vec::new(),
                             BuffSource::World,
                             *read_data.time,
@@ -179,7 +179,7 @@ impl<'a> System<'a> for Sys {
                         entity,
                         buff_change: BuffChange::Add(Buff::new(
                             BuffKind::Bleeding,
-                            BuffData::new(5.0, Some(Secs(3.0))).with_force_immediate(true),
+                            BuffData::new(5.0, Some(Secs(3.0))),
                             Vec::new(),
                             BuffSource::World,
                             *read_data.time,
@@ -215,7 +215,7 @@ impl<'a> System<'a> for Sys {
                         entity,
                         buff_change: BuffChange::Add(Buff::new(
                             BuffKind::Bleeding,
-                            BuffData::new(15.0, Some(Secs(0.1))).with_force_immediate(true),
+                            BuffData::new(15.0, Some(Secs(0.1))),
                             Vec::new(),
                             BuffSource::World,
                             *read_data.time,
@@ -420,7 +420,6 @@ impl<'a> System<'a> for Sys {
                             execute_effect(
                                 effect,
                                 buff.kind,
-                                &buff.data,
                                 buff.start_time,
                                 kind_start_time,
                                 &read_data,
@@ -478,7 +477,6 @@ impl<'a> System<'a> for Sys {
 fn execute_effect(
     effect: &BuffEffect,
     buff_kind: BuffKind,
-    buff_data: &BuffData,
     buff_start_time: Time,
     buff_kind_start_time: Time,
     read_data: &ReadData,
@@ -516,9 +514,7 @@ fn execute_effect(
         let prev_tick = ((time_passed - dt).max(0.0) / tick_dur.0).floor();
         let whole_ticks = curr_tick - prev_tick;
 
-        if buff_data.force_immediate {
-            Some((1.0 / tick_dur.0 * dt) as f32)
-        } else if buff_will_expire {
+        if buff_will_expire {
             // If the buff is ending, include the fraction of progress towards the next
             // tick.
             let fractional_tick = (time_passed % tick_dur.0) / tick_dur.0;