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

Unify usage of undo_redo in editor #65062

Merged
merged 1 commit into from
Nov 2, 2022
Merged

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Aug 29, 2022

While making #59564 I noticed that undo usage in the editor is very inconsistent. Some classes use singleton, some have EditorData pointer, but most classes have "cached" UndoRedo pointer (now EditorUndoRedoManager), which is often set with set_undo_redo(). The thing is, it's always initialized with the singleton UndoRedo XD. And only leads to useless forward-declares and useless calls that pass global object lol

This PR is supposed clean that up. It removes all references to EditorUndoRedoManager in header files. Now the undo usage is

Ref<EditorUndoRedoManager> &undo_redo = EditorNode::get_undo_redo();
undo_redo->do_stuff()

consistently. I also fixed some code that didn't use the undoredo as reference. The cleanup is not complete though. There are many changes, so I'm going to split it into multiple PRs.

@akien-mga
Copy link
Member

2022-11-02T15:35:46.7978909Z tile_set_editor.cpp
2022-11-02T15:35:50.1909492Z scons: *** [editor\plugins\tiles\tile_set_atlas_source_editor.windows.editor.x86_64.obj] Error 2
2022-11-02T15:35:50.1910138Z editor\plugins\tiles\tile_set_atlas_source_editor.cpp(1315): error C2220: the following warning is treated as an error
2022-11-02T15:35:52.0761081Z editor\plugins\tiles\tile_set_atlas_source_editor.cpp(1315): warning C4458: declaration of 'undo_redo' hides class member
2022-11-02T15:35:52.0762128Z D:\a\godot\godot\editor\plugins\tiles\tile_set_atlas_source_editor.h(117): note: see declaration of 'TileSetAtlasSourceEditor::undo_redo'
2022-11-02T15:35:52.0762837Z editor\plugins\tiles\tile_set_atlas_source_editor.cpp(1551): warning C4458: declaration of 'undo_redo' hides class member
2022-11-02T15:35:52.0763634Z D:\a\godot\godot\editor\plugins\tiles\tile_set_atlas_source_editor.h(117): note: see declaration of 'TileSetAtlasSourceEditor::undo_redo'
2022-11-02T15:35:52.0764329Z editor\plugins\tiles\tile_set_atlas_source_editor.cpp(2250): warning C4458: declaration of 'undo_redo' hides class member
2022-11-02T15:35:52.0765086Z D:\a\godot\godot\editor\plugins\tiles\tile_set_atlas_source_editor.h(117): note: see declaration of 'TileSetAtlasSourceEditor::undo_redo'

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@akien-mga akien-mga merged commit eefba5b into godotengine:master Nov 2, 2022
@akien-mga
Copy link
Member

Thanks!

@KoBeWi KoBeWi deleted the RedoUndo branch November 2, 2022 18:10
@KoBeWi KoBeWi modified the milestones: 4.x, 4.0 Nov 2, 2022
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