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.

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

Add a way to force undo/redo operations to be kept in MERGE_ENDS mode #53713

Merged

Conversation

groud
Copy link
Member

@groud groud commented Oct 12, 2021

The MERGE_ENDS mode is used by the inspector to merge together actions with the same name, keeping only the undo operations from the first action, and the do operations from the last one. Theoretically, this work fine with most situations, like when you are editing a simple property, however, this might cause issue when modifying a property value might impact other properties. In such case, you might want to add an undo method when you detect the problematic value change.

The problem with the current master implementation, is that adding such an undo operation in the middle of a set of merged actions is not possible. Such operation is in fact automatically dropped.

This PR solves the problem by adding two methods start_force_keep_in_merge_ends and end_force_keep_in_merge_ends, to respectively start and end a block where all defined undo/do operations are marked as to be kept and processed, even in the MERGE_ENDS mode.

This solution has the advantage to keep compatibility. As the use case is not very common, I think it's better to keep it that way instead of adding an argument to the add_do_* and add_undo_* methods, which would be mainly problematic with the add_do_method and add_undo_method as they have variable argument lists.

This PR also solve a mistake in the documentation of the MERGE_ENDS constant.

Here is the script I used to test my changes:

extends Node

var undoredo := UndoRedo.new()
var id = 0

func printing(s):
	print("Executed: " + s)

func _ready():
	undoredo.create_action("Action 1")
	undoredo.add_do_method(self, "printing", "do 1")
	undoredo.add_undo_method(self, "printing", "undo 1")
	undoredo.start_force_keep_in_merge_ends()
	undoredo.add_do_method(self, "printing", "do 1 (forced)")
	undoredo.add_undo_method(self, "printing", "undo 1 (forced)")
	undoredo.end_force_keep_in_merge_ends()
	undoredo.commit_action()
	
	undoredo.create_action("Action 1", UndoRedo.MERGE_ENDS)
	undoredo.add_do_method(self, "printing", "do 2")
	undoredo.add_undo_method(self, "printing", "undo 2")
	undoredo.start_force_keep_in_merge_ends()
	undoredo.add_do_method(self, "printing", "do 2 (forced)")
	undoredo.add_undo_method(self, "printing", "undo 2 (forced)")
	undoredo.end_force_keep_in_merge_ends()
	undoredo.commit_action()

func _input(event):
	if event.is_action_pressed("ui_left"):
		undoredo.undo()
	if event.is_action_pressed("ui_right"):
		undoredo.redo()

@groud groud added this to the 4.0 milestone Oct 12, 2021
@groud groud requested review from a team as code owners October 12, 2021 12:31
@akien-mga akien-mga merged commit 480fc31 into godotengine:master Oct 12, 2021
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants