Fix errors and improve UX relating to new animation libraries

- Fix a bug causing an error message when a scene containing an AnimationPlayer with a reset track is saved, by correctly referencing the temporary "default" library.
- Make library dropdown in new animation window assign correct library when creating an animation.
- Similarly allow choice of library when duplicating animation.
- Make library dropdown default to library of currently selected animation.
- Make library dropdown show when exactly one library exists, and it isn't [Global]. Include [Global] on the dropdown in this case (will be newly created if dialog is confirmed).
- When appending (x) to avoid New Anim name collisions, correctly check target library instead of [Global].
- Add parentheses when appending x when duplicating animations in the library editor, for consistency.
- Change titles and prompts to be distinct in name/rename/duplicate dialiogs.
- Fix bug in OprionButton.get_selectable_item(true) when last is not selectable.
- Fix issues where animation wasn't found on deletion/rename by correctly prepending library name.
- Remove an extraneous print_line from animation_track_editor.
- Add messages to errors when an animation isn't found.
This commit is contained in:
SnailRhymer 2022-05-14 17:54:14 +01:00
parent 067c1eb923
commit d79818acb6
7 changed files with 161 additions and 51 deletions

View file

@ -2999,8 +2999,6 @@ void AnimationTrackEdit::gui_input(const Ref<InputEvent> &p_event) {
}
}
print_line(hovering_key_idx);
if (hovering_key_idx != previous_hovering_key_idx) {
// Required to draw keyframe hover feedback on the correct keyframe.
update();

View file

@ -75,12 +75,16 @@ void AnimationLibraryEditor::_add_library_validate(const String &p_name) {
add_library_validate->set_text(error);
add_library_dialog->get_ok_button()->set_disabled(true);
} else {
add_library_validate->add_theme_color_override("font_color", get_theme_color(SNAME("success_color"), SNAME("Editor")));
if (adding_animation) {
add_library_validate->set_text(TTR("Animation name is valid."));
} else {
if (p_name == "") {
add_library_validate->set_text(TTR("Global library will be created."));
} else {
add_library_validate->set_text(TTR("Library name is valid."));
}
}
add_library_validate->add_theme_color_override("font_color", get_theme_color(SNAME("success_color"), SNAME("Editor")));
add_library_dialog->get_ok_button()->set_disabled(false);
}
}
@ -469,7 +473,7 @@ void AnimationLibraryEditor::_button_pressed(TreeItem *p_item, int p_column, int
int attempt = 1;
while (al->has_animation(name)) {
attempt++;
name = base_name + " " + itos(attempt);
name = base_name + " (" + itos(attempt) + ")";
}
UndoRedo *undo_redo = EditorNode::get_singleton()->get_undo_redo();

View file

@ -303,17 +303,23 @@ void AnimationPlayerEditor::_animation_selected(int p_which) {
}
void AnimationPlayerEditor::_animation_new() {
name_dialog_op = TOOL_NEW_ANIM;
name_title->set_text(TTR("New Animation Name:"));
int count = 1;
String base = TTR("New Anim");
String current_library_name = "";
if (animation->has_selectable_items()) {
String current_animation_name = animation->get_item_text(animation->get_selected());
Ref<Animation> current_animation = player->get_animation(current_animation_name);
if (current_animation.is_valid()) {
current_library_name = player->find_animation_library(current_animation);
}
}
String attempt_prefix = (current_library_name == "") ? "" : current_library_name + "/";
while (true) {
String attempt = base;
if (count > 1) {
attempt += " (" + itos(count) + ")";
}
if (player->has_animation(attempt)) {
if (player->has_animation(attempt_prefix + attempt)) {
count++;
continue;
}
@ -321,22 +327,13 @@ void AnimationPlayerEditor::_animation_new() {
break;
}
List<StringName> libraries;
player->get_animation_library_list(&libraries);
library->clear();
for (const StringName &K : libraries) {
library->add_item((K == StringName()) ? String(TTR("[Global]")) : String(K));
library->set_item_metadata(0, String(K));
}
_update_name_dialog_library_dropdown();
if (libraries.size() > 1) {
library->show();
} else {
library->hide();
}
name->set_text(base);
name_dialog_op = TOOL_NEW_ANIM;
name_dialog->set_title(TTR("Create New Animation"));
name_dialog->popup_centered(Size2(300, 90));
name_title->set_text(TTR("New Animation Name:"));
name->set_text(base);
name->select_all();
name->grab_focus();
}
@ -348,6 +345,12 @@ void AnimationPlayerEditor::_animation_rename() {
int selected = animation->get_selected();
String selected_name = animation->get_item_text(selected);
// Remove library prefix if present.
if (selected_name.contains("/")) {
selected_name = selected_name.get_slice("/", 1);
}
name_dialog->set_title(TTR("Rename Animation"));
name_title->set_text(TTR("Change Animation Name:"));
name->set_text(selected_name);
name_dialog_op = TOOL_RENAME_ANIM;
@ -375,6 +378,10 @@ void AnimationPlayerEditor::_animation_remove_confirmed() {
Ref<AnimationLibrary> al = player->get_animation_library(player->find_animation_library(anim));
ERR_FAIL_COND(al.is_null());
// For names of form lib_name/anim_name, remove library name prefix.
if (current.contains("/")) {
current = current.get_slice("/", 1);
}
undo_redo->create_action(TTR("Remove Animation"));
if (player->get_autoplay() == current) {
undo_redo->add_do_method(player, "set_autoplay", "");
@ -438,8 +445,14 @@ void AnimationPlayerEditor::_animation_name_edited() {
return;
}
if (player->has_animation(new_name)) {
error_dialog->set_text(TTR("Animation name already exists!"));
String test_name_prefix = "";
if (library->is_visible() && library->get_selected_id() != -1) {
test_name_prefix = library->get_item_metadata(library->get_selected_id());
test_name_prefix += (test_name_prefix != "") ? "/" : "";
}
if (player->has_animation(test_name_prefix + new_name)) {
error_dialog->set_text(vformat(TTR("Animation '%s' already exists!")));
error_dialog->popup_centered();
return;
}
@ -452,6 +465,13 @@ void AnimationPlayerEditor::_animation_name_edited() {
Ref<AnimationLibrary> al = player->get_animation_library(player->find_animation_library(anim));
ERR_FAIL_COND(al.is_null());
// Extract library prefix if present.
String new_library_prefix = "";
if (current.contains("/")) {
new_library_prefix = current.get_slice("/", 0) + "/";
current = current.get_slice("/", 1);
}
undo_redo->create_action(TTR("Rename Animation"));
undo_redo->add_do_method(al.ptr(), "rename_animation", current, new_name);
undo_redo->add_do_method(anim.ptr(), "set_name", new_name);
@ -461,19 +481,25 @@ void AnimationPlayerEditor::_animation_name_edited() {
undo_redo->add_undo_method(this, "_animation_player_changed", player);
undo_redo->commit_action();
_select_anim_by_name(new_name);
_select_anim_by_name(new_library_prefix + new_name);
} break;
case TOOL_NEW_ANIM: {
Ref<Animation> new_anim = Ref<Animation>(memnew(Animation));
new_anim->set_name(new_name);
String library_name;
Ref<AnimationLibrary> al;
if (library->is_visible()) {
al = player->get_animation_library(library->get_item_metadata(library->get_selected()));
library_name = library->get_item_metadata(library->get_selected());
// It's possible that [Global] was selected, but doesn't exist yet.
if (player->has_animation_library(library_name)) {
al = player->get_animation_library(library_name);
}
} else {
if (player->has_animation_library("")) {
al = player->get_animation_library("");
library_name = "";
}
}
@ -484,6 +510,7 @@ void AnimationPlayerEditor::_animation_name_edited() {
al.instantiate();
lib_added = true;
undo_redo->add_do_method(player, "add_animation_library", "", al);
library_name = "";
}
undo_redo->add_do_method(al.ptr(), "add_animation", new_name, new_anim);
@ -499,7 +526,11 @@ void AnimationPlayerEditor::_animation_name_edited() {
}
undo_redo->commit_action();
_select_anim_by_name(new_name);
if (library_name != "") {
library_name = library_name + "/";
}
_select_anim_by_name(library_name + new_name);
} break;
case TOOL_DUPLICATE_ANIM: {
@ -509,17 +540,44 @@ void AnimationPlayerEditor::_animation_name_edited() {
Ref<Animation> new_anim = _animation_clone(anim);
new_anim->set_name(new_name);
Ref<AnimationLibrary> library = player->get_animation_library(player->find_animation_library(anim));
String library_name;
Ref<AnimationLibrary> al;
if (library->is_visible()) {
library_name = library->get_item_metadata(library->get_selected());
// It's possible that [Global] was selected, but doesn't exist yet.
if (player->has_animation_library(library_name)) {
al = player->get_animation_library(library_name);
}
} else {
if (player->has_animation_library("")) {
al = player->get_animation_library("");
library_name = "";
}
}
undo_redo->create_action(TTR("Duplicate Animation"));
undo_redo->add_do_method(library.ptr(), "add_animation", new_name, new_anim);
undo_redo->add_undo_method(library.ptr(), "remove_animation", new_name);
undo_redo->add_do_method(player, "animation_set_next", new_name, player->animation_get_next(current));
bool lib_added = false;
if (al.is_null()) {
al.instantiate();
lib_added = true;
undo_redo->add_do_method(player, "add_animation_library", "", al);
library_name = "";
}
undo_redo->add_do_method(al.ptr(), "add_animation", new_name, new_anim);
undo_redo->add_undo_method(al.ptr(), "remove_animation", new_name);
undo_redo->add_do_method(this, "_animation_player_changed", player);
undo_redo->add_undo_method(this, "_animation_player_changed", player);
if (lib_added) {
undo_redo->add_undo_method(player, "remove_animation_library", "");
}
undo_redo->commit_action();
_select_anim_by_name(new_name);
if (library_name != "") {
library_name = library_name + "/";
}
_select_anim_by_name(library_name + new_name);
} break;
}
@ -851,6 +909,47 @@ void AnimationPlayerEditor::_update_animation_list_icons() {
}
}
void AnimationPlayerEditor::_update_name_dialog_library_dropdown() {
StringName current_library_name = StringName();
if (animation->has_selectable_items()) {
String current_animation_name = animation->get_item_text(animation->get_selected());
Ref<Animation> current_animation = player->get_animation(current_animation_name);
if (current_animation.is_valid()) {
current_library_name = player->find_animation_library(current_animation);
}
}
List<StringName> libraries;
player->get_animation_library_list(&libraries);
library->clear();
// When [Global] isn't present, but other libraries are, add option of creating [Global].
int index_offset = 0;
if (!player->has_animation_library(StringName())) {
library->add_item(String(TTR("[Global] (create)")));
library->set_item_metadata(0, "");
index_offset = 1;
}
int current_lib_id = index_offset; // Don't default to [Global] if it doesn't exist yet.
for (int i = 0; i < libraries.size(); i++) {
StringName library_name = libraries[i];
library->add_item((library_name == StringName()) ? String(TTR("[Global]")) : String(library_name));
library->set_item_metadata(i + index_offset, String(library_name));
// Default to duplicating into same library.
if (library_name == current_library_name) {
current_lib_id = i + index_offset;
}
}
if (library->get_item_count() > 1) {
library->select(current_lib_id);
library->show();
} else {
library->hide();
}
}
void AnimationPlayerEditor::edit(AnimationPlayer *p_player) {
if (player && pin->is_pressed()) {
return; // Ignore, pinned.
@ -946,9 +1045,17 @@ void AnimationPlayerEditor::_animation_duplicate() {
new_name = new_name + " (copy)";
}
name_title->set_text(TTR("New Animation Name:"));
name->set_text(new_name);
if (new_name.contains("/")) {
// Discard library prefix.
new_name = new_name.get_slice("/", 1);
}
_update_name_dialog_library_dropdown();
name_dialog_op = TOOL_DUPLICATE_ANIM;
name_dialog->set_title("Duplicate Animation");
name_title->set_text(TTR("Duplicated Animation Name:"));
name->set_text(new_name);
name_dialog->popup_centered(Size2(300, 90));
name->select_all();
name->grab_focus();

View file

@ -188,6 +188,7 @@ class AnimationPlayerEditor : public VBoxContainer {
void _update_animation();
void _update_player();
void _update_animation_list_icons();
void _update_name_dialog_library_dropdown();
void _blend_edited();
void _animation_player_changed(Object *p_pl);

View file

@ -1487,7 +1487,7 @@ bool AnimationPlayer::has_animation(const StringName &p_name) const {
}
Ref<Animation> AnimationPlayer::get_animation(const StringName &p_name) const {
ERR_FAIL_COND_V(!animation_set.has(p_name), Ref<Animation>());
ERR_FAIL_COND_V_MSG(!animation_set.has(p_name), Ref<Animation>(), vformat("Animation not found: %s.", p_name));
const AnimationData &data = animation_set[p_name];
@ -1509,8 +1509,8 @@ void AnimationPlayer::get_animation_list(List<StringName> *p_animations) const {
}
void AnimationPlayer::set_blend_time(const StringName &p_animation1, const StringName &p_animation2, float p_time) {
ERR_FAIL_COND(!animation_set.has(p_animation1));
ERR_FAIL_COND(!animation_set.has(p_animation2));
ERR_FAIL_COND_MSG(!animation_set.has(p_animation1), vformat("Animation not found: %s.", p_animation1));
ERR_FAIL_COND_MSG(!animation_set.has(p_animation2), vformat("Animation not found: %s.", p_animation2));
ERR_FAIL_COND_MSG(p_time < 0, "Blend time cannot be smaller than 0.");
BlendKey bk;
@ -1567,7 +1567,7 @@ void AnimationPlayer::play(const StringName &p_name, float p_custom_blend, float
name = playback.assigned;
}
ERR_FAIL_COND_MSG(!animation_set.has(name), "Animation not found: " + name + ".");
ERR_FAIL_COND_MSG(!animation_set.has(name), vformat("Animation not found: %s.", name));
Playback &c = playback;
@ -1670,7 +1670,7 @@ void AnimationPlayer::set_assigned_animation(const String &p_anim) {
if (is_playing()) {
play(p_anim);
} else {
ERR_FAIL_COND(!animation_set.has(p_anim));
ERR_FAIL_COND_MSG(!animation_set.has(p_anim), vformat("Animation not found: %s.", p_anim));
playback.current.pos = 0;
playback.current.from = &animation_set[p_anim];
playback.assigned = p_anim;
@ -1713,7 +1713,7 @@ float AnimationPlayer::get_playing_speed() const {
void AnimationPlayer::seek(double p_time, bool p_update) {
if (!playback.current.from) {
if (playback.assigned) {
ERR_FAIL_COND(!animation_set.has(playback.assigned));
ERR_FAIL_COND_MSG(!animation_set.has(playback.assigned), vformat("Animation not found: %s.", playback.assigned));
playback.current.from = &animation_set[playback.assigned];
}
ERR_FAIL_COND(!playback.current.from);
@ -1729,7 +1729,7 @@ void AnimationPlayer::seek(double p_time, bool p_update) {
void AnimationPlayer::seek_delta(double p_time, float p_delta) {
if (!playback.current.from) {
if (playback.assigned) {
ERR_FAIL_COND(!animation_set.has(playback.assigned));
ERR_FAIL_COND_MSG(!animation_set.has(playback.assigned), vformat("Animation not found: %s.", playback.assigned));
playback.current.from = &animation_set[playback.assigned];
}
ERR_FAIL_COND(!playback.current.from);
@ -1899,7 +1899,7 @@ void AnimationPlayer::_set_process(bool p_process, bool p_force) {
}
void AnimationPlayer::animation_set_next(const StringName &p_animation, const StringName &p_next) {
ERR_FAIL_COND(!animation_set.has(p_animation));
ERR_FAIL_COND_MSG(!animation_set.has(p_animation), vformat("Animation not found: %s.", p_animation));
animation_set[p_animation].next = p_next;
}
@ -2012,7 +2012,7 @@ Ref<AnimatedValuesBackup> AnimationPlayer::apply_reset(bool p_user_initiated) {
al.instantiate();
al->add_animation(SceneStringNames::get_singleton()->RESET, reset_anim);
aux_player->add_animation_library("default", al);
aux_player->set_assigned_animation(SceneStringNames::get_singleton()->RESET);
aux_player->set_assigned_animation("default/" + SceneStringNames::get_singleton()->RESET);
// Forcing the use of the original root because the scene where original player belongs may be not the active one
Node *root = get_node(get_root());
Ref<AnimatedValuesBackup> old_values = aux_player->backup_animated_values(root);

View file

@ -320,7 +320,7 @@ int OptionButton::get_selectable_item(bool p_from_last) const {
}
}
} else {
for (int i = get_item_count() - 1; i >= 0; i++) {
for (int i = get_item_count() - 1; i >= 0; i--) {
if (!is_item_disabled(i) && !is_item_separator(i)) {
return i;
}

View file

@ -63,7 +63,7 @@ Error AnimationLibrary::add_animation(const StringName &p_name, const Ref<Animat
}
void AnimationLibrary::remove_animation(const StringName &p_name) {
ERR_FAIL_COND(!animations.has(p_name));
ERR_FAIL_COND_MSG(!animations.has(p_name), vformat("Animation not found: %s.", p_name));
animations.erase(p_name);
emit_signal(SNAME("animation_removed"), p_name);
@ -71,9 +71,9 @@ void AnimationLibrary::remove_animation(const StringName &p_name) {
}
void AnimationLibrary::rename_animation(const StringName &p_name, const StringName &p_new_name) {
ERR_FAIL_COND(!animations.has(p_name));
ERR_FAIL_COND_MSG(!animations.has(p_name), vformat("Animation not found: %s.", p_name));
ERR_FAIL_COND_MSG(!is_valid_animation_name(p_new_name), "Invalid animation name: '" + String(p_new_name) + "'.");
ERR_FAIL_COND(animations.has(p_new_name));
ERR_FAIL_COND_MSG(animations.has(p_new_name), vformat("Animation name \"%s\" already exists in library.", p_new_name));
animations.insert(p_new_name, animations[p_name]);
animations.erase(p_name);