Lvl2 upgradeable attributes

Question

This is a follow up to:

• Lvl 1 upgradeable attributes

From before:

I've built the groundwork for an attributes (as in Strength, Intelligence, not as in DebuggerHidden, TestMethod) framework for my game. Each attribute can be upgraded and downgraded individually as well as part of a bulk change (such as for upgrades that have side effects, or leveling up).

I'm looking to get a thorough critique of the groundwork before I go expanding on shaky foundations.

I've made a full overhaul following the community's recommendations of my attributes system. I have kept the doc-level comments, because I intend to release this eventually with generated documentation, but I've made an effort to make the documentation more useful and detailed (of course, you will be the judge of that).

IAttribute

This is the basis for all attributes. It doesn't define much, simply that anything implementing this interface is an attribute and has a value of type T.

/// <summary>
/// Interface for all game attributes, modifiable or not.
/// </summary>
/// <typeparam name="T">The type of the attribute's value.</typeparam>
public interface IAttribute<T>
{
    /// <summary>
    /// Gets the value of this attribute. This is the final, post-modification value.
    /// </summary>
    /// <value>
    /// The value.
    /// </value>
    T Value
    {
        get;
    }
}

Attribute

Base concrete class for attributes. It has a fixed value that is set at construction. With the existence of this class, I'm not certain I need the IAttribute at all, but have kept it there to keep my options open later (in case I need an attribute for which a base value does not make sense).

/// <summary>
/// An attribute with a base value.
/// </summary>
/// <typeparam name="T">The attribute's value type.</typeparam>
public class Attribute<T> : IAttribute<T>
{
    /// <summary>
    /// Gets the value of this attribute
    /// </summary>
    /// <remarks>
    /// Unless overridden, this value is identical to the base value provided in the constructor.
    /// </remarks>
    /// <value>
    /// The base value provided in the constructor.
    /// </value>
    public virtual T Value
    {
        get
        {
            return baseValue;
        }
    }

    /// <summary>
    /// The base value of this attribute.
    /// </summary>
    protected T baseValue;

    /// <summary>
    /// Initializes a new instance of the <see cref="Attribute{T}"/> class.
    /// </summary>
    /// <param name="baseValue">The base value for this attribute.</param>
    public Attribute(T baseValue)
    {
        this.baseValue = baseValue;
    }
}

Modified Attribute

This is probably the most commonly-used attribute and why the whole system exists. This attribute has a number of modifiers applied to it to alter its base value.

/// <summary>
/// An attribute for which one or more modifiers may be applied.
/// </summary>
/// <typeparam name="T">The type of value this attribute has.</typeparam>
public class ModifiedAttribute<T> : Attribute<T>
{
    /// <summary>
    /// Gets the modifiers associated with this attribute.
    /// </summary>
    /// <value>
    /// The modifiers, including inactive modifiers.
    /// </value>
    public IList<IModifier<T>> Modifiers
    {
        get;
        private set;
    }

    public ModifiedAttribute(T baseValue)
        : base(baseValue)
    {
    }

    /// <summary>
    /// Gets the current value of this attribute.
    /// </summary>
    /// <value>
    /// The base value with all modifiers applied.
    /// </value>
    public override T Value
    {
        get
        {
            var value = base.Value;

            foreach (var modifier in Modifiers)
            {
                if (modifier.IsActive)
                {
                    value = modifier.Modify(value);
                }
            }

            return value;
        }
    }
}

IModifier

This is the base of all modifiers. Pretty simple again, a boolean for whether it's currently active or not (so I can turn them on and off in-order depending on game mechanics) and a modify function that transforms the parameter in some way.

/// <summary>
/// Base interface for modifiers applied to attributes.
/// </summary>
/// <typeparam name="T">The type of value this instance modifies.</typeparam>
public interface IModifier<T>
{
    /// <summary>
    /// Returns a modified value of the passed in item.
    /// </summary>
    /// <param name="item">The item to modify.</param>
    /// <returns>A modified item.</returns>
    T Modify(T item);

    /// <summary>
    /// Gets or sets a value indicating whether this instance is active.
    /// </summary>
    /// <value>
    ///   <c>true</c> if this instance is active; otherwise, <c>false</c>.
    /// </value>
    bool IsActive
    {
        get;
        set;
    }
}

OperatorModifier

This is the base class for all the +4 or *2 modifiers that are common in games. It applies a custom operator and right-hand-side operand to the parameter of Modify(). I toyed with making this abstract for readability purposes, but figured it was better to empower the developer.

/// <summary>
/// Modifier that applies an operator function to passed in values
/// </summary>
/// <typeparam name="T">The type of value this instance modifies.</typeparam>
public class OperatorModifier<T> : IModifier<T>
{
    /// <summary>
    /// Gets the multiplier.
    /// </summary>
    /// <value>
    /// The multiplier.
    /// </value>
    public Func<T, T, T> Operator
    {
        get;
        private set;
    }

    /// <summary>
    /// Gets the right-hand-side of the operator function
    /// </summary>
    /// <value>
    /// The right-hand-side applied to the operator.
    /// </value>
    public T RightHandSide
    {
        get;
        private set;
    }

    public OperatorModifier(T rightHandSide, Func<T, T, T> operation)
    {
        this.RightHandSide = rightHandSide;
        this.Operator = operation;
    }

    /// <summary>
    /// Modifies the specified item by applying the operator function to it.
    /// </summary>
    /// <param name="item">The item.</param>
    /// <returns></returns>
    public T Modify(T item)
    {
        return Operator(item, RightHandSide);
    }

    /// <summary>
    /// Gets or sets a value indicating whether this instance is active.
    /// </summary>
    /// <value>
    ///   <c>true</c> if this instance is active; otherwise, <c>false</c>.
    /// </value>
    public bool IsActive
    {
        get;
        set;
    }
}

AddModifier

Simple utility class for readability. It's essentially an OperatorModifier with the operator pre-set for addition. I would have liked to have kept this generic, but I'm not even sure it's possible to require that a generic support the + operator in some way, so I have instead opted to support the most commonly-used value type in game development, and will have to use OperatorModfier for other types.

public class AddModifier : OperatorModifier<float>
{
    public AddModifier(float delta)
        : base(delta, (x, y) => x + y)
    {
    }
}

MultiplyModifier

Same as AddModifier, with one obvious difference. Would be great to be able to keep these generic, as I said with AddModifier, but for now it will have to be a float.

public class MultiplyModifier : OperatorModifier<float>
{
    public MultiplyModifier(float factor)
        : base(factor, (x, y) => x * y)
    {
    }
}

Answer 1

Very nice, I like what you have done with it. Especially the Operator Modifier, clever use of lambdas clean way to avoid too much redundancy.

First let me tackle the IAttribute and explain why you want to keep it. Well an abstract attribute is great when the underlying type is a T item. but what about when it is not!

well how about this example:

class CompositeNumberAttribute : IAttribute<double>
{      

  public IList<IAttribute<double>> Attributes{ get; }
  public double Value 
  { 
     get { return  return Attributes.Any() ? Attributes.Sum(x => x.Value) : 0; }
  }
}

Now you can have a

var hitpoints = CompositeNumberAttribute();
hitpoints.Attributes.add(new AttributeImpl{ Name = "Left Leg", Value = 5});
hitpoints.Attributes.add(new AttributeImpl{ Name = "Right Leg", Value = 5});
hitpoints.Attributes.add(new AttributeImpl{ Name = "Head", Value = 10});
hitpoints.Attributes.add(new AttributeImpl{ Name = "Torso", Value = 20});

ok, silly example but effectively you have a single IAttribute, consumable by anything expecting a IAttribute only it is not simple, it is a crazy amalgamation of sub attributes, which in fact could each have their own modifiers!

In that example your base T in the Abstract class is not being used. it is redundant.

or another example,

public class AttributeClamped : IAttribute<double>
{
    public double Maximum { get; set; }
    public double Minimum { get; set; }

    IAttribute BaseAttribute{get;set;}

    AttributeClamped(IAttribute attribute)
    {
       BaseAttribute = attribute;
    }

    public override double Value
    {
        get { return BaseAttribute != null ? BaseAttribute.ClampInclusive(Minimum, Maximum) : 0; }
    }
}

Yeah I know you could effectively use the AAttribute for this one too again, the number comes fron a wrapped sub-attribute and not an actual base object so this is a tad cleaner.

So now you have an attribute decorator, you can take a really complex , modified attribute which, through whatever combination may end up returning you something like -999 and break your game, so now you take that modifable attribute, wrap it in a Clamped decorator and all the values returned from it are safely clamped to a pre-defined acceptable range.

Next, as for:

public override T Value
{
    get
    {
        var value = base.Value;

        foreach (var modifier in Modifiers)
        {
            if (modifier.IsActive)
            {
                value = modifier.Modify(value);
            }
        }

        return value;
    }
}

I suggested the list as an example but as a personal preference If i am using a custom interfaced type i try not to use lists.

instead i just have a single

IModifier Modifier{ get; set; }

The reason being the modifier is just like the above attribute. I could easily just pass it a

new MultiplyModifier(10);

or instead a

new CompositeModifier
{
 new MultiplyModifier(5),
 new AddModifier(4)
}

etc.

Which opens up options like

 new WeightedCompositeModifier
 {
   { new AddModifier(4), 5}
   { new MultiplyModifier(5), 9 } 
   { new AddModifier(4), 2}
   { new MultiplyModifier(5), 11 } 
 }

which could allow you to swap the application ordering on the fly. resolving an issue brought up by someone else.

also allowing a sort of tree nesting with a RandomModifier housing a composite Modifier which has a .... and so forth.

Answer 2

I like the architecture - it looks sensible, flexible and extensible so kudos for that. There's not really much to criticise with the code either - it's clean and readable. I do wonder what benefit you get with the generics particularly if you're expecting numbers that could reasonably be dealt with using float the whole way down.

I also think you should initialise the Modifiers property to an empty list in the constructor for ModifiedAttribute.

The only real concern I have, which may or may not be valid, is the order of applying the modifiers. At the moment it's a first in, first executed model. This means that adding the same modifiers to the attribute in a different order result in a different value.

Say I have a defence attribute and I can modify it by putting on some armour (+5) and/or by drinking some special purple liquid (*2). If the base level is 2, depending on which thing I do first, I can end up with a modified value of either 14 ((2+5)*2) or 9 ((2*2)+5). Is that desirable, or should you have a more advanced ordering - e.g. additions always come before multiplications?