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

Add AnimationNode lifecycle callbacks #41771

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

RolandMQuiros
Copy link

@RolandMQuiros RolandMQuiros commented Sep 4, 2020

Adds lifecycle script callbacks to AnimationNode so users can attach logic directly to animations.

Feature proposal: Fixes godotengine/godot-proposals#1462 (comment)

@RolandMQuiros RolandMQuiros requested a review from a team as a code owner September 4, 2020 20:45
@RolandMQuiros RolandMQuiros changed the title Animation node processing 4.0 AnimationNode lifecycle callbacks Sep 4, 2020
@Calinou Calinou added enhancement topic:core cherrypick:3.x Considered for cherry-picking into a future 3.x release labels Sep 4, 2020
@Calinou Calinou added this to the 4.0 milestone Sep 4, 2020
@fire
Copy link
Member

fire commented Sep 4, 2020

Can you provide a small demo? I'm very interested in this.

Edited:

I noticed you had some problems with the static checks. Can walk you through the process.

@RolandMQuiros
Copy link
Author

I'm throwing a demo project together now, hopefully will have it ready in a few hours.

Appreciate the support--I'm not sure what to make of the static check output, since it's showing me a diff on a whitespace.

@fire
Copy link
Member

fire commented Sep 5, 2020

misc\scripts\clang_format.sh can help.

@RolandMQuiros
Copy link
Author

RolandMQuiros commented Sep 5, 2020

I threw together a rough demo project with a simple character. WASD to move, space to jump, left click to attack: https://github.com/RolandMQuiros/animation-node-lifecycle-demo

I was having a rough time working through some UI bugs in 4.0, so I had to backport my changes to 3.2. You'll have to build Godot from this branch to run the demo: https://github.com/RolandMQuiros/godot/tree/animation-node-processing-3.2

If you look at the AnimationTree in the FSMCharacter scene, and click on one of the states, you'll see it has an fsm_node script attached, which exports an array of Resources. Each of those resources has a script attached that hooks into the lifecycle callbacks I implemented.

image

By organizing my code this way, I can write smaller and more reusable scripts that execute only when their attached AnimationNode is processing. For example, the Punch state doesn't have a Gravity behavior, so your dude will be suspended in midair while the animation plays. No need to explicitly check the Animator's current state in code.

Let me know if you have any issues getting the demo to run.

@RolandMQuiros
Copy link
Author

RolandMQuiros commented Sep 8, 2020

Messed up a pull, sorry for the notifications. I didn't mean to add every dev as a reviewer, and I'm not sure how to undo that.

@fire
Copy link
Member

fire commented Sep 8, 2020

I didn't have any time to review this. Try asking people in the discord.

Edited: Asked jfons

@RolandMQuiros RolandMQuiros force-pushed the animation-node-processing-4.0 branch from c8f5c93 to aaeb0ba Compare September 8, 2020 20:06
@fire
Copy link
Member

fire commented Sep 10, 2020

To be ready for merging, the commits have to be either merged or there is a good reason to separate them for logical consistency.

Edited:

See https://docs.godotengine.org/en/stable/community/contributing/pr_workflow.html

Also exposed child AnimationNodes as `child_nodes` property
@Rodeo-McCabe
Copy link

This is probably a solution to #28311

I would love to see this merged!

@Calinou Calinou changed the title AnimationNode lifecycle callbacks Add AnimationNode lifecycle callbacks Mar 30, 2021
@TokageItLab
Copy link
Member

TokageItLab commented May 24, 2021

The callback should be implemented as a signal, it is Godot's design concept. Also, it would be redundant to watch every AnimationNode's child class. So, I think "firing signals in AnimationNodeAnimation, AnimationNodeBlendTree, AnimationNodeBlendSpace2D and AnimationNodeBlendSpace1D which may will be called by AnimationNodeStateMachine" seems to be the right way to go.

@fire fire requested review from a team and removed request for bojidar-bg August 25, 2021 04:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherrypick:3.x Considered for cherry-picking into a future 3.x release enhancement topic:animation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add AnimationNode lifecycle callbacks
7 participants