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

Handle internal seek on AnimationPlayer to process discrete correctly #94420

Merged
merged 1 commit into from
Jul 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions editor/plugins/animation_player_editor_plugin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ void AnimationPlayerEditor::_play_from_pressed() {
player->clear_caches(); //so it won't blend with itself
}
ERR_FAIL_COND_EDMSG(!_validate_tracks(player->get_animation(current)), "Animation tracks may have any invalid key, abort playing.");
player->seek(time, true, true);
player->seek_internal(time, true, true, true);
player->play(current);
}

Expand Down Expand Up @@ -286,7 +286,7 @@ void AnimationPlayerEditor::_play_bw_from_pressed() {
player->clear_caches(); //so it won't blend with itself
}
ERR_FAIL_COND_EDMSG(!_validate_tracks(player->get_animation(current)), "Animation tracks may have any invalid key, abort playing.");
player->seek(time, true, true);
player->seek_internal(time, true, true, true);
player->play_backwards(current);
}

Expand Down Expand Up @@ -1295,7 +1295,7 @@ void AnimationPlayerEditor::_seek_value_changed(float p_value, bool p_timeline_o
pos = CLAMP(pos, 0, (double)anim->get_length() - CMP_EPSILON2); // Hack: Avoid fposmod with LOOP_LINEAR.

if (!p_timeline_only && anim.is_valid()) {
player->seek(pos, true, true);
player->seek_internal(pos, true, true, false);
}

track_editor->set_anim_pos(pos);
Expand Down Expand Up @@ -1706,7 +1706,7 @@ void AnimationPlayerEditor::_prepare_onion_layers_2_step_prepare(int p_step_offs
bool valid = anim->get_loop_mode() != Animation::LOOP_NONE || (pos >= 0 && pos <= anim->get_length());
onion.captures_valid[p_capture_idx] = valid;
if (valid) {
player->seek(pos, true, true);
player->seek_internal(pos, true, true, false);
OS::get_singleton()->get_main_loop()->process(0);
// This is the key: process the frame and let all callbacks/updates/notifications happen
// so everything (transforms, skeletons, etc.) is up-to-date visually.
Expand Down Expand Up @@ -1763,7 +1763,7 @@ void AnimationPlayerEditor::_prepare_onion_layers_2_epilog() {
// backed by a proper reset animation will work correctly with onion
// skinning and the possibility to restore the values mentioned in the
// first point above is gone. Still good enough.
player->seek(onion.temp.anim_player_position, true, true);
player->seek_internal(onion.temp.anim_player_position, true, true, false);
player->restore(onion.temp.anim_values_backup);

// Restore state of main editors.
Expand Down
27 changes: 17 additions & 10 deletions scene/animation/animation_player.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ void AnimationPlayer::_notification(int p_what) {
}
}

void AnimationPlayer::_process_playback_data(PlaybackData &cd, double p_delta, float p_blend, bool p_seeked, bool p_started, bool p_is_current) {
void AnimationPlayer::_process_playback_data(PlaybackData &cd, double p_delta, float p_blend, bool p_seeked, bool p_internal_seeked, bool p_started, bool p_is_current) {
double speed = speed_scale * cd.speed_scale;
bool backwards = signbit(speed); // Negative zero means playing backwards too.
double delta = p_started ? 0 : p_delta * speed;
Expand Down Expand Up @@ -237,9 +237,8 @@ void AnimationPlayer::_process_playback_data(PlaybackData &cd, double p_delta, f
if (Math::is_zero_approx(pi.delta) && backwards) {
pi.delta = -0.0; // Sign is needed to handle converted Continuous track from Discrete track correctly.
}
// AnimationPlayer doesn't have internal seeking.
// However, immediately after playback, discrete keys should be retrieved with EXACT mode since behind keys must be ignored at that time.
pi.is_external_seeking = !p_started;
// Immediately after playback, discrete keys should be retrieved with EXACT mode since behind keys must be ignored at that time.
pi.is_external_seeking = !p_internal_seeked && !p_started;
pi.looped_flag = looped_flag;
pi.weight = p_blend;
make_animation_instance(cd.from->name, pi);
Expand All @@ -259,13 +258,15 @@ void AnimationPlayer::_blend_playback_data(double p_delta, bool p_started) {
Playback &c = playback;

bool seeked = c.seeked; // The animation may be changed during process, so it is safer that the state is changed before process.
bool internal_seeked = c.internal_seeked;

if (!Math::is_zero_approx(p_delta)) {
c.seeked = false;
c.internal_seeked = false;
}

// Second, process current animation to check if the animation end reached.
_process_playback_data(c.current, p_delta, get_current_blend_amount(), seeked, p_started, true);
_process_playback_data(c.current, p_delta, get_current_blend_amount(), seeked, internal_seeked, p_started, true);

// Finally, if not end the animation, do blending.
if (end_reached) {
Expand All @@ -282,7 +283,7 @@ void AnimationPlayer::_blend_playback_data(double p_delta, bool p_started) {
}
// Note: There may be issues if an animation event triggers an animation change while this blend is active,
// so it is best to use "deferred" calls instead of "immediate" for animation events that can trigger new animations.
_process_playback_data(b.data, p_delta, b.blend_left, false, false);
_process_playback_data(b.data, p_delta, b.blend_left, false, false, false);
}
for (List<Blend>::Element *&E : to_erase) {
c.blend.erase(E);
Expand Down Expand Up @@ -450,10 +451,10 @@ void AnimationPlayer::_play(const StringName &p_name, double p_custom_blend, flo
} else {
if (p_from_end && c.current.pos == 0) {
// Animation reset but played backwards, set position to the end.
seek(c.current.from->animation->get_length(), true, true);
seek_internal(c.current.from->animation->get_length(), true, true, true);
} else if (!p_from_end && c.current.pos == c.current.from->animation->get_length()) {
// Animation resumed but already ended, set position to the beginning.
seek(0, true, true);
seek_internal(0, true, true, true);
} else if (playing) {
return;
}
Expand Down Expand Up @@ -579,7 +580,7 @@ float AnimationPlayer::get_playing_speed() const {
return speed_scale * playback.current.speed_scale;
}

void AnimationPlayer::seek(double p_time, bool p_update, bool p_update_only) {
void AnimationPlayer::seek_internal(double p_time, bool p_update, bool p_update_only, bool p_is_internal_seek) {
if (!active) {
return;
}
Expand All @@ -600,12 +601,18 @@ void AnimationPlayer::seek(double p_time, bool p_update, bool p_update_only) {
}

playback.seeked = true;
playback.internal_seeked = p_is_internal_seek;

if (p_update) {
_process_animation(is_backward ? -0.0 : 0.0, p_update_only);
playback.seeked = false; // If animation was proceeded here, no more seek in internal process.
}
}

void AnimationPlayer::seek(double p_time, bool p_update, bool p_update_only) {
seek_internal(p_time, p_update, p_update_only);
}

void AnimationPlayer::advance(double p_time) {
_check_immediately_after_start();
AnimationMixer::advance(p_time);
Expand Down Expand Up @@ -661,7 +668,7 @@ void AnimationPlayer::_stop_internal(bool p_reset, bool p_keep_state) {
c.current.pos = 0;
} else {
is_stopping = true;
seek(0, true, true);
seek_internal(0, true, true, true);
is_stopping = false;
}
c.current.from = nullptr;
Expand Down
4 changes: 3 additions & 1 deletion scene/animation/animation_player.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ class AnimationPlayer : public AnimationMixer {
PlaybackData current;
StringName assigned;
bool seeked = false;
bool internal_seeked = false;
bool started = false;
List<Blend> blend;
} playback;
Expand Down Expand Up @@ -116,7 +117,7 @@ class AnimationPlayer : public AnimationMixer {

void _play(const StringName &p_name, double p_custom_blend = -1, float p_custom_scale = 1.0, bool p_from_end = false);
void _capture(const StringName &p_name, bool p_from_end = false, double p_duration = -1.0, Tween::TransitionType p_trans_type = Tween::TRANS_LINEAR, Tween::EaseType p_ease_type = Tween::EASE_IN);
void _process_playback_data(PlaybackData &cd, double p_delta, float p_blend, bool p_seeked, bool p_started, bool p_is_current = false);
void _process_playback_data(PlaybackData &cd, double p_delta, float p_blend, bool p_seeked, bool p_internal_seeked, bool p_started, bool p_is_current = false);
void _blend_playback_data(double p_delta, bool p_started);
void _stop_internal(bool p_reset, bool p_keep_state);
void _check_immediately_after_start();
Expand Down Expand Up @@ -200,6 +201,7 @@ class AnimationPlayer : public AnimationMixer {
void set_movie_quit_on_finish_enabled(bool p_enabled);
bool is_movie_quit_on_finish_enabled() const;

void seek_internal(double p_time, bool p_update = false, bool p_update_only = false, bool p_is_internal_seek = false);
void seek(double p_time, bool p_update = false, bool p_update_only = false);

double get_current_animation_position() const;
Expand Down
Loading