Feat/alternative animation queuing mechanism

[riobah]

This PR is adding an alternative animation queuing system while keeping the existing code for backward compatibility.

Benefits compared to the existing mechanism:

• Ability to queue multiple animations
• Ability to wait till the end of the existing animation before starting the queued animation
• Ability to continue an animation infinitely running (like walking) and inserting one single animation in between its executions
• Simplified endOfAnimation: method implementation.

Old code is kept as it is. If we agree that the new code is supporting the existing functions of the code, we can clean the old code later providing the same interface for backward compatibility.

[mredig]

Just wanted to let you know I'm not ignoring these pull requests. I'll try to remember to review them after work tonight!

…

Sent from my iPhone

On Sep 6, 2017, at 4:43 AM, riobah ***@***.***> wrote: This PR is adding an alternative animation queuing system while keeping the existing code for backward compatibility. Benefits compared to the existing mechanism: Ability to queue multiple animations Ability to wait till the end of the existing animation before starting the queued animation Ability to continue an animation infinitely running (like walking) and inserting one single animation in between its executions Simplified endOfAnimation: method implementation. Old code is kept as it is. If we agree that the new code is supporting the existing functions of the code, we can clean the old code later providing the same interface for backward compatibility. You can view, comment on, or merge this pull request online at: #16 Commit Summary Implementing an alternative animation enqueing mechanism allowing multiple animations to be queued and the actully running one not being cancelled with an enqueue. Adding example to the test SKScene for the new enqueing methods File Changes M SpineImporter/SGG_Spine.h (7) M SpineImporter/SGG_Spine.m (141) M SpineTesting/SGG_MyScene.m (77) Patch Links: https://github.com/mredig/SGG_SKSpineImport/pull/16.patch https://github.com/mredig/SGG_SKSpineImport/pull/16.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[riobah]

No worries Michael. Feel free to check them anytime you are available. Appreciate your message.

[mredig]

I will have to review this PR in more depth later. Right now, I'm failing to see the distinguishment from the original.

Feel free to provide an example comparison to help speed things up, otherwise I'll get this this when I can.

Thanks!

[riobah]

Thank you Michael. I'm quite new to the code base, so I hope I'm not reaching to wrong conclusions because of my mis-usage of the library.

Below was my understanding:

1. While I have a queued animation (say "walk") and I call runAnimation method, it stops the already running animation instantly.

                [boy runAnimation:@"walk" andCount:-1 withIntroPeriodOf:0 andUseQueue:YES];
                dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(1.4 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{
                    [boy runAnimation:@"jump" andCount:1];         // jump once, and then continue walking
                });

It does not let the current cycle to reach to the end, because of the following code in runAnimation method.

 if  (self.isRunningAnimation) {
        [self stopAnimation]; //clear any current animations
 }

PR allows the new animation to be added to the queue and run after the existing animation's cycle ends.

2. I haven't dig to find the root cause, but with the following code boy jumps twice:

                [boy runAnimation:@"walk" andCount:-1 withIntroPeriodOf:0 andUseQueue:YES];
                dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(1.4 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{
                    [boy runAnimation:@"jump" andCount:1];         // jump once, and then continue walking
                });

3. With the queue mechanism, I was expecting to be able to queue multiple animations with seperate runAnimation method call. I see existing code is not working like this, and PR is adding this functionality. Assume with each tap of the user we add one jump animation to the queue, even if the jump animation is still in progress.

As an example, below code does not result with two jumps:

                [boy runAnimation:@"walk" andCount:-1 withIntroPeriodOf:0 andUseQueue:YES];

                dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(1.4 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{
                    [boy runAnimation:@"jump" andCount:1 withIntroPeriodOf:0 andUseQueue:YES];
                    [boy runAnimation:@"walk" andCount:1 withIntroPeriodOf:0 andUseQueue:YES]; 
                    [boy runAnimation:@"jump" andCount:1 withIntroPeriodOf:0 andUseQueue:YES];
                });

Again, I might be wrong with my conclusion as I'm pretty new to the library. Any comment is quite welcomed.

Thank you again for considering the PR.

[riobah]

Could you have the chance to check the PR Michael?

[mredig]

I am incredibly sorry this has taken so long!

If I'm understanding what you are attempting to do correctly, I think the functionality already exists... mostly.

When using a sequence, there is a property on the SGG_Spine object called currentAnimationSequence - it is a read only property. There may be cases where you wouldn't want to modify it directly from an object instance, so I wouldn't suggest removing that. However, you might want to instead add a function that inserts the name of the animation you want to play at index 1, obviously checking that that is safe to do so - that way when the current animation finshes, your queued animation plays next.

Will this solve your use case? If not, can you elaborate the distinction of your code addition?

Additionally, given the nature of what you are trying to do, I would suggest modifying the documentation to clarify that the property queuedAnimation would be more aptly named defaultAnimation, but we cannot rename the property as it would break backward compatibility.