Fix property references to not use copied guid refs (#1932)

* first pass, need to talk to Landon about fix * Update CHANGELOG.md * making found properties also look for a matching reference name * code review changes * code review bug fix and made copying properties/keywords form the blackboard consistent with how nodes behave in searching for properties/keywords * small bugfix for duplicating properties on graph/between graphs * more bugfixing
Unity-Technologies · Oct 6, 2020 · 4d7752f · 4d7752f
1 parent 234dec6
commit 4d7752f
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 13 deletions.
diff --git a/com.unity.shadergraph/CHANGELOG.md b/com.unity.shadergraph/CHANGELOG.md
@@ -28,6 +28,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Fixed an issue where ShaderGraph shaders did not reimport automatically when some of the included files changed [1269634]
 - Fixed an issue where building a context menu on a dragging block node would leave it floating and undo/redo would result in a soft-lock
 - Fixed an issue where ShaderGraph was logging error when edited in play mode [1274148].
+- Fixed a bug where properties copied over with their graph inputs would not hook up correctly in a new graph [1274306]
 - Fixed an issue where renaming a property in the blackboard at creation would trigger an error.
 - Fixed an issue where ShaderGraph shaders did not reimport automatically when missing dependencies were reintroduced [1182895]
 - Fixed an issue where ShaderGraph previews would not show error shaders when the active render pipeline is incompatible with the shader [1257015]

diff --git a/com.unity.shadergraph/Editor/Data/Graphs/GraphData.cs b/com.unity.shadergraph/Editor/Data/Graphs/GraphData.cs
@@ -1548,10 +1548,9 @@ internal void PasteGraph(CopyPasteGraph graphToPaste, List<AbstractMaterialNode>
                 {
                     // If the property is not in the current graph, do check if the
                     // property can be made into a concrete node.
-                    var index = graphToPaste.metaProperties.TakeWhile(x => x != propertyNode.property).Count();
-                    var originalId = graphToPaste.metaPropertyIds.ElementAt(index);
-                    var property = m_Properties.SelectValue().FirstOrDefault(x => x.objectId == originalId);
-                    if (property != null)
+                    var property = m_Properties.SelectValue().FirstOrDefault(x => x.objectId == propertyNode.property.objectId
+                                                                              || (x.propertyType == propertyNode.property.propertyType && x.referenceName == propertyNode.property.referenceName));
+                    if(property != null)
                     {
                         propertyNode.property = property;
                     }
@@ -1602,9 +1601,8 @@ internal void PasteGraph(CopyPasteGraph graphToPaste, List<AbstractMaterialNode>
                 // Check if the keyword nodes need to have their keywords copied.
                 if (node is KeywordNode keywordNode)
                 {
-                    var index = graphToPaste.metaKeywords.TakeWhile(x => x != keywordNode.keyword).Count();
-                    var originalId = graphToPaste.metaKeywordIds.ElementAt(index);
-                    var keyword = m_Keywords.SelectValue().FirstOrDefault(x => x.objectId == originalId);
+                    var keyword = m_Keywords.SelectValue().FirstOrDefault(x => x.objectId == keywordNode.keyword.objectId
+                                                                      || (x.keywordType == keywordNode.keyword.keywordType && x.referenceName == keywordNode.keyword.referenceName));
                     if (keyword != null)
                     {
                         keywordNode.keyword = keyword;

diff --git a/com.unity.shadergraph/Editor/Drawing/Views/MaterialGraphView.cs b/com.unity.shadergraph/Editor/Drawing/Views/MaterialGraphView.cs
@@ -1218,12 +1218,11 @@ internal static void InsertCopyPasteGraph(this MaterialGraphView graphView, Copy
             // Make new inputs from the copied graph
             foreach (ShaderInput input in copyGraph.inputs)
             {
-                ShaderInput copiedInput;
-
                 switch(input)
                 {
                     case AbstractShaderProperty property:
-                        copiedInput = DuplicateShaderInputs(input, graphView.graph, indicies[BlackboardProvider.k_PropertySectionIndex]);
+                        var copiedProperty = (AbstractShaderProperty) DuplicateShaderInputs(input, graphView.graph, indicies[BlackboardProvider.k_PropertySectionIndex]);
+                        graphView.graph.SanitizeGraphInputReferenceName(copiedProperty, input.referenceName);
 
                         // Increment for next within the same section
                         if (indicies[BlackboardProvider.k_PropertySectionIndex] >= 0)
@@ -1234,7 +1233,7 @@ internal static void InsertCopyPasteGraph(this MaterialGraphView graphView, Copy
                         foreach (var node in dependentPropertyNodes)
                         {
                             node.owner = graphView.graph;
-                            node.property = property;
+                            node.property = copiedProperty;
                         }
                         break;
 
@@ -1243,7 +1242,8 @@ internal static void InsertCopyPasteGraph(this MaterialGraphView graphView, Copy
                         if ((input as ShaderKeyword).isBuiltIn && graphView.graph.keywords.Where(p => p.referenceName == input.referenceName).Any())
                             continue;
 
-                        copiedInput = DuplicateShaderInputs(input, graphView.graph, indicies[BlackboardProvider.k_KeywordSectionIndex]);
+                        var copiedKeyword = (ShaderKeyword)DuplicateShaderInputs(input, graphView.graph, indicies[BlackboardProvider.k_KeywordSectionIndex]);
+                        graphView.graph.SanitizeGraphInputReferenceName(copiedKeyword, input.referenceName);
 
                         // Increment for next within the same section
                         if (indicies[BlackboardProvider.k_KeywordSectionIndex] >= 0)
@@ -1254,7 +1254,7 @@ internal static void InsertCopyPasteGraph(this MaterialGraphView graphView, Copy
                         foreach (var node in dependentKeywordNodes)
                         {
                             node.owner = graphView.graph;
-                            node.keyword = shaderKeyword;
+                            node.keyword = copiedKeyword;
                         }
 
                         // Pasting a new Keyword so need to test against variant limit