Skip to content

Improve undo redo API and docs#117851

Open
Goldenlion5648 wants to merge 1 commit intogodotengine:masterfrom
Goldenlion5648:undo-redo-tweaks
Open

Improve undo redo API and docs#117851
Goldenlion5648 wants to merge 1 commit intogodotengine:masterfrom
Goldenlion5648:undo-redo-tweaks

Conversation

@Goldenlion5648
Copy link
Copy Markdown
Contributor

@Goldenlion5648 Goldenlion5648 commented Mar 26, 2026

The API was a bit inconsistent and unclear. This attempts to fix some of that with examples and aliases for existing API methods, and exposing discard_redo()

Fixes godotengine/godot-proposals#14539

The examples can be updated to use bind once #107756 gets merged

Maybe in a followup PR or this one depending on what people think:

  • get_action_name could be get_action_name_at_index
  • Add a built in way to print the UndoRedo object (just the names of the states, and maybe function names?)
  • The editor inspector could show the state names, currently if the godot program hits a breakpoint in the editor, it is unknown what the state of the undo redo history was
image

@Goldenlion5648 Goldenlion5648 requested review from a team as code owners March 26, 2026 00:21
@Nintorch Nintorch added this to the 4.x milestone Mar 26, 2026
Copy link
Copy Markdown
Member

@AThousandShips AThousandShips left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the new method aliases is a good idea in addition to these changes, but my request for changes is about the documentation errors

Comment thread doc/classes/UndoRedo.xml Outdated
Comment thread doc/classes/UndoRedo.xml Outdated
Comment thread doc/classes/UndoRedo.xml Outdated
Comment thread doc/classes/UndoRedo.xml Outdated
Comment thread doc/classes/UndoRedo.xml Outdated
@Goldenlion5648
Copy link
Copy Markdown
Contributor Author

If we wanted to totally replace the existing methods, is there a preferred way to deprecate the old ones?

@AThousandShips
Copy link
Copy Markdown
Member

Yes to deprecate them, but I don't think there's any real strong reason to do so here, it creates clutter and confusion I'd say, so there would need to be some concrete demand or need for the change, is there a lot of confusion over these names?

Comment thread doc/classes/UndoRedo.xml Outdated
Comment thread doc/classes/UndoRedo.xml Outdated
Comment thread doc/classes/UndoRedo.xml Outdated
youfch added a commit to youfch/godot that referenced this pull request Mar 26, 2026
@MewPurPur
Copy link
Copy Markdown
Contributor

MewPurPur commented Mar 28, 2026

Sorry to shoehorn, but I have a proposal to turn UndoRedo into a RefCounted class, to avoid the need to free it manually or otherwise get memory leaks: godotengine/godot-proposals#11868

I'm not saying this PR has to be a full rework of UndoRedo, I wouldn't have mentioned it if the PR was just adding a new method without also doing aesthetic renames and deprecating stuff. But since it does, it seems like a good opportunity to fix the mistakes from the initial implementation in one swoop.

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.

Expose discard redo in UndoRedo

4 participants