diff --git a/src/DynamoCore/Graph/Nodes/NodeModel.cs b/src/DynamoCore/Graph/Nodes/NodeModel.cs index a6b767e7ccb..894c482f873 100644 --- a/src/DynamoCore/Graph/Nodes/NodeModel.cs +++ b/src/DynamoCore/Graph/Nodes/NodeModel.cs @@ -1643,15 +1643,7 @@ public void UseLevelAndReplicationGuide(List inputs) if (inputs == null || !inputs.Any()) return; - // Variadic nodes pack tail inputs into a single argument, so per-port - // level/lacing handling for the variadic slots must happen before packing. - // The hook lets such nodes scope this post-pack pass to the non-variadic - // prefix, preventing double-wrap. - int boundedCount = Math.Min(inputs.Count, LevelAndReplicationGuideInputCount(inputs.Count)); - if (boundedCount <= 0) - return; - - for (int i = 0; i < boundedCount; i++) + for (int i = 0; i < inputs.Count; i++) { if (InPorts[i].UseLevels) { @@ -1662,7 +1654,7 @@ public void UseLevelAndReplicationGuide(List inputs) switch (ArgumentLacing) { case LacingStrategy.Auto: - for (int i = 0; i < boundedCount; ++i) + for (int i = 0; i < inputs.Count; ++i) { if (InPorts[i].UseLevels) inputs[i] = AstFactory.AddReplicationGuide(inputs[i], new List { 1 }, false); @@ -1670,7 +1662,7 @@ public void UseLevelAndReplicationGuide(List inputs) break; case LacingStrategy.Shortest: - for (int i = 0; i < boundedCount; ++i) + for (int i = 0; i < inputs.Count; ++i) { inputs[i] = AstFactory.AddReplicationGuide(inputs[i], new List { 1 }, false); } @@ -1679,7 +1671,7 @@ public void UseLevelAndReplicationGuide(List inputs) case LacingStrategy.Longest: - for (int i = 0; i < boundedCount; ++i) + for (int i = 0; i < inputs.Count; ++i) { inputs[i] = AstFactory.AddReplicationGuide(inputs[i], new List { 1 }, true); } @@ -1688,7 +1680,7 @@ public void UseLevelAndReplicationGuide(List inputs) case LacingStrategy.CrossProduct: int guide = 1; - for (int i = 0; i < boundedCount; ++i) + for (int i = 0; i < inputs.Count; ++i) { inputs[i] = AstFactory.AddReplicationGuide(inputs[i], new List { guide }, false); guide++; @@ -1696,21 +1688,6 @@ public void UseLevelAndReplicationGuide(List inputs) break; } } - - /// - /// Returns the number of leading inputs that - /// should wrap with AtLevel / replication guides. - /// - /// - /// Variadic nodes (e.g. DSVarArgFunction) pack their tail inputs into a single - /// packed argument before this post-pack pass runs. Wrapping the packed slot here would - /// be either a no-op or a double-wrap, so variadic nodes override this hook to expose only - /// the non-variadic prefix and handle per-port level / lacing on the unpacked variadic - /// inputs themselves. - /// - /// Number of raw input AST nodes available to the pass. - /// Number of inputs (counted from index 0) that should be wrapped. - protected virtual int LevelAndReplicationGuideInputCount(int rawInputCount) => rawInputCount; #endregion #region Input and Output Connections diff --git a/src/DynamoCore/Graph/Nodes/ZeroTouch/DSVarArgFunction.cs b/src/DynamoCore/Graph/Nodes/ZeroTouch/DSVarArgFunction.cs index c2bb4866245..d3ca40aefb3 100644 --- a/src/DynamoCore/Graph/Nodes/ZeroTouch/DSVarArgFunction.cs +++ b/src/DynamoCore/Graph/Nodes/ZeroTouch/DSVarArgFunction.cs @@ -7,6 +7,7 @@ using Dynamo.Library; using Newtonsoft.Json; using ProtoCore.AST.AssociativeAST; +using ProtoCore.Utils; namespace Dynamo.Graph.Nodes.ZeroTouch { @@ -99,25 +100,6 @@ internal override bool HandleModelEventCore(string eventName, int value, UndoRed [JsonIgnore] public VariableInputNodeController VarInputController { get; private set; } - /// - /// Scopes to the non-variadic - /// prefix of this node's inputs. - /// - /// - /// Variadic zero-touch functions pack their tail inputs into a single - /// ExprListNode argument before - /// runs. AtLevel / replication-guide annotations on an ExprListNode are inert - /// at runtime, so per-port Use Levels and lacing on the variadic slots must be applied - /// pre-pack inside ZeroTouchVarArgNodeController.BuildOutputAst. This override - /// limits the post-pack pass to the non-variadic prefix so the packed variadic slot - /// is not touched twice. - /// - protected override int LevelAndReplicationGuideInputCount(int rawInputCount) - { - int prefix = Controller.Definition.Parameters.Count() - 1; - return prefix < 0 ? 0 : prefix; - } - #region VarInput Controller private sealed class ZeroTouchVarInputController : VariableInputNodeController { @@ -185,99 +167,109 @@ protected override void InitializeFunctionParameters(NodeModel model, IEnumerabl protected override void BuildOutputAst(NodeModel model, List inputAstNodes, List resultAst) { - // All inputs are provided, then we should pack all inputs that - // belong to variable input parameter into a single array. + // A variadic zero-touch function foo(x1, ..., xn, params y) is invoked at + // runtime as foo(x1, ..., xn, {y1, ..., ym}) -- every variadic input is + // packed into a single array argument. Suppose paramCount == n + 1 and the + // node inputs are i1, ..., in, y1, ..., ym. if (!model.IsPartiallyApplied) { - var paramCount = Definition.Parameters.Count(); + int variadicStart = Definition.Parameters.Count() - 1; - // Suppose a function foo() with var args, its signature is: - // - // foo(x1, x2, ..., xn, params y) - // - // so paramCount == n + 1 here, and suppose inputs are - // - // i1, i2, ...., in, y1, y2, ..., ym - // - // Why: per-port Use Levels and lacing-driven replication guides - // on the variadic inputs (y1..ym) must be applied *before* we - // pack them into an ExprListNode. AtLevel / replication-guide - // annotations carried inside an ExprListNode are inert at runtime, - // so packing first would silently drop per-port settings on every - // variadic port (the original DYN-10572 defect). The non-variadic - // prefix (i1..in) is handled by the post-pack - // NodeModel.UseLevelAndReplicationGuide pass, which DSVarArgFunction - // scopes to indices [0, paramCount - 1) via the - // LevelAndReplicationGuideInputCount override -- preventing the - // packed slot from being wrapped twice. - int variadicStart = paramCount - 1; - if (variadicStart >= 0 && variadicStart < inputAstNodes.Count) + // When any port uses levels, per-port replication must be honored. AtLevel / + // replication-guide annotations are only consumed at function-call argument + // positions and are inert inside the packed ExprList literal, and DesignScript + // cannot call a params method with separate arguments. So we route through a + // generated wrapper whose formal parameters are the individual ports: the + // per-port AtLevel and the node's lacing replication guides land on the + // wrapper-call arguments (replicating exactly like a non-variadic node), and + // the wrapper body packs the already-replicated values per invocation. See + // DYN-10572. Graphs with no Use Levels keep the original pack-then-call path, + // so their behavior (including their lacing) is unchanged. + if (variadicStart >= 0 && variadicStart < inputAstNodes.Count + && AnyPortUsesLevels(model, inputAstNodes.Count) + && TryBuildReplicatingOutputAst(model, inputAstNodes, resultAst, variadicStart)) { - for (int i = variadicStart; i < inputAstNodes.Count; i++) - { - if (model.InPorts[i].UseLevels) - { - inputAstNodes[i] = AstFactory.AddAtLevel( - inputAstNodes[i], - -model.InPorts[i].Level, - model.InPorts[i].KeepListStructure); - } - } + return; + } - switch (model.ArgumentLacing) - { - case LacingStrategy.Auto: - for (int i = variadicStart; i < inputAstNodes.Count; i++) - { - if (model.InPorts[i].UseLevels) - { - inputAstNodes[i] = AstFactory.AddReplicationGuide( - inputAstNodes[i], new List { 1 }, false); - } - } - break; + // Default path: pack all var arguments into an array {y1, ..., ym} + // (skipping the first n == paramCount - 1 inputs). + var argPack = AstFactory.BuildExprList(inputAstNodes.Skip(variadicStart).ToList()); + inputAstNodes = inputAstNodes.Take(variadicStart).ToList(); + inputAstNodes.Add(argPack); + } - case LacingStrategy.Shortest: - for (int i = variadicStart; i < inputAstNodes.Count; i++) - { - inputAstNodes[i] = AstFactory.AddReplicationGuide( - inputAstNodes[i], new List { 1 }, false); - } - break; + base.BuildOutputAst(model, inputAstNodes, resultAst); + } - case LacingStrategy.Longest: - for (int i = variadicStart; i < inputAstNodes.Count; i++) - { - inputAstNodes[i] = AstFactory.AddReplicationGuide( - inputAstNodes[i], new List { 1 }, true); - } - break; + /// + /// Returns true when any port has Use Levels enabled. Those settings cannot survive + /// the default pack-then-call path (they are inert inside the packed ExprList), so the + /// node must route through the per-port replicating wrapper instead. + /// + private static bool AnyPortUsesLevels(NodeModel model, int inputCount) + { + for (int i = 0; i < inputCount; i++) + { + if (model.InPorts[i].UseLevels) + return true; + } + return false; + } - case LacingStrategy.CrossProduct: - // Continue cross-product indices from where the prefix - // pass would have stopped: the prefix uses guides - // 1..paramCount-1, so the first variadic port starts at - // paramCount and each subsequent variadic port gets a - // distinct rank. - int guide = paramCount; - for (int i = variadicStart; i < inputAstNodes.Count; i++) - { - inputAstNodes[i] = AstFactory.AddReplicationGuide( - inputAstNodes[i], new List { guide }, false); - guide++; - } - break; - } - } + /// + /// Emits a generated wrapper function that exposes every port as a separate formal + /// parameter and packs the variadic tail per invocation, then calls it with per-port + /// Use Levels and lacing replication guides applied to the individual arguments. + /// Returns false (leaving untouched) if the wrapper + /// source cannot be parsed, so the caller falls back to the default pack path. + /// + private bool TryBuildReplicatingOutputAst( + NodeModel model, List inputAstNodes, List resultAst, int variadicStart) + { + // Build the call to the underlying function exactly as GetFunctionApplication + // would: Class.Function for member functions, the bare name for globals. + string callName = Definition.Type == FunctionType.GenericFunction + ? Definition.FunctionName + : Definition.ClassName + "." + Definition.FunctionName; - // Here we pack all var arguments in an array {y1, y2, ..., ym} - // (skipping the first n == paramCount - 1 inputs) - var argPack = AstFactory.BuildExprList(inputAstNodes.Skip(paramCount - 1).ToList()); - inputAstNodes = inputAstNodes.Take(paramCount - 1).ToList(); - inputAstNodes.Add(argPack); + // Unique, deterministic per node + arity so repeated runs redefine the same + // wrapper (cache-stable) and distinct nodes never collide. + string wrapperName = "__vararg_" + model.GUID.ToString("N") + "_" + inputAstNodes.Count; + + var paramNames = Enumerable.Range(0, inputAstNodes.Count).Select(i => "p" + i).ToList(); + var prefixArgs = paramNames.Take(variadicStart); + var packedArg = "[" + string.Join(", ", paramNames.Skip(variadicStart)) + "]"; + var innerArgs = string.Join(", ", prefixArgs.Append(packedArg)); + + string wrapperSource = + "def " + wrapperName + "(" + string.Join(", ", paramNames) + ")" + + " { return = " + callName + "(" + innerArgs + "); }"; + + FunctionDefinitionNode wrapperDef; + try + { + wrapperDef = ParserUtils.Parse(wrapperSource).Body + .OfType() + .FirstOrDefault(); + } + catch + { + return false; } - base.BuildOutputAst(model, inputAstNodes, resultAst); + if (wrapperDef == null) + return false; + + resultAst.Add(wrapperDef); + + // Per-port AtLevel / lacing replication guides land on the wrapper-call + // arguments, where they are honored just as on a non-variadic node. + model.UseLevelAndReplicationGuide(inputAstNodes); + + var rhs = AstFactory.BuildFunctionCall(wrapperName, inputAstNodes); + AssignIdentifiersForFunctionCall(model, rhs, resultAst); + return true; } } } \ No newline at end of file diff --git a/src/DynamoCore/PublicAPI.Unshipped.txt b/src/DynamoCore/PublicAPI.Unshipped.txt index 26c00244bbb..2f374cf2ec3 100644 --- a/src/DynamoCore/PublicAPI.Unshipped.txt +++ b/src/DynamoCore/PublicAPI.Unshipped.txt @@ -6,4 +6,3 @@ Dynamo.Models.DynamoModel.DefaultStartConfiguration.EnableUnTrustedLocationsNoti Dynamo.Models.DynamoModel.IStartConfiguration.EnableUnTrustedLocationsNotifications.get -> bool Dynamo.Models.DynamoModel.OpenFileCommand.OpenFileCommand(System.String filePath, System.Boolean forceManualExecutionMode, System.Boolean isTemplate, System.Boolean forceBlockRun) -> void Dynamo.Models.DynamoModel.InsertFileCommand.InsertFileCommand(System.String filePath, System.Boolean forceManualExecutionMode, System.Boolean forceBlockRun) -> void -virtual Dynamo.Graph.Nodes.NodeModel.LevelAndReplicationGuideInputCount(int rawInputCount) -> int diff --git a/test/DynamoCoreTests/Nodes/ListTests.cs b/test/DynamoCoreTests/Nodes/ListTests.cs index aa074c91d53..50de8476b4d 100644 --- a/test/DynamoCoreTests/Nodes/ListTests.cs +++ b/test/DynamoCoreTests/Nodes/ListTests.cs @@ -109,16 +109,17 @@ public void TestConcatenateListsNormalInput() public void TestListJoinUseLevelsOnVariadicPort() { // DYN-10572: List.Join is a pure-variadic DSVarArgFunction (no non-variadic - // prefix). With [IsLacingDisabled] on the C# method the node's lacing is - // Disabled, so per-port Use Levels must still be honored pre-pack even - // though no replication guides are applied. This test confirms the - // variadic Use Levels path runs without breaking the post-pack pass - // (which is a no-op here because LevelAndReplicationGuideInputCount is 0). + // prefix). Use Levels on a variadic port (list1 = [3,4] @L1) is now honored: + // the per-port AtLevel replicates the join at level 1, exactly as it would on + // a non-variadic node, independently of the other port. Before the fix the + // level was silently dropped and the result was the flat [1,2,3,4]; honoring + // it replicates element-wise to [[1,3],[2,4]]. string testFilePath = Path.Combine(listTestFolder, "TestListJoinUseLevelsOnVariadicPort.dyn"); RunModel(testFilePath); - AssertPreviewValue("dddd11110000222200003333dddd4444", new object[] { 1, 2, 3, 4 }); + AssertPreviewValue("dddd11110000222200003333dddd4444", + new object[] { new object[] { 1, 3 }, new object[] { 2, 4 } }); } #endregion diff --git a/test/DynamoCoreTests/Nodes/StringTests.cs b/test/DynamoCoreTests/Nodes/StringTests.cs index 7a27f15b717..4fa4f2ce130 100644 --- a/test/DynamoCoreTests/Nodes/StringTests.cs +++ b/test/DynamoCoreTests/Nodes/StringTests.cs @@ -230,10 +230,10 @@ public void TestJoinStringNormalInput() public void TestJoinStringUseLevelsOnVariadicPort() { // DYN-10572: String.Join has both a non-variadic prefix port (separator) - // and variadic ports. Set UseLevels on the separator port to exercise the - // bounded post-pack pass (LevelAndReplicationGuideInputCount == 1) AND on - // a variadic port to exercise the new pre-pack handling, confirming both - // code paths cooperate correctly. + // and variadic ports. The separator (prefix port) broadcasts unchanged while + // Use Levels on a variadic port (string1 = ["a","b"] @L1) replicates the join, + // confirming the prefix port passes through correctly and per-port Use Levels + // is honored on the variadic ports. string testFilePath = Path.Combine(localDynamoStringTestFolder, "TestJoinStringUseLevelsOnVariadicPort.dyn"); RunModel(testFilePath); diff --git a/test/core/string/TestJoinStringUseLevelsOnVariadicPort.dyn b/test/core/string/TestJoinStringUseLevelsOnVariadicPort.dyn index d88c1413c6f..78b02c0b4c7 100644 --- a/test/core/string/TestJoinStringUseLevelsOnVariadicPort.dyn +++ b/test/core/string/TestJoinStringUseLevelsOnVariadicPort.dyn @@ -82,7 +82,7 @@ "Description": "String used to separate strings.\n\nstring", "UsingDefaultValue": false, "Level": 1, - "UseLevels": true, + "UseLevels": false, "KeepListStructure": false }, {