GH-662 Exclude loop-based pins from return value check

CraterCrash · Aug 4, 2024 · d207236 · d207236
1 parent bc089e4
commit d207236
Show file tree

Hide file tree

Showing 8 changed files with 26 additions and 11 deletions.
diff --git a/src/script/node.h b/src/script/node.h
@@ -387,6 +387,9 @@ class OScriptNode : public Resource
     /// @return if the node can be duplicated
     virtual bool can_duplicate() const { return true; }
 
+    /// Return whether the specified port is a loop-based port
+    virtual bool is_loop_port(int p_port) const { return false; }
+
 protected:
     /// Notify that node pins have been changed.
     void _notify_pins_changed();

diff --git a/src/script/nodes/flow_control/for.cpp b/src/script/nodes/flow_control/for.cpp
@@ -175,6 +175,12 @@ String OScriptNodeForLoop::get_icon() const
     return "Loop";
 }
 
+bool OScriptNodeForLoop::is_loop_port(int p_port) const
+{
+    // Body & Index
+    return p_port <= 1;
+}
+
 void OScriptNodeForLoop::get_actions(List<Ref<OScriptAction>>& p_action_list)
 {
     if (_with_break)

diff --git a/src/script/nodes/flow_control/for.h b/src/script/nodes/flow_control/for.h
@@ -52,6 +52,7 @@ class OScriptNodeForLoop : public OScriptNode
     String get_node_title_color_name() const override { return "flow_control"; }
     String get_icon() const override;
     PackedStringArray get_keywords() const override { return Array::make("for", "loop"); }
+    bool is_loop_port(int p_port) const override;
     void get_actions(List<Ref<OScriptAction>>& p_action_list) override;
     OScriptNodeInstance* instantiate() override;
     void initialize(const OScriptNodeInitContext& p_context) override;

diff --git a/src/script/nodes/flow_control/for_each.cpp b/src/script/nodes/flow_control/for_each.cpp
@@ -164,6 +164,12 @@ String OScriptNodeForEach::get_icon() const
     return "Loop";
 }
 
+bool OScriptNodeForEach::is_loop_port(int p_port) const
+{
+    // Body, Index, Element
+    return p_port <= 2;
+}
+
 void OScriptNodeForEach::get_actions(List<Ref<OScriptAction>>& p_action_list)
 {
     if (_with_break)

diff --git a/src/script/nodes/flow_control/for_each.h b/src/script/nodes/flow_control/for_each.h
@@ -48,6 +48,7 @@ class OScriptNodeForEach : public OScriptNode
     String get_node_title_color_name() const override { return "flow_control"; }
     String get_icon() const override;
     PackedStringArray get_keywords() const override { return Array::make("for", "each", "loop"); }
+    bool is_loop_port(int p_port) const override;
     void get_actions(List<Ref<OScriptAction>>& p_action_list) override;
     OScriptNodeInstance* instantiate() override;
     void initialize(const OScriptNodeInitContext& p_context) override;

diff --git a/src/script/nodes/flow_control/while.cpp b/src/script/nodes/flow_control/while.cpp
@@ -63,6 +63,12 @@ String OScriptNodeWhile::get_icon() const
     return "Loop";
 }
 
+bool OScriptNodeWhile::is_loop_port(int p_port) const
+{
+    // Repeat
+    return p_port <= 0;
+}
+
 OScriptNodeInstance* OScriptNodeWhile::instantiate()
 {
     OScriptNodeWhileInstance* i = memnew(OScriptNodeWhileInstance);

diff --git a/src/script/nodes/flow_control/while.h b/src/script/nodes/flow_control/while.h
@@ -36,6 +36,7 @@ class OScriptNodeWhile : public OScriptNode
     String get_node_title_color_name() const override { return "flow_control"; }
     String get_icon() const override;
     PackedStringArray get_keywords() const override { return Array::make("while", "loop"); }
+    bool is_loop_port(int p_port) const override;
     OScriptNodeInstance* instantiate() override;
     void initialize(const OScriptNodeInitContext& p_context) override;
     //~ End OScriptNode Interface

diff --git a/src/script/nodes/functions/function_result.cpp b/src/script/nodes/functions/function_result.cpp
@@ -17,9 +17,6 @@
 #include "function_result.h"
 
 #include "common/property_utils.h"
-#include "script/nodes/flow_control/for.h"
-#include "script/nodes/flow_control/for_each.h"
-#include "script/nodes/flow_control/while.h"
 
 class OScriptNodeFunctionResultInstance : public OScriptNodeInstance
 {
@@ -140,13 +137,7 @@ void OScriptNodeFunctionResult::validate_node_during_build(BuildLog& p_log) cons
                     const Ref<OScriptNode> source = graph_nodes[E.from_node];
                     if (source.is_valid())
                     {
-                        Ref<OScriptNodeForEach> for_each_node = source;
-                        Ref<OScriptNodeForLoop> for_loop_node = source;
-                        Ref<OScriptNodeWhile> while_node = source;
-
-                        if ((for_each_node.is_valid() && E.from_port <= 2)
-                            || (for_loop_node.is_valid() && E.from_port <= 1)
-                            || (while_node.is_valid() && E.from_port <= 0))
+                        if (source->is_loop_port(E.from_port))
                             skipped.insert(E.to_node);
                     }
 
@@ -172,7 +163,7 @@ void OScriptNodeFunctionResult::validate_node_during_build(BuildLog& p_log) cons
                 {
                     for (const Ref<OScriptNodePin>& output : node->find_pins(PD_Output))
                     {
-                        if (output.is_valid() && output->is_execution() && !output->has_any_connections())
+                        if (output.is_valid() && output->is_execution() && !output->has_any_connections() && !node->is_loop_port(output->get_pin_index()))
                             p_log.error(node.ptr(), output, "This pin should be connected to the return node.");
                     }
                 }