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.