Feedback request: DotSpellHandler
PostPosted: Sun Apr 16, 2017 9:39 am
During my work on CL abilities i noticed some incorrect behaviour of DOT stacking and found that the algorithm responsible was this method from DoTSpellHandler.cs: DoTSpellHandler.IsOverwritable()
Testing on the live server came up with groups of DOTs wich will stack with each other but overwrite members of the same group:
group 1)
Sorceress Base Dot // Cabalist Base Dot // Champ Single Dot (Mid/Hib) // Bonedancer Base Dot // Shaman Base Dot // Mentalist Base Dot //
Item Charges n Procs
group 2)
Sorceress Spec Dot // Cabalist Spec Dot // Bonedancer Spec Dot // Mentalist Spec Dot
group 3)
Wizard Spec Dot // Shaman Spec Dot // Warlock Spec Dot // Druid Base Dot // Shadowblade Nightshade Infiltrator Poison
group 4)
Necro Spec Dot // Champ Ae Dot // Champ Single Dot (Alb)
group 5)
Essence Dot
group 6)
Bleed Effects
As you can see some groups contain different damage types and other irregularities such as Champ Single DOT (Alb) being in a completely different group than it's Hib/Mid counterparts. The test therefore entirely invalidates the current logic for DOT stacking.
I am aware that the naming and encapsulation of the logic is a bit irritating in the current Rev. the method should be renamed to HasEffect() as Tolakram suggested in a different post. Some reencapsulation might be in order in the code surrounding the call. The logic i would have expected from IsOverwritable() is something entirely different and depending on feedback may end up handling the question if a player can actually overwrite another player's/GameLiving's DOTs if it already has that same effect. The desire to stick to the design of the parent class as close as possible could make this a difficult consideration.
Now, i have a proposed code change that is actually working fine to the extend that i expect it to but i would like to ask for feedback on some issues:
3) I have read unsubstantiated claims on the net that DOTs can only be overwritten by their owners and will get resisted if the target already has the same effect from a different source. Can anyone provide tests and insight into that ?
2) If you have any other mechanical information about DOTs and DOT stacking be my guest.
3) You might think this could be modelled testing the spells assoctiated spellLine instead of using the EffectGroup but two things prevent that. CL single and AE DOTs are in the same spellLine but split up into different stacking groups. Mobs and NPCs might not even have an associated spellLine for their spells.
4) While i think this logic gives us the finest possible control over DOT stacking it also comes with the caveat of breaking a servers DOT stacking completely until all DOTs define an EffectGroup value. Is that asking too much of the server admins ? We plan on releasing the respective data sets with a future public database but some server owners probably won't be able to do it themselves. Should i wait on committing code changes until the Public DB is ready to support it ?
- Code: Select all
/// <summary> /// Determines wether this spell is compatible with given spell /// and therefore overwritable by better versions /// spells that are overwritable cannot stack /// </summary> /// <param name="compare"></param> /// <returns></returns> public override bool IsOverwritable(GameSpellEffect compare) { return Spell.SpellType == compare.Spell.SpellType && Spell.DamageType == compare.Spell.DamageType && SpellLine.IsBaseLine == compare.SpellHandler.SpellLine.IsBaseLine; }
Testing on the live server came up with groups of DOTs wich will stack with each other but overwrite members of the same group:
group 1)
Sorceress Base Dot // Cabalist Base Dot // Champ Single Dot (Mid/Hib) // Bonedancer Base Dot // Shaman Base Dot // Mentalist Base Dot //
Item Charges n Procs
group 2)
Sorceress Spec Dot // Cabalist Spec Dot // Bonedancer Spec Dot // Mentalist Spec Dot
group 3)
Wizard Spec Dot // Shaman Spec Dot // Warlock Spec Dot // Druid Base Dot // Shadowblade Nightshade Infiltrator Poison
group 4)
Necro Spec Dot // Champ Ae Dot // Champ Single Dot (Alb)
group 5)
Essence Dot
group 6)
Bleed Effects
As you can see some groups contain different damage types and other irregularities such as Champ Single DOT (Alb) being in a completely different group than it's Hib/Mid counterparts. The test therefore entirely invalidates the current logic for DOT stacking.
I am aware that the naming and encapsulation of the logic is a bit irritating in the current Rev. the method should be renamed to HasEffect() as Tolakram suggested in a different post. Some reencapsulation might be in order in the code surrounding the call. The logic i would have expected from IsOverwritable() is something entirely different and depending on feedback may end up handling the question if a player can actually overwrite another player's/GameLiving's DOTs if it already has that same effect. The desire to stick to the design of the parent class as close as possible could make this a difficult consideration.
Now, i have a proposed code change that is actually working fine to the extend that i expect it to but i would like to ask for feedback on some issues:
- Code: Select all
public override bool IsOverwritable(GameSpellEffect compare) { // If the spell to compare is not a DOT we don't overwrite it if (compare.Spell.SpellType != this.Spell.SpellType) return false; // Always allow DOT overwriting if EffectGroup is not defined explicitly in the Database if (compare.Spell.EffectGroup <= 0 || this.Spell.EffectGroup <= 0) return true; // DOTs of different EffectGroup can stack. Same group can be overwritten. return compare.Spell.EffectGroup == this.Spell.EffectGroup; }
3) I have read unsubstantiated claims on the net that DOTs can only be overwritten by their owners and will get resisted if the target already has the same effect from a different source. Can anyone provide tests and insight into that ?
2) If you have any other mechanical information about DOTs and DOT stacking be my guest.
3) You might think this could be modelled testing the spells assoctiated spellLine instead of using the EffectGroup but two things prevent that. CL single and AE DOTs are in the same spellLine but split up into different stacking groups. Mobs and NPCs might not even have an associated spellLine for their spells.
4) While i think this logic gives us the finest possible control over DOT stacking it also comes with the caveat of breaking a servers DOT stacking completely until all DOTs define an EffectGroup value. Is that asking too much of the server admins ? We plan on releasing the respective data sets with a future public database but some server owners probably won't be able to do it themselves. Should i wait on committing code changes until the Public DB is ready to support it ?
