What is wrong with this design of RPG game effect system or how may I remove all particular effects from a list of different lists of derived effects?

Question

I guess I've run into a design issue... which I'm not sure how to solve myself. So sorry, but I have to provide some basic design info...

A game. Players, monsters, objects, different things may be under different effects:

public interface IEffect {}

There may be different particular Effects:

public interface IResistanceBuff : IEffect
{
    void modifyIncomingDamage(Damage damage);
}

public interface IDamageBuff : IEffect
{
    void modifyOutgoingDamage(Damage damage);
}

Why this way? Well, the concept it to avoid duplicating code. For example, this way I can unify damage reductions from type resistances and, for example, wearing armor. Simply, I make the Type class and the Armor class implement IResistanceBuff. (Similarily, the Weapon class will implement IDamageBuff).

But some effects are temporary, not permanent. For this, we will have:

public interface ITemporaryEffect : IEffect
{
    long TimeRemaining {get; set;}
}

Again, the purpose here is to avoid duplicating code. For example, the PotionOfMight class will implement IDamageBuff interface and derive from Potion abstract class, which in turn will implement ITemporaryEffect interface. Or, we will have IStun interface, which will implement ITemporaryEffect.

Now every object that can be under certain effects will store a collection (HashSet maybe?) of all effects it is under. This is to be able to more easily call these effects. For example, a character will have a property public HashSet<IResistanceBuff> ResistanceBuffs {get; private set;} and so we can make a character's TakeDamage method look like this:

void TakeDamage(Damage damage)
{
    this.ResistanceBuffs.ForEach(resistanceBuff =>
        resistanceBuff.modifyIncomingDamage(damage)
    );

    this.HP -= damage.Amount;

    if(this.HP <= 0) {
        this.Faint();
    }
}

But here comes the problem. I want to have one piece of code to update the remaining duration of temporary effects and remove those whose duration has expired.

My (not working, by design of C#) idea for that was to make the Character class (and any other class that represents an object that can be under any sort of Effect - that means even WorldMap, because I thought I'd store weather conditions as Effects applied to the world) implement IEffectStorage interface:

public interface IEffectStorage
{
    List<HashSet<IEffect>> AllEffects {get;}
}

public class Character : IEffectStorage
{
    // ...
    public HashSet<IResistanceBuff> ResistanceBuffs {get; private set;}
    public HashSet<IDamageBuff> DamageBuffs {get; private set;}
    // ...
    public List<HashSet<IEffect>> AllEffects =>
        new List<HashSet<IEffect>> {ResistanceBuffs, DamageBuffs, /* ... */};
}

Now dealing with temporary effect expiration would be the responsibility of the Effect static class:

public static class Effect
{
    public static void ExpireTemporaryEffects(IEffectStorage effectStorage)
    {
        effectStorage.AllEffects.ForEach(effectSet =>
        {
            var effects = effectSet.ToList();
            effects.ForEach(effect =>
            {
                if(effect is ITemporaryEffect) {
                    effect.TimeRemaining--;
                    if(effect.TimeRemaining <= 0) {
                        effectSet.Remove(effect);
                    }
                }
            });
        });
    }
}

But, of course, as I said, I cannot do this.

Now I may start looking for some hackish and/or ugly ways to make this design working. ALtervatively, I may reason that I'm likely trying to do something against the design of the C# programming language and/or OO programming paradigm because of my failure to understand this paradigm well enough... Especially since this is the first project of this size I'm doing on my own, I choose to do the latter... So I'm asking this question :) Is my design all wrong and if yes, then how to fix it?

Sorry for the length of this post, but I feel all information above is necessary.

score 0 · Answer 1 · answered May 30 '18 at 06:50

I am not a game developer or have experience with game engines but I think most games or engines use something like frames as a meassure for time (maybe I am totally wrong on this). My Idea would be to change your ITemporaryEffect to

public interface ITemporaryEffect : IEffect
{
    bool IsActive(long currentFrame)
}

And implement it as follows:

public class SilencedEffect : ITemporaryEffect 
{
    private long duration = 1000;
    private long endFrame;

    public SilencedEffect(int currentFrame)
    {
        endFrame = currentFrame + duration;
    }

    public bool IsActive(long currentFrame)
    {
        return endFrame > currentFrame;
    }
}

This way you could iterate over it in your character or worldmap class and remove it if isn't is active anymore.

public class Character
{
    private IList<ITemporaryEffect> temporaryEffect;         

    public void UpdateEffects(long currentFrame)
    {
        var outdatedTempEffects = temporaryEffect.Where(p => !p.IsActive(currentFrame));
        temporaryEffects.RemoveRange(outdatedTempEffects);
    }
}

What is wrong with this design of RPG game effect system or how may I remove all particular effects from a list of different lists of derived effects?

1 Answers1