Optimizing a complex reference to a class property and overload issue

Question

I'm currently working on a Magic system for a game I'm building and I feel like the references to the exact statistic are a bit complicated. I got a Vital class:

public int baseValue;
public int currValue;

public string statName;

For reference to the vitals used in the game, I have an enum:

public enum VitalName { Health, Mana}

I also have a Spell class:

public string spellName;
public MagicEffect magicEffect ;

with a derived BlackMagic class:

public int BaseDamage;

The MagicEffect Base Class has nothing but VitalModifier is derived from it:

public int vitalToAdjustIndex;

and finally, a LivingBeing class with this:

public Vital[] vitals = new Vital[Enum.GetValues(typeof(VitalName)).Length];

private void SetupVitals()
{
    for(int i = 0; i < vitals.Length; i++)
    {
        vitals[i] = new Vital
        {
            statName = ((VitalName)i).ToString()
        };
    }
}

Now, what's happening is that the LivingBeing.CastSpell() function has what I think is a rather long reference to find the exact vital to modify:

public void CastSpell(LivingBeing target, BlackMagic spellToCast)
{
    target.vitals[((VitalModifier)spellToCast.magicEffect).vitalToAdjustIndex].currValue -= spellToCast.BaseDamage;
}

It might just be me but using that rather long line seems like it's not needed. The reason I have to cast spellToCast.MagicEffect to a VitalModifier is because the MagicEffect property of a spell doesn't have a VitalToAdjustIndex property...

I realize that casting is a major issue in my code since I have to use 4 overload for the LivingBeing.CastSpell() because the Spell doesn't have a baseDamage property... so I end up having one overload for each type of spell (White, Special, Ailment and Effect).

I don't really know if there's another way to do it, but if there is a way to avoid multiple overloads or the need for all that casting, I'd be thankful

Also, should I put all the code in the same "box" when I ask a question or separate it like I did?

Answer 1

First, modify Vital to use the VitalName enum for the name value. The name also needs to be immutable, because I'm going to use it in a dictionary.

class Vital
{
    public Vital(VitalName name) => Name = name;

    public int baseValue;
    public int currValue;
    public VitalName Name { get; }

    public override bool Equals(object obj)
    {
        return obj is Vital vital && Name == vital.Name;
    }

    public override int GetHashCode()
    {
        return 539060726 + Name.GetHashCode();
    }
}

Modify VitalModifier to use the VitalName enum to denote what it should change -

class VitalModifier : MagicEffect
{
    public VitalName VitalToAdjust;
}

Finally, change LivingBeing to use the changes.

class LivingBeing
{
    public LivingBeing()
    {
        SetupVitals();
    }

    public IDictionary<VitalName, Vital> vitals;

    private void SetupVitals()
    {
        var vitalNames = Enum.GetValues(typeof(VitalName)) as VitalName[];
        vitals = vitalNames.ToDictionary(name => name, name => new Vital(name));
    }

    public void CastSpell(LivingBeing target, BlackMagic spellToCast)
    {
        switch (spellToCast.magicEffect)
        {
            case VitalModifier m:
                target.vitals[m.VitalToAdjust].currValue -= spellToCast.BaseDamage;
                break;
            default:
                throw new UnexpectedSpellEffectException();
        }
    }
}

--

As an aside, when you are using fields on classes, consider properties instead.