From cf383870837748e83b99bf63d36d7a8709743715 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Thu, 15 Feb 2018 23:58:27 -0500 Subject: log mod errors and warnings as the mod (#438) --- src/SMAPI/Framework/SContentManager.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'src/SMAPI/Framework/SContentManager.cs') diff --git a/src/SMAPI/Framework/SContentManager.cs b/src/SMAPI/Framework/SContentManager.cs index ebea6c84..ff227fac 100644 --- a/src/SMAPI/Framework/SContentManager.cs +++ b/src/SMAPI/Framework/SContentManager.cs @@ -581,7 +581,7 @@ namespace StardewModdingAPI.Framework } catch (Exception ex) { - this.Monitor.Log($"{entry.Key.DisplayName} crashed when checking whether it could load asset '{info.AssetName}', and will be ignored. Error details:\n{ex.GetLogSummary()}", LogLevel.Error); + entry.Key.LogAsMod($"Mod failed when checking whether it could load asset '{info.AssetName}', and will be ignored. Error details:\n{ex.GetLogSummary()}", LogLevel.Error); return false; } }) @@ -608,14 +608,14 @@ namespace StardewModdingAPI.Framework } catch (Exception ex) { - this.Monitor.Log($"{mod.DisplayName} crashed when loading asset '{info.AssetName}'. SMAPI will use the default asset instead. Error details:\n{ex.GetLogSummary()}", LogLevel.Error); + mod.LogAsMod($"Mod crashed when loading asset '{info.AssetName}'. SMAPI will use the default asset instead. Error details:\n{ex.GetLogSummary()}", LogLevel.Error); return null; } // validate asset if (data == null) { - this.Monitor.Log($"{mod.DisplayName} incorrectly set asset '{info.AssetName}' to a null value; ignoring override.", LogLevel.Error); + mod.LogAsMod($"Mod incorrectly set asset '{info.AssetName}' to a null value; ignoring override.", LogLevel.Error); return null; } @@ -644,7 +644,7 @@ namespace StardewModdingAPI.Framework } catch (Exception ex) { - this.Monitor.Log($"{mod.DisplayName} crashed when checking whether it could edit asset '{info.AssetName}', and will be ignored. Error details:\n{ex.GetLogSummary()}", LogLevel.Error); + mod.LogAsMod($"Mod crashed when checking whether it could edit asset '{info.AssetName}', and will be ignored. Error details:\n{ex.GetLogSummary()}", LogLevel.Error); continue; } @@ -657,18 +657,18 @@ namespace StardewModdingAPI.Framework } catch (Exception ex) { - this.Monitor.Log($"{mod.DisplayName} crashed when editing asset '{info.AssetName}', which may cause errors in-game. Error details:\n{ex.GetLogSummary()}", LogLevel.Error); + mod.LogAsMod($"Mod crashed when editing asset '{info.AssetName}', which may cause errors in-game. Error details:\n{ex.GetLogSummary()}", LogLevel.Error); } // validate edit if (asset.Data == null) { - this.Monitor.Log($"{mod.DisplayName} incorrectly set asset '{info.AssetName}' to a null value; ignoring override.", LogLevel.Warn); + mod.LogAsMod($"Mod incorrectly set asset '{info.AssetName}' to a null value; ignoring override.", LogLevel.Warn); asset = GetNewData(prevAsset); } else if (!(asset.Data is T)) { - this.Monitor.Log($"{mod.DisplayName} incorrectly set asset '{asset.AssetName}' to incompatible type '{asset.Data.GetType()}', expected '{typeof(T)}'; ignoring override.", LogLevel.Warn); + mod.LogAsMod($"Mod incorrectly set asset '{asset.AssetName}' to incompatible type '{asset.Data.GetType()}', expected '{typeof(T)}'; ignoring override.", LogLevel.Warn); asset = GetNewData(prevAsset); } } -- cgit From d926133608b227add19f0aa711bf3efb5da5f0bd Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Fri, 16 Feb 2018 22:33:33 -0500 Subject: fix deadlock in rare cases when injecting an asset (#441) --- docs/release-notes.md | 1 + src/SMAPI/Framework/SContentManager.cs | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) (limited to 'src/SMAPI/Framework/SContentManager.cs') diff --git a/docs/release-notes.md b/docs/release-notes.md index c4c269eb..6b0c5d28 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -7,6 +7,7 @@ * For modders: * Fixed error when accessing a mod-provided API whose underlying class is `internal`. + * Fixed deadlock in rare cases when injecting a file with an asset loader. * For SMAPI developers: * All mod assemblies are now rewritten in-memory to support features like mod-provided APIs. diff --git a/src/SMAPI/Framework/SContentManager.cs b/src/SMAPI/Framework/SContentManager.cs index ff227fac..463fea0b 100644 --- a/src/SMAPI/Framework/SContentManager.cs +++ b/src/SMAPI/Framework/SContentManager.cs @@ -792,12 +792,12 @@ namespace StardewModdingAPI.Framework { try { - this.Lock.EnterReadLock(); + this.Lock.EnterWriteLock(); return action(); } finally { - this.Lock.ExitReadLock(); + this.Lock.ExitWriteLock(); } } } -- cgit From 3b4e81bf69e28c9bcc33c782f58e5099d73c4f91 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Mon, 19 Feb 2018 20:18:30 -0500 Subject: encapsulate path utilities for reuse, add unit tests --- src/SMAPI.Tests/Core/PathUtilitiesTests.cs | 70 +++++++++++++++++++++++++ src/SMAPI.Tests/StardewModdingAPI.Tests.csproj | 1 + src/SMAPI/Framework/Content/ContentCache.cs | 19 ++----- src/SMAPI/Framework/ModHelpers/ContentHelper.cs | 3 +- src/SMAPI/Framework/SContentManager.cs | 18 +------ src/SMAPI/Framework/Utilities/PathUtilities.cs | 59 +++++++++++++++++++++ src/SMAPI/StardewModdingAPI.csproj | 1 + 7 files changed, 138 insertions(+), 33 deletions(-) create mode 100644 src/SMAPI.Tests/Core/PathUtilitiesTests.cs create mode 100644 src/SMAPI/Framework/Utilities/PathUtilities.cs (limited to 'src/SMAPI/Framework/SContentManager.cs') diff --git a/src/SMAPI.Tests/Core/PathUtilitiesTests.cs b/src/SMAPI.Tests/Core/PathUtilitiesTests.cs new file mode 100644 index 00000000..268ba504 --- /dev/null +++ b/src/SMAPI.Tests/Core/PathUtilitiesTests.cs @@ -0,0 +1,70 @@ +using NUnit.Framework; +using StardewModdingAPI.Framework.Utilities; + +namespace StardewModdingAPI.Tests.Core +{ + /// Unit tests for . + [TestFixture] + public class PathUtilitiesTests + { + /********* + ** Unit tests + *********/ + [Test(Description = "Assert that GetSegments returns the expected values.")] + [TestCase("", ExpectedResult = "")] + [TestCase("/", ExpectedResult = "")] + [TestCase("///", ExpectedResult = "")] + [TestCase("/usr/bin", ExpectedResult = "usr|bin")] + [TestCase("/usr//bin//", ExpectedResult = "usr|bin")] + [TestCase("/usr//bin//.././boop.exe", ExpectedResult = "usr|bin|..|.|boop.exe")] + [TestCase(@"C:", ExpectedResult = "C:")] + [TestCase(@"C:/boop", ExpectedResult = "C:|boop")] + [TestCase(@"C:\boop\/usr//bin//.././boop.exe", ExpectedResult = "C:|boop|usr|bin|..|.|boop.exe")] + public string GetSegments(string path) + { + return string.Join("|", PathUtilities.GetSegments(path)); + } + + [Test(Description = "Assert that NormalisePathSeparators returns the expected values.")] +#if SMAPI_FOR_WINDOWS + [TestCase("", ExpectedResult = "")] + [TestCase("/", ExpectedResult = "")] + [TestCase("///", ExpectedResult = "")] + [TestCase("/usr/bin", ExpectedResult = @"usr\bin")] + [TestCase("/usr//bin//", ExpectedResult = @"usr\bin")] + [TestCase("/usr//bin//.././boop.exe", ExpectedResult = @"usr\bin\..\.\boop.exe")] + [TestCase("C:", ExpectedResult = "C:")] + [TestCase("C:/boop", ExpectedResult = @"C:\boop")] + [TestCase(@"C:\usr\bin//.././boop.exe", ExpectedResult = @"C:\usr\bin\..\.\boop.exe")] +#else + [TestCase("", ExpectedResult = "")] + [TestCase("/", ExpectedResult = "/")] + [TestCase("///", ExpectedResult = "/")] + [TestCase("/usr/bin", ExpectedResult = "/usr/bin")] + [TestCase("/usr//bin//", ExpectedResult = "/usr/bin")] + [TestCase("/usr//bin//.././boop.exe", ExpectedResult = "/usr/bin/.././boop.exe")] + [TestCase("C:", ExpectedResult = "C:")] + [TestCase("C:/boop", ExpectedResult = "C:/boop")] + [TestCase(@"C:\usr\bin//.././boop.exe", ExpectedResult = "C:/usr/bin/.././boop.exe")] +#endif + public string NormalisePathSeparators(string path) + { + return PathUtilities.NormalisePathSeparators(path); + } + + [Test(Description = "Assert that GetRelativePath returns the expected values.")] +#if SMAPI_FOR_WINDOWS + [TestCase(@"C:\", @"C:\", ExpectedResult = "./")] + [TestCase(@"C:\grandparent\parent\child", @"C:\grandparent\parent\sibling", ExpectedResult = @"..\sibling")] + [TestCase(@"C:\grandparent\parent\child", @"C:\cousin\file.exe", ExpectedResult = @"..\..\..\cousin\file.exe")] +#else + [TestCase("/", "/", ExpectedResult = "./")] + [TestCase("/grandparent/parent/child", "/grandparent/parent/sibling", ExpectedResult = "../sibling")] + [TestCase("/grandparent/parent/child", "/cousin/file.exe", ExpectedResult = "../../../cousin/file.exe")] +#endif + public string GetRelativePath(string sourceDir, string targetPath) + { + return PathUtilities.GetRelativePath(sourceDir, targetPath); + } + } +} diff --git a/src/SMAPI.Tests/StardewModdingAPI.Tests.csproj b/src/SMAPI.Tests/StardewModdingAPI.Tests.csproj index 6e7fa1f0..e4b2c8c6 100644 --- a/src/SMAPI.Tests/StardewModdingAPI.Tests.csproj +++ b/src/SMAPI.Tests/StardewModdingAPI.Tests.csproj @@ -49,6 +49,7 @@ Properties\GlobalAssemblyInfo.cs + diff --git a/src/SMAPI/Framework/Content/ContentCache.cs b/src/SMAPI/Framework/Content/ContentCache.cs index 4508e641..533da398 100644 --- a/src/SMAPI/Framework/Content/ContentCache.cs +++ b/src/SMAPI/Framework/Content/ContentCache.cs @@ -5,6 +5,7 @@ using System.Linq; using Microsoft.Xna.Framework; using StardewModdingAPI.Framework.ModLoading; using StardewModdingAPI.Framework.Reflection; +using StardewModdingAPI.Framework.Utilities; using StardewValley; namespace StardewModdingAPI.Framework.Content @@ -18,12 +19,6 @@ namespace StardewModdingAPI.Framework.Content /// The underlying asset cache. private readonly IDictionary Cache; - /// The possible directory separator characters in an asset key. - private readonly char[] PossiblePathSeparators; - - /// The preferred directory separator chaeacter in an asset key. - private readonly string PreferredPathSeparator; - /// Applies platform-specific asset key normalisation so it's consistent with the underlying cache. private readonly Func NormaliseAssetNameForPlatform; @@ -52,14 +47,10 @@ namespace StardewModdingAPI.Framework.Content /// Construct an instance. /// The underlying content manager whose cache to manage. /// Simplifies access to private game code. - /// The possible directory separator characters in an asset key. - /// The preferred directory separator chaeacter in an asset key. - public ContentCache(LocalizedContentManager contentManager, Reflector reflection, char[] possiblePathSeparators, string preferredPathSeparator) + public ContentCache(LocalizedContentManager contentManager, Reflector reflection) { // init this.Cache = reflection.GetField>(contentManager, "loadedAssets").GetValue(); - this.PossiblePathSeparators = possiblePathSeparators; - this.PreferredPathSeparator = preferredPathSeparator; // get key normalisation logic if (Constants.TargetPlatform == Platform.Windows) @@ -90,11 +81,7 @@ namespace StardewModdingAPI.Framework.Content [Pure] public string NormalisePathSeparators(string path) { - string[] parts = path.Split(this.PossiblePathSeparators, StringSplitOptions.RemoveEmptyEntries); - string normalised = string.Join(this.PreferredPathSeparator, parts); - if (path.StartsWith(this.PreferredPathSeparator)) - normalised = this.PreferredPathSeparator + normalised; // keep root slash - return normalised; + return PathUtilities.NormalisePathSeparators(path); } /// Normalise a cache key so it's consistent with the underlying cache. diff --git a/src/SMAPI/Framework/ModHelpers/ContentHelper.cs b/src/SMAPI/Framework/ModHelpers/ContentHelper.cs index 4a1d3853..7d8bec1e 100644 --- a/src/SMAPI/Framework/ModHelpers/ContentHelper.cs +++ b/src/SMAPI/Framework/ModHelpers/ContentHelper.cs @@ -8,6 +8,7 @@ using System.Linq; using Microsoft.Xna.Framework.Content; using Microsoft.Xna.Framework.Graphics; using StardewModdingAPI.Framework.Exceptions; +using StardewModdingAPI.Framework.Utilities; using StardewValley; using xTile; using xTile.Format; @@ -238,7 +239,7 @@ namespace StardewModdingAPI.Framework.ModHelpers string imageSource = tilesheet.ImageSource; // validate tilesheet path - if (Path.IsPathRooted(imageSource) || imageSource.Split(SContentManager.PossiblePathSeparators).Contains("..")) + if (Path.IsPathRooted(imageSource) || PathUtilities.GetSegments(imageSource).Contains("..")) throw new ContentLoadException($"The '{imageSource}' tilesheet couldn't be loaded. Tilesheet paths must be a relative path without directory climbing (../)."); // get seasonal name (if applicable) diff --git a/src/SMAPI/Framework/SContentManager.cs b/src/SMAPI/Framework/SContentManager.cs index 463fea0b..fa51bd53 100644 --- a/src/SMAPI/Framework/SContentManager.cs +++ b/src/SMAPI/Framework/SContentManager.cs @@ -35,9 +35,6 @@ namespace StardewModdingAPI.Framework /********* ** Properties *********/ - /// The preferred directory separator chaeacter in an asset key. - private static readonly string PreferredPathSeparator = Path.DirectorySeparatorChar.ToString(); - /// Encapsulates monitoring and logging. private readonly IMonitor Monitor; @@ -75,9 +72,6 @@ namespace StardewModdingAPI.Framework /// Interceptors which edit matching assets after they're loaded. internal IDictionary> Editors { get; } = new Dictionary>(); - /// The possible directory separator characters in an asset key. - internal static readonly char[] PossiblePathSeparators = new[] { '/', '\\', Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar }.Distinct().ToArray(); - /// The absolute path to the . internal string FullRootDirectory => Path.Combine(Constants.ExecutionPath, this.RootDirectory); @@ -100,7 +94,7 @@ namespace StardewModdingAPI.Framework { // init this.Monitor = monitor ?? throw new ArgumentNullException(nameof(monitor)); - this.Cache = new ContentCache(this, reflection, SContentManager.PossiblePathSeparators, SContentManager.PreferredPathSeparator); + this.Cache = new ContentCache(this, reflection); this.GetKeyLocale = reflection.GetMethod(this, "languageCode"); this.ModContentPrefix = this.GetAssetNameFromFilePath(Constants.ModPath); @@ -399,15 +393,7 @@ namespace StardewModdingAPI.Framework /// The target file path. private string GetRelativePath(string targetPath) { - // convert to URIs - Uri from = new Uri(this.FullRootDirectory + "/"); - Uri to = new Uri(targetPath + "/"); - if (from.Scheme != to.Scheme) - throw new InvalidOperationException($"Can't get path for '{targetPath}' relative to '{this.FullRootDirectory}'."); - - // get relative path - return Uri.UnescapeDataString(from.MakeRelativeUri(to).ToString()) - .Replace(Path.DirectorySeparatorChar == '/' ? '\\' : '/', Path.DirectorySeparatorChar); // use correct separator for platform + return PathUtilities.GetRelativePath(this.FullRootDirectory, targetPath); } /// Get the locale codes (like ja-JP) used in asset keys. diff --git a/src/SMAPI/Framework/Utilities/PathUtilities.cs b/src/SMAPI/Framework/Utilities/PathUtilities.cs new file mode 100644 index 00000000..4fa521f1 --- /dev/null +++ b/src/SMAPI/Framework/Utilities/PathUtilities.cs @@ -0,0 +1,59 @@ +using System; +using System.Diagnostics.Contracts; +using System.IO; +using System.Linq; + +namespace StardewModdingAPI.Framework.Utilities +{ + /// Provides utilities for normalising file paths. + internal static class PathUtilities + { + /********* + ** Properties + *********/ + /// The possible directory separator characters in a file path. + private static readonly char[] PossiblePathSeparators = new[] { '/', '\\', Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar }.Distinct().ToArray(); + + /// The preferred directory separator chaeacter in an asset key. + private static readonly string PreferredPathSeparator = Path.DirectorySeparatorChar.ToString(); + + + /********* + ** Public methods + *********/ + /// Get the segments from a path (e.g. /usr/bin/boop => usr, bin, and boop). + /// The path to split. + public static string[] GetSegments(string path) + { + return path.Split(PathUtilities.PossiblePathSeparators, StringSplitOptions.RemoveEmptyEntries); + } + + /// Normalise path separators in a file path. + /// The file path to normalise. + [Pure] + public static string NormalisePathSeparators(string path) + { + string[] parts = PathUtilities.GetSegments(path); + string normalised = string.Join(PathUtilities.PreferredPathSeparator, parts); + if (path.StartsWith(PathUtilities.PreferredPathSeparator)) + normalised = PathUtilities.PreferredPathSeparator + normalised; // keep root slash + return normalised; + } + + /// Get a directory or file path relative to a given source path. + /// The source folder path. + /// The target folder or file path. + [Pure] + public static string GetRelativePath(string sourceDir, string targetPath) + { + // convert to URIs + Uri from = new Uri(sourceDir.TrimEnd(PathUtilities.PossiblePathSeparators) + "/"); + Uri to = new Uri(targetPath.TrimEnd(PathUtilities.PossiblePathSeparators) + "/"); + if (from.Scheme != to.Scheme) + throw new InvalidOperationException($"Can't get path for '{targetPath}' relative to '{sourceDir}'."); + + // get relative path + return PathUtilities.NormalisePathSeparators(Uri.UnescapeDataString(from.MakeRelativeUri(to).ToString())); + } + } +} diff --git a/src/SMAPI/StardewModdingAPI.csproj b/src/SMAPI/StardewModdingAPI.csproj index 562da60d..14ed1531 100644 --- a/src/SMAPI/StardewModdingAPI.csproj +++ b/src/SMAPI/StardewModdingAPI.csproj @@ -125,6 +125,7 @@ + -- cgit