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.

Sign up for GitHub

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

Fix seeking Animation immediately after playback for Discrete track #92861

Merged
merged 1 commit into from
Jun 10, 2024
Merged

Fix seeking Animation immediately after playback for Discrete track #92861

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions doc/classes/Animation.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,9 +360,12 @@
<param index="1" name="time" type="float" />
<param index="2" name="find_mode" type="int" enum="Animation.FindMode" default="0" />
<param index="3" name="limit" type="bool" default="false" />
<param index="4" name="backward" type="bool" default="false" />
TokageItLab marked this conversation as resolved.
Show resolved Hide resolved
<description>
Finds the key index by time in a given track. Optionally, only find it if the approx/exact time is given.
If [param limit] is [code]true[/code], it does not return keys outside the animation range.
If [param backward] is [code]true[/code], the direction is reversed in methods that rely on one directional processing.
For example, in case [param find_mode] is [constant FIND_MODE_NEAREST], if there is no key in the current position just after seeked, the first key found is retrieved by searching before the position, but if [param backward] is [code]true[/code], the first key found is retrieved after the position.
</description>
</method>
<method name="track_get_interpolation_loop_wrap" qualifiers="const">
Expand Down Expand Up @@ -583,6 +586,7 @@
<param index="2" name="backward" type="bool" default="false" />
<description>
Returns the interpolated value at the given time (in seconds). The [param track_idx] must be the index of a value track.
A [param backward] mainly affects the direction of key retrieval of the track with [constant UPDATE_DISCRETE] converted by [constant AnimationMixer.ANIMATION_CALLBACK_MODE_DISCRETE_FORCE_CONTINUOUS] to match the result with [method track_find_key].
</description>
</method>
<method name="value_track_set_update_mode">
Expand Down
14 changes: 7 additions & 7 deletions editor/plugins/animation_player_editor_plugin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,12 +243,12 @@ void AnimationPlayerEditor::_play_from_pressed() {
String current = _get_current();

if (!current.is_empty()) {
float time = player->get_current_animation_position();
double time = player->get_current_animation_position();
if (current == player->get_assigned_animation() && player->is_playing()) {
player->stop(); //so it won't blend with itself
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);
player->seek(time, true, true);
player->play(current);
}

Expand Down Expand Up @@ -281,12 +281,12 @@ void AnimationPlayerEditor::_play_bw_from_pressed() {
String current = _get_current();

if (!current.is_empty()) {
float time = player->get_current_animation_position();
if (current == player->get_assigned_animation()) {
player->stop(); //so it won't blend with itself
double time = player->get_current_animation_position();
if (current == player->get_assigned_animation() && player->is_playing()) {
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);
player->seek(time, true, true);
player->play_backwards(current);
}

Expand Down
6 changes: 6 additions & 0 deletions misc/extension_api_validation/4.2-stable.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
This file contains the expected output of --validate-extension-api when run against the extension_api.json of the

Check warning on line 1 in misc/extension_api_validation/4.2-stable.expected

View workflow job for this annotation

GitHub Actions / 🐧 Linux / Editor w/ Mono (target=editor)

The following validation errors no longer occur (compared to 4.2-stable): 

Validate extension JSON: Error: Field 'classes/Animation/methods/track_find_key/arguments': size changed value in new API, from 3 to 4.
4.2-stable tag (the basename of this file).

Only lines that start with "Validate extension JSON:" matter, everything else is considered a comment and ignored. They
Expand Down Expand Up @@ -364,3 +364,9 @@
Validate extension JSON: Error: Field 'classes/EditorInspectorPlugin/methods/add_property_editor/arguments': size changed value in new API, from 3 to 4.

Optional arguments added. Compatibility methods registered.

GH-86661
--------
Validate extension JSON: Error: Field 'classes/Animation/methods/track_find_key/arguments': size changed value in new API, from 3 to 5.

Added optional argument to track_find_key to handle backward seeking. Compatibility method registered.
13 changes: 7 additions & 6 deletions scene/animation/animation_mixer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1109,6 +1109,7 @@ void AnimationMixer::_blend_process(double p_delta, bool p_update_only) {
real_t weight = ai.playback_info.weight;
Vector<real_t> track_weights = ai.playback_info.track_weights;
bool backward = signbit(delta); // This flag is used by the root motion calculates or detecting the end of audio stream.
bool seeked_backward = signbit(p_delta);
#ifndef _3D_DISABLED
bool calc_root = !seeked || is_external_seeking;
#endif // _3D_DISABLED
Expand Down Expand Up @@ -1463,7 +1464,7 @@ void AnimationMixer::_blend_process(double p_delta, bool p_update_only) {
}
} else {
if (seeked) {
int idx = a->track_find_key(i, time, is_external_seeking ? Animation::FIND_MODE_NEAREST : Animation::FIND_MODE_EXACT, true);
int idx = a->track_find_key(i, time, is_external_seeking ? Animation::FIND_MODE_NEAREST : Animation::FIND_MODE_EXACT, false, seeked_backward);
if (idx < 0) {
continue;
}
Expand Down Expand Up @@ -1744,8 +1745,10 @@ void AnimationMixer::_blend_apply() {
case Animation::TYPE_VALUE: {
TrackCacheValue *t = static_cast<TrackCacheValue *>(track);

if (t->use_discrete && !t->use_continuous) {
t->is_init = true; // If only disctere value is applied, no more RESET.
if (callback_mode_discrete == ANIMATION_CALLBACK_MODE_DISCRETE_FORCE_CONTINUOUS) {
t->is_init = false; // Always update in Force Continuous.
} else if (!t->use_continuous && (t->use_discrete || !deterministic)) {
t->is_init = true; // If there is no continuous value and only disctere value is applied or just started, don't RESET.
}

if ((t->is_init && (is_zero_amount || !t->use_continuous)) ||
Expand All @@ -1756,9 +1759,7 @@ void AnimationMixer::_blend_apply() {
break; // Don't overwrite the value set by UPDATE_DISCRETE.
}

if (callback_mode_discrete == ANIMATION_CALLBACK_MODE_DISCRETE_FORCE_CONTINUOUS) {
t->is_init = false; // Always update in Force Continuous.
} else {
if (callback_mode_discrete != ANIMATION_CALLBACK_MODE_DISCRETE_FORCE_CONTINUOUS) {
t->is_init = !t->use_continuous; // If there is no Continuous in non-Force Continuous type, it means RESET.
}

Expand Down
9 changes: 7 additions & 2 deletions scene/animation/animation_player.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,9 @@ void AnimationPlayer::_process_playback_data(PlaybackData &cd, double p_delta, f
pi.delta = delta;
pi.seeked = p_seeked;
}
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;
Expand All @@ -257,7 +260,7 @@ void AnimationPlayer::_blend_playback_data(double p_delta, bool p_started) {

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

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

Expand Down Expand Up @@ -581,6 +584,8 @@ void AnimationPlayer::seek(double p_time, bool p_update, bool p_update_only) {
return;
}

bool is_backward = p_time < playback.current.pos;

_check_immediately_after_start();

playback.current.pos = p_time;
Expand All @@ -596,7 +601,7 @@ void AnimationPlayer::seek(double p_time, bool p_update, bool p_update_only) {

playback.seeked = true;
if (p_update) {
_process_animation(0, p_update_only);
_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.
}
}
Expand Down
6 changes: 3 additions & 3 deletions scene/resources/animation.compat.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ Variant Animation::_value_track_interpolate_bind_compat_86629(int p_track, doubl
return value_track_interpolate(p_track, p_time, false);
}

int Animation::_track_find_key_bind_compat_86661(int p_track, double p_time, FindMode p_find_mode) const {
return track_find_key(p_track, p_time, p_find_mode, false);
int Animation::_track_find_key_bind_compat_92861(int p_track, double p_time, FindMode p_find_mode) const {
return track_find_key(p_track, p_time, p_find_mode, false, false);
}

void Animation::_bind_compatibility_methods() {
Expand All @@ -60,7 +60,7 @@ void Animation::_bind_compatibility_methods() {
ClassDB::bind_compatibility_method(D_METHOD("scale_track_interpolate", "track_idx", "time_sec"), &Animation::_scale_track_interpolate_bind_compat_86629);
ClassDB::bind_compatibility_method(D_METHOD("blend_shape_track_interpolate", "track_idx", "time_sec"), &Animation::_blend_shape_track_interpolate_bind_compat_86629);
ClassDB::bind_compatibility_method(D_METHOD("value_track_interpolate", "track_idx", "time_sec"), &Animation::_value_track_interpolate_bind_compat_86629);
ClassDB::bind_compatibility_method(D_METHOD("track_find_key", "track_idx", "time", "find_mode"), &Animation::_track_find_key_bind_compat_86661, DEFVAL(FIND_MODE_NEAREST));
ClassDB::bind_compatibility_method(D_METHOD("track_find_key", "track_idx", "time", "find_mode"), &Animation::_track_find_key_bind_compat_92861, DEFVAL(FIND_MODE_NEAREST));
}

#endif // DISABLE_DEPRECATED
22 changes: 11 additions & 11 deletions scene/resources/animation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1496,7 +1496,7 @@ void Animation::track_remove_key(int p_track, int p_idx) {
emit_changed();
}

int Animation::track_find_key(int p_track, double p_time, FindMode p_find_mode, bool p_limit) const {
int Animation::track_find_key(int p_track, double p_time, FindMode p_find_mode, bool p_limit, bool p_backward) const {
ERR_FAIL_INDEX_V(p_track, tracks.size(), -1);
Track *t = tracks[p_track];

Expand All @@ -1518,7 +1518,7 @@ int Animation::track_find_key(int p_track, double p_time, FindMode p_find_mode,
return key_index;
}

int k = _find(tt->positions, p_time, false, p_limit);
int k = _find(tt->positions, p_time, p_backward, p_limit);
if (k < 0 || k >= tt->positions.size()) {
return -1;
}
Expand All @@ -1545,7 +1545,7 @@ int Animation::track_find_key(int p_track, double p_time, FindMode p_find_mode,
return key_index;
}

int k = _find(rt->rotations, p_time, false, p_limit);
int k = _find(rt->rotations, p_time, p_backward, p_limit);
if (k < 0 || k >= rt->rotations.size()) {
return -1;
}
Expand All @@ -1572,7 +1572,7 @@ int Animation::track_find_key(int p_track, double p_time, FindMode p_find_mode,
return key_index;
}

int k = _find(st->scales, p_time, false, p_limit);
int k = _find(st->scales, p_time, p_backward, p_limit);
if (k < 0 || k >= st->scales.size()) {
return -1;
}
Expand All @@ -1599,7 +1599,7 @@ int Animation::track_find_key(int p_track, double p_time, FindMode p_find_mode,
return key_index;
}

int k = _find(bst->blend_shapes, p_time, false, p_limit);
int k = _find(bst->blend_shapes, p_time, p_backward, p_limit);
if (k < 0 || k >= bst->blend_shapes.size()) {
return -1;
}
Expand All @@ -1611,7 +1611,7 @@ int Animation::track_find_key(int p_track, double p_time, FindMode p_find_mode,
} break;
case TYPE_VALUE: {
ValueTrack *vt = static_cast<ValueTrack *>(t);
int k = _find(vt->values, p_time, false, p_limit);
int k = _find(vt->values, p_time, p_backward, p_limit);
if (k < 0 || k >= vt->values.size()) {
return -1;
}
Expand All @@ -1623,7 +1623,7 @@ int Animation::track_find_key(int p_track, double p_time, FindMode p_find_mode,
} break;
case TYPE_METHOD: {
MethodTrack *mt = static_cast<MethodTrack *>(t);
int k = _find(mt->methods, p_time, false, p_limit);
int k = _find(mt->methods, p_time, p_backward, p_limit);
if (k < 0 || k >= mt->methods.size()) {
return -1;
}
Expand All @@ -1635,7 +1635,7 @@ int Animation::track_find_key(int p_track, double p_time, FindMode p_find_mode,
} break;
case TYPE_BEZIER: {
BezierTrack *bt = static_cast<BezierTrack *>(t);
int k = _find(bt->values, p_time, false, p_limit);
int k = _find(bt->values, p_time, p_backward, p_limit);
if (k < 0 || k >= bt->values.size()) {
return -1;
}
Expand All @@ -1647,7 +1647,7 @@ int Animation::track_find_key(int p_track, double p_time, FindMode p_find_mode,
} break;
case TYPE_AUDIO: {
AudioTrack *at = static_cast<AudioTrack *>(t);
int k = _find(at->values, p_time, false, p_limit);
int k = _find(at->values, p_time, p_backward, p_limit);
if (k < 0 || k >= at->values.size()) {
return -1;
}
Expand All @@ -1659,7 +1659,7 @@ int Animation::track_find_key(int p_track, double p_time, FindMode p_find_mode,
} break;
case TYPE_ANIMATION: {
AnimationTrack *at = static_cast<AnimationTrack *>(t);
int k = _find(at->values, p_time, false, p_limit);
int k = _find(at->values, p_time, p_backward, p_limit);
if (k < 0 || k >= at->values.size()) {
return -1;
}
Expand Down Expand Up @@ -3836,7 +3836,7 @@ void Animation::_bind_methods() {
ClassDB::bind_method(D_METHOD("track_get_key_count", "track_idx"), &Animation::track_get_key_count);
ClassDB::bind_method(D_METHOD("track_get_key_value", "track_idx", "key_idx"), &Animation::track_get_key_value);
ClassDB::bind_method(D_METHOD("track_get_key_time", "track_idx", "key_idx"), &Animation::track_get_key_time);
ClassDB::bind_method(D_METHOD("track_find_key", "track_idx", "time", "find_mode", "limit"), &Animation::track_find_key, DEFVAL(FIND_MODE_NEAREST), DEFVAL(false));
ClassDB::bind_method(D_METHOD("track_find_key", "track_idx", "time", "find_mode", "limit", "backward"), &Animation::track_find_key, DEFVAL(FIND_MODE_NEAREST), DEFVAL(false), DEFVAL(false));

ClassDB::bind_method(D_METHOD("track_set_interpolation_type", "track_idx", "interpolation"), &Animation::track_set_interpolation_type);
ClassDB::bind_method(D_METHOD("track_get_interpolation_type", "track_idx"), &Animation::track_get_interpolation_type);
Expand Down
4 changes: 2 additions & 2 deletions scene/resources/animation.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ class Animation : public Resource {
Vector3 _scale_track_interpolate_bind_compat_86629(int p_track, double p_time) const;
float _blend_shape_track_interpolate_bind_compat_86629(int p_track, double p_time) const;
Variant _value_track_interpolate_bind_compat_86629(int p_track, double p_time) const;
int _track_find_key_bind_compat_86661(int p_track, double p_time, FindMode p_find_mode = FIND_MODE_NEAREST) const;
int _track_find_key_bind_compat_92861(int p_track, double p_time, FindMode p_find_mode = FIND_MODE_NEAREST) const;
static void _bind_compatibility_methods();
#endif // DISABLE_DEPRECATED

Expand Down Expand Up @@ -423,7 +423,7 @@ class Animation : public Resource {
void track_set_key_transition(int p_track, int p_key_idx, real_t p_transition);
void track_set_key_value(int p_track, int p_key_idx, const Variant &p_value);
void track_set_key_time(int p_track, int p_key_idx, double p_time);
int track_find_key(int p_track, double p_time, FindMode p_find_mode = FIND_MODE_NEAREST, bool p_limit = false) const;
int track_find_key(int p_track, double p_time, FindMode p_find_mode = FIND_MODE_NEAREST, bool p_limit = false, bool p_backward = false) const;
void track_remove_key(int p_track, int p_idx);
void track_remove_key_at_time(int p_track, double p_time);
int track_get_key_count(int p_track) const;
Expand Down
Loading