Fix for mobs and pet movement - Updated

[Valdain]

I've been messing around with the code to make mobs move smoothly instead of jumping on top of you. I re-wrote portions of the Mob::CalculateNewPosition to use a more refined approach to moving. Also, I changed all references to CalculateNewPosition2 to use CalculateNewPosition.

This was developed and tested on 0.5.3 DR2 code.

In MobAI.cpp, just drop in this code to replace the old function:

bool Mob::CalculateNewPosition(float x, float y, float z, float speed) { 
   float ratio; 
   float target_distance;

   if(GetID()==0) 
      return true; 

    // if NPC is rooted 
    if (speed == 0.0) { 
        SetHeading(CalculateHeadingToTarget(x, y)); 
      if(moved){ 
         SendPosition(); 
         SetMoving(false); 
         moved=false; 
      } 
      SetRunAnimSpeed(0); 
        return true; 
    } 

   float nx = this->x_pos; 
   float ny = this->y_pos; 
   float nz = this->z_pos; 
   float nh = this->heading; 

   tarx=x; 
   tary=y; 
   tarz=z; 
    
	tar_vx = x - nx; 
    tar_vy = y - ny; 
    tar_vz = z - nz; 

      //If we are at our destination, then we don't need move!
	  if (tar_vx == 0 && tar_vy == 0 && tar_vz==0)
         return false; 

      pRunAnimSpeed = (sint8)(speed*30); 
	  
	  speed *= 2.0f; //The 2.0 is adjustable to get the speed just right.

      target_distance = sqrt (tar_vx*tar_vx + tar_vy*tar_vy + tar_vz*tar_vz); 
      heading = CalculateHeadingToTarget(x, y); 

  	  if(target_distance >= speed)  // If we need to go father than we can in one cycle... 
      { 
	     ratio = (speed/target_distance);
         x_pos += tar_vx * ratio;
         y_pos += tar_vy * ratio;
 		 z_pos += tar_vz * ratio;
	  } 
      else //If we are with-in our one cycle distance... 
      { 
         x_pos = x; 
         y_pos = y; 
         z_pos = z;  
      } 
       
      //OP_MobUpdate 
      APPLAYER* outapp = NULL; 
      PlayerPositionUpdateServer_Struct* spu = NULL; 
      this->SetMoving(true); 
      moved=true; 
      outapp = new APPLAYER(OP_ClientUpdate, sizeof(PlayerPositionUpdateServer_Struct)); 
      spu = (PlayerPositionUpdateServer_Struct*)outapp->pBuffer; 
      delta_x=x_pos-nx; 
      delta_y=y_pos-ny; 
      delta_z=z_pos-nz; 
      delta_heading=heading-nh; 
      MakeSpawnUpdate(spu); 
      spu->heading*=8; 
      entity_list.QueueCloseClients(this, outapp, true, 300); 
      safe_delete(outapp); 
 
    SetAppearance(0, false);
    pLastChange = Timer::GetCurrentTime(); 
    return true; 
}

Another quick fix helps smooth out pet movement.

In MobAI.cpp, in the Mob::AI_Process function around line 892

		else if (AImovement_timer->Check() && !IsRooted()) {
------------>>>>>    //SetRunAnimSpeed(0);
			if (GetOwnerID()) {
				// we're a pet, do as we're told

By commenting out the SetRunAnimSpeed(0), the pet seems to move a little smoother, don't know why.

UPDATE - This is probably as good as it will get. Until the server can know the Z coordinate for each X,Y (or at least be pretty close), the mob will not handle large vertical changes between 2 way points that are far apart, such as steep stairs. My suggestion is to make a way point at the top and bottom of such things as stairs and hills.

I've tested the code out on Windows with great success. There is still some refining that needs to be done, but this gets us alot closer to properly moving creatures.

-Valdain

[Edit] - Fixed code after talking with Doodman

[Scorpious2k]

I'm going to give this a try on the Scorpious2k Server wityh a couple of exceptions:

The Z coord calculation is important. In hilly areas or where a dest is lower than the start position you will find NPCs walking on air, This is not good. And if the dest is lower, either the NPC moves underground or the client goes nuts resetting the position to the surface. For these reasons, I uncommented the line where the Z values are calculated,

My other change for our purposes was to change Mob::CalculateNewPosition2 to

	return CalculateNewPosition(x, y, z, speed);

for testing purposes rather then change all ocurrences of it. We make mods here with #ifdef... #endif so we can unplug them easily if there are problems... I do a lot of testing. Usually on our miniserver before the live one.

We will be trying this today.

[Valdain]

I noticed in my testing that "gravity" is applied to the mob elsewhere and you don't get floating or "buried" symptoms on the client. Let me know how adding the Z calcs back in work for you. I'm just getting familiar with the code base again, and there have been lots of updates since a year ago.

As for the function naming, it really isn't consistant in the code between CalculateNewPosition and CalculateNewPosition2, but the code is the same in both. My main goal was just to improve the function. Feel free to adjust to however needed. I know most coders will #ifdef to allow for easier testing.

Let me know how it goes for you and thanks for taking a look at it!
-Valdain

[Scorpious2k]

Originally Posted by Valdain

As for the function naming, it really isn't consistant in the code between CalculateNewPosition and CalculateNewPosition2, but the code is the same in both.

I noticed that too. For testing purposes, it was just easier to do it the way I did.

The change is live on our server. Feel free to drop in and check it out. That goes for anyone else who may be curious about the this change.

[Valdain]

I notice with the Z calcs back in the mob will do a little fall every once in a while. The right way to fix this is to determine what the Z coord is at the new position and set it that way, but I have no idea how to do that.

To see what I mean, attack a mob, then walk backwards away from it. It seems to be most prenounced when it is chasing you uphill.

-Valdain

[Doodman]

One note:

...
...
      ratio = (speed/target_distance);

   	  if(target_distance >= speed)  // If we need to go father than we can in one cycle... 
      { 
         x_pos += tar_vx * ratio;
         y_pos += tar_vy * ratio;
		 if(tar_vz>2)
			z_pos += tar_vz * ratio;
		 else
			z_pos = z;
      } 
      else //If we are with-in our one cycle distance... 
      { 
         x_pos = x; 
         y_pos = y; 
         z_pos = z;  
      } 
...
...

Should be:

...
...
   	  if(target_distance >= speed)  // If we need to go father than we can in one cycle... 
      { 
         ratio = (speed/target_distance);

         x_pos += tar_vx * ratio;
         y_pos += tar_vy * ratio;
		 if(tar_vz>2)
			z_pos += tar_vz * ratio;
		 else
			z_pos = z;
      } 
      else //If we are with-in our one cycle distance... 
      { 
         x_pos = x; 
         y_pos = y; 
         z_pos = z;  
      } 
...
...

Since ratio is only used in there, no point in wasting cycles on the division.

[Doodman]

Originally Posted by Valdain

[Edit] - Minor update to handle rare cases where very small distances messed up the function.

Describe "messed up". All that * and / is expensive in there. So, it'll have a big impact.

[Armanthuz]

This seemed to work very well for me...


One small problem i noticed thatmay or may not be related is lets say i fight a mob then mem blur him, when mob walks back to spawn spot, i can see him walk about 5-10 paces, the mob usually keeps walking right past spawn spot then dissapears until next reboot.

[Trumpcard]

Scorp, since you applied this to your server and checked it out, do you mind investigating merging it in?

[Scorpious2k]

Originally Posted by Trumpcard

Scorp, since you applied this to your server and checked it out, do you mind investigating merging it in?

Will do.

[Scorpious2k]

One little snag....

The original code this was designed to replace has been redone. Mob::CalculateNewPosition and Mob::CalculateNewPosition2 are no longer identical. Major changes have been made while this discussion was ocurring, making a direct merge of the new code impossible.

It looks to me like many of the ideas have been applied, although not as a merge of the code. Here is what we have now:

bool Mob::CalculateNewPosition(float x, float y, float z, float speed) {
	if(GetID()==0)
		return true;
	/*if(tar_ndx<20 && tarx==x && tary==y && tarz==z){
		tar_ndx++;
		x_pos = x_pos + tar_vx*tar_vector;
		y_pos = y_pos + tar_vy*tar_vector;
		z_pos = z_pos + tar_vz*tar_vector;
		return true;
	}
	else{
		tar_ndx=0;
		tarx=x;
		tary=y;
		tarz=z;
	}*/
    float nx = x_pos;
    float ny = y_pos;
    float nz = z_pos;
	float nh = heading;
	
    // if NPC is rooted
    if (speed == 0.0) {
        SetHeading(CalculateHeadingToTarget(x, y));
		if(moved){
			SendPosition();
			SetMoving(false);
			moved=false;
		}
		SetRunAnimSpeed(0);
        return true;
    }
		float old_test_vector=test_vector;
		tar_vx = x - nx;
		tar_vy = y - ny;
		tar_vz = z - nz;

		if (tar_vx == 0 && tar_vy == 0 && tar_vz == 0)
			return false;
		pRunAnimSpeed = (int8)(speed*43);
		speed *= 2.8;
		// --------------------------------------------------------------------------
		// 2: get unit vector
		// --------------------------------------------------------------------------
		test_vector=sqrt (x*x + y*y + z*z);
		tar_vector = speed / sqrt (tar_vx*tar_vx + tar_vy*tar_vy + tar_vz*tar_vz);
		heading = CalculateHeadingToTarget(x, y);

		if (tar_vector >= 1.0) {
			x_pos = x;
			y_pos = y;
			z_pos = z;
		}
		else {
			x_pos = x_pos + tar_vx*tar_vector;
			y_pos = y_pos + tar_vy*tar_vector;
			z_pos = z_pos + tar_vz*tar_vector;
		}
		//OP_MobUpdate
		if((old_test_vector!=test_vector) || tar_ndx>20){ //send update
			tar_ndx=0;
			APPLAYER* outapp = NULL;
			PlayerPositionUpdateServer_Struct* ppu = NULL;
			this->SetMoving(true);
			moved=true;
			outapp = new APPLAYER(OP_ClientUpdate, sizeof(PlayerPositionUpdateServer_Struct));
			ppu = (PlayerPositionUpdateServer_Struct*)outapp->pBuffer;
			delta_x=(x_pos-nx);
			delta_y=(y_pos-ny);
			delta_z=(z_pos-nz);
			delta_heading=0;//(heading-nh)*8;
			MakeSpawnUpdate(ppu);
			ppu->heading*=8;
			entity_list.QueueCloseClients(this, outapp, true, 300);
			safe_delete(outapp);
		}
		tar_ndx++;
    // now get new heading
	SetAppearance(0, false); // make sure they're standing
    pLastChange = Timer::GetCurrentTime();
    return true;
}

and

bool Mob::CalculateNewPosition2(float x, float y, float z, float speed) {
	if(GetID()==0)
		return true;
	if(tar_ndx<20 && tarx==x && tary==y && tarz==z){
		x_pos = x_pos + tar_vx*tar_vector;
		y_pos = y_pos + tar_vy*tar_vector;
		z_pos = z_pos + tar_vz*tar_vector;
		tar_ndx++;
		return true;
	}
	else{
		tar_ndx=0;
		tarx=x;
		tary=y;
		tarz=z;
	}
	float nx = this->x_pos;
    float ny = this->y_pos;
    float nz = this->z_pos;
	float nh = this->heading;
	
	tar_vx = x - nx;
	tar_vy = y - ny;
	tar_vz = z - nz;

	if (tar_vx == 0 && tar_vy == 0 && tar_vz == 0)
		return false;
	pRunAnimSpeed = (sint8)(speed*36);
	speed *= 46;
	// --------------------------------------------------------------------------
	// 2: get unit vector
	// --------------------------------------------------------------------------
	tar_vector = speed / sqrt (tar_vx*tar_vx + tar_vy*tar_vy + tar_vz*tar_vz);
	heading = CalculateHeadingToTarget(x, y);

	if (tar_vector >= 1.0) {
		x_pos = x;
		y_pos = y;
		z_pos = z;
	}
	else {
		tar_vector/=20;
		x_pos = x_pos + tar_vx*tar_vector;
		y_pos = y_pos + tar_vy*tar_vector;
		z_pos = z_pos + tar_vz*tar_vector;
	}
	APPLAYER* outapp = NULL;
	PlayerPositionUpdateServer_Struct* spu = NULL;
	this->SetMoving(true);
	moved=true;
	outapp = new APPLAYER(OP_ClientUpdate, sizeof(PlayerPositionUpdateServer_Struct));
	spu = (PlayerPositionUpdateServer_Struct*)outapp->pBuffer;
	delta_x=x_pos-nx;
	delta_y=y_pos-ny;
	delta_z=z_pos-nz;
	delta_heading=heading-nh;
	MakeSpawnUpdate(spu);
	spu->heading*=8;
	entity_list.QueueCloseClients(this, outapp, true, 300);
	safe_delete(outapp);
	SetAppearance(0, false);
    pLastChange = Timer::GetCurrentTime();
    return true;
}

I think you can see a lot of the same ideas being applied. One interesting addition is the supression of update packets here, which should help.

So, should we try to merge in the logic from each version? Merge the code anyway? Test both versions and pick the best?

Opinions?