diff --git a/src/editor/component_panels/component_panel.cpp b/src/editor/component_panels/component_panel.cpp index 5fa660dc..0251e0e7 100644 --- a/src/editor/component_panels/component_panel.cpp +++ b/src/editor/component_panels/component_panel.cpp @@ -69,7 +69,7 @@ void OrchestratorScriptComponentPanel::_tree_item_edited() ERR_FAIL_COND_MSG(!item, "Cannot edit item when no item selected"); const String old_name = item->get_meta("__name"); - const String new_name = item->get_text(0); + String new_name = item->get_text(0); // Nothing to edit if they're identical if (old_name.match(new_name)) @@ -78,7 +78,12 @@ void OrchestratorScriptComponentPanel::_tree_item_edited() return; } - _handle_item_renamed(old_name, new_name); + new_name = NameUtils::create_unique_name(new_name, _get_existing_names()); + if (!_handle_item_renamed(old_name, new_name)) + { + item->set_text(0, item->get_meta("__rollback_name")); + return; + } update(); } @@ -231,6 +236,23 @@ void OrchestratorScriptComponentPanel::_show_notification(const String& p_messag _notify->popup_centered(); } +void OrchestratorScriptComponentPanel::_show_invalid_name(const String& p_type, bool p_supports_friendly_names) +{ + String message = vformat("The %s name is not valid. Names must follow these requirements:\n\n", p_type); + message += "* Must start with a letter (A-Z, a-z) or an underscore ('_')\n"; + message += "* Can include letters (A-Z, a-z), numbers (0-9), and underscores ('_')\n"; + message += "* Should not start with a number (0-9)\n"; + message += "* Cannot contain spaces or special characters\n"; + + if (p_supports_friendly_names) + { + message += vformat("\nIf you want a space to appear in the %s name, please use camel-case (MyName).\n", p_type); + message += "With friendly names enabled, the name will be rendered as 'My Name' automatically."; + } + + _show_notification(message); +} + void OrchestratorScriptComponentPanel::_confirm_removal(TreeItem* p_item) { _confirm->set_text(_get_remove_confirm_text(p_item) + "\n\nDo you want to continue?"); @@ -270,7 +292,7 @@ bool OrchestratorScriptComponentPanel::_find_child_and_activate(const String& p_ { Ref timer = get_tree()->create_timer(0.1f); if (timer.is_valid()) - timer->connect("timeout", callable_mp(_tree, &Tree::edit_selected).bind(true)); + timer->connect("timeout", callable_mp(this, &OrchestratorScriptComponentPanel::_edit_selected_tree_item)); } return true; diff --git a/src/editor/component_panels/component_panel.h b/src/editor/component_panels/component_panel.h index 72d88ab5..cdb92b28 100644 --- a/src/editor/component_panels/component_panel.h +++ b/src/editor/component_panels/component_panel.h @@ -97,6 +97,11 @@ class OrchestratorScriptComponentPanel : public VBoxContainer /// @param p_message the text to notify void _show_notification(const String& p_message); + /// Shows a common dialog error about invalid identifier names. + /// @param p_type the type + /// @param p_supports_friendly_names true to show detail about friendly names + void _show_invalid_name(const String& p_type, bool p_supports_friendly_names = true); + /// Presents the user a dialog, confirming the removal of the tree item. /// @param p_item the item to be removed, should not be null void _confirm_removal(TreeItem* p_item); diff --git a/src/editor/component_panels/functions_panel.cpp b/src/editor/component_panels/functions_panel.cpp index 8841b51a..c5cb8729 100644 --- a/src/editor/component_panels/functions_panel.cpp +++ b/src/editor/component_panels/functions_panel.cpp @@ -160,7 +160,15 @@ bool OrchestratorScriptFunctionsComponentPanel::_handle_item_renamed(const Strin return false; } - _orchestration->rename_function(p_old_name, p_new_name); + if (!p_new_name.is_valid_identifier()) + { + _show_invalid_name("function"); + return false; + } + + if (!_orchestration->rename_function(p_old_name, p_new_name)) + return false; + emit_signal("graph_renamed", p_old_name, p_new_name); return true; } diff --git a/src/editor/component_panels/graphs_panel.cpp b/src/editor/component_panels/graphs_panel.cpp index be451dc0..bbd66a87 100644 --- a/src/editor/component_panels/graphs_panel.cpp +++ b/src/editor/component_panels/graphs_panel.cpp @@ -184,7 +184,15 @@ bool OrchestratorScriptGraphsComponentPanel::_handle_item_renamed(const String& return false; } - _orchestration->rename_graph(p_old_name, p_new_name); + if (!p_new_name.is_valid_identifier()) + { + _show_invalid_name("graph"); + return false; + } + + if (!_orchestration->rename_graph(p_old_name, p_new_name)) + return false; + emit_signal("graph_renamed", p_old_name, p_new_name); return true; } diff --git a/src/editor/component_panels/signals_panel.cpp b/src/editor/component_panels/signals_panel.cpp index 80f7d4ac..8e3a6d30 100644 --- a/src/editor/component_panels/signals_panel.cpp +++ b/src/editor/component_panels/signals_panel.cpp @@ -90,8 +90,13 @@ bool OrchestratorScriptSignalsComponentPanel::_handle_item_renamed(const String& return false; } - _orchestration->rename_custom_user_signal(p_old_name, p_new_name); - return true; + if (!p_new_name.is_valid_identifier()) + { + _show_invalid_name("signal", false); + return false; + } + + return _orchestration->rename_custom_user_signal(p_old_name, p_new_name); } void OrchestratorScriptSignalsComponentPanel::_handle_remove(TreeItem* p_item) diff --git a/src/editor/component_panels/variables_panel.cpp b/src/editor/component_panels/variables_panel.cpp index f4b2a308..f6e63311 100644 --- a/src/editor/component_panels/variables_panel.cpp +++ b/src/editor/component_panels/variables_panel.cpp @@ -153,8 +153,13 @@ bool OrchestratorScriptVariablesComponentPanel::_handle_item_renamed(const Strin return false; } - _orchestration->rename_variable(p_old_name, p_new_name); - return true; + if (!p_new_name.is_valid_identifier()) + { + _show_invalid_name("variable", false); + return false; + } + + return _orchestration->rename_variable(p_old_name, p_new_name); } void OrchestratorScriptVariablesComponentPanel::_handle_remove(TreeItem* p_item) diff --git a/src/orchestration/orchestration.cpp b/src/orchestration/orchestration.cpp index c6752938..a92dc5ac 100644 --- a/src/orchestration/orchestration.cpp +++ b/src/orchestration/orchestration.cpp @@ -514,6 +514,7 @@ Ref Orchestration::create_graph(const StringName& p_name, int p_fl { ERR_FAIL_COND_V_MSG(has_graph(p_name), nullptr, "A graph with that name already exists: " + p_name); ERR_FAIL_COND_V_MSG(p_name.is_empty(), nullptr, "A name is required to create a graph."); + ERR_FAIL_COND_V_MSG(!p_name.is_valid_identifier(), nullptr, "The name is not a valid graph name."); Ref graph(memnew(OScriptGraph)); graph->_orchestration = this; @@ -571,16 +572,23 @@ Ref Orchestration::find_graph(const Ref& p_node) ERR_FAIL_V_MSG(nullptr, "No graph contains the node with the unique ID: " + itos(p_node->get_id())); } -void Orchestration::rename_graph(const StringName& p_old_name, const StringName& p_new_name) +bool Orchestration::rename_graph(const StringName& p_old_name, const StringName& p_new_name) { - ERR_FAIL_COND_MSG(!has_graph(p_old_name), "No graph exists with the old name: " + p_old_name); - ERR_FAIL_COND_MSG(has_graph(p_new_name), "A graph already exists with the new name: " + p_new_name); + ERR_FAIL_COND_V_MSG(!has_graph(p_old_name), false, "No graph exists with the old name: " + p_old_name); + ERR_FAIL_COND_V_MSG(has_graph(p_new_name), false, "A graph already exists with the new name: " + p_new_name); + ERR_FAIL_COND_V_MSG(!p_new_name.is_valid_identifier(), false, "The new graph name is not a valid."); + + UtilityFunctions::print("Create Graph with name [", p_old_name, "] to [", p_new_name, "]"); Ref graph = get_graph(p_old_name); + if (!graph.is_valid()) + return false; + graph->set_graph_name(p_new_name); _graphs[p_new_name] = graph; _graphs.erase(p_old_name); + return true; } Vector> Orchestration::get_graphs() const @@ -675,31 +683,38 @@ Ref Orchestration::find_function(const Guid& p_guid) const return nullptr; } -void Orchestration::rename_function(const StringName& p_old_name, const StringName& p_new_name) +bool Orchestration::rename_function(const StringName& p_old_name, const StringName& p_new_name) { // Ignore if the old/new names are the same if (p_old_name == p_new_name) - return; + return false; - ERR_FAIL_COND_MSG(_has_instances(), "Cannot rename function, instances exist."); - ERR_FAIL_COND_MSG(!has_function(p_old_name), "Cannot rename, no function found with old name: " + p_old_name); - ERR_FAIL_COND_MSG(has_function(p_new_name), "Cannot rename, a function already exists with new name: " + p_new_name); - ERR_FAIL_COND_MSG(!String(p_new_name).is_valid_identifier(), "New function name is invalid: " + p_new_name); - ERR_FAIL_COND_MSG(has_variable(p_new_name), "Cannot rename function, a variable with name already exists: " + p_new_name); - ERR_FAIL_COND_MSG(has_custom_signal(p_new_name), "Cannot rename function, a signal with the name already exists: " + p_new_name); + ERR_FAIL_COND_V_MSG(_has_instances(), false, "Cannot rename function, instances exist."); + ERR_FAIL_COND_V_MSG(!has_function(p_old_name), false, "Cannot rename, no function found with old name: " + p_old_name); + ERR_FAIL_COND_V_MSG(has_function(p_new_name), false, "Cannot rename, a function already exists with new name: " + p_new_name); + ERR_FAIL_COND_V_MSG(!String(p_new_name).is_valid_identifier(), false, "New function name is invalid: " + p_new_name); + ERR_FAIL_COND_V_MSG(has_variable(p_new_name), false, "Cannot rename function, a variable with name already exists: " + p_new_name); + ERR_FAIL_COND_V_MSG(has_custom_signal(p_new_name), false, "Cannot rename function, a signal with the name already exists: " + p_new_name); const Ref function = _functions[p_old_name]; - function->rename(p_new_name); - - _functions.erase(p_old_name); - _functions[p_new_name] = function; + if (!function.is_valid() || !function->can_be_renamed()) + return false; // Rename function graph, if found const Ref function_graph = find_graph(p_old_name); if (function_graph.is_valid() && function_graph->get_flags().has_flag(OScriptGraph::GraphFlags::GF_FUNCTION)) - rename_graph(p_old_name, p_new_name); + { + if (!rename_graph(p_old_name, p_new_name)) + return false; + } + + function->rename(p_new_name); + + _functions.erase(p_old_name); + _functions[p_new_name] = function; _self->emit_signal("functions_changed"); + return true; } PackedStringArray Orchestration::get_function_names() const @@ -789,15 +804,15 @@ Ref Orchestration::get_variable(const StringName& p_name) return has_variable(p_name) ? _variables[p_name] : nullptr; } -void Orchestration::rename_variable(const StringName& p_old_name, const StringName& p_new_name) +bool Orchestration::rename_variable(const StringName& p_old_name, const StringName& p_new_name) { if (p_old_name == p_new_name) - return; + return false; - ERR_FAIL_COND_MSG(_has_instances(), "Cannot rename variable, instances exist."); - ERR_FAIL_COND_MSG(!has_variable(p_old_name), "Cannot rename, no variable exists with the old name: " + p_old_name); - ERR_FAIL_COND_MSG(has_variable(p_new_name), "Cannot rename, a variable already exists with the new name: " + p_new_name); - ERR_FAIL_COND_MSG(!String(p_new_name).is_valid_identifier(), "Cannot rename, variable name is not valid: " + p_new_name); + ERR_FAIL_COND_V_MSG(_has_instances(), false, "Cannot rename variable, instances exist."); + ERR_FAIL_COND_V_MSG(!has_variable(p_old_name), false, "Cannot rename, no variable exists with the old name: " + p_old_name); + ERR_FAIL_COND_V_MSG(has_variable(p_new_name), false, "Cannot rename, a variable already exists with the new name: " + p_new_name); + ERR_FAIL_COND_V_MSG(!String(p_new_name).is_valid_identifier(), false, "Cannot rename, variable name is not valid: " + p_new_name); const Ref variable = _variables[p_old_name]; variable->set_variable_name(p_new_name); @@ -812,6 +827,8 @@ void Orchestration::rename_variable(const StringName& p_old_name, const StringNa #ifdef TOOLS_ENABLED _update_placeholders(); #endif + + return true; } Vector> Orchestration::get_variables() const @@ -849,6 +866,7 @@ bool Orchestration::has_custom_signal(const StringName& p_name) const Ref Orchestration::create_custom_signal(const StringName& p_name) { ERR_FAIL_COND_V_MSG(has_custom_signal(p_name), nullptr, "A custom signal already exists with the name: " + p_name); + ERR_FAIL_COND_V_MSG(!p_name.is_valid_identifier(), nullptr, "The name is not a valid signal name."); MethodInfo method; method.name = p_name; @@ -894,15 +912,15 @@ Ref Orchestration::find_custom_signal(const StringName& p_name) c return has_custom_signal(p_name) ? _signals[p_name] : nullptr; } -void Orchestration::rename_custom_user_signal(const StringName& p_old_name, const StringName& p_new_name) +bool Orchestration::rename_custom_user_signal(const StringName& p_old_name, const StringName& p_new_name) { if (p_old_name == p_new_name) - return; + return false; - ERR_FAIL_COND_MSG(_has_instances(), "Cannot rename custom signal, instances exist."); - ERR_FAIL_COND_MSG(!has_custom_signal(p_old_name), "No custom signal exists with the old name: " + p_old_name); - ERR_FAIL_COND_MSG(has_custom_signal(p_new_name), "A custom signal already exists with the new name: " + p_new_name); - ERR_FAIL_COND_MSG(!String(p_new_name).is_valid_identifier(), "The custom signal name is invalid: " + p_new_name); + ERR_FAIL_COND_V_MSG(_has_instances(), false, "Cannot rename custom signal, instances exist."); + ERR_FAIL_COND_V_MSG(!has_custom_signal(p_old_name), false, "No custom signal exists with the old name: " + p_old_name); + ERR_FAIL_COND_V_MSG(has_custom_signal(p_new_name), false, "A custom signal already exists with the new name: " + p_new_name); + ERR_FAIL_COND_V_MSG(!String(p_new_name).is_valid_identifier(), false, "The custom signal name is invalid: " + p_new_name); const Ref signal = find_custom_signal(p_old_name); signal->rename(p_new_name); @@ -917,6 +935,8 @@ void Orchestration::rename_custom_user_signal(const StringName& p_old_name, cons #ifdef TOOLS_ENABLED _update_placeholders(); #endif + + return true; } Vector> Orchestration::get_custom_signals() const diff --git a/src/orchestration/orchestration.h b/src/orchestration/orchestration.h index 21b1c9ea..cf11fb38 100644 --- a/src/orchestration/orchestration.h +++ b/src/orchestration/orchestration.h @@ -190,7 +190,7 @@ class Orchestration Ref get_graph(const StringName& p_name) const; Ref find_graph(const StringName& p_name) const; Ref find_graph(const Ref& p_node); - void rename_graph(const StringName& p_old_name, const StringName& p_new_name); + bool rename_graph(const StringName& p_old_name, const StringName& p_new_name); Vector> get_graphs() const; //~ End Graph Interface @@ -200,7 +200,7 @@ class Orchestration void remove_function(const StringName& p_name); Ref find_function(const StringName& p_name) const; Ref find_function(const Guid& p_guid) const; - void rename_function(const StringName& p_old_name, const StringName& p_new_name); + bool rename_function(const StringName& p_old_name, const StringName& p_new_name); PackedStringArray get_function_names() const; int get_function_node_id(const StringName& p_name) const; Vector> get_functions() const; @@ -211,7 +211,7 @@ class Orchestration Ref create_variable(const StringName& p_name, Variant::Type p_type = Variant::NIL); void remove_variable(const StringName& p_name); Ref get_variable(const StringName& p_name); - void rename_variable(const StringName& p_old_name, const StringName& p_new_name); + bool rename_variable(const StringName& p_old_name, const StringName& p_new_name); Vector> get_variables() const; PackedStringArray get_variable_names() const; bool can_remove_variable(const StringName& p_name) const; @@ -223,7 +223,7 @@ class Orchestration void remove_custom_signal(const StringName& p_name); Ref get_custom_signal(const StringName& p_name); Ref find_custom_signal(const StringName& p_name) const; - void rename_custom_user_signal(const StringName& p_old_name, const StringName& p_new_name); + bool rename_custom_user_signal(const StringName& p_old_name, const StringName& p_new_name); Vector> get_custom_signals() const; PackedStringArray get_custom_signal_names() const; bool can_remove_custom_signal(const StringName& p_name) const;