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

EditorUndoRedoManager: Use fixed history when a custom context is provided #100061

Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion editor/editor_settings_dialog.cpp
Original file line number Diff line number Diff line change
@@ -332,7 +332,7 @@ void EditorSettingsDialog::_update_shortcut_events(const String &p_path, const A
Ref<Shortcut> current_sc = EditorSettings::get_singleton()->get_shortcut(p_path);

EditorUndoRedoManager *undo_redo = EditorUndoRedoManager::get_singleton();
undo_redo->create_action(vformat(TTR("Edit Shortcut: %s"), p_path));
undo_redo->create_action(vformat(TTR("Edit Shortcut: %s"), p_path), UndoRedo::MERGE_DISABLE, EditorSettings::get_singleton());
undo_redo->add_do_method(current_sc.ptr(), "set_events", p_events);
undo_redo->add_undo_method(current_sc.ptr(), "set_events", current_sc->get_events());
undo_redo->add_do_method(EditorSettings::get_singleton(), "mark_setting_changed", "shortcuts");
1 change: 1 addition & 0 deletions editor/editor_undo_redo_manager.cpp
Original file line number Diff line number Diff line change
@@ -150,6 +150,7 @@ void EditorUndoRedoManager::create_action(const String &p_name, UndoRedo::MergeM
if (p_custom_context) {
// This assigns history to pending action.
get_history_for_object(p_custom_context);
force_fixed_history();
Copy link
Member

Choose a reason for hiding this comment

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

force_fixed_history() is supposed to be called manually. EditorUndoRedoManager will automatically deduce optimal history, if you force a different one it might lead to bugs. The method should be used if automatic history fails (there are some cases where it can't be determined), but you know that the forced one is correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without a change similar to this one, the custom_context parameter of create_action does not always function correctly (or maybe it never functions correctly). If you want, I can open a new issue describing this in more detail?

Copy link
Member

Choose a reason for hiding this comment

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

Feel free to open it, but it might be misunderstanding.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've filed an issue to better describe the issue that this PR aims to resolve: #100108

}
}