From 632fe70d7d4d65f777045f807897833809e018c6 Mon Sep 17 00:00:00 2001 From: Sewer56 Date: Thu, 14 Nov 2024 05:33:17 +0000 Subject: [PATCH 1/6] Improved: ModuleIncompatibleIssue now passes full module for incompatible item. --- .../Issues/ModuleIssueV2.cs | 8 ++++---- src/Bannerlord.ModuleManager/ModuleUtilities.cs | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Bannerlord.ModuleManager.Models/Issues/ModuleIssueV2.cs b/src/Bannerlord.ModuleManager.Models/Issues/ModuleIssueV2.cs index d2f08bb..bb81328 100644 --- a/src/Bannerlord.ModuleManager.Models/Issues/ModuleIssueV2.cs +++ b/src/Bannerlord.ModuleManager.Models/Issues/ModuleIssueV2.cs @@ -456,7 +456,7 @@ public override string ToString() => /// This occurs when one module explicitly declares it cannot work with another module. /// /// The module that has declared an incompatibility -/// The ID of the module that is incompatible with the target +/// The module that is incompatible with the target /// /// This issue occurs when a module explicitly marks another module as incompatible. /// @@ -480,11 +480,11 @@ public override string ToString() => # endif sealed record ModuleIncompatibleIssue( ModuleInfoExtended Module, - string IncompatibleModuleId + ModuleInfoExtended IncompatibleModule ) : ModuleIssueV2(Module) { - public override string ToString() => $"'{IncompatibleModuleId}' is incompatible with this module"; - public override LegacyModuleIssue ToLegacy() => new(Module, IncompatibleModuleId, ModuleIssueType.Incompatible, ToString(), ApplicationVersionRange.Empty); + public override string ToString() => $"'{IncompatibleModule.Id}' is incompatible with this module"; + public override LegacyModuleIssue ToLegacy() => new(Module, IncompatibleModule.Id, ModuleIssueType.Incompatible, ToString(), ApplicationVersionRange.Empty); } /// diff --git a/src/Bannerlord.ModuleManager/ModuleUtilities.cs b/src/Bannerlord.ModuleManager/ModuleUtilities.cs index d756de6..926f2c8 100644 --- a/src/Bannerlord.ModuleManager/ModuleUtilities.cs +++ b/src/Bannerlord.ModuleManager/ModuleUtilities.cs @@ -517,7 +517,7 @@ private static IEnumerable ValidateModuleDependenciesEx( // If the incompatible mod is selected, this mod should be disabled if (isSelected(metadataModule)) - yield return new ModuleIncompatibleIssue(targetModule, metadataModule.Id); + yield return new ModuleIncompatibleIssue(targetModule, metadataModule); } // If another mod declared incompatibility and is selected, disable this @@ -535,7 +535,7 @@ private static IEnumerable ValidateModuleDependenciesEx( // If the incompatible mod is selected, this mod is disabled if (isSelected(module)) - yield return new ModuleIncompatibleIssue(targetModule, module.Id); + yield return new ModuleIncompatibleIssue(targetModule, module); } } } From ed67996942f88b257cf8314e2d02aa9bfde8dd0a Mon Sep 17 00:00:00 2001 From: Sewer56 Date: Thu, 14 Nov 2024 08:08:32 +0000 Subject: [PATCH 2/6] Added: Full module reference for circular dependencies --- .../Issues/ModuleIssueV2.cs | 18 +++++++++--------- .../ModuleUtilities.cs | 13 +++++++++---- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/Bannerlord.ModuleManager.Models/Issues/ModuleIssueV2.cs b/src/Bannerlord.ModuleManager.Models/Issues/ModuleIssueV2.cs index bb81328..ca4cc7d 100644 --- a/src/Bannerlord.ModuleManager.Models/Issues/ModuleIssueV2.cs +++ b/src/Bannerlord.ModuleManager.Models/Issues/ModuleIssueV2.cs @@ -610,7 +610,7 @@ DependentModuleMetadata ConflictingModule # endif sealed record ModuleDependencyConflictCircularIssue( ModuleInfoExtended Module, - DependentModuleMetadata CircularDependency + ModuleInfoExtended CircularDependency ) : ModuleIssueV2(Module) { public override string ToString() => $"Module '{Module.Id}' and '{CircularDependency.Id}' have circular dependencies"; @@ -648,11 +648,11 @@ DependentModuleMetadata CircularDependency # endif sealed record ModuleDependencyNotLoadedBeforeIssue( ModuleInfoExtended Module, - string DependencyId + DependentModuleMetadata Dependency ) : ModuleIssueV2(Module) { - public override string ToString() => $"'{DependencyId}' should be loaded before '{Module.Id}'"; - public override LegacyModuleIssue ToLegacy() => new(Module, DependencyId, ModuleIssueType.DependencyNotLoadedBeforeThis, ToString(), ApplicationVersionRange.Empty); + public override string ToString() => $"'{Dependency.Id}' should be loaded before '{Module.Id}'"; + public override LegacyModuleIssue ToLegacy() => new(Module, Dependency.Id, ModuleIssueType.DependencyNotLoadedBeforeThis, ToString(), ApplicationVersionRange.Empty); } /// @@ -686,11 +686,11 @@ string DependencyId # endif sealed record ModuleDependencyNotLoadedAfterIssue( ModuleInfoExtended Module, - string DependencyId + DependentModuleMetadata Dependency ) : ModuleIssueV2(Module) { - public override string ToString() => $"'{DependencyId}' should be loaded after '{Module.Id}'"; - public override LegacyModuleIssue ToLegacy() => new(Module, DependencyId, ModuleIssueType.DependencyNotLoadedAfterThis, ToString(), ApplicationVersionRange.Empty); + public override string ToString() => $"'{Dependency.Id}' should be loaded after '{Module.Id}'"; + public override LegacyModuleIssue ToLegacy() => new(Module, Dependency.Id, ModuleIssueType.DependencyNotLoadedAfterThis, ToString(), ApplicationVersionRange.Empty); } /// @@ -773,8 +773,8 @@ ModuleInfoExtended Module /// /// /// -/// -/// +/// +/// /// diff --git a/src/Bannerlord.ModuleManager/ModuleUtilities.cs b/src/Bannerlord.ModuleManager/ModuleUtilities.cs index 926f2c8..70bec2e 100644 --- a/src/Bannerlord.ModuleManager/ModuleUtilities.cs +++ b/src/Bannerlord.ModuleManager/ModuleUtilities.cs @@ -386,8 +386,12 @@ public static IEnumerable ValidateModuleDependenciesDeclarationsE .Where(x => x.LoadType != LoadType.None) .FirstOrDefault(x => string.Equals(x.Id, targetModule.Id, StringComparison.Ordinal)) is { } metadata) { - if (metadata.LoadType == module.LoadType) - yield return new ModuleDependencyConflictCircularIssue(targetModule, metadata); + if (metadata.LoadType != module.LoadType) + continue; + + // Find the full module with given ID. + var fullModule = modules.First(x => x.Id == metadata.Id); + yield return new ModuleDependencyConflictCircularIssue(targetModule, fullModule); } } } @@ -603,14 +607,15 @@ public static IEnumerable ValidateLoadOrderEx( continue; } + // TODO(sewer): Return full module here later. Right now I don't have a good way to test some assertions. if (metadata.LoadType == LoadType.LoadBeforeThis && metadataIdx > targetModuleIdx) { - yield return new ModuleDependencyNotLoadedBeforeIssue(targetModule, metadata.Id); + yield return new ModuleDependencyNotLoadedBeforeIssue(targetModule, metadata); } if (metadata.LoadType == LoadType.LoadAfterThis && metadataIdx < targetModuleIdx) { - yield return new ModuleDependencyNotLoadedAfterIssue(targetModule, metadata.Id); + yield return new ModuleDependencyNotLoadedAfterIssue(targetModule, metadata); } } } From b776ffa33301b598cf4b320f723a9f5de75e439d Mon Sep 17 00:00:00 2001 From: Sewer56 Date: Thu, 14 Nov 2024 08:25:13 +0000 Subject: [PATCH 3/6] Fixed: Incorrect behaviour of ModuleVersionMismatchLessThanOrEqualSpecificIssue --- .../Issues/ModuleIssueV2.cs | 24 +++++++++---------- .../ModuleUtilities.cs | 4 ++-- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/Bannerlord.ModuleManager.Models/Issues/ModuleIssueV2.cs b/src/Bannerlord.ModuleManager.Models/Issues/ModuleIssueV2.cs index ca4cc7d..87904c1 100644 --- a/src/Bannerlord.ModuleManager.Models/Issues/ModuleIssueV2.cs +++ b/src/Bannerlord.ModuleManager.Models/Issues/ModuleIssueV2.cs @@ -326,11 +326,11 @@ ApplicationVersionRange VersionRange ) : ModuleVersionMismatchIssue(Module, Dependency); /// -/// Represents an issue where a dependency's version is higher than the maximum allowed specific version. -/// This occurs when a dependency module's version exceeds an exact version requirement. +/// Represents an issue where a dependency's version is lower than the minimum allowed specific version. +/// This occurs when a dependency module's version is below an version requirement. /// /// The module with the version constraint -/// The dependency module that exceeds the version requirement +/// The dependency module that is under the version requirement /// The specific version that should not be exceeded /// /// This issue occurs when a module specifies incompatible versions. @@ -338,32 +338,32 @@ ApplicationVersionRange VersionRange /// Example scenario: /// ```xml /// -/// -/// +/// +/// +/// /// -/// -/// -/// +/// +/// +/// /// /// /// ``` /// -/// If a higher version of Harmony (e.g., `v2.3.0`) is installed than allowed, this issue will be raised. +/// If a higher or equal version of Harmony (e.g., `v2.3.0`) is installed than allowed, this issue will be solved. /// #if !BANNERLORDBUTRMODULEMANAGER_PUBLIC internal #else public # endif - sealed record ModuleVersionMismatchLessThanOrEqualSpecificIssue( + sealed record ModuleVersionTooLowIssue( ModuleInfoExtended Module, ModuleInfoExtended Dependency, ApplicationVersion Version ) : ModuleVersionMismatchSpecificIssue(Module, Dependency, Version) { public override string ToString() => - $"The module '{Module.Id}' requires version {Version} or lower of '{Dependency.Id}', but version {Dependency.Version} is installed"; + $"The module '{Module.Id}' requires version {Version} or higher of '{Dependency.Id}', but version {Dependency.Version} is installed"; public override LegacyModuleIssue ToLegacy() => new(Module, Dependency.Id, ModuleIssueType.VersionMismatchLessThanOrEqual, ToString(), new ApplicationVersionRange(Version, Version)); } diff --git a/src/Bannerlord.ModuleManager/ModuleUtilities.cs b/src/Bannerlord.ModuleManager/ModuleUtilities.cs index 70bec2e..6344dc4 100644 --- a/src/Bannerlord.ModuleManager/ModuleUtilities.cs +++ b/src/Bannerlord.ModuleManager/ModuleUtilities.cs @@ -1,4 +1,4 @@ -#region License +#region License // MIT License // // Copyright (c) Bannerlord's Unofficial Tools & Resources @@ -481,7 +481,7 @@ private static IEnumerable ValidateModuleDependenciesEx( // dependedModuleMetadata.Version > dependedModule.Version if (!metadata.IsOptional && (ApplicationVersionComparer.CompareStandard(metadata.Version, metadataModule.Version) > 0)) { - yield return new ModuleVersionMismatchLessThanOrEqualSpecificIssue( + yield return new ModuleVersionTooLowIssue( targetModule, metadataModule, metadata.Version); From d5574c7b34261d0d49f3f99126d2c48d8be23546 Mon Sep 17 00:00:00 2001 From: Sewer56 Date: Thu, 14 Nov 2024 08:26:13 +0000 Subject: [PATCH 4/6] Added: Flag to recursively validate dependencies to public API. --- src/Bannerlord.ModuleManager/ModuleUtilities.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Bannerlord.ModuleManager/ModuleUtilities.cs b/src/Bannerlord.ModuleManager/ModuleUtilities.cs index 6344dc4..4487e99 100644 --- a/src/Bannerlord.ModuleManager/ModuleUtilities.cs +++ b/src/Bannerlord.ModuleManager/ModuleUtilities.cs @@ -1,4 +1,4 @@ -#region License +#region License // MIT License // // Copyright (c) Bannerlord's Unofficial Tools & Resources @@ -263,12 +263,14 @@ public static IEnumerable ValidateModuleEx( /// The module to validate /// Function that determines if a module is enabled /// Function that determines if a module is valid + /// Set this to true to also report errors in the target module's dependencies. e.g. Missing dependencies of dependencies. /// Any errors that were detected during inspection public static IEnumerable ValidateModuleEx( IReadOnlyList modules, ModuleInfoExtended targetModule, Func isSelected, - Func isValid) + Func isValid, + bool validateDependencies = true) { var visited = new HashSet(); foreach (var issue in ValidateModuleEx(modules, targetModule, visited, isSelected, isValid)) @@ -287,7 +289,7 @@ public static IEnumerable ValidateModuleEx( /// Function that determines if a module is valid /// Set this to true to also report errors in the target module's dependencies. e.g. Missing dependencies of dependencies. /// Any errors that were detected during inspection - private static IEnumerable ValidateModuleEx( + public static IEnumerable ValidateModuleEx( IReadOnlyList modules, ModuleInfoExtended targetModule, HashSet visitedModules, From 89a41b44b3a06725a7f7eb142abb448892e8a1ed Mon Sep 17 00:00:00 2001 From: Sewer56 Date: Thu, 14 Nov 2024 08:41:23 +0000 Subject: [PATCH 5/6] Fixed: Circular dependency issue returning wrong dependency name. --- src/Bannerlord.ModuleManager/ModuleUtilities.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Bannerlord.ModuleManager/ModuleUtilities.cs b/src/Bannerlord.ModuleManager/ModuleUtilities.cs index 4487e99..515b83a 100644 --- a/src/Bannerlord.ModuleManager/ModuleUtilities.cs +++ b/src/Bannerlord.ModuleManager/ModuleUtilities.cs @@ -392,7 +392,7 @@ public static IEnumerable ValidateModuleDependenciesDeclarationsE continue; // Find the full module with given ID. - var fullModule = modules.First(x => x.Id == metadata.Id); + var fullModule = modules.First(x => moduleInfo.Id == x.Id); yield return new ModuleDependencyConflictCircularIssue(targetModule, fullModule); } } From 8f4c5c6e030989a28a7d3245e94fe965ee2e2872 Mon Sep 17 00:00:00 2001 From: Sewer56 Date: Thu, 14 Nov 2024 09:23:37 +0000 Subject: [PATCH 6/6] Added: ToString for ApplicationVersion(s) no longer prints ChangeSet if not specified. --- .../ApplicationVersion.cs | 9 ++++++++- .../ApplicationVersionRange.cs | 11 ++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/Bannerlord.ModuleManager.Models/ApplicationVersion.cs b/src/Bannerlord.ModuleManager.Models/ApplicationVersion.cs index 2eb85b3..91a1385 100644 --- a/src/Bannerlord.ModuleManager.Models/ApplicationVersion.cs +++ b/src/Bannerlord.ModuleManager.Models/ApplicationVersion.cs @@ -62,7 +62,14 @@ public bool IsSame(ApplicationVersion? other) => public bool IsSameWithChangeSet(ApplicationVersion? other) => Major == other?.Major && Minor == other.Minor && Revision == other.Revision && ChangeSet == other.ChangeSet; - public override string ToString() => $"{GetPrefix(ApplicationVersionType)}{Major}.{Minor}.{Revision}.{ChangeSet}"; + public override string ToString() + { + // Most user mods skip this, so to be user-friendly we can omit it if it was not originally specified. + return ChangeSet == 0 + ? $"{GetPrefix(ApplicationVersionType)}{Major}.{Minor}.{Revision}" + : $"{GetPrefix(ApplicationVersionType)}{Major}.{Minor}.{Revision}.{ChangeSet}"; + } + public string ToStringWithoutChangeset() => $"{GetPrefix(ApplicationVersionType)}{Major}.{Minor}.{Revision}"; public int CompareTo(ApplicationVersion? other) => ApplicationVersionComparer.CompareStandard(this, other); diff --git a/src/Bannerlord.ModuleManager.Models/ApplicationVersionRange.cs b/src/Bannerlord.ModuleManager.Models/ApplicationVersionRange.cs index 04185b2..ba82ae3 100644 --- a/src/Bannerlord.ModuleManager.Models/ApplicationVersionRange.cs +++ b/src/Bannerlord.ModuleManager.Models/ApplicationVersionRange.cs @@ -54,7 +54,16 @@ public bool IsSame(ApplicationVersionRange? other) => public bool IsSameWithChangeSet(ApplicationVersionRange? other) => Min.IsSameWithChangeSet(other?.Min) && Max.IsSameWithChangeSet(other?.Max); - public override string ToString() => $"{Min} - {Max}"; + public override string ToString() + { + // If the min and max changeset are at their default (0 and i32::MAX), we can + // simplify how we print the version. End user mods rarely use the changeset, + // it's only really used in official packages. + if (Min.ChangeSet == 0 && Max.ChangeSet == int.MaxValue) + return $"{Min.ToStringWithoutChangeset()} - {Max.ToStringWithoutChangeset()}"; + + return $"{Min} - {Max}"; + } public static bool TryParse(string versionRangeAsString, out ApplicationVersionRange versionRange) {