Fix Go to Previous Keyframe shortcut needs to be pressed twice to work

[ydeltastar]

• Fix Go to Previous Keyframe shortcut needs to be pressed twice to work #117543

go_to_nearest_keyframe() seeks to the exact keyframe time, then updates the timeline spinbox with frame->set_value(nearest_key_time). This triggers _seek_value_changed(), because it is connected to the spinner's value_changed signal. The method snaps the position to the editor step grid and sets the animation player to that snapped value, overwriting the exact keyframe time.

It worked the second time because the spinbox doesn't emit the value_changed signal since it's the same snapped value.

[TokageItLab]

This PR might be okay as a temporary band-aid fix.

However, judging from the description, I'm skeptical that "snap" should even be performed within that _seek_value_changed(). Since snapping is performed within _animation_key_editor_seek(), which calls _seek_value_changed(), shouldn't we be passing the snapped value instead?

void AnimationPlayerEditor::_animation_key_editor_seek(float p_pos, bool p_timeline_only, bool p_update_position_only) {
	double pos = track_editor->is_snap_timeline_enabled() ? Math::snapped(p_pos, _get_editor_step()) : p_pos; // Snapped pos.
	timeline_position = pos;

	if (!is_visible_in_tree() ||
			p_update_position_only ||
			!player ||
			player->is_playing() ||
			!player->has_animation(player->get_assigned_animation())) {
		return;
	}

	updating = true;
	frame->set_value(pos );
	updating = false;
	_seek_value_changed(pos , p_timeline_only);
}

Also, it seems that snapping via the frame SpinBox signal is not functioning correctly in the first place (see also #34431). So I think it should be refactored and PR #118031 will likely need to be reconsidered/rewritten with that premise in mind.

[ydeltastar]

Snapping at _animation_key_editor_seek() seems to break the interaction with snap features altogether.

Well, the main issue is that the system is too intertwined with the UI and signals. The logic sets the UI, then UI emit signals which also set the state, then the state triggers UI updates, and so on. The updating flag seems to be used in many places to fix similar issues for the same reasons, so I followed the pattern.

A general solution is probably to refactor all state logic in AnimationPlayerEditor using set_block_signals(), like AnimationTrackEditor does. But it's outside of this PR context.