architecture - Making character's skills and abilities as commands, good practice?

I am designing for a game that consist of characters who have unique offensive skills and other abilities such as building, repairing, etc. Players can control multiple of such characters.

I'm thinking of putting all of such skills and abilities into individual commands. A static controller would register all of these commands into a static command list. The static list would consist of all the available skills and abilities of all the characters in the game. So when a player selects one of the characters and click on a button on the UI to cast a spell or perform an ability, the View would call the static controller to fetch the desired command from the list and execute it.

What I'm not sure of however if this is a good design given that I'm building my game in Unity. I am thinking I could have made all the skills and abilities as individual components, which would then be attached to the GameObjects representing the characters in the game. Then the UI would need to hold the GameObject of the character and then execute the command.

What would be a better design and practice for a game that I am designing?

Answer

TL;DR

This answer goes a little crazy. But it's because I see you're talking about implementing your abilities as "Commands," which implies C++/Java/.NET design patterns, which implies a code-heavy approach. That approah is valid, but there's a better way. Maybe you're already doing the other way. If so, oh well. Hopefully others find it useful if that's the case.

Look at Data-Driven Approach below to cut to the chase. Get Jacob Pennock's CustomAssetUility here and read his post about it.

Working With Unity

Like others have mentioned, traversing a list of 100-300 items is not as big a deal as you might think. So if that's an intuitive approach for you, then just do that. Optimize for brain efficiency. But the Dictionary, as @Norguard demonstrated in his answer, is the easy no-brainpower-required way to eliminate that problem since you get constant-time insertion and retrieval. You should probably use it.

In terms of making this work well within Unity, my gut tells me that one MonoBehaviour per ability is a dangerous path to go down. If any of your abilities maintain state over time a they execute, you'll need to manage that a provide a way to reset that state. Coroutines alleviate this problem, but you're still managing an IEnumerator reference on every update frame of that script, and have to absolutely make sure you have a sure-fire way to reset abilities lest incomplete and stuck-in-a-state-loop abilities quietly start screwing up the stability of your game when they go unnoticed. "Of course I'll do that!" you say, "I'm a 'Good Programmer'!". But really, you know, we're all objectively terrible programmers and even the greatest AI researchers and compiler writers screw stuff up all the time.

Of all the ways you could implement command instantiation and retrieval in Unity, I can think of two: one is fine and won't give you an aneurysm, and the other allows for UNBOUNDED MAGICAL CREATIVITY. Sort of.

Code-Centric Approach

First is a mostly-in-code approach. What I recommend is that you make each command a simple class that either inherits from a BaseCommand abtract class or implements an ICommand interface (I'm assuming for the sake of brevity that these Commands will only ever be character abilities, it's not hard to incorporate other uses). This system assumes that each command is an ICommand, has a public constructor that takes no parameters, and requires updating each frame while it is active.

Things are simpler if you use an abstract base class, but my version uses interfaces.

It's important that your MonoBehaviours encapsulate one specific behavior, or a system of closely-related behaviors. It's okay to have lots of MonoBehaviours that effectively just proxy out to plain C# classes, but if you find yourself doing too may update calls out to all sorts of different objects to the point where it's starting to look like an XNA game, then you're in serious trouble and need to change your architecture.

// ICommand.cs
public interface ICommand
{
    public void Execute(AbilityActivator originator, TargetingInfo targets);
    public void Update();

    public bool IsActive { get; }
}


// CommandList.cs
// Attach this to a game object in your loading screen
public static class CommandList
{
    public static ICommand GetInstance(string key)
    {

        return commandDict[key].GetRef();
    }


    static CommandListInitializerScript()
    {
        commandDict = new Dictionary() {

            { "SwordSpin", new CommandRef() },


            { "BellyRub", new CommandRef() },

            { "StickyShield", new CommandRef() },

            // Add more commands here
        };
    }


    private class CommandRef where T : ICommand, new()

    {
        public ICommand GetNew()
        {
            return new T();
        }
    }

    private static Dictionary commandDict;
}



// AbilityActivator.cs
// Attach this to your character objects
public class AbilityActivator : MonoBehaviour
{
    List activeAbilities = new List();

    void Update()
    {
        string activatedAbility = GetActivatedAbilityThisFrame();

        if (!string.IsNullOrEmpty(acitvatedAbility))
            ICommand command = CommandList.Get(activatedAbility).GetRef();
            command.Execute(this, this.GetTargets());
            activeAbilities.Add(command);
        }

        foreach (var ability in activeAbilities) {
            ability.Update();
        }


        activeAbilities.RemoveAll(a => !a.IsActive);
    }
}

This works totally fine, but you can do better (also, a List isn't the optimal data structure for storing timed abilities, you might want a LinkedList or a SortedDictionary).

Data-Driven Approach

It's probably possible that you can reduce your ability's effects down into logical behaviors that can be parameterized. This is what Unity was really built for. You, as a programmer, design a system that then either you or a designer can go and manipulate in the editor to produce a wide variety of effects. This will great simplify the "rigging" of the code, and focus exclusively on execution of an ability. No need to juggle base classes or interfaces and generics here. It will all be purely data driven (which also simplifies initializing command instances).

The first thing you need is a ScriptableObject that can describe your abilities. ScriptableObjects are awesome. They're designed to work like MonoBehaviours in that you can set their public fields in Unity's inspector, and those changes will get serialized to disk. However, they're not attached to any object and don't have to be attached to a game object in a scene or instantiated. They are the catch-all data buckets of Unity. They can serialize basic types, enums, and simple classes (no inheritance) marked [Serializable]. Structs can't be serialized in Unity, and serialization is what allows you to edit the object fields in the inspector, so remember that.

Here's a ScriptableObject that tries to do a lot. You can break this out into more serialized classes and ScriptableObjects, but this is supposed to just give you an idea of how to go about doing it. Normally this looks ugly in a nice modern object oriented language like C#, since it really feels like some C89 shit with all those enums, but the real power here is that now you can create all sorts of different abilities without ever writing new code to support them. And if your first format doesn't do what you need it to do, just keep adding to it until it does. So long as you don't change field names, all your old serialized asset files will still work.

// CommandAbilityDescription.cs

public class CommandAbilityDecription : ScriptableObject
{

    // Identification and information
    public string displayName; // Name used for display purposes for the GUI
    // We don't need an identifier field, because this will actually be stored
    // as a file on disk and thus implicitly have its own identifier string.

    // Description of damage to targets


    // I put this enum inside the class for answer readability, but it really belongs outside, inside a namespace rather than nested inside a class
    public enum DamageType
    {
        None,
        SingleTarget,
        SingleTargetOverTime,
        Area,
        AreaOverTime,
    }


    public DamageType damageType;
    public float damage; // Can represent either insta-hit damage, or damage rate over time (depend)
    public float duration; // Used for over-time type damages, or as a delay for insta-hit damage

    // Visual FX
    public enum EffectPlacement
    {
        CenteredOnTargets,
        CenteredOnFirstTarget,
        CenteredOnCharacter,

    }

    [Serializable]
    public class AbilityVisualEffect
    {
        public EffectPlacement placement;
        public VisualEffectBehavior visualEffect;
    }

    public AbilityVisualEffect[] visualEffects;

}

// VisualEffectBehavior.cs
public abtract class VisualEffectBehavior : MonoBehaviour
{
    // When an artist makes a visual effect, they generally make a GameObject Prefab.
    // You can extend this base class to support different kinds of visual effects
    // such as particle systems, post-processing screen effects, etc.
    public virtual void PlayEffect(); 
}

You could further abstract the Damage section into a Serializable class so you could define abilities that deal damage, or heal, and have multiple damage types in one ability. The only rule is no inheritance unless you use multiple scriptable objects and reference the different complex damage configuration files on disk.

You still need the AbilityActivator MonoBehaviour, but now he does a little more work.

// AbilityActivator.cs
public class AbilityActivator : MonoBehaviour
{
    public void ActivateAbility(string abilityName)
    {
        var command = (CommandAbilityDescription) Resources.Load(string.Format("Abilities/{0}", abilityName));
        ProcessCommand(command);

    }

    private void ProcessCommand(CommandAbilityDescription command)
    {

        foreach (var fx in command.visualEffects) {
            fx.PlayEffect();
        }

        switch(command.damageType) {

            // yatta yatta yatta
        }

        // and so forth, whatever your needs require

        // You could even make a copy of the CommandAbilityDescription
        var myCopy = Object.Instantiate(command);

        // So you can keep track of state changes (ie: damage duration)
    }

}

So the interface and generic trickery in the first approach will work fine. But in order to really get the most out of Unity, ScriptableObjects will get you where you want to be. Unity is great in that it provides a very consistent and logical environment for programmers, but also has all the data entry niceties for designers and artists you get from GameMaker, UDK, et. al.

Last month, our artist took a powerup ScriptableObject type that was supposed to define behavior for different kinds of homing missiles, combined it with an AnimationCurve and a behavior that made missiles hover along the ground, and made this crazy new spinning-hockey-puck-of-death weapon.

I still need to go back and add specific support for this behavior to make sure it's running efficiently. But because we made this generic data description interface, he was able to pull this idea out of thin air and put it into the game without us programmers even knowing he was trying to do it until he came over and said, "Hey guys, look at this cool thing!" And because it was clearly awesome, I'm excited to go add more robust support for it.