From c1c6911b5c364100a4c500e06ac0a0d5dfc3a371 Mon Sep 17 00:00:00 2001 From: Fabio Alessandrelli Date: Tue, 29 Aug 2023 12:44:01 +0200 Subject: [PATCH] [MP] Improve ReplicationEditor UX Use an option button for the replication mode making sync and watch (now "Always" and "On Change") mutually exclusive. --- .../multiplayer/editor/replication_editor.cpp | 107 +++++++++--------- .../multiplayer/editor/replication_editor.h | 5 +- 2 files changed, 53 insertions(+), 59 deletions(-) diff --git a/modules/multiplayer/editor/replication_editor.cpp b/modules/multiplayer/editor/replication_editor.cpp index cac5d0d3a8f..cf1db9d43db 100644 --- a/modules/multiplayer/editor/replication_editor.cpp +++ b/modules/multiplayer/editor/replication_editor.cpp @@ -176,11 +176,6 @@ ReplicationEditor::ReplicationEditor() { delete_dialog->connect("confirmed", callable_mp(this, &ReplicationEditor::_dialog_closed).bind(true)); add_child(delete_dialog); - error_dialog = memnew(AcceptDialog); - error_dialog->set_ok_button_text(TTR("Close")); - error_dialog->set_title(TTR("Error!")); - add_child(error_dialog); - VBoxContainer *vb = memnew(VBoxContainer); vb->set_v_size_flags(SIZE_EXPAND_FILL); add_child(vb); @@ -272,20 +267,17 @@ ReplicationEditor::ReplicationEditor() { tree = memnew(Tree); tree->set_hide_root(true); - tree->set_columns(5); + tree->set_columns(4); tree->set_column_titles_visible(true); tree->set_column_title(0, TTR("Properties")); tree->set_column_expand(0, true); tree->set_column_title(1, TTR("Spawn")); tree->set_column_expand(1, false); tree->set_column_custom_minimum_width(1, 100); - tree->set_column_title(2, TTR("Sync")); + tree->set_column_title(2, TTR("Replicate")); tree->set_column_custom_minimum_width(2, 100); - tree->set_column_title(3, TTR("Watch")); - tree->set_column_custom_minimum_width(3, 100); tree->set_column_expand(2, false); tree->set_column_expand(3, false); - tree->set_column_expand(4, false); tree->create_item(); tree->connect("button_clicked", callable_mp(this, &ReplicationEditor::_tree_button_pressed)); tree->connect("item_edited", callable_mp(this, &ReplicationEditor::_tree_item_edited)); @@ -304,7 +296,7 @@ ReplicationEditor::ReplicationEditor() { void ReplicationEditor::_bind_methods() { ClassDB::bind_method(D_METHOD("_update_config"), &ReplicationEditor::_update_config); - ClassDB::bind_method(D_METHOD("_update_checked", "property", "column", "checked"), &ReplicationEditor::_update_checked); + ClassDB::bind_method(D_METHOD("_update_value", "property", "column", "value"), &ReplicationEditor::_update_value); } bool ReplicationEditor::_can_drop_data_fw(const Point2 &p_point, const Variant &p_data, Control *p_from) const { @@ -375,20 +367,17 @@ void ReplicationEditor::_notification(int p_what) { void ReplicationEditor::_add_pressed() { if (!current) { - error_dialog->set_text(TTR("Please select a MultiplayerSynchronizer first.")); - error_dialog->popup_centered(); + EditorNode::get_singleton()->show_warning(TTR("Please select a MultiplayerSynchronizer first.")); return; } if (current->get_root_path().is_empty()) { - error_dialog->set_text(TTR("The MultiplayerSynchronizer needs a root path.")); - error_dialog->popup_centered(); + EditorNode::get_singleton()->show_warning(TTR("The MultiplayerSynchronizer needs a root path.")); return; } String np_text = np_line_edit->get_text(); if (np_text.is_empty()) { - error_dialog->set_text(TTR("Property/path must not be empty.")); - error_dialog->popup_centered(); + EditorNode::get_singleton()->show_warning(TTR("Property/path must not be empty.")); return; } @@ -399,6 +388,10 @@ void ReplicationEditor::_add_pressed() { np_text = "." + np_text; } NodePath path = NodePath(np_text); + if (path.is_empty()) { + EditorNode::get_singleton()->show_warning(vformat(TTR("Invalid property path: '%s'"), np_text)); + return; + } _add_sync_property(path); } @@ -413,36 +406,36 @@ void ReplicationEditor::_tree_item_edited() { return; } int column = tree->get_edited_column(); - ERR_FAIL_COND(column < 1 || column > 3); + ERR_FAIL_COND(column < 1 || column > 2); const NodePath prop = ti->get_metadata(0); EditorUndoRedoManager *undo_redo = EditorUndoRedoManager::get_singleton(); - bool value = ti->is_checked(column); - // We have a hard limit of 64 watchable properties per synchronizer. - if (column == 3 && value && config->get_watch_properties().size() > 64) { - error_dialog->set_text(TTR("Each MultiplayerSynchronizer can have no more than 64 watched properties.")); - error_dialog->popup_centered(); - ti->set_checked(column, false); - return; - } - String method; if (column == 1) { undo_redo->create_action(TTR("Set spawn property")); - method = "property_set_spawn"; + bool value = ti->is_checked(column); + undo_redo->add_do_method(config.ptr(), "property_set_spawn", prop, value); + undo_redo->add_undo_method(config.ptr(), "property_set_spawn", prop, !value); + undo_redo->add_do_method(this, "_update_value", prop, column, value ? 1 : 0); + undo_redo->add_undo_method(this, "_update_value", prop, column, value ? 1 : 0); + undo_redo->commit_action(); } else if (column == 2) { undo_redo->create_action(TTR("Set sync property")); - method = "property_set_sync"; - } else if (column == 3) { - undo_redo->create_action(TTR("Set watch property")); - method = "property_set_watch"; + int value = ti->get_range(column); + int old_value = config->property_get_replication_mode(prop); + // We have a hard limit of 64 watchable properties per synchronizer. + if (value == SceneReplicationConfig::REPLICATION_MODE_ON_CHANGE && config->get_watch_properties().size() >= 64) { + EditorNode::get_singleton()->show_warning(TTR("Each MultiplayerSynchronizer can have no more than 64 watched properties.")); + ti->set_range(column, old_value); + return; + } + undo_redo->add_do_method(config.ptr(), "property_set_replication_mode", prop, value); + undo_redo->add_undo_method(config.ptr(), "property_set_replication_mode", prop, old_value); + undo_redo->add_do_method(this, "_update_value", prop, column, value); + undo_redo->add_undo_method(this, "_update_value", prop, column, old_value); + undo_redo->commit_action(); } else { ERR_FAIL(); } - undo_redo->add_do_method(config.ptr(), method, prop, value); - undo_redo->add_undo_method(config.ptr(), method, prop, !value); - undo_redo->add_do_method(this, "_update_checked", prop, column, value); - undo_redo->add_undo_method(this, "_update_checked", prop, column, !value); - undo_redo->commit_action(); } void ReplicationEditor::_tree_button_pressed(Object *p_item, int p_column, int p_id, MouseButton p_button) { @@ -467,15 +460,13 @@ void ReplicationEditor::_dialog_closed(bool p_confirmed) { const NodePath prop = deleting; int idx = config->property_get_index(prop); bool spawn = config->property_get_spawn(prop); - bool sync = config->property_get_sync(prop); - bool watch = config->property_get_watch(prop); + SceneReplicationConfig::ReplicationMode mode = config->property_get_replication_mode(prop); EditorUndoRedoManager *undo_redo = EditorUndoRedoManager::get_singleton(); undo_redo->create_action(TTR("Remove Property")); undo_redo->add_do_method(config.ptr(), "remove_property", prop); undo_redo->add_undo_method(config.ptr(), "add_property", prop, idx); undo_redo->add_undo_method(config.ptr(), "property_set_spawn", prop, spawn); - undo_redo->add_undo_method(config.ptr(), "property_set_sync", prop, sync); - undo_redo->add_undo_method(config.ptr(), "property_set_watch", prop, watch); + undo_redo->add_undo_method(config.ptr(), "property_set_replication_mode", prop, mode); undo_redo->add_do_method(this, "_update_config"); undo_redo->add_undo_method(this, "_update_config"); undo_redo->commit_action(); @@ -483,14 +474,18 @@ void ReplicationEditor::_dialog_closed(bool p_confirmed) { deleting = NodePath(); } -void ReplicationEditor::_update_checked(const NodePath &p_prop, int p_column, bool p_checked) { +void ReplicationEditor::_update_value(const NodePath &p_prop, int p_column, int p_value) { if (!tree->get_root()) { return; } TreeItem *ti = tree->get_root()->get_first_child(); while (ti) { if (ti->get_metadata(0).operator NodePath() == p_prop) { - ti->set_checked(p_column, p_checked); + if (p_column == 1) { + ti->set_checked(p_column, p_value != 0); + } else if (p_column == 2) { + ti->set_range(p_column, p_value); + } return; } ti = ti->get_next(); @@ -511,7 +506,7 @@ void ReplicationEditor::_update_config() { } for (int i = 0; i < props.size(); i++) { const NodePath path = props[i]; - _add_property(path, config->property_get_spawn(path), config->property_get_sync(path), config->property_get_watch(path)); + _add_property(path, config->property_get_spawn(path), config->property_get_replication_mode(path)); } } @@ -553,14 +548,13 @@ static bool can_sync(const Variant &p_var) { } } -void ReplicationEditor::_add_property(const NodePath &p_property, bool p_spawn, bool p_sync, bool p_watch) { +void ReplicationEditor::_add_property(const NodePath &p_property, bool p_spawn, SceneReplicationConfig::ReplicationMode p_mode) { String prop = String(p_property); TreeItem *item = tree->create_item(); item->set_selectable(0, false); item->set_selectable(1, false); item->set_selectable(2, false); item->set_selectable(3, false); - item->set_selectable(4, false); item->set_text(0, prop); item->set_metadata(0, prop); Node *root_node = current && !current->get_root_path().is_empty() ? current->get_node(current->get_root_path()) : nullptr; @@ -577,22 +571,23 @@ void ReplicationEditor::_add_property(const NodePath &p_property, bool p_spawn, bool valid = false; Variant value = node->get(subpath, &valid); if (valid && !can_sync(value)) { - item->set_icon(3, get_theme_icon(SNAME("StatusWarning"), SNAME("EditorIcons"))); - item->set_tooltip_text(3, TTR("Property of this type not supported.")); + item->set_icon(0, get_theme_icon(SNAME("StatusWarning"), SNAME("EditorIcons"))); + item->set_tooltip_text(0, TTR("Property of this type not supported.")); + } else { + item->set_icon(0, icon); } + } else { + item->set_icon(0, icon); } - item->set_icon(0, icon); - item->add_button(4, get_theme_icon(SNAME("Remove"), SNAME("EditorIcons"))); + item->add_button(3, get_theme_icon(SNAME("Remove"), SNAME("EditorIcons"))); item->set_text_alignment(1, HORIZONTAL_ALIGNMENT_CENTER); item->set_cell_mode(1, TreeItem::CELL_MODE_CHECK); item->set_checked(1, p_spawn); item->set_editable(1, true); item->set_text_alignment(2, HORIZONTAL_ALIGNMENT_CENTER); - item->set_cell_mode(2, TreeItem::CELL_MODE_CHECK); - item->set_checked(2, p_sync); + item->set_cell_mode(2, TreeItem::CELL_MODE_RANGE); + item->set_range_config(2, 0, 2, 1); + item->set_text(2, "Never,Always,On Change"); + item->set_range(2, (int)p_mode); item->set_editable(2, true); - item->set_text_alignment(3, HORIZONTAL_ALIGNMENT_CENTER); - item->set_cell_mode(3, TreeItem::CELL_MODE_CHECK); - item->set_checked(3, p_watch); - item->set_editable(3, true); } diff --git a/modules/multiplayer/editor/replication_editor.h b/modules/multiplayer/editor/replication_editor.h index 07fac6eb3b0..80c1892ec31 100644 --- a/modules/multiplayer/editor/replication_editor.h +++ b/modules/multiplayer/editor/replication_editor.h @@ -51,7 +51,6 @@ class ReplicationEditor : public VBoxContainer { private: MultiplayerSynchronizer *current = nullptr; - AcceptDialog *error_dialog = nullptr; ConfirmationDialog *delete_dialog = nullptr; Button *add_pick_button = nullptr; Button *add_from_path_button = nullptr; @@ -75,10 +74,10 @@ private: void _np_text_submitted(const String &p_newtext); void _tree_item_edited(); void _tree_button_pressed(Object *p_item, int p_column, int p_id, MouseButton p_button); - void _update_checked(const NodePath &p_prop, int p_column, bool p_checked); + void _update_value(const NodePath &p_prop, int p_column, int p_checked); void _update_config(); void _dialog_closed(bool p_confirmed); - void _add_property(const NodePath &p_property, bool p_spawn = true, bool p_sync = true, bool p_watch = false); + void _add_property(const NodePath &p_property, bool p_spawn, SceneReplicationConfig::ReplicationMode p_mode); void _pick_node_filter_text_changed(const String &p_newtext); void _pick_node_select_recursive(TreeItem *p_item, const String &p_filter, Vector &p_select_candidates);