# Spell design and comunication with Player

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.

• I would go for mix of 1 and 2. Fireball in your current implementation can be implemented like DamageSpell. And then create some data class with Fireball is DamageSpell, and has this graphics and this damage. So when you create big fireball you dont need new class with same logic but diffrent data. Btw you can change Player and Unit to implement same class, so you dont need that if. – Kikaimaru Jul 11 '13 at 11:15
• Yes, in this case I could do that with some of fields, but Player and Spell don't have much common fields. So I need that if anyway. Also, I wanted to make spells that have different effects, depending on target. I'll edit to make it more clear. – Kmaczek Jul 11 '13 at 11:41
• And of course right, I should change name to DamageSpell and parametrize it a bit. I understood that when I was writing this question, btw it took some time :P Then I forgot to change it. But this was in my mind also. – Kmaczek Jul 11 '13 at 11:56
• By the way, you can take a look at WoW emualtor WCell or Ultima Online emulator RunUO. Both games have many spells and have nice solutions to this. – Kikaimaru Jul 11 '13 at 12:15

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
• Thank you for opinion, adding new stuff was on my mind also. Generally I felt that first is better, but wanted to ask professionals :P to be sure. And I know loading is possible in first, but in my opinion is a bit harder. – Kmaczek Jul 11 '13 at 11:31

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.

• Brilliant! I forgot that I can inherit... – Kmaczek Jul 11 '13 at 11:24
• Although this can work, I would suggest including a few classes for spells. I don't mean one for every spell (that would be way too much overkill) but a few classes that extend a ISpell interface like "Buff", "AreaOfEffect", etc. By using these classes you don't have to worry as much about the logic errors that can be introduced with loading external files or using generic parameters. Using those classes with the above solution would probably work well enough to fit your needs. – Benjamin Danger Johnson Jul 11 '13 at 16:36
• I wasn't sarcastic :P I'm not that advanced as u think, I rly forgot. – Kmaczek Jul 11 '13 at 21:14