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

Added signals to AnimationTree for when an animation starts, finishes, or changes. #30390

Closed
wants to merge 1 commit into from

Conversation

Rodeo-McCabe
Copy link

This is one possible solution to #28311. The AnimationTree defines its own signals for animation_started, animation_finished, and animation_changed. These are connected to the AnimationPlayer's signals of the same names, and emitted when the AnimationPlayer emits the signal.

Basically the AnimationTree is forwarding the signals so they can used in a state machine node. But ... it doesn't work for some reason. I'd appreciate some guidance.

@Xrayez
Copy link
Contributor

Xrayez commented Jul 13, 2019

Basically the AnimationTree is forwarding the signals so they can used in a state machine node.

#30508: aims to do something similar from inside animation state machine instead.

What if a state machine doesn't operate on AnimationNodeAnimation but AnimationNodeBlendTree or similar? This could only work for state machine if it has animation nodes exclusively I guess.

The feature is worth to implement anyways.

@Xrayez
Copy link
Contributor

Xrayez commented Jul 13, 2019

But ... it doesn't work for some reason. I'd appreciate some guidance.

Something tells me that AnimationTree may bypass the AnimationPlayer logic, and it might use it only as a container for animations?

@Rodeo-McCabe
Copy link
Author

In AnimationTree::_process_graph() I see there are a few places where an AnimationPlayer is having its play() function called. This made me think that it was using the AnimationPlayer for processing logic, but on further inspection it seems that AnimationPlayer is only used if simply playing an animation and nothing else. Blending and everything else is re-implemented in AnimationTree, making it hard to find the proper places to emit the signals.

It seems to me like AnimationTree should be taking advantage of the code already present in AnimationPlayer, but perhaps that's not possible for some reason? In any case, I spent a few days over the last week trying to get it to work and I've been unsuccessful. Need some review by reduz or someone else who knows animation inside out.

@Rodeo-McCabe
Copy link
Author

Revisiting this now, I don't think this is the correct way to go about this. As @Xrayez pointed out, this won't work if the tree has something other than AnimationNodeAnimation nodes. I think #30508 is probably the better way to go about it, although it may not be a perfect replacement in all cases.

I'm going to close this since it doesn't work and I'm not planning to continue working on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants