fix(weaver): #696 detect .mystruct = new MyStruct() changes with sync…

…vars (#702)

* clearing struct should set dirty bit

This reproduces issue #696

using .mystruct = default uses initobj instead of stfld, so the weaver misses it

* refactor: use index for so that we can insert instructions

* refactor: rename test

* fix: #696 use generated setter for .mystruct = new MyStruct()

* Update PropertySiteProcessor.cs

* Update PropertySiteProcessor.cs

Assets/Mirror/Editor/Weaver/Processors/PropertySiteProcessor.cs

@@ -85,11 +85,10 @@ static void ProcessSiteMethod(ModuleDefinition moduleDef, TypeDefinition td, Met
                     }
                 }
 
-                int iCount = 0;
-                foreach (Instruction i in md.Body.Instructions)
+                for (int iCount= 0; iCount < md.Body.Instructions.Count;)
                 {
-                    ProcessInstruction(moduleDef, td, md, i, iCount);
-                    iCount += 1;
+                    Instruction instr = md.Body.Instructions[iCount];
+                    iCount += ProcessInstruction(moduleDef, td, md, instr, iCount);
                 }
             }
         }
@@ -175,33 +174,84 @@ static void ProcessInstructionGetterField(TypeDefinition td, MethodDefinition md
             }
         }
 
-        static void ProcessInstruction(ModuleDefinition moduleDef, TypeDefinition td, MethodDefinition md, Instruction i, int iCount)
+        static int ProcessInstruction(ModuleDefinition moduleDef, TypeDefinition td, MethodDefinition md, Instruction instr, int iCount)
         {
-            if (i.OpCode == OpCodes.Call || i.OpCode == OpCodes.Callvirt)
+            if (instr.OpCode == OpCodes.Call || instr.OpCode == OpCodes.Callvirt)
             {
-                if (i.Operand is MethodReference opMethod)
+                if (instr.Operand is MethodReference opMethod)
                 {
-                    ProcessInstructionMethod(moduleDef, td, md, i, opMethod, iCount);
+                    ProcessInstructionMethod(moduleDef, td, md, instr, opMethod, iCount);
                 }
             }
 
-            if (i.OpCode == OpCodes.Stfld)
+            if (instr.OpCode == OpCodes.Stfld)
             {
                 // this instruction sets the value of a field. cache the field reference.
-                if (i.Operand is FieldDefinition opField)
+                if (instr.Operand is FieldDefinition opField)
                 {
-                    ProcessInstructionSetterField(td, md, i, opField);
+                    ProcessInstructionSetterField(td, md, instr, opField);
                 }
             }
 
-            if (i.OpCode == OpCodes.Ldfld)
+            if (instr.OpCode == OpCodes.Ldfld)
             {
                 // this instruction gets the value of a field. cache the field reference.
-                if (i.Operand is FieldDefinition opField)
+                if (instr.Operand is FieldDefinition opField)
                 {
-                    ProcessInstructionGetterField(td, md, i, opField);
+                    ProcessInstructionGetterField(td, md, instr, opField);
                 }
             }
+
+            if (instr.OpCode == OpCodes.Ldflda)
+            {
+                // loading a field by reference,  watch out for initobj instruction
+                // see https://github.com/vis2k/Mirror/issues/696
+
+                if (instr.Operand is FieldDefinition opField)
+                {
+                    return ProcessInstructionLoadAddress(td, md, instr, opField, iCount);
+                }
+            }
+
+            return 1;
+        }
+
+        private static int ProcessInstructionLoadAddress(TypeDefinition td, MethodDefinition md, Instruction instr, FieldDefinition opField, int iCount)
+        {
+            // dont replace property call sites in constructors
+            if (md.Name == ".ctor")
+                return 1;
+
+            // does it set a field that we replaced?
+            if (Weaver.WeaveLists.replacementSetterProperties.TryGetValue(opField, out MethodDefinition replacement))
+            {
+                // we have a replacement for this property
+                // is the next instruction a initobj?
+                Instruction nextInstr = md.Body.Instructions[iCount + 1];
+
+                if (nextInstr.OpCode == OpCodes.Initobj)
+                {
+                    // we need to replace this code with:
+                    //     var tmp = new MyStruct();
+                    //     this.set_Networkxxxx(tmp);
+                    ILProcessor worker = md.Body.GetILProcessor();
+                    VariableDefinition tmpVariable = new VariableDefinition(opField.FieldType);
+                    md.Body.Variables.Add(tmpVariable);
+
+                    worker.InsertBefore(instr, worker.Create(OpCodes.Ldloca, tmpVariable));
+                    worker.InsertBefore(instr, worker.Create(OpCodes.Initobj, opField.FieldType));
+                    worker.InsertBefore(instr, worker.Create(OpCodes.Ldloc, tmpVariable));
+                    worker.InsertBefore(instr, worker.Create(OpCodes.Call, replacement));
+
+                    worker.Remove(instr);
+                    worker.Remove(nextInstr);
+                    return 4;
+
+                }
+
+            }
+
+            return 1;
         }
 
         static void ProcessInstructionMethod(ModuleDefinition moduleDef, TypeDefinition td, MethodDefinition md, Instruction instr, MethodReference opMethodRef, int iCount)

Assets/Mirror/Tests/SyncVarTest.cs

@@ -26,17 +26,15 @@ public bool IsDirty()
     public class SyncVarTest
     {
 
-
-
         [Test]
-        public void TestSettingGuild()
+        public void TestSettingStruct()
         {
 
             GameObject gameObject = new GameObject();
 
             MockPlayer player = gameObject.AddComponent<MockPlayer>();
 
-            Assert.That(!player.IsDirty());
+            Assert.That(player.IsDirty(), Is.False, "First time object should not be dirty");
 
             MockPlayer.Guild myGuild = new MockPlayer.Guild
             {
@@ -45,7 +43,13 @@ public void TestSettingGuild()
 
             player.guild = myGuild;
 
-            Assert.That(player.IsDirty());
+            Assert.That(player.IsDirty(), "Setting struct should mark object as dirty");
+            player.ClearAllDirtyBits();
+            Assert.That(player.IsDirty(), Is.False, "ClearAllDirtyBits() should clear dirty flag");
+
+            // clearing the guild should set dirty bit too
+            player.guild = default;
+            Assert.That(player.IsDirty(), "Clearing struct should mark object as dirty");
         }
 
     }