fix: provide a HanziWriter.cancelAnimation() to stop correctly al…

[vaab]

…l animations.

For several reason, animation where hard to stop:

• no general HanziWriter.cancelAnimation() was provided, this is
  fixed, and the cancellation process is a complex one due to the many
  queues and hidden callbacks hanging after promises.
• timeout function where used here and there in animation chains
  and would not check if the animation was canceled, inducing delays
  as they would not quit before completion.
• the looping mechanism did not check for animation stop neither.

The timeout function was expanded to allow any number of predicates
to be checked every 5ms for early cancellation of the timeout.

cancelAnimation() will also return a promise and ensure that the animation are
stopped whenever the promise is resolved.

[chanind]

Ack, cancelAnimation is another regression. I really need to add a lot more test coverage to prevent this from happening. Previously calling hide() or show() during an animation would cause the animation to stop immediately. This behavior is fixed in #46.

The way animations are handled right now is a mess that needs to be refactored. I think you've hit on the problem which is to be able to have some animations that can happen in parallel, while others need to take priority immediately. We also need to make sure that whenever we call a method we force the writer into a correct state no matter what animations are in progress. I think this PR is on the right track, but I want to take some time to rethink the animation model before merging this PR.

[vaab]

I totally understand the need of tests and the time to think about what to do about this PR.

If this makes any sense, and is of any help, in this PR, I had to use a common promise (HanziWrite._withDataPromise) as the common communication queue for all animation to double check that a stop was requested. This has allowed me to reach my goal (removing all lags and rogue animation) in a down to the ground and timely manner while not having to go through all your code (especially svg.js that I barely glanced at).

I'm thus not defending this structure, and of course if all animation are not stacked in various promises and chains, and they do their own checks would definitively be much better. If you need any help, I might have some times to put in that, though I completely understand also that you'll probably want to solve this yourself and it might be difficult to have someone helping you at this stage.

[vaab]

I've tried the latest master (d30aa79) to see if I could avoid using this current PR. This is not yet the case: I don't know where to hook my event to be sure to be after the the ending of all animation.

for instance, I call this._writer._characterRenderer.hideImmediate() and will get the character to hide, but also I'll get the next stroke animated.
An alternate option would be that, by some means, hideImmediate() could stop the animation ?
I still feel that a cancelAnimation() should exist on HanziWriter (how to stop a running animation ?), and that would return a promise to be sure to hook any custom event after the end of the animation.

I've sent an updated code for this PR, I'm afraid I had to remove the part that forced a second setCharacter() in the HanziWriter._withData() as it would generate an infinite recursion. I still don't feel comfortable about this last function wanting to reload the character. But you are the decider here ! Just want to help as I can.

I would be very happy not to need this PR to have clean cut-off of animations, so any other solution will be welcomed !. You may also have some other suggestion on the client code side that I'm ready to try also.

[chanind]

Do hide() and show() not cancel the currently running animation on 0.6.0? If not they should. It may make sense to add a cancelAnimation() method that's just an alias of hide() to make the API clearer?

[vaab]

I use hideImmediate(), could this be the cultprit ? With hideImmediate(), a lone rogue stroke will always be fired after the character is hidden. The full animation of one stroke only will run, then the animation will stop and the stroke will disappear itself. And, yes, I'm using 0.6.0.

If cancelAnimation() ends up cancelling the current animation as fast as possible (I'm thinking about all promises and chains that are animating the char), then this is nice... I'm afraid that calling hide() will suffer from some duration settings, won't it ?

[chanind]

oops sorry I meant to say writer.hideCharacter() and writer.showCharacter(). Yeah - they both are animated with a duration. Should we add an official hideCharacterImmediate() and showCharacterImmediate() to the API? Or maybe we can make it so you can pass an optional duration option to hideCharacter() and showCharacter() which can be 0 to make it immediate?

[vaab]

That's a wonderful idea. I would be for a simplification of the API on this side. all hide/show and hide*/show* should have this duration option and all return a promise. You can then remove all *Immediate(). If you like this idea, and don't have time, I can make you a PR for that.

[chanind]

Closing this for now as most of these issues should have been fixed by now. As of #75, { duration: 0 } can be passed to all show/hide methods to make them complete instantly as well.