some code logic discussion

**Congdar** · #1 10-25-2008, 03:07 PM

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

Code:

	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.

Code:

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

Code:

	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()

Code:

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:

Code:

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

to:

Code:

             FaceTarget(target);

and changing FaceTarget() to
mob.h

Code:

	void				FaceTarget(Mob* MobToFace = 0);

Code:

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?

**Congdar** · #2 10-25-2008, 03:30 PM

It seems CalculateHeadingToTarget() returns a sint8
Everywhere it is used as a float though since heading is a float.
Also, in FaceTarget() heading is getting updated regardless of it needing to be updated or not because it is getting set directly since it is not a private or protected variable so here's an additional change for FaceTarget()

Code:

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

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

Would changing the return value of CalculateHeadingToTarget() be as simple as this:
waypoints.cpp

Code:

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

to:

Code:

float Mob::CalculateHeadingToTarget(float in_x, float in_y) {

and

Code:

	return (sint8) (256*(360-angle)/360.0f);

to:

Code:

	return (256*(360-angle)/360.0f);

**Congdar** · #3 10-26-2008, 01:46 PM

the only place that seems to do some kind of calc to determine if true or false should be sent to FaceTarget() method is in perl_mob.cpp

Code:

XS(XS_Mob_FaceTarget); /* prototype to pass -Wmissing-prototypes */
XS(XS_Mob_FaceTarget)
{
	dXSARGS;
	if (items < 1 || items > 3)
		Perl_croak(aTHX_ "Usage: Mob::FaceTarget(THIS, MobToFace= 0, bool update)");
	{
		Mob *		THIS;
		Mob*		MobToFace;
		bool		update;

		if (sv_derived_from(ST(0), "Mob")) {
			IV tmp = SvIV((SV*)SvRV(ST(0)));
			THIS = INT2PTR(Mob *,tmp);
		}
		else
			Perl_croak(aTHX_ "THIS is not of type Mob");
		if(THIS == NULL)
			Perl_croak(aTHX_ "THIS is NULL, avoiding crash.");

		if (items < 2)
			MobToFace = 0;
		else {
			if (sv_derived_from(ST(1), "Mob")) {
				IV tmp = SvIV((SV*)SvRV(ST(1)));
				MobToFace = INT2PTR(Mob *,tmp);
			}
			else
				Perl_croak(aTHX_ "MobToFace is not of type Mob");
			if(MobToFace == NULL)
				Perl_croak(aTHX_ "MobToFace is NULL, avoiding crash.");
		}

		if (items < 3)
			update = false;
		else {
			update = (bool)SvTRUE(ST(2));
		}

		THIS->FaceTarget(MobToFace, update);
	}
	XSRETURN_EMPTY;
}

Im not familiar with this part of the code, can somebody comment on this? I didn't see quest::FaceTarget() used in any perl files. Is that what this is for?

**trevius** · #4 10-26-2008, 04:44 PM

That looks like a quest object, as apposed to a normal quest:: command.

$mob->FaceTarget(MobToFace= 0, update= false)

Quest Objects
http://www.eqemulator.net/wiki/wikka...a=QuestObjects

Quest Commands
http://www.eqemulator.net/wiki/wikka...=QuestTutorial

**KLS** · #5 10-27-2008, 02:16 AM

Trev is right it's a Mob object exported to perl. I don't see any problems with what's posted so far.

**Congdar** · #6 10-27-2008, 10:16 AM

The confusing thing is that the update portion is to force the update of the pLastChange variable but this variable seems to be only used by Clients and not NPCs. Not sure why it's even needed for $mob-> except for the target facing part.