Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue With New Animation System and Loops (sometimes) #9056

Closed
1 of 7 tasks
titansoftime opened this issue Jun 2, 2016 · 16 comments
Closed
1 of 7 tasks

Issue With New Animation System and Loops (sometimes) #9056

titansoftime opened this issue Jun 2, 2016 · 16 comments

Comments

@titansoftime
Copy link
Contributor

titansoftime commented Jun 2, 2016

Description of the problem

Going between the looped animations (which does not trigger an oncomplete event) work fine, however the attack actions RepeatOnce with the attached event do not behave correctly.

http://jsfiddle.net/titansoftime/tqjyjn0v/

I could very well be doing this wrong but I cannot find an example of utilizing the oncomplete event in the examples or docs, only here in this comment in #6934 from @bhouston:

I've just updated the branch so that Mixer is an EventDispatcher and that events are now sent on when Actions loop or finish. Shouldn't be hard to add more types of events.

Any help would be greatly appreciated =]

UPDATE

This issue affects basic loops as well w/o any type of complete event or anything fancy.

72: http://jsfiddle.net/titansoftime/8v0pasp5/ (works correctly)
79dev: http://jsfiddle.net/titansoftime/n6apnj3z/ (delay on loop)

Three.js version
  • [ x] Dev
  • [ x] r77
  • [ x] ... Back to 73 I'm pretty sure
Browser
  • All of them
  • Chrome
  • Firefox
  • Internet Explorer
OS
  • [] All of them
  • [x ] Windows
  • Linux
  • Android
  • IOS
@titansoftime
Copy link
Contributor Author

titansoftime commented Jun 3, 2016

Did some source code digging so...

http://jsfiddle.net/titansoftime/v2682epf/

Now I am calling reset() on the action before calling the crossfade and it helped. (js line 138)

Now the problem is the delay before the "finished" event is fired (in this case crossfading back to the "Idle" animation).

@titansoftime
Copy link
Contributor Author

OK so perhaps it's not an issue with the event fire timing but with the animation itself?

Granted all animations worked fine in 72, no delays. In 77 however there are.

In this fiddle: http://jsfiddle.net/titansoftime/2sh95etj/

The "Magic Attack" animation flows seemlessly back into Idle, the "Melee Attack" however does not. Both worked without delay in 72.

Did 72's Animation system have some kind of auto correcting aspect to it? I'm pretty lost here, please help.

@titansoftime
Copy link
Contributor Author

titansoftime commented Jun 26, 2016

Here are the 2 fiddles. 72 works fine, 78 is a little finicky (on magic animation). I have several hundred models, most have a similar delay problem in 78 whereas in 72 they were fine.

72: http://jsfiddle.net/titansoftime/a93w5hw0/
79dev: http://jsfiddle.net/titansoftime/2sh95etj/

Edit: I now have an example of this delay in a simple animation loop, no cross-fading, no callbacks, just a loop.

72: http://jsfiddle.net/titansoftime/8v0pasp5/ (works correctly)
79dev: http://jsfiddle.net/titansoftime/n6apnj3z/ (delay on loop)

I have tried to figure out what is going on with no success. Please @tschw help.

@titansoftime titansoftime changed the title Possible Issue With Animations oncomplete Event Issue With New Animation System and Loops (sometimes) Jun 28, 2016
@tschw
Copy link
Contributor

tschw commented Jun 30, 2016

@titansoftime
Sorry for the late response & thanks for reporting. Always amazing how much stuff can stack up in just a few days, while there's apparent complete silence on the wire when checking frequently...

The supposedly delayed fiddles don't produce any output here. I guess that's not what they're supposed to be doing?
Maybe has to do with that other issue you just reported. Bookmarked this thread - will check back later.

BTW, it would be great if you (and anyone else for that matter 😄) could always use the non-minified builds in tests like these.

From a quick look at the code:

Clamping pauses the action at the end instead of stopping it, therefore keeping its influence on the object alive, but from the rest of the code that may not be exactly what you want... I guess you may have had problems without it?

In either case, the finished event is dispatched immediately when the action stops or pauses, but that doesn't mean that scheduling of a subsequent action from within the event handler works correctly (that is, takes effect within the same frame). In fact, I guess it may not, and then there'd be a frame with neither of the two actions having influence with the object in rest pose. The (admittedly ugly) workaround would be to clamp and then stop one frame later, so the gap frame is taken care of...

Basically the API is intended to be smoothly usable with chained calls (most methods return this). Yes, you can do special stuff, but shouldn't have to do so for normal use (e.g. action.enabled = true is only needed when you disable actions at some point). That's the goal - when there' something standing in the way, we'll straighten it out :).

The event part has not have had much interaction with real world code yet, I think. Could you brush up your fiddle to an example once we have it working nicely as it should?

@titansoftime
Copy link
Contributor Author

titansoftime commented Jun 30, 2016

Sorry for the late response & thanks for reporting. Always amazing how much stuff can stack up in just a few days, while there's apparent complete silence on the wire when checking frequently..

No worries, I'm just SUPER happy you are looking into this =]

The supposedly delayed fiddles don't produce any output here. I guess that's not what they're supposed to be doing?

Nope, no output intended, just visual results.

BTW, it would be great if you (and anyone else for that matter 😄) could always use the non-minified builds in tests like these.

Gotcha, done. Though I had to set bugged (79dev) fiddles to 78 since a recent push destroyed the animation system (soon to be reverted #9258)

Clamping pauses the action at the end instead of stopping it, therefore keeping its influence on the object alive, but from the rest of the code that may not be exactly what you want... I guess you may have had problems without it?

I tried it with both clamping enabled and disabled, This did not result in a solution.

In either case, the finished event is dispatched immediately.....

It turns out this issue has nothing to do with the finished event, that seems to be working perfectly. The issue is from what I can tell now is that 'some' animations no longer loop smoothly (some are fine, which I find odd...).

This is the main fiddle I would look at: http://jsfiddle.net/titansoftime/n6apnj3z/ As you can see there is a delay in the "Sail" animation after the last frame that did not exist in 72 (http://jsfiddle.net/titansoftime/8v0pasp5/). The "Idle" animation loops fine in both fiddles.

Basically the API is intended to be smoothly usable with chained calls (most methods return this). Yes, you can do special stuff, but shouldn't have to do so for normal use (e.g. action.enabled = true is only needed when you disable actions at some point). That's the goal - when there' something standing in the way, we'll straighten it out :).

Seems pretty cool so far, minus this issue =]

The event part has not have had much interaction with real world code yet, I think. Could you brush up your fiddle to an example once we have it working nicely as it should?

Of course =]

@tschw
Copy link
Contributor

tschw commented Jul 1, 2016

One frame of delay would have been an easy fix, but this one looks definitely weird and will take some digging. Guessing it somewhere in the content pipeline, but couldn't see anything obvious... Need to come back here with more time in one piece than right now :/.

@titansoftime
Copy link
Contributor Author

Ha, I hear ya. I look forward to your findings =]

@titansoftime
Copy link
Contributor Author

@tschw

Hi again good sir. Have you had any time to see what is going wrong here? let me know if there is anything I can do to help =]

tschw added a commit to tschw/three.js that referenced this issue Aug 28, 2016
@tschw
Copy link
Contributor

tschw commented Aug 28, 2016

@tschw
Copy link
Contributor

tschw commented Aug 28, 2016

Sorry it took me so long. Lots of stuff on my plate these days - miss y'all.

@titansoftime
Copy link
Contributor Author

Hmm =[ I'm still seeing the delay.

http://jsfiddle.net/o0Lzzro5/

@tschw
Copy link
Contributor

tschw commented Aug 28, 2016

The wizard looked OK. With the ship, the last keyframe is at 5.833330154418945, which is after the track has ended at 5.83333, thus it's trimmed. I guess we could make trimming explicit.

@titansoftime
Copy link
Contributor Author

Did 72's Animation system have some kind of auto correcting aspect to it?

Confirmed. The wizard is now correct.

I guess we could make trimming explicit.

Regarding the ship; I figured 72 had some auto correcting aspect to it heh. Hopefully that works =]

@tschw
Copy link
Contributor

tschw commented Aug 28, 2016

Ship sails nicely now. Took some time for rawgit to update and please take care of your browser's cache when testing.

@tschw
Copy link
Contributor

tschw commented Aug 28, 2016

@titansoftime

I figured 72 had some auto correcting aspect to it heh.

Well, I guess it just didn't trim automatically. The comment removed in the latest PR states it was supposed to be temporary; to have all code under test and pick up quirks during implementation.

@titansoftime
Copy link
Contributor Author

Excellent! All looks good now. I will let you know if I find anything else.

Thank you for fixing this =]

spiderworm pushed a commit to spiderworm/three.js that referenced this issue Sep 1, 2016
aardgoose pushed a commit to aardgoose/three.js that referenced this issue Oct 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants