From f249667dc8d490d25624122d8a05e229af61ef62 Mon Sep 17 00:00:00 2001 From: Sofox Date: Sun, 26 Nov 2023 17:13:35 +0000 Subject: [PATCH] Fixed MERGE_ALL commit from repeating actions --- core/object/undo_redo.cpp | 16 ++- core/object/undo_redo.h | 1 + tests/core/object/test_undo_redo.h | 202 +++++++++++++++++++++++++++++ tests/test_main.cpp | 1 + 4 files changed, 219 insertions(+), 1 deletion(-) create mode 100644 tests/core/object/test_undo_redo.h diff --git a/core/object/undo_redo.cpp b/core/object/undo_redo.cpp index 569e6ae19f6..6a1385e2682 100644 --- a/core/object/undo_redo.cpp +++ b/core/object/undo_redo.cpp @@ -71,7 +71,14 @@ bool UndoRedo::_redo(bool p_execute) { } current_action++; - _process_operation_list(actions.write[current_action].do_ops.front(), p_execute); + + List::Element *start_doops_element = actions.write[current_action].do_ops.front(); + while (merge_total > 0 && start_doops_element) { + start_doops_element = start_doops_element->next(); + merge_total--; + } + + _process_operation_list(start_doops_element, p_execute); version++; emit_signal(SNAME("version_changed")); @@ -104,6 +111,12 @@ void UndoRedo::create_action(const String &p_name, MergeMode p_mode, bool p_back } } + if (p_mode == MERGE_ALL) { + merge_total = actions.write[current_action + 1].do_ops.size(); + } else { + merge_total = 0; + } + actions.write[actions.size() - 1].last_tick = ticks; // Revert reverse from previous commit. @@ -121,6 +134,7 @@ void UndoRedo::create_action(const String &p_name, MergeMode p_mode, bool p_back actions.push_back(new_action); merge_mode = MERGE_DISABLE; + merge_total = 0; } } diff --git a/core/object/undo_redo.h b/core/object/undo_redo.h index 62aebff8775..19d178635c5 100644 --- a/core/object/undo_redo.h +++ b/core/object/undo_redo.h @@ -84,6 +84,7 @@ private: MergeMode merge_mode = MERGE_DISABLE; bool merging = false; uint64_t version = 1; + int merge_total = 0; void _pop_history_tail(); void _process_operation_list(List::Element *E, bool p_execute); diff --git a/tests/core/object/test_undo_redo.h b/tests/core/object/test_undo_redo.h new file mode 100644 index 00000000000..ad3554b58c8 --- /dev/null +++ b/tests/core/object/test_undo_redo.h @@ -0,0 +1,202 @@ +/**************************************************************************/ +/* test_undo_redo.h */ +/**************************************************************************/ +/* This file is part of: */ +/* GODOT ENGINE */ +/* https://godotengine.org */ +/**************************************************************************/ +/* Copyright (c) 2014-present Godot Engine contributors (see AUTHORS.md). */ +/* Copyright (c) 2007-2014 Juan Linietsky, Ariel Manzur. */ +/* */ +/* Permission is hereby granted, free of charge, to any person obtaining */ +/* a copy of this software and associated documentation files (the */ +/* "Software"), to deal in the Software without restriction, including */ +/* without limitation the rights to use, copy, modify, merge, publish, */ +/* distribute, sublicense, and/or sell copies of the Software, and to */ +/* permit persons to whom the Software is furnished to do so, subject to */ +/* the following conditions: */ +/* */ +/* The above copyright notice and this permission notice shall be */ +/* included in all copies or substantial portions of the Software. */ +/* */ +/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, */ +/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF */ +/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. */ +/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY */ +/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, */ +/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE */ +/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ +/**************************************************************************/ + +#ifndef TEST_UNDO_REDO_H +#define TEST_UNDO_REDO_H + +#include "core/object/undo_redo.h" +#include "tests/test_macros.h" + +// Declared in global namespace because of GDCLASS macro warning (Windows): +// "Unqualified friend declaration referring to type outside of the nearest enclosing namespace +// is a Microsoft extension; add a nested name specifier". +class _TestUndoRedoObject : public Object { + GDCLASS(_TestUndoRedoObject, Object); + int property_value = 0; + +protected: + static void _bind_methods() { + ClassDB::bind_method(D_METHOD("set_property", "property"), &_TestUndoRedoObject::set_property); + ClassDB::bind_method(D_METHOD("get_property"), &_TestUndoRedoObject::get_property); + ADD_PROPERTY(PropertyInfo(Variant::INT, "property"), "set_property", "get_property"); + } + +public: + void set_property(int value) { property_value = value; } + int get_property() const { return property_value; } + void add_to_property(int value) { property_value += value; } + void subtract_from_property(int value) { property_value -= value; } +}; + +namespace TestUndoRedo { + +void set_property_action(UndoRedo *undo_redo, const String &name, _TestUndoRedoObject *test_object, int value, UndoRedo::MergeMode merge_mode = UndoRedo::MERGE_DISABLE) { + undo_redo->create_action(name, merge_mode); + undo_redo->add_do_property(test_object, "property", value); + undo_redo->add_undo_property(test_object, "property", test_object->get_property()); + undo_redo->commit_action(); +} + +void increment_property_action(UndoRedo *undo_redo, const String &name, _TestUndoRedoObject *test_object, int value, UndoRedo::MergeMode merge_mode = UndoRedo::MERGE_DISABLE) { + undo_redo->create_action(name, merge_mode); + undo_redo->add_do_method(callable_mp(test_object, &_TestUndoRedoObject::add_to_property).bind(value)); + undo_redo->add_undo_method(callable_mp(test_object, &_TestUndoRedoObject::subtract_from_property).bind(value)); + undo_redo->commit_action(); +} + +TEST_CASE("[UndoRedo] Simple Property UndoRedo") { + GDREGISTER_CLASS(_TestUndoRedoObject); + UndoRedo *undo_redo = memnew(UndoRedo()); + + _TestUndoRedoObject *test_object = memnew(_TestUndoRedoObject()); + + CHECK(test_object->get_property() == 0); + CHECK(undo_redo->get_version() == 1); + CHECK(undo_redo->get_history_count() == 0); + + set_property_action(undo_redo, "Set Property", test_object, 10); + + CHECK(test_object->get_property() == 10); + CHECK(undo_redo->get_version() == 2); + CHECK(undo_redo->get_history_count() == 1); + + undo_redo->undo(); + + CHECK(test_object->get_property() == 0); + CHECK(undo_redo->get_version() == 1); + CHECK(undo_redo->get_history_count() == 1); + + undo_redo->redo(); + + CHECK(test_object->get_property() == 10); + CHECK(undo_redo->get_version() == 2); + CHECK(undo_redo->get_history_count() == 1); + + set_property_action(undo_redo, "Set Property", test_object, 100); + + CHECK(test_object->get_property() == 100); + CHECK(undo_redo->get_version() == 3); + CHECK(undo_redo->get_history_count() == 2); + + set_property_action(undo_redo, "Set Property", test_object, 1000); + + CHECK(test_object->get_property() == 1000); + CHECK(undo_redo->get_version() == 4); + CHECK(undo_redo->get_history_count() == 3); + + undo_redo->undo(); + + CHECK(test_object->get_property() == 100); + CHECK(undo_redo->get_version() == 3); + CHECK(undo_redo->get_history_count() == 3); + + memdelete(test_object); + memdelete(undo_redo); +} + +TEST_CASE("[UndoRedo] Merge Property UndoRedo") { + GDREGISTER_CLASS(_TestUndoRedoObject); + UndoRedo *undo_redo = memnew(UndoRedo()); + + _TestUndoRedoObject *test_object = memnew(_TestUndoRedoObject()); + + CHECK(test_object->get_property() == 0); + CHECK(undo_redo->get_version() == 1); + CHECK(undo_redo->get_history_count() == 0); + + set_property_action(undo_redo, "Merge Action 1", test_object, 10, UndoRedo::MERGE_ALL); + + CHECK(test_object->get_property() == 10); + CHECK(undo_redo->get_version() == 2); + CHECK(undo_redo->get_history_count() == 1); + + set_property_action(undo_redo, "Merge Action 1", test_object, 100, UndoRedo::MERGE_ALL); + + CHECK(test_object->get_property() == 100); + CHECK(undo_redo->get_version() == 2); + CHECK(undo_redo->get_history_count() == 1); + + set_property_action(undo_redo, "Merge Action 1", test_object, 1000, UndoRedo::MERGE_ALL); + + CHECK(test_object->get_property() == 1000); + CHECK(undo_redo->get_version() == 2); + CHECK(undo_redo->get_history_count() == 1); + + memdelete(test_object); + memdelete(undo_redo); +} + +TEST_CASE("[UndoRedo] Merge Method UndoRedo") { + GDREGISTER_CLASS(_TestUndoRedoObject); + UndoRedo *undo_redo = memnew(UndoRedo()); + + _TestUndoRedoObject *test_object = memnew(_TestUndoRedoObject()); + + CHECK(test_object->get_property() == 0); + CHECK(undo_redo->get_version() == 1); + CHECK(undo_redo->get_history_count() == 0); + + increment_property_action(undo_redo, "Merge Increment 1", test_object, 10, UndoRedo::MERGE_ALL); + + CHECK(test_object->get_property() == 10); + CHECK(undo_redo->get_version() == 2); + CHECK(undo_redo->get_history_count() == 1); + + increment_property_action(undo_redo, "Merge Increment 1", test_object, 10, UndoRedo::MERGE_ALL); + + CHECK(test_object->get_property() == 20); + CHECK(undo_redo->get_version() == 2); + CHECK(undo_redo->get_history_count() == 1); + + increment_property_action(undo_redo, "Merge Increment 1", test_object, 10, UndoRedo::MERGE_ALL); + + CHECK(test_object->get_property() == 30); + CHECK(undo_redo->get_version() == 2); + CHECK(undo_redo->get_history_count() == 1); + + undo_redo->undo(); + + CHECK(test_object->get_property() == 0); + CHECK(undo_redo->get_version() == 1); + CHECK(undo_redo->get_history_count() == 1); + + undo_redo->redo(); + + CHECK(test_object->get_property() == 30); + CHECK(undo_redo->get_version() == 2); + CHECK(undo_redo->get_history_count() == 1); + + memdelete(test_object); + memdelete(undo_redo); +} + +} //namespace TestUndoRedo + +#endif // TEST_UNDO_REDO_H diff --git a/tests/test_main.cpp b/tests/test_main.cpp index f40f5629730..423932e2cda 100644 --- a/tests/test_main.cpp +++ b/tests/test_main.cpp @@ -73,6 +73,7 @@ #include "tests/core/object/test_class_db.h" #include "tests/core/object/test_method_bind.h" #include "tests/core/object/test_object.h" +#include "tests/core/object/test_undo_redo.h" #include "tests/core/os/test_os.h" #include "tests/core/string/test_node_path.h" #include "tests/core/string/test_string.h"