fix: correct TryKillProcess retry loop condition#145
Open
dchukkapalli-dev wants to merge 1 commit into
Open
Conversation
The loop "for (int i = 0; processes.Length > i || i < 5; i++)" compared the process count to the loop index and, because of the "|| i < 5", always ran at least 5 iterations of Thread.Sleep(250) -- stalling ~1.25s before every patch/restore even when WeMod was not running. Use "processes.Length > 0 && i < 5" so it retries only while a target process is still alive, capped at 5 attempts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the
TryKillProcessretry-loop condition inWandEnhancer/Utils/Common.cs(see #144).The previous condition compared the process count to the loop index, and the
|| i < 5forced at least 5 iterations ofThread.Sleep(250)regardless of state — soTryKillProcessstalled ~1.25s before every patch/restore even when WeMod was not running. The new condition retries only while a target process is still alive, capped at 5 attempts.Changes
WandEnhancer/Utils/Common.cs— corrected retry-loop condition (one line + explanatory comment).Testing
No automated test project exists for the C# solution and I don't have the WeMod runtime; verified by static reasoning about the loop. The change is a one-line condition fix with no API or behaviour change beyond avoiding the needless spin. Please build the solution to confirm compilation per
CONTRIBUTING.md.Fixes #144