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

Blending/Animating String or StringName tracks cause odd interpolation #91867

Open
takanan opened this issue May 12, 2024 · 8 comments
Open

Blending/Animating String or StringName tracks cause odd interpolation #91867

takanan opened this issue May 12, 2024 · 8 comments

Comments

@takanan
Copy link

takanan commented May 12, 2024

Tested versions

  • Reproducible in: v4.3.dev6.official [64520fe], v4.3.dev5.official [c9c17d6]
  • Not reproducible in: v4.2.2

System information

Godot v4.3.dev6 - Arch Linux #1 SMP PREEMPT_DYNAMIC Tue, 07 May 2024 21:35:54 +0000 - X11 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 2060 (nvidia; 550.78) - Intel(R) Core(TM) i7-6700K CPU @ 4.00GHz (8 Threads)

Issue description

  • Blending two animations with each having a single keyframe of String/StringName, it interpolates in a weird way.

    • blending_two_string_keyframes.webm
    • Those will occur regardless setting keyframes to discrete update mode or nearest interpolation mode.
    • Especially blending with Add2, it will show broken text. Might be showing something internal memory things, which is not good.
      • No security issues for interpolating Strings
  • Also just animating a String/StringName track with continuous update mode AND linear interpolation causes same issue.

Steps to reproduce

  • Create any node which has String or StringName property
  • Create two animations of the property
  • Blend those two animations on AnimationTree with any blending methods (e.g. Add2, Blend2, OneShot with fadein/fadeout time, ...)

Minimal reproduction project (MRP)

N/A

@Protowalker
Copy link

There's no memory safety issues happening here; it's intentional, it's trying to interpolate between the two strings and find a string that's somewhere in the middle.

case Variant::STRING_NAME: {
Not sure when that would be desired, but it looks like it's not a bug.

@takanan
Copy link
Author

takanan commented May 13, 2024

I've looked the provided part of code and there is no security issue (reading out of memory boundary). Thank you for the indication. Treating string as float array and interpolating it is fancy and I like that.

Although, a string property track being interpolated even it's set to discrete update mode and nearest interpolation is a bug to me.

For example, AnimatedSprite2D:animation has StringName type. If you have two animations each changes the property, and blend them on AnimationTree, despite the track is set to discrete and nearest, you will get error messages like this every time it's being blended (which is expected error message).
Screenshot_20240513_200431

@Protowalker
Copy link

As far as I can tell, this behavior happens for any property track, not just string properties, correct? If so, this seems less like a bug more like a design decision. Personally I think it makes sense for a blend node to just interpolate between whatever the two values of a track would be in the two source animations. Maybe the docs could be more explicit?

@takanan
Copy link
Author

takanan commented May 13, 2024

A track with discrete update or nearest interpolation for String/StringName will be properly interpolated like a binary inbetween keyframes. But for blending on AnimationTree, it will be interpolated fancy way.

Is this a design decision?
For me it's just incoherent.

@takanan
Copy link
Author

takanan commented May 13, 2024

this behavior happens for any property track, not just string properties

I don't have enough time for exhaustive test to prove this, but I guess so. Confirmed enums are also interpolated as they treated as integer.

I'm not dev so I don't know if this is desired behavior or not, but if so, please add document on animation section to not use discrete property for AnimationTree. Since any discrete property is forced being interpolated otherwise its inbetween value is unintentionally used (e.g. Node:process_mode from Inherit=0, Paused=1, WhenPaused=2, Always=3 to Disabled=4).

@TokageItLab
Copy link
Member

TokageItLab commented May 13, 2024

The change in behavior in 4.3.dev is due to a change in the handling of UpdateMode.Discrete.

"All tracks are Discrete" in 4.2 means that CallbackModeDiscrete in 4.3 is Dominant. The default in 4.3 is ForceContinuous, which is suitable for blending; This means that during blending, Discrete/int is treated as Contiunous/float, and after blending, the values are rounded back to int. Also, most users are not aware of the difference between Discrete and Continuous, and that fact make the issues about integer numbers not being blended have been posted many times in the past, it is the reason why ForceContinuous is the default in AnimationTree. And, I believe that in Godot 5 the int value should default to Continuous + Nearest and the AnimationTree default to Dominant in the future.

Eventually, if you want the same behavior as in 4.2 with proper behavior, set CallbackModeDiscrete to Dominant.

image

I am preparing a migration document for this and will close this issue after publishing it.

@takanan
Copy link
Author

takanan commented May 14, 2024

This is little bit complicated so I try to clear it for myself.

  • A discrete property
    • value like StringName or Enum flags where interpolation is not preferable
  • An animation track with discrete update mode
    • property is updated only on keyframes

If I set AnimationTree:CallbackMode to Dominant, an animation track with Discrete update mode doesn't keep updated in blending (#90897) because its nature of "only update on keyframes". In order to keep property updated on animation blending, continuous update mode must be applied.

I have a discrete property that I want to use in animation blending and keep it being updated, my only option is create a track for the property with Continuous update mode with Nearest interpolation. But as I reported, this is not work since the property is getting linearly interpolated in blending process. So, the problem is Nearest interpolation is not working in animation blending. Am I correct?

@TokageItLab
Copy link
Member

TokageItLab commented May 22, 2024

InterpolationType defines the interpolation when the animation is played back. Rotation properties reference it as a Hack for special cases, but otherwise InterpolationType, whether int, String and others, has no effect on blending.

There is no implementation in the current AnimationTree that switches between two properties with a threshold for example 0.5. If you want to do that, you currently have to use Disctrete and exceed the key (although I understand that it is different from a switching by a threshold). See also godotengine/godot-proposals#8085.

For that reason, such as Resource (and other non-interpolatable type) property is forced to blend in Disctrete mode, even when using Continuous mode.

Another possible option would be BlendSpace, since it has a Discrete “blend” mode that you could use.

image

Well, although you may need to separate the AnimationTree for the String. In any case, it is impossible to mix linear and nonlinear blends without using Discrete for now.

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

No branches or pull requests

4 participants