I coding a Zelda clone, but I have a problem with attack animation. When I press the attack button it just animates way too fast. I want to slow it down, but everything I have tried hasn't worked. this is my attack animation code:

void Link::attack()
{
	if(linkInput.CheckStateInput(ALLEGRO_KEY_S) && !linkInput.CheckStateInput(ALLEGRO_KEY_DOWN))
	{
		canAttack = true;

		//Ready to attack?
		if(canAttack == true)
		{
		
			//Dont let link move or jump while attacking
			if(jump == JUMPIT)
			{
				linkSprite.x = linkSprite.oldpx;
				linkSprite.y = linkSprite.oldpy;
			}

			if(linkSprite.curframe != 4 || linkSprite.curframe != 5)
				linkSprite.curframe = 4;

			if(++linkSprite.framecount > linkSprite.framedelay)
			{
				linkSprite.framecount = 0;

				if(++linkSprite.curframe == 5)
				{
					if(++attackanimcount < attackanimdelay)
					{
						linkSprite.curframe = 5;
						canAttack = false;
					}
				}
			}
		}
	}
}

I want to slow it down so that I can see the animation complete itself realistically

			if(linkSprite.curframe != 4 || linkSprite.curframe != 5)
				linkSprite.curframe = 4;

you set the current frame to 4 no matter what you do. this is because linkSprite.curframe can not be both 4 and 5, so then you might only ever get a flash of when the frame is one the 5th frame.

You can implement a time-step to slow things down. Right now, your computer is just blowing through your frames as quickly as it can. A time step will also make it run on the same speed if on a faster or slower computer, or even if the resources on the computer you're currently using change:

 frameCounter= 0;
 switchFrame = 100;
 frameSpeed = 800;

frameCounter += frameSpeed * clock.restart().asSeconds();//get elapsed time and restart the clock, add to framecounter.
    if(frameCounter >= switchFrame)
        {
        frameCounter = 0;
        source.x++;/*these are just the texture coordinates for the texture atlas,
                     change it to however it is you're switching textures with curframe in allegro, 
                     like curframe +=1 or whathaveyou.*/
        if(source.x * 200 >= pTex.getSize().x)//resets the animation to first frame
            source.x = 0;
        } 

Basically, you're tying the animation to a clock (clock.restart() gets the elapsed time in SFML while resetting the clock, change to whatever you use currently for time in allegro), and only going to the next frame at a fixed rate. Each time your computer goes through the code, it adds a little to your framecounter (since it's multiplied by the time passed since the last time it ran through the code, the faster it goes through the code, the less it adds, if it's running through slower, it adds more), if it goes over a certain amount, it moves your animation to the next frame and resets the framecounter. When it gets to the end of your frames, it then resets the animation. This way, the animation always runs at the same speed, regardless of how quickly your computer is running, and it also lets you set the rate at which the animation moves. Just alter the switchframe/framespeed values.

This example may/may not be all that helpful, but a quick search for time-step animation should turn up plenty of tutorials. This example is for SFML, but it shouldn't be too hard to implement in allegro. If I'm explaining this poorly, let me know and I'll try to be a little clearer.

So, from your code I am assuming your attack animation is composed of only two frames, frames #4 and #5.
Is that correct?

One thing you can try is resetting your framecount when you change the animation:

if(linkSprite.curframe != 4 || linkSprite.curframe != 5) //Should this be &&?
{
	linkSprite.framecount = 0;
	linkSprite.curframe = 4;
}

One other possibility would be to increase the delay necessary in order to change the frame:

const double animation_delay_modifier = 2;
if( ++linkSprite.framecount > linkSprite.framedelay * animation_delay_modifier )
{
	linkSprite.framecount = 0;
	[...]

Maybe I misunderstood your code...

if(++linkSprite.curframe == 5)
{
	if(++attackanimcount < attackanimdelay)
	{
		linkSprite.curframe = 5;
		canAttack = false;
	}
}

I didn't get this part. First, you check if the pre-incremented variable equals to 5, but you then make it 5, even though it was already. There may be some confusion here.

dejaime, on 08 Jul 2014 - 11:09 PM, said:

So, from your code I am assuming your attack animation is composed of only two frames, frames #4 and #5.
Is that correct?

One thing you can try is resetting your framecount when you change the animation:

One other possibility would be to increase the delay necessary in order to change the frame:

const double animation_delay_modifier = 0.5;
if( ++linkSprite.framecount > linkSprite.framedelay * animation_delay_modifier )
{
	linkSprite.framecount = 0;
	[...]

Maybe I misunderstood your code...

if(++linkSprite.curframe == 5)
{
	if(++attackanimcount < attackanimdelay)
	{
		linkSprite.curframe = 5;
		canAttack = false;
	}
}

I didn't get this part. First, you check if the pre-incremented variable equals to 5, but you then make it 5, even though it was already. There may be some confusion here.

Okay lemme explain this part

if(++linkSprite.curframe == 5) { if(++attackanimcount < attackanimdelay) { linkSprite.curframe = 5; canAttack = false; } }

Basically when the attack animation reaches the final frame, I want it to stay on that frame for the just a lil bit before it resets back to frame 4. that's what attackanimcount and attackanimdelay do. If attackanimcount is less than the delay, keep on staying on frame 5 until it surpasses the delay amount. But looks like it isn't working. Ill try some of the solutions that you and the others mentioned.

You can implement a time-step to slow things down. Right now, your computer is just blowing through your frames as quickly as it can. A time step will also make it run on the same speed if on a faster or slower computer, or even if the resources on the computer you're currently using change:

while i agree with everything you've said, i don't believe this is his problem. if he has vsync on then that will basically act as his timer, and without seeing the rest of his code, it's impossible to say if he does or does not use a timer(likely he does not though). so i think he should follow your post, but in this instance i don't believe that is his issue.

edit: however, upon re-reading your code, you should not be setting frameCounter back to 0, instead subtract switchFrame from that counter. this is because by setting it to 0, you lose any amount of time that has accumulated past switchFrame. for example, let's say on the first frame you get 15ms, just shy of 16ms to run at 60fps, the os switchs away, then switchs back, this took 18ms to do, so now frameCounter is at 33ms, this would be two complete frames, with an extra ms into the third but you only process one frame, then reset frameCounter to 0, which loses all that accumulated time.

Okay lemme explain this part

if(++linkSprite.curframe == 5) { if(++attackanimcount < attackanimdelay) { linkSprite.curframe = 5; canAttack = false; } }

Basically when the attack animation reaches the final frame, I want it to stay on that frame for the just a lil bit before it resets back to frame 4. that's what attackanimcount and attackanimdelay do. If attackanimcount is less than the delay, keep on staying on frame 5 until it surpasses the delay amount. But looks like it isn't working. Ill try some of the solutions that you and the others mentioned.

let me reiterate, that part of your code doesn't do anything, when you explicitly set the frame back to 4 on each frame:

if(linkSprite.curframe != 4 || linkSprite.curframe != 5)
linkSprite.curframe = 4;

this is basically saying: "hey, am i not 4? then set me to 4. hey, am i also not 5? then set me to 4." this logic will run on every single frame because a variable can not be 2 values at once. because this line is before that other logic, it's possible for you to achieve a single-frame on frame 5, but if you are running at 60 fps, you'll see this on screen for a mere 16ms of time.

also, a couple other things you should note:
-setting canAttack = true at the start of your function negates setting it to false at the end, and makes the if(canAttack==true) logic pointless.
-you will only animate while holding down the down or S key, so i'd recommend moving your animation code outside of your input logic code.

i'm not going to fix your code completely, but as it is, a simple change like so should get you your desired result:

void Link::attack()
{
    if(linkInput.CheckStateInput(ALLEGRO_KEY_S) && !linkInput.CheckStateInput(ALLEGRO_KEY_DOWN))
    {
        canAttack = true;
        //Ready to attack?
        if(canAttack == true)
        {
            //Dont let link move or jump while attacking
            if(jump == JUMPIT)
            {
                linkSprite.x = linkSprite.oldpx;
                linkSprite.y = linkSprite.oldpy;
            }
            if(++linkSprite.framecount > linkSprite.framedelay)
            {
                linkSprite.framecount = 0;
                linkSprite.curframe++;
                if(linkSprite.curframe==6) linkSprite.curframe = 4; //we go back to 4 once we've passed the 5th frame.
            }
        }
    }
}

edit: however, upon re-reading your code, you should not be setting frameCounter back to 0, instead subtract switchFrame from that counter. this is because by setting it to 0, you lose any amount of time that has accumulated past switchFrame. for example, let's say on the first frame you get 15ms, just shy of 16ms to run at 60fps, the os switchs away, then switchs back, this took 18ms to do, so now frameCounter is at 33ms, this would be two complete frames, with an extra ms into the third but you only process one frame, then reset frameCounter to 0, which loses all that accumulated time.

Could you clarify this (I'm not being argumentative, I'm newish myself, and just want to make sure I'm not doing things wrong myself here)? I was operating under the impression that this was the point of doing it this way. For my physics and main function time-step, I generally do do things the way you suggest, by subtracting time passed, but for animation, I was doing things this way because I wanted several frames to pass between updating the animation (for simple animations it's generally way to quick to update on each pass). I understand that any time accumulated over the point where it switches is lost and would be better to add that back into the framecounter, but I find in practice, since it happens at a pretty steady rate, and no physics are involved, it isn't noticeable (and also didn't want to make this example any more complicated). I've been resetting to zero so that several more passes occur through the code occur before it switches frames again. So, the loss of accumulated time would be intentional (excepting the time that goes over the framecounter "if >=" point).

And, I don't mean to derail the thread, if you're still correct, feel free to just pm me so we don't change the OP's topic.

You really need to have each animation frame stay on the screen for a certain time duration (not number of frames...), and move to the next frame after that time has expired. I'd suggest storing your animation frames, and the times they should stay on, in an external data file (json or XML), and using that file for all your animations.

This will allow you to work with fast framerate machines or slow framerate machines.

edit: however, upon re-reading your code, you should not be setting frameCounter back to 0, instead subtract switchFrame from that counter. this is because by setting it to 0, you lose any amount of time that has accumulated past switchFrame. for example, let's say on the first frame you get 15ms, just shy of 16ms to run at 60fps, the os switchs away, then switchs back, this took 18ms to do, so now frameCounter is at 33ms, this would be two complete frames, with an extra ms into the third but you only process one frame, then reset frameCounter to 0, which loses all that accumulated time.

Could you clarify this (I'm not being argumentative, I'm newish myself, and just want to make sure I'm not doing things wrong myself here)? I was operating under the impression that this was the point of doing it this way. For my physics and main function time-step, I generally do do things the way you suggest, by subtracting time passed, but for animation, I was doing things this way because I wanted several frames to pass between updating the animation (for simple animations it's generally way to quick to update on each pass). I understand that any time accumulated over the point where it switches is lost and would be better to add that back into the framecounter, but I find in practice, since it happens at a pretty steady rate, and no physics are involved, it isn't noticeable (and also didn't want to make this example any more complicated). I've been resetting to zero so that several more passes occur through the code occur before it switches frames again. So, the loss of accumulated time would be intentional (excepting the time that goes over the framecounter "if >=" point).

And, I don't mean to derail the thread, if you're still correct, feel free to just pm me so we don't change the OP's topic.

this might differ by each person's expectations of what a game should be. for me, personally, i try to make all my games deterministic in nature, this includes animation. if you are working at a fixed time-step, then for a game you can start thinking of animations in terms of gameplay ticks, where 1 tick = 1/Frequency of time-step. this can greatly simplify the entire animation system when you no longer think in terms of seconds, but in terms of ticks(for example, let's say i have an animation that runs 1.5 seconds, on completion i should do a task, this means that for each frame i check to see if the animation has finished. if i'm running at 1/5th a second per frame, this means that depending on how much accumulation time, that task could occur on the 7th or 8th frame since the animation was started, however if i say it will finish in 8 frames(so 1.6 seconds), then i can always know that it will fire the event 8 frames after the animation). this also has the added advantage in that you can change the frequency to be faster/slower and all animations will adequately run at the different simulation speed with the overall game.

now some of you might be saying: "well, what happens if the pc can't keep up with your targeted framerate?". that's a valid point, however if you are running gameplay and rendering logic in different threads(and hopefully your target platform has multiple core's), i've generally found that for nearly all game's i've ever built, the actual game logic itself uses a very tiny fraction of cpu time, compared to the rendering logic.

for example, when developing a PSM game, i found that doing transformations of the asteroid/player ship was faster to do on the cpu, then it was on the gpu for low-end devices such as the tablet P, or xperia play. this is because i was also doing a full screen 2-pass gaussian blur, which ate up alot of my rendering time, by doing the vertex transformations on cpu(this was ~500 asteroids, or ~15000 vertices), i freed up enough render time to do that 2-pass blur and still maintain a solid 60 fps.

Well I got it to partially work as the animation is a lot slower, but it still seems like it flies off frame 5 bit too fast:

void Link::attack()
{
	if(linkInput.CheckStateInput(ALLEGRO_KEY_S) && !linkInput.CheckStateInput(ALLEGRO_KEY_DOWN))
	{
		canAttack = true;

		//Dont let link move or jump while attacking
		if(jump == JUMPIT)
		{
			linkSprite.x = linkSprite.oldpx;
			linkSprite.y = linkSprite.oldpy;
		}

		if(++linkSprite.framecount > linkSprite.framedelay)
		{
			linkSprite.framecount = 0;

			if(++linkSprite.curframe > 4)
			{
				if(++attackanimcount < attackanimdelay)
					linkSprite.curframe = 4;
				else
				{
					linkSprite.curframe = 5;
					attackanimcount = 0;
				}
			}
				
			else if(++linkSprite.curframe == 5)
			{
				if(++attackanimcount < attackanimdelay)
					linkSprite.curframe = 5;
				else
				{
					linkSprite.curframe = 0;
					attackanimcount = 0;
				}
			}
			else if(++linkSprite.curframe == 6)
				linkSprite.curframe = 4;
		}	
	}
	else
	{
		canAttack = false;
	}
}

But it seems like its interfering with my moving code:

//the basic controls for link
void Link::moveLink()
{
	//Record previous positions
	linkSprite.oldpx = linkSprite.x;
	linkSprite.oldpy = linkSprite.y;

	//Move right and animate
	if(linkInput.CheckStateInput(ALLEGRO_KEY_RIGHT))
	{
		linkSprite.direction = right;
		linkSprite.x += linkSprite.velx;

		if(++linkSprite.framecount > linkSprite.framedelay)
		{
			linkSprite.framecount = 0;

			if(++linkSprite.curframe > 3)
			{
				linkSprite.curframe = 0;
			}
		}
	}

	//Move left and animate
	else if(linkInput.CheckStateInput(ALLEGRO_KEY_LEFT))
	{
		linkSprite.direction = left;
		linkSprite.x -= linkSprite.velx;

		if(++linkSprite.framecount > linkSprite.framedelay)
		{
			linkSprite.framecount = 0;

			if(++linkSprite.curframe > 3)
			{
				linkSprite.curframe = 0;
			}
		}
	}

	//Duck
	else if(linkInput.CheckStateInput(ALLEGRO_KEY_DOWN))
	{
		linkSprite.curframe = 6;

		if(linkInput.CheckStateInput(ALLEGRO_KEY_DOWN))
		{
			if(linkInput.CheckStateInput(ALLEGRO_KEY_S))
				linkSprite.curframe = 7;
		}
	}

	//When doing nothing, just stand
	else 
	{
		if(!canAttack)
			linkSprite.curframe = 0;
	}
}