Fix MakerCardSave applying patches multiple times in some cases #92

2025-10-23T14:07:52Z

ManlyMarco commented

2025-10-23 14:07:52 +00:00

(Migrated from github.com)

No description provided.

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

2025-10-23 14:09:29 +00:00

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

Pull Request Overview

This PR fixes an issue where MakerCardSave patches were being applied multiple times. The fix refactors the patching logic from using HarmonyTranspiler attributes to explicit patching in the static constructor, ensuring patches are only applied once during class initialization.

Key Changes:

Moved patching logic from attribute-based transpiler to explicit static constructor initialization
Converted FindSaveMethod from a transpiler that patches as a side effect to a pure method that returns the target method
Added error handling around the patching process

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

## Pull Request Overview This PR fixes an issue where `MakerCardSave` patches were being applied multiple times. The fix refactors the patching logic from using HarmonyTranspiler attributes to explicit patching in the static constructor, ensuring patches are only applied once during class initialization. **Key Changes:** - Moved patching logic from attribute-based transpiler to explicit static constructor initialization - Converted `FindSaveMethod` from a transpiler that patches as a side effect to a pure method that returns the target method - Added error handling around the patching process --- <sub>**Tip:** Customize your code reviews with copilot-instructions.md. <a href="/IllusionMods/IllusionModdingAPI/new/master/.github?filename=copilot-instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Create the file</a> or <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>.</sub>

src/Shared.KKalike/Maker/MakerCardSave.cs

The error message should include the exception details. Consider using LogError overload that accepts the exception as a second parameter, or use string interpolation to include the full exception information: KoikatuAPI.Logger.LogError($\"Patching MakerCardSave failed! {e.Message}\", e);

                KoikatuAPI.Logger.LogError($"Patching MakerCardSave failed! {e.Message}", e);

The error message should include the exception details. Consider using LogError overload that accepts the exception as a second parameter, or use string interpolation to include the full exception information: `KoikatuAPI.Logger.LogError($\"Patching MakerCardSave failed! {e.Message}\", e);` ```suggestion KoikatuAPI.Logger.LogError($"Patching MakerCardSave failed! {e.Message}", e); ```

src/Shared.KKalike/Maker/MakerCardSave.cs

					
				@ -70,38 +72,29 @@ namespace KKAPI.Maker

				            il.GotoNext(instruction => instruction.MatchLdftn(out targetMethodReference));

The error message should provide more context about what was expected versus what was found. Consider: throw new ArgumentException($\"Expected MethodReference but found {code.Operand?.GetType().FullName ?? \"null\"}\");

                        throw new ArgumentException($"Expected MethodReference but found {code.Operand?.GetType().FullName ?? "null"}");

The error message should provide more context about what was expected versus what was found. Consider: `throw new ArgumentException($\"Expected MethodReference but found {code.Operand?.GetType().FullName ?? \"null\"}\");` ```suggestion throw new ArgumentException($"Expected MethodReference but found {code.Operand?.GetType().FullName ?? "null"}"); ```

Sign in to join this conversation.

No reviewers