Add Window Closing Animations Mod - #5187
Conversation
Key features: - Add 7 custom animations (Fire, Shatter, CRT, Glitch, Fold, Iris, Pixelate). - Hook DefWindowProcW, ShowWindow, SetWindowPos, DestroyWindow for universal coverage. - Add ExitProcess hook safeguard to prevent browser cut-offs. - Implement dynamic UWP/sandbox exclusion to fix Snipping Tool/Photos crashes. - Optimize rendering engine with high-precision time-delta (QPC) math.
|
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. The animation effects themselves are nice, but the integration with Windhawk and with the host process has several blocking problems — a detached thread that outlives the mod, a window class that is never unregistered, and synchronous 800 ms message-pump waits inside 1. Substantial overlap with the existing "Windows Animations" mod. mods/windows-animations.wh.cpp already ships closing animations with 2. std::mutex g_threadsMutex;
std::vector<HANDLE> g_animThreads;
std::atomic<bool> g_unloading{false};
// ...
void Wh_ModUninit() {
g_unloading = true; // animation loop checks this and bails early
std::vector<HANDLE> threads;
{ std::lock_guard g(g_threadsMutex); threads.swap(g_animThreads); }
for (HANDLE h : threads) { WaitForSingleObject(h, INFINITE); CloseHandle(h); }
// ... only now GdiplusShutdown / UnregisterClass
}See mods/windows-animations.wh.cpp#L6149-L6164 for the signal-then- 3. The
4. The
The animation must be asynchronous: let the original call proceed immediately and drive the overlay purely from the worker thread, the way mods/genie-minimize-animation.wh.cpp#L315 does (start the thread, return, never block the UI thread). 5. The robust pattern is to re-post the original message with a bypass prop instead of inventing one — see mods/windows-animations.wh.cpp#L5916-L5923 and its 6. Hiding a window is not closing it. 7. GDI leak and use-after-free in DestroyWindow(hOverlay);
DeleteObject(hOverlayBmp); // still selected into hdcMem -> fails, bitmap leaks
DeleteDC(hdcMem); // graphics still wraps this HDC
ReleaseDC(NULL, hdcScreen);
DeleteObject(hCapturedBmp); // `original` still wraps this HBITMAP
// ... `graphics` and `original` destruct here, after their backing objects are gone
Same issue in 8. GDI+ startup/shutdown is racy and can be torn down under a live animation. 9. The 10. The overlay steals focus and flashes uninitialized content. 11. 12. The README has no screenshots or GIFs. This is a purely visual mod with seven distinct effects, and users choose the effect from a dropdown without any way to know what it looks like. Please add a short GIF per effect (only 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. |
Key features:
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.