NPC Wont Move (and no collision)

[N36fSL3A]

Well thanks for past help with my RPG once again. Right now I have another problem.

Before I can put enemies in my game I'll have to finish movement on my NPC class. Enemies aren't going to have a separate class, they'll just be a NPC object, which is then made into the appropriate type by be setting a "type" variable.

So, this is why me setting up the NPC class is so vital to combat.

The biggest problem with the NPC right now is that it wont move. I generated a random number and the NPC moves to the correct direction according to it. Here's NPC.cpp (Sorry, I know it's big. I didn't optimize my code yet)

#include "NPC.h"

int times = 0;

int random;

void NPC::Update(Map &map, Player &actor)
{
	SDL_Color Black = {0, 0, 0, 0};


	//MsgSurf = TTF_RenderText_Solid(Res.stdFONT, Name.c_str(), Black);
	MsgSurf = TTF_RenderText_Solid(Res.stdFONT, Int2String::int2string(random).c_str(), Black);

	if(Health > 0)
	{
		// If the player is moving, add one value to the delay
		if(moving == true)
		{
			delay++;
		}

		else
		{
			// If he/she isn't moving, make sure the player
			// doesn't get animated
			delay = 0;
		}

		// If the delay reaches the point where it
		// needs to add a frame, go to the next frame
		// and reset the delay
		if(delay == delaychange)
		{
			frame++;
			delay = 0;
		}

		// If the frame is larger than the frames in the
		// image, reset the frame to 0
		if(frame > 3)
		{
			frame = 0;
		}

		// Make copies of the X and Y values
		// incase we collide with something, so
		// we can revert the position
		int prevx = X;
		int prevy = Y;

		HandleMovement();

		// Add the velocity to the
		// proper coordinates
		X += Xvel;
		Y += Yvel;

		if(Yvel != 0) {moving = true;} else {moving = false;}
		if(Xvel != 0) {moving = true;} else {moving = false;}

		// Update the collision box position
		Box.x = X + 5;
		Box.y = Y + 24;

		if(moving == false) {Xvel = 0; Yvel = 0;}

		if(Collision::ChkCollision(actor.getBox(), Box))
		{
			Collided = true;

			actor.setCollided(true);

			X = prevx;
			Y = prevy;
		}

		Collided = map.TouchSolid(Box);

		// If there is a X collision, revert the X coordinate
		if((Box.x < 0) || (Box.x + Box.w > map.getMapW()) || Collided)
		{
			//std::cout << "Collision Detected.\n";
			X = prevx;
			moving = false;
		}
		
		// If there is a Y collision, revert the Y coordinate
		if((Box.y < 0) || (Box.y + Box.h > map.getMapH()) || Collided)
		{
			//std::cout << "Collision Detected.\n";

			Y = prevy;
			moving = false;
		}
	}
}

void NPC::Render(SDL_Surface* Dest)
{
	if(Health > 0)
	{
		switch(Position)
		{
			case LEFT:
				Draw::DrawGFX(Graphic, LClips[frame], Dest, X - monitor.getCamera().x, Y - monitor.getCamera().y);
				//LeftAnim.RenderObj(Dest);
			break;

			case RIGHT:
				Draw::DrawGFX(Graphic, RClips[frame], Dest, X - monitor.getCamera().x, Y - monitor.getCamera().y);
				//RightAnim.RenderObj(Dest);
			break;

			case UP:
				Draw::DrawGFX(Graphic, UClips[frame], Dest, X - monitor.getCamera().x, Y - monitor.getCamera().y);
			break;

			case DOWN:
				Draw::DrawGFX(Graphic, DClips[frame], Dest, X - monitor.getCamera().x, Y - monitor.getCamera().y);
			break;
		}
	}
}

void NPC::DrawTextBox(SDL_Surface* Dest)
{
	if(Talking == true)
	{
		SDL_FillRect(Dest, &Window[0], SDL_MapRGB(Dest->format, 0, 0, 0));
		SDL_FillRect(Dest, &Window[1], SDL_MapRGB(Dest->format, 6, 61, 121));

		// Draw the message
		//Draw::DrawGFX(MsgSurf, Dest, Window[1].x + 10, Window[1].y + 5);

		Draw::DrawGFX(MsgSurf, Dest, 200, 10);
	}
	Draw::DrawGFX(MsgSurf, Dest, 200, 10);
}

void NPC::HandleMovement()
{
	bool temp = false;
	times++;


	if(moving)
	{
		if(Position == LEFT  && moving == true) {Xvel = -Vel;}
		if(Position == RIGHT && moving == true) {Xvel =  Vel;}
		if(Position == UP    && moving == true) {Yvel = -Vel;}
		if(Position == DOWN  && moving == true) {Yvel =  Vel;}
	}

	if(times == WaitTime)
	{
		times = 0;
		// This should be a while loop
		while(temp == false)
		{
			// Generate a random number :)
			srand(SDL_GetTicks());
			random = rand();

			random = random % 4;

			if(random < 4 || random < 0)
			{
				if(random == LEFT)
				{
					Position = LEFT;
					Xvel = -Vel;

					moving = true;
				}

				if(random == RIGHT)
				{
					Position = RIGHT;
					Xvel = Vel;

					moving = true;
				}

				if(random == UP)
				{
					Position = UP;
					Yvel = -Vel;
					
					moving = true;
				}

				if(random == DOWN)
				{
					Position = DOWN;
					Yvel = Vel;

					moving = true;
				}

				if(random == 4)
				{
					Xvel = 0;
					Yvel = 0;

					temp = false;
				}
				temp = true;
			}
		}
	}
}

[N36fSL3A]

I know this is low to bump with no answers but... BUMP.

[Lachlan Easton]

So you're saying that they move correctly in accordance with the random value, but you're saying that they don't move?

[N36fSL3A]

No I'm saying they SHOULD move due to how I set it up. Obviously there is a logical error.

[cire]

There are a number of things that just do not make sense in your code.

Why are you seeding the random number generator every time you need a random number?

Why is there a variable of type bool named temp in HandleMovement? What is it supposed to represent? Why do you seem to be using it interchangeably with (what I can only assume is) a member of the NPC class named moving? Why is temp set to true immediately after it is set to false? Temp seems to be your way of negating the while loop. A loop that only ever iterates one time is not a loop.

Why do you have two global variables (times and random?) Why are you using a counter for animation instead of basing animation on time?

Why does line 60 make line 59 pointless? Why do you try (unsuccessfully) to set the value of moving according to the x and y velocities, and a few lines later try to set the value of the x and y velocities according to the value of moving?

Why, if there's going to be a collision and you revert to the previous coordinates (thereby averting the collision) do you indicate that there actually is a collision?

[N36fSL3A]

Before I update the program, and before the player moves, every cycle there is a copy of x and y coords. If there is collision, then it goes back to the position it was before the collision.

The line 60 and 59 thing is a check to see if a key is pressed then I'd move accordingly.

I'm not familiar with random numbers so I thought I had to seed it each time to get a different number.

The temp thing with the while loop is actually useless now. It was for experimentation once I catch some sleep I'm gonna change it. It was supposed to represent if the random number doesn't equal 5 then we keep on doing it till it does.

[cire]

Before I update the program, and before the player moves, every cycle there is a copy of x and y coords. If there is collision, then it goes back to the position it was before the collision.

I got that. If there would be a collision, the position reverts so no collision takes place. The question is, why do you still indicate there is a collision, when one didn't take place?

The line 60 and 59 thing is a check to see if a key is pressed then I'd move accordingly.

No it isn't.

I'm not familiar with random numbers so I thought I had to seed it each time to get a different number.

No, you don't. You usually only want to seed a random number generator once per run.

Honestly, I would suggest scrapping this whole implementation and starting over with an algorithm in mind and preferably on paper before you start with the code. This code appears to be formed of guesses and experimentation and that's no sort of formula for productive coding.

Work on movement first. Ignore collisions. When you have movement working, then worry about collisions. Don't try to tackle everything at one time.

[N36fSL3A]

I'm not worried about collision as much as movement right now. That is basically directly copied from my player class and obviously needs to be converted.

What I basically want to do is this

get random number

add one value to limit

if player is already moving
    move it according to position
    set moving to true

if limit equals the limit cap
    if random number matches positions
        change npc position accordingly
        set the player in motion
        player is now moving

[N36fSL3A]

Bump.

[N36fSL3A]

Bump.

[Disch]

You might just have to do some old fashioned debugging, Fredbill.

Temporarily make your NPC choose the same direction every time, then set breakpoints and make sure the code is doing what you expect.

[N36fSL3A]

Everything works, it's just that it changes position, but isn't going forward or anything. I don't know where to start with debugging.

[Disch]

Everything works, it's just that it changes position, but isn't going forward or anything

So.... everything is working except for the parts that aren't working? =P

I don't know where to start with debugging.

Now is the perfect time to learn. What IDE are you using?

[N36fSL3A]

You taught me how to debug but I don't know WHERE in the code to start looking.

(offtopic: How do you check if a string is an integer?)

[Disch]

Start in the code that moves the NPC. Set a breakpoint at the start of the function that moves it... and step through the code so you can see where it's screwing up.

[N36fSL3A]

Pinpointed the problem. Apparently the "moving" variable is not being set to true. This is how I determined the problem.

// If the NPC is moving, then we add values to Xvel accordingly.
	if(moving)
	{
		if(Position == LEFT ) {Xvel = -Vel;}
		if(Position == RIGHT) {Xvel =  Vel;}
		if(Position == UP   ) {Yvel = -Vel;}
		if(Position == DOWN ) {Yvel =  Vel;}

		std::cout << "The npc SHOULD be moving";
	}

And the cout isn't spamming the console like it's supposed to.

[Disch]

dumping to cout is a poor man's debugger. You should really shake that habit.

You could have just set a breakpoint on any of those if(Position statements, then you wouldn't have had to rebuild to test it (and you wouldn't have lingering debug code that you may or may not forget to remove later). In fact... not only do you not have to rebuild... you don't even have to stop/restart the program if it's already running.


From here, you can set a breakpoint at the beginning of the routine where moving should be getting set to true... and step through it (with the debugger's "Step Over" command) to see why it isn't being set properly.

Remember you can always look at the contents of any variable in the Watch window. So as you're stepping through, keep an eye on key variables and make sure they're getting updated in the way you intended.

[N36fSL3A]

Well I made a breakpoint and not only found out moving = false, but also this...

http://prntscr.com/1ekezm

Turns out I didn't initialize. Anyway my mom is raging at me for working on this so early with no sleep, so I'm gonna be offline working on this in my room in the meantime.

[N36fSL3A]

UPDATE - Thanks everyone I can now begin on Inventories/Combat. I hope you can suggest battle systems and stuff here - http://www.cplusplus.com/forum/lounge/106078/