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

AnimationPlayer timeline error #95479

Closed
pmoosi opened this issue Aug 13, 2024 · 8 comments · Fixed by #95481
Closed

AnimationPlayer timeline error #95479

pmoosi opened this issue Aug 13, 2024 · 8 comments · Fixed by #95481

Comments

@pmoosi
Copy link

pmoosi commented Aug 13, 2024

Tested versions

Reproducible in 4.3 after commit [29a0e51]

System information

Godot v4.3.rc.mono (f7f2023) - Ubuntu 22.04.4 LTS 22.04 - Wayland - Vulkan (Forward+) - integrated AMD Unknown (RADV RENOIR) - AMD Ryzen 7 5800H with Radeon Graphics (16 Threads)

Issue description

Trying to do anything with the animation players timeline since #95400 results in

core/object/object.cpp:1200 - Error calling from signal 'timeline_changed' to callable: 'AnimationPlayerEditor::AnimationPlayerEditor::_animation_key_editor_seek': Method expected 3 arguments, but called with 4.

Steps to reproduce

Create a AnimationPlayer node, an animation and click anywhere in the timeline.

Minimal reproduction project (MRP)

N/A

@akien-mga
Copy link
Member

CC @TokageItLab @AThousandShips

@AThousandShips
Copy link
Member

I'll take a look, that's very weird

@AThousandShips
Copy link
Member

Seems the signal was declared with two arguments, but is called some times with three, will write up a fix

@AThousandShips AThousandShips self-assigned this Aug 13, 2024
@akien-mga
Copy link
Member

akien-mga commented Aug 13, 2024

The timeline_changed signal is emitted sometimes with 2 args and sometimes with 3 in AnimationTrackEditor.

So I guess we need to remove the bind(false) and manually add false to all the emits that only have 2 args.

And the signal registration likely needs to register the 3rd arg too:

ADD_SIGNAL(MethodInfo("timeline_changed", PropertyInfo(Variant::FLOAT, "position"), PropertyInfo(Variant::BOOL, "timeline_only")));

@AThousandShips
Copy link
Member

AThousandShips commented Aug 13, 2024

Will do and update the signature so it matches what it actually does

There's also three different signals, will dig through and fix

Edit: This set of signals is a whole jumble, will see how to best handle it

@AThousandShips
Copy link
Member

So AnimationTrackEdit has this signal, but doesn't seem to ever emit it, with two arguments, that is connected to AinmationTrackEditor::_timeline_changed which takes two arguments and emits this signal with false for the third, it also emits the signal directly in another place with true, and twice without third argument

AnimationTimelineEdit also has this signal but doesn't use a third argument

So will write up a basic fix to make the AnimationTrackEdtor registered correctly, and make it emit the correct number of arguments in those cases, and leave the rest for future cleanup

@akien-mga
Copy link
Member

Thanks @pmoosi for the quick testing and regression report btw!

@pmoosi
Copy link
Author

pmoosi commented Aug 14, 2024

Thank you for the quick fix. You guys are doing amazing work :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Release Blocker
Development

Successfully merging a pull request may close this issue.

3 participants