Game Character class with many skills

Question

Im creating a class that has all of my data in and can see it would quickly become messy.

I feel that there must be a way of making it neater rather than making a new method each time

For example. I have a character that out of the huge skill list each with different values for different things, they just have 1h weapon and shield. I declare this by

Character character = new Character();
character.Skills.Add(Skill.OneHanded());
character.Skills.Add(Skill.Shield());

it also makes it fairly easy if they level up and learn a new skill as i can just do

character.Skills.Add(Skill.Polearm());

however currently my Skill class looks like a mess. (note that there will be many more skills (about 30 in total) and each one will have more attributes such as applicable weapons, level requirements etc, but not currently coded.

public class Skill
{
    public string Description { get; set; }
    public bool Advanced{ get; set; }
    public int PointsToLearn{ get; set; }



    public static Skill OneHanded()
    {
        return new Skill
        {
            Description = "Skill needed for the use of one handed weapons",
            Advanced = false,
            PointsToLearn= 1
        };
    }
    public static Skill TwoHanded()
    {
        return new Skill
        {
            Description = "Skill needed for use of two handed weapons.",
            Advanced= false,
            PointsToLearn = 2
        };
    }
    public static Skill Polearm()
    {
        return new Skill
        {
            Description = "Skill needed for use of polearms",
            Advanced = false,
            PointsToLearn = 3
        };
    }
    public static Skill Shield()
    {
        return new Skill
        {
            Description = "Skill needed to use shields",
            Advanced = false,
            PointsToLearn = 1
        };
    }
    public static Skill ResistMagic()
    {
        return new Skill
        {
            Description = "Negates the first spell cast at the character",
            Advanced = true,
            PointsToLearn = 0
        };
    }
}

As requested the Character class is

class Character
    {
        public List<Skill> Skills = new List<Skill>();
        public int Level { get; set; }
        public string Name { get; set; }
        public int Command { get; set; }
        public int Coordination { get; set; }
        public int Reflex { get; set; }

    }

There are many characters (i guess think fire emblem if you have ever played that) each one can be skilled and levelled (and die) independently . The class is mostly to keep a record of all characters that have joined the players mercenary company, with their current skills, attributes, level etc etc.

It will called when ever a new character joins the company and added to a character list and then remain in the list ready to be called with the attributes of the character when needing to know information about them.

is there a way to simply this? I also got advised I could do

public class Skill
{
    public enum Skills{ OneHanded,TwoHanded,Polearm,Shield};
}

which opens up using Skill.Skills."Skill" but I'm not sure how I would set the data against each of the options.

Answer 1

In the way you show that you create each Skill instance, you really have anything than the Description to distinguish them from each other.

I read your Skill class as merely an advanced flag that defines a small set of properties. In the below I further anticipate, that each skill type doesn't change throughout the game. If that is correct, then I would make the Skill class as a "singleton" per Skill type (onehanded, twohanded etc.) and let a static Factory method create/return the right Skill according to a Skill Type enum:

public class Skill
{
  public string Description { get; private set; }
  public bool Advanced { get; private set; }
  public int PointsToLearn { get; private set; }
  public SkillType Type { get; private set; }

  private Skill()
  {
  }

  private static Dictionary<SkillType, Skill> s_skills = new Dictionary<SkillType, Skill>
  {
    { SkillType.OneHanded, new Skill { Type = SkillType.OneHanded, Description = "Skill needed for the use of one handed weapons", Advanced = false, PointsToLearn = 1, } },
    { SkillType.TwoHanded, new Skill { Type = SkillType.TwoHanded, Description = "Skill needed for use of two handed weapons", Advanced = false, PointsToLearn = 2, } },
    { SkillType.Polearm, new Skill { Type = SkillType.Polearm, Description = "Skill needed for use of polearms", Advanced = false, PointsToLearn = 3, } },
    { SkillType.Shield, new Skill { Type = SkillType.Shield, Description = "Skill needed to use shields", Advanced = false, PointsToLearn = 1, } },
    { SkillType.ResistMagic, new Skill { Type = SkillType.ResistMagic, Description = "Negates the first spell cast at the character", Advanced = true, PointsToLearn = 0, } },
  };

  public static Skill Factory(SkillType type)
  {
    return s_skills[type];
  }
}

public enum SkillType
{
  OneHanded,
  TwoHanded,
  Polearm,
  Shield,
  ResistMagic
}

---

If their role is more advanced than that with methods and state, it IMO calls for at class hierarchy in some way like:

public abstract class Skill
{
  public string Description { get; set; }
  public bool Advanced { get; set; }
  public int PointsToLearn { get; set; }
  public SkillLevel SkillLevel { get; set; }
}

public class OneHanded: Skill
{

}

public class TwoHanded : Skill
{

}
...

which of cause requires a lot more work.

Update

If you want to filter a list of skills by SkillLevel, it can be done like this:

List<Skill> skills = new List<Skill> { new OneHanded(), new TwoHanded(), ... etc. }

foreach (Skill skill in skills.Where(s => s.SkillLevel == SkillLevel.Basic))
{
  Console.WriteLine(skill.GetType().Name);
}

---

Beside that I would change the Advanced flag to an enum too, because you can then distinguish between more levels like:

enum SkillLevel
{
  Basic,
  Medium,
  Advanced,
}

Answer 2

You should override equals and use HashSet so the same skill cannot be entered twice.

Description is generic but I think this makes cleaner code.

public enum SkillType { OneHanded, TwoHanded, Polearm, Shield }
public class Skill
{
    private Dictionary<SkillType, bool> TypeToAdvanced = new Dictionary<SkillType, bool> { { SkillType.OneHanded, true },
                                                                                            { SkillType.Polearm,   true },
                                                                                            { SkillType.Shield,   false },
                                                                                            { SkillType.Shield,    true }, };
    private Dictionary<SkillType, int> TypeToPoints = new Dictionary<SkillType, int> { { SkillType.OneHanded, 1 },
                                                                                        { SkillType.Polearm,   2 },
                                                                                        { SkillType.Shield,    2 },
                                                                                        { SkillType.Shield,    1 }, };
    protected bool Equals(Skill other)
    {
        return this.Type == other.Type;
    }
    public override bool Equals(object obj)
    {
        if (ReferenceEquals(null, obj)) return false;
        if (obj.GetType() != this.GetType()) return false;
        return Equals((Skill)obj);
    }
    public override int GetHashCode()
    {
        return (int)Type;
    }

    public SkillType Type { get; }
    public string Description { get { return $"Skill needed for use of {Type.ToString()}"; } }
    public int PointsToLearn { get { return TypeToPoints[Type]; } }
    public bool Advancedd { get { return TypeToAdvanced[Type]; } }
    public Skill (SkillType type)
    {
        Type = type;
    }