Add TextureSaveHandlerBase #97

2025-10-31T10:28:29Z

starstormhun commented

2025-10-31 10:28:29 +00:00

(Migrated from github.com)

No description provided.

starstormhun commented

2025-11-01 14:42:40 +00:00

(Migrated from github.com)

@ManlyMarco Copilot me senpai

ManlyMarco (Migrated from github.com) reviewed

2025-11-02 02:11:29 +00:00

src/PHAPI/Maker/CharaLocalTextures.cs

					
				@ -56,0 +56,4 @@

				            KoikatuAPI.Instance.Config.Bind("Local Textures", "Audit Local Files", 0, new ConfigDescription("Parse all character / scene files and check for missing or unused local files. Takes a long time if you have many cards and scenes.", null, new ConfigurationManagerAttributes

				            {

				                CustomDrawer = new System.Action<ConfigEntryBase>(TextureSaveHandlerBase.AuditOptionDrawer),

				                Order = 0,

It could also just be an unassigned hotkey, though it would need code in an Update then.

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

2025-11-02 02:12:44 +00:00

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

Pull Request Overview

This PR adds a new TextureSaveHandlerBase class that provides a template for handling texture saving operations and introduces an audit feature for checking missing or unused local texture files. The audit system scans character cards and scenes to identify textures that are referenced but missing, or exist locally but are not used.

Key changes:

Added TextureSaveHandlerBase class with auditing functionality for local texture files
Added SubSet extension method for extracting array segments
Integrated audit UI into the configuration system with custom drawer

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
src/Shared.Core/Utilities/TextureSaveHandlerBase.cs	New base class for texture save handling with multi-threaded audit functionality
src/Shared.Core/Utilities/Extensions.cs	Added SubSet extension method for array segmentation
src/Shared.Core/Shared.Core.projitems	Added TextureSaveHandlerBase to project compilation
src/Shared.Core/Maker/CharaLocalTextures.cs	Integrated audit button and added card save path modifier
src/Shared.Core/KoikatuAPIBase.cs	Added DoOnGUI call for audit UI rendering
src/Shared.CharaStudio/Studio/SceneLocalTextures.cs	Updated config entry Order attribute
src/PHAPI/Maker/CharaLocalTextures.cs	Added audit configuration for PH platform

Comments suppressed due to low confidence (5)

src/Shared.Core/Utilities/TextureSaveHandlerBase.cs:515

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

                                File.Delete(Path.Combine(auditPlugins[auditNowPlugin].LocalTexturePath, kvp.Value));

src/Shared.Core/Utilities/TextureSaveHandlerBase.cs:521

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

                            string unusedFolder = Path.Combine(auditPlugins[auditNowPlugin].LocalTexturePath, auditPlugins[auditNowPlugin].LocalTexUnusedFolder);

src/Shared.Core/Utilities/TextureSaveHandlerBase.cs:526

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

                                    Path.Combine(auditPlugins[auditNowPlugin].LocalTexturePath, kvp.Value),

src/Shared.Core/Utilities/TextureSaveHandlerBase.cs:527

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

                                    Path.Combine(Path.Combine(auditPlugins[auditNowPlugin].LocalTexturePath, auditPlugins[auditNowPlugin].LocalTexUnusedFolder), kvp.Value));

src/Shared.Core/Utilities/TextureSaveHandlerBase.cs:527

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

                                    Path.Combine(Path.Combine(auditPlugins[auditNowPlugin].LocalTexturePath, auditPlugins[auditNowPlugin].LocalTexUnusedFolder), kvp.Value));

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

## Pull Request Overview This PR adds a new `TextureSaveHandlerBase` class that provides a template for handling texture saving operations and introduces an audit feature for checking missing or unused local texture files. The audit system scans character cards and scenes to identify textures that are referenced but missing, or exist locally but are not used. Key changes: - Added `TextureSaveHandlerBase` class with auditing functionality for local texture files - Added `SubSet` extension method for extracting array segments - Integrated audit UI into the configuration system with custom drawer ### Reviewed Changes Copilot reviewed 7 out of 7 changed files in this pull request and generated 13 comments. <details> <summary>Show a summary per file</summary> | File | Description | | ---- | ----------- | | src/Shared.Core/Utilities/TextureSaveHandlerBase.cs | New base class for texture save handling with multi-threaded audit functionality | | src/Shared.Core/Utilities/Extensions.cs | Added SubSet extension method for array segmentation | | src/Shared.Core/Shared.Core.projitems | Added TextureSaveHandlerBase to project compilation | | src/Shared.Core/Maker/CharaLocalTextures.cs | Integrated audit button and added card save path modifier | | src/Shared.Core/KoikatuAPIBase.cs | Added DoOnGUI call for audit UI rendering | | src/Shared.CharaStudio/Studio/SceneLocalTextures.cs | Updated config entry Order attribute | | src/PHAPI/Maker/CharaLocalTextures.cs | Added audit configuration for PH platform | </details> <details> <summary>Comments suppressed due to low confidence (5)</summary> **src/Shared.Core/Utilities/TextureSaveHandlerBase.cs:515** * Call to 'System.IO.Path.Combine'. ``` File.Delete(Path.Combine(auditPlugins[auditNowPlugin].LocalTexturePath, kvp.Value)); ``` **src/Shared.Core/Utilities/TextureSaveHandlerBase.cs:521** * Call to 'System.IO.Path.Combine'. ``` string unusedFolder = Path.Combine(auditPlugins[auditNowPlugin].LocalTexturePath, auditPlugins[auditNowPlugin].LocalTexUnusedFolder); ``` **src/Shared.Core/Utilities/TextureSaveHandlerBase.cs:526** * Call to 'System.IO.Path.Combine'. ``` Path.Combine(auditPlugins[auditNowPlugin].LocalTexturePath, kvp.Value), ``` **src/Shared.Core/Utilities/TextureSaveHandlerBase.cs:527** * Call to 'System.IO.Path.Combine'. ``` Path.Combine(Path.Combine(auditPlugins[auditNowPlugin].LocalTexturePath, auditPlugins[auditNowPlugin].LocalTexUnusedFolder), kvp.Value)); ``` **src/Shared.Core/Utilities/TextureSaveHandlerBase.cs:527** * Call to 'System.IO.Path.Combine'. ``` Path.Combine(Path.Combine(auditPlugins[auditNowPlugin].LocalTexturePath, auditPlugins[auditNowPlugin].LocalTexUnusedFolder), kvp.Value)); ``` </details> --- 💡 <a href="/IllusionMods/IllusionModdingAPI/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/PHAPI/Maker/CharaLocalTextures.cs

					
				@ -51,8 +51,15 @@ namespace KKAPI.Maker

				        static CharaLocalTextures()

Corrected spelling of 'times' to 'time' in the description.

            KoikatuAPI.Instance.Config.Bind("Local Textures", "Audit Local Files", 0, new ConfigDescription("Parse all character / scene files and check for missing or unused local files. Takes a long time if you have many cards and scenes.", null, new ConfigurationManagerAttributes

Corrected spelling of 'times' to 'time' in the description. ```suggestion KoikatuAPI.Instance.Config.Bind("Local Textures", "Audit Local Files", 0, new ConfigDescription("Parse all character / scene files and check for missing or unused local files. Takes a long time if you have many cards and scenes.", null, new ConfigurationManagerAttributes ```

src/Shared.Core/Maker/CharaLocalTextures.cs

					
				@ -56,10 +56,18 @@ namespace KKAPI.Maker {

				        static CharaLocalTextures()

Corrected spelling of 'times' to 'time' in the description.

            KoikatuAPI.Instance.Config.Bind("Local Textures", "Audit Local Files", 0, new ConfigDescription("Parse all character / scene files and check for missing or unused local files. Takes a long time if you have many cards and scenes.", null, new ConfigurationManagerAttributes

Corrected spelling of 'times' to 'time' in the description. ```suggestion KoikatuAPI.Instance.Config.Bind("Local Textures", "Audit Local Files", 0, new ConfigDescription("Parse all character / scene files and check for missing or unused local files. Takes a long time if you have many cards and scenes.", null, new ConfigurationManagerAttributes ```

src/Shared.Core/Utilities/Extensions.cs

					
				@ -434,5 +434,30 @@ namespace KKAPI.Utilities

				            var height = corners[1].y - corners[0].y;

The ArgumentOutOfRangeException constructor should specify which parameter is invalid. Use the overload that accepts a parameter name: throw new ArgumentOutOfRangeException(nameof(size), \"Size should be positive!\") and throw new ArgumentOutOfRangeException(nameof(start), \"Start + size goes beyond array bounds!\").

            if (size < 1) throw new ArgumentOutOfRangeException(nameof(size), "Size should be positive!");
            if (start + size > array.Length) throw new ArgumentOutOfRangeException(nameof(start), "Start + size goes beyond array bounds!");

The `ArgumentOutOfRangeException` constructor should specify which parameter is invalid. Use the overload that accepts a parameter name: `throw new ArgumentOutOfRangeException(nameof(size), \"Size should be positive!\")` and `throw new ArgumentOutOfRangeException(nameof(start), \"Start + size goes beyond array bounds!\")`. ```suggestion if (size < 1) throw new ArgumentOutOfRangeException(nameof(size), "Size should be positive!"); if (start + size > array.Length) throw new ArgumentOutOfRangeException(nameof(start), "Start + size goes beyond array bounds!"); ```

src/Shared.Core/Utilities/TextureSaveHandlerBase.cs

					
				@ -0,0 +1,724 @@

				using BepInEx;

The hardcoded cutoff date (October 21, 2025) appears to be a magic value. Consider making this configurable or using a more meaningful constant with documentation explaining its purpose (e.g., why files before this date should be ignored).

        /// <summary>
        /// Files with a last write time before this date are ignored during audit.
        /// This cutoff is used to exclude legacy or irrelevant files from processing.
        /// Update this value if the audit scope changes.
        /// </summary>
        private static readonly DateTime AuditFileCutoffDate = new DateTime(2025, 10, 21);

        private static void AuditLocalFilesProcessor(List<string> pngs, int period, int offset)
        {
            lock (KoikatuAPI.Logger)
                KoikatuAPI.Logger.LogDebug($"Starting new local file processor with period {period} and offset {offset}!");

            DateTime cutoff = AuditFileCutoffDate;

The hardcoded cutoff date (October 21, 2025) appears to be a magic value. Consider making this configurable or using a more meaningful constant with documentation explaining its purpose (e.g., why files before this date should be ignored). ```suggestion /// <summary> /// Files with a last write time before this date are ignored during audit. /// This cutoff is used to exclude legacy or irrelevant files from processing. /// Update this value if the audit scope changes. /// </summary> private static readonly DateTime AuditFileCutoffDate = new DateTime(2025, 10, 21); private static void AuditLocalFilesProcessor(List<string> pngs, int period, int offset) { lock (KoikatuAPI.Logger) KoikatuAPI.Logger.LogDebug($"Starting new local file processor with period {period} and offset {offset}!"); DateTime cutoff = AuditFileCutoffDate; ```

This clears the entire auditUnusedTextures dictionary instead of just the current plugin's entry. It should be auditUnusedTextures[auditPlugins[auditNowPlugin]].Clear() to match the behavior at line 516.

                            auditUnusedTextures[auditPlugins[auditNowPlugin]].Clear();

This clears the entire `auditUnusedTextures` dictionary instead of just the current plugin's entry. It should be `auditUnusedTextures[auditPlugins[auditNowPlugin]].Clear()` to match the behavior at line 516. ```suggestion auditUnusedTextures[auditPlugins[auditNowPlugin]].Clear(); ```

The destination path has nested Path.Combine calls making it harder to read. Consider computing the unused folder path once (similar to line 521) and reusing it: store Path.Combine(auditPlugins[auditNowPlugin].LocalTexturePath, auditPlugins[auditNowPlugin].LocalTexUnusedFolder) in the unusedFolder variable and use it here.

                                    Path.Combine(unusedFolder, kvp.Value));

The destination path has nested `Path.Combine` calls making it harder to read. Consider computing the unused folder path once (similar to line 521) and reusing it: store `Path.Combine(auditPlugins[auditNowPlugin].LocalTexturePath, auditPlugins[auditNowPlugin].LocalTexUnusedFolder)` in the `unusedFolder` variable and use it here. ```suggestion Path.Combine(unusedFolder, kvp.Value)); ```

Poor error handling: empty catch block.

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

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

These 'if' statements can be combined.

                if (file != null && File.Exists(file) && File.GetLastWriteTime(file) > cutoff && new FileInfo(file).Length <= int.MaxValue)
                {
                    byte[] fileData = File.ReadAllBytes(file);
                    foreach (var plugin in auditPlugins)
                        ScourData(file, fileData, plugin);

These 'if' statements can be combined. ```suggestion if (file != null && File.Exists(file) && File.GetLastWriteTime(file) > cutoff && new FileInfo(file).Length <= int.MaxValue) { byte[] fileData = File.ReadAllBytes(file); foreach (var plugin in auditPlugins) ScourData(file, fileData, plugin); ```

Generic catch clause.

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

Generic catch clause. ```suggestion catch (Exception ex) { KoikatuAPI.Logger.LogError($"Error closing ConfigurationManager window: {ex}"); } ```

Generic catch clause.

                catch (MessagePackSerializationException ex)
                {
                    Debug.LogWarning($"Failed to deserialize hashDict in ScourData: {ex}");
                    break;
                }
                catch (ArgumentException ex)
                {
                    Debug.LogWarning($"ArgumentException in ScourData: {ex}");
                    break;
                }
                catch (Exception ex)
                {
                    Debug.LogError($"Unexpected exception in ScourData: {ex}");
                    throw;
                }

Generic catch clause. ```suggestion catch (MessagePackSerializationException ex) { Debug.LogWarning($"Failed to deserialize hashDict in ScourData: {ex}"); break; } catch (ArgumentException ex) { Debug.LogWarning($"ArgumentException in ScourData: {ex}"); break; } catch (Exception ex) { Debug.LogError($"Unexpected exception in ScourData: {ex}"); throw; } ```

src/Shared.Core/Utilities/TextureSaveHandlerBase.cs

					
				@ -0,0 +237,4 @@

				            var pngs = new List<string>();

				            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'.

            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", "Studio", "scene"), "*.png", SearchOption.AllDirectories)); ```

src/Shared.Core/Utilities/TextureSaveHandlerBase.cs

					
				@ -0,0 +376,4 @@

				            foreach (var plugin in auditPlugins)

				                foreach (var kvp in auditFoundHashToFiles[plugin])

				                    if (!auditUnusedTextures[plugin].Remove(kvp.Key))

				                        auditMissingTextures[plugin].Add(kvp.Key, kvp.Value);

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

                foreach (var kvp in auditFoundHashToFiles[plugin].Where(kvp => !auditUnusedTextures[plugin].Remove(kvp.Key)))
                    auditMissingTextures[plugin].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[plugin].Where(kvp => !auditUnusedTextures[plugin].Remove(kvp.Key))) auditMissingTextures[plugin].Add(kvp.Key, kvp.Value); ```

starstormhun (Migrated from github.com) reviewed

2025-11-02 03:26:31 +00:00

src/PHAPI/Maker/CharaLocalTextures.cs

					
				@ -56,0 +56,4 @@

				            KoikatuAPI.Instance.Config.Bind("Local Textures", "Audit Local Files", 0, new ConfigDescription("Parse all character / scene files and check for missing or unused local files. Takes a long time if you have many cards and scenes.", null, new ConfigurationManagerAttributes

				            {

				                CustomDrawer = new System.Action<ConfigEntryBase>(TextureSaveHandlerBase.AuditOptionDrawer),

				                Order = 0,

Nah that's stupid, then you'd need to bind it, use it, then unbind it. No one's gonna actually want to keep around a hotkey for this.

ManlyMarco (Migrated from github.com) reviewed

2025-11-02 10:32:02 +00:00

src/PHAPI/Maker/CharaLocalTextures.cs

					
				@ -56,0 +56,4 @@

				            KoikatuAPI.Instance.Config.Bind("Local Textures", "Audit Local Files", 0, new ConfigDescription("Parse all character / scene files and check for missing or unused local files. Takes a long time if you have many cards and scenes.", null, new ConfigurationManagerAttributes

				            {

				                CustomDrawer = new System.Action<ConfigEntryBase>(TextureSaveHandlerBase.AuditOptionDrawer),

				                Order = 0,

How about having a button for this in the texture storage type selection box itself? That would be way easier to discover and use by users.

ManlyMarco (Migrated from github.com) approved these changes

2025-11-02 11:51:09 +00:00

Sign in to join this conversation.

No reviewers

ManlyMarco

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

IllusionMods/IllusionModdingAPI!97

				`@ -51,8 +51,15 @@ namespace KKAPI.Maker`
				`static CharaLocalTextures()`

				`@ -56,10 +56,18 @@ namespace KKAPI.Maker {`
				`static CharaLocalTextures()`

				`@ -434,5 +434,30 @@ namespace KKAPI.Utilities`
				`var height = corners[1].y - corners[0].y;`

				`@ -0,0 +1,724 @@`
				`using BepInEx;`

Rows
Columns