A C++ version of AnimationManger

[reepblue]

Trying to convert the animationmanager script to a C++ class. However, it's not behaving like the lua variant. It doesn't seem to find the length value, and it skips some time before animation. On top of that, restarting the map, the model is still animating.

 

#include "../Core/base.h"

#include "puppeteeranimating.h"

 

class Animation

{

public:

Animation() {};

virtual ~Animation() {};

 

long blendstart;

long blendfinish;

float blendtime;

std::string sequence;

long length;

float speed;

bool mode;

long starttime;

bool endOfSequenceReached = false;

void endHook() {};

int endFrame;

};

 

vector<Animation*> animations;

 

AnimationManager::AnimationManager(Entity* pEntity)

{

if (pEntity == NULL)

{

Debug::Error("AnimationManager: Entity cannot be nil.");

}

 

entity = pEntity;

frameoffset = Math::Random(0, 1000);

 

}

 

AnimationManager::~AnimationManager()

{

vector<Animation*>::iterator it;

for (it = animations.begin(); it != animations.end(); ++it)

{

SAFE_DELETE(*it);

}

 

ClearAnimations();

}

 

void AnimationManager::SetAnimationSequence(std::string pSequence, const float pSpeed, const float pBlendTime, bool pMode, void EndHook(), int pEndFrame)

{

// Check for redundant animation descriptor

if (pMode = false)

{

if (animations.size() > 0)

{

if (animations[animations.size()]->sequence == pSequence)

{

if (animations[animations.size()]->speed == pSpeed)

{

// No change to blend time, so don't alter this?

animations[animations.size()]->blendtime = pBlendTime;

return;

}

}

}

}

 

// Create new animation descriptor and add to animation stack

Animation* animation = new Animation();

animation->blendstart = Time::GetCurrent();

animation->blendfinish = animation->blendstart + pBlendTime;

animation->sequence = pSequence;

animation->length = entity->GetAnimationLength(animation->sequence, true);

animation->speed = pSpeed;

animation->mode = pMode;

animation->starttime = animation->blendstart;

//animation->endHook = EndHook();

animation->endOfSequenceReached = false;

animation->endFrame = pEndFrame;

 

animations.push_back(animation);

}

 

void AnimationManager::ClearAnimations()

{

animations.clear();

}

 

void AnimationManager::Update()

{

int i = 0;

long blend;

long frame;

int n = 0;

bool doanimation = false;

long currenttime = Time::GetCurrent();

int maxanim = -1;

 

//for (i; animation; animations)

//{

vector<Animation*>::iterator iter = animations.begin();

for (iter; iter != animations.end(); iter++)

{

Animation* animation = *iter;

 

// Lock the matrix before the first sequence is applied

if (doanimation == false)

{

doanimation = true;

entity->LockMatrix();

}

 

// Calculate blend value

blend = (currenttime - animation->blendstart) / (animation->blendfinish - animation->blendstart);

blend = Math::Min(1.0, blend);

 

if (animation->mode == false)

{

frame = currenttime * animation->speed + frameoffset;

}

else

{

frame = (currenttime - animation->blendstart) * animation->speed;

long length = entity->GetAnimationLength(animation->sequence, true);

//long length = animation->length;

 

// If a certain frame was specified, call the hook early and remove it

if (animation->endFrame != NULL)

{

if (frame > animation->endFrame)

{

//animation->endHook();

animation->endFrame = NULL;

//animation->endHook = NULL;

 

}

}

 

if (frame / 100 >= length - 1)

{

frame = length - 1;

maxanim = i + 1;

if (animation->endOfSequenceReached == false)

{

DevMsg("AnimationManager: endOfSequenceReached.");

animation->endOfSequenceReached = true;

}

}

} // if (animation->mode)

 

// Apply the animation

if (animation->sequence != S_NULL)

{

entity->SetAnimationFrame(frame, blend, animation->sequence, true);

}

 

// If this animation is blended in 100%, all previous looping animation sequences can be dropped

if (blend >= 1.0)

{

maxanim = Math::Max(maxanim, i);

}

} // forloop

 

// Unlock entity matrix if any animations were applied

if (doanimation == true)

{

entity->UnlockMatrix();

}

 

// Clear blended out animation - moved this out of the loop to prevent jittering

if (maxanim > -1)

{

int index = 1;

vector<Animation*>::iterator it = animations.begin();

for (it; it != animations.end(); it++)

{

Animation* completedanimation = *it;

 

if (n < maxanim)

{

if (completedanimation->mode == false || completedanimation->endOfSequenceReached == true)

{

DevMsg("AnimationManager: Removing completedanimation.");

animations.erase(animations.end()-index);

}

else

{

index = index + 1;

}

}

else

{

break;

}

}

}

}

 

// The test entity:

PuppeteerAnimating::PuppeteerAnimating(Entity* pEntity) : Puppeteer(pEntity)

{

Start();

}

 

PuppeteerAnimating::~PuppeteerAnimating()

{

SAFE_DELETE(animationmanager);

}

 

void PuppeteerAnimating::Start()

{

animationmanager = new AnimationManager(entity);

}

 

void PuppeteerAnimating::Use(Entity* pCaller)

{

DevMsg("use");

//animationmanager->SetAnimationSequence("Run", 0.04);

animationmanager->SetAnimationSequence("Death", 0.04, 300, true);

}

 

 

void PuppeteerAnimating::Draw(Camera* camera)

{

animationmanager->Update();

}

 

Did I convert the code correctly? I got confused with the for loops.

[martyj]

I've found that the GetAnimationLength doesn't return the proper length, so I've just hard coded my lengths inside my own animation class.

[reepblue]

I've found that the GetAnimationLength doesn't return the proper length, so I've just hard coded my lengths inside my own animation class.

 

Interestingly, if I use the debugger and use animation->length, it'll return 46. However. getting GetAnimationLength in the loop doesn't seem to return anything. I tried a hard coded value and it made no difference.

[Crazycarpet]

I have a feeling your code's getting to 'break' in your loop because the 'if' condition isn't met... I don't really have a demo so I can't test anything. So it seems 'Animation* completedanimation = *it;' is the last thing called, but I bet it's doing that, then breaking because 'n>= maxanim'.

 

Edit: Also 'n' is always 0 in your code, isn't it? I did a quick skim through so maybe I missed something... You should be setting 'n' to the index of the current animation you're on in your loop.

n = it - animations.begin();

 

or change to a range based loop when you need the index and are traversing a std::vector (lists aren't contiguous so you can't.):

for (int n = 0; n < animations.size(); ++n) { }

Then you'd have to set 'completedanimation' using animations[n];

 

Because in Lua 'for n,completedanimation in ipairs(self.animations)' makes 'n' the index of the current table member and 'completedanimation' the value itself. (Although I'm pretty sure you know this and just forgot to set 'n' )

 

If it works any chance you could post the working one? I know it's a quick change but I need to do this soon myself and if you've already done it it'd be cool.

 

Edit #2: You forgot to make a few different variables equal the current index in the vector you're traversing. Like 'i' in Update();

 

Also 'maxanim' could be wrong because animations table in Lua would start at index 1 where as in C++ it'd start at index 0.

 

'animations' should be a member or everything will share one queue.

 

Edit 3:

Here's a fixed animation manager, thanks reepblue for the project so I could test and fix:

 

.cpp file: http://pastebin.com/PCfEBzW0

.h file: http://pastebin.com/fHRhPrvp

 

Also Josh have a look at animationmanager.lua. Where you have the loop with 'n' as the key and a separate 'index' variable, I think you mean to just be using 'n' in table.remove() and remove the 'index' variable all together. If two animations finish in the same frame, the wrong animation could be removed from the stack, no?

 

NOTE: endHook calls your callback (can't be a member function) and passes the Leadwerks::Entity* the AnimManager belongs to as the first argument, and a 'const std::string&' which is the sequence name as the second argument. Example usage is in the cpp file.

 

Here's a std::list version of the .cpp file that's unbenchmarked.

http://pastebin.com/AQh4w74M

^ The list version could very well be slower (and most likely is), because although faster on removing elements from the middle (avoids copies) traversal, etc, is all slower. It's almost always slower to use std::list in my experience unless you're removing big objects that aren't at the back of your vector. (Big, slow, expensive copies.) *OUTDATED*

 

NOTE NOTE: The animations seem to be a little jittery, will fix tomorrow.

[reepblue]

Crazycarpet and I have been back and forth until 3AM EST! It's pretty much done minus an error in animation initiation and frametime. I believe that's all that needs to be fixed now.

 

This overall is really great. Hopefully now more games will be done in C++.

[Crazycarpet]

Sleep's overrated anyways.

 

Also I updated the pastebins and changed the example so 'animationmanager' isn't allocated to the heap, but to the stack as a member of AnimationManager instead of as a pointer to the object just so no-one thinks we were doing it the right way in the example by creating the 'animationmanager' using the 'new' keyword. But yeah, now it's proper in the pastebins and it's all good.

 

Edit (bcus 2 ppl asked why it's better to make it a member over a pointer to the object):

 

-Since I made it so 'animationmanager' is a member of the PuppeteerAnimating class it's now both allocated on the stack (faster because it already knows where to insert the memory, instead of finding a place to put it where it'll fit) and it's an automatic variable which means we don't need to destroy it in the destructor, because it'll be destroyed automatically when it goes out of scope. (In this case when the parent, PuppeteerAnimating object is destroyed.)

 

-Because it's no longer a pointer, and now an actual member you have to access it using 'animationmanager.' instead of 'animationmanager->'.

 

-Stack allocation is much faster than heap allocation ('new' keyword, malloc, etc) because it's just moving the stack pointer, it already knows where to put the object in memory. Where as allocating memory on the heap it has to find a big enough block, split up the memory, keep information about where it's located, etc.

 

Not only is it much faster, but it also has to do with the lifespan of the object, this goes back to the fact that it's now an 'automatic variable' because it's a member. Memory that's allocated to the stack is destructed in the reverse order it was allocated in (so it's lifespan is the same as the scope it's declared in, then it's automatically destroyed.)

Memory that's been allocated to the heap (or free-store) has a lifespan you control, which should generally either be shorter or longer than the lifespan of the scope it's allocated in. (If it's the same, odds are you could be allocating to the stack instead and getting those performance gains and slightly lower memory usage! [no pointer]. Although when you NEED dynamic allocation you need it.. just if you don't, don't use it. [like when you don't know the size of an array you have to initialize beforehand.])

 

(^ in a nut shell, so please don't kill me about heap/free-store terminology, etc. It's just a quick explanation for anyone who was wondering why I changed it.)

[Josh]

When you are clearing the completed animations, you will skip animations since you remove one and make the vector smaller:

       for (int n = 0; n < animations.size(); ++n) {
           Animation* completedanimation = animations[n];

           if (n < maxanim)
           {
               if (completedanimation->mode == false || completedanimation->endOfSequenceReached == true)
               {
                   DevMsg("AnimationManager: Removing completedanimation. " + to_string(animations.size()));
                   animations.erase(animations.begin() + n);

                  //---------------------------------------------------------------
                  //---------------------------------------------------------------

                  n = n - 1; // fixed

                  //---------------------------------------------------------------
                  //---------------------------------------------------------------

               }
           }
           else
           {
               //DevMsg("AnimationManager:Break.");
               break;
           }
       }

 

Another way of handling this is to make the Animation class an extension of the Object class. Store an iterator in the object itself and have it self-release in the destructor. You can use some handy commands to prevent the object from actually being deleted during the loop:

System::GCSuspend()

for each animation...
{
       if animation finished
       {
               animation->Release();// decrement ref count, will delete if refcount reaches zero
       }
}

System::GCResume()//destructors of released objects will be called here

 

This is how Leadwerks is able to allow an entity to self-delete in an update loop.

[Crazycarpet]

Good point, overlooked the fact that one we remove the animation, the vector shifts all elements above it down an index at this point in the script. I prefer the first method because otherwise they'll all delete at once after and that'd be a slight performance drop... not to mention we'll save a tiny bit of memory by keeping Animation it's own class, and keeps the vectors copies when shifting smaller.

 

Also figured out jittering thanks to Josh, fixing now... will re-upload.

 

Edit:

*Big thanks to Josh for helping me figure out how to fix jittering, and the last of the issues in the AnimationManager*

 

Anyways, I cleaned up the code, used forward declaration, removed redundant checks, and this version should work out of box:

 

.cpp file: http://pastebin.com/PCfEBzW0

.h file: http://pastebin.com/fHRhPrvp

 

(My projects currently unusable til I generate a few toLua++ bindings and I can't test on it so if for some odd reason the above doesn't work, I fixed the links in my first post to contain a working version but if you use this version you'll have to make a few changes for it to work in your project, just minor things...

 

SAFE_DELETE(*it) would become delete *it. (You should know when it's safe to delete something reepblue!!

Puppeteer class won't exist for you, etc.)

[Rick]

You could also be a little more standard library friendly and use remove_if:

 

items.remove_if([] (item*i) {

bool isActive = (*i)->update();

if (!isActive)

return true;

 

other_code_involving(*i);

return false;

});

[Crazycarpet]

You could also be a little more standard library friendly and use remove_if:

 

items.remove_if([] (item*i) {

bool isActive = (*i)->update();

if (!isActive)

return true;

 

other_code_involving(*i);

return false;

});

 

Yeah that's a really handy feature many people don't know is even there. In the new examples we switched to range based loops or I'd definitely be using that. (We didn't update the code in the original post, sorry.)

[reepblue]

I recently got home and was happy to see that progress was made on this. Just plopped in the code in my test entity and it works exactly like the lua version! Really awesome, thanks Crazycarpet and Josh for helping with this.

 

Now like if you were making a lua project, you can make animated objects without worrying about animations in C++. Really cool!

 

This will be part of the "core" files in LEX2. Just commited the files to the dev git!

[AggrorJorn]

Hi Reepblue,

 

Can this be added to the C++ section in the wiki? http://leadwerks.wikidot.com/wiki:c-snippets

[reepblue]

I don't see why not! Crazycarpet made it so it can be installed in any LE project without any issues.

 

Nice to see that wiki getting some attention again.

[Crazycarpet]

Just make sure you post this one:

 

.cpp file: http://pastebin.com/PCfEBzW0

.h file: http://pastebin.com/fHRhPrvp

 

The other ones dependant on reepblue's template.

[AggrorJorn]

Thanks guys. Added it the C++ snippets. Its good to see some activity on the C++ front.

[Crazycarpet]

Thanks guys. Added it the C++ snippets. Its good to see some activity on the C++ front.

 

Fixed a simple bug that was caused by a Lua work-around not needed in the C++ verison (--n;), can you re-upload if its not linked to the pastebin directly.?

[Roland]

Tested the C++ version. There's a memory leak here

 
if (n < maxanim) {
    if (completedanimation->mode == false || completedanimation->endOfSequenceReached == true) {
          animations.erase(animations.begin() + n);
    }
}

 

should be something like

 

 
if (n < maxanim) {
    if (completedanimation->mode == false || completedanimation->endOfSequenceReached == true) {
       delete animations[n]; // prevent memory leak   
       animations.erase(animations.begin() + n);
    }
}

[Crazycarpet]

Woahh nice catch. Cant believe we missed this.

Thanks. Will update pastebin when im by a computer.

 

Edit: Updated.

[Roland]

To be picky

 

The public method ClearAnimations also have a potential memory leak as it can be called from anywhere and it erases all elements without deleting them.

 

Easy fixed by moving the delete loop from the destructor to ClearAnimations

 

AnimationManager::~AnimationManager() {
         ClearAnimations();
}

void AnimationManager::ClearAnimations()
{
    for (auto it = animations.begin(); it != animations.end(); ++it)
   {
         delete *it;
   }
   animations.clear();
}

[Crazycarpet]

To be picky

 

Again, definitely not picky... necessary lol, glad you noticed thanks again.

Edit: Although this one I did have fixed in my version. 0.o must've forgot to upload.

[Josh]

I implemented this into the engine. Here is my final code.

AnimationManager.cpp

AnimationManager.h