Some stuff

[KLS]

Gonna try to keep most of my future small posts in this thread to avoid my name over-running the development forum.

Was rather quiet in IRC tonight so I'm gonna post the ideas I came up with tonight.

Looking at the recourse code and something seems not quite right with it, I use a SK for testing a lot and noticed torrent of pain doesn't hit either my self or my group, I think it's because of how the target types are done with the recourse code. Also I think it's not well placed in the casting sequence; the recourse code is put in before the resist check so even if your target resists the spell the recourse will still go off and this seems wrong to me.

Reference Spells:
http://lucy.allakhazam.com/spell.html?id=2486 Group v1 (ST_GroupTeleport)
http://lucy.allakhazam.com/spell.html?id=2480 Group v1 (ST_GroupTeleport)


Recourse was in Mob::SpellFinished():

at the top:

int recourse_spell=0;

little further down approx ln 1423

// Recourse means there is a spell linked to that spell in that the recourse spell will
	// be automatically casted on the casters group or the caster only depending on Targettype
	// solar: this is for things like dark empathy, shadow vortex
	recourse_spell = spells[spell_id].RecourseLink;
	if(recourse_spell != 0)
	{
		if(spells[recourse_spell].targettype == ST_Group) {
			if(IsGrouped()) {
				Group *g = entity_list.GetGroupByMob(this);;
				g->CastGroupSpell(this, recourse_spell);
			} else {
				SpellOnTarget(recourse_spell, this);
#ifdef GROUP_BUFF_PETS
				if (HasPet())
					SpellOnTarget(recourse_spell, GetPet());
#endif
			}
		} else if(spells[recourse_spell].targettype == ST_GroupTeleport) {
		// EverHood - Necro Epic 2 Pet Proc Recourse
			if(HasOwner()) {
				if(GetOwner()->IsGrouped()) {
					Group *g = entity_list.GetGroupByMob(this->GetOwner());
					g->CastGroupSpell(this, recourse_spell);
				} else {
					SpellOnTarget(recourse_spell, this->GetOwner());
				}
			}
		} else {
			SpellOnTarget(recourse_spell, this);
		}
	}

First I'd move it down to spellontarget() I suppose, right after the resist check to make sure the spell landed before the recourse goes off.

Then I'd change the code a bit to be something like:

// Recourse means there is a spell linked to that spell in that the recourse spell will
	// be automatically casted on the casters group or the caster only depending on Targettype
	// solar: this is for things like dark empathy, shadow vortex
	int recourse_spell=0;
	recourse_spell = spells[spell_id].RecourseLink;
	if(recourse_spell)
	{
		if(spells[recourse_spell].targettype == ST_Group || spells[recourse_spell].targettype == ST_GroupTeleport)
		{
			if(IsGrouped())
			{
				Group *g = entity_list.GetGroupByMob(this);
				g->CastGroupSpell(this, recourse_spell);
			}
			else if(HasOwner())
			{
				if(GetOwner->IsGrouped())
				{
					Group *g = entity_list.GetGroupByMob(GetOwner());
					g->CastGroupSpell(this, recourse_spell);
				}

			}
			else
			{
				SpellOnTarget(recourse_spell, this);
#ifdef GROUP_BUFF_PETS
				if (HasPet())
					SpellOnTarget(recourse_spell, GetPet());
#endif
			}	

		}
		else
		{
			SpellOnTarget(recourse_spell, this);
		}
	}

Another thing I want to change is a bit of code in Client::GetFocusEffect() because the focus is calculated even for duration spells, like it should be but you always get the glow message as well which is really annoying, you should really only be getting it for non duration spells, heals, nukes etc.

you'll find the lines

if (realTotal > 0 && UsedItem) {
		Message_StringID(MT_Spells, BEGINS_TO_GLOW, UsedItem->Name);
	}

I just replaced it with

if (realTotal > 0 && UsedItem && spells[spell_id].buffduration == 0) {
		Message_StringID(MT_Spells, BEGINS_TO_GLOW, UsedItem->Name);
	}

I think that should be effective in eliminating redundant spell focus spam.

Also I noticed in the last version I pulled up from source the first few lines in Client::Attack() remained unchanged despite what the changelog says. Really I think the first log statement needs to be changed because of it's potential to cause a crash.

bool Client::Attack(Mob* other, int Hand, bool bRiposte)
{
	_ZP(Client_Attack);
	
	mlog(COMBAT__ATTACKS, "Attacking %s with hand %d %s", other->GetName(), Hand, bRiposte?"(this is a riposte)":"");
	
	//SetAttackTimer();
	if (
		   (IsCasting() && GetClass() != BARD)
		|| other == NULL
		|| ((IsClient() && CastToClient()->dead) || (other->IsClient() && other->CastToClient()->dead))
		|| (GetHP() < 0)
		|| (!IsAttackAllowed(other))
		) {
		mlog(COMBAT__ATTACKS, "Attack canceled, invalid circumstances.");
		return false; // Only bards can attack while casting
	}

Perhaps it could be changed to something like

bool Client::Attack(Mob* other, int Hand, bool bRiposte)
{
	_ZP(Client_Attack);
	
	//SetAttackTimer();
	if (
		   (IsCasting() && GetClass() != BARD)
		|| other == NULL
		|| ((IsClient() && CastToClient()->dead) || (other->IsClient() && other->CastToClient()->dead))
		|| (GetHP() < 0)
		|| (!IsAttackAllowed(other))
		) {
		mlog(COMBAT__ATTACKS, "Was trying to attack someone with hand %d, but the attack was canceled because of invalid circumstances.", Hand);
		return false; // Only bards can attack while casting
	}
	else{
		mlog(COMBAT__ATTACKS, "Attacking %s with hand %d %s", other->GetName(), Hand, bRiposte?"(this is a riposte)":"");
	}

This would make sure we verify other before we attempt to access it, avoiding a potential crash.

Also in Client::Message()

void Client::Message(uint32 type, const char* message, ...) {
	va_list argptr;
	char *buffer = new char[4096];
	
	if (GetFilter(FilterSpellDamage) == FilterHide && type == MT_NonMelee)
		return;
	if (GetFilter(FilterMeleeCrits) == FilterHide && type == MT_CritMelee) //98 is self...
		return;
	if (GetFilter(FilterSpellCrits) == FilterHide && type == MT_SpellCrits)
		return;
	
	va_start(argptr, message);
	vsnprintf(buffer, 4096, message, argptr);
	va_end(argptr);
	
	uint32 len = strlen(buffer);
	
	//client dosent like our packet all the time unless
	//we make it really big, then it seems to not care that
	//our header is malformed.
	//len = 4096 - sizeof(SpecialMesg_Struct);
	
	uint32 len_packet = sizeof(SpecialMesg_Struct)+len;
	EQApplicationPacket* app = new EQApplicationPacket(OP_SpecialMesg, len_packet);
	SpecialMesg_Struct* sm=(SpecialMesg_Struct*)app->pBuffer;
	sm->header[0] = 0x00; // Header used for #emote style messages..
	sm->header[1] = 0x00; // Play around with these to see other types
	sm->header[2] = 0x00;
	//sm->msg_type = type;
	sm->msg_type = 0x0A;

Is there any reason that we fix the type to 0x0A? And I'm guessing it's preferable to use the preformatted messages where it's possible as well, since I'm guessing the packet they send is smaller?

Don't have time to compile and test this stuff tonight, I'll try to get around to it tomorrow but in the mean time if you see anything wrong with my ideas feel free to let me have it.

[KLS]

Got a little work done tonight, nothing I wrote about above, was more looking at the endurance implementation. Some things:

The calc max endurance formula used is wrong, while trying to simplify it certainly makes it easier to read it also leads it to some pretty bad rounding errors. My warrior lost a good 800 endurance with your simplified version.
This is the new version I wrote that didn't use non standard min/max() and added in commenting so the logic is understandable

//Info taken from magelo, it's a *little* off but accurate enough.
void Client::CalcMaxEndurance()
{
	//"Stats" the total of (Str + Dex + Sta + Agi)
	int Stats = GetSTR()+GetSTA()+GetDEX()+GetAGI();

	//"Levelbonus" your Level * .075
	//Endurance = level * 15 plus

	//Levelbonus times the sum of the next 4 lines (this is calculated on each line, not at the end because of rounding errors otherwise)
	max_end = GetLevel() * 15;
	//plus lesser of Stats and 800, divide that by 4.
	max_end += int((Stats>800?800:Stats)/4)*0.075*GetLevel();
	//plus bigger of (lesser of Stats and 800)-400, and 0. all of that /4
	max_end += int((((Stats>800?800:Stats)-400)>0?((Stats>800?800:Stats)-400):0)/4)*(0.075*GetLevel());
	//plus bigger of (lesser of Stats and 800)-400, and 0. all of that /8
	max_end += int((((Stats>800?800:Stats)-400)>0?((Stats>800?800:Stats)-400):0)/8)*(0.075*GetLevel());
	//plus bigger of (Stats - 800 and zero) / 8
	max_end += int((Stats>800?Stats:0)/8)*(0.075*GetLevel())*2;
	//plus bigger of (Stats - 800 and zero) / 16 
	max_end += int((Stats>800?Stats:0)/16)*(0.075*GetLevel());
	//plus our endurance from items and spells.
	max_end += spellbonuses.Endurance + itembonuses.Endurance;
	//Maurice of Magelo fame explained that we can't simplify the statements 
	//because we have to round every step of the way.
}

Also something appears up with how stats are calculated for clients, I'm not sure exactly where there's a problem but well let me show you:

Sarrie 65 War naked
http://i8.photobucket.com/albums/a2/...rite/naked.jpg

Sarrie 65 War with her full gear on
http://i8.photobucket.com/albums/a2/...e/notnaked.jpg

It appears almost like itembonuses are being counted twice serverside. Ex. Sarrie had +86 wisdom fully geared and had a base of 70 shoulda been 156 serverside like clientside but was 242. 70 + (86*2) = 242. And obviously stats aren't capped correctly which is gonna mess with hp/mana/endurance calculations, among other things.

Not exactly sure what causes it though.
Also though I haven't gotten around to testing it I'm pretty sure this is the case:

In the itemfield encoding for titanium I believe

/* 083 */	I(AugSlotType[0])
/* 084 */	I(AugSlotUnk[0])
/* 085 */	I(AugSlotType[1])
/* 086 */	I(AugSlotUnk[1])
/* 087 */	I(AugSlotType[2])
/* 088 */	I(AugSlotUnk[2])
/* 089 */	I(AugSlotType[3])
/* 090 */	I(AugSlotUnk[3])
/* 091 */	I(AugSlotType[4])
/* 092 */	I(AugSlotUnk[4])

is actually backwards with the unknown spot coming before the slot, I believe that's why aug slots don't appear on items for titanium.

I'll try to look into all this a little bit more and get some more info.

[KLS]

//Info taken from magelo, it's a *little* off but accurate enough.
void Client::CalcMaxEndurance()
{
	//"Stats" the total of (Str + Dex + Sta + Agi)
	int Stats = GetSTR()+GetSTA()+GetDEX()+GetAGI();

	//"Levelbonus" your Level * .075
	//Endurance = level * 15 plus

	//Levelbonus times the sum of the next 4 lines (this is calculated on each line, not at the end because of rounding errors otherwise)
	max_end = GetLevel() * 15;
	//plus lesser of Stats and 800, divide that by 4.
	max_end += int((Stats>800?800:Stats)/4)*0.075*GetLevel();
	//plus bigger of (lesser of Stats and 800)-400, and 0. all of that /4
	max_end += int((((Stats>800?800:Stats)-400)>0?((Stats>800?800:Stats)-400):0)/4)*(0.075*GetLevel());
	//plus bigger of (lesser of Stats and 800)-400, and 0. all of that /8
	max_end += int((((Stats>800?800:Stats)-400)>0?((Stats>800?800:Stats)-400):0)/8)*(0.075*GetLevel());
	//plus bigger of (Stats - 800 and zero) / 8
	max_end += int(((Stats-800)>0?(Stats-800):0)/8)*(0.075*GetLevel())*2;
	//plus bigger of (Stats - 800 and zero) / 16 
	max_end += int(((Stats-800)?(Stats-800):0)/16)*(0.075*GetLevel());
	//plus our endurance from items and spells.
	max_end += spellbonuses.Endurance + itembonuses.Endurance;
	//Maurice of Magelo fame explained that we can't simplify the statements 
	//because we have to round every step of the way.
}

Was two typos in the calcmaxend I made, what I get for working while tired.

The double stats comes from functions that use the Mob::Get<somestat>() that instead of return <somestat>; return <somestat> + itembonuses.<somestat> + spellbonuses.<somestat>. Not what solution there would be other than making sure code that involves clients uses client::getstat instead of mob::getstat in the code, which will probably be a little tedious given the amount of times they're referenced.

The aug thing, I'm not sure how they're serialized in the code yet so take that with a grain of salt but I confirmed when I set in the database:

augslottype1 = 0;
augslot1unk = 7;
That I created an item with an aug slot of 7 clientside, trying to apply it to an item was a little troublesome. I successfully inserted the augment into the item, however after you finish the insert the items in the pool are not deleted immediatly and attempting to remove them causes the client to crash, so obviously some work to be done there.

The glow message thing appears to work, and tested recourse; took out old recourse code and replaced it with this in spellontarget rightbelow where resists are calculated.

	if(spell_effectiveness == 100)
	{
			// Recourse means there is a spell linked to that spell in that the recourse spell will
		// be automatically casted on the casters group or the caster only depending on Targettype
		// solar: this is for things like dark empathy, shadow vortex
		int recourse_spell=0;
		recourse_spell = spells[spell_id].RecourseLink;
		if(recourse_spell)
		{
			if(spells[recourse_spell].targettype == ST_Group || spells[recourse_spell].targettype == ST_GroupTeleport)
			{
				if(IsGrouped())
				{
					Group *g = entity_list.GetGroupByMob(this);
					g->CastGroupSpell(this, recourse_spell);
				}
				else if(HasOwner())
				{
					if(GetOwner()->IsGrouped())
					{
						Group *g = entity_list.GetGroupByMob(GetOwner());
						g->CastGroupSpell(this, recourse_spell);
					}

				}
				else
				{
					SpellOnTarget(recourse_spell, this);
#ifdef GROUP_BUFF_PETS
					if (HasPet())
						SpellOnTarget(recourse_spell, GetPet());
#endif
				}	

			}
			else
			{
				SpellOnTarget(recourse_spell, this);
			}
		}
	}

[KLS]

Now some actual changes:

-Fixed the atk mlog bug, again.
-Changed how recourse works
-Made glow messages only go off for instant spells
-Changed how haste is calculated to account for caps and stacking correctly and to make sure we account for overhaste spells(ex warsong of the vahshir). Should fix one of the current dev issues with wonderous rapidity and nature's melody etc etc.
-The way hundredhands is calculated isn't correct, but I wasn't sure how to address it so I kept it the way it was implemented for now but replacing the true/false with actual effect values.
-Changed the end calculations slightly (but in a big way) GetLevel()*0.075 is added in every line to address some pretty massive rounding errors if you try to simplify it, we're talking a loss of 8-900 endurance when you get up in pop gear kinda rounding errors.
-Special Messages should be able to show a type other than 0x0A.

And svn created me a nifty little patch file
http://hmproject.org/files/haste.patch

[KLS]

Another quick change. Did this pretty quickly so tell me if I messed any part of it up.

Dunno if it bothers anyone else as much as it bothers me but the small quickly used timers for the hard coded client skills keep bugging out on me, my sneak is down for the next 11 days according to my database for instance =(

Not sure at all what causes these short timers to bug out so I added a rule here on whether or not to enforce them, I mean do we really need to enforce them? They're hard coded into the client and it's going to be pretty rare that someone hacks into the client to change the timers, most are only a few seconds anyway.

Also threw in another rule that helps server admins modify the endurance regen since there really isn't any way to raise your regen outside of +endurance regen items, and def seems like something admins might want to change, just like an exp modifier.

new rules:

RULE_BOOL( Character, EnforceSkillTimers, false )
RULE_REAL( Character, EnduranceRegenBonus, 1.0 )

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


Also would like to add that I love this rule system, it's a brilliant concept.

People will hack the client for the timers, trust me. Instant Back Stab for instance.

This was added in because of the amount of people that were hacking the client.

[vales]

I love you man!

These are some seriously bad-ass fixes. I'm gonna patch up the server and see the changes. Will report back if I see something out of the ordinary.

Keep up the awesome work!