From aa1c561041d6263faced1d6575c114207c05e053 Mon Sep 17 00:00:00 2001 From: Merlin <36685500+MerlinVR@users.noreply.github.com> Date: Tue, 15 Jun 2021 17:42:58 -0700 Subject: [PATCH 1/3] Add support for FieldChangeCallback - Also fix usage of `nameof` in attributes since it will be a common ask for the callback attribute --- .../UdonSharp/Editor/UdonSharpASTVisitor.cs | 45 +++---------- .../Editor/UdonSharpCompilationModule.cs | 22 ++++--- .../Editor/UdonSharpExpressionCapture.cs | 3 + .../UdonSharp/Editor/UdonSharpFieldVisitor.cs | 26 ++++++++ .../UdonSharp/Editor/UdonSharpSyntaxWalker.cs | 46 +++++++++++++ .../UdonSharp/Scripts/UdonSharpAttributes.cs | 18 +++++ .../UdonSharp/Scripts/UdonSharpBehaviour.cs | 17 +++++ .../Tests/IntegrationTestScene.unity | 2 +- .../Tests/TestScripts/Core/PropertyTest.asset | 65 ++++++++++++++++++- .../Tests/TestScripts/Core/PropertyTest.cs | 18 +++++ 10 files changed, 215 insertions(+), 47 deletions(-) diff --git a/Assets/UdonSharp/Editor/UdonSharpASTVisitor.cs b/Assets/UdonSharp/Editor/UdonSharpASTVisitor.cs index 554fc98f..741b25f7 100644 --- a/Assets/UdonSharp/Editor/UdonSharpASTVisitor.cs +++ b/Assets/UdonSharp/Editor/UdonSharpASTVisitor.cs @@ -26,6 +26,7 @@ public class ASTVisitorContext public List definedMethods; public List definedProperties; + public Dictionary onModifyCallbackFields = new Dictionary(); // Tracking labels for the current function and flow control public JumpLabel returnLabel = null; @@ -531,6 +532,15 @@ public override void VisitPropertyDeclaration(PropertyDeclarationSyntax node) { var setter = definition.setter; + // Handle VRC field modification callbacks + if (visitorContext.onModifyCallbackFields.TryGetValue(definition.originalPropertyName, out FieldDefinition targetField)) + { + string exportStr = VRC.Udon.Common.VariableChangedEvent.EVENT_PREFIX + targetField.fieldSymbol.symbolUniqueName; + visitorContext.uasmBuilder.AppendLine($".export {exportStr}", 1); + visitorContext.uasmBuilder.AppendLine($"{exportStr}:", 1); + visitorContext.uasmBuilder.AddCopy(setter.paramSymbol, targetField.fieldSymbol); + } + if ((node.Modifiers.HasModifier("public") && setter.declarationFlags == PropertyDeclFlags.None) || setter.declarationFlags == PropertyDeclFlags.Public) { visitorContext.uasmBuilder.AppendLine($".export {setter.accessorName}", 1); @@ -2473,41 +2483,6 @@ public override void VisitSwitchStatement(SwitchStatementSyntax node) visitorContext.breakLabelStack.Pop(); } - private void HandleNameOfExpression(InvocationExpressionSyntax node) - { - SyntaxNode currentNode = node.ArgumentList.Arguments[0].Expression; - string currentName = ""; - - while (currentNode != null) - { - switch (currentNode.Kind()) - { - case SyntaxKind.SimpleMemberAccessExpression: - MemberAccessExpressionSyntax memberNode = (MemberAccessExpressionSyntax)currentNode; - currentName = memberNode.Name.ToString(); - currentNode = memberNode.Name; - break; - case SyntaxKind.IdentifierName: - IdentifierNameSyntax identifierName = (IdentifierNameSyntax)currentNode; - currentName = identifierName.ToString(); - currentNode = null; - break; - default: - currentNode = null; - break; - } - - if (currentNode != null) - UpdateSyntaxNode(currentNode); - } - - if (currentName == "") - throw new System.ArgumentException("Expression does not have a name"); - - if (visitorContext.topCaptureScope != null) - visitorContext.topCaptureScope.SetToLocalSymbol(visitorContext.topTable.CreateConstSymbol(typeof(string), currentName)); - } - public override void VisitCaseSwitchLabel(CaseSwitchLabelSyntax node) { UpdateSyntaxNode(node); diff --git a/Assets/UdonSharp/Editor/UdonSharpCompilationModule.cs b/Assets/UdonSharp/Editor/UdonSharpCompilationModule.cs index d07f0620..fbb29382 100644 --- a/Assets/UdonSharp/Editor/UdonSharpCompilationModule.cs +++ b/Assets/UdonSharp/Editor/UdonSharpCompilationModule.cs @@ -86,15 +86,15 @@ public CompileTaskResult Compile(List classDefinitions, Microso debugInfo = new ClassDebugInfo(sourceCode, settings == null || settings.includeInlineCode); } - UdonSharpFieldVisitor fieldVisitor = new UdonSharpFieldVisitor(fieldsWithInitializers, resolver, moduleSymbols, moduleLabels, classDefinitions, debugInfo); - + PropertyVisitor propertyVisitor = new PropertyVisitor(resolver, moduleSymbols, moduleLabels); + try { - fieldVisitor.Visit(syntaxTree.GetRoot()); + propertyVisitor.Visit(syntaxTree.GetRoot()); } catch (System.Exception e) { - LogException(result, e, fieldVisitor.visitorContext.currentNode, out string logMessage); + LogException(result, e, propertyVisitor.visitorContext.currentNode, out string logMessage); programAsset.compileErrors.Add(logMessage); @@ -104,15 +104,16 @@ public CompileTaskResult Compile(List classDefinitions, Microso if (ErrorCount > 0) return result; - MethodVisitor methodVisitor = new MethodVisitor(resolver, moduleSymbols, moduleLabels); + UdonSharpFieldVisitor fieldVisitor = new UdonSharpFieldVisitor(fieldsWithInitializers, resolver, moduleSymbols, moduleLabels, classDefinitions, debugInfo); + fieldVisitor.visitorContext.definedProperties = propertyVisitor.definedProperties; try { - methodVisitor.Visit(syntaxTree.GetRoot()); + fieldVisitor.Visit(syntaxTree.GetRoot()); } catch (System.Exception e) { - LogException(result, e, methodVisitor.visitorContext.currentNode, out string logMessage); + LogException(result, e, fieldVisitor.visitorContext.currentNode, out string logMessage); programAsset.compileErrors.Add(logMessage); @@ -122,15 +123,15 @@ public CompileTaskResult Compile(List classDefinitions, Microso if (ErrorCount > 0) return result; - PropertyVisitor propertyVisitor = new PropertyVisitor(resolver, moduleSymbols, moduleLabels); + MethodVisitor methodVisitor = new MethodVisitor(resolver, moduleSymbols, moduleLabels); try { - propertyVisitor.Visit(syntaxTree.GetRoot()); + methodVisitor.Visit(syntaxTree.GetRoot()); } catch (System.Exception e) { - LogException(result, e, propertyVisitor.visitorContext.currentNode, out string logMessage); + LogException(result, e, methodVisitor.visitorContext.currentNode, out string logMessage); programAsset.compileErrors.Add(logMessage); @@ -141,6 +142,7 @@ public CompileTaskResult Compile(List classDefinitions, Microso return result; ASTVisitor visitor = new ASTVisitor(resolver, moduleSymbols, moduleLabels, methodVisitor.definedMethods, propertyVisitor.definedProperties, classDefinitions, debugInfo); + visitor.visitorContext.onModifyCallbackFields = fieldVisitor.visitorContext.onModifyCallbackFields; try { diff --git a/Assets/UdonSharp/Editor/UdonSharpExpressionCapture.cs b/Assets/UdonSharp/Editor/UdonSharpExpressionCapture.cs index 0a1374f6..1d17970f 100644 --- a/Assets/UdonSharp/Editor/UdonSharpExpressionCapture.cs +++ b/Assets/UdonSharp/Editor/UdonSharpExpressionCapture.cs @@ -740,6 +740,9 @@ public void ExecuteSet(SymbolDefinition value, bool explicitCast = false) } else if (captureArchetype == ExpressionCaptureArchetype.ExternUserField) { + if (visitorContext.onModifyCallbackFields.Values.Any(e => e.fieldSymbol.symbolUniqueName == captureExternUserField.fieldSymbol.symbolUniqueName)) + throw new System.InvalidOperationException($"Cannot set field with {nameof(FieldChangeCallbackAttribute)}, use a property or SetProgramVariable"); + using (ExpressionCaptureScope setVariableMethodScope = new ExpressionCaptureScope(visitorContext, null)) { setVariableMethodScope.SetToLocalSymbol(accessSymbol); diff --git a/Assets/UdonSharp/Editor/UdonSharpFieldVisitor.cs b/Assets/UdonSharp/Editor/UdonSharpFieldVisitor.cs index a4bab8e5..bbcac6d6 100644 --- a/Assets/UdonSharp/Editor/UdonSharpFieldVisitor.cs +++ b/Assets/UdonSharp/Editor/UdonSharpFieldVisitor.cs @@ -3,6 +3,7 @@ using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; using System.Collections.Generic; +using System.Linq; using UnityEngine; namespace UdonSharp.Compiler @@ -49,11 +50,14 @@ public override void VisitFieldDeclaration(FieldDeclarationSyntax node) List fieldAttributes = GetFieldAttributes(node); + bool isPublic = (node.Modifiers.Any(SyntaxKind.PublicKeyword) || fieldAttributes.Find(e => e is SerializeField) != null) && fieldAttributes.Find(e => e is System.NonSerializedAttribute) == null; bool isConst = (node.Modifiers.Any(SyntaxKind.ConstKeyword) || node.Modifiers.Any(SyntaxKind.ReadOnlyKeyword)); SymbolDeclTypeFlags flags = (isPublic ? SymbolDeclTypeFlags.Public : SymbolDeclTypeFlags.Private) | (isConst ? SymbolDeclTypeFlags.Readonly : 0); + FieldChangeCallbackAttribute varChange = fieldAttributes.OfType().FirstOrDefault(); + List fieldSymbols = HandleVariableDeclaration(node.Declaration, flags, fieldSyncMode); foreach (SymbolDefinition fieldSymbol in fieldSymbols) { @@ -77,6 +81,28 @@ public override void VisitFieldDeclaration(FieldDeclarationSyntax node) } visitorContext.localFieldDefinitions.Add(fieldSymbol.symbolUniqueName, fieldDefinition); + + if (varChange != null) + { + string targetProperty = varChange.CallbackPropertyName; + + if (variables.Count > 1 || visitorContext.onModifyCallbackFields.ContainsKey(targetProperty)) + throw new System.Exception($"Only one field may target property '{targetProperty}'"); + + PropertyDefinition foundProperty = visitorContext.definedProperties.FirstOrDefault(e => e.originalPropertyName == targetProperty); + + if (foundProperty == null) + throw new System.ArgumentException($"Invalid target property for {nameof(FieldChangeCallbackAttribute)} on {node.Declaration}"); + + PropertyDefinition property = visitorContext.definedProperties.FirstOrDefault(e => e.originalPropertyName == targetProperty); + if (property == null) + throw new System.ArgumentException($"Property not found for '{targetProperty}'"); + + if (property.type != fieldDefinition.fieldSymbol.userCsType) + throw new System.Exception($"Types must match between property and variable change field"); + + visitorContext.onModifyCallbackFields.Add(targetProperty, fieldDefinition); + } } } diff --git a/Assets/UdonSharp/Editor/UdonSharpSyntaxWalker.cs b/Assets/UdonSharp/Editor/UdonSharpSyntaxWalker.cs index dc3bda1f..76353e93 100644 --- a/Assets/UdonSharp/Editor/UdonSharpSyntaxWalker.cs +++ b/Assets/UdonSharp/Editor/UdonSharpSyntaxWalker.cs @@ -319,6 +319,52 @@ public override void VisitUsingDirective(UsingDirectiveSyntax node) } } + protected void HandleNameOfExpression(InvocationExpressionSyntax node) + { + SyntaxNode currentNode = node.ArgumentList.Arguments[0].Expression; + string currentName = ""; + + while (currentNode != null) + { + switch (currentNode.Kind()) + { + case SyntaxKind.SimpleMemberAccessExpression: + MemberAccessExpressionSyntax memberNode = (MemberAccessExpressionSyntax)currentNode; + currentName = memberNode.Name.ToString(); + currentNode = memberNode.Name; + break; + case SyntaxKind.IdentifierName: + IdentifierNameSyntax identifierName = (IdentifierNameSyntax)currentNode; + currentName = identifierName.ToString(); + currentNode = null; + break; + default: + currentNode = null; + break; + } + + if (currentNode != null) + UpdateSyntaxNode(currentNode); + } + + if (currentName == "") + throw new System.ArgumentException("Expression does not have a name"); + + if (visitorContext.topCaptureScope != null) + visitorContext.topCaptureScope.SetToLocalSymbol(visitorContext.topTable.CreateConstSymbol(typeof(string), currentName)); + } + + public override void VisitInvocationExpression(InvocationExpressionSyntax node) + { + UpdateSyntaxNode(node); + + if (node.Expression != null && node.Expression.ToString() == "nameof") // nameof is not a dedicated node and the Kind of the node isn't the nameof kind for whatever reason... + { + HandleNameOfExpression(node); + return; + } + } + public override void VisitIdentifierName(IdentifierNameSyntax node) { UpdateSyntaxNode(node); diff --git a/Assets/UdonSharp/Scripts/UdonSharpAttributes.cs b/Assets/UdonSharp/Scripts/UdonSharpAttributes.cs index 59b3fb17..6d5ea216 100644 --- a/Assets/UdonSharp/Scripts/UdonSharpAttributes.cs +++ b/Assets/UdonSharp/Scripts/UdonSharpAttributes.cs @@ -89,5 +89,23 @@ public class RecursiveMethodAttribute : Attribute public RecursiveMethodAttribute() { } } + + /// + /// Calls the target property's setter when the marked field is modified by network sync or SetProgramVariable(). + /// The field will already be modified so use this to react to changes after the fact. + /// + [PublicAPI] + [AttributeUsage(AttributeTargets.Field, AllowMultiple = false)] + public class FieldChangeCallbackAttribute : Attribute + { + public string CallbackPropertyName { get; private set; } + + private FieldChangeCallbackAttribute() { } + + public FieldChangeCallbackAttribute(string targetPropertyName) + { + CallbackPropertyName = targetPropertyName; + } + } } diff --git a/Assets/UdonSharp/Scripts/UdonSharpBehaviour.cs b/Assets/UdonSharp/Scripts/UdonSharpBehaviour.cs index 274bff49..f221ce69 100644 --- a/Assets/UdonSharp/Scripts/UdonSharpBehaviour.cs +++ b/Assets/UdonSharp/Scripts/UdonSharpBehaviour.cs @@ -35,6 +35,23 @@ public void SetProgramVariable(string name, object value) if (variableField != null) { variableField.SetValue(this, value); + + FieldChangeCallbackAttribute fieldChangeCallback = variableField.GetCustomAttribute(); + + if (fieldChangeCallback == null) + return; + + PropertyInfo targetProperty = variableField.DeclaringType.GetProperty(fieldChangeCallback.CallbackPropertyName, BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Instance); + + if (targetProperty == null) + return; + + MethodInfo setMethod = targetProperty.GetSetMethod(true); + + if (setMethod == null) + return; + + setMethod.Invoke(this, new object[] { variableField.GetValue(this) }); } } diff --git a/Assets/UdonSharp/Tests/IntegrationTestScene.unity b/Assets/UdonSharp/Tests/IntegrationTestScene.unity index 496f7336..10e5633a 100644 --- a/Assets/UdonSharp/Tests/IntegrationTestScene.unity +++ b/Assets/UdonSharp/Tests/IntegrationTestScene.unity @@ -8356,7 +8356,7 @@ MonoBehaviour: serializedProgramAsset: {fileID: 11400000, guid: c341eeb82008276418f85fd69d4dd8b3, type: 2} programSource: {fileID: 11400000, guid: e6b13e0cbf910824dbf27af03fe07b7f, type: 2} - serializedPublicVariablesBytesString: Ai8AAAAAATIAAABWAFIAQwAuAFUAZABvAG4ALgBDAG8AbQBtAG8AbgAuAFUAZABvAG4AVgBhAHIAaQBhAGIAbABlAFQAYQBiAGwAZQAsACAAVgBSAEMALgBVAGQAbwBuAC4AQwBvAG0AbQBvAG4AAAAAAAYBAAAAAAAAACcBBAAAAHQAeQBwAGUAAWgAAABTAHkAcwB0AGUAbQAuAEMAbwBsAGwAZQBjAHQAaQBvAG4AcwAuAEcAZQBuAGUAcgBpAGMALgBMAGkAcwB0AGAAMQBbAFsAVgBSAEMALgBVAGQAbwBuAC4AQwBvAG0AbQBvAG4ALgBJAG4AdABlAHIAZgBhAGMAZQBzAC4ASQBVAGQAbwBuAFYAYQByAGkAYQBiAGwAZQAsACAAVgBSAEMALgBVAGQAbwBuAC4AQwBvAG0AbQBvAG4AXQBdACwAIABtAHMAYwBvAHIAbABpAGIAAQEJAAAAVgBhAHIAaQBhAGIAbABlAHMALwEAAAABaAAAAFMAeQBzAHQAZQBtAC4AQwBvAGwAbABlAGMAdABpAG8AbgBzAC4ARwBlAG4AZQByAGkAYwAuAEwAaQBzAHQAYAAxAFsAWwBWAFIAQwAuAFUAZABvAG4ALgBDAG8AbQBtAG8AbgAuAEkAbgB0AGUAcgBmAGEAYwBlAHMALgBJAFUAZABvAG4AVgBhAHIAaQBhAGIAbABlACwAIABWAFIAQwAuAFUAZABvAG4ALgBDAG8AbQBtAG8AbgBdAF0ALAAgAG0AcwBjAG8AcgBsAGkAYgABAAAABgAAAAAAAAAABwUHBQ== + serializedPublicVariablesBytesString: Ai8AAAAAATIAAABWAFIAQwAuAFUAZABvAG4ALgBDAG8AbQBtAG8AbgAuAFUAZABvAG4AVgBhAHIAaQBhAGIAbABlAFQAYQBiAGwAZQAsACAAVgBSAEMALgBVAGQAbwBuAC4AQwBvAG0AbQBvAG4AAAAAAAYBAAAAAAAAACcBBAAAAHQAeQBwAGUAAWgAAABTAHkAcwB0AGUAbQAuAEMAbwBsAGwAZQBjAHQAaQBvAG4AcwAuAEcAZQBuAGUAcgBpAGMALgBMAGkAcwB0AGAAMQBbAFsAVgBSAEMALgBVAGQAbwBuAC4AQwBvAG0AbQBvAG4ALgBJAG4AdABlAHIAZgBhAGMAZQBzAC4ASQBVAGQAbwBuAFYAYQByAGkAYQBiAGwAZQAsACAAVgBSAEMALgBVAGQAbwBuAC4AQwBvAG0AbQBvAG4AXQBdACwAIABtAHMAYwBvAHIAbABpAGIAAQEJAAAAVgBhAHIAaQBhAGIAbABlAHMALwEAAAABaAAAAFMAeQBzAHQAZQBtAC4AQwBvAGwAbABlAGMAdABpAG8AbgBzAC4ARwBlAG4AZQByAGkAYwAuAEwAaQBzAHQAYAAxAFsAWwBWAFIAQwAuAFUAZABvAG4ALgBDAG8AbQBtAG8AbgAuAEkAbgB0AGUAcgBmAGEAYwBlAHMALgBJAFUAZABvAG4AVgBhAHIAaQBhAGIAbABlACwAIABWAFIAQwAuAFUAZABvAG4ALgBDAG8AbQBtAG8AbgBdAF0ALAAgAG0AcwBjAG8AcgBsAGkAYgABAAAABgEAAAAAAAAAAi8CAAAAAUoAAABWAFIAQwAuAFUAZABvAG4ALgBDAG8AbQBtAG8AbgAuAFUAZABvAG4AVgBhAHIAaQBhAGIAbABlAGAAMQBbAFsAUwB5AHMAdABlAG0ALgBTAGkAbgBnAGwAZQAsACAAbQBzAGMAbwByAGwAaQBiAF0AXQAsACAAVgBSAEMALgBVAGQAbwBuAC4AQwBvAG0AbQBvAG4AAgAAAAYCAAAAAAAAACcBBAAAAHQAeQBwAGUAARcAAABTAHkAcwB0AGUAbQAuAFMAdAByAGkAbgBnACwAIABtAHMAYwBvAHIAbABpAGIAJwEKAAAAUwB5AG0AYgBvAGwATgBhAG0AZQABFwAAAGIAYQBjAGsAaQBuAGcAQwBhAGwAbABiAGEAYwBrAFAAcgBvAHAAZQByAHQAeQAnAQQAAAB0AHkAcABlAAEXAAAAUwB5AHMAdABlAG0ALgBTAGkAbgBnAGwAZQAsACAAbQBzAGMAbwByAGwAaQBiAB8BBQAAAFYAYQBsAHUAZQAAAAAABwUHBQcF publicVariablesUnityEngineObjects: [] publicVariablesSerializationDataFormat: 0 --- !u!4 &1954857073 diff --git a/Assets/UdonSharp/Tests/TestScripts/Core/PropertyTest.asset b/Assets/UdonSharp/Tests/TestScripts/Core/PropertyTest.asset index f828436e..02ba6042 100644 --- a/Assets/UdonSharp/Tests/TestScripts/Core/PropertyTest.asset +++ b/Assets/UdonSharp/Tests/TestScripts/Core/PropertyTest.asset @@ -44,7 +44,7 @@ MonoBehaviour: Data: - Name: Entry: 12 - Data: 4 + Data: 5 - Name: Entry: 7 Data: @@ -297,6 +297,69 @@ MonoBehaviour: - Name: Entry: 8 Data: + - Name: + Entry: 7 + Data: + - Name: $k + Entry: 1 + Data: backingCallbackProperty + - Name: $v + Entry: 7 + Data: 18|UdonSharp.Compiler.FieldDefinition, UdonSharp.Editor + - Name: fieldSymbol + Entry: 7 + Data: 19|UdonSharp.Compiler.SymbolDefinition, UdonSharp.Editor + - Name: internalType + Entry: 9 + Data: 9 + - Name: declarationType + Entry: 3 + Data: 1 + - Name: syncMode + Entry: 3 + Data: 0 + - Name: symbolResolvedTypeName + Entry: 1 + Data: SystemSingle + - Name: symbolOriginalName + Entry: 1 + Data: backingCallbackProperty + - Name: symbolUniqueName + Entry: 1 + Data: backingCallbackProperty + - Name: symbolDefaultValue + Entry: 6 + Data: + - Name: + Entry: 8 + Data: + - Name: fieldAttributes + Entry: 7 + Data: 20|System.Collections.Generic.List`1[[System.Attribute, mscorlib]], mscorlib + - Name: + Entry: 12 + Data: 1 + - Name: + Entry: 7 + Data: 21|UdonSharp.FieldChangeCallbackAttribute, UdonSharp.Runtime + - Name: + Entry: 8 + Data: + - Name: + Entry: 13 + Data: + - Name: + Entry: 8 + Data: + - Name: userBehaviourSource + Entry: 6 + Data: + - Name: + Entry: 8 + Data: + - Name: + Entry: 8 + Data: - Name: Entry: 13 Data: diff --git a/Assets/UdonSharp/Tests/TestScripts/Core/PropertyTest.cs b/Assets/UdonSharp/Tests/TestScripts/Core/PropertyTest.cs index 2db56162..0a6b099b 100644 --- a/Assets/UdonSharp/Tests/TestScripts/Core/PropertyTest.cs +++ b/Assets/UdonSharp/Tests/TestScripts/Core/PropertyTest.cs @@ -35,6 +35,19 @@ public int MyIntBackedProperty int accessCounter = 0; public int MyAccessCounter { set { /*Debug.Log("Setting access counter to " + value); */accessCounter = value; } get { /*Debug.Log("Access counter: " + accessCounter);*/ return accessCounter++; } } + [FieldChangeCallback(nameof(CallbackProperty))] + public float backingCallbackProperty; + + public float CallbackProperty + { + set + { + Debug.Log($"Value current val: {backingCallbackProperty}, new val: {value}"); + backingCallbackProperty = value; + } + get => backingCallbackProperty; + } + public void ExecuteTests() { PropertyTest self = this; @@ -104,6 +117,11 @@ public void ExecuteTests() int additionResult = MyAccessCounter += 6; tester.TestAssertion("In place addition", accessCounter == 17 && additionResult == 17); + + CallbackProperty = 4f; + + SetProgramVariable(nameof(backingCallbackProperty), 10f); + //self.backingCallbackProperty = 20f; } } } From 644e37b5328b72f6a14ec49504d1a459c3a37db4 Mon Sep 17 00:00:00 2001 From: Merlin <36685500+MerlinVR@users.noreply.github.com> Date: Thu, 17 Jun 2021 10:45:19 -0700 Subject: [PATCH 2/3] Add property callback tests --- .../Tests/TestScripts/Core/PropertyTest.asset | 6 +-- .../Tests/TestScripts/Core/PropertyTest.cs | 38 ++++++++++++++++--- 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/Assets/UdonSharp/Tests/TestScripts/Core/PropertyTest.asset b/Assets/UdonSharp/Tests/TestScripts/Core/PropertyTest.asset index 02ba6042..80aa149f 100644 --- a/Assets/UdonSharp/Tests/TestScripts/Core/PropertyTest.asset +++ b/Assets/UdonSharp/Tests/TestScripts/Core/PropertyTest.asset @@ -302,7 +302,7 @@ MonoBehaviour: Data: - Name: $k Entry: 1 - Data: backingCallbackProperty + Data: backingCallbackfield - Name: $v Entry: 7 Data: 18|UdonSharp.Compiler.FieldDefinition, UdonSharp.Editor @@ -323,10 +323,10 @@ MonoBehaviour: Data: SystemSingle - Name: symbolOriginalName Entry: 1 - Data: backingCallbackProperty + Data: backingCallbackfield - Name: symbolUniqueName Entry: 1 - Data: backingCallbackProperty + Data: backingCallbackfield - Name: symbolDefaultValue Entry: 6 Data: diff --git a/Assets/UdonSharp/Tests/TestScripts/Core/PropertyTest.cs b/Assets/UdonSharp/Tests/TestScripts/Core/PropertyTest.cs index 0a6b099b..4a4fbe44 100644 --- a/Assets/UdonSharp/Tests/TestScripts/Core/PropertyTest.cs +++ b/Assets/UdonSharp/Tests/TestScripts/Core/PropertyTest.cs @@ -36,16 +36,33 @@ public int MyIntBackedProperty public int MyAccessCounter { set { /*Debug.Log("Setting access counter to " + value); */accessCounter = value; } get { /*Debug.Log("Access counter: " + accessCounter);*/ return accessCounter++; } } [FieldChangeCallback(nameof(CallbackProperty))] - public float backingCallbackProperty; + public float backingCallbackfield; + int callbackCount = 0; public float CallbackProperty { set { - Debug.Log($"Value current val: {backingCallbackProperty}, new val: {value}"); - backingCallbackProperty = value; + //Debug.Log($"Value current val: {backingCallbackfield}, new val: {value}"); + backingCallbackfield = value; + ++callbackCount; + } + get => backingCallbackfield; + } + + [FieldChangeCallback(nameof(GOCallbackTest))] + private GameObject _GOCallbackTest; + int goCallbackCounter = 0; + + public GameObject GOCallbackTest + { + get => _GOCallbackTest; + set + { + //Debug.LogFormat("Setting GO callback to {0}, last value: {1}", new object[] { value, _GOCallbackTest }); + _GOCallbackTest = value; + ++goCallbackCounter; } - get => backingCallbackProperty; } public void ExecuteTests() @@ -120,8 +137,17 @@ public void ExecuteTests() CallbackProperty = 4f; - SetProgramVariable(nameof(backingCallbackProperty), 10f); - //self.backingCallbackProperty = 20f; + SetProgramVariable(nameof(backingCallbackfield), 10f); + + tester.TestAssertion("Property callback", callbackCount == 2); + + GOCallbackTest = gameObject; + SetProgramVariable(nameof(_GOCallbackTest), null); + SetProgramVariable(nameof(_GOCallbackTest), gameObject); + SetProgramVariable(nameof(_GOCallbackTest), gameObject); + SetProgramVariable(nameof(_GOCallbackTest), null); + + tester.TestAssertion("Property callback modification count", goCallbackCounter == 4); } } } From b6341a49d3eaa7e2dc27a3eb683e2ed0f55f018a Mon Sep 17 00:00:00 2001 From: Merlin <36685500+MerlinVR@users.noreply.github.com> Date: Wed, 23 Jun 2021 12:52:48 -0700 Subject: [PATCH 3/3] Update to handle new oldValue variable on field change callbacks - Properties now work like normal C# properties when used with the field changed callback where the field will stay the old value and the new value is passed in via the property's `value` --- .../UdonSharp/Editor/UdonSharpASTVisitor.cs | 4 +++ .../UdonSharp/Scripts/UdonSharpAttributes.cs | 2 +- .../UdonSharp/Scripts/UdonSharpBehaviour.cs | 26 ++++++++++--------- 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/Assets/UdonSharp/Editor/UdonSharpASTVisitor.cs b/Assets/UdonSharp/Editor/UdonSharpASTVisitor.cs index 741b25f7..b8150cab 100644 --- a/Assets/UdonSharp/Editor/UdonSharpASTVisitor.cs +++ b/Assets/UdonSharp/Editor/UdonSharpASTVisitor.cs @@ -538,7 +538,11 @@ public override void VisitPropertyDeclaration(PropertyDeclarationSyntax node) string exportStr = VRC.Udon.Common.VariableChangedEvent.EVENT_PREFIX + targetField.fieldSymbol.symbolUniqueName; visitorContext.uasmBuilder.AppendLine($".export {exportStr}", 1); visitorContext.uasmBuilder.AppendLine($"{exportStr}:", 1); + + SymbolDefinition oldPropertyVal = visitorContext.topTable.GetGlobalSymbolTable().CreateNamedSymbol($"{VRC.Udon.Common.VariableChangedEvent.OLD_VALUE_PREFIX}{targetField.fieldSymbol.symbolUniqueName}", targetField.fieldSymbol.userCsType, SymbolDeclTypeFlags.Private); + visitorContext.uasmBuilder.AddCopy(setter.paramSymbol, targetField.fieldSymbol); + visitorContext.uasmBuilder.AddCopy(targetField.fieldSymbol, oldPropertyVal); } if ((node.Modifiers.HasModifier("public") && setter.declarationFlags == PropertyDeclFlags.None) || setter.declarationFlags == PropertyDeclFlags.Public) diff --git a/Assets/UdonSharp/Scripts/UdonSharpAttributes.cs b/Assets/UdonSharp/Scripts/UdonSharpAttributes.cs index 6d5ea216..d4f22f73 100644 --- a/Assets/UdonSharp/Scripts/UdonSharpAttributes.cs +++ b/Assets/UdonSharp/Scripts/UdonSharpAttributes.cs @@ -92,7 +92,7 @@ public RecursiveMethodAttribute() /// /// Calls the target property's setter when the marked field is modified by network sync or SetProgramVariable(). - /// The field will already be modified so use this to react to changes after the fact. + /// Fields marked with this will instead have the target property's setter called. The setter is expected to set the field if you want the field to change. /// [PublicAPI] [AttributeUsage(AttributeTargets.Field, AllowMultiple = false)] diff --git a/Assets/UdonSharp/Scripts/UdonSharpBehaviour.cs b/Assets/UdonSharp/Scripts/UdonSharpBehaviour.cs index f221ce69..6fd62870 100644 --- a/Assets/UdonSharp/Scripts/UdonSharpBehaviour.cs +++ b/Assets/UdonSharp/Scripts/UdonSharpBehaviour.cs @@ -34,24 +34,26 @@ public void SetProgramVariable(string name, object value) if (variableField != null) { - variableField.SetValue(this, value); - FieldChangeCallbackAttribute fieldChangeCallback = variableField.GetCustomAttribute(); - if (fieldChangeCallback == null) - return; + if (fieldChangeCallback != null) + { + PropertyInfo targetProperty = variableField.DeclaringType.GetProperty(fieldChangeCallback.CallbackPropertyName, BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Instance); - PropertyInfo targetProperty = variableField.DeclaringType.GetProperty(fieldChangeCallback.CallbackPropertyName, BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Instance); + if (targetProperty == null) + return; - if (targetProperty == null) - return; + MethodInfo setMethod = targetProperty.GetSetMethod(true); - MethodInfo setMethod = targetProperty.GetSetMethod(true); - - if (setMethod == null) - return; + if (setMethod == null) + return; - setMethod.Invoke(this, new object[] { variableField.GetValue(this) }); + setMethod.Invoke(this, new object[] { value }); + } + else + { + variableField.SetValue(this, value); + } } }