Fixes to skel keyframe handling that would usually skip the first keyframe

This didn't seem to cause problems since apparently nothing ever depended
on the first keyframe being processed properly (ie. animationKey(),
bone commands, sounds, particle spawning, etc).
Curiously this was never an issue in the editor since the first keyframe
is hit immediately (since it keeps the time clamped to 0 when not animating),
and the first keyframe triggered ONLY when t==0 exactly.

With this change no keyframes are skipped, and during an animation loop,
the last and the first keyframe are hit together.
entity_setAnimationTime() got an extra param whether to skip keyframes
when setting the time manually (if not skipping, it'll catch up).

This commit also fixes a bug(?) that animationKey() was called with key -1
when changing animations, which was a side-effect of how the old code worked.
(I hope nothing depended on this behavior, i'm not sure but i think it's safe to fix)

Aquaria/ScriptInterface.cpp

@@ -4804,7 +4804,9 @@ luaFunc(entity_setAnimationTime)
 		AnimationLayer *a = skel->getAnimationLayer(layer);
 		if(a)
 		{
-			a->timer = lua_tonumber(L, 2);
+			float t = lua_tonumber(L, 2);
+			bool runkeyframes = getBool(L, 3);
+			a->setTimer(t, runkeyframes);
 		}
 	}
 	luaReturnNil();

BBGE/SkeletalSprite.cpp

@@ -31,6 +31,13 @@ Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.
 #include <tinyxml2.h>
 using namespace tinyxml2;
 
+
+enum
+{
+	RESET_KEYFRAME_IDX = -2,
+	INITIAL_KEYFRAME_IDX = -1,
+};
+
 std::string SkeletalSprite::animationPath				= "data/animations/";
 std::string SkeletalSprite::skinPath					= "skins/";
 
@@ -433,7 +440,7 @@ void BoneCommand::run()
 
 AnimationLayer::AnimationLayer()
 {
-	lastNewKey = 0;
+	lastKeyframeIndex = INITIAL_KEYFRAME_IDX;
 	fallThru= 0;
 
 	timer = 0;
@@ -482,6 +489,12 @@ void AnimationLayer::animate(const std::string &a, int loop)
 
 void AnimationLayer::playAnimation(int idx, int loop)
 {
+	// When just looping, keep the last keyframe as-is.
+	// This way, wrapping the timer around to 0 will correctly proceed to process the last (yet unreached) keyframe
+	// before immediately wrapping around and hitting the first keyframe
+	if(idx != currentAnimation || !loop)
+		lastKeyframeIndex = INITIAL_KEYFRAME_IDX;
+
 	if (!(&s->animLayers[0] == this))
 	{
 		fallThru = 1;
@@ -489,6 +502,7 @@ void AnimationLayer::playAnimation(int idx, int loop)
 	}
 	timeMultiplier = 1;
 
+
 	currentAnimation = idx;
 	timer = 0;
 	animating = true;
@@ -496,7 +510,6 @@ void AnimationLayer::playAnimation(int idx, int loop)
 	this->loop = loop;
 
 	animationLength = getCurrentAnimation()->getAnimationLength();
-
 }
 
 void AnimationLayer::enqueueAnimation(const std::string& anim, int loop)
@@ -586,6 +599,7 @@ void AnimationLayer::createTransitionAnimation(Animation& to, float time)
 	blendAnimation.keyframes.push_back(k2);
 
 	blendAnimation.name = to.name;
+	blendAnimation.resetOnEnd = false;
 }
 
 
@@ -594,6 +608,7 @@ void AnimationLayer::stopAnimation()
 	if(s->loaded && getCurrentAnimation()->resetOnEnd)
 		resetPass();
 	animating = false;
+	lastKeyframeIndex = INITIAL_KEYFRAME_IDX;
 	if (!enqueuedAnimation.empty())
 	{
 		animate(enqueuedAnimation, enqueuedAnimationLoop);
@@ -617,15 +632,14 @@ Animation::Animation()
 {
 }
 
-size_t Animation::getNumKeyframes()
+size_t Animation::getNumKeyframes() const
 {
 	return keyframes.size();
 }
 
 SkeletalKeyframe *Animation::getKeyframe(size_t key)
 {
-	if (key >= keyframes.size()) return 0;
-	return &keyframes[key];
+	return key < keyframes.size() ? &keyframes[key] : NULL;
 }
 
 void Animation::reverse()
@@ -650,7 +664,7 @@ float Animation::getAnimationLength()
 SkeletalKeyframe *Animation::getLastKeyframe()
 {
 	if (!keyframes.empty())
-		return &keyframes[keyframes.size()-1];
+		return &keyframes.back();
 	return 0;
 }
 
@@ -671,6 +685,16 @@ void Animation::reorderKeyframes()
 	std::sort(keyframes.begin(), keyframes.end(), keyframeCmp);
 }
 
+size_t Animation::getKeyframeIndexBefore(float t) const
+{
+	const size_t n = getNumKeyframes();
+	for(size_t i = 1; i < n; ++i)
+		if(keyframes[i].t > t)
+			return i - 1;
+	return 0;
+}
+
+
 void Animation::cloneKey(size_t key, float toffset)
 {
 	std::vector<SkeletalKeyframe> copy = this->keyframes;
@@ -730,43 +754,6 @@ BoneKeyframe *SkeletalKeyframe::getBoneKeyframe(size_t idx)
 	return 0;
 }
 
-SkeletalKeyframe *Animation::getPrevKeyframe(float t)
-{
-	size_t kf = -1;
-	for (size_t i = keyframes.size(); i-- > 0; )
-	{
-		if (t >= keyframes[i].t)
-		{
-			kf = i;
-			break;
-		}
-	}
-	if (kf == -1)
-		return 0;
-	if (kf >= keyframes.size())
-		kf = keyframes.size()-1;
-	return &keyframes[kf];
-}
-
-SkeletalKeyframe *Animation::getNextKeyframe(float t)
-{
-	size_t kf = -1;
-	for (size_t i = 0; i < keyframes.size(); i++)
-	{
-		if (t <= keyframes[i].t)
-		{
-			kf = i;
-			break;
-		}
-	}
-
-	if (kf == -1)
-		return 0;
-	if (kf >= keyframes.size())
-		kf = keyframes.size()-1;
-	return &keyframes[kf];
-}
-
 SkeletalSprite::SkeletalSprite() : RenderObject()
 {
 	frozen = false;
@@ -1938,6 +1925,15 @@ void AnimationLayer::resetPass()
 	}
 }
 
+void AnimationLayer::setTimer(float t, bool runKeyframes)
+{
+	timer = t;
+	if(!runKeyframes)
+	{
+		lastKeyframeIndex = RESET_KEYFRAME_IDX;
+	}
+}
+
 bool AnimationLayer::contains(const Bone *b) const
 {
 	const int idx = b->boneIdx;
@@ -1958,44 +1954,81 @@ bool AnimationLayer::contains(const Bone *b) const
 	return true;
 }
 
+
+
+void AnimationLayer::keyframeReached(SkeletalKeyframe * k, size_t idx)
+{
+	if (!k->sound.empty())
+	{
+		core->sound->playSfx(k->sound);
+	}
+	if (!k->commands.empty())
+	{
+		for (size_t i = 0; i < k->commands.size(); i++)
+		{
+			k->commands[i].run();
+		}
+	}
+	if (s->animKeyNotify)
+	{
+		s->animKeyNotify->onAnimationKeyPassed(idx);
+	}
+}
+
 void AnimationLayer::updateBones()
 {
 	if (!animating && !(&s->animLayers[0] == this) && fallThru == 0) return;
 
-	SkeletalKeyframe *key1 = getCurrentAnimation()->getPrevKeyframe(timer);
-	SkeletalKeyframe *key2 = getCurrentAnimation()->getNextKeyframe(timer);
-	if (!key1 || !key2) return;
-	float t1 = key1->t;
-	float t2 = key2->t;
+	Animation *a = getCurrentAnimation();
+	SkeletalKeyframe *key1; // .t <= timer
+	SkeletalKeyframe *key2; // .t >= timer
 
+	{
+		int last = lastKeyframeIndex;
+		if(last == RESET_KEYFRAME_IDX) // Requested to skip keyframes?
+			last = a->getKeyframeIndexBefore(timer);
 
+		const unsigned maxidx = (unsigned)a->keyframes.size() - 1;
+		unsigned curidx = std::max(0, last); // signed min() is intentional, this covers the special case of -1 to start at 0 regardless
 
-	float diff = t2-t1;
-	float dt;
-	if (diff != 0)
-		dt = (timer - t1)/(t2-t1);
-	else
-		dt = 0;
-
-	if (lastNewKey != key2)
-	{
-		if (!key2->sound.empty())
+		// Assume a big timer step that would potentially step over multiple keyframes (which we don't want!)
+		// So we step keyframes until we catch up to the timer, so that (key1->t <= timer <= key2->t).
+		for(unsigned i = 0; i <= maxidx; ++i)
 		{
-			core->sound->playSfx(key2->sound);
-		}
-		if (!key2->commands.empty())
-		{
-			for (size_t i = 0; i < key2->commands.size(); i++)
+			key1 = &a->keyframes[curidx];
+
+			if((int)curidx != last)
 			{
-				key2->commands[i].run();
+				keyframeReached(key1, curidx);
+				last = (int)curidx;
 			}
+
+			unsigned nextidx = curidx + 1;
+			if(nextidx > maxidx)
+				nextidx = 0;
+			key2 = &a->keyframes[nextidx];
+
+			if((key1->t <= timer && timer <= key2->t) || !maxidx)
+				break;
+
+			curidx = nextidx;
 		}
-		if (s->animKeyNotify)
-		{
-			s->animKeyNotify->onAnimationKeyPassed(getCurrentAnimation()->getSkeletalKeyframeIndex(lastNewKey));
-		}
+
+		lastKeyframeIndex = last;
 	}
-	lastNewKey = key2;
+
+	const float t1 = key1->t;
+	const float t2 = key2->t;
+	assert(t1 <= t2);
+
+
+
+	const float diff = t2-t1;
+	float dt;
+	if (diff != 0)
+		dt = (timer - t1)/(t2-t1);
+	else
+		dt = 0;
 
 	for (size_t i = 0; i < s->bones.size(); i++)
 	{

BBGE/SkeletalSprite.h

@@ -176,14 +176,14 @@ public:
 	SkeletalKeyframe *getKeyframe(size_t key);
 	SkeletalKeyframe *getLastKeyframe();
 	SkeletalKeyframe *getFirstKeyframe();
-	SkeletalKeyframe *getPrevKeyframe(float t);
-	SkeletalKeyframe *getNextKeyframe(float t);
+	size_t getKeyframeIndexBefore(float t) const; // last anim that's <= t
+
 	void cloneKey(size_t key, float toffset);
 	void deleteKey(size_t key);
 	void reorderKeyframes();
 	float getAnimationLength();
 	size_t getSkeletalKeyframeIndex(SkeletalKeyframe *skey);
-	size_t getNumKeyframes();
+	size_t getNumKeyframes() const;
 	void reverse();
 	bool resetOnEnd;
 
@@ -206,6 +206,7 @@ public:
 	void animate(const std::string &animation, int loop);
 	void update(float dt);
 	void updateBones();
+	void keyframeReached(SkeletalKeyframe *k, size_t idx);
 	void stopAnimation();
 	float getAnimationLength();
 	void createTransitionAnimation(Animation& to, float time);
@@ -217,6 +218,8 @@ public:
 	bool isAnimating() const;
 	bool contains(const Bone *b) const;
 	void resetPass();
+	void setTimer(float t, bool runKeyframes);
+
 
 	//----
 	float fallThru;
@@ -226,7 +229,7 @@ public:
 	std::vector<int> includeBones;
 	SkeletalSprite *s;
 
-	SkeletalKeyframe *lastNewKey;
+	int lastKeyframeIndex;
 
 	float timer;
 	int loop;