Merge branch 'develop' of https://github.com/Pathoschild/SMAPI into android

 Conflicts:

	src/SMAPI/Metadata/InstructionMetadata.cs

docs/release-notes.md

@@ -15,6 +15,7 @@
   * Fixed broken URL in update alerts for unofficial versions.
   * Fixed rare error when a mod adds/removes event handlers asynchronously.
   * Fixed rare issue where the console showed incorrect colors when mods wrote to it asynchronously.
+  * Fixed SMAPI not always detecting broken field references in mod code.
   * Removed the experimental `RewriteInParallel` option added in SMAPI 3.6 (it was already disabled by default). Unfortunately this caused intermittent and unpredictable errors when enabled.
 
 * For modders:
@@ -25,12 +26,12 @@
 
 * For SMAPI developers:
   * The web API now returns an update alert in two new cases: any newer unofficial update (previously only shown if the mod was incompatible), and a newer prerelease version if the installed non-prerelease version is broken (previously only shown if the installed version was prerelease).
-  * Internal refactoring to simplify game updates:
+  * Reorganised SMAPI core to reduce coupling to `Game1`, make it easier to navigate, and simplify future game updates.
-    * Reorganised SMAPI core to reduce coupling to `Game1` and make it easier to navigate.
+  * SMAPI now automatically fixes code broken by these changes in game code, so manual rewriters are no longer needed:
-    * Added rewriter for any method broken due to new optional parameters.
+    * reference to a method with new optional parameters;
-    * Added rewriter for any field which was replaced by a property.
+    * reference to a field replaced by a property;
-    * `FieldReplaceRewriter` now supports mapping to a different target type.
+    * reference to a field replaced by a `const` field.
-  * Internal refactoring to simplify future game updates.
+  * `FieldReplaceRewriter` now supports mapping to a different target type.
 
 ## 3.6.2
 Released 02 August 2020 for Stardew Valley 1.4.1 or later.

src/SMAPI/Framework/Events/EventManager.cs

@@ -11,7 +11,7 @@ namespace StardewModdingAPI.Framework.Events
     internal class EventManager
     {
         /*********
-        ** Events (new)
+        ** Events
         *********/
         /****
         ** Display

src/SMAPI/Framework/ModLoading/Finders/ReferenceToMissingMemberFinder.cs

@@ -1,5 +1,4 @@
 using System.Collections.Generic;
-using System.Linq;
 using Mono.Cecil;
 using Mono.Cecil.Cil;
 using StardewModdingAPI.Framework.ModLoading.Framework;
@@ -35,8 +34,8 @@ namespace StardewModdingAPI.Framework.ModLoading.Finders
             FieldReference fieldRef = RewriteHelper.AsFieldReference(instruction);
             if (fieldRef != null && this.ShouldValidate(fieldRef.DeclaringType))
             {
-                FieldDefinition target = fieldRef.DeclaringType.Resolve()?.Fields.FirstOrDefault(p => p.Name == fieldRef.Name);
+                FieldDefinition target = fieldRef.Resolve();
-                if (target == null)
+                if (target == null || target.HasConstant)
                 {
                     this.MarkFlag(InstructionHandleResult.NotCompatible, $"reference to {fieldRef.DeclaringType.FullName}.{fieldRef.Name} (no such field)");
                     return false;

src/SMAPI/Framework/ModLoading/Framework/RewriteHelper.cs

@@ -67,6 +67,24 @@ namespace StardewModdingAPI.Framework.ModLoading.Framework
                 : null;
         }
 
+        /// <summary>Get the CIL instruction to load a value onto the stack.</summary>
+        /// <param name="rawValue">The constant value to inject.</param>
+        /// <returns>Returns the instruction, or <c>null</c> if the value type isn't supported.</returns>
+        public static Instruction GetLoadValueInstruction(object rawValue)
+        {
+            return rawValue switch
+            {
+                null => Instruction.Create(OpCodes.Ldnull),
+                bool value => Instruction.Create(value ? OpCodes.Ldc_I4_1 : OpCodes.Ldc_I4_0),
+                int value => Instruction.Create(OpCodes.Ldc_I4, value), // int32
+                long value => Instruction.Create(OpCodes.Ldc_I8, value), // int64
+                float value => Instruction.Create(OpCodes.Ldc_R4, value), // float32
+                double value => Instruction.Create(OpCodes.Ldc_R8, value), // float64
+                string value => Instruction.Create(OpCodes.Ldstr, value),
+                _ => null
+            };
+        }
+
         /// <summary>Get whether a type matches a type reference.</summary>
         /// <param name="type">The defined type.</param>
         /// <param name="reference">The type reference.</param>

src/SMAPI/Framework/ModLoading/Rewriters/HeuristicFieldRewriter.cs

@@ -6,7 +6,7 @@ using StardewModdingAPI.Framework.ModLoading.Framework;
 
 namespace StardewModdingAPI.Framework.ModLoading.Rewriters
 {
-    /// <summary>Automatically fix references to fields that have been replaced by a property.</summary>
+    /// <summary>Automatically fix references to fields that have been replaced by a property or const field.</summary>
     internal class HeuristicFieldRewriter : BaseInstructionHandler
     {
         /*********
@@ -36,23 +36,16 @@ namespace StardewModdingAPI.Framework.ModLoading.Rewriters
                 return false;
 
             // skip if not broken
-            if (fieldRef.Resolve() != null)
+            FieldDefinition fieldDefinition = fieldRef.Resolve();
+            if (fieldDefinition != null && !fieldDefinition.HasConstant)
                 return false;
 
-            // get equivalent property
+            // rewrite if possible
-            PropertyDefinition property = fieldRef.DeclaringType.Resolve().Properties.FirstOrDefault(p => p.Name == fieldRef.Name);
+            TypeDefinition declaringType = fieldRef.DeclaringType.Resolve();
-            MethodDefinition method = instruction.OpCode == OpCodes.Ldsfld || instruction.OpCode == OpCodes.Ldfld
+            bool isRead = instruction.OpCode == OpCodes.Ldsfld || instruction.OpCode == OpCodes.Ldfld;
-                ? property?.GetMethod
+            return
-                : property?.SetMethod;
+                this.TryRewriteToProperty(module, instruction, fieldRef, declaringType, isRead)
-            if (method == null)
+                || this.TryRewriteToConstField(instruction, fieldDefinition);
-                return false;
-
-            // rewrite field to property
-            instruction.OpCode = OpCodes.Call;
-            instruction.Operand = module.ImportReference(method);
-
-            this.Phrases.Add($"{fieldRef.DeclaringType.Name}.{fieldRef.Name} (field => property)");
-            return this.MarkRewritten();
         }
 
 
@@ -65,5 +58,49 @@ namespace StardewModdingAPI.Framework.ModLoading.Rewriters
         {
             return type != null && this.RewriteReferencesToAssemblies.Contains(type.Scope.Name);
         }
+
+        /// <summary>Try rewriting the field into a matching property.</summary>
+        /// <param name="module">The assembly module containing the instruction.</param>
+        /// <param name="instruction">The CIL instruction to rewrite.</param>
+        /// <param name="fieldRef">The field reference.</param>
+        /// <param name="declaringType">The type on which the field was defined.</param>
+        /// <param name="isRead">Whether the field is being read; else it's being written to.</param>
+        private bool TryRewriteToProperty(ModuleDefinition module, Instruction instruction, FieldReference fieldRef, TypeDefinition declaringType, bool isRead)
+        {
+            // get equivalent property
+            PropertyDefinition property = declaringType.Properties.FirstOrDefault(p => p.Name == fieldRef.Name);
+            MethodDefinition method = isRead ? property?.GetMethod : property?.SetMethod;
+            if (method == null)
+                return false;
+
+            // rewrite field to property
+            instruction.OpCode = OpCodes.Call;
+            instruction.Operand = module.ImportReference(method);
+
+            this.Phrases.Add($"{fieldRef.DeclaringType.Name}.{fieldRef.Name} (field => property)");
+            return this.MarkRewritten();
+        }
+
+        /// <summary>Try rewriting the field into a matching const field.</summary>
+        /// <param name="instruction">The CIL instruction to rewrite.</param>
+        /// <param name="field">The field definition.</param>
+        private bool TryRewriteToConstField(Instruction instruction, FieldDefinition field)
+        {
+            // must have been a static field read, and the new field must be const
+            if (instruction.OpCode != OpCodes.Ldsfld || field?.HasConstant != true)
+                return false;
+
+            // get opcode for value type
+            Instruction loadInstruction = RewriteHelper.GetLoadValueInstruction(field.Constant);
+            if (loadInstruction == null)
+                return false;
+
+            // rewrite to constant
+            instruction.OpCode = loadInstruction.OpCode;
+            instruction.Operand = loadInstruction.Operand;
+
+            this.Phrases.Add($"{field.DeclaringType.Name}.{field.Name} (field => const)");
+            return this.MarkRewritten();
+        }
     }
 }

src/SMAPI/Framework/ModLoading/Rewriters/HeuristicMethodRewriter.cs

@@ -64,7 +64,7 @@ namespace StardewModdingAPI.Framework.ModLoading.Rewriters
 
             // get instructions to inject parameter values
             var loadInstructions = method.Parameters.Skip(methodRef.Parameters.Count)
-                .Select(p => this.GetLoadValueInstruction(p.Constant))
+                .Select(p => RewriteHelper.GetLoadValueInstruction(p.Constant))
                 .ToArray();
             if (loadInstructions.Any(p => p == null))
                 return false; // SMAPI needs to load the value onto the stack before the method call, but the optional parameter type wasn't recognized
@@ -105,23 +105,5 @@ namespace StardewModdingAPI.Framework.ModLoading.Rewriters
 
             return true;
         }
-
-        /// <summary>Get the CIL instruction to load a value onto the stack.</summary>
-        /// <param name="rawValue">The constant value to inject.</param>
-        /// <returns>Returns the instruction, or <c>null</c> if the value type isn't supported.</returns>
-        private Instruction GetLoadValueInstruction(object rawValue)
-        {
-            return rawValue switch
-            {
-                null => Instruction.Create(OpCodes.Ldnull),
-                bool value => Instruction.Create(value ? OpCodes.Ldc_I4_1 : OpCodes.Ldc_I4_0),
-                int value => Instruction.Create(OpCodes.Ldc_I4, value), // int32
-                long value => Instruction.Create(OpCodes.Ldc_I8, value), // int64
-                float value => Instruction.Create(OpCodes.Ldc_R4, value), // float32
-                double value => Instruction.Create(OpCodes.Ldc_R8, value), // float64
-                string value => Instruction.Create(OpCodes.Ldstr, value),
-                _ => null
-            };
-        }
     }
 }

src/SMAPI/Framework/ModLoading/Rewriters/StaticFieldToConstantRewriter.cs

@@ -1,65 +0,0 @@
-using System;
-using Mono.Cecil;
-using Mono.Cecil.Cil;
-using StardewModdingAPI.Framework.ModLoading.Framework;
-
-namespace StardewModdingAPI.Framework.ModLoading.Rewriters
-{
-    /// <summary>Rewrites static field references into constant values.</summary>
-    /// <typeparam name="TValue">The constant value type.</typeparam>
-    internal class StaticFieldToConstantRewriter<TValue> : BaseInstructionHandler
-    {
-        /*********
-        ** Fields
-        *********/
-        /// <summary>The type containing the field to which references should be rewritten.</summary>
-        private readonly Type Type;
-
-        /// <summary>The field name to which references should be rewritten.</summary>
-        private readonly string FromFieldName;
-
-        /// <summary>The constant value to replace with.</summary>
-        private readonly TValue Value;
-
-
-        /*********
-        ** Public methods
-        *********/
-        /// <summary>Construct an instance.</summary>
-        /// <param name="type">The type whose field to which references should be rewritten.</param>
-        /// <param name="fieldName">The field name to rewrite.</param>
-        /// <param name="value">The constant value to replace with.</param>
-        public StaticFieldToConstantRewriter(Type type, string fieldName, TValue value)
-            : base(defaultPhrase: $"{type.FullName}.{fieldName} field")
-        {
-            this.Type = type;
-            this.FromFieldName = fieldName;
-            this.Value = value;
-        }
-
-        /// <inheritdoc />
-        public override bool Handle(ModuleDefinition module, ILProcessor cil, Instruction instruction)
-        {
-            // get field reference
-            FieldReference fieldRef = RewriteHelper.AsFieldReference(instruction);
-            if (!RewriteHelper.IsFieldReferenceTo(fieldRef, this.Type.FullName, this.FromFieldName))
-                return false;
-
-            // rewrite to constant
-            if (typeof(TValue) == typeof(int))
-            {
-                instruction.OpCode = OpCodes.Ldc_I4;
-                instruction.Operand = this.Value;
-            }
-            else if (typeof(TValue) == typeof(string))
-            {
-                instruction.OpCode = OpCodes.Ldstr;
-                instruction.Operand = this.Value;
-            }
-            else
-                throw new NotSupportedException($"Rewriting to constant values of type {typeof(TValue)} isn't currently supported.");
-
-            return this.MarkRewritten();
-        }
-    }
-}

src/SMAPI/Metadata/InstructionMetadata.cs

@@ -106,8 +106,6 @@ namespace StardewModdingAPI.Metadata
             yield return new FieldReplaceRewriter(typeof(ItemGrabMenu), "context", "specialObject");
 
 #endif
-            // rewrite for Stardew Valley 1.3
-            yield return new StaticFieldToConstantRewriter<int>(typeof(Game1), "tileSize", Game1.tileSize);
 
             // heuristic rewrites
             yield return new HeuristicFieldRewriter(this.ValidateReferencesToAssemblies);