Skill handling

[exectails]

I haven't taken a close look at how exactly ToS's skills work yet, but I'd like to talk about something before people try to implement them, as there are two conflicting approaches to it.

If you look at Athena or OdinMS for example, you will see that they basically handle skills in switch-cases. Related skills go into the same function, and what happens then is decided based on the skill's id. Here's a quick example of what something like that might look like.

void HandleDamageSkill(Skill skill, Character target)
{
	var splashRange = GetSplashRange(skill, target);
	var damageMultipler = GetDamageMultiplier(skill, target);
	
	var targets = GetTargets(target, splashRange);
	var damage = CalculateDamage(skill, target, damageMultiplier);
	targets.DealDamage(damage);
}

double GetSplashRange(Skill skill, Character target)
{
	switch (skill.Id)
	{
		case Bash:
			return 100;
		
		case Thrust:
		case PommelBeat:
			return 0;
	}
}

double GetDamageMultiplier(Skill skill, Character target)
{
	switch (skill.Id)
	{
		case Bash:
		case Thrust:
			return 1;
			
		case PommelBeat:
			if (target.IsStunned)
				return 1.5;
			break;
	}
}

RL example from eAthena:

		if(skill_num)
			switch(skill_num)
		{	//Hit skill modifiers
			//It is proven that bonus is applied on final hitrate, not hit.
			case SM_BASH:
			case MS_BASH:
				hitrate += hitrate * 5 * skill_lv / 100;
				break;
			case MS_MAGNUM:
			case SM_MAGNUM:
				hitrate += hitrate * 10 * skill_lv / 100;
				break;
			case KN_AUTOCOUNTER:
			case PA_SHIELDCHAIN:
			case NPC_WATERATTACK:
			case NPC_GROUNDATTACK:
			case NPC_FIREATTACK:
			case NPC_WINDATTACK:
			case NPC_POISONATTACK:
			case NPC_HOLYATTACK:
			case NPC_DARKNESSATTACK:
			case NPC_UNDEADATTACK:
			case NPC_TELEKINESISATTACK:
			case NPC_BLEEDING:
				hitrate += hitrate * 20 / 100;
				break;

There's basically a function and/or switch for any eventuality. Variables, buffs, effects, you name it. This approach has a huge advantage: it reduces redundancies. In your typical MMO there are very few skills that have actual special effects. For example, most damage dealing skills do pretty much the same thing, but with varying amounts of splash and damage. As a result, implementing a new damage skill in this system is generally as simple as adding a few fall-through cases, if any, because even defaulting might be an option.

This system was a little confusing to me when I started poking around eAthena as a beginner, because it wasn't compatible with what I expected. If I wanted to, say, change the damage of a skill and maybe play around with its effects, I couldn't search for a single function that handled one skill in its entirety. Instead I had to search for the general handler functions related to the thing I wanted to change, then find the switch, and either find the case for my skill, or add it if it didn't exist yet. The defaulting made it harder, because you might not even be able to search for the skill's id. It can be hard to wrap your head around this, and modding becomes harder.

For that reason I said to myself "if I ever create a server, I'll do better", so I went OOP for our Mabinogi emulator Aura. There, every skill has its own handler class, and you can change an entire skill's behavior in there. No jumping around, no searching, you want skill X, you get skill X.

class BashHandler : ISkillHandler
{
	void Handle(Skill skill, Character target)
	{
		var targets = GetTargets(target, 100);
		var damage = 123;
		targets.DealDamage(damage);
	}
}

class ThrustHandler : ISkillHandler
{
	void Handle(Skill skill, Character target)
	{
		var damage = 123;
		target.DealDamage(damage);
	}
}

class PommelBeatHandler : ISkillHandler
{
	void Handle(Skill skill, Character target)
	{
		var damage = 123;
		if (target.IsStunned)
			damage *= 1.5;
		target.DealDamage(damage);
	}
}

This, in my opinion, is much more manageable. Anybody is able to easily look up that one skill's handler and figure out what it does, because it's all right there, it leaves no questions open. The skill does exactly what it says in its handler, no more and no less.

It also eliminates one potential source of bugs. In eAthena I know of at least one case where a damage buff affected a new skill for a while that wasn't supposed to receive that buff. It was applied because the switch-case defaulted. While you could argue that that's a minor point, because in a dedicated handler design there's a potential for forgetting to include effects or buffs, I would argue that with handlers you can at least see that it's missing.

One disadvantage is redundancy, lots of it. Every slightly similar skill's handler will look pretty much the same. You can try to use inheritance to reduce that, but I've found that to not work very well, as it makes things even more confusing than the switch-case approach. Not only can't you find everything in one handler anymore, you don't even know which handler to look in from the potential countless parent handlers, and you constantly wonder where to put something new.

The other disadvantage is maintenance. If anything changes about the majority of damage dealing skills, you potentially have to modify every single damage skill handler, instead of a single switch-case.

I was glad that I decided to use dedicated handlers in Aura in the end, because Mabinogi's skills turned out to be very special. There was redundancy, but in hindsight I can't imagine doing it any other way, it would've been a mess. Many skills had multiple phases, skill specific parameters coming from the client, features you couldn't really generalize, and all in all, from dozens of skills, there might've been 3-4 instances where a switch-case would've actually worked well.

That doesn't mean that it's necessarily right for ToS and Melia, but it's something we should consider.

[Hirasu]

I'd prefer the second option.

No jumping around, no searching, you want skill X, you get skill X.

👍

[exectails]

Since we're already very much going in the proposed direction, I think we can close this discussion.