Some stuff

[KLS]

I think I've found the issues that affect timers. A timer that is not enabled and as a 0 duration will return false on get expired and never be cleared from the database, sometimes they do(I dunno why? but can we put a work around in the expired code?).

Also some of the hard coded combat skills are affected by haste clientside but not taking that into account server side. Ex just timed my backstab clientside with a stopwatch, about 8 seconds without haste 5.5 with.

The issues with get stats is it uses the Mob:: version of stats instead of Client:: we need to be very careful as a result when things can be both for clients and npcs. Get stats for instance uses the mob version and the mob version counts the item and spell bonuses twice for clients because they aren't counted at all for non clients outside of that.

I'm not sure what's with my head and the endurance sorry, I really had documented a major loss in it the other day but just went to go make sure I wasn't crazy and apparently I am, oh no =( I work too late, sorry mate, if I actually find the problem again I'll bring it back to you in excrutiating detail.

I'm editing this post way too much but yeah also I'm gonna go through and see about getting close to the actual times on the client side skills since some seem off, some by a lot(track/forage!).

[KLS]

Alright after being informed last night how common the hackers were I looked into the timers further. Instead of the enforce rule I did this.

Checks to see if the timer is enabled where we check to see if timers are expired ex:

-	if(!p_timers.Expired(&database, pTimerCombatAbility, false)) {
+	if(p_timers.Enabled(pTimerCombatAbility) && !p_timers.Expired(&database, pTimerCombatAbility, false)) {

If a timer is disabled for whatever reason we'll count it as expired for these situations, while this really shouldn't be happening it has happened a few times to me and made the skill that was timer enforced completely unusable unless I modified the database to fix it.

Also applied haste to the timers for combat, tested it somewhat and seemed to work fine.

http://hmproject.org/files/timers2.patch

[KLS]

Notice you redid some of the special attack code to be more consistant with regular attacks. Looking over it I saw a few things, not really much from what you changed but what was already there in the orig. design.

In avoid damage there are several calls to functions Mob::CanThisClass<Whatever>() that always return false for npcs meaning they never dodge parry riposte except under certain conditions.

Example:

bool Mob::CanThisClassParry(void) const
{
	// Trumpcard
	switch(GetClass()) // Lets make sure they are the right level! -image
	{
	case WARRIOR:
		{
		if(GetLevel() < 10)
			return false;
		break;
		}
	case ROGUE:
	case BERSERKER:
		{
		if(GetLevel() < 12)
			return false;
		break;
		}
	case BARD:
		{
		if(GetLevel() < 53)
			return false;
		break;
		}
	case RANGER:
		{
		if(GetLevel() < 18)
			return false;
		break;
		}
	case SHADOWKNIGHT:
	case PALADIN:
		{
		if(GetLevel() < 17)
			return false;
		break;
		}
	default:
		{
			return false;
		}
	}

	if (this->IsClient())
		return(this->CastToClient()->GetSkill(PARRY) != 0);	// No skill = no chance
	else
		return false;
}

We return false if it is not the correct level and get past the switch statement if we are, then we check if we are a client and have skill if so we're okay if not we're not a client we get a false return 100% of the time from that last return false which really should be true. Happens in the following functions:

bool Mob::CanThisClassParry(void) const
bool Mob::CanThisClassDodge(void) const
bool Mob::CanThisClassRiposte(void) const

Also the riposte code is seperate from avoid damage somewhat in that we only do the auto attack in the Attack() functions instead of in the avoid damage, as a result being riposted with special skills wont ever get you hurt because we don't do the counter attack in the special attack code.

I also question how we do the input damage to that code with regards to AC mitigation. What we do now is take a random number from the min and max hit in terms of combat and then throw that into the avoid damage which has a sort of confusing mitigation formula and this gives us an illusion of real AC mitigation with the random numbers being spit out at us. Really though we should be throwing in the max value and the mitigation should be throwing us back a new value that seems random based on our pure AC from items and our attackers str+atk+offense_skill, then checking the out number versus the min hit we can make.

Then there's also critical hits, currently nothing but auto attack can crit, and the code really just makes the attack() function look messy. Special attacks should be able to crit too so what if we had something like this that we could throw in after every time we do anything with melee:

void Mob::CriticalHit(Mob* defender, int16 skillinuse, sint32 &damage)
{
	if(damage <= 0)
		return;
	//defender isn't used right now but it's nice to have incase we need to use
	//it for the future.
	float critChance = 0.0f
	int critMod = 2;
	if((GetClass() == WARRIOR || GetClass() == BERSERKER) && GetLevel() >= 12 && IsClient()) 
	{
		critChance += 0.03f
		if(CastToClient()->berserk)
		{
			critChance += 0.06f
			critMod = 4;
		}
	}

	switch(GetAA(aaCombatFury))
	{
	case 1:
		critChance += 0.02f;
		break;
	case 2:
		critChance += 0.04f;
		break;
	case 3:
		critChance += 0.07f;
		break;
	default:
		break;
	}

	critChance += ((critChance) * (spellbonuses.CriticalHitChance + itembonuses.CriticalHitChance) / 100.0f); //crit chance is a % increase to your reg chance
	if(critChance > 0)
		if(MakeRandomFloat(0, 1) <= critChance)
		{
			damage = (damage * critMod);
			if(IsClient() && CastToClient()->berserk)
			{
				entity_list.MessageClose(this, false, 200, 10, "%s lands a crippling blow!(%d)", GetCleanName(), damage);
			}
			else
			{
				entity_list.MessageClose(this, false, 200, 10, "%s scores a critical hit!(%d)", GetCleanName(), damage);
			}
		}
}

[fathernitwit]

I have always thought that the whole enabled/disabled concept was strange for ptimers. I think the best solution for them is to completely remove the concept... either they exist and are running, or they do not exist.

good eye on the CanThisClass* stuff.

as for mitigation, basically it isnt implemented for shit because nobody has been able to find reliable equations for it.

as for the special attacks/riposte stuff, we can keep patching the holes, and end up with a duplicate of Client::Attack(), but really we just need to refactor the attack functions to support special attacks, etc such that we are not duplicating code. I took the special attack code about as far as I thought was reasonable before crossing into duplication land.

I think the crit function looks reasonable, I would rename it to TryCriticalHit.

[KLS]

I've come up with what I think is a reasonable AC mitigation formula but I'm still tweaking it, I have a question though, I'm using extensive rules in it as part of the tweaking so I don't have to recompile to push a value up or down a point or two, is the overhead low enough with rules that this is okay?

Example I'm using about oh:

RULE_INT( Combat, TankACModifier, 14 )
RULE_INT( Combat, MediumTankACModifier, 11 )
RULE_INT( Combat, LightTankACModifier, 9 )
RULE_INT( Combat, PureCasterACModifier, 6 )
RULE_INT( Combat, IksarACModifier, 1 )
RULE_INT( Combat, NPCACModifier, 10 )
RULE_INT( Combat, AttackCalcModifier, 9 )
RULE_INT( Combat, ACBase, 65 )
RULE_INT( Combat, RollDiffACMod, 6 )
RULE_INT( Combat, RollDiffLevelMod, 5 )

all those per AvoidDamage call, some more than once, is there going to be some kind of speed hit where I should hard code those values in the end or is using the rules fine?

The crit function seems the most reasonable solution to me since it can later be expanded to let NPCs crit under certain circumstances, via crit buffs or pets if their owners have petcrit AAs etc; as well as being easily plugged into the special attack code.

[KLS]

Some more stuff. Mostly aggro changes and one change to corpses.

Adds Rules:
RULE_INT (Spells, SpellAggroModifier, 100)
% Aggro spells generate 100% default
RULE_INT (Spells, BardSpellAggroMod, 3)
Bard Aggro From Spells / BardSpellAggroMod
RULE_INT (Spells, PetSpellAggroMod, 10)
Pet Aggro From Spells / PetSpellAggroMod

Today was the first time I dual-boxed since I did corpses which is why I didn't really see this earlier. Was testing my AC mitigation and heal aggro and thought this was fixed.

There's this is Client:: Death()

			entity_list.AddCorpse(new_corpse, GetID());
			
			//send the become corpse packet to everybody else in the zone.
			entity_list.QueueClients(this, &app2, true);

Which should be

			entity_list.AddCorpse(new_corpse, GetID());
			SetID(0);
			//send the become corpse packet to everybody else in the zone.
			entity_list.QueueClients(this, &app2, true);

I'm not 100% on the logic behind it but I can't argue with results, without that meager line if someone is around when you die and sees you fall your corpse will immediatly disappear on their client but will be there server side and they'll have to zone in and out to see it. Should be included in the diff.

http://hmproject.org/files/Aggro.patch

I surely hope some of my previous changes make it in, I notice some haven't yet. Really think haste affecting combat timers, change to how haste is calculated, recourse and glow messages are nice changes.

Also have been working on Mitigation recently as we don't really have any real mitigation to speak of. Here's the formula I'm working with atm:

RULE_CATEGORY ( Combat )
RULE_INT( Combat, AttackCalcModifier, 15 )
RULE_INT( Combat, ACBase, 100 )
RULE_INT( Combat, RollDiffACMod, 12 ) 
RULE_INT( Combat, RollDiffLevelMod, 4 ) 
RULE_INT( Combat, MaxClientDamageMultiplier, 100 )
RULE_INT( Combat, ACRollSecondaryChance, 50 )
RULE_INT( Combat, PrimaryReductionBase, 25 )
RULE_INT( Combat, SecondaryReduction, 25 )
RULE_CATEGORY_END()

	if(damage > 0){
		int Mit_AC; 
		int Power_ATK;
		
		Mit_AC = RuleI(Combat, ACBase) + (defender->GetAC());
		Power_ATK = (attacker->GetSkill(OFFENSE) + attacker->GetSTR())*RuleI(Combat,AttackCalcModifier)/10;
		Power_ATK += attacker->GetATK();

		int Roll1, Roll2;
		int RollDiff = 0;
		Roll1 = MakeRandomInt(0, Power_ATK);
		Roll2 = MakeRandomInt(0, Mit_AC);

		if(Roll1 < Roll2){//this is success roll for mitigation
			RollDiff = Roll2 - Roll1;
			int32 PotentialReduction = RuleI(Combat, PrimaryReductionBase) + (100*((RollDiff/RuleI(Combat, RollDiffACMod))))/((20+RuleI(Combat, RollDiffLevelMod)*GetLevel()));
			int32 MinReduction = PotentialReduction/20;
			if(PotentialReduction > 100)
				PotentialReduction = 100;

			if(MinReduction > 100)
				MinReduction = 100;

			int32 DamageReduced = (damage * MakeRandomInt(MinReduction, PotentialReduction)) / 100;
			if(DamageReduced < 1)
				DamageReduced = 1;
			mlog(COMBAT__DAMAGE, "PowerATK: %d, MitAC: %d, PowerATKRoll: %d, MitACRoll: %d, Potential Red: %d, Min Red: %d, Dmg Reduced: %d", 
				Power_ATK, Mit_AC, Roll1, Roll2, PotentialReduction, MinReduction, DamageReduced);
			damage -= DamageReduced;
		}
		else if(MakeRandomInt(0, 100) <= RuleI(Combat, ACRollSecondaryChance)){
			//we fail the initial roll but get lucky and reduce damage by up to a fixed percent
			int32 DamageReduced = (damage * MakeRandomInt(0, RuleI(Combat, SecondaryReduction))) / 100;
			if(DamageReduced < 1)
				DamageReduced = 1;
			
			damage -= DamageReduced;
		}
		else{
			//we completely fail the roll /cry
		}

		if (damage < 1)
			damage = 1;
	}

It puts out some decent results, it's a pain to balance at the high level versus the low level though and I'm still working on it.

But for anyone who's interested a parse as of last night:
http://hmproject.org/files/parserogue.txt
Moderatly High AC Mob with a rogue with just under 300str and not much +atk.

OH before I forget:
Both the GetAC() and GetATK() are unsigned int values but can attempt to return signed values. Ex with an AC debuff on you can get -100 AC and it will try to return -100 AC but the return type is unsigned = bad stuff.

[fathernitwit]

havent had time to give this the attention it needs yet, but.. rules are designed to be fast, it basically works out to this much work:
rules_array[index].value

also, your change for groups seems to have caused more harm than good, people are reporting that once a group is formed, you cannot invite anybody else (which means to me that nobody is the leader, client side). Im going to revert it, in case you want to work on it more.