What's up, guys.

I want to make my enemy patrol around some area and I don't like the code I've written.

My function is:

void AI::patrol( glm::vec3 startPoint, glm::vec3 endPoint, float speed )
{
   //Find out on which direction to move towards.
   glm::vec3 movementDirection = glm::normalize( endPoint - startPoint );

   //Move the player to that direction with a certain speed.
   mainCam.setCurrentPosition( mainCam.getCurrentPosition() + movementDirection*speed );

   /*Now the problem is that I need to somehow say: If the enemy is already at the
     end of the route, turn in the opposite direction.
   
    If the game was one-dimensional, I could just compare startPoint and endPoint, find
    the biggest one, and check if I'm between the two points. But it's not.

    The only way that comes to mind is to do the same thing that I do for 1D, but do it 3 times for 3D.
    This is to find the biggest x,y,z and the smallest x,y and z
    among the two vectors and check if I'm between them (as the code below ) */

    glm::vec3 pos = mainCam.getCurrentPosition() //too much writing. stupid getters.
    if( pos.x < findMax( startPoint.x, endPoint.x ) && pos.x > findMin( startPoint.x, endPoint.x ) &&
        pos.y < findMax( startPoint.y, endPoint.y ) && pos.y > findMin( startPoint.y, endPoint.y ) &&
        pos.z < findMax( startPoint.z, endPoint.z ) && pos.z > findMin( startPoint.z, endPoint.z )
    {
       //keep walking;
    }
    else movementDirection *= -1;

}

This is kind of messed up, because I can get some kind of floating-point errors.

Is there another way of doing this?

I don't understand this function, or your last 2 paragraphs in that comment. What is startPoint, and why is it relevant to the direction of movement? Why is there a 'mainCam' object involved - that sounds like a camera, but this is supposed to be AI.

Patrol code is normally quite straightforward - you have a list of patrol points, you keep track of which one you're aiming at next, you move towards that point each update, and when you're close enough, you switch to moving towards the next point. When you run out of points, you start from the beginning again.

key point for your problem that Kylotan stated is that "when you're close enough". When you are getting your movement direction in endPoint - startPoint, don't get the normalized version right away. First get the magnitude and that will tell you how close you are. If it is within a certain threshold, then change your endPoint to be the next point.

furthermore, the else movementDirection *= -1; at the end of your function does no good at all since movementDirection is a local variable in the function scope, it looses all meaning after the function returns. The next time the function executes it will not be as if movementDirection had been reversed.

Without seeing the code, if your trying to learn "pathfinding", you could start with defining waypoints, then let the npc move from A to B, then when it arrives at a waypoint, set the next waypoint based on the target of the npc (i.e distance to what it's chasing). When it works you can improve it by increasing the number of waypoints.

This is not the "best" way, but a good starting point/ learning curve.

I don't understand this function, or your last 2 paragraphs in that comment. What is startPoint, and why is it relevant to the direction of movement?

startPoint and endPoint are just the 2 points that the the enemy patrols around.

Enemy needs to know the direction, so it can go from startPoint to endPoint.

Why is there a 'mainCam' object involved - that sounds like a camera, but this is supposed to be AI.

When I started coding my first 3D game, the first class I made was Camera, because it seemed really cool to me to actually render 3d view at the time. And that camera had position, rotation, scale, direction, model, view, projection matrices and all. Then I had to write the same code within the Character class, because characters again need position, rotation and all that stuff. I was too lazy to copy/paste all the getters and setters and stuff from Camera to Character, that's why I just instantiated one Camera in Character to hold positions,matrices and so on.

I spent 5 minutes writing this, I could have fixed this Camera thing twice by now. :lol:

Patrol code is normally quite straightforward - you have a list of patrol points, you keep track of which one you're aiming at next, you move towards that point each update, and when you're close enough, you switch to moving towards the next point. When you run out of points, you start from the beginning again.

Yeah, you gave me an even better idea. I could pass a vector of points, instead of only two points.

But I kind of don't know how to check whether an enemy has reached a certain point.

First get the magnitude and that will tell you how close you are. If it is within a certain threshold, then change your endPoint to be the next point.

Sounds simple. I will try that now.

Thanks for the feedback, guys.

Fixed.

class AI
{
  // Some code here
  ...

  Camera mainCam; //holds current and previous position and some other stuff
  public int currentPatrolPoint = 0; //Points to the 0th element of the vector

  ...
}

void AI::patrol( vector<glm::vec3> patrolPoints )
{
    //This is the enemy's current position.
    glm::vec3 pos = mainCam.getCurrentPosition(); //too much writing. stupid getters.

    //Find out the direction.
    glm::vec3 direction = patrolPoints[ currentPatrolPoint + 1 ] - patrolPoints[ currentPatrolPoint ];

    //Move the player towards that direction.
    pos += glm::normalize( direction )*0.1f;
    mainCam.setCurrentPosition( pos ); //Set the new position.

    if( glm::length( pos - patrolPoints[ currentPatrolPoint + 1 ] ) < 0.3f ) //If within a certain threshold
    {
        if( ++ currentPatrolPoint == patrolPoints.size() ) //Go to the next point, if no more points, go back to 0.
        {
            currentPatrolPoint = 0;
        }
    }
}

It works perfectly. It doesn't handle rotations but, one thing at a time. If you see something wrong, tell me. I'm open to suggestions. :cool:

EDIT: I'm calculating the direction too frequently. Is it ok if I store it as an instance variable and just calculate the direction once per point, or is it better to keep the code more compact like this?

If you see something wrong, tell me

1. There should be no 'Camera' in the AI.

2. The direction of travel shouldn't be directly dependent on the previous patrol point. What if the AI doesn't start exactly on such a point, or gets moved away from it for some reason? Direction is from "here" to "destination", nothing more.

3. Your code has an off-by-one error, because currentPatrolPoint+1 is not always a valid index.

And no, you're not calculating direction too often, unless you're running this on a calculator.

3. Your code has an off-by-one error, because currentPatrolPoint+1 is not always a valid index.

wtf, you are right, I don't even know how I didn't get a compile error. It was working perfectly fine.

Nevermind, if you have time, skim through that code, it seems ok now. And ignore the camera.

void AI::patrol( vector<glm::vec3> patrolPoints )
{
    //This is the enemy's current position.
    glm::vec3 pos = mainCam.getCurrentPosition(); // just ignore the camera.

    //Find the destination point.
    int destinationPatrolPoint = ( currentPatrolPoint == patrolPoints.size() - 1 ) ? 0 : currentPatrolPoint + 1; //fix no.3 - off-by-one error.

    //Find the direction.
    glm::vec3 direction = patrolPoints[ destinationPatrolPoint ] - pos;     //fix no.2 - direction is from "here" to "destination".

    //Move the player towards that direction.
    pos += glm::normalize( direction )*0.1f;
    mainCam.setCurrentPosition( pos ); //Set the new position.

    if( glm::length( direction ) < 0.1f ) //If within a certain threshold
    {
        if( ++ currentPatrolPoint == patrolPoints.size() ) //Go to the next point, if no more points, go back to 0.
        {
            currentPatrolPoint = 0;
        }
    }
}

Looks okay to me. Some things that stand out are your 0.1fs. I'd make those two separate constants. Also, are you running this at a set cadence? Right now, if you run this every frame, on a faster computer, your character will move faster.

You might also think about travelling in reverse instead of to the first point. If you're patrol path is a line, and not a circle, for example, it would be better to turn around and walk from (length-1) to (length-2). Depends on the game and the path, but you could always code up both solutions and have two different patrol behaviors.

Also, are you running this at a set cadence?

Yes, the rate is the same to all computers. If a computer is faster, it renders more times and it looks smoother, but speed is the same.

You might also think about travelling in reverse instead of to the first point. If you're patrol path is a line, and not a circle, for example, it would be better to turn around and walk from (length-1) to (length-2). Depends on the game and the path, but you could always code up both solutions and have two different patrol behaviors.

Yeah, sounds cool. It's on my features list.

I didn't expect to spend so much time on a simple patrol() function, but.....nevermind.

Thanks to all for the help and for the code reviews. :)