roact-animate icon indicating copy to clipboard operation
roact-animate copied to clipboard

Proposed fix for memory leaks

Open headjoe3 opened this issue 6 years ago • 0 comments

In my TS port, I discovered that a memory leak that will occur 100% of the time a new animation is created, because of the way animations are currently handled.

These changes should solve this problem, although I think the system may need some redesigning. Currently, the behavior is changed so that

  1. Signals are now asynchronous (but fire instantly so long as there is no yielding). I did this in order to make a yielding call when an animation starts, because this fix relies on the guarantee that the tween's Completed event fires (whether or not the animation has actually completed, or was just canceled) local finishStatus = _currentTween.Completed:Wait()
  2. AnimationFinished events should now be guaranteed to fire for any created tween once it finishes, regardless of whether it made it to completion
  3. AnimationFinished events have an extra parameter wasCompleted, which is true iff all tweened values have made it to completion. If a value is ever overridden, it will fire with wasCompleted set to false.
  4. Because of this guarantee, all listeners related to playing animations can safely rely on being disconnected the next time AnimationFinished is called.

What this means is that an overridden tween on a value will halt the playing AnimatedValue, which will then halt the playing Animation, which will then halt the playing AnimationSequence or Parallel.

This system needs to be redesigned in the future so that the transition from one interrupting animation to the next is seamless and GC's nicely.

fixes #13

headjoe3 avatar Feb 24 '19 09:02 headjoe3