From d4741324920d81ef8d1a0dd0f6786676c997ff54 Mon Sep 17 00:00:00 2001 From: Chris Cranford Date: Sun, 22 Sep 2024 19:27:03 -0400 Subject: [PATCH] GH-811 Correctly initialize promoted variables --- src/editor/graph/graph_node_pin.cpp | 28 +-- src/editor/graph/graph_node_pin.h | 4 - src/orchestration/orchestration.cpp | 25 +++ src/orchestration/orchestration.h | 1 + src/script/variable.cpp | 291 +++++++++++++++++----------- src/script/variable.h | 32 +++ 6 files changed, 243 insertions(+), 138 deletions(-) diff --git a/src/editor/graph/graph_node_pin.cpp b/src/editor/graph/graph_node_pin.cpp index 5cdb726d..3dbfe5e9 100644 --- a/src/editor/graph/graph_node_pin.cpp +++ b/src/editor/graph/graph_node_pin.cpp @@ -262,23 +262,19 @@ void OrchestratorGraphNodePin::_remove_editable_pin() void OrchestratorGraphNodePin::_promote_as_variable() { - Orchestration* orchestation = _node->get_script_node()->get_orchestration(); - - Ref variable = orchestation->create_variable(_create_promoted_variable_name(), _pin->get_type()); + Ref variable = _node->get_script_node()->get_orchestration()->promote_to_variable(_pin); if (!variable.is_valid()) return; - variable->set_default_value(_pin->get_effective_default_value()); - OScriptNodeInitContext context; context.variable_name = variable->get_variable_name(); - Vector2 offset = Vector2(200, 25); Vector2 position = _node->get_script_node()->get_position(); - if (is_input()) { - position -= offset; + position += get_graph_node()->get_input_port_position(_pin->get_pin_index()); + position -= Vector2(250, 0); + get_graph()->spawn_node(context, position, callable_mp_lambda(this, [&, this](const Ref& p_node) { p_node->find_pin(0, PD_Output)->link(_pin); @@ -286,7 +282,9 @@ void OrchestratorGraphNodePin::_promote_as_variable() } else { - position += offset + Vector2(25, 0); + position += get_graph_node()->get_output_port_position(_pin->get_pin_index()); + position += Vector2(75, 0); + get_graph()->spawn_node(context, position, callable_mp_lambda(this, [&, this](const Ref& p_node) { _pin->link(p_node->find_pin(1, PD_Input)); @@ -294,18 +292,6 @@ void OrchestratorGraphNodePin::_promote_as_variable() } } -String OrchestratorGraphNodePin::_create_promoted_variable_name() -{ - Orchestration* orchestration = _node->get_script_node()->get_orchestration(); - - int index = 0; - String name = _pin->get_pin_name() + itos(index++); - while (orchestration->has_variable(name)) - name = _pin->get_pin_name() + itos(index++); - - return name; -} - void OrchestratorGraphNodePin::_create_widgets() { _default_value = nullptr; diff --git a/src/editor/graph/graph_node_pin.h b/src/editor/graph/graph_node_pin.h index 655b143e..bc0c9b23 100644 --- a/src/editor/graph/graph_node_pin.h +++ b/src/editor/graph/graph_node_pin.h @@ -221,10 +221,6 @@ class OrchestratorGraphNodePin : public HBoxContainer /// Promote this pin to a variable void _promote_as_variable(); - /// Creates a new variable name - /// @return the variable name - String _create_promoted_variable_name(); - /// Creates the pin's rendered icon /// @param p_visible whether the icon is visible /// @return the icon texture rect diff --git a/src/orchestration/orchestration.cpp b/src/orchestration/orchestration.cpp index e091167f..95ccde1c 100644 --- a/src/orchestration/orchestration.cpp +++ b/src/orchestration/orchestration.cpp @@ -856,6 +856,31 @@ bool Orchestration::can_remove_variable(const StringName& p_name) const return true; } +Ref Orchestration::promote_to_variable(const Ref& p_pin) +{ + int index = 0; + String name = vformat("%s_%d", p_pin->get_pin_name(), index++); + while (has_variable(name)) + name = vformat("%s_%d", p_pin->get_pin_name(), index++); + + Ref variable = create_variable(name); + if (variable.is_valid()) + { + ClassificationParser parser; + if (parser.parse(p_pin->get_property_info())) + variable->set_classification(parser.get_classification()); + + variable->set_default_value(p_pin->get_effective_default_value()); + + variable->emit_changed(); + variable->notify_property_list_changed(); + + _self->emit_signal("variables_changed"); + } + + return variable; +} + bool Orchestration::has_custom_signal(const StringName& p_name) const { return _signals.has(p_name); diff --git a/src/orchestration/orchestration.h b/src/orchestration/orchestration.h index fc884a46..93fdef29 100644 --- a/src/orchestration/orchestration.h +++ b/src/orchestration/orchestration.h @@ -215,6 +215,7 @@ class Orchestration Vector> get_variables() const; PackedStringArray get_variable_names() const; bool can_remove_variable(const StringName& p_name) const; + Ref promote_to_variable(const Ref& p_pin); //~ End Variable Interface //~ Begin Signals Interface diff --git a/src/script/variable.cpp b/src/script/variable.cpp index 61a2e26a..5d6a42ec 100644 --- a/src/script/variable.cpp +++ b/src/script/variable.cpp @@ -22,6 +22,173 @@ #include "common/variant_utils.h" #include "script/script.h" +bool ClassificationParser::parse(const String& p_classification) +{ + _classification = p_classification; + + // Reset property state + _property = PropertyInfo(); + _property.hint = PROPERTY_HINT_NONE; + _property.usage = PROPERTY_USAGE_NO_EDITOR; + + // Make sure classification starts with expected prefixes + if (!_classification.begins_with("type:") + && !_classification.begins_with("enum:") + && !_classification.begins_with("bitfield:") + && !_classification.begins_with("class_enum:") + && !_classification.begins_with("class_bitfield:") + && !_classification.begins_with("class:")) + return false; + + if (_classification.begins_with("type:")) + { + // basic types + const String type_name = _classification.substr(_classification.find(":") + 1); + for (int i = 0; i < Variant::VARIANT_MAX; i++) + { + const Variant::Type type = VariantUtils::to_type(i); + if (Variant::get_type_name(type).match(type_name)) + { + if (_property.type != type) + _convert_default_value = true; + + _property.type = type; + _property.hint = PROPERTY_HINT_NONE; + _property.hint_string = ""; + _property.class_name = ""; + + // These cannot have default values + if (type == Variant::CALLABLE || type == Variant::SIGNAL || type == Variant::RID || type == Variant::NIL) + _property.usage = PROPERTY_USAGE_STORAGE; + else + _property.usage = PROPERTY_USAGE_DEFAULT; + + if (type == Variant::NIL) + _property.usage |= PROPERTY_USAGE_NIL_IS_VARIANT; + + break; + } + } + } + else if (_classification.begins_with("enum:") || _classification.begins_with("bitfield:")) + { + // enum/bitfields + const String name = _classification.substr(_classification.find(":") + 1); + const EnumInfo& enum_info = ExtensionDB::get_global_enum(name); + if (!enum_info.values.is_empty()) + { + PackedStringArray hints; + for (int i = 0; i < enum_info.values.size(); i++) + hints.push_back(enum_info.values[i].name); + + _property.type = Variant::INT; + _property.hint = _classification.begins_with("bitfield:") ? PROPERTY_HINT_FLAGS : PROPERTY_HINT_ENUM; + _property.hint_string = StringUtils::join(",", hints); + _property.class_name = name; + if (_classification.begins_with("bitfield:")) + _property.usage = PROPERTY_USAGE_DEFAULT | PROPERTY_USAGE_CLASS_IS_BITFIELD; + else + _property.usage = PROPERTY_USAGE_DEFAULT | PROPERTY_USAGE_CLASS_IS_ENUM; + } + } + else if (_classification.begins_with("class:")) + { + // class type + const String class_name = _classification.substr(_classification.find(":") + 1); + if (ClassDB::is_parent_class(class_name, "Resource")) + { + _property.type = Variant::OBJECT; + _property.hint = PROPERTY_HINT_RESOURCE_TYPE; + _property.hint_string = class_name; + _property.class_name = class_name; + _property.usage = PROPERTY_USAGE_DEFAULT; + } + else if (ClassDB::is_parent_class(class_name, "Node")) + { + _property.type = Variant::OBJECT; + _property.hint = PROPERTY_HINT_NODE_TYPE; + _property.hint_string = class_name; + _property.class_name = class_name; + _property.usage = PROPERTY_USAGE_DEFAULT; + } + else + { + _property.type = Variant::OBJECT; + _property.hint = PROPERTY_HINT_NONE; + _property.hint_string = ""; + _property.class_name = class_name; + _property.usage = PROPERTY_USAGE_NO_EDITOR; + } + } + else if (_classification.begins_with("class_enum:") || _classification.begins_with("class_bitfield:")) + { + const String class_enum_name = _classification.substr(_classification.find(":") + 1); + const String class_name = class_enum_name.substr(0, class_enum_name.find(".")); + const String enum_name = class_enum_name.substr(class_enum_name.find(".") + 1); + const String hint_string = StringUtils::join(",", ClassDB::class_get_enum_constants(class_name, enum_name, true)); + const bool bitfield = _classification.begins_with("class_bitfield:"); + + _property.type = Variant::INT; + _property.hint = bitfield ? PROPERTY_HINT_FLAGS : PROPERTY_HINT_ENUM; + _property.hint_string = hint_string; + _property.class_name = class_enum_name; + _property.usage = PROPERTY_USAGE_DEFAULT | (bitfield ? PROPERTY_USAGE_CLASS_IS_BITFIELD : PROPERTY_USAGE_CLASS_IS_ENUM); + } + else if (_classification.begins_with("custom_enum:") || _classification.begins_with("custom_bitfield:")) + { + const bool bitfield = _classification.begins_with("custom_bitfield:"); + _property.type = Variant::INT; + _property.hint = bitfield ? PROPERTY_HINT_FLAGS : PROPERTY_HINT_ENUM; + _property.hint_string = ""; + _property.class_name = ""; + _property.usage = PROPERTY_USAGE_NO_EDITOR; + } + return true; +} + +bool ClassificationParser::parse(const PropertyInfo& p_property) +{ + _property = p_property; + + // Reset classification + _classification = ""; + + if (PropertyUtils::is_class_enum(_property)) + { + // Class enum type + _classification = vformat("class_enum:%s", _property.class_name); + } + else if (PropertyUtils::is_class_bitfield(_property)) + { + // Class bitfield type + _classification = vformat("class_bitfield:%s", _property.class_name); + } + else if (PropertyUtils::is_enum(_property)) + { + // Enum + _classification = vformat("enum:%s", _property.class_name); + } + else if (PropertyUtils::is_bitfield(_property)) + { + // Bitfield + _classification = vformat("bitfield:%s", _property.class_name); + } + else if (PropertyUtils::is_class(_property)) + { + // Class + _classification = vformat("class:%s", _property.class_name); + } + else + { + // Basic type + _classification = vformat("type:%s", Variant::get_type_name(_property.type)); + } + + return true; +} + +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + void OScriptVariable::_bind_methods() { // This is read-only to avoid name changes in the inspector, which creates cache issues with the owning script @@ -233,123 +400,21 @@ void OScriptVariable::set_classification(const String& p_classification) { _classification = p_classification; - if (_classification.contains(":")) + ClassificationParser parser; + if (parser.parse(_classification)) { - if (_classification.begins_with("type:")) - { - // basic types - const String type_name = _classification.substr(_classification.find(":") + 1); - for (int i = 0; i < Variant::VARIANT_MAX; i++) - { - const Variant::Type type = VariantUtils::to_type(i); - if (Variant::get_type_name(type).match(type_name)) - { - if (_info.type != type) - _convert_default_value(type); - - _info.type = type; - _info.hint = PROPERTY_HINT_NONE; - _info.hint_string = ""; - _info.class_name = ""; - - // These cannot have default values - if (type == Variant::CALLABLE || type == Variant::SIGNAL || type == Variant::RID || type == Variant::NIL) - _info.usage = PROPERTY_USAGE_STORAGE; - else - _info.usage = PROPERTY_USAGE_DEFAULT; - - if (type == Variant::NIL) - _info.usage |= PROPERTY_USAGE_NIL_IS_VARIANT; - - break; - } - } - } - else if (_classification.begins_with("enum:") || _classification.begins_with("bitfield:")) - { - // enum/bitfields - const String name = _classification.substr(_classification.find(":") + 1); - const EnumInfo& enum_info = ExtensionDB::get_global_enum(name); - if (!enum_info.values.is_empty()) - { - PackedStringArray hints; - for (int i = 0; i < enum_info.values.size(); i++) - hints.push_back(enum_info.values[i].name); - - _info.type = Variant::INT; - _info.hint = _classification.begins_with("bitfield:") ? PROPERTY_HINT_FLAGS : PROPERTY_HINT_ENUM; - _info.hint_string = StringUtils::join(",", hints); - _info.class_name = name; - if (_classification.begins_with("bitfield:")) - _info.usage = PROPERTY_USAGE_DEFAULT | PROPERTY_USAGE_CLASS_IS_BITFIELD; - else - _info.usage = PROPERTY_USAGE_DEFAULT | PROPERTY_USAGE_CLASS_IS_ENUM; - } - } - else if (_classification.begins_with("class:")) - { - // class type - const String class_name = _classification.substr(_classification.find(":") + 1); - if (ClassDB::is_parent_class(class_name, "Resource")) - { - _info.type = Variant::OBJECT; - _info.hint = PROPERTY_HINT_RESOURCE_TYPE; - _info.hint_string = class_name; - _info.class_name = ""; - _info.usage = PROPERTY_USAGE_DEFAULT; - } - else if (ClassDB::is_parent_class(class_name, "Node")) - { - _info.type = Variant::OBJECT; - _info.hint = PROPERTY_HINT_NODE_TYPE; - _info.hint_string = class_name; - _info.class_name = class_name; - _info.usage = PROPERTY_USAGE_DEFAULT; - } - else - { - _info.type = Variant::OBJECT; - _info.hint = PROPERTY_HINT_NONE; - _info.hint_string = ""; - _info.class_name = class_name; - _info.usage = PROPERTY_USAGE_NO_EDITOR; - } - } - else if (_classification.begins_with("class_enum:") || _classification.begins_with("class_bitfield:")) - { - const String class_enum_name = _classification.substr(_classification.find(":") + 1); - const String class_name = class_enum_name.substr(0, class_enum_name.find(".")); - const String enum_name = class_enum_name.substr(class_enum_name.find(".") + 1); - const String hint_string = StringUtils::join(",", ClassDB::class_get_enum_constants(class_name, enum_name, true)); - const bool bitfield = _classification.begins_with("class_bitfield:"); - - _info.type = Variant::INT; - _info.hint = bitfield ? PROPERTY_HINT_FLAGS : PROPERTY_HINT_ENUM; - _info.hint_string = hint_string; - _info.class_name = class_enum_name; - _info.usage = PROPERTY_USAGE_DEFAULT | (bitfield ? PROPERTY_USAGE_CLASS_IS_BITFIELD : PROPERTY_USAGE_CLASS_IS_ENUM); - } - else if (_classification.begins_with("custom_enum:") || _classification.begins_with("custom_bitfield:")) - { - const bool bitfield = _classification.begins_with("custom_bitfield:"); - _info.type = Variant::INT; - _info.hint = bitfield ? PROPERTY_HINT_FLAGS : PROPERTY_HINT_ENUM; - _info.hint_string = ""; - _info.class_name = ""; - _info.usage = PROPERTY_USAGE_NO_EDITOR; - } - } - else - { - _info.type = Variant::NIL; - _info.hint = PROPERTY_HINT_NONE; - _info.hint_string = ""; - _info.class_name = ""; - _info.usage = PROPERTY_USAGE_NO_EDITOR; // no default value + if (parser.is_default_value_converted()) + _convert_default_value(parser.get_property().type); } + const PropertyInfo property = parser.get_property(); + _info.type = property.type; + _info.hint = property.hint; + _info.hint_string = property.hint_string; + _info.class_name = property.class_name; + _info.usage = property.usage | PROPERTY_USAGE_SCRIPT_VARIABLE; + _exportable = _is_exportable_type(_info); - _info.usage |= PROPERTY_USAGE_SCRIPT_VARIABLE; notify_property_list_changed(); emit_changed(); diff --git a/src/script/variable.h b/src/script/variable.h index e84489c8..4631c313 100644 --- a/src/script/variable.h +++ b/src/script/variable.h @@ -24,6 +24,38 @@ using namespace godot; /// Forward declarations class Orchestration; +/// Utility class for converting between PropertyInfo and classification strings +class ClassificationParser +{ +protected: + PropertyInfo _property; + String _classification; + bool _convert_default_value{ false }; + +public: + /// Parses a classification string + /// @param p_classification the classification to parse + /// @return true if the parse was successful, false otherwise + bool parse(const String& p_classification); + + /// Parses a property structure + /// @param p_property the property + /// /// @return true if the parse was successful, false otherwise + bool parse(const PropertyInfo& p_property); + + /// Get the classification as a string + /// @return the string representation of the classification + String get_classification() const { return _classification; } + + /// Get the property structure + /// @return the property representation of the classification + PropertyInfo get_property() const { return _property; } + + /// Return whether the default value is converted + /// @return true if the default value is converted, false otherwise + bool is_default_value_converted() const { return _convert_default_value; } +}; + /// Defines a script variable /// /// Variables are defined as resources which provides multiple benefits. First, it allows