tweak new code

4 files changed, 104 insertions, 116 deletions

diff --git a/src/SMAPI/Framework/Content/AssetDataForImage.cs b/src/SMAPI/Framework/Content/AssetDataForImage.cs
index 3abcd328..0380dd9e 100644
--- a/src/SMAPI/Framework/Content/AssetDataForImage.cs
+++ b/src/SMAPI/Framework/Content/AssetDataForImage.cs
@@ -33,71 +33,59 @@ namespace StardewModdingAPI.Framework.Content
         /// <inheritdoc />
         public void PatchImage(IRawTextureData source, Rectangle? sourceArea = null, Rectangle? targetArea = null, PatchMode patchMode = PatchMode.Replace)
         {
-            // nullcheck
             if (source == null)
                 throw new ArgumentNullException(nameof(source), "Can't patch from null source data.");
 
+            // get normalized bounds
             this.GetPatchBounds(ref sourceArea, ref targetArea, source.Width, source.Height);
-
-            // check to see if the Data is sufficiently long.
-            // while SMAPI's impl is going to be, it's not necessarily the case for mod impl.
             if (source.Data.Length < (sourceArea.Value.Bottom - 1) * source.Width + sourceArea.Value.Right)
-                throw new ArgumentException("Source data insufficiently long for this operation.");
-
-            // get the pixels for the source area
-            Color[] sourceData;
+                throw new ArgumentException("Can't apply image patch because the source image is smaller than the source area.", nameof(source));
+            int areaX = sourceArea.Value.X;
+            int areaY = sourceArea.Value.Y;
+            int areaWidth = sourceArea.Value.Width;
+            int areaHeight = sourceArea.Value.Height;
+
+            // shortcut: if the area width matches the source image, we can apply the image as-is without needing
+            // to copy the pixels into a smaller subset. It's fine if the source is taller than the area, since we'll
+            // just ignore the extra data at the end of the pixel array.
+            if (areaWidth == source.Width)
             {
-                int areaX = sourceArea.Value.X;
-                int areaY = sourceArea.Value.Y;
-                int areaWidth = sourceArea.Value.Width;
-                int areaHeight = sourceArea.Value.Height;
+                this.PatchImageImpl(source.Data, source.Width, source.Height, sourceArea.Value, targetArea.Value, patchMode, areaY);
+                return;
+            }
 
-                if (areaWidth == source.Width)
+            // else copy the pixels within the smaller area & apply that
+            int pixelCount = areaWidth * areaHeight;
+            Color[] sourceData = ArrayPool<Color>.Shared.Rent(pixelCount);
+            try
+            {
+                for (int y = areaY, maxY = areaY + areaHeight; y < maxY; y++)
                 {
-                    // It's actually fine if the source is taller than the sourceArea
-                    // the "extra" bits on the end of the array can just be ignored.
-                    sourceData = source.Data;
-                    this.PatchImageImpl(sourceData, source.Width, source.Height, sourceArea.Value, targetArea.Value, patchMode, areaY);
+                    int sourceIndex = (y * source.Width) + areaX;
+                    int targetIndex = (y - areaY) * areaWidth;
+                    Array.Copy(source.Data, sourceIndex, sourceData, targetIndex, areaWidth);
                 }
-                else
-                {
-                    int pixelCount = areaWidth * areaHeight;
-                    sourceData = ArrayPool<Color>.Shared.Rent(pixelCount);
-                    try
-                    {
-                        // slower copying, line by line
-                        for (int y = areaY, maxY = areaY + areaHeight; y < maxY; y++)
-                        {
-                            int sourceIndex = (y * source.Width) + areaX;
-                            int targetIndex = (y - areaY) * areaWidth;
-                            Array.Copy(source.Data, sourceIndex, sourceData, targetIndex, areaWidth);
-                        }
 
-                        // apply
-                        this.PatchImageImpl(sourceData, source.Width, source.Height, sourceArea.Value, targetArea.Value, patchMode);
-                    }
-                    finally
-                    {
-                        ArrayPool<Color>.Shared.Return(sourceData);
-                    }
-                }
+                this.PatchImageImpl(sourceData, source.Width, source.Height, sourceArea.Value, targetArea.Value, patchMode);
+            }
+            finally
+            {
+                ArrayPool<Color>.Shared.Return(sourceData);
             }
         }
 
         /// <inheritdoc />
         public void PatchImage(Texture2D source, Rectangle? sourceArea = null, Rectangle? targetArea = null, PatchMode patchMode = PatchMode.Replace)
         {
-            // nullcheck
             if (source == null)
                 throw new ArgumentNullException(nameof(source), "Can't patch from a null source texture.");
 
+            // get normalized bounds
             this.GetPatchBounds(ref sourceArea, ref targetArea, source.Width, source.Height);
-
-            // validate source bounds
             if (!source.Bounds.Contains(sourceArea.Value))
                 throw new ArgumentOutOfRangeException(nameof(sourceArea), "The source area is outside the bounds of the source texture.");
 
-            // get source data
+            // get source data & apply
             int pixelCount = sourceArea.Value.Width * sourceArea.Value.Height;
             Color[] sourceData = ArrayPool<Color>.Shared.Rent(pixelCount);
             try
@@ -164,94 +152,91 @@ namespace StardewModdingAPI.Framework.Content
             if (sourceArea.Size != targetArea.Size)
                 throw new InvalidOperationException("The source and target areas must be the same size.");
 
+            // shortcut: replace the entire area
             if (patchMode == PatchMode.Replace)
-                target.SetData(0, targetArea, sourceData, startRow * sourceArea.Width, pixelCount);
-            else
             {
-                // merge data
-
-                // Content packs have a habit of using large amounts of blank space.
-                // Adjusting bounds to ignore transparent pixels at the start and end.
+                target.SetData(0, targetArea, sourceData, startRow * sourceArea.Width, pixelCount);
+                return;
+            }
 
-                int startIndex = -1;
+            // skip transparent pixels at the start & end (e.g. large spritesheet with a few sprites replaced)
+            int startIndex = -1;
+            int endIndex = -1;
+            {
                 for (int i = startRow * sourceArea.Width; i < pixelCount; i++)
                 {
-                    if (sourceData[i].A >= MinOpacity)
+                    if (sourceData[i].A >= AssetDataForImage.MinOpacity)
                     {
                         startIndex = i;
                         break;
                     }
                 }
-
                 if (startIndex == -1)
-                    return; // apparently a completely blank texture?
+                    return; // blank texture
 
-                int endIndex = -1;
                 for (int i = startRow * sourceArea.Width + pixelCount - 1; i >= startIndex; i--)
                 {
-                    if (sourceData[i].A >= MinOpacity)
+                    if (sourceData[i].A >= AssetDataForImage.MinOpacity)
                     {
                         endIndex = i;
                         break;
                     }
                 }
-
                 if (endIndex == -1)
-                    return; // should never happen
+                    return; // ???
+            }
 
-                // Calculate new Y bounds
-                int topoffset = startIndex / sourceArea.Width;
-                int bottomoffset = endIndex / sourceArea.Width;
+            // update target rectangle
+            int sourceOffset;
+            {
+                int topOffset = startIndex / sourceArea.Width;
+                int bottomOffset = endIndex / sourceArea.Width;
 
-                // Update target rectangle
-                targetArea = new(targetArea.X, targetArea.Y + topoffset, targetArea.Width, bottomoffset - topoffset + 1);
+                targetArea = new(targetArea.X, targetArea.Y + topOffset, targetArea.Width, bottomOffset - topOffset + 1);
                 pixelCount = targetArea.Width * targetArea.Height;
+                sourceOffset = topOffset * sourceArea.Width;
+            }
 
-                int sourceoffset = topoffset * sourceArea.Width;
+            // apply
+            Color[] mergedData = ArrayPool<Color>.Shared.Rent(pixelCount);
+            try
+            {
+                target.GetData(0, targetArea, mergedData, 0, pixelCount);
 
-                // get target data
-                Color[] mergedData = ArrayPool<Color>.Shared.Rent(pixelCount);
-                try
+                for (int i = startIndex; i <= endIndex; i++)
                 {
-                    target.GetData(0, targetArea, mergedData, 0, pixelCount);
+                    int targetIndex = i - sourceOffset;
 
-                    // merge pixels
-                    for (int i = startIndex; i <= endIndex; i++)
-                    {
-                        int targetIndex = i - sourceoffset;
+                    Color above = sourceData[i];
+                    Color below = mergedData[targetIndex];
 
-                        // ref locals here? Not sure.
-                        Color above = sourceData[i];
-                        Color below = mergedData[targetIndex];
+                    // shortcut transparency
+                    if (above.A < AssetDataForImage.MinOpacity)
+                        continue;
+                    if (below.A < AssetDataForImage.MinOpacity || above.A == byte.MaxValue)
+                        mergedData[targetIndex] = above;
 
-                        // shortcut transparency
-                        if (above.A < MinOpacity)
-                            continue;
-                        if (below.A < MinOpacity || above.A == byte.MaxValue)
-                            mergedData[targetIndex] = above;
-
-                        // merge pixels
-                        else
-                        {
-                            // This performs a conventional alpha blend for the pixels, which are already
-                            // premultiplied by the content pipeline. The formula is derived from
-                            // https://blogs.msdn.microsoft.com/shawnhar/2009/11/06/premultiplied-alpha/.
-                            float alphaBelow = 1 - (above.A / 255f);
-                            mergedData[targetIndex] = new Color(
-                                r: (int)(above.R + (below.R * alphaBelow)),
-                                g: (int)(above.G + (below.G * alphaBelow)),
-                                b: (int)(above.B + (below.B * alphaBelow)),
-                                alpha: Math.Max(above.A, below.A)
-                            );
-                        }
+                    // merge pixels
+                    else
+                    {
+                        // This performs a conventional alpha blend for the pixels, which are already
+                        // premultiplied by the content pipeline. The formula is derived from
+                        // https://blogs.msdn.microsoft.com/shawnhar/2009/11/06/premultiplied-alpha/.
+                        float alphaBelow = 1 - (above.A / 255f);
+                        mergedData[targetIndex] = new Color(
+                            r: (int)(above.R + (below.R * alphaBelow)),
+                            g: (int)(above.G + (below.G * alphaBelow)),
+                            b: (int)(above.B + (below.B * alphaBelow)),
+                            alpha: Math.Max(above.A, below.A)
+                        );
                     }
-
-                    target.SetData(0, targetArea, mergedData, 0, pixelCount);
-                }
-                finally
-                {
-                    ArrayPool<Color>.Shared.Return(mergedData);
                 }
+
+                target.SetData(0, targetArea, mergedData, 0, pixelCount);
+            }
+            finally
+            {
+                ArrayPool<Color>.Shared.Return(mergedData);
             }
         }
     }
diff --git a/src/SMAPI/Framework/ContentManagers/ModContentManager.cs b/src/SMAPI/Framework/ContentManagers/ModContentManager.cs
index 6b8a5874..72dcf6e1 100644
--- a/src/SMAPI/Framework/ContentManagers/ModContentManager.cs
+++ b/src/SMAPI/Framework/ContentManagers/ModContentManager.cs
@@ -241,7 +241,7 @@ namespace StardewModdingAPI.Framework.ContentManagers
             {
                 using FileStream stream = File.OpenRead(file.FullName);
                 Texture2D texture = Texture2D.FromStream(Game1.graphics.GraphicsDevice, stream);
-                texture = this.PremultiplyTransparency(texture);
+                this.PremultiplyTransparency(texture);
                 return (T)(object)texture;
             }
         }
@@ -345,17 +345,15 @@ namespace StardewModdingAPI.Framework.ContentManagers
                 this.ThrowLoadError(assetName, ContentLoadErrorType.InvalidData, $"can't read file with extension '{file.Extension}' as type '{typeof(TAsset)}'; must be type '{string.Join("' or '", validTypes.Select(p => p.FullName))}'.");
         }
 
-        /// <summary>Throws an error which indicates that an asset couldn't be loaded.</summary>
+        /// <summary>Throw an error which indicates that an asset couldn't be loaded.</summary>
         /// <param name="errorType">Why loading an asset through the content pipeline failed.</param>
         /// <param name="assetName">The asset name that failed to load.</param>
         /// <param name="reasonPhrase">The reason the file couldn't be loaded.</param>
         /// <param name="exception">The underlying exception, if applicable.</param>
+        /// <exception cref="SContentLoadException" />
         [DoesNotReturn]
         [DebuggerStepThrough, DebuggerHidden]
         [MethodImpl(MethodImplOptions.NoInlining)]
-#if NET6_0_OR_GREATER
-        [StackTraceHidden]
-#endif
         private void ThrowLoadError(IAssetName assetName, ContentLoadErrorType errorType, string reasonPhrase, Exception? exception = null)
         {
             throw new SContentLoadException(errorType, $"Failed loading asset '{assetName}' from {this.Name}: {reasonPhrase}", exception);
@@ -390,9 +388,8 @@ namespace StardewModdingAPI.Framework.ContentManagers
         /// <param name="texture">The texture to premultiply.</param>
         /// <returns>Returns a premultiplied texture.</returns>
         /// <remarks>Based on <a href="https://gamedev.stackexchange.com/a/26037">code by David Gouveia</a>.</remarks>
-        private Texture2D PremultiplyTransparency(Texture2D texture)
+        private void PremultiplyTransparency(Texture2D texture)
         {
-            // premultiply pixels
             int count = texture.Width * texture.Height;
             Color[] data = ArrayPool<Color>.Shared.Rent(count);
             try
@@ -412,8 +409,6 @@ namespace StardewModdingAPI.Framework.ContentManagers
 
                 if (changed)
                     texture.SetData(data, 0, count);
-
-                return texture;
             }
             finally
             {
diff --git a/src/SMAPI/Framework/Logging/LogOnceCacheKey.cs b/src/SMAPI/Framework/Logging/LogOnceCacheKey.cs
new file mode 100644
index 00000000..4d31ffeb
--- /dev/null
+++ b/src/SMAPI/Framework/Logging/LogOnceCacheKey.cs
@@ -0,0 +1,10 @@
+using System.Diagnostics.CodeAnalysis;
+
+namespace StardewModdingAPI.Framework.Logging
+{
+    /// <summary>The cache key for the <see cref="Monitor.LogOnceCache"/>.</summary>
+    /// <param name="Message">The log message.</param>
+    /// <param name="Level">The log level.</param>
+    [SuppressMessage("ReSharper", "NotAccessedPositionalProperty.Local", Justification = "This is only used as a lookup key.")]
+    internal readonly record struct LogOnceCacheKey(string Message, LogLevel Level);
+}
diff --git a/src/SMAPI/Framework/Monitor.cs b/src/SMAPI/Framework/Monitor.cs
index d33bf259..4ed2c9bb 100644
--- a/src/SMAPI/Framework/Monitor.cs
+++ b/src/SMAPI/Framework/Monitor.cs
@@ -25,15 +25,13 @@ namespace StardewModdingAPI.Framework
         private readonly LogFileManager LogFile;
 
         /// <summary>The maximum length of the <see cref="LogLevel"/> values.</summary>
-        private static readonly int MaxLevelLength = (from level in Enum.GetValues<LogLevel>() select level.ToString().Length).Max();
+        private static readonly int MaxLevelLength = Enum.GetValues<LogLevel>().Max(level => level.ToString().Length);
 
-        /// <summary>A mapping of console log levels to their string form.</summary>
-        private static readonly Dictionary<ConsoleLogLevel, string> LogStrings = Enum.GetValues<ConsoleLogLevel>().ToDictionary(k => k, v => v.ToString().ToUpper().PadRight(MaxLevelLength));
-
-        private readonly record struct LogOnceCacheEntry(string message, LogLevel level);
+        /// <summary>The cached representation for each level when added to a log header.</summary>
+        private static readonly Dictionary<ConsoleLogLevel, string> LogStrings = Enum.GetValues<ConsoleLogLevel>().ToDictionary(level => level, level => level.ToString().ToUpper().PadRight(Monitor.MaxLevelLength));
 
         /// <summary>A cache of messages that should only be logged once.</summary>
-        private readonly HashSet<LogOnceCacheEntry> LogOnceCache = new();
+        private readonly HashSet<LogOnceCacheKey> LogOnceCache = new();
 
         /// <summary>Get the screen ID that should be logged to distinguish between players in split-screen mode, if any.</summary>
         private readonly Func<int?> GetScreenIdForLog;
@@ -89,7 +87,7 @@ namespace StardewModdingAPI.Framework
         /// <inheritdoc />
         public void LogOnce(string message, LogLevel level = LogLevel.Trace)
         {
-            if (this.LogOnceCache.Add(new LogOnceCacheEntry(message, level)))
+            if (this.LogOnceCache.Add(new LogOnceCacheKey(message, level)))
                 this.LogImpl(this.Source, message, (ConsoleLogLevel)level);
         }
 
@@ -152,7 +150,7 @@ namespace StardewModdingAPI.Framework
         /// <param name="level">The log level.</param>
         private string GenerateMessagePrefix(string source, ConsoleLogLevel level)
         {
-            string levelStr = LogStrings[level];
+            string levelStr = Monitor.LogStrings[level];
             int? playerIndex = this.GetScreenIdForLog();
 
             return $"[{DateTime.Now:HH:mm:ss} {levelStr}{(playerIndex != null ? $" screen_{playerIndex}" : "")} {source}]";