GH-607 Improve changing variable types

- Add confirmation dialog with warning
- Attempt to coerce most common types between one another
- Retain VariableSet values if they can be converted
- Break any connections with Variable Get/Set if types are incompatible

src/common/variant_utils.cpp

@@ -120,4 +120,179 @@ namespace VariantUtils
 
         return UtilityFunctions::type_convert(Variant(), p_type);
     }
+
+    Variant convert(const Variant& p_value, Variant::Type p_target_type)
+    {
+        if (Variant::can_convert(p_value.get_type(), p_target_type))
+            return UtilityFunctions::type_convert(p_value, p_target_type);
+
+        const Variant::Type type = p_value.get_type();
+
+        if (p_target_type == Variant::BOOL)
+        {
+            if (type == Variant::VECTOR2 || type == Variant::VECTOR2I)
+                return p_value.operator Vector2().x;
+
+            if (type == Variant::VECTOR3 || type == Variant::VECTOR3I)
+                return p_value.operator Vector3().x;
+
+            if (type == Variant::VECTOR4 || type == Variant::VECTOR4I)
+                return p_value.operator Vector4().x;
+
+            if (type == Variant::INT || type == Variant::FLOAT)
+                return p_value.operator int64_t();
+
+            if (type == Variant::STRING || type == Variant::STRING_NAME)
+            {
+                const String value = p_value;
+                return value.to_lower().match("true") || value.strip_edges() == "1";
+            }
+        }
+
+        if (p_target_type == Variant::INT || p_target_type == Variant::FLOAT)
+        {
+            if (type == Variant::VECTOR2 || type == Variant::VECTOR2I)
+                return p_value.operator Vector2().x;
+
+            if (type == Variant::VECTOR3 || type == Variant::VECTOR3I)
+                return p_value.operator Vector3().x;
+
+            if (type == Variant::VECTOR4 || type == Variant::VECTOR4I)
+                return p_value.operator Vector4().x;
+
+            if (type == Variant::STRING || type == Variant::STRING_NAME)
+            {
+                String value = p_value;
+                if (value.begins_with("(") && value.ends_with(")"))
+                {
+                    value = value.substr(1, value.length() - 1);
+                    convert(value.split(",")[0], p_target_type);
+                }
+                else if (value.to_lower().match("true"))
+                    return convert(true, p_target_type);
+                else if (value.strip_edges().match("1"))
+                    return convert(true, p_target_type);
+            }
+        }
+
+        if (p_target_type == Variant::VECTOR2 || p_target_type == Variant::VECTOR2I)
+        {
+            if (type == Variant::BOOL || type == Variant::INT || type == Variant::FLOAT)
+                return Vector2(p_value, p_value);
+
+            if (type == Variant::STRING || type == Variant::STRING_NAME)
+            {
+                String value = p_value;
+                if (value.begins_with("(") && value.ends_with(")"))
+                {
+                    value = value.substr(1, value.length() - 1);
+
+                    Vector2 result;
+                    const PackedStringArray parts = value.split(",");
+                    for (int i = 0; i < parts.size() && i < 2; i++)
+                        result[i] = parts[i].to_float();
+                    return result;
+                }
+                else if (value.to_lower().match("true"))
+                    return convert(true, p_target_type);
+                else if (value.strip_edges().match("1"))
+                    return convert(true, p_target_type);
+            }
+
+            if (type == Variant::VECTOR3 || type == Variant::VECTOR3I)
+            {
+                const Vector3 v3 = p_value;
+                return Vector2(v3.x, v3.y);
+            }
+
+            if (type == Variant::VECTOR4 || type == Variant::VECTOR4I)
+            {
+                const Vector4 v4 = p_value;
+                return Vector2(v4.x, v4.y);
+            }
+        }
+
+        if (p_target_type == Variant::VECTOR3 || p_target_type == Variant::VECTOR3I)
+        {
+            if (type == Variant::INT || type == Variant::FLOAT || type == Variant::BOOL)
+                return Vector3(p_value, p_value, p_value);
+
+            if (type == Variant::STRING || type == Variant::STRING_NAME)
+            {
+                String value = p_value;
+                if (value.begins_with("(") && value.ends_with(")"))
+                {
+                    value = value.substr(1, value.length() - 1);
+
+                    Vector3 result;
+                    const PackedStringArray parts = value.split(",");
+                    for (int i = 0; i < parts.size() && i < 3; i++)
+                        result[i] = parts[i].to_float();
+                    return result;
+                }
+                else if (value.to_lower().match("true"))
+                    return convert(true, p_target_type);
+                else if (value.strip_edges().match("1"))
+                    return convert(true, p_target_type);
+            }
+
+            if (type == Variant::VECTOR2 || type == Variant::VECTOR2I)
+            {
+                const Vector2 v2 = p_value;
+                return Vector3(v2.x, v2.y, 0);
+            }
+
+            if (type == Variant::VECTOR4 || type == Variant::VECTOR4I)
+            {
+                const Vector4 v4 = p_value;
+                return Vector3(v4.x, v4.y, v4.z);
+            }
+        }
+
+        if (p_target_type == Variant::VECTOR4 || p_target_type == Variant::VECTOR4I)
+        {
+            if (type == Variant::INT || type == Variant::FLOAT || type == Variant::BOOL)
+                return Vector4(p_value, p_value, p_value, p_value);
+
+            if (type == Variant::STRING || type == Variant::STRING_NAME)
+            {
+                String value = p_value;
+                if (value.begins_with("(") && value.ends_with(")"))
+                {
+                    value = value.substr(1, value.length() - 1);
+
+                    Vector4 result;
+                    const PackedStringArray parts = value.split(",");
+                    for (int i = 0; i < parts.size() && i < 4; i++)
+                        result[i] = parts[i].to_float();
+                    return result;
+                }
+                else if (value.to_lower().match("true"))
+                    return convert(true, p_target_type);
+                else if (value.strip_edges().match("1"))
+                    return convert(true, p_target_type);
+            }
+
+            if (type == Variant::VECTOR2 || type == Variant::VECTOR2I)
+            {
+                const Vector2 v2 = p_value;
+                return Vector4(v2.x, v2.y, 0, 0);
+            }
+
+            if (type == Variant::VECTOR3 || type == Variant::VECTOR3I)
+            {
+                const Vector3 v3 = p_value;
+                return Vector4(v3.x, v3.y, v3.z, 0);
+            }
+        }
+
+        if (p_target_type == Variant::STRING_NAME)
+            return StringName(convert(p_value, Variant::STRING));
+
+        if (p_value.get_type() == Variant::STRING_NAME)
+            return convert(String(p_value), p_target_type);
+
+        return make_default(p_target_type);
+    }
+
 }

src/common/variant_utils.h

@@ -57,6 +57,12 @@ namespace VariantUtils
     /// @return the default Variant value of the specified type
     Variant make_default(Variant::Type p_type);
 
+    /// Converts the value to the specified type
+    /// @param p_value the value to convert
+    /// @param p_target_type the target type
+    /// @return the converted type
+    Variant convert(const Variant& p_value, Variant::Type p_target_type);
+
     /// Cast to a desired type.
     /// @param p_value the value to be cast
     /// @param T the cast type

src/editor/inspector/property_type_button_property.cpp

@@ -16,6 +16,7 @@
 //
 #include "editor/inspector/property_type_button_property.h"
 
+#include "common/callable_lambda.h"
 #include "common/scene_utils.h"
 #include "editor/select_type_dialog.h"
 
@@ -27,8 +28,28 @@ void OrchestratorEditorPropertyVariableClassification::_property_selected()
 void OrchestratorEditorPropertyVariableClassification::_search_selected()
 {
     _selected_name = _dialog->get_selected_type();
-    emit_changed(get_edited_property(), _selected_name);
-    update_property();
+    if (get_edited_object()->get(get_edited_property()) != _selected_name)
+    {
+        ConfirmationDialog* confirm = memnew(ConfirmationDialog);
+        confirm->set_text("This could break connections and reset default values on variable set nodes.\nDo you want to change the variable type?");
+        confirm->set_title("Change Variable Type");
+        confirm->set_ok_button_text("Change Variable Type");
+        add_child(confirm);
+
+        confirm->connect("confirmed", callable_mp_lambda(this, [confirm, this] {
+            emit_changed(get_edited_property(), _selected_name);
+            update_property();
+            if (confirm)
+                confirm->queue_free();
+        }));
+
+        confirm->connect("canceled", callable_mp_lambda(this, [confirm, this] {
+            if (confirm)
+                confirm->queue_free();
+        }));
+
+        confirm->popup_centered();
+    }
 }
 
 void OrchestratorEditorPropertyVariableClassification::_update_property()

src/script/node_pin.cpp

@@ -507,7 +507,7 @@ bool OScriptNodePin::can_accept(const Ref<OScriptNodePin>& p_pin) const
     }
 
     // Coercion is allowed here
-    if (_property.type == Variant::STRING || p_pin->get_type() == Variant::STRING)
+    if (_property.type == Variant::STRING)
     {
         // File targets should only accept string sources
         if (_property.hint == PROPERTY_HINT_FILE)

src/script/nodes/variables/variable.cpp

@@ -53,6 +53,9 @@ void OScriptNodeVariable::_on_variable_changed()
     {
         _variable_name = _variable->get_variable_name();
         reconstruct_node();
+
+        // This must be triggered after reconstruction
+        _variable_changed();
     }
 }
 

src/script/nodes/variables/variable.h

@@ -38,6 +38,9 @@ class OScriptNodeVariable : public OScriptNode
     /// Called when the script variable is modified
     void _on_variable_changed();
 
+    /// Allows subclasses to handle variable changed 
+    virtual void _variable_changed() { }
+
 public:
     OScriptNodeVariable();
 

src/script/nodes/variables/variable_get.cpp

@@ -62,6 +62,22 @@ void OScriptNodeVariableGet::_upgrade(uint32_t p_version, uint32_t p_current_ver
     super::_upgrade(p_version, p_current_version);
 }
 
+void OScriptNodeVariableGet::_variable_changed()
+{
+    if (_is_in_editor())
+    {
+        Ref<OScriptNodePin> output = find_pin("value", PD_Output);
+        if (output.is_valid() && output->has_any_connections())
+        {
+            Ref<OScriptNodePin> target = output->get_connections()[0];
+            if (target.is_valid() && !target->can_accept(output))
+                output->unlink_all();
+        }
+    }
+
+    super::_variable_changed();
+}
+
 void OScriptNodeVariableGet::allocate_default_pins()
 {
     create_pin(PD_Output, PT_Data, PropertyUtils::as("value", _variable->get_info()))->set_label(_variable_name, false);

src/script/nodes/variables/variable_get.h

@@ -30,6 +30,10 @@ class OScriptNodeVariableGet : public OScriptNodeVariable
     void _upgrade(uint32_t p_version, uint32_t p_current_version) override;
     //~ End OScriptNode Interface
 
+    //~ Begin OScriptNodeVariable Interface
+    void _variable_changed() override;
+    //~ End OScriptNodeVariable Interface
+
 public:
     //~ Begin OScriptNode Interface
     void allocate_default_pins() override;

src/script/nodes/variables/variable_set.cpp

@@ -75,10 +75,34 @@ void OScriptNodeVariableSet::_upgrade(uint32_t p_version, uint32_t p_current_ver
     super::_upgrade(p_version, p_current_version);
 }
 
+void OScriptNodeVariableSet::_variable_changed()
+{
+    if (_is_in_editor())
+    {
+        Ref<OScriptNodePin> input = find_pin(1, PD_Input);
+        if (input.is_valid() && input->has_any_connections())
+        {
+            Ref<OScriptNodePin> source = input->get_connections()[0];
+            if (!input->can_accept(source))
+                input->unlink_all();
+        }
+
+        Ref<OScriptNodePin> output = find_pin("value", PD_Output);
+        if (output.is_valid() && output->has_any_connections())
+        {
+            Ref<OScriptNodePin> target = output->get_connections()[0];
+            if (!target->can_accept(output))
+                output->unlink_all();
+        }
+    }
+
+    super::_variable_changed();
+}
+
 void OScriptNodeVariableSet::allocate_default_pins()
 {
     create_pin(PD_Input, PT_Execution, PropertyUtils::make_exec("ExecIn"));
-    create_pin(PD_Input, PT_Data, _variable->get_info(), _variable->get_default_value())->no_pretty_format();
+    create_pin(PD_Input, PT_Data, _variable->get_info())->no_pretty_format();
 
     create_pin(PD_Output, PT_Execution, PropertyUtils::make_exec("ExecOut"));
     create_pin(PD_Output, PT_Data, PropertyUtils::as("value", _variable->get_info()))->hide_label();
@@ -99,6 +123,27 @@ String OScriptNodeVariableSet::get_node_title() const
     return vformat("Set %s", _variable->get_variable_name());
 }
 
+void OScriptNodeVariableSet::reallocate_pins_during_reconstruction(const Vector<Ref<OScriptNodePin>>& p_old_pins)
+{
+    super::reallocate_pins_during_reconstruction(p_old_pins);
+
+    // Keep old default value if one was set that differs from the variable's default value
+    for (const Ref<OScriptNodePin>& old_pin : p_old_pins)
+    {
+        if (old_pin->is_input() && !old_pin->is_execution())
+        {
+            if (old_pin->get_effective_default_value() != _variable->get_default_value())
+            {
+                Ref<OScriptNodePin> value_pin = find_pin(_variable->get_variable_name(), PD_Input);
+                if (value_pin.is_valid() && !value_pin->has_any_connections())
+                    value_pin->set_default_value(VariantUtils::convert(old_pin->get_effective_default_value(), value_pin->get_type()));
+
+                break;
+            }
+        }
+    }
+}
+
 OScriptNodeInstance* OScriptNodeVariableSet::instantiate()
 {
     OScriptNodeVariableSetInstance *i = memnew(OScriptNodeVariableSetInstance);

src/script/nodes/variables/variable_set.h

@@ -30,11 +30,16 @@ class OScriptNodeVariableSet : public OScriptNodeVariable
     void _upgrade(uint32_t p_version, uint32_t p_current_version) override;
     //~ End OScriptNode Interface
 
+    //~ Begin OScriptNodeVariable Interface
+    void _variable_changed() override;
+    //~ End OScriptNodeVariable Interface
+
 public:
     //~ Begin OScriptNode Interface
     void allocate_default_pins() override;
     String get_tooltip_text() const override;
     String get_node_title() const override;
+    void reallocate_pins_during_reconstruction(const Vector<Ref<OScriptNodePin>>& p_old_pins) override;
     OScriptNodeInstance* instantiate() override;
     //~ End OScriptNode Interface
 };

src/script/variable.cpp

@@ -163,6 +163,12 @@ bool OScriptVariable::_is_exportable_type(const PropertyInfo& p_property) const
     return true;
 }
 
+bool OScriptVariable::_convert_default_value(Variant::Type p_new_type)
+{
+    set_default_value(VariantUtils::convert(get_default_value(), p_new_type));
+    return true;
+}
+
 OScriptVariable::OScriptVariable()
 {
     _info.type = Variant::NIL;
@@ -224,7 +230,7 @@ void OScriptVariable::set_classification(const String& p_classification)
                     if (Variant::get_type_name(type).match(type_name))
                     {
                         if (_info.type != type)
-                            set_default_value(VariantUtils::make_default(type));
+                            _convert_default_value(type);
 
                         _info.type = type;
                         _info.hint = PROPERTY_HINT_NONE;