CreateJS / TweenJS

A simple but powerful tweening / animation library for Javascript. Part of the CreateJS suite of libraries.
http://createjs.com/
MIT License
3.56k stars 967 forks source link

Tweening 2+ objects cause page crashing after changing tabs #101

Closed foreverpinetree closed 6 years ago

foreverpinetree commented 6 years ago

I used the lastest version 1.0.2, when tweening 2+ objects then turned to a new tab, went back after a while the page became crashing. This problem would not be found with version 0.6.2. So now I have to use 0.6.2 again, waiting for fixing this problem.

clark-stevenson commented 6 years ago

Hey guys!

I believe I have a related issue too. I am on the latest combined release.

I have inherited a bunch of code which is maybe not the prettiest anyway however, I noticed that games crash when switching tab as well. In particular I have noticed a pattern that any function that async calls itself leads to the browser crashing.

This code is a piece of a function called tremble. I had very similar code on a Flappy Bird clone where the bird would fly up and down in a loop waiting for you to "Click to Start". The rest of the game was OK, but that particular part crashed the browser.

createjs.Tween.get(_oSprite).to({ x: _oSprite.x + iXOffset }, iTime).call(function () {
            if (tTremble < 20) {
                tTremble++;
                that.tremble(iXOffset * -1, iTime)
            } else {
                tTremble = 0;
                that.stopTremble();
            }
        });

Firefox is more resilient to this problem (it takes longer to crash) and it says "A Script it causing this page to run slowly do you want to abort it" but chrome/canary just lock up until you go to dev tools and pause js execution.

If I comment out that self executing tween, I cannot reproduce the problem and it appears resolved.

Stacktrace from Firefox

Error: Script terminated by timeout at: p._appendProps@http://192.168.1.70:2949/lib/createjs.js:29448:3 p.to@http://192.168.1.70:2949/lib/createjs.js:29216:3 CBall/this.tremble@http://192.168.1.70:2949/src/plugin/CBall.js:91:9 CBall/this.tremble/<@http://192.168.1.70:2949/src/plugin/CBall.js:97:17 p._runActionsRange@http://192.168.1.70:2949/lib/createjs.js:29429:5 p._runActions@http://192.168.1.70:2949/lib/createjs.js:28772:13 p.setPosition@http://192.168.1.70:2949/lib/createjs.js:28569:25 p.advance@http://192.168.1.70:2949/lib/createjs.js:28526:3 Tween.tick@http://192.168.1.70:2949/lib/createjs.js:29050:11 Tween.handleEvent@http://192.168.1.70:2949/lib/createjs.js:29068:4 p._dispatchEvent@http://192.168.1.70:2949/lib/createjs.js:814:26 p._dispatchEvent@http://192.168.1.70:2949/lib/createjs.js:822:27 p.dispatchEvent@http://192.168.1.70:2949/lib/createjs.js:739:4 Ticker._tick@http://192.168.1.70:2949/lib/createjs.js:1386:4 Ticker._handleTimeout@http://192.168.1.70:2949/lib/createjs.js:1338:3

I mentioned the Flappy Bird clone before. I had some success here by explicitly using createjs.Tweens.removeTweens(object) although in this other case it doesn't seem to work. Maybe like the OP said, multiple objects are an issue since there is a single bird but many "balls".

Finally, I am using Visiblityjs and am explicitly calling createjs.Ticker.paused = true/false as part of that process. Not sure if it matters but a related detail.

danzen commented 6 years ago

Funny - was just coming to report this. Here is my test code - I know you can use loop for this, but I use it in ZIM wiggle where the values change each time. It is crashing my holiday card, so hopefully there is a merry fix! EDIT - have just dropped back to createjs-2015.11.26.min.js and confirm that there is no problem there.

var circle = new createjs.Shape();
circle.graphics.f("black").dc(100,100,20,20);
stage.addChild(circle);
function again() {
    circle.x = 0;
    createjs.Tween.get(circle).to({x:123}, 1000).call(again);
    console.log("again");
}
again();

var circle2 = new createjs.Shape();
circle2.graphics.f("black").dc(100,200,20,20);
stage.addChild(circle2);
function again2() {
    circle2.x = 0;
    createjs.Tween.get(circle2).to({x:123}, 1000).call(again2);
    console.log("again2");
}
again2();
createjs.Ticker.on("tick", stage);

(sorry - can't seem to get the code insert to work and keep indenting, etc.)

danzen commented 6 years ago

Any time something like this happens to me it is a loose global variable issue... It is affecting ZIM wiggle so I can't currently advise using wiggle with the new CreateJS.

ZIM wiggle would not be needed if TweenJS could handle dynamic values when looping. ZIM handles this with the ZIM VEE value and then in behind with zik() http://zimjs.com/code/docs.html?item=zik Here is a quick example: createjs.Tween.get(rect, {loop:-1}).to({x:{min:10,max:100}}, [500,1000]) Then when TweenJS loops it would pick a random x value in that range and it would take either 500ms or 1000ms each time. That would give us a wiggle like in AfterEffects.

Happy Holidays! Here is a CreateJS/ZIM Holiday Card - using wiggle at the end http://zimjs.com/code/present/ Here is a lightshow I worked on yesterday using wiggle: http://zimjs.com/code/lights/soundhaus All the best.

danzen commented 6 years ago

Hey folks - any news on this? We are having people ask questions about it too and I currently am advising to drop back to earlier CreateJS to avoid the problem. Thanks!

gskinner commented 6 years ago

Has anyone tried adding a maxDelta value to Ticker? If that solves it, I have some ideas on how to resolve this. I'm planning to look at this over the next few days.

gskinner commented 6 years ago

Ok. I ran a test, and can confirm that this only happens if there is two (or more) active tweens. The issue looks like it can be resolved for now by setting maxDelta. I'm looking into why this is happening for a proper fix.

Proof of concept: https://codepen.io/gskinner/pen/rJMvXo?#

gskinner commented 6 years ago

Ok. I've isolated the issue. Its boils down to a complicated race condition: leaving and returning to the tab results in a large delta value. This causes both tweens to finish at the same time, triggering their call, and adding two new tweens at the end of the linked list. These new tweens are then erroneously advanced by the large delta, which again causes them to add two more tweens.

I have a fix for the immediate issue, but have concerns about other more edge cases (ex. one tween causing the immediate next tween in the list to be removed). Far less critical, but something I'd like to consider.

I'll push this fix (or something more wholistic) in the next day.

foreverpinetree commented 6 years ago

Oh, good news, thank you :) Best regards, Huanjian On 02/07/2018 07:59, Grant Skinner wrote: Ok. I've isolated the issue. Its boils down to a complicated race condition: leaving and returning to the tab results in a large delta value. This causes both tweens to finish at the same time, triggering their call, and adding two new tweens at the end of the linked list. These new tweens are then erroneously advanced by the large delta, which again causes them to add two more tweens. I have a fix for the immediate issue, but have concerns about other more edge cases (ex. one tween causing the immediate next tween in the list to be removed). Far less critical, but something I'd like to consider. I'll push this fix (or something more wholistic) in the next day.

—You are receiving this because you authored the thread.Reply to this email directly, view it on GitHub, or mute the thread.

{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/CreateJS/TweenJS","title":"CreateJS/TweenJS","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/CreateJS/TweenJS"}},"updates":{"snippets":[{"icon":"PERSON","message":"@gskinner in #101: Ok. I've isolated the issue. Its boils down to a complicated race condition: leaving and returning to the tab results in a large delta value. This causes both tweens to finish at the same time, triggering their call, and adding two new tweens at the end of the linked list. These new tweens are then erroneously advanced by the large delta, which again causes them to add two more tweens.\r\n\r\nI have a fix for the immediate issue, but have concerns about other more edge cases (ex. one tween causing the immediate next tween in the list to be removed). Far less critical, but something I'd like to consider.\r\n\r\nI'll push this fix (or something more wholistic) in the next day."}],"action":{"name":"View Issue","url":"https://github.com/CreateJS/TweenJS/issues/101#issuecomment-363607904"}}}

gskinner commented 6 years ago

Quick update: I've implemented a robust solution that should handle all edge cases. I'm currently testing, and then will push it. At that point I would really appreciate any testing people can do, since this is a core architectural change, and could mess up a lot of content if it has bugs.

gskinner commented 6 years ago

Ok. I just pushed a fix for this. I tested the immediate cases, but have NOT performed robust testing yet. I would really appreciate any testing you can perform. I also updated the NEXT lib files for convenience.

https://github.com/CreateJS/TweenJS/commit/2b6edc6d826a486a65a188448f1fc9f38a3544b1

gskinner commented 6 years ago

Also committed tests to make sure we don't undo this fix at some point: 7a42b4fb22f99b2f75476b02fbc9bff73fdefa49

gskinner commented 6 years ago

Has anyone tested this? Can you confirm it has solved your issues?

danzen commented 6 years ago

Sorry Grant - my email was out for a bit and missed the updates until now. Will test and let you know.

clark-stevenson commented 6 years ago

I am using the combined build so my test is a bit of a hack with the new next

However I tried for 10 minutes there to reproduce the issue on chrome and firefox and I couldn't. No matter how often I flip back and forward, the game no longer locks up. I used to be able to reproduce the freeze after only a few attempts.

I also tried my Flappy Bird and removed the removeTween fix, now he bobs up and down quite happily.

As far as my original problems go, everything seems fine! Although because of my hacky setup, I would feel better knowing others agree.

Thanks @gskinner and whilst here, for all the other tools you provided since the early 00s. I am a long term lurker 👍

danzen commented 6 years ago

Yes - it works for me too. I tried the simple case from before and my lights here http://zimjs.com/lights/soundhaus/ and both run after switching back and forth between tabs. I inserted your updates into the combined code from the CreateJS dist folder. Thanks Grant - that was a tricky one!

danzen commented 6 years ago

I just put the code in an animation example with all sorts of different types of animations http://zimjs.com/animation/ and it works there too.

gskinner commented 6 years ago

Great! Thanks for the testing everyone. I'll close this, and because this is a crash bug we will likely do a 1.03 build in the near future.