simplify error shown for duplicate mods

6 files changed, 103 insertions, 30 deletions

diff --git a/src/SMAPI/Framework/IModMetadata.cs b/src/SMAPI/Framework/IModMetadata.cs
index 6a635b76..70cf0036 100644
--- a/src/SMAPI/Framework/IModMetadata.cs
+++ b/src/SMAPI/Framework/IModMetadata.cs
@@ -31,6 +31,9 @@ namespace StardewModdingAPI.Framework
         /// <summary>The metadata resolution status.</summary>
         ModMetadataStatus Status { get; }
 
+        /// <summary>The reason the mod failed to load, if applicable.</summary>
+        ModFailReason? FailReason { get; }
+
         /// <summary>Indicates non-error issues with the mod.</summary>
         ModWarning Warnings { get; }
 
@@ -65,12 +68,17 @@ namespace StardewModdingAPI.Framework
         /*********
         ** Public methods
         *********/
+        /// <summary>Set the mod status to <see cref="ModMetadataStatus.Found"/>.</summary>
+        /// <returns>Return the instance for chaining.</returns>
+        IModMetadata SetStatusFound();
+
         /// <summary>Set the mod status.</summary>
         /// <param name="status">The metadata resolution status.</param>
+        /// <param name="reason">The reason a mod could not be loaded.</param>
         /// <param name="error">The reason the metadata is invalid, if any.</param>
         /// <param name="errorDetails">A detailed technical message, if any.</param>
         /// <returns>Return the instance for chaining.</returns>
-        IModMetadata SetStatus(ModMetadataStatus status, string error = null, string errorDetails = null);
+        IModMetadata SetStatus(ModMetadataStatus status, ModFailReason reason, string error, string errorDetails = null);
 
         /// <summary>Set a warning flag for the mod.</summary>
         /// <param name="warning">The warning to set.</param>
diff --git a/src/SMAPI/Framework/Logging/LogManager.cs b/src/SMAPI/Framework/Logging/LogManager.cs
index d0936f3f..094dd749 100644
--- a/src/SMAPI/Framework/Logging/LogManager.cs
+++ b/src/SMAPI/Framework/Logging/LogManager.cs
@@ -6,6 +6,7 @@ using System.Linq;
 using System.Text.RegularExpressions;
 using System.Threading;
 using StardewModdingAPI.Framework.Commands;
+using StardewModdingAPI.Framework.ModLoading;
 using StardewModdingAPI.Internal.ConsoleWriting;
 using StardewModdingAPI.Toolkit.Framework.ModData;
 using StardewModdingAPI.Toolkit.Utilities;
@@ -397,10 +398,22 @@ namespace StardewModdingAPI.Framework.Logging
             if (skippedMods.Any())
             {
                 // get logging logic
+                HashSet<string> loggedDuplicateIds = new HashSet<string>();
                 void LogSkippedMod(IModMetadata mod)
                 {
                     string message = $"      - {mod.DisplayName}{(mod.Manifest?.Version != null ? " " + mod.Manifest.Version.ToString() : "")} because {mod.Error}";
 
+                    // handle duplicate mods
+                    // (log first duplicate only, don't show redundant version)
+                    if (mod.FailReason == ModFailReason.Duplicate && mod.HasManifest())
+                    {
+                        if (!loggedDuplicateIds.Add(mod.Manifest.UniqueID))
+                            return; // already logged
+
+                        message = $"      - {mod.DisplayName} because {mod.Error}";
+                    }
+
+                    // log message
                     this.Monitor.Log(message, LogLevel.Error);
                     if (mod.ErrorDetails != null)
                         this.Monitor.Log($"        ({mod.ErrorDetails})");
diff --git a/src/SMAPI/Framework/ModLoading/ModFailReason.cs b/src/SMAPI/Framework/ModLoading/ModFailReason.cs
new file mode 100644
index 00000000..cd4623e7
--- /dev/null
+++ b/src/SMAPI/Framework/ModLoading/ModFailReason.cs
@@ -0,0 +1,27 @@
+namespace StardewModdingAPI.Framework.ModLoading
+{
+    /// <summary>Indicates why a mod could not be loaded.</summary>
+    internal enum ModFailReason
+    {
+        /// <summary>The mod has been disabled by prefixing its folder with a dot.</summary>
+        DisabledByDotConvention,
+
+        /// <summary>Multiple copies of the mod are installed.</summary>
+        Duplicate,
+
+        /// <summary>The mod has incompatible code instructions, needs a newer SMAPI version, or is marked 'assume broken' in the SMAPI metadata list.</summary>
+        Incompatible,
+
+        /// <summary>The mod's manifest is missing or invalid.</summary>
+        InvalidManifest,
+
+        /// <summary>The mod was deemed compatible, but SMAPI failed when it tried to load it.</summary>
+        LoadFailed,
+
+        /// <summary>The mod requires other mods which aren't installed, or its dependencies have a circular reference.</summary>
+        MissingDependencies,
+
+        /// <summary>The mod is marked obsolete in the SMAPI metadata list.</summary>
+        Obsolete
+    }
+}
diff --git a/src/SMAPI/Framework/ModLoading/ModMetadata.cs b/src/SMAPI/Framework/ModLoading/ModMetadata.cs
index e793b0cd..18d2b112 100644
--- a/src/SMAPI/Framework/ModLoading/ModMetadata.cs
+++ b/src/SMAPI/Framework/ModLoading/ModMetadata.cs
@@ -38,6 +38,9 @@ namespace StardewModdingAPI.Framework.ModLoading
         public ModMetadataStatus Status { get; private set; }
 
         /// <inheritdoc />
+        public ModFailReason? FailReason { get; private set; }
+
+        /// <inheritdoc />
         public ModWarning Warnings { get; private set; }
 
         /// <inheritdoc />
@@ -93,9 +96,18 @@ namespace StardewModdingAPI.Framework.ModLoading
         }
 
         /// <inheritdoc />
-        public IModMetadata SetStatus(ModMetadataStatus status, string error = null, string errorDetails = null)
+        public IModMetadata SetStatusFound()
+        {
+            this.SetStatus(ModMetadataStatus.Found, ModFailReason.Incompatible, null);
+            this.FailReason = null;
+            return this;
+        }
+
+        /// <inheritdoc />
+        public IModMetadata SetStatus(ModMetadataStatus status, ModFailReason reason, string error, string errorDetails = null)
         {
             this.Status = status;
+            this.FailReason = reason;
             this.Error = error;
             this.ErrorDetails = errorDetails;
             return this;
diff --git a/src/SMAPI/Framework/ModLoading/ModResolver.cs b/src/SMAPI/Framework/ModLoading/ModResolver.cs
index 8bbeb2a3..08df7b76 100644
--- a/src/SMAPI/Framework/ModLoading/ModResolver.cs
+++ b/src/SMAPI/Framework/ModLoading/ModResolver.cs
@@ -43,8 +43,13 @@ namespace StardewModdingAPI.Framework.ModLoading
                     ? ModMetadataStatus.Found
                     : ModMetadataStatus.Failed;
 
-                yield return new ModMetadata(folder.DisplayName, folder.Directory.FullName, rootPath, manifest, dataRecord, isIgnored: shouldIgnore)
-                    .SetStatus(status, shouldIgnore ? "disabled by dot convention" : folder.ManifestParseErrorText);
+                var metadata = new ModMetadata(folder.DisplayName, folder.Directory.FullName, rootPath, manifest, dataRecord, isIgnored: shouldIgnore);
+                if (shouldIgnore)
+                    metadata.SetStatus(status, ModFailReason.DisabledByDotConvention, "disabled by dot convention");
+                else
+                    metadata.SetStatus(status, ModFailReason.InvalidManifest, folder.ManifestParseErrorText);
+
+                yield return metadata;
             }
         }
 
@@ -67,7 +72,7 @@ namespace StardewModdingAPI.Framework.ModLoading
                 switch (mod.DataRecord?.Status)
                 {
                     case ModStatus.Obsolete:
-                        mod.SetStatus(ModMetadataStatus.Failed, $"it's obsolete: {mod.DataRecord.StatusReasonPhrase}");
+                        mod.SetStatus(ModMetadataStatus.Failed, ModFailReason.Obsolete, $"it's obsolete: {mod.DataRecord.StatusReasonPhrase}");
                         continue;
 
                     case ModStatus.AssumeBroken:
@@ -97,7 +102,7 @@ namespace StardewModdingAPI.Framework.ModLoading
                                 error += $"version newer than {mod.DataRecord.StatusUpperVersion}";
                             error += " at " + string.Join(" or ", updateUrls);
 
-                            mod.SetStatus(ModMetadataStatus.Failed, error);
+                            mod.SetStatus(ModMetadataStatus.Failed, ModFailReason.Incompatible, error);
                         }
                         continue;
                 }
@@ -105,7 +110,7 @@ namespace StardewModdingAPI.Framework.ModLoading
                 // validate SMAPI version
                 if (mod.Manifest.MinimumApiVersion?.IsNewerThan(apiVersion) == true)
                 {
-                    mod.SetStatus(ModMetadataStatus.Failed, $"it needs SMAPI {mod.Manifest.MinimumApiVersion} or later. Please update SMAPI to the latest version to use this mod.");
+                    mod.SetStatus(ModMetadataStatus.Failed, ModFailReason.Incompatible, $"it needs SMAPI {mod.Manifest.MinimumApiVersion} or later. Please update SMAPI to the latest version to use this mod.");
                     continue;
                 }
 
@@ -117,12 +122,12 @@ namespace StardewModdingAPI.Framework.ModLoading
                     // validate field presence
                     if (!hasDll && !isContentPack)
                     {
-                        mod.SetStatus(ModMetadataStatus.Failed, $"its manifest has no {nameof(IManifest.EntryDll)} or {nameof(IManifest.ContentPackFor)} field; must specify one.");
+                        mod.SetStatus(ModMetadataStatus.Failed, ModFailReason.InvalidManifest, $"its manifest has no {nameof(IManifest.EntryDll)} or {nameof(IManifest.ContentPackFor)} field; must specify one.");
                         continue;
                     }
                     if (hasDll && isContentPack)
                     {
-                        mod.SetStatus(ModMetadataStatus.Failed, $"its manifest sets both {nameof(IManifest.EntryDll)} and {nameof(IManifest.ContentPackFor)}, which are mutually exclusive.");
+                        mod.SetStatus(ModMetadataStatus.Failed, ModFailReason.InvalidManifest, $"its manifest sets both {nameof(IManifest.EntryDll)} and {nameof(IManifest.ContentPackFor)}, which are mutually exclusive.");
                         continue;
                     }
 
@@ -132,14 +137,14 @@ namespace StardewModdingAPI.Framework.ModLoading
                         // invalid filename format
                         if (mod.Manifest.EntryDll.Intersect(Path.GetInvalidFileNameChars()).Any())
                         {
-                            mod.SetStatus(ModMetadataStatus.Failed, $"its manifest has invalid filename '{mod.Manifest.EntryDll}' for the EntryDLL field.");
+                            mod.SetStatus(ModMetadataStatus.Failed, ModFailReason.InvalidManifest, $"its manifest has invalid filename '{mod.Manifest.EntryDll}' for the EntryDLL field.");
                             continue;
                         }
 
                         // invalid path
                         if (!File.Exists(Path.Combine(mod.DirectoryPath, mod.Manifest.EntryDll)))
                         {
-                            mod.SetStatus(ModMetadataStatus.Failed, $"its DLL '{mod.Manifest.EntryDll}' doesn't exist.");
+                            mod.SetStatus(ModMetadataStatus.Failed, ModFailReason.InvalidManifest, $"its DLL '{mod.Manifest.EntryDll}' doesn't exist.");
                             continue;
                         }
 
@@ -147,7 +152,7 @@ namespace StardewModdingAPI.Framework.ModLoading
                         string actualFilename = new DirectoryInfo(mod.DirectoryPath).GetFiles(mod.Manifest.EntryDll).FirstOrDefault()?.Name;
                         if (actualFilename != mod.Manifest.EntryDll)
                         {
-                            mod.SetStatus(ModMetadataStatus.Failed, $"its {nameof(IManifest.EntryDll)} value '{mod.Manifest.EntryDll}' doesn't match the actual file capitalization '{actualFilename}'. The capitalization must match for crossplatform compatibility.");
+                            mod.SetStatus(ModMetadataStatus.Failed, ModFailReason.InvalidManifest, $"its {nameof(IManifest.EntryDll)} value '{mod.Manifest.EntryDll}' doesn't match the actual file capitalization '{actualFilename}'. The capitalization must match for crossplatform compatibility.");
                             continue;
                         }
                     }
@@ -158,7 +163,7 @@ namespace StardewModdingAPI.Framework.ModLoading
                         // invalid content pack ID
                         if (string.IsNullOrWhiteSpace(mod.Manifest.ContentPackFor.UniqueID))
                         {
-                            mod.SetStatus(ModMetadataStatus.Failed, $"its manifest declares {nameof(IManifest.ContentPackFor)} without its required {nameof(IManifestContentPackFor.UniqueID)} field.");
+                            mod.SetStatus(ModMetadataStatus.Failed, ModFailReason.InvalidManifest, $"its manifest declares {nameof(IManifest.ContentPackFor)} without its required {nameof(IManifestContentPackFor.UniqueID)} field.");
                             continue;
                         }
                     }
@@ -177,14 +182,14 @@ namespace StardewModdingAPI.Framework.ModLoading
 
                     if (missingFields.Any())
                     {
-                        mod.SetStatus(ModMetadataStatus.Failed, $"its manifest is missing required fields ({string.Join(", ", missingFields)}).");
+                        mod.SetStatus(ModMetadataStatus.Failed, ModFailReason.InvalidManifest, $"its manifest is missing required fields ({string.Join(", ", missingFields)}).");
                         continue;
                     }
                 }
 
                 // validate ID format
                 if (!PathUtilities.IsSlug(mod.Manifest.UniqueID))
-                    mod.SetStatus(ModMetadataStatus.Failed, "its manifest specifies an invalid ID (IDs must only contain letters, numbers, underscores, periods, or hyphens).");
+                    mod.SetStatus(ModMetadataStatus.Failed, ModFailReason.InvalidManifest, "its manifest specifies an invalid ID (IDs must only contain letters, numbers, underscores, periods, or hyphens).");
             }
 
             // validate IDs are unique
@@ -199,13 +204,8 @@ namespace StardewModdingAPI.Framework.ModLoading
                         if (mod.Status == ModMetadataStatus.Failed)
                             continue; // don't replace metadata error
 
-                        string folderList = string.Join(", ",
-                            from entry in @group
-                            let relativePath = entry.GetRelativePathWithRoot()
-                            orderby relativePath
-                            select $"{relativePath} ({entry.Manifest.Version})"
-                        );
-                        mod.SetStatus(ModMetadataStatus.Failed, $"you have multiple copies of this mod installed. Found in folders: {folderList}.");
+                        string folderList = string.Join(", ", group.Select(p => p.GetRelativePathWithRoot()).OrderBy(p => p));
+                        mod.SetStatus(ModMetadataStatus.Failed, ModFailReason.Duplicate, $"you have multiple copies of this mod installed. To fix this, delete these folders and reinstall the mod: {folderList}.");
                     }
                 }
             }
@@ -298,7 +298,7 @@ namespace StardewModdingAPI.Framework.ModLoading
                 if (failedModNames.Any())
                 {
                     sortedMods.Push(mod);
-                    mod.SetStatus(ModMetadataStatus.Failed, $"it requires mods which aren't installed ({string.Join(", ", failedModNames)}).");
+                    mod.SetStatus(ModMetadataStatus.Failed, ModFailReason.MissingDependencies, $"it requires mods which aren't installed ({string.Join(", ", failedModNames)}).");
                     return states[mod] = ModDependencyStatus.Failed;
                 }
             }
@@ -315,7 +315,7 @@ namespace StardewModdingAPI.Framework.ModLoading
                 if (failedLabels.Any())
                 {
                     sortedMods.Push(mod);
-                    mod.SetStatus(ModMetadataStatus.Failed, $"it needs newer versions of some mods: {string.Join(", ", failedLabels)}.");
+                    mod.SetStatus(ModMetadataStatus.Failed, ModFailReason.MissingDependencies, $"it needs newer versions of some mods: {string.Join(", ", failedLabels)}.");
                     return states[mod] = ModDependencyStatus.Failed;
                 }
             }
@@ -338,7 +338,7 @@ namespace StardewModdingAPI.Framework.ModLoading
                     if (states[requiredMod] == ModDependencyStatus.Checking)
                     {
                         sortedMods.Push(mod);
-                        mod.SetStatus(ModMetadataStatus.Failed, $"its dependencies have a circular reference: {string.Join(" => ", subchain.Select(p => p.DisplayName))} => {requiredMod.DisplayName}).");
+                        mod.SetStatus(ModMetadataStatus.Failed, ModFailReason.MissingDependencies, $"its dependencies have a circular reference: {string.Join(" => ", subchain.Select(p => p.DisplayName))} => {requiredMod.DisplayName}).");
                         return states[mod] = ModDependencyStatus.Failed;
                     }
 
@@ -354,7 +354,7 @@ namespace StardewModdingAPI.Framework.ModLoading
                         // failed, which means this mod can't be loaded either
                         case ModDependencyStatus.Failed:
                             sortedMods.Push(mod);
-                            mod.SetStatus(ModMetadataStatus.Failed, $"it needs the '{requiredMod.DisplayName}' mod, which couldn't be loaded.");
+                            mod.SetStatus(ModMetadataStatus.Failed, ModFailReason.MissingDependencies, $"it needs the '{requiredMod.DisplayName}' mod, which couldn't be loaded.");
                             return states[mod] = ModDependencyStatus.Failed;
 
                         // unexpected status
diff --git a/src/SMAPI/Framework/SCore.cs b/src/SMAPI/Framework/SCore.cs
index bfe6e277..52b4b9cf 100644
--- a/src/SMAPI/Framework/SCore.cs
+++ b/src/SMAPI/Framework/SCore.cs
@@ -1339,9 +1339,10 @@ namespace StardewModdingAPI.Framework
                 // load mods
                 foreach (IModMetadata mod in mods)
                 {
-                    if (!this.TryLoadMod(mod, mods, modAssemblyLoader, proxyFactory, jsonHelper, contentCore, modDatabase, suppressUpdateChecks, out string errorPhrase, out string errorDetails))
+                    if (!this.TryLoadMod(mod, mods, modAssemblyLoader, proxyFactory, jsonHelper, contentCore, modDatabase, suppressUpdateChecks, out ModFailReason? failReason, out string errorPhrase, out string errorDetails))
                     {
-                        mod.SetStatus(ModMetadataStatus.Failed, errorPhrase, errorDetails);
+                        failReason ??= ModFailReason.LoadFailed;
+                        mod.SetStatus(ModMetadataStatus.Failed, failReason.Value, errorPhrase, errorDetails);
                         skippedMods.Add(mod);
                     }
                 }
@@ -1437,16 +1438,17 @@ namespace StardewModdingAPI.Framework
         /// <summary>Load a given mod.</summary>
         /// <param name="mod">The mod to load.</param>
         /// <param name="mods">The mods being loaded.</param>
-        /// <param name="assemblyLoader">Preprocesses and loads mod assemblies</param>
+        /// <param name="assemblyLoader">Preprocesses and loads mod assemblies.</param>
         /// <param name="proxyFactory">Generates proxy classes to access mod APIs through an arbitrary interface.</param>
         /// <param name="jsonHelper">The JSON helper with which to read mods' JSON files.</param>
         /// <param name="contentCore">The content manager to use for mod content.</param>
         /// <param name="modDatabase">Handles access to SMAPI's internal mod metadata list.</param>
         /// <param name="suppressUpdateChecks">The mod IDs to ignore when validating update keys.</param>
+        /// <param name="failReason">The reason the mod couldn't be loaded, if applicable.</param>
         /// <param name="errorReasonPhrase">The user-facing reason phrase explaining why the mod couldn't be loaded (if applicable).</param>
         /// <param name="errorDetails">More detailed details about the error intended for developers (if any).</param>
         /// <returns>Returns whether the mod was successfully loaded.</returns>
-        private bool TryLoadMod(IModMetadata mod, IModMetadata[] mods, AssemblyLoader assemblyLoader, InterfaceProxyFactory proxyFactory, JsonHelper jsonHelper, ContentCoordinator contentCore, ModDatabase modDatabase, HashSet<string> suppressUpdateChecks, out string errorReasonPhrase, out string errorDetails)
+        private bool TryLoadMod(IModMetadata mod, IModMetadata[] mods, AssemblyLoader assemblyLoader, InterfaceProxyFactory proxyFactory, JsonHelper jsonHelper, ContentCoordinator contentCore, ModDatabase modDatabase, HashSet<string> suppressUpdateChecks, out ModFailReason? failReason, out string errorReasonPhrase, out string errorDetails)
         {
             errorDetails = null;
 
@@ -1469,6 +1471,7 @@ namespace StardewModdingAPI.Framework
             if (mod.Status == ModMetadataStatus.Failed)
             {
                 this.Monitor.Log($"      Failed: {mod.Error}");
+                failReason = mod.FailReason;
                 errorReasonPhrase = mod.Error;
                 return false;
             }
@@ -1485,6 +1488,7 @@ namespace StardewModdingAPI.Framework
                             .FirstOrDefault(otherMod => otherMod.HasID(dependency.UniqueID))
                             ?.DisplayName ?? dependency.UniqueID;
                         errorReasonPhrase = $"it needs the '{dependencyName}' mod, which couldn't be loaded.";
+                        failReason = ModFailReason.MissingDependencies;
                         return false;
                     }
                 }
@@ -1502,6 +1506,7 @@ namespace StardewModdingAPI.Framework
                 this.ModRegistry.Add(mod);
 
                 errorReasonPhrase = null;
+                failReason = null;
                 return true;
             }
 
@@ -1524,17 +1529,20 @@ namespace StardewModdingAPI.Framework
                 {
                     string[] updateUrls = new[] { modDatabase.GetModPageUrlFor(manifest.UniqueID), "https://smapi.io/mods" }.Where(p => p != null).ToArray();
                     errorReasonPhrase = $"it's no longer compatible. Please check for a new version at {string.Join(" or ", updateUrls)}";
+                    failReason = ModFailReason.Incompatible;
                     return false;
                 }
                 catch (SAssemblyLoadFailedException ex)
                 {
                     errorReasonPhrase = $"its DLL couldn't be loaded: {ex.Message}";
+                    failReason = ModFailReason.LoadFailed;
                     return false;
                 }
                 catch (Exception ex)
                 {
                     errorReasonPhrase = "its DLL couldn't be loaded.";
                     errorDetails = $"Error: {ex.GetLogSummary()}";
+                    failReason = ModFailReason.LoadFailed;
                     return false;
                 }
 
@@ -1543,7 +1551,10 @@ namespace StardewModdingAPI.Framework
                 {
                     // get mod instance
                     if (!this.TryLoadModEntry(modAssembly, out Mod modEntry, out errorReasonPhrase))
+                    {
+                        failReason = ModFailReason.LoadFailed;
                         return false;
+                    }
 
                     // get content packs
                     IContentPack[] GetContentPacks()
@@ -1591,11 +1602,13 @@ namespace StardewModdingAPI.Framework
                     // track mod
                     mod.SetMod(modEntry, translationHelper);
                     this.ModRegistry.Add(mod);
+                    failReason = null;
                     return true;
                 }
                 catch (Exception ex)
                 {
                     errorReasonPhrase = $"initialization failed:\n{ex.GetLogSummary()}";
+                    failReason = ModFailReason.LoadFailed;
                     return false;
                 }
             }