fix: collapse the triple auto-switch re-apply in the Options OK handler - #7
Merged
Conversation
Pressing OK could call CheckAndApplyAutoSwitch() three times: once in the auto-switch settings block, once in the base-mouse-count block, and once in the forced re-apply after the cached device state is discarded. Each call enumerates raw input devices, retrying GetRawInputDeviceList up to three times, so a single click cost up to three enumerations. The repeats were harmless — the direction is persisted before all three calls and ApplyMouseOrientation() sets an absolute value rather than toggling — but only the last call could do useful work in the common case, since the first two early-return on unchanged state. Remove the two earlier calls and keep the forced re-apply at the end of the handler. By that point every setting the check reads is persisted, so one pass applies them all; the first call previously ran before SetBaseMouseCount(), so it could not see a changed base count anyway. Dropping the base-mouse-count call also stops a re-apply from running when the auto-switch flag itself failed to persist: that call was gated on autoSwitchEnabled alone, not on autoSwitchWritten. The surviving call is gated on both, matching 3a00a42. Closes #2 Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Jonathan Bnayahu <bnayahu@il.ibm.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.
Closes #2
Problem
OptionsDialogProc'sIDOKhandler could callCheckAndApplyAutoSwitch()three times per click — in the auto-switch settings block, in the base-mouse-count block, and in the forced re-apply after the cached device state is discarded. Each call enumerates raw input devices with up to threeGetRawInputDeviceListretries.Why it was safe, and why it is still safe
ApplyMouseOrientation()passes an absolute value toSwapMouseButton()rather than toggling, so repeated application is idempotent.Change
Remove the two earlier calls; keep the forced re-apply at the end of the handler, still gated on
autoSwitchEnabled && autoSwitchWrittenand still preceded by theg_lastExternalMouseState = EXTERNAL_MOUSE_UNKNOWNreset that makes it effective.The surviving call is strictly the better one: by the end of the handler every setting it reads — direction, auto-switch flag, base device count — is persisted. The first call ran before
SetBaseMouseCount(), so it could never see a changed base count.Dropping the base-mouse-count call also closes a small inconsistency: it was gated on
autoSwitchEnabledalone, so it could re-apply even when the auto-switch flag failed to persist and monitoring was never started. The surviving call is gated on both, matching 3a00a42.Verification
./build.shsucceeds, no compiler warnings.Assisted-By: Claude (Anthropic AI) noreply@anthropic.com