From 569cc5bd145a4840be37abec69966c87d721be43 Mon Sep 17 00:00:00 2001
From: Anairkoen Schno <aaron.cirr.com@gmail.com>
Date: Tue, 1 Jun 2021 01:37:42 -0500
Subject: [PATCH] Dependency loops now don't kill everything

---
 .../DependencyResolutionLoopException.cs      | 21 ++++++++
 IPA.Loader/Loader/PluginLoader.cs             | 51 +++++++++++++++----
 2 files changed, 62 insertions(+), 10 deletions(-)
 create mode 100644 IPA.Loader/Loader/DependencyResolutionLoopException.cs

diff --git a/IPA.Loader/Loader/DependencyResolutionLoopException.cs b/IPA.Loader/Loader/DependencyResolutionLoopException.cs
new file mode 100644
index 00000000..3d7fd89b
--- /dev/null
+++ b/IPA.Loader/Loader/DependencyResolutionLoopException.cs
@@ -0,0 +1,21 @@
+using System;
+using System.Diagnostics.CodeAnalysis;
+
+namespace IPA.Loader
+{
+    [SuppressMessage("Design", "CA1064:Exceptions should be public", Justification = "This is only thrown and caught in local code")]
+    internal sealed class DependencyResolutionLoopException : Exception
+    {
+        public DependencyResolutionLoopException(string message) : base(message)
+        {
+        }
+
+        public DependencyResolutionLoopException(string message, Exception innerException) : base(message, innerException)
+        {
+        }
+
+        public DependencyResolutionLoopException()
+        {
+        }
+    }
+}
diff --git a/IPA.Loader/Loader/PluginLoader.cs b/IPA.Loader/Loader/PluginLoader.cs
index 60bacee6..64739d8b 100644
--- a/IPA.Loader/Loader/PluginLoader.cs
+++ b/IPA.Loader/Loader/PluginLoader.cs
@@ -357,7 +357,7 @@ namespace IPA.Loader
     public enum Reason
     {
         /// <summary>
-        /// An error was thrown either loading plugin information fomr disk, or when initializing the plugin.
+        /// An error was thrown either loading plugin information from disk, or when initializing the plugin.
         /// </summary>
         /// <remarks>
         /// When this is the set <see cref="Reason"/> in an <see cref="IgnoreReason"/> structure, the member
@@ -850,10 +850,37 @@ namespace IPA.Loader
                         meta = plugin.Meta;
                         if (!disabled)
                         {
-                            Resolve(plugin.Meta, ref disabled, out ignored);
+                            try
+                            {
+                                ignored = false;
+                                Resolve(plugin.Meta, ref disabled, out ignored);
+                            }
+                            catch (Exception e)
+                            {
+                                if (e is not DependencyResolutionLoopException)
+                                {
+                                    Logger.loader.Error($"While performing load order resolution for {id}:");
+                                    Logger.loader.Error(e);
+                                }
+
+                                if (!ignored)
+                                {
+                                    ignoredPlugins.Add(plugin.Meta, new(Reason.Error)
+                                    {
+                                        Error = e
+                                    });
+                                }
+
+                                ignored = true;
+                            }
+                        }
+
+                        if (!loadedPlugins.ContainsKey(id))
+                        {
+                            // this condition is specifically for when we fail resolution because of a graph loop
+                            Logger.loader.Trace($"- '{id}' resolved as ignored:{ignored},disabled:{disabled}");
+                            loadedPlugins.Add(id, (plugin.Meta, disabled, ignored));
                         }
-                        Logger.loader.Trace($"- '{id}' resolved as ignored:{ignored},disabled:{disabled}");
-                        loadedPlugins.Add(id, (plugin.Meta, disabled, ignored));
                         return true;
                     }
                     Logger.loader.Trace($"- Not found");
@@ -868,12 +895,10 @@ namespace IPA.Loader
                     if (isProcessing.Contains(plugin))
                     {
                         Logger.loader.Error($"Loop detected while processing '{plugin.Name}'; flagging as ignored");
-                        // we can't safely add it to ignoredPlugins, because then when the ignore propagates up the stack,
-                        //   we may end up ignoring outselves again
-                        ignored = true;
-                        return;
+                        throw new DependencyResolutionLoopException();
                     }
 
+                    isProcessing.Add(plugin);
                     using var _removeProcessing = Utils.ScopeGuard(() => isProcessing.Remove(plugin));
 
                     // if this method is being called, this is the first and only time that it has been called for this plugin.
@@ -994,8 +1019,14 @@ namespace IPA.Loader
                         }
                     }
 
-                    // we can now load the current plugin
-                    outputOrder!.Add(plugin);
+                    // specifically check if some strange stuff happened (like graph loops) causing this to be ignored
+                    // from some other invocation
+                    if (!ignoredPlugins.ContainsKey(plugin))
+                    {
+                        // we can now load the current plugin
+                        Logger.loader.Trace($"->'{plugin.Name}' loads here");
+                        outputOrder!.Add(plugin);
+                    }
 
                     // loadbefores have already been preprocessed into loadafters