[ME] Local / deduped textures #370

2025-10-30T02:06:00Z

starstormhun commented

2025-10-30 02:06:00 +00:00

(Migrated from github.com)

No description provided.

ManlyMarco (Migrated from github.com) reviewed

2025-10-30 10:19:45 +00:00

ManlyMarco (Migrated from github.com) left a comment

That's a lot of changes. Functionally it seems correct, just badly in need of some refactoring. Avoid changing privates into internals whenever possible since it promotes spaghetti code that's hard to maintain later.

src/MaterialEditor.Core.Studio/Core.MaterialEditor.SceneController.cs

Why not use the SceneLoad event instead? You can then invoke delayed from the event handler, if it's necessary.

src/MaterialEditor.Core.Studio/Core.MaterialEditor.SceneController.cs

Not a fan of exposing internals specific to scenes, ideally there would be a cleaner way to do it.

src/MaterialEditor.Core.Studio/Core.MaterialEditor.Studio.cs

Since base.ExportTexture is always overriden why not make it abstract and move both logic branches to ExportTextureOriginal? Name it more appropriately then though.

src/MaterialEditor.Core/Core.MaterialEditor.CharaController.cs

Rather than call into studio directly from CharaController maybe the entire texture saving process from both charas and scenes should be extracted to a third class that deals just with that?

src/MaterialEditor.Core/Core.MaterialEditor.cs

					
				@ -252,0 +276,4 @@

				                    ConfigLocalTexturePath.Value = "";

				                return;

				            }

				            if (ConfigLocalTexturePath.Value.Split(Path.GetInvalidPathChars()).Length == 1)

If it's set in the modding api then should it still be a setting in ME? That seems redundant and likely to cause issues. If it's an override then it should be called as such, and probably marked as a debug feature instead.

src/MaterialEditor.Core/Core.MaterialEditor.cs

Auditing and all of the related members should really be put into a separate class. Same with everything related to texture deduping, it should be split as cleanly as possible from the rest of ME code to keep it easy to maintain.

src/Shared.TextureContainer/Shared.TextureContainer.cs

This is naughty. If you need the hash then expose just the hash in a new method or prop, eg GetTextureHash.

copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed

2025-10-30 10:32:31 +00:00

copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull Request Overview

This PR updates package dependencies and implements a new texture storage system for MaterialEditor. The changes introduce three texture save modes (Bundled, Local, and Deduped) to optimize file sizes and texture management, along with a local texture audit tool to identify unused and missing texture files.

Key changes:

Updated ExtensibleSaveFormat packages from 19.3.3 to 21.1.2 across all game variants
Updated IllusionModdingAPI packages from 1.38.0 to 1.44.0 across all projects
Updated Microsoft.Unity.Analyzers from 1.21.0 to 1.25.0
Implemented CRC64 hash calculation for texture deduplication
Added local texture storage functionality with audit capabilities

Reviewed Changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 40 comments.

Show a summary per file

File	Description
Multiple .csproj files	Updated package dependencies to newer versions
Shared.TextureContainerManager.cs	Changed hash from CRC32 (int) to CRC64 (long) and added CRC64Calculator class
Shared.TextureContainer.cs	Changed _token field from private to internal
Core.MaterialEditor.cs	Added texture save type configurations, local texture management, and audit functionality
Core.MaterialEditor.CharaController.cs	Implemented character texture save/load logic with support for multiple save types
Core.MaterialEditor.SceneController.cs	Implemented scene texture save/load logic with deduplication support
Core.MaterialEditor.Studio.cs	Added ExportTexture override to export original texture data
Core.MaterialEditor.Maker.cs	Added ExportTexture override for maker context
UI.cs	Changed ExportTexture to virtual and added ExportTextureOriginal method
PluginBase.cs	Added LocalTexturePath configuration and IsAutoSave helper method
Extensions.cs	Added SubSet extension method for arrays

Comments suppressed due to low confidence (1)

src/MaterialEditor.Core.Studio/Core.MaterialEditor.SceneController.cs:72

The expression 'A == false' can be simplified to '!A'.

                if (isLoading == wasLoading == false && counter == 0)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

## Pull Request Overview This PR updates package dependencies and implements a new texture storage system for MaterialEditor. The changes introduce three texture save modes (Bundled, Local, and Deduped) to optimize file sizes and texture management, along with a local texture audit tool to identify unused and missing texture files. Key changes: - Updated ExtensibleSaveFormat packages from 19.3.3 to 21.1.2 across all game variants - Updated IllusionModdingAPI packages from 1.38.0 to 1.44.0 across all projects - Updated Microsoft.Unity.Analyzers from 1.21.0 to 1.25.0 - Implemented CRC64 hash calculation for texture deduplication - Added local texture storage functionality with audit capabilities ### Reviewed Changes Copilot reviewed 24 out of 24 changed files in this pull request and generated 40 comments. <details> <summary>Show a summary per file</summary> | File | Description | | ---- | ----------- | | Multiple .csproj files | Updated package dependencies to newer versions | | Shared.TextureContainerManager.cs | Changed hash from CRC32 (int) to CRC64 (long) and added CRC64Calculator class | | Shared.TextureContainer.cs | Changed _token field from private to internal | | Core.MaterialEditor.cs | Added texture save type configurations, local texture management, and audit functionality | | Core.MaterialEditor.CharaController.cs | Implemented character texture save/load logic with support for multiple save types | | Core.MaterialEditor.SceneController.cs | Implemented scene texture save/load logic with deduplication support | | Core.MaterialEditor.Studio.cs | Added ExportTexture override to export original texture data | | Core.MaterialEditor.Maker.cs | Added ExportTexture override for maker context | | UI.cs | Changed ExportTexture to virtual and added ExportTextureOriginal method | | PluginBase.cs | Added LocalTexturePath configuration and IsAutoSave helper method | | Extensions.cs | Added SubSet extension method for arrays | </details> <details> <summary>Comments suppressed due to low confidence (1)</summary> **src/MaterialEditor.Core.Studio/Core.MaterialEditor.SceneController.cs:72** * The expression 'A == false' can be simplified to '!A'. ``` if (isLoading == wasLoading == false && counter == 0) ``` </details> --- 💡 <a href="/IllusionMods/KK_Plugins/new/master/.github?filename=*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.

src/MaterialEditor.Base/PluginBase.cs

					
				@ -39,1 +45,4 @@

				        /// </summary>

				        public static string LocalTexturePath = LocalTexturePathDefault;

				        /// <summary>

				        /// Saved material edits

Call to 'System.IO.Path.Combine'.

src/MaterialEditor.Base/UI/UI.cs

Call to 'System.IO.Path.Combine'.

            matName = string.Concat(matName.Split(Path.GetInvalidFileNameChars())).Trim();
            property = string.Concat(property.Split(Path.GetInvalidFileNameChars())).Trim();
            ext = string.Concat(ext.Split(Path.GetInvalidFileNameChars())).Trim();

Call to 'System.IO.Path.Combine'. ```suggestion matName = string.Concat(matName.Split(Path.GetInvalidFileNameChars())).Trim(); property = string.Concat(property.Split(Path.GetInvalidFileNameChars())).Trim(); ext = string.Concat(ext.Split(Path.GetInvalidFileNameChars())).Trim(); ```

src/MaterialEditor.Core.Studio/Core.MaterialEditor.SceneController.cs

The chained equality comparison isLoading == wasLoading == false does not work as intended in C#. This evaluates as (isLoading == wasLoading) == false, which checks if the boolean result of the first comparison equals false. Replace with if (!isLoading && !wasLoading && counter == 0) or if (isLoading == false && wasLoading == false && counter == 0).

                if (!isLoading && !wasLoading && counter == 0)

The chained equality comparison `isLoading == wasLoading == false` does not work as intended in C#. This evaluates as `(isLoading == wasLoading) == false`, which checks if the boolean result of the first comparison equals false. Replace with `if (!isLoading && !wasLoading && counter == 0)` or `if (isLoading == false && wasLoading == false && counter == 0)`. ```suggestion if (!isLoading && !wasLoading && counter == 0) ```

[nitpick] Missing space between 'while' and parenthesis. Should be while (true) for consistency with C# style conventions.

            while (true)

[nitpick] Missing space between 'while' and parenthesis. Should be `while (true)` for consistency with C# style conventions. ```suggestion while (true) ```

src/MaterialEditor.Core.Studio/Core.MaterialEditor.SceneController.cs

Write to static field from instance method, property, or constructor.

src/MaterialEditor.Core/Core.MaterialEditor.CharaController.cs

					
				@ -31,17 +34,21 @@ namespace KK_Plugins.MaterialEditor

				    /// </summary>

				    public class MaterialEditorCharaController : CharaCustomFunctionController

Field 'charaControllers' can be 'readonly'.

        internal static readonly List<MaterialEditorCharaController> charaControllers = new List<MaterialEditorCharaController>();

Field 'charaControllers' can be 'readonly'. ```suggestion internal static readonly List<MaterialEditorCharaController> charaControllers = new List<MaterialEditorCharaController>(); ```

src/MaterialEditor.Core/Core.MaterialEditor.CharaController.cs

Variable sceneController.DedupedTextureData may be null at this access as suggested by this null check.

                        if (sceneController.DedupedTextureData != null)
                            foreach (var x in MessagePackSerializer.Deserialize<Dictionary<int, string>>((byte[])texDicDeduped))
                                importDictionary[x.Key] = SetAndGetTextureID(sceneController.DedupedTextureData[x.Value]);
                        else
                            MaterialEditorPluginBase.Logger.LogMessage("[MaterialEditor] DedupedTextureData is null, cannot import deduped textures!");

Variable [sceneController.DedupedTextureData](1) may be null at this access as suggested by [this](2) null check. ```suggestion if (sceneController.DedupedTextureData != null) foreach (var x in MessagePackSerializer.Deserialize<Dictionary<int, string>>((byte[])texDicDeduped)) importDictionary[x.Key] = SetAndGetTextureID(sceneController.DedupedTextureData[x.Value]); else MaterialEditorPluginBase.Logger.LogMessage("[MaterialEditor] DedupedTextureData is null, cannot import deduped textures!"); ```

src/MaterialEditor.Core/Core.MaterialEditor.cs

Field 'auditFoundHashToFiles' can be 'readonly'.

        internal static readonly Dictionary<string, List<string>> auditFoundHashToFiles = new Dictionary<string, List<string>>();

Field 'auditFoundHashToFiles' can be 'readonly'. ```suggestion internal static readonly Dictionary<string, List<string>> auditFoundHashToFiles = new Dictionary<string, List<string>>(); ```

Field 'auditLock' can be 'readonly'.

        internal static readonly object auditLock = new object();

Field 'auditLock' can be 'readonly'. ```suggestion internal static readonly object auditLock = new object(); ```

src/MaterialEditor.Core/Core.MaterialEditor.cs

					
				@ -252,0 +276,4 @@

				                    ConfigLocalTexturePath.Value = "";

				                return;

				            }

				            if (ConfigLocalTexturePath.Value.Split(Path.GetInvalidPathChars()).Length == 1)

[nitpick] Trailing comma after the last property in the ConfigurationManagerAttributes object initializer is inconsistent with other entries. Remove the comma for consistency.

                IsAdvanced = true

[nitpick] Trailing comma after the last property in the ConfigurationManagerAttributes object initializer is inconsistent with other entries. Remove the comma for consistency. ```suggestion IsAdvanced = true ```

src/MaterialEditor.Core/Core.MaterialEditor.cs

Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.

            LocalTexturePath = ConfigLocalTexturePath.Value == "" ? LocalTexturePathDefault : ConfigLocalTexturePath.Value;

Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better. ```suggestion LocalTexturePath = ConfigLocalTexturePath.Value == "" ? LocalTexturePathDefault : ConfigLocalTexturePath.Value; ```

src/MaterialEditor.Core/Core.MaterialEditor.cs

The hardcoded cutoff date (October 21, 2025) will become outdated quickly. Consider making this configurable, using a relative date (e.g., DateTime.Now.AddMonths(-1)), or explaining why this specific date was chosen in a comment.

            // Use a relative cutoff date: files modified within the last month
            DateTime cutoff = DateTime.Now.AddMonths(-1);

The hardcoded cutoff date (October 21, 2025) will become outdated quickly. Consider making this configurable, using a relative date (e.g., DateTime.Now.AddMonths(-1)), or explaining why this specific date was chosen in a comment. ```suggestion // Use a relative cutoff date: files modified within the last month DateTime cutoff = DateTime.Now.AddMonths(-1); ```

[nitpick] Missing space between 'while' and parenthesis. Should be while (true) for consistency with C# style conventions.

                        while (true)

[nitpick] Missing space between 'while' and parenthesis. Should be `while (true)` for consistency with C# style conventions. ```suggestion while (true) ```

This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.

                foreach (var controllerKey in controllerKeys.Where(x => !hashes.Contains(x.hash)))
                {
                    hashes.Add(controllerKey.hash);
                    dicHashToData.Add(controllerKey.hash.ToString("X16"), controllerKey.data);
                }

This foreach loop [implicitly filters its target sequence](1) - consider filtering the sequence explicitly using '.Where(...)'. ```suggestion foreach (var controllerKey in controllerKeys.Where(x => !hashes.Contains(x.hash))) { hashes.Add(controllerKey.hash); dicHashToData.Add(controllerKey.hash.ToString("X16"), controllerKey.data); } ```

This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.

            foreach (var kvp in auditFoundHashToFiles.Where(kvp => !auditUnusedTextures.Remove(kvp.Key)))
                auditMissingTextures.Add(kvp.Key, kvp.Value);

This foreach loop [implicitly filters its target sequence](1) - consider filtering the sequence explicitly using '.Where(...)'. ```suggestion foreach (var kvp in auditFoundHashToFiles.Where(kvp => !auditUnusedTextures.Remove(kvp.Key))) auditMissingTextures.Add(kvp.Key, kvp.Value); ```

Write to static field from instance method, property, or constructor.

                            ClearAuditMissingTextures();

Write to static field from instance method, property, or constructor. ```suggestion ClearAuditMissingTextures(); ```

Write to static field from instance method, property, or constructor.

                            this.auditUnusedTextures = null;

Write to static field from instance method, property, or constructor. ```suggestion this.auditUnusedTextures = null; ```

Write to static field from instance method, property, or constructor.

Call to 'System.IO.Path.Combine'.

            pngs.AddRange(Directory.GetFiles(Path.Combine(Paths.GameRootPath, "UserData", "chara"), "*.png", SearchOption.AllDirectories));
            pngs.AddRange(Directory.GetFiles(Path.Combine(Paths.GameRootPath, "UserData", "Studio", "scene"), "*.png", SearchOption.AllDirectories));

Call to 'System.IO.Path.Combine'. ```suggestion pngs.AddRange(Directory.GetFiles(Path.Combine(Paths.GameRootPath, "UserData", "chara"), "*.png", SearchOption.AllDirectories)); pngs.AddRange(Directory.GetFiles(Path.Combine(Paths.GameRootPath, "UserData", "Studio", "scene"), "*.png", SearchOption.AllDirectories)); ```

Call to 'System.IO.Path.Combine'.

            pngs.AddRange(Directory.GetFiles(Path.Combine(Paths.GameRootPath, "UserData", "chara"), "*.png", SearchOption.AllDirectories));
            pngs.AddRange(Directory.GetFiles(Path.Combine(Paths.GameRootPath, "UserData", "Studio", "scene"), "*.png", SearchOption.AllDirectories));

Call to 'System.IO.Path.Combine'. ```suggestion pngs.AddRange(Directory.GetFiles(Path.Combine(Paths.GameRootPath, "UserData", "chara"), "*.png", SearchOption.AllDirectories)); pngs.AddRange(Directory.GetFiles(Path.Combine(Paths.GameRootPath, "UserData", "Studio", "scene"), "*.png", SearchOption.AllDirectories)); ```

Call to 'System.IO.Path.Combine'.

                                    Path.Combine(LocalTexturePath, LocalTexUnusedFolder, kvp.Value));

Call to 'System.IO.Path.Combine'. ```suggestion Path.Combine(LocalTexturePath, LocalTexUnusedFolder, kvp.Value)); ```

Poor error handling: empty catch block.

                catch (Exception ex)
                {
                    Logger.LogError($"Error closing ConfigurationManager window: {ex}");
                }

Poor error handling: empty catch block. ```suggestion catch (Exception ex) { Logger.LogError($"Error closing ConfigurationManager window: {ex}"); } ```

Inefficient use of 'ContainsKey' and indexer.
Inefficient use of 'ContainsKey' and indexer.

                                                        if (!auditFoundHashToFiles.TryGetValue(kvp.Value, out var fileList))
                                                            auditFoundHashToFiles.Add(kvp.Value, new List<string> { file });
                                                        else
                                                            lock (fileList)
                                                                fileList.Add(file);

Inefficient use of 'ContainsKey' and [indexer](1). Inefficient use of 'ContainsKey' and [indexer](2). ```suggestion if (!auditFoundHashToFiles.TryGetValue(kvp.Value, out var fileList)) auditFoundHashToFiles.Add(kvp.Value, new List<string> { file }); else lock (fileList) fileList.Add(file); ```

Variable MaterialEditorPlugin.auditUnusedTextures may be null at this access as suggested by this null check.

Variable [MaterialEditorPlugin.auditUnusedTextures](1) may be null at this access as suggested by [this](2) null check.

Variable MaterialEditorPlugin.auditUnusedTextures may be null at this access as suggested by this null check.

Variable [MaterialEditorPlugin.auditUnusedTextures](1) may be null at this access as suggested by [this](2) null check.

These 'if' statements can be combined.

Generic catch clause.

                catch (Exception ex)
                {
                    Logger.LogDebug($"Exception hiding ConfigurationManager window: {ex}");
                }

Generic catch clause. ```suggestion catch (Exception ex) { Logger.LogDebug($"Exception hiding ConfigurationManager window: {ex}"); } ```

Generic catch clause.

                                        catch (MessagePackSerializationException)
                                        {
                                            data.RemoveAt(0);
                                        }
                                        catch (InvalidOperationException)
                                        {
                                            data.RemoveAt(0);
                                        }
                                        catch (Exception ex)
                                        {
                                            // Log unexpected exceptions for debugging
                                            Logger.LogError($"Unexpected exception during MessagePack deserialization: {ex}");
                                            data.RemoveAt(0);
                                        }

Generic catch clause. ```suggestion catch (MessagePackSerializationException) { data.RemoveAt(0); } catch (InvalidOperationException) { data.RemoveAt(0); } catch (Exception ex) { // Log unexpected exceptions for debugging Logger.LogError($"Unexpected exception during MessagePack deserialization: {ex}"); data.RemoveAt(0); } ```

src/Shared.TextureContainer/Shared.TextureContainerManager.cs

				`@ -55,7 +55,8 @@ namespace KK_Plugins`

Casting a long hash to int truncates the upper 32 bits, which may lead to hash collisions. Consider using HashCode.Combine or implementing a proper hash code that incorporates all bits of the long value, such as return (int)(hash ^ (hash >> 32)).

                // Combine upper and lower 32 bits to reduce hash collisions
                return (int)(hash ^ (hash >> 32));

Casting a long hash to int truncates the upper 32 bits, which may lead to hash collisions. Consider using HashCode.Combine or implementing a proper hash code that incorporates all bits of the long value, such as `return (int)(hash ^ (hash >> 32))`. ```suggestion // Combine upper and lower 32 bits to reduce hash collisions return (int)(hash ^ (hash >> 32)); ```

ManlyMarco (Migrated from github.com) reviewed

2025-10-30 10:45:48 +00:00

src/MaterialEditor.Core/Core.MaterialEditor.cs

At least make the tower smaller by inverting them.

ManlyMarco (Migrated from github.com) reviewed

2025-10-30 10:45:50 +00:00

src/MaterialEditor.Core/Core.MaterialEditor.cs

At least make the tower smaller by inverting them.

starstormhun (Migrated from github.com) reviewed

2025-10-30 10:55:23 +00:00

src/MaterialEditor.Core/Core.MaterialEditor.cs

Fair point

starstormhun (Migrated from github.com) reviewed

2025-10-30 10:56:55 +00:00

src/MaterialEditor.Core/Core.MaterialEditor.cs

					
				@ -252,0 +276,4 @@

				                    ConfigLocalTexturePath.Value = "";

				                return;

				            }

				            if (ConfigLocalTexturePath.Value.Split(Path.GetInvalidPathChars()).Length == 1)

It's only for the autosaves, not the normal saves.
...I guess the autosave overrides can be added to KKAPI instead, then.

It's only for the autosaves, not the normal saves. ...I guess the autosave overrides can be added to KKAPI instead, then.

starstormhun (Migrated from github.com) reviewed

2025-10-30 11:05:24 +00:00

src/MaterialEditor.Core.Studio/Core.MaterialEditor.SceneController.cs

Because it was 4AM probably and I didn't wanna test which order Scene / Characontrollers fired their loads... :D
But it seems charas always go first, which is perfect, so I can just clear the variable when scenecontroller is done

Because it was 4AM probably and I didn't wanna test which order Scene / Characontrollers fired their loads... :D But it seems charas always go first, which is perfect, so I can just clear the variable when scenecontroller is done

starstormhun (Migrated from github.com) reviewed

2025-10-30 12:41:24 +00:00

src/MaterialEditor.Core/Core.MaterialEditor.CharaController.cs

Did so

starstormhun (Migrated from github.com) reviewed

2025-10-30 12:47:49 +00:00

src/MaterialEditor.Core.Studio/Core.MaterialEditor.Studio.cs

I'd have to duplicate even more code in that case, this lets the Maker / Studio side figure out the specifics and then the actual export code is in the base class

starstormhun (Migrated from github.com) reviewed

2025-10-30 12:49:25 +00:00

src/MaterialEditor.Core.Studio/Core.MaterialEditor.SceneController.cs

The Maker / Studio UIs need access to it for exporting original files. I could add a Property, but that's about the same.

ManlyMarco (Migrated from github.com) reviewed

2025-10-30 13:13:26 +00:00

src/MaterialEditor.Core/Core.MaterialEditor.cs

					
				@ -252,0 +276,4 @@

				                    ConfigLocalTexturePath.Value = "";

				                return;

				            }

				            if (ConfigLocalTexturePath.Value.Split(Path.GetInvalidPathChars()).Length == 1)

Then it should explicitly state that it's for Autosave

ManlyMarco (Migrated from github.com) approved these changes

2025-11-02 19:21:28 +00:00

copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed

2025-11-06 13:57:25 +00:00

copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull Request Overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated 16 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

## Pull Request Overview Copilot reviewed 26 out of 26 changed files in this pull request and generated 16 comments. --- 💡 <a href="/IllusionMods/KK_Plugins/new/master/.github/instructions?filename=*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.

src/MaterialEditor.Base/UI/UI.cs

Call to 'System.IO.Path.Combine'.

            var safeProperty = string.Concat(property.Split(Path.GetInvalidFileNameChars())).Trim();
            var safeExt = string.Concat(ext.Split(Path.GetInvalidFileNameChars())).Trim();
            string filename = Path.Combine(ExportPath, $"_Export_{DateTime.Now:yyyy-MM-dd-HH-mm-ss}_{matName}_{safeProperty}.{safeExt}");

Call to 'System.IO.Path.Combine'. ```suggestion var safeProperty = string.Concat(property.Split(Path.GetInvalidFileNameChars())).Trim(); var safeExt = string.Concat(ext.Split(Path.GetInvalidFileNameChars())).Trim(); string filename = Path.Combine(ExportPath, $"_Export_{DateTime.Now:yyyy-MM-dd-HH-mm-ss}_{matName}_{safeProperty}.{safeExt}"); ```

src/MaterialEditor.Core.Maker/Core.MaterialEditor.Maker.cs

The magic string "XXX" is used as a sentinel value for unknown image types. Consider defining this as a named constant (e.g., private const string UnknownImageType = \"XXX\";) to improve code clarity and maintainability.

The magic string \"XXX\" is used as a sentinel value for unknown image types. Consider defining this as a named constant (e.g., `private const string UnknownImageType = \"XXX\";`) to improve code clarity and maintainability.

src/MaterialEditor.Core.Studio/Core.MaterialEditor.SceneController.cs

Write to static field from instance method, property, or constructor.

src/MaterialEditor.Core.Studio/Core.MaterialEditor.Studio.cs

The magic string "XXX" is used as a sentinel value for unknown image types. Consider defining this as a named constant (e.g., private const string UnknownImageType = \"XXX\";) to improve code clarity and maintainability.

The magic string \"XXX\" is used as a sentinel value for unknown image types. Consider defining this as a named constant (e.g., `private const string UnknownImageType = \"XXX\";`) to improve code clarity and maintainability.

src/MaterialEditor.Core/Core.MaterialEditor.TextureSaveHandler.cs

					
				@ -0,0 +1,186 @@

				using System.Collections.Generic;

The condition != null is checking if the return value of TryGetValue is not null, but TryGetValue returns a bool, not a nullable type. This will always be true. The condition should be == true or simply omit the comparison: if (MEStudio.GetSceneController().GetExtendedData()?.data.TryGetValue(..., out var dataBytes) == true && dataBytes != null).

                    if (MEStudio.GetSceneController().GetExtendedData()?.data.TryGetValue(DedupedTexSavePrefix + key + DedupedTexSavePostfix, out var dataBytes) && dataBytes != null)

The condition `!= null` is checking if the return value of `TryGetValue` is not null, but `TryGetValue` returns a bool, not a nullable type. This will always be true. The condition should be `== true` or simply omit the comparison: `if (MEStudio.GetSceneController().GetExtendedData()?.data.TryGetValue(..., out var dataBytes) == true && dataBytes != null)`. ```suggestion if (MEStudio.GetSceneController().GetExtendedData()?.data.TryGetValue(DedupedTexSavePrefix + key + DedupedTexSavePostfix, out var dataBytes) && dataBytes != null) ```

This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.

                foreach (var textureContainer in controller.TextureDictionary.Values.Where(tc => !hashes.Contains(tc.Hash)))
                {
                    hashes.Add(textureContainer.Hash);
                    dicHashToData.Add(textureContainer.Hash.ToString("X16"), textureContainer.Data);
                }

This foreach loop [implicitly filters its target sequence](1) - consider filtering the sequence explicitly using '.Where(...)'. ```suggestion foreach (var textureContainer in controller.TextureDictionary.Values.Where(tc => !hashes.Contains(tc.Hash))) { hashes.Add(textureContainer.Hash); dicHashToData.Add(textureContainer.Hash.ToString("X16"), textureContainer.Data); } ```

Variable dict may be null at this access because of this assignment.

            var dict = dictRaw as Dictionary<int, TextureContainer>;
            if (dict == null)
                throw new ArgumentException("dictRaw must be a Dictionary<int, TextureContainer>", nameof(dictRaw));

Variable [dict](1) may be null at this access because of [this](2) assignment. ```suggestion var dict = dictRaw as Dictionary<int, TextureContainer>; if (dict == null) throw new ArgumentException("dictRaw must be a Dictionary<int, TextureContainer>", nameof(dictRaw)); ```

Variable dict may be null at this access because of this assignment.

            var dict = dictRaw as Dictionary<int, TextureContainer>;
            if (dict == null)
                throw new ArgumentNullException(nameof(dictRaw), "dictRaw must be a non-null Dictionary<int, TextureContainer>.");

Variable [dict](1) may be null at this access because of [this](2) assignment. ```suggestion var dict = dictRaw as Dictionary<int, TextureContainer>; if (dict == null) throw new ArgumentNullException(nameof(dictRaw), "dictRaw must be a non-null Dictionary<int, TextureContainer>."); ```

Variable dict may be null at this access because of this assignment.

            var dict = dictRaw as Dictionary<int, TextureContainer>;
            if (dict == null)
                throw new ArgumentException("dictRaw must be a Dictionary<int, TextureContainer>", nameof(dictRaw));

Variable [dict](1) may be null at this access because of [this](2) assignment. ```suggestion var dict = dictRaw as Dictionary<int, TextureContainer>; if (dict == null) throw new ArgumentException("dictRaw must be a Dictionary<int, TextureContainer>", nameof(dictRaw)); ```

Generic catch clause.

            catch (Exception ex)
            {
                MaterialEditorPluginBase.Logger.LogWarning($"Save method failed with exception: {ex.Message}\n{ex.StackTrace}\nFalling back to Bundled saving!");

Generic catch clause. ```suggestion catch (Exception ex) { MaterialEditorPluginBase.Logger.LogWarning($"Save method failed with exception: {ex.Message}\n{ex.StackTrace}\nFalling back to Bundled saving!"); ```

src/MaterialEditor.Core/Core.MaterialEditor.TextureSaveHandler.cs

					
				@ -0,0 +29,4 @@

				            dedupedTexSavePrefix,

				            dedupedTexSavePostfix,

				            localTexUnusedFolder

				        ) { Instance = this; }

Write to static field from instance method, property, or constructor.

src/MaterialEditor.Core/Core.MaterialEditor.TextureSaveHandler.cs

					
				@ -0,0 +145,4 @@

				            foreach (var kvp in hashDict)

				            {

				                string fileName = LocalTexPrefix + kvp.Value + "." + ImageTypeIdentifier.Identify(dict[kvp.Key].Data);

				                string filePath = Path.Combine(LocalTexturePath, fileName);

Call to 'System.IO.Path.Combine'.

src/MaterialEditor.Core/Core.MaterialEditor.cs

					
				@ -249,6 +266,32 @@ namespace KK_Plugins.MaterialEditor

				            LoadNormalMapConverter();

Calling .ToLower() on both strings creates two new string allocations. Consider using StringComparison.OrdinalIgnoreCase with StartsWith for better performance: ConfigLocalTexturePath.Value.StartsWith(Paths.GameRootPath, StringComparison.OrdinalIgnoreCase).

            if (ConfigLocalTexturePath.Value.StartsWith(Paths.GameRootPath, StringComparison.OrdinalIgnoreCase))

Calling `.ToLower()` on both strings creates two new string allocations. Consider using `StringComparison.OrdinalIgnoreCase` with `StartsWith` for better performance: `ConfigLocalTexturePath.Value.StartsWith(Paths.GameRootPath, StringComparison.OrdinalIgnoreCase)`. ```suggestion if (ConfigLocalTexturePath.Value.StartsWith(Paths.GameRootPath, StringComparison.OrdinalIgnoreCase)) ```

src/MaterialEditor.Core/Core.MaterialEditor.cs

Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.

                ConfigLocalTexturePath.Value = ConfigLocalTexturePath.Value.Length > Paths.GameRootPath.Length
                    ? ConfigLocalTexturePath.Value.Substring(Paths.GameRootPath.Length + 1)
                    : "";

Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better. ```suggestion ConfigLocalTexturePath.Value = ConfigLocalTexturePath.Value.Length > Paths.GameRootPath.Length ? ConfigLocalTexturePath.Value.Substring(Paths.GameRootPath.Length + 1) : ""; ```

src/MaterialEditor.Core/Core.MaterialEditor.cs

Call to 'System.IO.Path.Combine'.

src/Shared.TextureContainer/Shared.TextureContainerManager.cs

					
				@ -22,14 +22,14 @@ namespace KK_Plugins

				        internal class TextureKey

The hardcoded values 1 << 11 (2048) and 1 << 9 (512) for the hash calculation parameters lack explanation. Consider adding a comment explaining why these specific values are chosen for the start and end sizes when calculating the texture hash.

                // First part of the data is sufficient to calculate the hash.
                // Use 2048 (1 << 11) as the start size and 512 (1 << 9) as the end size for the hash calculation.
                // These values are chosen to balance performance and uniqueness: hashing a fixed-size prefix of the data
                // is usually enough to distinguish textures, while keeping the operation efficient.

The hardcoded values `1 << 11` (2048) and `1 << 9` (512) for the hash calculation parameters lack explanation. Consider adding a comment explaining why these specific values are chosen for the start and end sizes when calculating the texture hash. ```suggestion // First part of the data is sufficient to calculate the hash. // Use 2048 (1 << 11) as the start size and 512 (1 << 9) as the end size for the hash calculation. // These values are chosen to balance performance and uniqueness: hashing a fixed-size prefix of the data // is usually enough to distinguish textures, while keeping the operation efficient. ```