some code logic discussion

[Congdar]

in Attack.cpp at method NPC::Attack ~line 1900 you see

	float calcheading=CalculateHeadingToTarget(target->GetX(), target->GetY());
	if((calcheading)!=GetHeading()){
		SetHeading(calcheading);
		FaceTarget(target, true);
	}

This checks to see if the npc is facing its target when it attacks. If the npc's target never moves then its not a big issue, but if the target is moving around or the target changes due to death of the target or hate list target changes then some code here is made useless and other code doesn't get run.

CalculateHeadingToTarget() in waypoints.cpp has some heavy calculations that you don't want to call too often.

sint8 Mob::CalculateHeadingToTarget(float in_x, float in_y) {
	float angle;

	if (in_x-x_pos > 0)
		angle = - 90 + atan((float)(in_y-y_pos) / (float)(in_x-x_pos)) * 180 / M_PI;
	else if (in_x-x_pos < 0)
		angle = + 90 + atan((float)(in_y-y_pos) / (float)(in_x-x_pos)) * 180 / M_PI;
	else // Added?
	{
		if (in_y-y_pos > 0)
			angle = 0;
		else
			angle = 180;
	}
	if (angle < 0)
		angle += 360;
	if (angle > 360)
		angle -= 360;
	return (sint8) (256*(360-angle)/360.0f);
}

SetHeading() makes a call to Timer::GetCurrentTime() to update pLastChange and updates heading;
Mob.h ~line 771

	void				SetHeading(float iHeading) { if (heading != iHeading) { pLastChange = Timer::GetCurrentTime(); heading = iHeading; } }

FaceTarget() makes a check that there's a target and then calls CalculateHeadingToTarget() a second time but since heading was set in SetHeading is is the same as what it gets from the extra call and the code at the bottom of the method sends a graphical update to every client in the zone and oldheading == heading because of the previous call to SetHeading() so pLastChange doesn't get updated.
in Mob.cpp ~line 1928 is Mob::FaceTarget()

void Mob::FaceTarget(Mob* MobToFace, bool update) {
	if (MobToFace == 0)
		MobToFace = target;
	if (MobToFace == 0 || MobToFace == this)
		return;
	// TODO: Simplify?
	float oldheading = heading;
	heading=(CalculateHeadingToTarget(MobToFace->GetX(),MobToFace->GetY()));

	// 30+ lines of rem'd out code removed for easier reading

	SendPosUpdate();
	if (update && oldheading != heading)
		pLastChange = Timer::GetCurrentTime();
}

current execution summed up:
calc heading to target
if not facing target set heading to face target
calc heading to target again
update all(800 * 800 is a long ways) clients in zone of graphical heading change
don't update pLastChange even if bool is set to true

To clean this code up, I'm suggesting this:
calc heading to target
if not facing target set heading to face target
if heading changed send graphical update to clients

by changing:

	float calcheading=CalculateHeadingToTarget(target->GetX(), target->GetY());
	if((calcheading)!=GetHeading()){
		SetHeading(calcheading);
		FaceTarget(target, true);
	}

to:

             FaceTarget(target);

and changing FaceTarget() to
mob.h

	void				FaceTarget(Mob* MobToFace = 0);

void Mob::FaceTarget(Mob* MobToFace) {
	if(!MobToFace) {
		if(!target) {
			return;
		}
		else {
			MobToFace = target;
		}
	}

	float oldheading = heading;
	heading = CalculateHeadingToTarget(MobToFace->GetX(), MobToFace->GetY());
	if(oldheading != heading) {
		SetHeading(heading);
		SendPosUpdate();
	}
}

SetHeading() updates pLastChange but FaceTarget() had a bool to change it or not so what cases would pLastChange not need to be updated?