Player: Can we improve the ledge animation call?

[NathanLovato]

I've changed Skin.gd, the node responsible for playing the player skin's animations, so it has a single public play method. But for animations like ledge, you need to animate the skin moving in the game world independently from the player. So you need some information from the corresponding state. I've made this an optional dictionary:

var animation_data: = {
  from=global_position_start,
  to=player.global_position,
}
player.skin.play(ledge, animation_data)

As a result, to use this special animation, you've got to know which parameters to use in the dictionary. And you can only know that by opening Skin.gd. Can we improve this? Should skin expose animate_ledge, a special method just to animate the ledge, and unique methods for each animation that works in a similar way?

[razcore-rad]

I don't see these changes in Skin.gd just yet, I'd need to see it in code to have a better understanding.

[NathanLovato]

I had forgotten to push the commit as I worked offline. Now it's in the project, a bit simplified as well. You can check Player/Skin.gd

[razcore-rad]

I've been thinking about this a bit. We can do a simplification from the API point of view using signals, so Ledge.gd state emits a "start_animation" say signal that is used in Skin.gd to set the positions before playing the animation. That said, it complicates variable/data passing. So I'm not sure if this is the better solution. I'd probably end up writing some dosctring on the Player.gd script to let programmers know the Skin API if it's a problem to check for such things in the script.

While exploring this I simplified something else instead :)... I've come up with:

extends Position2D
"""
The player's animated skin. Provides a simple interface to play animations.
"""

onready var anim: AnimationPlayer = $AnimationPlayer

func play(name: String, data: Dictionary = {}) -> void:
    """
    Plays the requested animation and safeguards against errors
    """
    anim.stop()
    if name == "ledge":
        assert 'from' in data
        position = data.from
        var anim_ledge: = anim.get_animation(name)
        anim_ledge.track_set_key_value(0, 0, position)
    anim.play(name)

For Skin.gd, no need for Tween and such, and instead of Skin.gd emitting the signal, we just use the anim node directly in Ledge.gd:

func setup(player: KinematicBody2D, state_machine: Node) -> void:
    .setup(player, state_machine)
    _player.skin.anim.connect("animation_finished", self, "_on_Skin_animation_finished")

This is a simplification from the nodes/code point of view, but it is more "advanced" in the sense that we're using less known Godot features and they aren't as clear as using Tween. But it does unify the animation handling a bit more.

Don't know if this helps, but there you go, some options.

see. razcore-art/godot-metroidvania-2d@0f4142ab9e2256f8182a0f6e8a06820b5113b154 for reference

[NathanLovato]

Ah, you're using the frame capture mode? I didn't consider it as you can only have linear interpolation, but it works for character movement. Great idea :)

Can you PR your improvements and make them close this issue?

[razcore-rad]

No... Not frame capture mode, that rings a bell but I forgot what it does. I just created a ledge animation in the AnimationPlayer and set the keyframe value programmatically. It works with other interpolation methods too. The interpolation can be set per keyframe.

Only thing that I don't like on this solution is that we're using player.skin.play to do the animation, but we're connecting to player.skin.anim's animation_finished signal. There's a bit of asymmetry there.

I think it's better to bring back the animation_finished signal on Skin.

I'll submit a PR shortly. Just woke up and have a couple of hours before I go to the country side.

[NathanLovato]

In capture mode, you don't have to change the keyframes from code: Godot will interpolate all tracks set to capture from the nodes' current state. When you have multiple anim tracks it can be useful.

[NathanLovato]

I'll have to try, that would be nice to cover in a tutorial as well, as it's a 3.1 feature