Pathoschild / SMAPI

The modding API for Stardew Valley.
https://smapi.io/
GNU Lesser General Public License v3.0
1.71k stars 258 forks source link

Fix trailing space mod load and system dialogs #916

Closed jakerosado closed 2 days ago

jakerosado commented 8 months ago

Resolves #851, resolves #905

Pathoschild commented 2 days ago

I merged the open-folder command changes into develop manually for the upcoming SMAPI 4.1.0. Thanks for the help!

I was going to keep this PR open for this other change in StardewModdingAPI.Toolkit.Utilities.PathUtilities:

        public static string? NormalizePath(string? path)
        {
-            path = path?.Trim();
-            if (string.IsNullOrEmpty(path))
-                return path;
+            // safety check if directory is null or contains trailing spaces
+            string? trailingPath = path;
+
+            trailingPath = trailingPath?.Trim();
+            if (string.IsNullOrEmpty(trailingPath))
+                return trailingPath;

Unfortunately splitting the PR confused GitHub, so it closed the PR.

It seems like that change would cause unintended consequences though. For example, paths everywhere would no longer be trimmed, which may cause errors in cases where user input is normalized (e.g. Content Patcher content packs which should ignore the surrounding spaces in {{HasFile: asset/somePath.png }}).

jakerosado commented 2 days ago

Made a fix in #958. I changed the null/empty check on the path from the get-go and perform the trimming only if there's no trailing space. I tested with several mods (FastAnimations, Fishnets, FrameRateLogger) by swapping which had trailing spaces. It also handles duplicate instances where there would be two of the same mods, i.e. FastAnimations, with and without a trailing space.