From 4a24ba6e77c7128e64dee31cf05f6960abdb8108 Mon Sep 17 00:00:00 2001 From: Juan Linietsky Date: Thu, 14 Feb 2019 10:19:03 -0300 Subject: [PATCH] -Fixes to undo redo to avoid crash, closes #24251 -Changed Animation to have a special signal when tracks are changed, to avoid unnecesary track cache rebuilds in AnimationPlayer -Added missing emit_changed whe modifying keys to Animation -Changed AnimationPlayer to use the new refcounted connections instead of the previous hacky way to keep references -Changed AnimationEditor to update the current track when keys are edited -Fixed bug where undo/redo did not work with AnimationKeyEdit (was not being updated) -Made sure UndoRedo does not mind deleted objects in undo/redo history, this would corrupt the history or clear it without need. --- core/undo_redo.cpp | 11 +++++----- editor/animation_track_editor.cpp | 33 +++++++++++++++++++++++----- editor/array_property_edit.cpp | 5 +++++ editor/array_property_edit.h | 2 ++ editor/dictionary_property_edit.cpp | 5 +++++ editor/dictionary_property_edit.h | 2 ++ editor/editor_inspector.cpp | 2 +- scene/animation/animation_player.cpp | 17 ++------------ scene/animation/animation_player.h | 2 -- scene/resources/animation.cpp | 15 +++++++++++++ scene/scene_string_names.cpp | 2 ++ scene/scene_string_names.h | 2 ++ 12 files changed, 68 insertions(+), 30 deletions(-) diff --git a/core/undo_redo.cpp b/core/undo_redo.cpp index 6bc882eaec8..00894b41d8f 100644 --- a/core/undo_redo.cpp +++ b/core/undo_redo.cpp @@ -234,7 +234,9 @@ void UndoRedo::_pop_history_tail() { } actions.remove(0); - current_action--; + if (current_action >= 0) { + current_action--; + } } void UndoRedo::commit_action() { @@ -258,11 +260,8 @@ void UndoRedo::_process_operation_list(List::Element *E) { Operation &op = E->get(); Object *obj = ObjectDB::get_instance(op.object); - if (!obj) { - //corruption - clear_history(); - ERR_FAIL_COND(!obj); - } + if (!obj) //may have been deleted and this is fine + continue; switch (op.type) { diff --git a/editor/animation_track_editor.cpp b/editor/animation_track_editor.cpp index 662137d60db..929d6c7642e 100644 --- a/editor/animation_track_editor.cpp +++ b/editor/animation_track_editor.cpp @@ -45,18 +45,22 @@ class AnimationTrackKeyEdit : public Object { public: bool setting; - bool hidden; bool _hide_script_from_inspector() { return true; } + bool _dont_undo_redo() { + return true; + } + static void _bind_methods() { ClassDB::bind_method("_update_obj", &AnimationTrackKeyEdit::_update_obj); ClassDB::bind_method("_key_ofs_changed", &AnimationTrackKeyEdit::_key_ofs_changed); ClassDB::bind_method("_hide_script_from_inspector", &AnimationTrackKeyEdit::_hide_script_from_inspector); ClassDB::bind_method("get_root_path", &AnimationTrackKeyEdit::get_root_path); + ClassDB::bind_method("_dont_undo_redo", &AnimationTrackKeyEdit::_dont_undo_redo); } //PopupDialog *ke_dialog; @@ -82,16 +86,13 @@ public: void _update_obj(const Ref &p_anim) { if (setting) return; - if (hidden) - return; if (!(animation == p_anim)) return; + notify_change(); } void _key_ofs_changed(const Ref &p_anim, float from, float to) { - if (hidden) - return; if (!(animation == p_anim)) return; if (from != key_ofs) @@ -168,6 +169,7 @@ public: undo_redo->add_do_method(this, "_update_obj", animation); undo_redo->add_undo_method(this, "_update_obj", animation); undo_redo->commit_action(); + setting = false; return true; @@ -191,6 +193,7 @@ public: undo_redo->add_do_method(this, "_update_obj", animation); undo_redo->add_undo_method(this, "_update_obj", animation); undo_redo->commit_action(); + setting = false; return true; } @@ -267,6 +270,7 @@ public: undo_redo->add_do_method(this, "_update_obj", animation); undo_redo->add_undo_method(this, "_update_obj", animation); undo_redo->commit_action(); + setting = false; if (change_notify_deserved) notify_change(); @@ -286,6 +290,7 @@ public: undo_redo->add_do_method(this, "_update_obj", animation); undo_redo->add_undo_method(this, "_update_obj", animation); undo_redo->commit_action(); + setting = false; return true; } @@ -301,6 +306,7 @@ public: undo_redo->add_do_method(this, "_update_obj", animation); undo_redo->add_undo_method(this, "_update_obj", animation); undo_redo->commit_action(); + setting = false; return true; } @@ -316,6 +322,7 @@ public: undo_redo->add_do_method(this, "_update_obj", animation); undo_redo->add_undo_method(this, "_update_obj", animation); undo_redo->commit_action(); + setting = false; return true; } @@ -335,6 +342,7 @@ public: undo_redo->add_do_method(this, "_update_obj", animation); undo_redo->add_undo_method(this, "_update_obj", animation); undo_redo->commit_action(); + setting = false; return true; } @@ -350,6 +358,7 @@ public: undo_redo->add_do_method(this, "_update_obj", animation); undo_redo->add_undo_method(this, "_update_obj", animation); undo_redo->commit_action(); + setting = false; return true; } @@ -365,6 +374,7 @@ public: undo_redo->add_do_method(this, "_update_obj", animation); undo_redo->add_undo_method(this, "_update_obj", animation); undo_redo->commit_action(); + setting = false; return true; } @@ -639,6 +649,8 @@ public: PropertyInfo hint; NodePath base; + AnimationTrackEditor *track_editor; + void notify_change() { _change_notify(); @@ -649,11 +661,11 @@ public: } AnimationTrackKeyEdit() { - hidden = true; key_ofs = 0; track = -1; setting = false; root_path = NULL; + track_editor = NULL; } }; @@ -3414,6 +3426,14 @@ void AnimationTrackEditor::_update_tracks() { void AnimationTrackEditor::_animation_changed() { + if (key_edit && key_edit->setting) { + //if editing a key, just update the edited track, makes refresh less costly + if (key_edit->track < track_edits.size()) { + track_edits[key_edit->track]->update(); + } + return; + } + timeline->update(); timeline->update_values(); if (block_animation_update) { @@ -3944,6 +3964,7 @@ void AnimationTrackEditor::_update_key_edit() { key_edit = memnew(AnimationTrackKeyEdit); key_edit->animation = animation; key_edit->track = selection.front()->key().track; + key_edit->track_editor = this; float ofs = animation->track_get_key_time(key_edit->track, selection.front()->key().key); key_edit->key_ofs = ofs; diff --git a/editor/array_property_edit.cpp b/editor/array_property_edit.cpp index 7a3edfde50f..72beeaaf459 100644 --- a/editor/array_property_edit.cpp +++ b/editor/array_property_edit.cpp @@ -288,12 +288,17 @@ Node *ArrayPropertyEdit::get_node() { return Object::cast_to(ObjectDB::get_instance(obj)); } +bool ArrayPropertyEdit::_dont_undo_redo() { + return true; +} + void ArrayPropertyEdit::_bind_methods() { ClassDB::bind_method(D_METHOD("_set_size"), &ArrayPropertyEdit::_set_size); ClassDB::bind_method(D_METHOD("_set_value"), &ArrayPropertyEdit::_set_value); ClassDB::bind_method(D_METHOD("_notif_change"), &ArrayPropertyEdit::_notif_change); ClassDB::bind_method(D_METHOD("_notif_changev"), &ArrayPropertyEdit::_notif_changev); + ClassDB::bind_method(D_METHOD("_dont_undo_redo"), &ArrayPropertyEdit::_dont_undo_redo); } ArrayPropertyEdit::ArrayPropertyEdit() { diff --git a/editor/array_property_edit.h b/editor/array_property_edit.h index b2a3564c8cb..fd17ebe9f83 100644 --- a/editor/array_property_edit.h +++ b/editor/array_property_edit.h @@ -52,6 +52,8 @@ class ArrayPropertyEdit : public Reference { void _set_size(int p_size); void _set_value(int p_idx, const Variant &p_value); + bool _dont_undo_redo(); + protected: static void _bind_methods(); bool _set(const StringName &p_name, const Variant &p_value); diff --git a/editor/dictionary_property_edit.cpp b/editor/dictionary_property_edit.cpp index 483f5ae2d0c..9f4096ca029 100644 --- a/editor/dictionary_property_edit.cpp +++ b/editor/dictionary_property_edit.cpp @@ -102,12 +102,17 @@ Node *DictionaryPropertyEdit::get_node() { return cast_to(o); } +bool DictionaryPropertyEdit::_dont_undo_redo() { + return true; +} + void DictionaryPropertyEdit::_bind_methods() { ClassDB::bind_method(D_METHOD("_set_key"), &DictionaryPropertyEdit::_set_key); ClassDB::bind_method(D_METHOD("_set_value"), &DictionaryPropertyEdit::_set_value); ClassDB::bind_method(D_METHOD("_notif_change"), &DictionaryPropertyEdit::_notif_change); ClassDB::bind_method(D_METHOD("_notif_changev"), &DictionaryPropertyEdit::_notif_changev); + ClassDB::bind_method(D_METHOD("_dont_undo_redo"), &DictionaryPropertyEdit::_dont_undo_redo); } bool DictionaryPropertyEdit::_set(const StringName &p_name, const Variant &p_value) { diff --git a/editor/dictionary_property_edit.h b/editor/dictionary_property_edit.h index f6bd13785a4..a2a81c261c8 100644 --- a/editor/dictionary_property_edit.h +++ b/editor/dictionary_property_edit.h @@ -46,6 +46,8 @@ class DictionaryPropertyEdit : public Reference { Variant get_dictionary() const; + bool _dont_undo_redo(); + protected: static void _bind_methods(); bool _set(const StringName &p_name, const Variant &p_value); diff --git a/editor/editor_inspector.cpp b/editor/editor_inspector.cpp index 1078fabc2e6..94761cadefe 100644 --- a/editor/editor_inspector.cpp +++ b/editor/editor_inspector.cpp @@ -1937,7 +1937,7 @@ void EditorInspector::_edit_set(const String &p_name, const Variant &p_value, bo } } - if (!undo_redo || Object::cast_to(object) || Object::cast_to(object)) { //kind of hacky + if (!undo_redo || bool(object->call("_dont_undo_redo"))) { object->set(p_name, p_value); if (p_refresh_all) diff --git a/scene/animation/animation_player.cpp b/scene/animation/animation_player.cpp index 54be2aa1598..86fd1ef14c0 100644 --- a/scene/animation/animation_player.cpp +++ b/scene/animation/animation_player.cpp @@ -991,25 +991,12 @@ void AnimationPlayer::remove_animation(const StringName &p_name) { void AnimationPlayer::_ref_anim(const Ref &p_anim) { - if (used_anims.has(p_anim)) - used_anims[p_anim]++; - else { - used_anims[p_anim] = 1; - Ref(p_anim)->connect("changed", this, "_animation_changed"); - } + Ref(p_anim)->connect(SceneStringNames::get_singleton()->tracks_changed, this, "_animation_changed", varray(), CONNECT_REFERENCE_COUNTED); } void AnimationPlayer::_unref_anim(const Ref &p_anim) { - ERR_FAIL_COND(!used_anims.has(p_anim)); - - int &n = used_anims[p_anim]; - n--; - if (n == 0) { - - Ref(p_anim)->disconnect("changed", this, "_animation_changed"); - used_anims.erase(p_anim); - } + Ref(p_anim)->disconnect(SceneStringNames::get_singleton()->tracks_changed, this, "_animation_changed"); } void AnimationPlayer::rename_animation(const StringName &p_name, const StringName &p_new_name) { diff --git a/scene/animation/animation_player.h b/scene/animation/animation_player.h index 9bba848298f..fea4819821f 100644 --- a/scene/animation/animation_player.h +++ b/scene/animation/animation_player.h @@ -181,8 +181,6 @@ private: int cache_update_bezier_size; Set playing_caches; - Map, int> used_anims; - uint64_t accum_pass; float speed_scale; float default_blend_time; diff --git a/scene/resources/animation.cpp b/scene/resources/animation.cpp index 3156e12c505..a7544cc0314 100644 --- a/scene/resources/animation.cpp +++ b/scene/resources/animation.cpp @@ -29,6 +29,7 @@ /*************************************************************************/ #include "animation.h" +#include "scene/scene_string_names.h" #include "core/math/geometry.h" @@ -668,6 +669,7 @@ int Animation::add_track(TrackType p_type, int p_at_pos) { } } emit_changed(); + emit_signal(SceneStringNames::get_singleton()->tracks_changed); return p_at_pos; } @@ -719,6 +721,7 @@ void Animation::remove_track(int p_track) { memdelete(t); tracks.remove(p_track); emit_changed(); + emit_signal(SceneStringNames::get_singleton()->tracks_changed); } int Animation::get_track_count() const { @@ -737,6 +740,7 @@ void Animation::track_set_path(int p_track, const NodePath &p_path) { ERR_FAIL_INDEX(p_track, tracks.size()); tracks[p_track]->path = p_path; emit_changed(); + emit_signal(SceneStringNames::get_singleton()->tracks_changed); } NodePath Animation::track_get_path(int p_track) const { @@ -1410,6 +1414,8 @@ void Animation::track_set_key_value(int p_track, int p_key_idx, const Variant &p } break; } + + emit_changed(); } void Animation::track_set_key_transition(int p_track, int p_key_idx, float p_transition) { @@ -1445,6 +1451,8 @@ void Animation::track_set_key_transition(int p_track, int p_key_idx, float p_tra // they don't use transition } break; } + + emit_changed(); } template @@ -2554,6 +2562,7 @@ void Animation::track_move_up(int p_track) { } emit_changed(); + emit_signal(SceneStringNames::get_singleton()->tracks_changed); } void Animation::track_set_imported(int p_track, bool p_imported) { @@ -2588,6 +2597,7 @@ void Animation::track_move_down(int p_track) { SWAP(tracks.write[p_track], tracks.write[p_track - 1]); } emit_changed(); + emit_signal(SceneStringNames::get_singleton()->tracks_changed); } void Animation::track_swap(int p_track, int p_with_track) { @@ -2598,6 +2608,7 @@ void Animation::track_swap(int p_track, int p_with_track) { return; SWAP(tracks.write[p_track], tracks.write[p_with_track]); emit_changed(); + emit_signal(SceneStringNames::get_singleton()->tracks_changed); } void Animation::set_step(float p_step) { @@ -2716,6 +2727,8 @@ void Animation::_bind_methods() { ADD_PROPERTY(PropertyInfo(Variant::BOOL, "loop"), "set_loop", "has_loop"); ADD_PROPERTY(PropertyInfo(Variant::REAL, "step", PROPERTY_HINT_RANGE, "0,4096,0.001"), "set_step", "get_step"); + ADD_SIGNAL(MethodInfo("tracks_changed")); + BIND_ENUM_CONSTANT(TYPE_VALUE); BIND_ENUM_CONSTANT(TYPE_TRANSFORM); BIND_ENUM_CONSTANT(TYPE_METHOD); @@ -2740,6 +2753,8 @@ void Animation::clear() { tracks.clear(); loop = false; length = 1; + emit_changed(); + emit_signal(SceneStringNames::get_singleton()->tracks_changed); } bool Animation::_transform_track_optimize_key(const TKey &t0, const TKey &t1, const TKey &t2, float p_alowed_linear_err, float p_alowed_angular_err, float p_max_optimizable_angle, const Vector3 &p_norm) { diff --git a/scene/scene_string_names.cpp b/scene/scene_string_names.cpp index 305e4fc735f..634f4d7160e 100644 --- a/scene/scene_string_names.cpp +++ b/scene/scene_string_names.cpp @@ -203,4 +203,6 @@ SceneStringNames::SceneStringNames() { _mesh_changed = StaticCString::create("_mesh_changed"); parameters_base_path = "parameters/"; + + tracks_changed = "tracks_changed"; } diff --git a/scene/scene_string_names.h b/scene/scene_string_names.h index 4d1a7f255c7..bc75165b8c4 100644 --- a/scene/scene_string_names.h +++ b/scene/scene_string_names.h @@ -205,6 +205,8 @@ public: StringName parameters_base_path; + StringName tracks_changed; + enum { MAX_MATERIALS = 32 };