diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e06b3b21f..2761c39df5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ All notable changes to this project will be documented in this file. - [Core] Improve handling of missing game version (#2444 by: HebaruSan; reviewed: politas) - [Core] Handle zero byte registry.json file (#2435 by: HebaruSan; reviewed: politas) - [Multiple] Pass game instance from cmdline to GUI/ConsoleUI (#2449 by: HebaruSan; reviewed: politas) +- [GUI] Show conflict messages in status bar (#2442 by: HebaruSan; reviewed: dbent, politas) ### Internal - [Core] Test upgrading mod with conflict on its own provides (#2431 by: HebaruSan; reviewed: politas) diff --git a/Core/Extensions/EnumerableExtensions.cs b/Core/Extensions/EnumerableExtensions.cs index fa7daeab21..6be1c8d49e 100644 --- a/Core/Extensions/EnumerableExtensions.cs +++ b/Core/Extensions/EnumerableExtensions.cs @@ -8,7 +8,7 @@ internal static class EnumerableExtensions { public static ICollection AsCollection(this IEnumerable source) { - if (source is null) + if (source == null) throw new ArgumentNullException(nameof(source)); return source is ICollection collection ? collection : source.ToArray(); @@ -16,7 +16,7 @@ public static ICollection AsCollection(this IEnumerable source) public static HashSet ToHashSet(this IEnumerable source) { - if (source is null) + if (source == null) throw new ArgumentNullException(nameof(source)); return new HashSet(source); diff --git a/Core/Relationships/RelationshipResolver.cs b/Core/Relationships/RelationshipResolver.cs index e0032a63a3..6da9cd94f4 100644 --- a/Core/Relationships/RelationshipResolver.cs +++ b/Core/Relationships/RelationshipResolver.cs @@ -201,13 +201,13 @@ public void AddModulesToInstall(IEnumerable modules) // adding them to the list. This *must* be pre-populated with all // user-specified modules, as they may be supplying things that provide // virtual packages. - foreach (var module in ckan_modules) + foreach (CkanModule module in ckan_modules) { log.DebugFormat("Preparing to resolve relationships for {0} {1}", module.identifier, module.version); //Need to check against installed mods and those to install. var mods = modlist.Values.Concat(installed_modules).Where(listed_mod => listed_mod.ConflictsWith(module)); - foreach (var listed_mod in mods) + foreach (CkanModule listed_mod in mods) { if (options.procede_with_inconsistencies) { @@ -216,8 +216,8 @@ public void AddModulesToInstall(IEnumerable modules) } else { - throw new InconsistentKraken(string.Format("{0} conflicts with {1}, can't install both.", module, - listed_mod)); + throw new InconsistentKraken( + $"{module} conflicts with {listed_mod}"); } } @@ -234,16 +234,29 @@ public void AddModulesToInstall(IEnumerable modules) Resolve(module, options); } - if (!options.without_enforce_consistency) + try { - var final_modules = new List(modlist.Values); - final_modules.AddRange(installed_modules); // Finally, let's do a sanity check that our solution is actually sane. SanityChecker.EnforceConsistency( - final_modules, + modlist.Values.Concat(installed_modules), registry.InstalledDlls, registry.InstalledDlc - ); + ); + } + catch (BadRelationshipsKraken k) + { + // Add to this.conflicts (catches conflicting DLLs and DLCs that the above loops miss) + foreach (var kvp in k.Conflicts) + { + conflicts.Add(new KeyValuePair( + kvp.Key, null + )); + } + if (!options.without_enforce_consistency) + { + // Only re-throw if caller asked for consistency enforcement + throw; + } } } @@ -301,9 +314,7 @@ private void Resolve(CkanModule module, RelationshipResolverOptions options, IEn /// options.without_toomanyprovides_kraken is not set. /// /// See RelationshipResolverOptions for further adjustments that can be made. - /// /// - private void ResolveStanza(IEnumerable stanza, SelectionReason reason, RelationshipResolverOptions options, bool soft_resolve = false, IEnumerable old_stanza = null) { @@ -312,7 +323,7 @@ private void ResolveStanza(IEnumerable stanza, Selection return; } - foreach (var descriptor in stanza) + foreach (RelationshipDescriptor descriptor in stanza) { string dep_name = descriptor.name; log.DebugFormat("Considering {0}", dep_name); @@ -419,7 +430,7 @@ private void ResolveStanza(IEnumerable stanza, Selection var fixed_mods = new HashSet(modlist.Values); fixed_mods.UnionWith(installed_modules); - var conflicting_mod = fixed_mods.FirstOrDefault(mod => mod.ConflictsWith(candidate)); + CkanModule conflicting_mod = fixed_mods.FirstOrDefault(mod => mod.ConflictsWith(candidate)); if (conflicting_mod == null) { // Okay, looks like we want this one. Adding. @@ -440,8 +451,8 @@ private void ResolveStanza(IEnumerable stanza, Selection } else { - throw new InconsistentKraken(string.Format("{0} conflicts with {1}, can't install both.", conflicting_mod, - candidate)); + throw new InconsistentKraken( + $"{conflicting_mod} conflicts with {candidate}"); } } } @@ -535,11 +546,11 @@ public Dictionary ConflictList var dict = new Dictionary(); foreach (var conflict in conflicts) { - var module = conflict.Key; + CkanModule module = conflict.Key; + CkanModule other = conflict.Value; dict[module] = string.Format("{0} conflicts with {1}\r\n\r\n{0}:\r\n{2}\r\n{1}:\r\n{3}", - module.identifier, conflict.Value.identifier, - ReasonStringFor(module), ReasonStringFor(conflict.Value)); - ; + module.identifier, other?.identifier ?? "an unmanaged DLL or DLC", + ReasonStringFor(module), ReasonStringFor(other)); } return dict; } @@ -557,6 +568,11 @@ public bool IsConsistent /// public string ReasonStringFor(CkanModule mod) { + if (mod == null) + { + // If we don't have a CkanModule, it must be a DLL or DLC + return "Unmanaged"; + } var reason = ReasonFor(mod); var is_root_type = reason.GetType() == typeof(SelectionReason.UserRequested) || reason.GetType() == typeof(SelectionReason.Installed); diff --git a/Core/Relationships/SanityChecker.cs b/Core/Relationships/SanityChecker.cs index c4ef5f0a9e..8487dfc080 100644 --- a/Core/Relationships/SanityChecker.cs +++ b/Core/Relationships/SanityChecker.cs @@ -24,37 +24,26 @@ public static ICollection ConsistencyErrors( IDictionary dlc ) { - modules = modules?.AsCollection(); - dlls = dlls?.AsCollection(); - + List> unmetDepends; + List> conflicts; var errors = new HashSet(); - - // If we have no modules, then everything is fine. DLLs can't depend or conflict on things. - if (modules == null) - { - return errors; - } - - foreach (var kvp in FindUnsatisfiedDepends(modules.ToList(), dlls?.ToHashSet(), dlc)) + if (!CheckConsistency(modules, dlls, dlc, out unmetDepends, out conflicts)) { - errors.Add($"{kvp.Key} has an unsatisfied dependency: {kvp.Value} is not installed"); - } - - // Conflicts are more difficult. Mods are allowed to conflict with themselves. - // So we walk all our mod conflicts, find what (if anything) provide those - // conflicts, and return false if it's not the module we're examining. - foreach (var kvp in FindConflicting(modules, dlls?.ToHashSet(), dlc)) - { - errors.Add($"{kvp.Key} conflicts with {kvp.Value}"); + foreach (var kvp in unmetDepends) + { + errors.Add($"{kvp.Key} has an unsatisfied dependency: {kvp.Value} is not installed"); + } + foreach (var kvp in conflicts) + { + errors.Add($"{kvp.Key} conflicts with {kvp.Value}"); + } } - - // Return whatever we've found, which could be empty. return errors; } /// /// Ensures all modules in the list provided can co-exist. - /// Throws a InconsistentKraken containing a list of inconsistences if they do not. + /// Throws a BadRelationshipsKraken describing the problems otherwise. /// Does nothing if the modules can happily co-exist. /// public static void EnforceConsistency( @@ -63,11 +52,11 @@ public static void EnforceConsistency( IDictionary dlc = null ) { - ICollection errors = ConsistencyErrors(modules, dlls, dlc); - - if (errors.Count != 0) + List> unmetDepends; + List> conflicts; + if (!CheckConsistency(modules, dlls, dlc, out unmetDepends, out conflicts)) { - throw new InconsistentKraken(errors); + throw new BadRelationshipsKraken(unmetDepends, conflicts); } } @@ -80,7 +69,22 @@ public static bool IsConsistent( IDictionary dlc = null ) { - return ConsistencyErrors(modules, dlls, dlc).Count == 0; + List> unmetDepends; + List> conflicts; + return CheckConsistency(modules, dlls, dlc, out unmetDepends, out conflicts); + } + + private static bool CheckConsistency( + IEnumerable modules, + IEnumerable dlls, + IDictionary dlc, + out List> UnmetDepends, + out List> Conflicts + ) + { + UnmetDepends = FindUnsatisfiedDepends(modules?.ToList(), dlls?.ToHashSet(), dlc); + Conflicts = FindConflicting( modules, dlls?.ToHashSet(), dlc); + return !UnmetDepends.Any() && !Conflicts.Any(); } /// @@ -154,9 +158,9 @@ IDictionary dlc private sealed class ProvidesInfo { - public string ProviderIdentifier { get; } + public string ProviderIdentifier { get; } public ModuleVersion ProviderVersion { get; } - public string ProvideeIdentifier { get; } + public string ProvideeIdentifier { get; } public ModuleVersion ProvideeVersion { get; } public ProvidesInfo( @@ -167,9 +171,9 @@ ModuleVersion provideeVersion ) { ProviderIdentifier = providerIdentifier; - ProviderVersion = providerVersion; + ProviderVersion = providerVersion; ProvideeIdentifier = provideeIdentifier; - ProvideeVersion = provideeVersion; + ProvideeVersion = provideeVersion; } } } diff --git a/Core/Types/CkanModule.cs b/Core/Types/CkanModule.cs index e674ff7b26..871916fece 100644 --- a/Core/Types/CkanModule.cs +++ b/Core/Types/CkanModule.cs @@ -539,14 +539,9 @@ public bool ConflictsWith(CkanModule module) /// internal static bool UniConflicts(CkanModule mod1, CkanModule mod2) { - if (mod1.conflicts == null) - { - return false; - } - return - mod1.conflicts.Any( - conflict => - mod2.ProvidesList.Contains(conflict.name) && conflict.WithinBounds(mod2.version)); + return mod1?.conflicts?.Any( + conflict => conflict.MatchesAny(new CkanModule[] {mod2}, null, null) + ) ?? false; } /// diff --git a/Core/Types/Kraken.cs b/Core/Types/Kraken.cs index 24b6094221..ed06ff971e 100644 --- a/Core/Types/Kraken.cs +++ b/Core/Types/Kraken.cs @@ -175,6 +175,14 @@ public string InconsistenciesPretty } } + public string ShortDescription + { + get + { + return String.Join("; ", inconsistencies); + } + } + public InconsistentKraken(ICollection inconsistencies, Exception innerException = null) : base(null, innerException) { @@ -195,6 +203,33 @@ public override string ToString() private const string header = "The following inconsistencies were found:\r\n"; } + /// + /// A mutation of InconsistentKraken that allows catching code to extract the causes of the errors. + /// + public class BadRelationshipsKraken : InconsistentKraken + { + public BadRelationshipsKraken( + ICollection> depends, + ICollection> conflicts + ) : base( + (depends?.Select(dep => $"{dep.Key} missing dependency {dep.Value}") + ?? new string[] {} + ).Concat( + conflicts?.Select(conf => $"{conf.Key} conflicts with {conf.Value}") + ?? new string[] {} + ).ToArray() + ) + { + Depends = depends?.ToList() + ?? new List>(); + Conflicts = conflicts?.ToList() + ?? new List>(); + } + + public List> Depends { get; private set; } + public List> Conflicts { get; private set; } + } + /// /// The terrible state when a file exists when we expect it not to be there. /// For example, when we install a mod, and it tries to overwrite a file from another mod. diff --git a/Core/Versioning/ModuleVersion.cs b/Core/Versioning/ModuleVersion.cs index 0965265ad3..5c99d4d525 100644 --- a/Core/Versioning/ModuleVersion.cs +++ b/Core/Versioning/ModuleVersion.cs @@ -31,7 +31,7 @@ public partial class ModuleVersion private static readonly ConcurrentDictionary, int> ComparisonCache = new ConcurrentDictionary, int>(); } - + public partial class ModuleVersion { private readonly int _epoch; @@ -298,7 +298,7 @@ Comparison numComp(string v1, string v2) return comparison; } - if (other is null) + if (other == null) throw new ArgumentNullException(nameof(other)); if (other._epoch == _epoch && other._version.Equals(_version)) diff --git a/Core/Versioning/UnmanagedModuleVersion.cs b/Core/Versioning/UnmanagedModuleVersion.cs index 23e18bdf76..1a7ac2f911 100644 --- a/Core/Versioning/UnmanagedModuleVersion.cs +++ b/Core/Versioning/UnmanagedModuleVersion.cs @@ -12,8 +12,8 @@ public sealed class UnmanagedModuleVersion : ModuleVersion // HACK: Hardcoding a value of "0" for autodetected DLLs preserves previous behavior. public UnmanagedModuleVersion(string version) : base(version ?? "0") { - IsUnknownVersion = version is null; - _string = version is null ? "(unmanaged)" : $"{version} (unmanaged)"; + IsUnknownVersion = version == null; + _string = version == null ? "(unmanaged)" : $"{version} (unmanaged)"; } public override string ToString() diff --git a/GUI/Main.Designer.cs b/GUI/Main.Designer.cs index 615976f444..79fd3003f2 100644 --- a/GUI/Main.Designer.cs +++ b/GUI/Main.Designer.cs @@ -617,6 +617,7 @@ private void InitializeComponent() this.StatusProgress.Maximum = 100; this.StatusProgress.Size = new System.Drawing.Size(300, 20); this.StatusProgress.Alignment = System.Windows.Forms.ToolStripItemAlignment.Right; + // // MainTabControl // this.MainTabControl.Controls.Add(this.ManageModsTabPage); diff --git a/GUI/Main.cs b/GUI/Main.cs index 8105d69bc3..926cca603f 100644 --- a/GUI/Main.cs +++ b/GUI/Main.cs @@ -108,14 +108,18 @@ private Dictionary Conflicts private void ConflictsUpdated() { + if (Conflicts == null) { + // Clear status bar if no conflicts + AddStatusMessage(""); + } foreach (DataGridViewRow row in ModList.Rows) { - var module = (GUIMod)row.Tag; + GUIMod module = (GUIMod)row.Tag; string value; if (Conflicts != null && Conflicts.TryGetValue(module, out value)) { - var conflict_text = value; + string conflict_text = value; foreach (DataGridViewCell cell in row.Cells) { cell.ToolTipText = conflict_text; @@ -127,18 +131,15 @@ private void ConflictsUpdated() ModList.InvalidateRow(row.Index); } } - else + else if (row.DefaultCellStyle.BackColor != Color.Empty) { - if (row.DefaultCellStyle.BackColor != Color.Empty) + foreach (DataGridViewCell cell in row.Cells) { - foreach (DataGridViewCell cell in row.Cells) - { - cell.ToolTipText = null; - } - - row.DefaultCellStyle.BackColor = Color.Empty; - ModList.InvalidateRow(row.Index); + cell.ToolTipText = null; } + + row.DefaultCellStyle.BackColor = Color.Empty; + ModList.InvalidateRow(row.Index); } } } @@ -597,9 +598,10 @@ private async Task UpdateChangeSetAndConflicts(IRegistryQuerier registry) var module_installer = ModuleInstaller.GetInstance(CurrentInstance, GUI.user); full_change_set = await mainModList.ComputeChangeSetFromModList(registry, user_change_set, module_installer, CurrentInstance.VersionCriteria()); } - catch (InconsistentKraken) + catch (InconsistentKraken k) { // Need to be recomputed due to ComputeChangeSetFromModList possibly changing it with too many provides handling. + AddStatusMessage(k.ShortDescription); user_change_set = mainModList.ComputeUserChangeSet(); new_conflicts = MainModList.ComputeConflictsFromModList(registry, user_change_set, CurrentInstance.VersionCriteria()); full_change_set = null; diff --git a/GUI/MainDialogs.cs b/GUI/MainDialogs.cs index fa323312c8..651e32c4bc 100644 --- a/GUI/MainDialogs.cs +++ b/GUI/MainDialogs.cs @@ -25,7 +25,7 @@ public void AddStatusMessage(string text, params object[] args) string msg = String.Format(text, args); // No newlines in status bar Util.Invoke(statusStrip1, () => - StatusLabel.Text = msg.Replace("\r\n", " ").Replace("\n", " ") + StatusLabel.ToolTipText = StatusLabel.Text = msg.Replace("\r\n", " ").Replace("\n", " ") ); AddLogMessage(msg); } diff --git a/GUI/MainModList.cs b/GUI/MainModList.cs index 3e35980c56..42fea9460c 100644 --- a/GUI/MainModList.cs +++ b/GUI/MainModList.cs @@ -223,7 +223,7 @@ public void MarkModForUpdate(string identifier) { Util.Invoke(this, () => _MarkModForUpdate(identifier)); } - + public void _MarkModForUpdate(string identifier) { DataGridViewRow row = mainModList.full_list_of_mod_rows[identifier]; @@ -235,8 +235,6 @@ private void ModList_SelectedIndexChanged(object sender, EventArgs e) { var module = GetSelectedModule(); - AddStatusMessage(string.Empty); - ModInfoTabControl.SelectedModule = module; if (module == null) return;