Trying to implement cross-class functionality that needs a field to operate in C#

Question

Crosspost from here, it was suggested that this is better place to pose my question.

---

I'm implementing a series of classes in a game that all derive from one base class that references the scripting of the object that it pertains. Most of these are objects that are generated by certain spells/abilities, and very frequently there is a lot of overlap between functionality each script needs.

Unfortunately, the functionality requires the use of some kind of field to use, whether it be a Dictionary or a couple of primitives. Normally I'd just apply an interface to each one, but because each "piece of functionality" requires a field to operate correctly that doesn't seem optimal. And adding every kind of multi-use functionality into the base class seems like it would make each object unnecessarily bloated.

Here's an example:

BlazingAura_Server

...
public BlazingAuraC_Server conditionInstance;
public Queue<KeyValuePair<int, float>> reapplyDelay = new Queue<KeyValuePair<int, float>>();

...
void Update()
{
    KeyValuePair<int, float> id;
    while (reapplyDelay.Count > 0)
    {
        if (reapplyDelay.Peek().Value - BlazingAuraC_Server.BURNING_REAPPLY_DELAY > conditionInstance.duration)
        {
            id = reapplyDelay.Dequeue();
        }
        else
        {
            break;
        }
    }
}

void OnTriggerEnter(Collider collider)
{
    if (collider.tag != "Spell")
    {
        EntityInfoServer collidedInfo = collider.transform.GetComponentInChildren<EntityInfoServer>();
        collidedInfo.QueueDamageRaw(conditionInstance, BlazingAuraC_Server.TICK_FREQ * BlazingAuraC_Server.BASE_DPS, ValueModDamageSubtype.Elemental);

        if (!reapplyDelay.Any(id => id.Key == collidedInfo.entityId))
        {
            reapplyDelay.Enqueue(new KeyValuePair<int, float>(collidedInfo.entityId, conditionInstance.duration));
            collider.transform.GetComponentInChildren<EntityInfoServer>().AddConditionServer(Condition.Burning, conditionInstance);
        }
    } 
}

And here's another example:

Storm_Server

...
public float duration;
public Queue<KeyValuePair<int, float>> reapplyDelay = new Queue<KeyValuePair<int, float>>();

...
void Update()
{
    duration += Time.deltaTime;

    if (Mathf.CeilToInt((duration + Time.deltaTime) / TICK_FREQ) > Mathf.CeilToInt(duration / TICK_FREQ))
    {
        Activate(stormCollider);
    }

    KeyValuePair<int, float> id;
    while (reapplyDelay.Count > 0)
    {
        if (reapplyDelay.Peek().Value - STORM_DRENCH_DELAY > duration)
        {
            id = reapplyDelay.Dequeue();
        }
        else
        {
            break;
        }
    }
}

public void OnTriggerEnter(Collider collider)
{
    if (collider.tag == "Player")
    {
        //Debug.Log("IN STORM");
        EntityInfoServer collidedInfo = collider.transform.GetComponentInChildren<EntityInfoServer>();

        if (!reapplyDelay.Any(id => id.Key == collidedInfo.entityId))
        {
            reapplyDelay.Enqueue(new KeyValuePair<int, float>(collidedInfo.entityId, duration));
            collider.transform.GetComponentInChildren<EntityInfoServer>().AddConditionServer(Condition.Drenched, stormInfo);
        }
    }
}

Basically, each class keeps track of entities that have collided with it and store it in a queue, and make sure that a condition doesn't get reapplied unless the entity has been dequeued after a certain duration. However, it requires the Queue to work correctly, but declaring a Queue and utilizing an interface in each class doesn't seem like very good abstraction and feels sloppy in general.

What are my best options for including functionality like this (i.e. having a reapply delay) across multiple classes?

Answer 1

I think the rule you should follow here is "favor composition over inheritance".

What that means in your case is that you create a separate class, something like ReapplyDelay and make both classes own an instance of that class.

The code could look something like this:

class ReapplyDelay
{
    private Queue<KeyValuePair<int, float>> reapplyDelay = new Queue<KeyValuePair<int, float>>();

    public void RemoveOld(float delay, float duration)
    {
        while (reapplyDelay.Count > 0)
        {
            if (reapplyDelay.Peek().Value - delay > duration)
            {
                reapplyDelay.Dequeue();
            }
            else
            {
                break;
            }
        }
    }

    public bool AddNew(EntityInfoServer collidedInfo, float duration)
    {
        if (!reapplyDelay.Any(id => id.Key == collidedInfo.entityId))
        {
            reapplyDelay.Enqueue(new KeyValuePair<int, float>(collidedInfo.entityId, duration));
            return true;
        }

        return false;
    }
}

And your two original classes would then look like this:

BlazingAura_Server:

private BlazingAuraC_Server conditionInstance;
private ReapplyDelay reapplyDelay = new ReapplyDelay();

void Update()
{
    reapplyDelay.RemoveOld(BlazingAuraC_Server.BURNING_REAPPLY_DELAY, conditionInstance.duration);
}

void OnTriggerEnter(Collider collider)
{
    if (collider.tag != "Spell")
    {
        EntityInfoServer collidedInfo = collider.transform.GetComponentInChildren<EntityInfoServer>();
        collidedInfo.QueueDamageRaw(conditionInstance, BlazingAuraC_Server.TICK_FREQ * BlazingAuraC_Server.BASE_DPS, ValueModDamageSubtype.Elemental);

        if (reapplyDelay.AddNew(collidedInfo, conditionInstance.duration))
        {
            collider.transform.GetComponentInChildren<EntityInfoServer>().AddConditionServer(Condition.Burning, conditionInstance);
        }
    } 
}

Storm_Server:

private float duration;
private ReapplyDelay reapplyDelay = new ReapplyDelay();

void Update()
{
    duration += Time.deltaTime;

    if (Mathf.CeilToInt((duration + Time.deltaTime) / TICK_FREQ) > Mathf.CeilToInt(duration / TICK_FREQ))
    {
        Activate(stormCollider);
    }

    reapplyDelay.RemoveOld(STORM_DRENCH_DELAY, duration);
}

public void OnTriggerEnter(Collider collider)
{
    if (collider.tag == "Player")
    {
        //Debug.Log("IN STORM");
        EntityInfoServer collidedInfo = collider.transform.GetComponentInChildren<EntityInfoServer>();

        if (reapplyDelay.AddNew(collidedInfo, duration))
        {
            collider.transform.GetComponentInChildren<EntityInfoServer>().AddConditionServer(Condition.Drenched, stormInfo);
        }
    }
}

You know your code better than I do, so the details might be different, but the general idea should be clear: extract common independent pieces of functionality into a separate class and then use that.

I have also changed your fields from public to private: if you really need to expose them, you should use public properties, not fields.