Spell design and comunication with Player

Question

Im trying to design Spell class that can modify almost anything, Player, Units, other spells, etc. I figured out two ways of doing it, but none of this seems proper for me. Thats mainly because of Player class that got a lot of parameters and setting spell is a bit troublesome.

First is:

Spell interface that Im adding to each class of spell, which have method: execute.

Pros:

• dont have to pass a lot of parameters, just call proper function of target class of a spell
• To use spell just pick spell Player.GetSpell("Fireball").Execute(targets)
• Big flexibility

Cons:

• A LOT OF CLASSES, like 30 or more. One for each spell type
• Each spell need to be wrote manually

Creating spell

class Fireball : SpellBase, ISpell
{
    public void Execute(Player caster, List<Object> target)
    {
        caster.Mana -= ManaCost;

        foreach (var o in target)
        {
            if (o.GetType() == typeof(Player))
            {
                var p = (Player)o;
                p.Health -= 10;
                Console.WriteLine("Player: {0} recieved 10 dmg, now has {1}",
                    p.Name, p.Health);
            }
            else if (o.GetType() == typeof(Spell))
            {
                var s = (Spell)o;
                s.Cooldown += 1;
                Console.WriteLine("Unit: {0} recieved 10 dmg, now has {1}",
                    s.Name, s.Health);
            }
        }
    }
}

Construction

spells.Add(new Fireball());

Executing

spells.First().Execute(caster, enemy);

---

Second approach:

Write one or few classes that can have different functionality depending on parameters set in constructor.

Pros:

• few classes
• loading parameters from file or data base

Cons:

• setting a lot of parameters for different classes(spells and player really dont have much common fields)
• One gigantic method, to handle spell execution

Creating spell

class Spell:SpellBase
    {
        public SpellData Data { get; set; }

        public Spell(SpellData param)
        {
            Data = param;
        }
    }

Construction

 spells.Add(new Frostball(params));
                {
                    HealthMod = -10
                });

Executing

public void CastSpell(Player caster, List<Object> enemy)
        {
            Caster.Mana -= ManaCost;

            foreach (var o in enemy)
            {
                if (o.GetType() == typeof(Player))
                {
                    var p = (Player)o;
                    p.Health += spell.Data.HealthMod;
                    //p.Defence += spell.Data.DefenceMod;
                    //and a lot of atributes and dependencies
                    //.
                    //.
                    //..........
                }
                else if (o.GetType() == typeof(Spell))
                {
                    var s = (Unit)o;
                    s.Cooldown += spell.Data.CooldownMod;
                    //s.Defence += spell.Data.DefenceMod;
                    //and a lot of atributes
                    //.
                    //.
                    //..........
                }
            }
        }

So... I need flexibility a lot. Im not saying its not possible to make it with second way but it will make one big Method with a lot of ifs, which I want to avoid, and i know that with time complexity of this will overwhelm me. Also setting parameters of all spells easily from DB would be nice.

Which one approach do you suggest? Maybe change sth or even you have ideas for whole different way of doing this? Please help me :)

Designin's hard... Sorry for english.

Answer 1

I'd like to preface by saying that you can make things less complicated but you cannot eliminate complexity. If you have N distinct types of spells, then somewhere in your code there has to be N functions (or one giant function with N parts, but that's hard to read). In this sense, your two approaches are more or less equally complicated, so it's not too big of a deal.

What your examples show however is the Open/Closed Principle: your class design should be open for extension but closed for modification. What this means is that, when you want to add new stuff, you should be able to just add new code, and modify existing code as little as possible. This both makes it easy to extend functionality, and hard to break existing functionality.

In this sense the first approach is better. With the first approach, adding a new spell is done by creating a new class, which is isolated from the existing classes, minimising the chance that you'll accidentally break existing code. With the second approach, you'll need to modify the CastSpell function, which contains code relating to all the other spells, increasing the chance of accidentally breaking something.

Having said that, your examples are not fully clear-cut, and there are a few other issues:

• A lot of classes is better than a big function that does a lot of things, but you can use multiple classes or functions with your second approach too
• Loading parameters is not dependent on approaches; you can parameterise different spells with the first approach too

Answer 2

I would definately go for option 1.

Why:

You could load all the different spells from an xml file / database procedurally. This would need to be a list with spell names, possible targets, damage / heal, cast time etc.

You will only have to write the logic only once using inheritance like:

determining if you can actually target the passed parameter targets with this spell, check the mana cost, check the player level starting the render loop, calculating the position in the world, taking hp from the enemy player etc. So I think this means you do not have to write each spell manually, just the class. (If you use C# you can use .tt files to generate lots of classes autimaticly. Maybe java has an equivalent?)

You can combine / chain spells and effects this way. You can execute spell B when spell A is done. for example, when fireball (spell A) hits an enemy, next, set him on fire (spell B).

You can use these spells also in combination with applied effects, like poison, fatigue, etc.

When you have ingame items like scrolls and weapons you can link the spell execution to the use of this weapon. This will separate all the logic nicely.

Having constructors with lots of parameters can get quite confusing.