diff --git a/Core/Games/IGame.cs b/Core/Games/IGame.cs
index 1eb31a2849..7918826626 100644
--- a/Core/Games/IGame.cs
+++ b/Core/Games/IGame.cs
@@ -19,9 +19,10 @@ public interface IGame
// What do we contain?
string PrimaryModDirectoryRelative { get; }
string PrimaryModDirectory(GameInstance inst);
- string[] StockFolders { get; }
- string[] ReservedPaths { get; }
- string[] CreateableDirs { get; }
+ string[] StockFolders { get; }
+ string[] ReservedPaths { get; }
+ string[] CreateableDirs { get; }
+ string[] AutoRemovableDirs { get; }
bool IsReservedDirectory(GameInstance inst, string path);
bool AllowInstallationIn(string name, out string path);
void RebuildSubdirectories(string absGameRoot);
@@ -39,5 +40,4 @@ public interface IGame
Uri DefaultRepositoryURL { get; }
Uri RepositoryListURL { get; }
}
-
}
diff --git a/Core/Games/KerbalSpaceProgram.cs b/Core/Games/KerbalSpaceProgram.cs
index 2ea2b5c660..c94961c168 100644
--- a/Core/Games/KerbalSpaceProgram.cs
+++ b/Core/Games/KerbalSpaceProgram.cs
@@ -119,6 +119,11 @@ public string PrimaryModDirectory(GameInstance inst)
"GameData", "Tutorial", "Scenarios", "Missions", "Ships/Script"
};
+ public string[] AutoRemovableDirs => new string[]
+ {
+ "@thumbs"
+ };
+
///
/// Checks the path against a list of reserved game directories
///
diff --git a/Core/Games/KerbalSpaceProgram2.cs b/Core/Games/KerbalSpaceProgram2.cs
index af1ac89c4b..4b063d7edb 100644
--- a/Core/Games/KerbalSpaceProgram2.cs
+++ b/Core/Games/KerbalSpaceProgram2.cs
@@ -121,6 +121,8 @@ public string PrimaryModDirectory(GameInstance inst)
"BepInEx/plugins",
};
+ public string[] AutoRemovableDirs => new string[] { };
+
///
/// Checks the path against a list of reserved game directories
///
diff --git a/Core/ModuleInstaller.cs b/Core/ModuleInstaller.cs
index 173d538415..040a8ddfdb 100644
--- a/Core/ModuleInstaller.cs
+++ b/Core/ModuleInstaller.cs
@@ -14,6 +14,7 @@
using CKAN.Extensions;
using CKAN.Versioning;
using CKAN.Configuration;
+using CKAN.Games;
namespace CKAN
{
@@ -762,7 +763,7 @@ private void Uninstall(string modName, ref HashSet possibleConfigOnlyDir
}
// Walk our registry to find all files for this mod.
- IEnumerable files = mod.Files;
+ var files = mod.Files.ToArray();
// We need case insensitive path matching on Windows
var directoriesToDelete = Platform.IsWindows
@@ -826,7 +827,7 @@ private void Uninstall(string modName, ref HashSet possibleConfigOnlyDir
// Sort our directories from longest to shortest, to make sure we remove child directories
// before parents. GH #78.
- foreach (string directory in directoriesToDelete.OrderBy(dir => dir.Length).Reverse())
+ foreach (string directory in directoriesToDelete.OrderByDescending(dir => dir.Length))
{
log.DebugFormat("Checking {0}...", directory);
// It is bad if any of this directories gets removed
@@ -838,16 +839,42 @@ private void Uninstall(string modName, ref HashSet possibleConfigOnlyDir
continue;
}
- var contents = Directory
- .EnumerateFileSystemEntries(directory, "*", SearchOption.AllDirectories)
- .Select(f => ksp.ToRelativeGameDir(f))
- .Memoize();
- log.DebugFormat("Got contents: {0}", string.Join(", ", contents));
- var owners = contents.Select(f => registry.FileOwner(f));
- log.DebugFormat("Got owners: {0}", string.Join(", ", owners));
- if (!contents.Any())
+ // See what's left in this folder and what we can do about it
+ GroupFilesByRemovable(ksp.ToRelativeGameDir(directory),
+ registry, files, ksp.game,
+ (Directory.Exists(directory)
+ ? Directory.EnumerateFileSystemEntries(directory, "*", SearchOption.AllDirectories)
+ : Enumerable.Empty())
+ .Select(f => ksp.ToRelativeGameDir(f)),
+ out string[] removable,
+ out string[] notRemovable);
+
+ // Delete the auto-removable files and dirs
+ foreach (var relPath in removable)
{
+ var absPath = ksp.ToAbsoluteGameDir(relPath);
+ if (File.Exists(absPath))
+ {
+ log.DebugFormat("Attempting transaction deletion of file {0}", absPath);
+ file_transaction.Delete(absPath);
+ }
+ else if (Directory.Exists(absPath))
+ {
+ log.DebugFormat("Attempting deletion of directory {0}", absPath);
+ try
+ {
+ Directory.Delete(absPath);
+ }
+ catch
+ {
+ // There might be files owned by other mods, oh well
+ log.DebugFormat("Failed to delete {0}", absPath);
+ }
+ }
+ }
+ if (!notRemovable.Any())
+ {
// We *don't* use our file_transaction to delete files here, because
// it fails if the system's temp directory is on a different device
// to KSP. However we *can* safely delete it now we know it's empty,
@@ -860,9 +887,10 @@ private void Uninstall(string modName, ref HashSet possibleConfigOnlyDir
log.DebugFormat("Removing {0}", directory);
Directory.Delete(directory);
}
- else if (contents.All(f => registry.FileOwner(f) == null))
+ else if (notRemovable.All(f => registry.FileOwner(f) == null && !files.Contains(f)))
{
- log.DebugFormat("Directory {0} contains only non-registered files, ask user about it later", directory);
+ log.DebugFormat("Directory {0} contains only non-registered files, ask user about it later: {1}",
+ directory, string.Join(", ", notRemovable));
if (possibleConfigOnlyDirs == null)
{
possibleConfigOnlyDirs = new HashSet();
@@ -879,6 +907,35 @@ private void Uninstall(string modName, ref HashSet possibleConfigOnlyDir
}
}
+ internal static void GroupFilesByRemovable(string relRoot,
+ Registry registry,
+ string[] alreadyRemoving,
+ IGame game,
+ IEnumerable relPaths,
+ out string[] removable,
+ out string[] notRemovable)
+ {
+ log.DebugFormat("Getting contents of {0}", relRoot);
+ var contents = relPaths
+ // Split into auto-removable and not-removable
+ // Removable must not be owned by other mods
+ .GroupBy(f => registry.FileOwner(f) == null
+ // Also skip owned by this module since it's already deregistered
+ && !alreadyRemoving.Contains(f)
+ // Must have a removable dir name somewhere in path AFTER main dir
+ && f.Substring(relRoot.Length)
+ .Split('/')
+ .Where(piece => !string.IsNullOrEmpty(piece))
+ .Any(piece => game.AutoRemovableDirs.Contains(piece)))
+ .ToDictionary(grp => grp.Key,
+ grp => grp.OrderByDescending(f => f.Length)
+ .ToArray());
+ removable = contents.TryGetValue(true, out string[] val1) ? val1 : new string[] {};
+ notRemovable = contents.TryGetValue(false, out string[] val2) ? val2 : new string[] {};
+ log.DebugFormat("Got removable: {0}", string.Join(", ", removable));
+ log.DebugFormat("Got notRemovable: {0}", string.Join(", ", notRemovable));
+ }
+
///
/// Takes a collection of directories and adds all parent directories within the GameData structure.
///
diff --git a/GUI/Main/MainInstall.cs b/GUI/Main/MainInstall.cs
index 7db5bea1d9..0b80a4d2a9 100644
--- a/GUI/Main/MainInstall.cs
+++ b/GUI/Main/MainInstall.cs
@@ -268,8 +268,10 @@ private void HandlePossibleConfigOnlyDirs(Registry registry, HashSet pos
// Check again for registered files, since we may
// just have installed or upgraded some
possibleConfigOnlyDirs.RemoveWhere(
- d => Directory.EnumerateFileSystemEntries(d, "*", SearchOption.AllDirectories)
- .Any(f => registry.FileOwner(CurrentInstance.ToRelativeGameDir(f)) != null));
+ d => !Directory.Exists(d)
+ || Directory.EnumerateFileSystemEntries(d, "*", SearchOption.AllDirectories)
+ .Select(absF => CurrentInstance.ToRelativeGameDir(absF))
+ .Any(relF => registry.FileOwner(relF) != null));
if (possibleConfigOnlyDirs.Count > 0)
{
AddStatusMessage("");
diff --git a/Tests/Core/ModuleInstaller.cs b/Tests/Core/ModuleInstallerTests.cs
similarity index 82%
rename from Tests/Core/ModuleInstaller.cs
rename to Tests/Core/ModuleInstallerTests.cs
index 8d1d7cef89..51e515b3f2 100644
--- a/Tests/Core/ModuleInstaller.cs
+++ b/Tests/Core/ModuleInstallerTests.cs
@@ -5,6 +5,7 @@
using System.Linq;
using System.Text.RegularExpressions;
using System.Transactions;
+
using ICSharpCode.SharpZipLib.Zip;
using NUnit.Framework;
@@ -16,7 +17,7 @@
namespace Tests.Core
{
[TestFixture]
- public class ModuleInstaller
+ public class ModuleInstallerTests
{
private string flag_path;
private string dogezip;
@@ -607,6 +608,141 @@ public void UninstallEmptyDirs()
}
}
+ [Test,
+ // Empty dir
+ TestCase("GameData/SomeMod/Parts",
+ new string[] {},
+ new string[] {},
+ new string[] {},
+ new string[] {}),
+ // A few regular files and some thumbnails
+ TestCase("GameData/SomeMod/Parts",
+ new string[] {},
+ new string[]
+ {
+ "GameData/SomeMod/Parts/userfile.cfg",
+ "GameData/SomeMod/Parts/userfile2.cfg",
+ "GameData/SomeMod/Parts/@thumbs",
+ "GameData/SomeMod/Parts/@thumbs/part1.png",
+ "GameData/SomeMod/Parts/@thumbs/part3.png",
+ "GameData/SomeMod/Parts/@thumbs/part4.png",
+ },
+ new string[]
+ {
+ "GameData/SomeMod/Parts/@thumbs/part1.png",
+ "GameData/SomeMod/Parts/@thumbs/part3.png",
+ "GameData/SomeMod/Parts/@thumbs/part4.png",
+ "GameData/SomeMod/Parts/@thumbs",
+ },
+ new string[]
+ {
+ "GameData/SomeMod/Parts/userfile2.cfg",
+ "GameData/SomeMod/Parts/userfile.cfg",
+ }),
+ // Just regular files
+ TestCase("GameData/SomeMod/Parts",
+ new string[] {},
+ new string[]
+ {
+ "GameData/SomeMod/Parts/userfile.cfg",
+ "GameData/SomeMod/Parts/userfile2.cfg",
+ },
+ new string[] {},
+ new string[]
+ {
+ "GameData/SomeMod/Parts/userfile2.cfg",
+ "GameData/SomeMod/Parts/userfile.cfg",
+ }),
+ // Just thumbnails
+ TestCase("GameData/SomeMod/Parts",
+ new string[] {},
+ new string[]
+ {
+ "GameData/SomeMod/Parts/@thumbs",
+ "GameData/SomeMod/Parts/@thumbs/part1.png",
+ "GameData/SomeMod/Parts/@thumbs/part3.png",
+ "GameData/SomeMod/Parts/@thumbs/part4.png",
+ },
+ new string[]
+ {
+ "GameData/SomeMod/Parts/@thumbs/part1.png",
+ "GameData/SomeMod/Parts/@thumbs/part3.png",
+ "GameData/SomeMod/Parts/@thumbs/part4.png",
+ "GameData/SomeMod/Parts/@thumbs",
+ },
+ new string[] {}),
+ // A few regular files and some thumbnails, some of which are owned by another mod
+ TestCase("GameData/SomeMod/Parts",
+ new string[]
+ {
+ "GameData/SomeMod/Parts/userfile2.cfg",
+ "GameData/SomeMod/Parts/@thumbs/part1.png",
+ },
+ new string[]
+ {
+ "GameData/SomeMod/Parts/userfile.cfg",
+ "GameData/SomeMod/Parts/userfile2.cfg",
+ "GameData/SomeMod/Parts/@thumbs",
+ "GameData/SomeMod/Parts/@thumbs/part1.png",
+ "GameData/SomeMod/Parts/@thumbs/part3.png",
+ "GameData/SomeMod/Parts/@thumbs/part4.png",
+ },
+ new string[]
+ {
+ "GameData/SomeMod/Parts/@thumbs/part3.png",
+ "GameData/SomeMod/Parts/@thumbs/part4.png",
+ "GameData/SomeMod/Parts/@thumbs",
+ },
+ new string[]
+ {
+ "GameData/SomeMod/Parts/@thumbs/part1.png",
+ "GameData/SomeMod/Parts/userfile2.cfg",
+ "GameData/SomeMod/Parts/userfile.cfg",
+ }),
+ ]
+ public void GroupFilesByRemovable_WithFiles_CorrectOutput(string relRoot,
+ string[] registeredFiles,
+ string[] relPaths,
+ string[] correctRemovable,
+ string[] correctNotRemovable)
+ {
+ // Arrange
+ using (var inst = new DisposableKSP())
+ {
+ var game = new KerbalSpaceProgram();
+ var registry = CKAN.RegistryManager.Instance(inst.KSP).registry;
+ // Make files to be registered to another mod
+ var absFiles = registeredFiles.Select(f => inst.KSP.ToAbsoluteGameDir(f))
+ .ToArray();
+ foreach (var absPath in absFiles)
+ {
+ Directory.CreateDirectory(Path.GetDirectoryName(absPath));
+ File.Create(absPath).Dispose();
+ }
+ // Register the other mod
+ registry.RegisterModule(CkanModule.FromJson(@"{
+ ""spec_version"": 1,
+ ""identifier"": ""otherMod"",
+ ""version"": ""1.0"",
+ ""download"": ""https://github.com/""
+ }"),
+ absFiles, inst.KSP, false);
+
+ // Act
+ CKAN.ModuleInstaller.GroupFilesByRemovable(relRoot,
+ registry,
+ new string[] {},
+ game,
+ relPaths,
+ out string[] removable,
+ out string[] notRemovable);
+
+ // Assert
+ Assert.AreEqual(correctRemovable, removable);
+ Assert.AreEqual(correctNotRemovable, notRemovable);
+ }
+ }
+
[Test]
public void ModuleManagerInstancesAreDecoupled()
{