migrate to Harmony 2.0 finalizers (#711)

5 files changed, 54 insertions, 156 deletions

diff --git a/src/SMAPI/Framework/Patching/PatchHelper.cs b/src/SMAPI/Framework/Patching/PatchHelper.cs
deleted file mode 100644
index 4cb436f0..00000000
--- a/src/SMAPI/Framework/Patching/PatchHelper.cs
+++ /dev/null
@@ -1,34 +0,0 @@
-using System;
-using System.Collections.Generic;
-
-namespace StardewModdingAPI.Framework.Patching
-{
-    /// <summary>Provides generic methods for implementing Harmony patches.</summary>
-    internal class PatchHelper
-    {
-        /*********
-        ** Fields
-        *********/
-        /// <summary>The interception keys currently being intercepted.</summary>
-        private static readonly HashSet<string> InterceptingKeys = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
-
-
-        /*********
-        ** Public methods
-        *********/
-        /// <summary>Track a method that will be intercepted.</summary>
-        /// <param name="key">The intercept key.</param>
-        /// <returns>Returns false if the method was already marked for interception, else true.</returns>
-        public static bool StartIntercept(string key)
-        {
-            return PatchHelper.InterceptingKeys.Add(key);
-        }
-
-        /// <summary>Track a method as no longer being intercepted.</summary>
-        /// <param name="key">The intercept key.</param>
-        public static void StopIntercept(string key)
-        {
-            PatchHelper.InterceptingKeys.Remove(key);
-        }
-    }
-}
diff --git a/src/SMAPI/Patches/DialogueErrorPatch.cs b/src/SMAPI/Patches/DialogueErrorPatch.cs
index 80029540..cddf29d6 100644
--- a/src/SMAPI/Patches/DialogueErrorPatch.cs
+++ b/src/SMAPI/Patches/DialogueErrorPatch.cs
@@ -1,8 +1,8 @@
 using System;
 using System.Collections.Generic;
 using System.Diagnostics.CodeAnalysis;
-using System.Reflection;
 using HarmonyLib;
+using StardewModdingAPI.Framework;
 using StardewModdingAPI.Framework.Patching;
 using StardewModdingAPI.Framework.Reflection;
 using StardewValley;
@@ -51,11 +51,11 @@ namespace StardewModdingAPI.Patches
         {
             harmony.Patch(
                 original: AccessTools.Constructor(typeof(Dialogue), new[] { typeof(string), typeof(NPC) }),
-                prefix: new HarmonyMethod(this.GetType(), nameof(DialogueErrorPatch.Before_Dialogue_Constructor))
+                finalizer: new HarmonyMethod(this.GetType(), nameof(DialogueErrorPatch.Finalize_Dialogue_Constructor))
             );
             harmony.Patch(
                 original: AccessTools.Property(typeof(NPC), nameof(NPC.CurrentDialogue)).GetMethod,
-                prefix: new HarmonyMethod(this.GetType(), nameof(DialogueErrorPatch.Before_NPC_CurrentDialogue))
+                finalizer: new HarmonyMethod(this.GetType(), nameof(DialogueErrorPatch.Finalize_NPC_CurrentDialogue))
             );
         }
 
@@ -63,71 +63,44 @@ namespace StardewModdingAPI.Patches
         /*********
         ** Private methods
         *********/
-        /// <summary>The method to call instead of the Dialogue constructor.</summary>
+        /// <summary>The method to call after the Dialogue constructor.</summary>
         /// <param name="__instance">The instance being patched.</param>
         /// <param name="masterDialogue">The dialogue being parsed.</param>
         /// <param name="speaker">The NPC for which the dialogue is being parsed.</param>
-        /// <returns>Returns whether to execute the original method.</returns>
-        private static bool Before_Dialogue_Constructor(Dialogue __instance, string masterDialogue, NPC speaker)
+        /// <param name="__exception">The exception thrown by the wrapped method, if any.</param>
+        /// <returns>Returns the exception to throw, if any.</returns>
+        private static Exception Finalize_Dialogue_Constructor(Dialogue __instance, string masterDialogue, NPC speaker, Exception __exception)
         {
-            // get private members
-            bool nameArraysTranslated = DialogueErrorPatch.Reflection.GetField<bool>(typeof(Dialogue), "nameArraysTranslated").GetValue();
-            IReflectedMethod translateArraysOfStrings = DialogueErrorPatch.Reflection.GetMethod(typeof(Dialogue), "TranslateArraysOfStrings");
-            IReflectedMethod parseDialogueString = DialogueErrorPatch.Reflection.GetMethod(__instance, "parseDialogueString");
-            IReflectedMethod checkForSpecialDialogueAttributes = DialogueErrorPatch.Reflection.GetMethod(__instance, "checkForSpecialDialogueAttributes");
-            IReflectedField<List<string>> dialogues = DialogueErrorPatch.Reflection.GetField<List<string>>(__instance, "dialogues");
-
-            // replicate base constructor
-            if (dialogues.GetValue() == null)
-                dialogues.SetValue(new List<string>());
-
-            // duplicate code with try..catch
-            try
-            {
-                if (!nameArraysTranslated)
-                    translateArraysOfStrings.Invoke();
-                __instance.speaker = speaker;
-                parseDialogueString.Invoke(masterDialogue);
-                checkForSpecialDialogueAttributes.Invoke();
-            }
-            catch (Exception baseEx) when (baseEx.InnerException is TargetInvocationException invocationEx && invocationEx.InnerException is Exception ex)
+            if (__exception != null)
             {
+                // log message
                 string name = !string.IsNullOrWhiteSpace(speaker?.Name) ? speaker.Name : null;
-                DialogueErrorPatch.MonitorForGame.Log($"Failed parsing dialogue string{(name != null ? $" for {name}" : "")}:\n{masterDialogue}\n{ex}", LogLevel.Error);
+                DialogueErrorPatch.MonitorForGame.Log($"Failed parsing dialogue string{(name != null ? $" for {name}" : "")}:\n{masterDialogue}\n{__exception.GetLogSummary()}", LogLevel.Error);
 
+                // set default dialogue
+                IReflectedMethod parseDialogueString = DialogueErrorPatch.Reflection.GetMethod(__instance, "parseDialogueString");
+                IReflectedMethod checkForSpecialDialogueAttributes = DialogueErrorPatch.Reflection.GetMethod(__instance, "checkForSpecialDialogueAttributes");
                 parseDialogueString.Invoke("...");
                 checkForSpecialDialogueAttributes.Invoke();
             }
 
-            return false;
+            return null;
         }
 
-        /// <summary>The method to call instead of <see cref="NPC.CurrentDialogue"/>.</summary>
+        /// <summary>The method to call after <see cref="NPC.CurrentDialogue"/>.</summary>
         /// <param name="__instance">The instance being patched.</param>
         /// <param name="__result">The return value of the original method.</param>
-        /// <param name="__originalMethod">The method being wrapped.</param>
-        /// <returns>Returns whether to execute the original method.</returns>
-        private static bool Before_NPC_CurrentDialogue(NPC __instance, ref Stack<Dialogue> __result, MethodInfo __originalMethod)
+        /// <param name="__exception">The exception thrown by the wrapped method, if any.</param>
+        /// <returns>Returns the exception to throw, if any.</returns>
+        private static Exception Finalize_NPC_CurrentDialogue(NPC __instance, ref Stack<Dialogue> __result, Exception __exception)
         {
-            const string key = nameof(Before_NPC_CurrentDialogue);
-            if (!PatchHelper.StartIntercept(key))
-                return true;
+            if (__exception == null)
+                return null;
 
-            try
-            {
-                __result = (Stack<Dialogue>)__originalMethod.Invoke(__instance, new object[0]);
-                return false;
-            }
-            catch (TargetInvocationException ex)
-            {
-                DialogueErrorPatch.MonitorForGame.Log($"Failed loading current dialogue for NPC {__instance.Name}:\n{ex.InnerException ?? ex}", LogLevel.Error);
-                __result = new Stack<Dialogue>();
-                return false;
-            }
-            finally
-            {
-                PatchHelper.StopIntercept(key);
-            }
+            DialogueErrorPatch.MonitorForGame.Log($"Failed loading current dialogue for NPC {__instance.Name}:\n{__exception.GetLogSummary()}", LogLevel.Error);
+            __result = new Stack<Dialogue>();
+
+            return null;
         }
     }
 }
diff --git a/src/SMAPI/Patches/EventErrorPatch.cs b/src/SMAPI/Patches/EventErrorPatch.cs
index a2b94e8b..de9dea29 100644
--- a/src/SMAPI/Patches/EventErrorPatch.cs
+++ b/src/SMAPI/Patches/EventErrorPatch.cs
@@ -1,5 +1,5 @@
+using System;
 using System.Diagnostics.CodeAnalysis;
-using System.Reflection;
 using HarmonyLib;
 using StardewModdingAPI.Framework.Patching;
 using StardewValley;
@@ -42,7 +42,7 @@ namespace StardewModdingAPI.Patches
         {
             harmony.Patch(
                 original: AccessTools.Method(typeof(GameLocation), "checkEventPrecondition"),
-                prefix: new HarmonyMethod(this.GetType(), nameof(EventErrorPatch.Before_GameLocation_CheckEventPrecondition))
+                finalizer: new HarmonyMethod(this.GetType(), nameof(EventErrorPatch.Finalize_GameLocation_CheckEventPrecondition))
             );
         }
 
@@ -51,32 +51,19 @@ namespace StardewModdingAPI.Patches
         ** Private methods
         *********/
         /// <summary>The method to call instead of the GameLocation.CheckEventPrecondition.</summary>
-        /// <param name="__instance">The instance being patched.</param>
         /// <param name="__result">The return value of the original method.</param>
         /// <param name="precondition">The precondition to be parsed.</param>
-        /// <param name="__originalMethod">The method being wrapped.</param>
-        /// <returns>Returns whether to execute the original method.</returns>
-        private static bool Before_GameLocation_CheckEventPrecondition(GameLocation __instance, ref int __result, string precondition, MethodInfo __originalMethod)
+        /// <param name="__exception">The exception thrown by the wrapped method, if any.</param>
+        /// <returns>Returns the exception to throw, if any.</returns>
+        private static Exception Finalize_GameLocation_CheckEventPrecondition(ref int __result, string precondition, Exception __exception)
         {
-            const string key = nameof(Before_GameLocation_CheckEventPrecondition);
-            if (!PatchHelper.StartIntercept(key))
-                return true;
-
-            try
-            {
-                __result = (int)__originalMethod.Invoke(__instance, new object[] { precondition });
-                return false;
-            }
-            catch (TargetInvocationException ex)
+            if (__exception != null)
             {
                 __result = -1;
-                EventErrorPatch.MonitorForGame.Log($"Failed parsing event precondition ({precondition}):\n{ex.InnerException}", LogLevel.Error);
-                return false;
-            }
-            finally
-            {
-                PatchHelper.StopIntercept(key);
+                EventErrorPatch.MonitorForGame.Log($"Failed parsing event precondition ({precondition}):\n{__exception.InnerException}", LogLevel.Error);
             }
+
+            return null;
         }
     }
 }
diff --git a/src/SMAPI/Patches/ObjectErrorPatch.cs b/src/SMAPI/Patches/ObjectErrorPatch.cs
index b9655043..189a14a0 100644
--- a/src/SMAPI/Patches/ObjectErrorPatch.cs
+++ b/src/SMAPI/Patches/ObjectErrorPatch.cs
@@ -1,6 +1,6 @@
+using System;
 using System.Collections.Generic;
 using System.Diagnostics.CodeAnalysis;
-using System.Reflection;
 using HarmonyLib;
 using StardewModdingAPI.Framework.Patching;
 using StardewValley;
@@ -38,7 +38,7 @@ namespace StardewModdingAPI.Patches
             // object.getDisplayName
             harmony.Patch(
                 original: AccessTools.Method(typeof(SObject), "loadDisplayName"),
-                prefix: new HarmonyMethod(this.GetType(), nameof(ObjectErrorPatch.Before_Object_loadDisplayName))
+                finalizer: new HarmonyMethod(this.GetType(), nameof(ObjectErrorPatch.Finalize_Object_loadDisplayName))
             );
 
             // IClickableMenu.drawToolTip
@@ -68,42 +68,25 @@ namespace StardewModdingAPI.Patches
             return true;
         }
 
-        /// <summary>The method to call instead of <see cref="StardewValley.Object.loadDisplayName"/>.</summary>
-        /// <param name="__instance">The instance being patched.</param>
+        /// <summary>The method to call after <see cref="StardewValley.Object.loadDisplayName"/>.</summary>
         /// <param name="__result">The patched method's return value.</param>
-        /// <param name="__originalMethod">The method being wrapped.</param>
-        /// <returns>Returns whether to execute the original method.</returns>
-        private static bool Before_Object_loadDisplayName(SObject __instance, ref string __result, MethodInfo __originalMethod)
+        /// <param name="__exception">The exception thrown by the wrapped method, if any.</param>
+        /// <returns>Returns the exception to throw, if any.</returns>
+        private static Exception Finalize_Object_loadDisplayName(ref string __result, Exception __exception)
         {
-            const string key = nameof(Before_Object_loadDisplayName);
-            if (!PatchHelper.StartIntercept(key))
-                return true;
-
-            try
-            {
-                __result = (string)__originalMethod.Invoke(__instance, new object[0]);
-                return false;
-            }
-            catch (TargetInvocationException ex) when (ex.InnerException is KeyNotFoundException)
+            if (__exception is KeyNotFoundException)
             {
                 __result = "???";
-                return false;
-            }
-            catch
-            {
-                return true;
-            }
-            finally
-            {
-                PatchHelper.StopIntercept(key);
+                return null;
             }
+
+            return __exception;
         }
 
         /// <summary>The method to call instead of <see cref="IClickableMenu.drawToolTip"/>.</summary>
-        /// <param name="__instance">The instance being patched.</param>
         /// <param name="hoveredItem">The item for which to draw a tooltip.</param>
         /// <returns>Returns whether to execute the original method.</returns>
-        private static bool Before_IClickableMenu_DrawTooltip(IClickableMenu __instance, Item hoveredItem)
+        private static bool Before_IClickableMenu_DrawTooltip(Item hoveredItem)
         {
             // invalid edible item cause crash when drawing tooltips
             if (hoveredItem is SObject obj && obj.Edibility != -300 && !Game1.objectInformation.ContainsKey(obj.ParentSheetIndex))
diff --git a/src/SMAPI/Patches/ScheduleErrorPatch.cs b/src/SMAPI/Patches/ScheduleErrorPatch.cs
index 386230a6..df6ffab3 100644
--- a/src/SMAPI/Patches/ScheduleErrorPatch.cs
+++ b/src/SMAPI/Patches/ScheduleErrorPatch.cs
@@ -1,7 +1,8 @@
+using System;
 using System.Collections.Generic;
 using System.Diagnostics.CodeAnalysis;
-using System.Reflection;
 using HarmonyLib;
+using StardewModdingAPI.Framework;
 using StardewModdingAPI.Framework.Patching;
 using StardewValley;
 
@@ -43,7 +44,7 @@ namespace StardewModdingAPI.Patches
         {
             harmony.Patch(
                 original: AccessTools.Method(typeof(NPC), "parseMasterSchedule"),
-                prefix: new HarmonyMethod(this.GetType(), nameof(ScheduleErrorPatch.Before_NPC_parseMasterSchedule))
+                finalizer: new HarmonyMethod(this.GetType(), nameof(ScheduleErrorPatch.Finalize_NPC_parseMasterSchedule))
             );
         }
 
@@ -55,29 +56,17 @@ namespace StardewModdingAPI.Patches
         /// <param name="rawData">The raw schedule data to parse.</param>
         /// <param name="__instance">The instance being patched.</param>
         /// <param name="__result">The patched method's return value.</param>
-        /// <param name="__originalMethod">The method being wrapped.</param>
-        /// <returns>Returns whether to execute the original method.</returns>
-        private static bool Before_NPC_parseMasterSchedule(string rawData, NPC __instance, ref Dictionary<int, SchedulePathDescription> __result, MethodInfo __originalMethod)
+        /// <param name="__exception">The exception thrown by the wrapped method, if any.</param>
+        /// <returns>Returns the exception to throw, if any.</returns>
+        private static Exception Finalize_NPC_parseMasterSchedule(string rawData, NPC __instance, ref Dictionary<int, SchedulePathDescription> __result, Exception __exception)
         {
-            const string key = nameof(Before_NPC_parseMasterSchedule);
-            if (!PatchHelper.StartIntercept(key))
-                return true;
-
-            try
-            {
-                __result = (Dictionary<int, SchedulePathDescription>)__originalMethod.Invoke(__instance, new object[] { rawData });
-                return false;
-            }
-            catch (TargetInvocationException ex)
+            if (__exception != null)
             {
-                ScheduleErrorPatch.MonitorForGame.Log($"Failed parsing schedule for NPC {__instance.Name}:\n{rawData}\n{ex.InnerException ?? ex}", LogLevel.Error);
+                ScheduleErrorPatch.MonitorForGame.Log($"Failed parsing schedule for NPC {__instance.Name}:\n{rawData}\n{__exception.GetLogSummary()}", LogLevel.Error);
                 __result = new Dictionary<int, SchedulePathDescription>();
-                return false;
-            }
-            finally
-            {
-                PatchHelper.StopIntercept(key);
             }
+
+            return null;
         }
     }
 }