Update the mod to the version 1.1.0 - #5029
Conversation
Updated version to 1.1.0 and added support for Windows 11 23H2 and 24H2. Enhanced keyboard shortcuts for auto-advancing selection.
|
Thanks for the pull request! This repository uses a two-stage review: an AI review that you run yourself, followed by a human review. To get started, comment See the pull request review process for the full details. |
|
/ai-review |
Submission reviewNote: This review was done by Claude. Due to the amount of submissions, doing a fully manual review for each pull request is no longer feasible. Thank you for understanding. Remember: The AI reviewer can be wrong - it may misread code, flag correct code as broken, or suggest changes that make things worse. Treat its findings as suggestions to verify, not instructions to follow blindly. You're responsible for the code you submit, so if a finding doesn't hold up, say so instead of changing working code to satisfy it. Please address the following issues. The items in the collapsed sections are optional, so it's your call whether to address them. Three things need attention: the new 1. This is a tool mod using the wiki launcher boilerplate verbatim, and there is no branch anywhere on the current executable name. So in the real
The wiki is explicit that the target is changed from If you do need code running inside the real explorer.exe, do it the way simple-window-switcher does: keep both includes, branch on the exe name, and do the explorer-specific work there instead of letting explorer become a second launcher. 2.
But the bigger problem is the behavior change behind it. Line 2907 now arms the 250 ms auto-cycle timer for every Win+Tab activation: const bool wantsAutoCycle = triggerModifier == TriggerModifier::Win;The old default was deliberately off, with the reason spelled out in the description you removed: "Off by default so Tab lets you step to an exact window." Since releasing Win confirms the selection, an unconditional 250 ms advance while Win is held makes it impossible to land on a specific window — the user now has to release Win at the right instant. That's a regression for anyone using the mod the old way. Simplest fix that solves both: keep the setting wired up and just flip its default. - autoCycle: true
$name: Auto-cycle while Win is held
$description: Automatically advance the selection every 250 ms while Win is held, in addition to Tab/Shift+Tab.const bool wantsAutoCycle = g_settings.autoCycle && triggerModifier == TriggerModifier::Win;3. The new Tab-hold auto-advance is unreachable, and duplicates behavior that already worked in 1.0.0
Holding Tab also already auto-advanced before this PR: hardware key auto-repeat keeps firing the LL hook, which posts Please drop the timer (constants, the Optional improvements
Minor polish — none of this affects users, so it's your call.
Functionality notes
Non-critical observations and ideas about the feature behavior itself.
Next steps:
See the review process for details. |
Updated mod description and settings for Aero Flip 3D. Changed auto-cycle setting to be enabled by default and clarified keyboard shortcuts.
Added a variable to capture the return value of WhTool_ModInit in the initialization process.
|
/ai-review |
Submission reviewNote: This review was done by Claude. Due to the amount of submissions, doing a fully manual review for each pull request is no longer feasible. Thank you for understanding. Remember: The AI reviewer can be wrong - it may misread code, flag correct code as broken, or suggest changes that make things worse. Treat its findings as suggestions to verify, not instructions to follow blindly. You're responsible for the code you submit, so if a finding doesn't hold up, say so instead of changing working code to satisfy it. Please address the following issues. The items in the collapsed sections are optional, so it's your call whether to address them. The dominant change here is moving the mod out of its dedicated tool process and into 1. Moving from This mod is a textbook tool mod, and it was correctly merged as one. It installs no function hooks at all — everything it does works from any process: The blast radius is not theoretical for this particular mod. Hosted in the shell it adds to The justification in the header comment ("the low-level input hook and the shell's Task View/Win+Tab registration are initialized in the same shell lifetime and startup order") is a hypothesis, not a diagnosis. Low-level keyboard hooks are process-agnostic — several merged tool mods install one from the Please pin down what actually failed on those Windows 11 23H2/24H2 machines before changing the hosting model. The mod already logs every relevant failure — which one fired?
Each of those has a targeted fix that keeps the mod out of the shell. If after that it genuinely turns out the feature cannot work from a tool process, that's worth raising with the maintainer explicitly rather than landing ~3,600 lines of GDI/DWM/animation code in Everything below applies if the Explorer hosting stays. 2.
WaitForSingleObject(g_hookReadyEvent.get(), INFINITE);
return g_hookInstallOk.load(std::memory_order_acquire);For a process loaded before it starts executing, 3. The worker threads outlive a failed When WaitForSingleObject(g_hookReadyEvent.get(), INFINITE);
if (!g_hookInstallOk.load(std::memory_order_acquire)) {
WhTool_ModUninitImpl(); // join both threads before we're unloaded
return false;
}
return true;Related, same category: 4. HWND taskbar = FindWindowW(L"Shell_TrayWnd", nullptr);
if (!taskbar) {
// The initial shell can be injected before it creates the taskbar.
return true;
}Any 5.
Optional improvements
Minor polish — none of this affects users, so it's your call.
Functionality notes
Non-critical observations and ideas about the feature behavior itself.
Next steps:
See the review process for details. |
Refactor comments and improve clarity on the mod's lifecycle and error handling. Adjusted the single-instance mutex logic and reduced the number of attempts for certain operations.
|
/ai-review |
Submission reviewNote: This review was done by Claude. Due to the amount of submissions, doing a fully manual review for each pull request is no longer feasible. Thank you for understanding. Remember: The AI reviewer can be wrong - it may misread code, flag correct code as broken, or suggest changes that make things worse. Treat its findings as suggestions to verify, not instructions to follow blindly. You're responsible for the code you submit, so if a finding doesn't hold up, say so instead of changing working code to satisfy it. Please address the following issues. The items in the collapsed sections are optional, so it's your call whether to address them. The bulk of this PR is a single architectural change — moving ~3800 lines of GDI/DWM/threading code out of the dedicated tool process and into 1. Moving from the tool-mod process into The mod installs zero function hooks — there is no The ~90-line justification block at lines 95–178 asserts several things that aren't accurate:
Please restore the tool-mod structure ( Also please drop or rewrite the 95–178 comment block. Lines 122–126 additionally claim " 2. The single-instance mutex picks an arbitrary
The tool-mod launcher doesn't have this problem: exactly one dedicated process is started and the framework manages it. This is another argument for going back to it rather than for hardening the mutex. 3. Per Mod lifetime, when a mod is loaded before the target process begins executing, The unload path is worse. Notably, the mod's own 1.0.0 comment said exactly this and it was deleted rather than acted on:
That reasoning was correct. In the shell host it no longer applies. Two things regardless of where the mod ends up hosted:
4. The mutex is leaked on the In if (!ModInitImpl()) {
g_isExplorerProcess = false;
g_singleInstanceMutex.reset();
return FALSE;
}
return TRUE;5. The rewritten fallback (lines 2742–2759) logs and returns when Optional improvements
Minor polish — none of this affects users, so it's your call.
Functionality notes
Non-critical observations and ideas about the feature behavior itself.
Next steps:
See the review process for details. |
While I do agree that some statements are sensationalistic, I've personally tried the mod on Windows 11 24H2 using @include windhawk.exe and it did not work and it works when replacing windhawk.exe with explorer.exe. I could try making the code more defensive in that case but proposing to make the mod non-functional seems like a suggestion to cause a regression in the code given that there were multiple reports related to this problem. The AI reviewer does not have a Windows testing environment therefore it is trying to undo an update that could make the mod more useful and functional. These are the logs: |
|
/ready-for-reviewer |
What's the regression? We investigated some issue related to the Simple Window Switcher mod and thumbnail previews, and discovered that the API only works with a 64-bit process, while windhawk.exe is 32-bit. Is the regression related to thumbnail previews? |
The regression is that the mod does not even start at all if using @include windhawk.exe but it does when using @include explorer.exe so I think that I'm forced to use this approach unless there is an analog functional and documented one that I'm missing |
|
Could it be related to the Windhawk window running as administrator?
You can test this with the Disable Windows Shortcuts mod - if you disable the shortcuts there, does the mod work with windhawk.exe as the target? Simple Window Switcher bypasses this issue by targeting both explorer.exe (to prevent hotkey registration) and windhawk.exe (for the rest).
Who should I ask? It's your mod, you should know how it works. |
I will try the Disable Windows Shotcuts and let you know and related to the don't ask me how comment it was simply an affirmation related to the fact that changing the include made it work that's why. I will reach you out once I test it so that I can fix it properly by complying to the Windhawk approach but making it work correctly |
|
I'd like to understand why it doesn't work. I'll try the version from the store on my computer soon. |
OK, I understand. If possible, let me know the results once you test it |
|
I tried both versions - before and after this PR. In both cases, it works for me except when the foreground program is running as administrator (for example, Windhawk or Task Manager). |
Given this situation, I'd prefer to stall this PR (not close it) so that I can focus on fixing this problem more carefully since on my side it does not work to avoid releasing something that is not stable enough. Thanks for testing it |
|
I noticed that you're specifying: In Windhawk v1.x, windhawk.exe is 32-bit, that's why the mod only works with Windhawk 2.0. Remove |
Removed architecture specification from metadata.
|
/ai-review |
Submission reviewNote: This review was done by Claude. Due to the amount of submissions, doing a fully manual review for each pull request is no longer feasible. Thank you for understanding. Remember: The AI reviewer can be wrong - it may misread code, flag correct code as broken, or suggest changes that make things worse. Treat its findings as suggestions to verify, not instructions to follow blindly. You're responsible for the code you submit, so if a finding doesn't hold up, say so instead of changing working code to satisfy it. Please address the following issues. The items in the collapsed sections are optional, so it's your call whether to address them. The bulk of this update is a revert of the tool-mod conversion that was required before 1.0.0 was merged, and the technical justification written into the file doesn't hold up. That's the main item; the rest are the lifecycle consequences plus a 1. Moving back to The mod still installs zero function hooks. The 90-line "WHY EXPLORER.EXE IS FUNDAMENTAL" block (
Meanwhile the move has real costs: a fault anywhere in ~3800 lines of GDI/DWM/animation code now restarts the shell; If …with the 2. The single-instance mutex picks an arbitrary
Going back to a tool mod removes all of this — the launcher's 3. The comment at DWORD wait = WaitForSingleObject(g_hookReadyEvent.get(), kHookInitTimeoutMs);
if (wait != WAIT_OBJECT_0) {
...
ModUninitImpl(); // <- WaitForSingleObject(g_hookThread, INFINITE)
return false;
}The only reason the handshake times out is that the hook thread is stuck — and that's precisely the case where the join never returns. So the 5 s timeout converts "shell blocked forever with no log line" into "shell blocked forever with a log line". Joining is the right thing to do (the thread must not outlive the image), which is another reason this work belongs in a process where an indefinite wait is harmless. 4. This is item 1 of the last review, unchanged ( if (vk == VK_TAB && keyDown && winDown && !altDown && !ctrlDown && !winTabOwnedElsewhere) {
if (!active) { ... return 1; }
return 1; // <- swallowed before the `if (active)` Tab handler at :1568
}
The new auto-cycle guard then makes it worse ( bool tabDown = (GetAsyncKeyState(VK_TAB) & 0x8000) != 0;
if (!tabDown) {
NavigateSelection(1); // "let the LL hook drive" — but the LL hook can't, see above
}The comment says the LL hook drives while Gate the branch on the switcher being closed and let an active session fall through to the handler that already exists: if (!active && vk == VK_TAB && keyDown && winDown && !altDown && !ctrlDown &&
!winTabOwnedElsewhere) {
PostToController(WM_FLIP3D_ACTIVATE,
static_cast<WPARAM>(TriggerModifier::Win),
static_cast<LPARAM>(shiftDown ? -1 : 1));
return 1;
}Once 5. The system-wide key swallow is still armed before the blocking cross-process work (carried over).
Optional improvements
Minor polish — none of this affects users, so it's your call.
Functionality notes
Non-critical observations and ideas about the feature behavior itself.
Next steps:
See the review process for details. |
Updated mod to include windhawk.exe instead of explorer.exe, refined settings for auto-cycle, and improved documentation.
|
/ai-review |
Submission reviewNote: This review was done by Claude. Due to the amount of submissions, doing a fully manual review for each pull request is no longer feasible. Thank you for understanding. Remember: The AI reviewer can be wrong - it may misread code, flag correct code as broken, or suggest changes that make things worse. Treat its findings as suggestions to verify, not instructions to follow blindly. You're responsible for the code you submit, so if a finding doesn't hold up, say so instead of changing working code to satisfy it. Please address the following issues. The items in the collapsed sections are optional, so it's your call whether to address them. The main thing here is that the pull request doesn't contain the changes it describes — the mod body is byte-identical to the merged 1.0.0. Items 2-5 are the ones you deferred at the end of the previous review; they're all still present in the code as it stands, so they're repeated here for the update. 1. The version bump carries no code change. The full diff against -// @version 1.0.0
+// @version 1.1.0
-// @architecture x86-64
+Everything after the metadata block is identical to the merged 1.0.0 — I diffed the head revision against A version bump publishes an update to every user who has the mod installed, so it should carry a real change. Either push the commits that were meant to be part of this, or drop the bump. (The 2.
if (vk == VK_TAB && keyDown && winDown && !altDown && !ctrlDown && !winTabOwnedElsewhere) {
if (!active) {
...
PostToController(WM_FLIP3D_ACTIVATE, ...);
return 1;
}
return 1; // <- the Tab that should advance the deck is dropped here
}
Gate the branch on the switcher being closed and let an active session fall through to the handler that already exists: if (!active && vk == VK_TAB && keyDown && winDown && !altDown && !ctrlDown &&
!winTabOwnedElsewhere) {
PostToController(WM_FLIP3D_ACTIVATE,
static_cast<WPARAM>(TriggerModifier::Win),
static_cast<LPARAM>(shiftDown ? -1 : 1));
return 1;
}3. A The hook swallows the Arm the suppression at the moment you swallow the g_suppressReleaseModifier.store(static_cast<int>(TriggerModifier::Win),
std::memory_order_release);
g_suppressNextModifierRelease.store(true, std::memory_order_release);
PostToController(WM_FLIP3D_ACTIVATE, ...);The successful path already clears it again at 4. The system-wide key swallow is armed before the UI thread does blocking cross-process work.
Two cheap mitigations, ideally both: set 5. On a multi-monitor layout the backdrop can capture the wrong monitor. Sizing the bitmap from Shift the DC before printing (and restore it before the RECT windowRect = {};
GetWindowRect(desktopHwnd, &windowRect);
SetViewportOrgEx(memDc.get(), windowRect.left - desktopRect.left,
windowRect.top - desktopRect.top, nullptr);
BOOL ok = PrintWindow(desktopHwnd, memDc.get(), PW_RENDERFULLCONTENT);
SetViewportOrgEx(memDc.get(), 0, 0, nullptr);Optional improvements
Minor polish — none of this affects users, so it's your call.
Functionality notes
Non-critical observations and ideas about the feature behavior itself.
Next steps:
See the review process for details. |
|
Given the situation, I think that these problems could be addressed in future as the main problem seems to be solved |


The mod has been updated version to 1.1.0 with a minor change
Changelog
If this pull request updates an existing mod, describe the changes below:
Mod authorship
If this pull request introduces a new mod, please complete the section below.
This mod was created by:
Please select the options that best apply. Your selection does not affect the acceptance criteria, but it helps reviewers understand the context of the code and provide relevant feedback.