A Generic “Augmentation” Collection for RPG Attributes/Resistances/Aversions

Question

After having showed a previous implementation of my "Resistance System", I decided that I was completely wrong about how I had been going about things. The system, as it had been presented, was completely erroneous in both theory and execution. I can admit that. Realizing this, I took a step back and realized that I could take an entirely different approach for how I was doing things.

My game originally required that I have:

AttributeTable.cs

This class represents primary attributes. Such attributes encompass: Strength, Agility, Stamina, Intellect, Wisdom, Spirit, Defense, and Willpower. This class holds data from 0-510 for each stat. Items found by the player, such as swords, armor, etc, would have their own instance of this class, which are then combined with the "actors" other equipment bonuses to total/aggregate their primary AttributeTable.

ElementalTable.cs

This class governed elemental "resistance/aversion" attributes, such as: Fire, Frost, Energy, Sonic, Nature, Arcane, Holy, Shadow. The concept of this class was to be percentile based. In other words, 0-100, which is then computed to facillitate spell-damage mitigation/aggravation.

AttributeModificationTable.cs

This is where things started to get crazy...this class is was the exact same as the ElementalTable but dealt with percentile based Attributes, Strength, Agility, Stamina, etc. This class would have been used to increase stats based upon a percent, which would have been useful for Buffs and Debuffs. Say you have a buff called "Vigor", which increases Strength by 35% for x turns. The buff would have an instance of this class to help with that.

CreatureMasteryTable.cs

Again...getting crazier. This class was the exact same as the ElementalTable and AttributeModificationTable, except it dealt with Creature types, and how effective an actor is when attacking them.

I quickly realized that I was making my project/game engine entirely unmaintainable. I needed three different classes, which looked the exact same to help solve the same problem. I also had numerous other fallacies, but I won't mention the rest here, if you want to know more you can follow my original link above.

So how do I fix this? How do I unify my percentile-based classes and create less work later on? The answer: A single generic class. This lead me to the following class:

using System;
using System.Collections.Generic;
using GrimoireEngine.Framework.Logic;

namespace GrimoireEngine.Framework.Collections
{

    public enum AugmentationType
    {
        Increase,
        Decrease
    }

    public class AugmentationTable<T> where T : struct, IConvertible, IComparable, IFormattable
    {
        private Dictionary<T, int> _augmentationTable;
        private static T[] _augmentationTypeKeys;
        public AugmentationType TableType { get; set; }
        public string Name { get; set; }

        public static T[] AugmentationTypeKeys
        {
            get
            {
                if (_augmentationTypeKeys == null)
                {
                    _augmentationTypeKeys = (T[])Enum.GetValues(typeof(T));
                }
                return _augmentationTypeKeys;
            }
        }

        public T[] Keys
        {
            get
            {
                /**
                 * We grab this from the static initialized
                 * property, which is only initialized once.
                 * We could reference the internal Dictionary for
                 * this, but that would require us to cast to an
                 * array, which would create heap allocations.
                 * Doing it this way creates no heap allocations,
                 * and because the array is readonly, this won't
                 * yield any exterior conflicts.
                 */
                return AugmentationTypeKeys;
            }
        }

        public int this[T type]
        {
            get
            {
                return GetAugmentation(type);
            }
            set
            {
                SetAugmentation(type, value);
            }
        }

        public const int DefaultAugmentation = MinimumAugmentation;
        public const int MinimumAugmentation = 0;
        public const int MaximumAugmentation = 100;

        public AugmentationTable(
            AugmentationType tableType = AugmentationType.Increase,
            int defaultValue = DefaultAugmentation)
        {
            TableType = tableType;
            Initialize(defaultValue);
        }

        public void Initialize(int defaultValue = DefaultAugmentation)
        {
            if (this._augmentationTable == null)
            {
                this._augmentationTable = new Dictionary<T, int>();
            }
            for (int i = 0; i < AugmentationTypeKeys.Length; i++)
            {
                this._augmentationTable.Add(AugmentationTypeKeys[i], defaultValue);
            }
        }

        public void IncreaseAugmentation(T type, int value)
        {
            SetAugmentation(type, GetAugmentation(type) + value);
        }

        public void DecreaseAugmentation(T type, int value)
        {
            SetAugmentation(type, GetAugmentation(type) - value);
        }

        public void ClearAugmentations()
        {
            Initialize();
        }

        public void ClearAugmentation(T type)
        {
            SetAugmentation(type, DefaultAugmentation);
        }

        public int GetAugmentation(T type)
        {
            return _augmentationTable[type];
        }

        public void SetAugmentation(T type, int value)
        {
            this._augmentationTable[type] = Clamp(value);
        }

        public int ComputeAugmentation(T type, int value)
        {
            return ComputeAugmentation(this.TableType,type,value);
        }

        public int ComputeAugmentation(AugmentationType augmentationType, T type, int value)
        {
            switch(augmentationType)
            {
                case AugmentationType.Increase:
                    return (int)(value * (1.0f + (GetAugmentation(type) / 100f)));
                case AugmentationType.Decrease:
                    return (int)(value - ((value * (GetAugmentation(type) / 100f))));
                default:
                    throw new ArgumentOutOfRangeException(nameof(augmentationType), augmentationType, null);
            }
        }

        private int Clamp(int value)
        {
            return (value < MinimumAugmentation) ? MinimumAugmentation : (value > MaximumAugmentation) ? MaximumAugmentation : value;
        }

        public AugmentationTable<T> Clone()
        {
            AugmentationTable<T> table = new AugmentationTable<T>();
            for (int i = 0; i < AugmentationTypeKeys.Length; i++)
            {
                table.SetAugmentation(AugmentationTypeKeys[i], GetAugmentation(AugmentationTypeKeys[i]));
            }
            table.TableType = TableType;
            return table;
        }

        public void Copy(AugmentationTable<T> table)
        {
            TableType = table.TableType;
            for (int i = 0; i < AugmentationTypeKeys.Length; i++)
            {
                this.SetAugmentation(AugmentationTypeKeys[i], table.GetAugmentation(AugmentationTypeKeys[i]));
            }
        }
    }
}

With that class I can now use my enums in a very unique way. The class auto-allocates itself, so if I ever wanted to add or remove an attribute/element/creature type, all I would have to do is remove the item from the enum itself. Here is an example of the usage:

using System;
using System.Windows;
using GrimoireEngine.Framework.Collections;
using GrimoireEngine.Framework.Logic;

namespace GrimoireDevelopmentKit
{
    /// <summary>
    /// Interaction logic for App.xaml
    /// </summary>
    public partial class App : Application
    {
        protected override void OnStartup(StartupEventArgs e)
        {
            AttributeTable attributes = new AttributeTable(50); // Init all Primary Attributes to 50 
            AugmentationTable<CreatureType> creatureDamageBuffs = new AugmentationTable<CreatureType>();
            AugmentationTable<AttributeType> attributeBuffs = new AugmentationTable<AttributeType>();
            AugmentationTable<ElementType> resistanceBuffs = new AugmentationTable<ElementType>();

            // Which can then be further used like:
            AugmentationTable<AttributeType> attributesAugmentations = new AugmentationTable<AttributeType>
            {
                TableType = AugmentationType.Decrease, // Increase or decrease, but only one can be chosen.
                // These attributes are auto-allocated by the AugmentationTable constructor.
                [AttributeType.Strength] = 15, // 15% Decrease
                [AttributeType.Agility] = 0, // 0% Decrease
                [AttributeType.Stamina] = 50, // 50% Decrease
                [AttributeType.Intellect] = 75, // 75% Decrease
                [AttributeType.Wisdom] = 100, // 100% Decrease
                [AttributeType.Spirit] = 45, // 45% Decrease
                [AttributeType.Defense] = 20, // 20% Decrease
                [AttributeType.Willpower] = 24 // 24% Decrease
            };

            // Rewrite the attributes using the percentile modifications
            attributes.Append(attributesAugmentations);

            // Print things out.
            foreach (AttributeType key in attributes.Keys)
            {
                Console.WriteLine(key + " " + attributes[key]);
            }


        }
    }
}

My Enum classes are

AttributeType

public enum AttributeType
{
    Strength = 0,
    Agility = 1,
    Stamina = 2,
    Intellect = 3,
    Wisdom = 4,
    Spirit = 5,
    Defense = 6,
    Willpower = 7
}

ElementType

public enum ElementType
{
    /// <summary>
    /// Represents a Typeless-based Element.
    /// </summary>
    Normal = 0,
    /// <summary>
    /// Represents a Fire-based Element.
    /// </summary>
    Fire = 1,
    /// <summary>
    /// Represents a Frost-based Element.
    /// </summary>
    Frost = 2,
    /// <summary>
    /// Represents an Energy-based Element.
    /// </summary>
    Energy = 3,
    /// <summary>
    /// Represents a Sound-based Element.
    /// </summary>
    Sonic = 4,
    /// <summary>
    /// Represents a Nature-based Element.
    /// </summary>
    Nature = 5,
    /// <summary>
    /// Represents an Arcane/Magic-based Element.
    /// </summary>
    Arcane = 6,
    /// <summary>
    /// Represents a Holy-based Element.
    /// </summary>
    Holy = 7,
    /// <summary>
    /// Represents a Shadow-based Element.
    /// </summary>
    Shadow = 8

}

CreatureType

public enum CreatureType
    {
        Humanoid = 0,
        Beast = 1,
        Elemental = 2,
        Undead = 3,
        Demon = 4,
        Dragonkin = 6,
        Aberration = 7,
        Construct = 8,
        Insect = 9
    }

And lastly, my AttributeTable class, which is being included so you can easily test all my code if desired.

using System;
using System.Collections.Generic;
using GrimoireEngine.Framework.Logic;

namespace GrimoireEngine.Framework.Collections
{
    public class AttributeTable
    {
        private Dictionary<AttributeType, int> _attributeTable;
        private static AttributeType[] _attributeKeys;
        public static AttributeType[] AttributeKeys
        {
            get
            {
                if (_attributeKeys == null)
                {
                    _attributeKeys = (AttributeType[])Enum.GetValues(typeof(AttributeType));
                }
                return _attributeKeys;
            }
        }

        public AttributeType[] Keys
        {
            get
            {
                /**
                 * We grab this from the static initialized
                 * property, which is only initialized once.
                 * We could reference the internal Dictionary for
                 * this, but that would require us to cast to an
                 * array, which would create heap allocations.
                 * Doing it this way creates no heap allocations,
                 * and because the array is readonly, this won't
                 * yield any exterior conflicts.
                 */
                return AttributeKeys;
            }
        }

        /// <summary>
        /// 
        /// </summary>
        public const int MinimumAttribute = 0; // GameConstants.MinimumAttribute
        /// <summary>
        /// 
        /// </summary>
        public const int MaximumAttribute = 510; // GameConstants.MaximumAttribute
        /// <summary>
        /// 
        /// </summary>
        public const int DefaultAttribute = 0;

        public int this[AttributeType type]
        {
            get
            {
                return GetAttribute(type);
            }
            set
            {
                SetAttribute(type, value);
            }
        }

        public AttributeTable(int defaultValue = DefaultAttribute)
        {
            Initialize(defaultValue);
        }

        /// <summary>
        /// 
        /// </summary>
        /// <param name="defaultValue"></param>
        public void Initialize(int defaultValue = DefaultAttribute)
        {
            if (this._attributeTable == null)
            {
                this._attributeTable = new Dictionary<AttributeType, int>();
            }
            for (int i = 0; i < AttributeKeys.Length; i++)
            {
                this._attributeTable.Add(AttributeKeys[i], defaultValue);
            }
        }

        public AttributeTable(AttributeTable other)
        {
            Copy(other);
        }

        /// <summary>
        /// 
        /// </summary>
        /// <param name="type"></param>
        /// <returns></returns>
        public int GetAttribute(AttributeType type)
        {
            return _attributeTable[type];
        }

        /// <summary>
        /// 
        /// </summary>
        /// <param name="type"></param>
        /// <param name="value"></param>
        public void SetAttribute(AttributeType type, int value)
        {
            this._attributeTable[type] = Clamp(value, MinimumAttribute, MaximumAttribute);
        }

        public void IncreaseAttribute(AttributeType attribute, int amount)
        {
            SetAttribute(attribute, GetAttribute(attribute) + amount);
        }

        public void DecreaseAttribute(AttributeType attribute, int amount)
        {
            SetAttribute(attribute, GetAttribute(attribute) - amount);
        }

        private static int Clamp(int value, int minimum, int maximum)
        {
            return (value < minimum) ? minimum : (value > maximum) ? maximum : value;
        }

        /// <summary>
        /// 
        /// </summary>
        /// <returns></returns>
        public AttributeTable Clone()
        {
            AttributeTable table = new AttributeTable();
            for (int i = 0; i < AttributeKeys.Length; i++)
            {
                table.SetAttribute(AttributeKeys[i], GetAttribute(AttributeKeys[i]));
            }
            return table;
        }

        /// <summary>
        /// 
        /// </summary>
        /// <param name="table"></param>
        public void Copy(AttributeTable table)
        {
            for (int i = 0; i < AttributeKeys.Length; i++)
            {
                this.SetAttribute(AttributeKeys[i], table.GetAttribute(AttributeKeys[i]));
            }
        }

        public void Append(AugmentationTable<AttributeType> augmentations)
        {
            for (int i = 0; i < AttributeKeys.Length; i++)
            {
                AttributeType attribute = AttributeKeys[i];
                this[attribute] = augmentations.ComputeAugmentation(attribute, this[attribute]);
            }
        }

        public AttributeTable AppendToCopy(AugmentationTable<AttributeType> augmentations)
        {
            AttributeTable attributes = this.Clone();
            attributes.Append(augmentations);
            return attributes;
        }
    }
}

Finally, to the end of this. I am putting this code here to see if I finally have a good implementation, or are at least on the right track. My question is if this can still be improved; if there is a better way that I could be doing such an "increase/decrease by percent" collection for my game. Do you have any more suggestions for me?

Edit: Modified code as it currently stands can be found here.

Answer 1

This looks much better now with the big blocks of repeating code gone. Let's see what else we have.

---

AugmentationTable

I see you have another one nearly identical class AttributeTable so most comments will apply to it too. But I find you should try to find a common denominator for them and implement the repeated code it an abstract base class.

---

private static T[] _augmentationTypeKeys;

and

public static T[] AugmentationTypeKeys
{
    get
    {
        if (_augmentationTypeKeys == null)
        {
            _augmentationTypeKeys = (T[])Enum.GetValues(typeof(T));
        }
        return _augmentationTypeKeys;
    }
}

and

public T[] Keys
{
    get
    {
        return AugmentationTypeKeys;
    }
}

Having two public properties AugmentationTypeKeys and Keys returning the exact same values but under different names is confusing. You should use only one property.

If you are on C#6 all this could be replaced with an intialization for the private readonly field and a readonly property:

private static readonly T[] _keys = (T[])Enum.GetValues(typeof(T));

public T[] Keys => _keys;

if not then add a private setter and initialize it in the constructor.

Be aware of the fact that the array is not immutable and can be manipulated elsewhere.

---

public int this[T type]
{
  get
  {
      return GetAugmentation(type);
  }
  set
  {
      SetAugmentation(type, value);
  }
}

and

public void ClearAugmentation(T type)
{
  SetAugmentation(type, DefaultAugmentation);
}

public int GetAugmentation(T type)
{
  return _augmentationTable[type];
}

Again, multiple APIs doing the same thing. I'd keep the indexer and remove the methods.

---

public AugmentationType TableType { get; set; }

This property is set in the constructor and yet it has a public setter. Are you sure you want to allow to change it later? I don't know of corse how it exactly is used but I'd say the setter should most probably be private.

---

public void Initialize(int defaultValue = DefaultAugmentation)
{
  if (this._augmentationTable == null)
  {
      this._augmentationTable = new Dictionary<T, int>();
  }
  for (int i = 0; i < AugmentationTypeKeys.Length; i++)
  {
      this._augmentationTable.Add(AugmentationTypeKeys[i], defaultValue);
  }
}

This method does not need to check for _augmentationTable == null because it should initialize it. This means it is definitely null so just create a new instance of the dictionary and add the items.

The public modifier needs to be changed to private. There is no point in calling it again by the user (I think) but if he did it, then it will crash because it'll try to re-add the already added keys which is not allowed.

---

public int ComputeAugmentation(AugmentationType augmentationType, T type, int value)
{
  switch (augmentationType)
  {
      case AugmentationType.Increase:
          return (int)(value * (1.0f + (GetAugmentation(type) / 100f)));
      case AugmentationType.Decrease:
          return (int)(value - ((value * (GetAugmentation(type) / 100f))));
      default:
          throw new ArgumentOutOfRangeException(nameof(augmentationType), augmentationType, null);
  }
}

Seeing this, I need to say stop! This is an indicator that the AugmentationTable should be abstract and the ComputeAugmentation method should be abstract too and overriden by the derived class to implement its own calculation.

Alternatively you could pass the calc function to the constructor as a delegate if you know there won't by any other differences. Or implement them in an AugmentationCalculator and pass this as a service via DI. As you see, there are multiple solutions.

I prefer abstract class if there are more differences and a service if just a few small like this one. But hey, nothing stops you to do both if necessary.

If you don't want to do it (yet) (it'll probably bite you later when you have even more differences between the two) then you should at least make it private and use only this one:

public int ComputeAugmentation(T type, int value)
{
  return ComputeAugmentation(this.TableType, type, value);
}

---

this.

It's better not to use this keyword. It doesn't really help the readability and you're not using it consistently anyway.

---

App

The AugmentationTable has a constructor but you don't use it in the OnStartup event handler. Do those values should really be mutable? If not then let the constructor take all the required parameters.