Fix loadBefore/conflictsWith to behave correctly

IPA.Loader/Loader/PluginLoader.cs

@@ -811,15 +811,41 @@ namespace IPA.Loader
             }
 
             // preprocess LoadBefore into LoadAfter
-            foreach (var kvp in metadataCache)
+            foreach (var (_, (meta, _)) in metadataCache)
             { // we iterate the metadata cache because it contains both disabled and enabled plugins
-                var loadBefore = kvp.Value.Meta.Manifest.LoadBefore;
+                var loadBefore = meta.Manifest.LoadBefore;
                 foreach (var id in loadBefore)
                 {
                     if (metadataCache.TryGetValue(id, out var plugin))
                     {
                         // if the id exists in our metadata cache, make sure it knows to load after the plugin in kvp
-                        _ = plugin.Meta.LoadsAfter.Add(kvp.Value.Meta);
+                        _ = plugin.Meta.LoadsAfter.Add(meta);
+                    }
+                }
+            }
+
+            // preprocess conflicts to be mutual
+            foreach (var (_, (meta, _)) in metadataCache)
+            {
+                foreach (var (id, range) in meta.Manifest.Conflicts)
+                {
+                    if (metadataCache.TryGetValue(id, out var plugin)
+                        && range.IsSatisfied(plugin.Meta.Version))
+                    {
+                        // make sure that there's a mutual dependency
+                        var targetRange = new Range($"={meta.Version}", true);
+                        var targetConflicts = plugin.Meta.Manifest.Conflicts;
+                        if (!targetConflicts.TryGetValue(meta.Id, out var realRange))
+                        {
+                            // there's not already a listed conflict
+                            targetConflicts.Add(meta.Id, targetRange);
+                        }
+                        else if (!realRange.IsSatisfied(meta.Version))
+                        {
+                            // there is already a listed conflict that isn't mutual
+                            targetRange = new Range($"{realRange} || {targetRange}", true);
+                            targetConflicts[meta.Id] = targetRange;
+                        }
                     }
                 }
             }
@@ -829,12 +855,12 @@ namespace IPA.Loader
             var isProcessing = new HashSet<PluginMetadata>();
 
             {
-                bool TryResolveId(string id, [MaybeNullWhen(false)] out PluginMetadata meta, out bool disabled, out bool ignored)
+                bool TryResolveId(string id, [MaybeNullWhen(false)] out PluginMetadata meta, out bool disabled, out bool ignored, bool partial = false)
                 {
                     meta = null;
                     disabled = false;
                     ignored = true;
-                    Logger.loader.Trace($"Trying to resolve plugin '{id}'");
+                    Logger.loader.Trace($"Trying to resolve plugin '{id}' partial:{partial}");
                     if (loadedPlugins.TryGetValue(id, out var foundMeta))
                     {
                         meta = foundMeta.Meta;
@@ -846,6 +872,12 @@ namespace IPA.Loader
                     if (metadataCache!.TryGetValue(id, out var plugin))
                     {
                         Logger.loader.Trace($"- In metadata cache");
+                        if (partial)
+                        {
+                            Logger.loader.Trace($"  - but requested in a partial lookup");
+                            return false;
+                        }
+
                         disabled = !plugin.Enabled;
                         meta = plugin.Meta;
                         if (!disabled)
@@ -1003,7 +1035,8 @@ namespace IPA.Loader
                     foreach (var conflict in plugin.Manifest.Conflicts)
                     {
                         Logger.loader.Trace($">- Checking conflict '{conflict.Key}' {conflict.Value}");
-                        if (TryResolveId(conflict.Key, out var meta, out var conflDisabled, out var conflIgnored)
+                        // this lookup must be partial to prevent loadBefore/conflictsWith from creating a recursion loop
+                        if (TryResolveId(conflict.Key, out var meta, out var conflDisabled, out var conflIgnored, partial: true)
                             && conflict.Value.IsSatisfied(meta.Version)
                             && !conflIgnored && !conflDisabled) // the conflict is only *actually* a problem if it is both not ignored and not disabled
                         {

IPA.Loader/Loader/PluginMetadata.cs

@@ -144,6 +144,6 @@ namespace IPA.Loader
         /// Gets all of the metadata as a readable string.
         /// </summary>
         /// <returns>the readable printable metadata string</returns>
-        public override string ToString() => $"{Name}({Id}@{Version})({PluginType?.FullName}) from '{Utils.GetRelativePath(File?.FullName, UnityGame.InstallPath)}'";
+        public override string ToString() => $"{Name}({Id}@{Version})({PluginType?.FullName}) from '{Utils.GetRelativePath(File?.FullName ?? "", UnityGame.InstallPath)}'";
     }
 }

IPA.Loader/Utilities/Utils.cs

@@ -1,4 +1,5 @@
-using System;
+#nullable enable
+using System;
 using System.IO;
 using System.Text;
 using System.Linq;
@@ -6,6 +7,7 @@ using System.Collections.Generic;
 using Mono.Cecil;
 using System.Runtime.CompilerServices;
 using System.Threading;
+using System.Diagnostics.CodeAnalysis;
 #if NET3
 using File = Net3_Proxy.File;
 #endif
@@ -24,6 +26,8 @@ namespace IPA.Utilities
         /// <returns>the corresponding byte array</returns>
         public static byte[] StringToByteArray(string hex)
         {
+            if (hex is null)
+                throw new ArgumentNullException(nameof(hex));
             int numberChars = hex.Length;
             byte[] bytes = new byte[numberChars / 2];
             for (int i = 0; i < numberChars; i += 2)
@@ -38,9 +42,11 @@ namespace IPA.Utilities
         /// <returns>the hex form of the array</returns>
         public static string ByteArrayToString(byte[] ba)
         {
-            StringBuilder hex = new StringBuilder(ba.Length * 2);
+            if (ba is null)
+                throw new ArgumentNullException(nameof(ba));
+            var hex = new StringBuilder(ba.Length * 2);
             foreach (byte b in ba)
-                hex.AppendFormat("{0:x2}", b);
+                _ = hex.AppendFormat("{0:x2}", b);
             return hex.ToString();
         }
 
@@ -64,9 +70,9 @@ namespace IPA.Utilities
                 byte* x1 = p1, x2 = p2;
                 int l = a1.Length;
                 for (int i = 0; i < l / 8; i++, x1 += 8, x2 += 8)
-                    if (*((long*)x1) != *((long*)x2)) return false;
-                if ((l & 4) != 0) { if (*((int*)x1) != *((int*)x2)) return false; x1 += 4; x2 += 4; }
-                if ((l & 2) != 0) { if (*((short*)x1) != *((short*)x2)) return false; x1 += 2; x2 += 2; }
+                    if (*(long*)x1 != *(long*)x2) return false;
+                if ((l & 4) != 0) { if (*(int*)x1 != *(int*)x2) return false; x1 += 4; x2 += 4; }
+                if ((l & 2) != 0) { if (*(short*)x1 != *(short*)x2) return false; x1 += 2; x2 += 2; }
                 if ((l & 1) != 0) if (*x1 != *x2) return false;
                 return true;
             }
@@ -80,13 +86,18 @@ namespace IPA.Utilities
         /// <returns>a path to get from <paramref name="folder"/> to <paramref name="file"/></returns>
         public static string GetRelativePath(string file, string folder)
         {
-            Uri pathUri = new Uri(file);
+            if (file is null)
+                throw new ArgumentNullException(nameof(file));
+            if (folder is null)
+                throw new ArgumentNullException(nameof(folder));
+
+            var pathUri = new Uri(file);
             // Folders must end in a slash
-            if (!folder.EndsWith(Path.DirectorySeparatorChar.ToString()))
+            if (!folder.EndsWith(Path.DirectorySeparatorChar.ToString(), StringComparison.Ordinal))
             {
                 folder += Path.DirectorySeparatorChar;
             }
-            Uri folderUri = new Uri(folder);
+            var folderUri = new Uri(folder);
             return Uri.UnescapeDataString(folderUri.MakeRelativeUri(pathUri).ToString().Replace('/', Path.DirectorySeparatorChar));
         }
 
@@ -98,9 +109,14 @@ namespace IPA.Utilities
         /// <param name="appendFileName">the filename of the file to append together</param>
         /// <param name="onCopyException">a delegate called when there is an error copying. Return true to keep going.</param>
         public static void CopyAll(DirectoryInfo source, DirectoryInfo target, string appendFileName = "",
-            Func<Exception, FileInfo, bool> onCopyException = null)
+            Func<Exception, FileInfo, bool>? onCopyException = null)
         {
-            if (source.FullName.ToLower() == target.FullName.ToLower())
+            if (source is null)
+                throw new ArgumentNullException(nameof(source));
+            if (target is null)
+                throw new ArgumentNullException(nameof(target));
+
+            if (source.FullName.ToUpperInvariant() == target.FullName.ToUpperInvariant())
             {
                 return;
             }
@@ -108,18 +124,18 @@ namespace IPA.Utilities
             // Check if the target directory exists, if not, create it.
             if (Directory.Exists(target.FullName) == false)
             {
-                Directory.CreateDirectory(target.FullName);
+                _ = Directory.CreateDirectory(target.FullName);
             }
 
             // Copy each file into it's new directory.
-            foreach (FileInfo fi in source.GetFiles())
+            foreach (var fi in source.GetFiles())
             {
                 try
                 {
                     if (fi.Name == appendFileName)
                         File.AppendAllLines(Path.Combine(target.ToString(), fi.Name), File.ReadAllLines(fi.FullName));
                     else
-                        fi.CopyTo(Path.Combine(target.ToString(), fi.Name), true);
+                        _ = fi.CopyTo(Path.Combine(target.ToString(), fi.Name), true);
                 }
                 catch (Exception e)
                 {
@@ -130,10 +146,9 @@ namespace IPA.Utilities
             }
 
             // Copy each subdirectory using recursion.
-            foreach (DirectoryInfo diSourceSubDir in source.GetDirectories())
+            foreach (var diSourceSubDir in source.GetDirectories())
             {
-                DirectoryInfo nextTargetSubDir =
-                    target.CreateSubdirectory(diSourceSubDir.Name);
+                var nextTargetSubDir = target.CreateSubdirectory(diSourceSubDir.Name);
                 CopyAll(diSourceSubDir, nextTargetSubDir, appendFileName, onCopyException);
             }
         }
@@ -153,7 +168,7 @@ namespace IPA.Utilities
         {
             if (DateTimeSafetyUnknown)
             {
-                DateTime time = DateTime.MinValue;
+                var time = DateTime.MinValue;
                 try
                 {
                     time = DateTime.Now;
@@ -180,6 +195,9 @@ namespace IPA.Utilities
         /// <returns>&lt; 0 if l is less than r, 0 if they are equal in the numeric portion, or &gt; 0 if l is greater than r</returns>
         public static int VersionCompareNoPrerelease(SemVer.Version l, SemVer.Version r)
         {
+            if (l is null) throw new ArgumentNullException(nameof(l));
+            if (r is null) throw new ArgumentNullException(nameof(r));
+
             var cmpVal = l.Major - r.Major;
             if (cmpVal != 0) return cmpVal;
             cmpVal = l.Minor - r.Minor;
@@ -188,6 +206,7 @@ namespace IPA.Utilities
             return cmpVal;
         }
 
+
         /// <summary>
         /// An object used to manage scope guards.
         /// </summary>
@@ -197,6 +216,10 @@ namespace IPA.Utilities
         /// </code>
         /// </example>
         /// <seealso cref="ScopeGuard(Action)"/>
+        [SuppressMessage("Design", "CA1034:Nested types should not be visible",
+            Justification = "This type needs to be public to avoid allocations")]
+        [SuppressMessage("Performance", "CA1815:Override equals and operator equals on value types",
+            Justification = "This type is never supposed to be compared")]
         public struct ScopeGuardObject : IDisposable
         {
             private readonly Action action;
@@ -221,9 +244,20 @@ namespace IPA.Utilities
         /// </code>
         /// </example>
         public static ScopeGuardObject ScopeGuard(Action action)
-            => new ScopeGuardObject(action);
+            => new(action);
+
+        /// <summary>
+        /// Deconstructs a <see cref="KeyValuePair{TKey, TValue}"/> as its key and value.
+        /// </summary>
+        /// <typeparam name="TKey">The type of the key.</typeparam>
+        /// <typeparam name="TValue">The type of the value.</typeparam>
+        /// <param name="kvp">The <see cref="KeyValuePair{TKey, TValue}"/> to deconstruct.</param>
+        /// <param name="key">The key in <paramref name="kvp"/>.</param>
+        /// <param name="value">The value in <paramref name="kvp"/>.</param>
+        public static void Deconstruct<TKey, TValue>(this KeyValuePair<TKey, TValue> kvp, out TKey key, out TValue value)
+            => (key, value) = (kvp.Key, kvp.Value);
 
-        internal static bool HasInterface(this TypeDefinition type, string interfaceFullName)
+        internal static bool HasInterface(this TypeDefinition? type, string interfaceFullName)
         {
             return (type?.Interfaces?.Any(i => i.InterfaceType.FullName == interfaceFullName) ?? false)
                     || (type?.Interfaces?.Any(t => HasInterface(t?.InterfaceType?.Resolve(), interfaceFullName)) ?? false);