From 4d7752fe3a90654bcdad86daebd1417bcd8fe8e2 Mon Sep 17 00:00:00 2001 From: elizabeth-legros <59933602+elizabeth-legros@users.noreply.github.com> Date: Tue, 6 Oct 2020 13:10:02 -0500 Subject: [PATCH] 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 --- com.unity.shadergraph/CHANGELOG.md | 1 + .../Editor/Data/Graphs/GraphData.cs | 12 +++++------- .../Editor/Drawing/Views/MaterialGraphView.cs | 12 ++++++------ 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/com.unity.shadergraph/CHANGELOG.md b/com.unity.shadergraph/CHANGELOG.md index b0d37128a89..af48a8553a4 100644 --- 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 index 9ba06be3ce2..aab4420734a 100644 --- 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 { // 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 // 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 index cb404082f5c..cbaa9e05f43 100644 --- 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