How do I simplify this if-else purgatory?

I'm trying to work up a generic method which I can use to manage all of my buffs: temporary status effects, permanent status effects, and effects over time (like DoTs or trickle heals). To this end, I have a base class StatusEffect, which has two public virtual functions: Apply, and Remove. Apply adds the specified effect to the player's list of effects, while Remove removes the effect from the list.

Each buff has its own class, which extends StatusEffect and uses a public override void of the Apply and Remove classes to add stat changes, which is the central utility I need. This works perfectly for passive effects ("Strength + 5 until buff wears off"), but where I'm running into trouble is with active effects ("Lose 1HP every second"). I initially tried creating a bool activeEffect to indicate whether the buff's effect should be applied once or every tick, but since Apply() includes a line that adds the effect to a list of effects, this produced an infinite loop. The way I've gotten around this is by using a separate version of Apply(), called Proc(), which contains all of Apply() except for the invocation of base.Apply() that adds the effect to the list. This works as desired, but it's messy and full of conditionals and re-used code, which is bugging the heck out of me- is there a massively simpler way to refactor this that I'm not hitting on?

Base Effect class:

Code (csharp):

1.  
2. public int duration; //Measured in in-game minutes
3.  
4. public bool permanent;
5. public bool activeEffect = false;
6.  
7. public virtual void Apply(Character target){
8.      if (activeEffect) {
9.        target.activeEffects.Add(this);
10.      }
11.      else{
12.        target.passiveEffects.Add(this);
13.      }
14. }
15.  
16.      public virtual void Proc(Character target){
17.      }
18.  
19. public virtual void Remove (Character target){
20.  
21.      if (activeEffect) {
22.        target.activeEffects.Remove(this);
23.      }
24.      else{
25.        target.passiveEffects.Remove(this);
26.      }
27.    }
28. }
29.  

Extension in a specific buff (Heal.cs, Poison.cs, etc)

Code (csharp):

1.  public override void Apply(Character target){
2.          base.Apply (target);
3.          //Apply effects AND add to list
4.        }
5.  
6.        public override void Remove(Character target){
7.          base.Remove (target);
8.          //remove target from list, and remove effects
9.        }
10.  
11.        public override void Proc(Character target){
12.        //Apply effects, but do not add to list
13.        }
14.  

Processing active and passive buffs each tick:

Code (csharp):

1.  
2.   void ApplyBuffs(Character character){
3.      //Iterate through buffs/debuffs, and apply them
4.      for (int i = 0; i < character.passiveEffects.Count; i++) {   //Iterate through list of passive effects, decrement timer, and remove effects that have expired
5.        if (character.passiveEffects[i].permanent == false && character.passiveEffects[i].duration > 0)
6.          character.passiveEffects[i].duration -= 1;
7.        else
8.          character.passiveEffects[i].Remove(character);
9.      }
10.    }
11.  
12.    void ApplyActiveEffects(Character character){
13.      //Apply active effects
14.      for (int i = 0; i < character.activeEffects.Count; i++) {
15.        character.activeEffects[i].Proc(character);
16.      }
17.    
18.    }
19.  

 

Have you tried using events handlers?

Code (CSharp):

1. using UnityEngine;
2.  
3. public class Character
4. {
5.     public delegate void UpdateBuffs(Character character);
6.     public event UpdateBuffs OnUpdateBuff;
7.  
8.     void UpdateCharacter()
9.     {
10.         OnUpdateBuff(this);
11.     }
12. }
13.  
14. public class Buff
15. {
16.     public void Apply(Character character)
17.     {
18.         character.OnUpdateBuff += UpdateBuff;
19.     }
20.  
21.     public void Remove(Character character)
22.     {
23.         character.OnUpdateBuff -= UpdateBuff;
24.     }
25.  
26.     public void UpdateBuff(Character character)
27.     {
28.         //  Apply the tick effect to the character here.
29.     }
30. }

That way, if your buff is not tick-orientented, you dont need to add its updatebuff fuction the the OnUpdateBuff event of the character. Or do it as you like, you can use multiple events, one for permanent buffs, other for tick buffs

Take a look at this : https://unity3d.com/pt/learn/tutorials/modules/intermediate/scripting/events

 

Oh this is great, I didn't even know that events were a thing, thank you! If you don't mind two quick clarifications, though, I have one silly question and one logical problem: first off, does using events here remove the need to curate a list for active buffs? It appears to, as everything listening for the UpdateBuffs event means I don't need to iterate through my list. However, if that's the case, where does each Buff "live"? Most of the time, I'm not applying a permanent class, I'm doing something like running a function that reads

Code (csharp):

1.  
2. PoisonSpell(Character character){
3. Buff poisonBuff = new Buff();
4. //set all of poisonBuff's desired properties here
5. character.ActiveEffects.Add(poisonBuff);
6. }

That being the case, wouldn't I want a second line in the Add and Remove functions in the base class in order to curate the list of buffs?

Second, I'm struggling a bit to figure out the most logical way to organize my events: there are three classes that need to interact here, a Timer class (whose only job is to keep time), a Simulator class (whose job is to know what time it is, and curate the character and NPC data), and a whole boatload of Character classes that belong to the player and NPCs, and that need to be edited by Simulator. Am I correct that the most logical way to approach this would be to declare a MinuteTick event in Timer, listen for that in Simulator, and declare a second event, UpdateBuff, that gets run by Simulator and listened for by each individual buff?

 

I think you want something like this.

Code (CSharp):

1. using UnityEngine;
2. using System.Collections;
3. using System.Collections.Generic;
4.  
5. public class Simulator : MonoBehaviour
6. {
7.  
8.     public IList<Character> characters = new List<Character>();     //  A list of all the characters in your game.
9.     public delegate void UpdateSimulator(Simulator simulator);
10.     public event UpdateSimulator OnSimulatorUpdate;
11.  
12.     // Update is called once per frame
13.     void Update()
14.     {
15.         OnSimulatorUpdate(this);
16.         //  Additional simulator update goes here.
17.     }
18. }
19.  
20. public class Character
21. {
22.     public IList<IBuff> buffs = new List<IBuff>();        //  A list of the buffs the character has. You want to keep track of them to know whether or not a character has a specified buff.
23.     public delegate void CharacterUpdate(Character character);
24.     public event CharacterUpdate OncharacterUpdate;
25.  
26.     public void Add(Simulator simulator)
27.     {
28.         simulator.characters.Add(this);
29.         simulator.OnSimulatorUpdate += UpdateCharacter;
30.     }
31.  
32.     public void Remove(Simulator simulator)
33.     {
34.         simulator.characters.Remove(this);
35.         simulator.OnSimulatorUpdate -= UpdateCharacter;
36.     }
37.  
38.     public void UpdateCharacter(Simulator simulator)
39.     {
40.         OncharacterUpdate(this);
41.         //  Additional individual character updates would go here.
42.     }
43.  
44. }
45.  
46. public interface IBuff
47. {
48.     public string name;
49.     public Texture2D icon;
50.  
51.     public void Add(Character target);
52.  
53.     public void Remove(Character target);
54.  
55.     public void UpdateBuff(Character target);
56. }
57.  
58. public class PermanentBuff : IBuff
59. {
60.     public string name;
61.     public Texture2D icon;
62.  
63.     public void Add(Character target)
64.     {
65.         target.buffs.Add(this);
66.         target.OncharacterUpdate += UpdateBuff;     //  Permanent buffs dont really need this, but they could have just in case you want to change something in them over time. Example, a permanent buff that heals the character by a % of his life every second. It is permanent and wont be removed ever, but has effects over time.
67.         //  Apply permanent buff effects here.
68.     }
69.  
70.     public void Remove(Character target)
71.     {
72.         target.buffs.Remove(this);
73.         target.OncharacterUpdate -= UpdateBuff;
74.         //  Remove permanent buff effects here.
75.     }
76.  
77.     public void UpdateBuff(Character target)
78.     {
79.     }
80. }
81.  
82. public class TemporaryBuff : IBuff
83. {
84.     public string name;
85.     public Texture2D icon;
86.     public float duration;      //  This is a unique value for the temporary buffs
87.  
88.     public void Add(Character target)
89.     {
90.         target.buffs.Add(this);
91.         target.OncharacterUpdate += UpdateBuff;
92.         //  Maybe the temporary buff has some special effect upon activation?
93.     }
94.  
95.     public void Remove(Character target)
96.     {
97.         target.buffs.Remove(this);
98.         target.OncharacterUpdate -= UpdateBuff;
99.         //  Maybe the temporary buff has some special effect upon deactivation?
100.     }
101.  
102.     public void UpdateBuff(Character target)
103.     {
104.         UpdateDuration(target);
105.         //  Over tim effects go here.
106.     }
107.  
108.     private void UpdateDuration(Character target)   //  This is a unique method for the temporary buffs.
109.     {
110.         duration -= Time.deltaTime;
111.         if (duration < 0)
112.         {
113.             Remove(target);
114.         }
115.     }
116. }
117.  

You could also add more events to the simulator, such as OnGUI and assign characters OnGUI to those, i think unity supports that for GUI, not sure.

 

And for the spells :

Code (CSharp):

1. public interface IActiveSpell : ISpell
2. {
3.     public string name;
4.     public Texture2D icon;
5.  
6.     public void Activate(Character target);
7. }
8.  
9. public class PoisonSpell : IActiveSpell
10. {
11.     public string name;
12.     public Texture2D icon;
13.  
14.     public void Activate(Character target)
15.     {
16.         PoisonBuff b = new PoisonBuff();    //  You could pass the level of the buff and other information to it.
17.         b.Add(target);
18.     }
19. }
20.  
21. public class PoisonBuff : TemporaryBuff     //  In this case, Temporary buff could also be a interface.
22. {
23.     public string name;
24.     public Texture2D icon;
25.     public float duration;
26.  
27.     public void Add(Character target)
28.     {
29.         target.buffs.Add(this);
30.         target.OncharacterUpdate += UpdateBuff;
31.     }
32.  
33.     public void Remove(Character target)
34.     {
35.         target.buffs.Remove(this);
36.         target.OncharacterUpdate -= UpdateBuff;
37.     }
38.  
39.     public void UpdateBuff(Character target)
40.     {
41.         UpdateDuration(target);
42.         //  target.health -= 1.0f;
43.     }
44.  
45.     private void UpdateDuration(Character target)
46.     {
47.         duration -= Time.deltaTime;
48.         if (duration < 0)
49.         {
50.             Remove(target);
51.         }
52.     }
53. }

There are lots of ways of doing this, this one uses unique buffs and unique spell scripts for each spell or buff.
You could use generic buff scripts, and use events to these buffs and use the spell to apply the desired effect over time using the buff events, that way each spell would use a generic buff script and a unique spell script.

EDIT : Oh, you actually cant use fields on the interfaces, so you would need an abstract class for each interface i guess.

 

Holy cow, that's a heck of a piece of work Mathias, thank you! between this and your suggestion of looking at event handlers, I'm getting a pretty nifty system assembled: right now I'm handling the duration issue with inheritance, although now that I've seen this, I think that interfaces might be more efficient... the way I do it right now, the inheritance chain goes Buff > TimedBuff > <specific buff.cs>. The only difference between them is that TimedBuff overrides UpdateBuff with an extra three lines to check the duration and reduce the timer if it's >0, or call Buff.Remove(this) if it's expired. Buffs that proc once are added and removed in Apply/Remove, buffs that proc every tick do so in UpdateBuff. It's a tiny bit more work to create, but it's pretty fast.