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

[Editor] Fix AnimationTrackEditor::timeline_changed signal #95481

Merged
merged 1 commit into from
Aug 13, 2024

Conversation

AThousandShips
Copy link
Member

Signal was declared with two arguments, emitted with both two and three

The signal emitted by AnimationTimelineEdit has two arguments and left as is, as is the signal from AnimationTrackEdit which is connected via a helper method in AnimationTrackEditor, unsure what the intention might have been there but that signal is never emitted by AnimationTrackEdit, but leaving that for a future cleanup and assessment

Confirmed properly that this fixes the issue, should have confirmed more properly in last attempted fix

Copy link
Member

@TokageItLab TokageItLab left a comment

Choose a reason for hiding this comment

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

I think the two signals should have different names to avoid confusion, and after then fix the incorrect bindings.

editor/animation_track_editor.cpp Show resolved Hide resolved
@AThousandShips
Copy link
Member Author

The signal for AnimationTimelineEdit and AnimationTrackEdit you mean?

@AThousandShips
Copy link
Member Author

AThousandShips commented Aug 13, 2024

To clarify, there are tree signals:


1:

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

Emitted here:

emit_signal(SNAME("timeline_changed"), ofs, mm->is_alt_pressed());

emit_signal(SNAME("timeline_changed"), ofs, mb->is_alt_pressed());

And connected to here:

timeline->connect("timeline_changed", callable_mp(this, &AnimationTrackEditor::_timeline_changed));


2:

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

Never emitted

And connected to here:

track_edit->connect("timeline_changed", callable_mp(this, &AnimationTrackEditor::_timeline_changed));


3:

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

Emitted here:

emit_signal(SNAME("timeline_changed"), pos, p_timeline_only);

emit_signal(SNAME("timeline_changed"), pos, false);

emit_signal(SNAME("timeline_changed"), p_pos, true, true);

emit_signal(SNAME("timeline_changed"), p_new_pos, p_timeline_only, false);

And connected to here:

track_editor->connect(SNAME("timeline_changed"), callable_mp(this, &AnimationPlayerEditor::_animation_key_editor_seek).bind(false));

Signal was declared with two arguments, emitted with both two and three
@akien-mga akien-mga merged commit 06fbc83 into godotengine:master Aug 13, 2024
25 checks passed
@akien-mga
Copy link
Member

Thanks!

@AThousandShips AThousandShips deleted the anim_signal_fix_2 branch August 13, 2024 13:45
@AThousandShips
Copy link
Member Author

Thank you!

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

Successfully merging this pull request may close these issues.

AnimationPlayer timeline error
3 participants