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

Characters animations using AnimationPlayer #7

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

mgdesign
Copy link

@mgdesign mgdesign commented Jan 26, 2023

Done:

  • character facing direction switch logic based on destination point
  • character animation based on AnimationPlayer and naming conventions
  • character turning and animation triggers while moving through the walk cycle
  • public play_animation() on the singleton, that can also be used in other interactions
  • animation added to the AnimationPlayer for any orientation are now play according to the player looking point
  • basic fallback logic on walk animations
  • added talk animation to test using the simplest case of animation
  • updated demo game main character sprite to introduce animated walk up/down/left cycles

TODO:

  • figure out if it is possible to implement the logic that relies on character looking direction and fallback for all the animations in general
  • add the AnimationPlayer in the character template

if has_node("AnimationPlayer"):
# Check if animation is null in case it is call dynamically.
if animation_label == null:
$AnimationPlayer.stop()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you stopping the animation if you call play_animation() without parameters? Isn't it better to log a warning to the console and leave the running animation going.

In my head you want to stop an anim by explicitely invoking a stop_animation() method.

Copy link
Contributor

@stickgrinder stickgrinder Jan 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that I think of it, you can't reach this point with a null value, the type is explicit for that argument.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, thanks for the feedback!

Comment on lines 318 to 319
LOOKING.DOWN: animation_label = 'walk_f'
LOOKING.UP: animation_label = 'walk_b'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would keep the animation suffix consistend with "down" and "up", so walk_d' and walk_u`.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! I'm fixing this

@mgdesign
Copy link
Author

Add a few improvements (update the main comment accordingly) thanks to a great pair programming session with @stickgrinder and drbb

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

Successfully merging this pull request may close these issues.

2 participants