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

AnimationTree does not apply Call Method Track for a method in an AudioStreamPlayer script. #48526

Closed
apples opened this issue May 7, 2021 · 2 comments · Fixed by #86687
Closed

Comments

@apples
Copy link
Contributor

apples commented May 7, 2021

Godot version:
3.3

OS/device including version:
Windows 10 Home, version 20H2, build 19042.928, HP Pavilion Gaming Laptop 16-a0xxx.
nVidia GeForce GTX 1660 Ti Max-Q, driver version 457.63, any backend.

Issue description:
Apologies, this is a weird one.

What happens: AnimationTree state machine does not apply method calls to an AudioStreamPlayer script in a Call Method Track in an Animation when another Animation exists on the same AnimationPlayer and which is alphabetically after the current animation and has an Audio Playback Track.

(yes, all of those details are important, see steps to reproduce below)

What I expected: The Call Method Track should get called regardless of other animations' Audio Playback Tracks and alphabetical status.

Steps to reproduce:
Gosh, where to begin...

Setup:

  1. Create a new Node2D scene with a child Sprite node to animate (optional but useful for debugging).
  2. Add AudioStreamPlayer node to the scene with some audio stream imported.
  3. Attach a script to the AudioStreamPlayer with a simple method that prints a debug message or something.
    extends AudioStreamPlayer
    
    func play_bloop():
        print("hi")
        play()
  4. Add an AnimationPlayer node to the scene.
  5. Create a simple animation named default which translates the Sprite (optional) and which has a Call Method track pointing to the AudioStreamPlayer. Optionally, make this looping.
  6. Add a keyframe to the Call Method track which calls the AudioStreamPlayer's method which was added in step 3.
    image
  7. Create a second animation named zzz which has an Audio Playback track with a keyframe that plays a stream to the AudioStreamPlayer (edit: actually, the keyframe is optional, only the track itself needs to exist).
    image
  8. Add an AnimationTree node to the scene.
  9. Point the AnimationTree to the AnimationPlayer, mark it as Active, and create a new state machine tree root.
  10. In the tree root, add a single node to play the default animation.
  11. Attach a script to the scene root node which starts the AnimationTree's state machine at the default node on ready:
    extends Node2D
    
    func _ready():
        #$AnimationPlayer.play("default")
        $AnimationTree["parameters/playback"].start("default")

Actual bug reproduction:

  1. Play the scene. Observe that the default animation is playing, but the method in the Call Method track is not being called.
  2. Do any one of the following:
    • Change the root node script to play the animation via the AnimationPlayer directly, without using the AnimationTree.
    • Remove the Audio Playback track from the zzz animation.
    • Rename the zzz animation to aaa.
  3. Play the scene. Observe that the method is now being called as expected.

Additional notes:

  • If there are multiple other animations which are similar to zzz, they must all be "fixed" in order to get the method calls to function.
  • This does not affect a Call Method track which targets a node other than the AudioStreamPlayer.
  • [screaming]

Minimal reproduction project:
AudioAnimationTreeTest.zip

@TokageItLab
Copy link
Member

TokageItLab commented May 11, 2021

The opposite problem occurs if the order of the animations is swapped. The AnimationTree first iterates through all the animations and their tracks in the AnimationPlayer in preparation for blending, but there is a problem with this process.

line 546 in AnimationTree.cpp

//if not valid, delete track
if (track && (track->type != track_type || ObjectDB::get_instance(track->object_id) == nullptr)) {
	playing_caches.erase(track);
	memdelete(track);
	track_cache.erase(path);
	track = nullptr;
}

The same problem also occurs with BezieTrack and ValueTrack.

AnimationTreeTest.zip (Godot 4.0 Project File)

What does the track->type != track_type? Is this to prevent multiple types of track with same path in animations? If it is restriction as specification, I think it should be done as a UI, like giving a warning when adding a track. Or, if we want to solve this problem fundamentally, we should change the key of the cache map to path+track_type instead of only path.

@TokageItLab
Copy link
Member

TokageItLab commented May 12, 2021

Here is suggestion for AnimationTree improvement.

First of all, there are two solutions to the problem of duplicate paths being removed.

  • Generate a TrackCache for each animation
  • Append the TrackType to the path

The first one is not good from a performance point of view, because it expands the number of elements. So we should pick the latter.

Next, consider the original reason for being: to remove duplicate paths. Perhaps it want to prevent cases like the one in the image below.

スクリーンショット 2021-05-13 4 32 11

In this case, it is not possible to determine which track should be preferred, so we need to validate (BezierTrack splits the Vector into separate tracks, so we need to take this into account when validating). However, this should not be an implicit validation, but rather a UI warning when a track is added.

The most important thing to keep in mind is that we don't know if blending between things with different TrackTypes will work. A blend between a BezierTrack and a ValueTrack may work fine with some fixes, but there is no way to ensure that a blend between a ValueTrack and a MethodTrack will work.

As far as AudioTracks are concerned, forcing the blend to override the AudioStream's volume is also problematic - it may be useful for crossfading background music, but it's inconvenient when playing sound effects in OneShot. We think it should be selectable.

With all this in mind, I suggest that the blending issue should be resolved.

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