I've decided to implement an ability system for my game and set the following requirements:

• Abilities must be MonoBehaviors, that-is, components of Player/NPC gameobjects
• Abilities must be able to be added/removed at runtime. Instead of all entites having all abilities on their gameobjects that are disabled/enabled, I'd like to dynamically add/remove abilities using AddComponent/Destroy(component)

Given these I've implemented the following:

• Settings classes which inherit from a base AbilitySettings class which is a ScriptableObject. These contain configurable ability settings as well as an enum called AbilityIdentifier which identifies the ability (for example a jump ability would have the identifier AbilityIdentifier.JUMP)

• IAbility non-generic interface containing a few common ability methods (such as TriggerAbility and CanTrigger)

• AbstractAbility<T> class which implements IAbility and T is a type that extends AbilitySettings. It implements some of the IAbility methods and defines others as abstract. Actual abilities extend this class.

• AbilityManager is a MonoBehavior which contains an array of all possible settings for that entity (added through unity editor) and internally contains a dictionary of <AbilityIdentifier, IAbility>. All of the entities abilities are added/removed using the AbilityManager

It looks something like this:

public class AbilityManager : MonoBehavior
{
    [SerializeField] private AbstractAbilitySettings[] allAbilitiesSettings = { };    

    private readonly Dictionary<AbilityIdentifier, IAbility> abilities = new Dictionary<AbilityIdentifier, IAbility>();

    // Add/remove ability methods
}

For example, a jump ability pickup gameobject is set somewhere in the world as a trigger. When the player moves over the pick-up object and OnTriggerEnter is executed. The script on the pick-up object gets the AbilityManager and calls AddAbility(AbilityIdentifier.JUMP)

This sounds good but It's far from perfect. First of all, I couldn't figure out an elegant way of creating/removing a component when given the settings class so I've added the creation/destruction code to the settings class itself. That-is I've added the following abstract methods to AbilitySettings

public abstract IAbility InstantiateAbility(GameObject gameObject);

public abstract void RemoveAbility(GameObject gameObject);

which are then implemented in each of the concrete settings classes like this:

public override IAbility InstantiateAbility(GameObject gameObject)
{
    JumpAbility ability = gameObject.AddComponent<JumpAbility>();
    ability.Settings = this;
    return ability;
}

public override void RemoveAbility(GameObject gameObject)
{
    JumpAbility ability = gameObject.GetComponent<JumpAbility>();
    Destroy(ability);
}

And these methods are called in the AbilityManager like this

public void AddAbility(AbilityIdentifier identifier)
{
    AbilitySettings abilitySettings = Array.Find(allAbilitiesSettings, s => s.Identifier == identifier);

    abilitySettings.InstantiateAbility(gameObject);
}

The implementation of InstantiateAbility and RemoveAbility is the same for every single ability, the only difference being the ability type. This is a big smell for me. I can't make AbilitySettings generic and generify the two methods as these settings are in an array.

My questions are:

• Adding methods such as InstantiateAbility and RemoveAbility to a scriptable object seems like a code smell to me. Take into account that I'm using the AbilityIdentifier to specify to the manager which ability I want to create. I have thought of perhaps creating an AbilityFactory<T> but since it's a generic class it can't be a part of an array/list so I'm facing the same problem I did with the settings. Is there a different way I could handle this without having the code in the scriptable object?

• Having the implementation of these two methods InstantiateAbility and RemoveAbility be the same for every implementation with the only difference being the type is also a big code smell. Is there any way I can generify this but at the same time avoid problems with the inability of having an array or list of those generic classes?