Merge pull request #79796 from KoBeWi/hack_and_slash

Add a way to force history for undoredo
godotengine · Jun 26, 2024 · 35683de · 35683de
2 parents 95110dd + 57e0465
commit 35683de
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 2 deletions.
diff --git a/doc/classes/EditorUndoRedoManager.xml b/doc/classes/EditorUndoRedoManager.xml
@@ -88,6 +88,13 @@
 				The way undo operation are ordered in actions is dictated by [param backward_undo_ops]. When [param backward_undo_ops] is [code]false[/code] undo option are ordered in the same order they were added. Which means the first operation to be added will be the first to be undone.
 			</description>
 		</method>
+		<method name="force_fixed_history">
+			<return type="void" />
+			<description>
+				Forces the next operation (e.g. [method add_do_method]) to use the action's history rather than guessing it from the object. This is sometimes needed when a history can't be correctly determined, like for a nested resource that doesn't have a path yet.
+				This method should only be used when absolutely necessary, otherwise it might cause invalid history state. For most of complex cases, the [code]custom_context[/code] parameter of [method create_action] is sufficient.
+			</description>
+		</method>
 		<method name="get_history_undo_redo" qualifiers="const">
 			<return type="UndoRedo" />
 			<param index="0" name="id" type="int" />

diff --git a/editor/editor_undo_redo_manager.cpp b/editor/editor_undo_redo_manager.cpp
@@ -104,8 +104,13 @@ int EditorUndoRedoManager::get_history_id_for_object(Object *p_object) const {
 }
 
 EditorUndoRedoManager::History &EditorUndoRedoManager::get_history_for_object(Object *p_object) {
-	int history_id = get_history_id_for_object(p_object);
-	ERR_FAIL_COND_V_MSG(pending_action.history_id != INVALID_HISTORY && history_id != pending_action.history_id, get_or_create_history(pending_action.history_id), vformat("UndoRedo history mismatch: expected %d, got %d.", pending_action.history_id, history_id));
+	int history_id;
+	if (!forced_history) {
+		history_id = get_history_id_for_object(p_object);
+		ERR_FAIL_COND_V_MSG(pending_action.history_id != INVALID_HISTORY && history_id != pending_action.history_id, get_or_create_history(pending_action.history_id), vformat("UndoRedo history mismatch: expected %d, got %d.", pending_action.history_id, history_id));
+	} else {
+		history_id = pending_action.history_id;
+	}
 
 	History &history = get_or_create_history(history_id);
 	if (pending_action.history_id == INVALID_HISTORY) {
@@ -116,6 +121,11 @@ EditorUndoRedoManager::History &EditorUndoRedoManager::get_history_for_object(Ob
 	return history;
 }
 
+void EditorUndoRedoManager::force_fixed_history() {
+	ERR_FAIL_COND_MSG(pending_action.history_id == INVALID_HISTORY, "The current action has no valid history assigned.");
+	forced_history = true;
+}
+
 void EditorUndoRedoManager::create_action_for_history(const String &p_name, int p_history_id, UndoRedo::MergeMode p_mode, bool p_backward_undo_ops) {
 	if (pending_action.history_id != INVALID_HISTORY) {
 		// Nested action.
@@ -236,6 +246,7 @@ void EditorUndoRedoManager::commit_action(bool p_execute) {
 		return; // Empty action, do nothing.
 	}
 
+	forced_history = false;
 	is_committing = true;
 
 	History &history = get_or_create_history(pending_action.history_id);
@@ -469,6 +480,7 @@ void EditorUndoRedoManager::_bind_methods() {
 	ClassDB::bind_method(D_METHOD("create_action", "name", "merge_mode", "custom_context", "backward_undo_ops"), &EditorUndoRedoManager::create_action, DEFVAL(UndoRedo::MERGE_DISABLE), DEFVAL((Object *)nullptr), DEFVAL(false));
 	ClassDB::bind_method(D_METHOD("commit_action", "execute"), &EditorUndoRedoManager::commit_action, DEFVAL(true));
 	ClassDB::bind_method(D_METHOD("is_committing_action"), &EditorUndoRedoManager::is_committing_action);
+	ClassDB::bind_method(D_METHOD("force_fixed_history"), &EditorUndoRedoManager::force_fixed_history);
 
 	{
 		MethodInfo mi;

diff --git a/editor/editor_undo_redo_manager.h b/editor/editor_undo_redo_manager.h
@@ -67,6 +67,7 @@ class EditorUndoRedoManager : public Object {
 	HashMap<int, History> history_map;
 	Action pending_action;
 
+	bool forced_history = false;
 	bool is_committing = false;
 
 	History *_get_newest_undo();
@@ -79,6 +80,7 @@ class EditorUndoRedoManager : public Object {
 	UndoRedo *get_history_undo_redo(int p_idx) const;
 	int get_history_id_for_object(Object *p_object) const;
 	History &get_history_for_object(Object *p_object);
+	void force_fixed_history();
 
 	void create_action_for_history(const String &p_name, int p_history_id, UndoRedo::MergeMode p_mode = UndoRedo::MERGE_DISABLE, bool p_backward_undo_ops = false);
 	void create_action(const String &p_name = "", UndoRedo::MergeMode p_mode = UndoRedo::MERGE_DISABLE, Object *p_custom_context = nullptr, bool p_backward_undo_ops = false);

diff --git a/editor/plugins/sprite_frames_editor_plugin.cpp b/editor/plugins/sprite_frames_editor_plugin.cpp
@@ -1056,21 +1056,25 @@ void SpriteFramesEditor::_rename_node_animation(EditorUndoRedoManager *undo_redo
 		for (Node *E : nodes) {
 			String current_name = E->call("get_animation");
 			if (current_name == p_filter) {
+				undo_redo->force_fixed_history(); // Fixes corner-case when editing SpriteFrames stored as separate file.
 				undo_redo->add_undo_method(E, "set_animation", p_new_animation);
 			}
 			String autoplay_name = E->call("get_autoplay");
 			if (autoplay_name == p_filter) {
+				undo_redo->force_fixed_history();
 				undo_redo->add_undo_method(E, "set_autoplay", p_new_autoplay);
 			}
 		}
 	} else {
 		for (Node *E : nodes) {
 			String current_name = E->call("get_animation");
 			if (current_name == p_filter) {
+				undo_redo->force_fixed_history();
 				undo_redo->add_do_method(E, "set_animation", p_new_animation);
 			}
 			String autoplay_name = E->call("get_autoplay");
 			if (autoplay_name == p_filter) {
+				undo_redo->force_fixed_history();
 				undo_redo->add_do_method(E, "set_autoplay", p_new_autoplay);
 			}
 		}

diff --git a/editor/plugins/visual_shader_editor_plugin.cpp b/editor/plugins/visual_shader_editor_plugin.cpp
@@ -3649,12 +3649,15 @@ void VisualShaderEditor::_add_node(int p_idx, const Vector<Variant> &p_ops, cons
 
 			if (output_port_type == VisualShaderNode::PORT_TYPE_SAMPLER) {
 				if (is_texture2d) {
+					undo_redo->force_fixed_history(); // vsnode is freshly created and has no path, so history can't be correctly determined.
 					undo_redo->add_do_method(vsnode.ptr(), "set_source", VisualShaderNodeTexture::SOURCE_PORT);
 				}
 				if (is_texture3d || is_texture2d_array) {
+					undo_redo->force_fixed_history();
 					undo_redo->add_do_method(vsnode.ptr(), "set_source", VisualShaderNodeSample3D::SOURCE_PORT);
 				}
 				if (is_cubemap) {
+					undo_redo->force_fixed_history();
 					undo_redo->add_do_method(vsnode.ptr(), "set_source", VisualShaderNodeCubemap::SOURCE_PORT);
 				}
 			}
@@ -3754,16 +3757,19 @@ void VisualShaderEditor::_add_node(int p_idx, const Vector<Variant> &p_ops, cons
 		//post-initialization
 
 		if (is_texture2d || is_texture3d || is_curve || is_curve_xyz) {
+			undo_redo->force_fixed_history();
 			undo_redo->add_do_method(vsnode.ptr(), "set_texture", ResourceLoader::load(p_resource_path));
 			return;
 		}
 
 		if (is_cubemap) {
+			undo_redo->force_fixed_history();
 			undo_redo->add_do_method(vsnode.ptr(), "set_cube_map", ResourceLoader::load(p_resource_path));
 			return;
 		}
 
 		if (is_texture2d_array) {
+			undo_redo->force_fixed_history();
 			undo_redo->add_do_method(vsnode.ptr(), "set_texture_array", ResourceLoader::load(p_resource_path));
 		}
 	}