Skip to content

Toggle a game's profile from a hotkey (#143) - #14

Merged
SwatX18 merged 1 commit into
masterfrom
work/toggle-hotkey
Aug 27, 2026
Merged

Toggle a game's profile from a hotkey (#143)#14
SwatX18 merged 1 commit into
masterfrom
work/toggle-hotkey

Conversation

@SwatX18

@SwatX18 SwatX18 commented Aug 27, 2026

Copy link
Copy Markdown
Owner

Upstream issue #143 is a title and nothing else — "[Suggestion] Add reset/toggle keybind", empty body, no comments. So the product shape was decided rather than transcribed, and every choice below is an assumption a maintainer may want to argue with. They're listed explicitly at the end.

What it does

Press the key while a configured game holds the foreground → that display flips between the game's level and your Windows level. Press it anywhere else → nothing happens at all: no writes, no state change.

The choice then wins over the automatic behaviour for that game until pressed again or the app restarts — so alt-tabbing back into the game does not quietly undo it.

RegisterHotKey, never a low-level keyboard hook

The README names CS:GO. A program installing a system-wide keyboard hook while someone plays a competitive shooter is the exact shape anti-cheat heuristics look for, so this is a product constraint, not an implementation detail. Nothing here is loaded into, injected into, or hooked onto another process — the OS posts WM_HOTKEY to our own thread's queue. grep for SetWindowsHookEx returns zero hits repo-wide; the property is structural.

The cost is real and stated rather than chased. A game can suppress the key by registering raw input with RIDEV_NOHOTKEYS, or by installing its own low-level hook after ours. Exclusive fullscreen alone does not cause this. The only mechanism that could reach those cases is the one the constraint rules out — a low-level hook is strictly worse here, not a trade: it loses the same install-ordering race, and UIPI additionally blocks a non-elevated hook from seeing input bound for an elevated foreground process.

The far more common cause of a dead hotkey — another program already owning the combination — is reported inline in Settings, synchronously, at the moment it's set.

The state

A set of suppressed profile names, deliberately not a set of enabled ones, so an empty set means today's behaviour and none of the existing checks change meaning. Keyed by name rather than by the ApplicationSetting object, because editing a profile removes and re-adds a new one.

Nothing persists it. The helper contains no I/O at all, so there is no read path to review — a game silently left off cannot survive a restart and be discovered weeks later.

Two things had to change underneath

Restoring a single display carries no guard against the Windows level being unknown. That guard lives one level up, and every pre-existing caller reaches the single-display path through it. Calling it directly — the natural implementation — would write 0 to a display during startup, which is the defect fixed in #11 arriving through a new door, and the existing check wouldn't catch it because it drives the wrapper. The check now sits in the decision function itself.

AMD's SetSaturationOnDisplay returned void, so "only report success when the write landed" was unbuildable there. It now returns whether a display actually matched and every call succeeded — which also distinguishes "the name matched nothing, so nothing was called" from a write that worked. Those were previously indistinguishable. The underlying ADL call always returned a status; it was being discarded. A latent null-reference on the same line (the delegate resolves lazily and can be null) is guarded in passing.

AMD's existing restore loop still ignores the new bool, so the checks pinning its behaviour keep their meaning.

Testing

53 checks behind a fake registrar, a fake foreground reader, and the existing fake devices. Suite total 328. No test registers a real hotkey or touches a real display, and the fixture must never grow a hardware variant.

Every check was proven by breaking the line it guards. Two findings from that process are worth recording:

Two checks were rewritten after review because they exercised a copy of the logic rather than the logic. A fixture-local helper mirrored the real gate verbatim. QA then mutated the real production line — and the entire 322-check suite still passed, because nothing anywhere reflected into that class. The expression is now hoisted into a method both the app and the tests call, so the copy is gone.

Two checks that look redundant are not. Moving the suppression gate to after the screen assignment leaves the "zero device calls" check passing and only fails the "screen untouched" check. Both halves reproduced independently.

The B2 fix also broke three pre-existing checks that had been implicitly relying on the old unconditional behaviour. They were repaired with explicit setup rather than adjusted assertions.

Assumptions a maintainer may disagree with

  • Toggles the foreground game's profile, not a global pause. Both readings fit "reset/toggle"; this one matches how every other setting in the app is scoped.
  • State does not persist across restarts. PR #153 persists its equivalent. Scoped per-game, a forgotten "off" means one title silently never gets vibrance again — easier to miss than a global switch, which is why we went the other way.
  • No default binding. Unset out of the box, opt-in — a hotkey the user never chose reacting to their keystrokes is a support ticket waiting to happen.
  • The toggle writes vibrance only, while the suppression gate covers the whole profile. Gamma restore cannot be scoped to a single display, so including it would conflict with "touches only that display". Gamma and resolution unwind on the next alt-tab out, as they do today — a user-visible asymmetry nobody would guess.
  • A no-op press is silent — no balloon, no sound. It is logged once per distinct process name so "why didn't my hotkey work" is answerable from the log.
  • Suppression is keyed by profile, not display, so two games sharing a profile suppress together.

Known gaps

  • Nothing in the UI shows which profiles are currently toggled off. Once the balloon fades, the only way to find out is to press the key and watch. Worth an ON/OFF column or a styled list item; not built here.
  • The no-op press log grows unbounded across distinct foreground process names. Bounded in practice by how many programs a user alt-tabs to.
  • The .NET 4.0 constraint is verified by inspection and API scan, not by compilation — this machine has no v4.0 reference assemblies, so builds pass -p:TargetFrameworkVersion=v4.8. The csproj still declares v4.0 and was not edited.
  • Nobody has pressed this key with a real game running. Every check drives fakes; the mechanism is proven from source and by fixture, not in the wild.

Relationship to PR juv#153

harisonw's #153 implements an adjacent feature and its mechanism choice was rightRegisterHotKey over a hook — which this adopts and credits. Its product decisions differ (it persists toggle state; it matches profiles on name alone, so a profile matched by install directory would be reachable by the automatic path and invisible to the hotkey), and it bundles a .NET 4.8 framework upgrade across 15 files, which this deliberately does not.

Reviewing it also corrected a claim that would otherwise have appeared here: an earlier draft asserted that clicking the tray icon recreates the form's handle and orphans juv#153's registration. That is falseShowInTaskbar is already true and its setter no-ops, measured. The handle is still cached at registration time in this implementation because that is unconditionally more correct, but as a defensive choice, not a fix for a defect anyone has.

The issue is a title and nothing else - "[Suggestion] Add reset/toggle
keybind" - so the shape was decided rather than transcribed, and every
choice below is an assumption a maintainer may want to argue with.

Press the key while a configured game holds the foreground and that
display flips between the game's level and the Windows level. Press it
anywhere else and nothing happens at all. The choice then wins over the
automatic behaviour for that game until pressed again or the app
restarts, so alt-tabbing back in does not quietly undo it.

RegisterHotKey, never a low-level keyboard hook. The readme names CS:GO,
and a program installing a system-wide keyboard hook while someone plays
a competitive shooter is the exact shape anti-cheat heuristics look for.
Nothing here is loaded into, injected into or hooked onto another
process. The cost is real and documented: a game can suppress the key by
registering raw input against hotkeys, or by installing its own hook
after ours. The only mechanism that could reach those cases is the one
ruled out, so they are stated rather than chased. The far more common
cause of a dead hotkey - another program already owning the combination -
is reported inline the moment it is set, where the old behaviour was to
store the failure in a boolean and tell nobody.

The state is a set of suppressed profile names, deliberately not a set of
enabled ones, so an empty set means today's behaviour and none of the
existing checks change meaning. It is keyed by name rather than by the
setting object, because editing a profile replaces that object. Nothing
persists it: the helper has no I/O at all, so a game silently left off
cannot survive a restart and be discovered weeks later.

Toggling off restores one display through the single-display path, not
the one that walks the whole work list and the primary as well. On AMD
with the default configuration that path has to widen, because there the
automatic apply writes every display - restoring one would leave the
others saturated while the notification claimed otherwise.

Two things had to change underneath. Restoring one display carries no
guard against the Windows level being unknown - that lives a level up -
so reaching it directly would have written zero to a display during
startup, which is the defect fixed three changes ago arriving through a
new door. The check now sits in the decision itself. And AMD's set-
saturation call returned void, so there was no way to honour "only report
success when the write landed": it now returns whether a display actually
matched and every call succeeded, which also distinguishes a name that
matched nothing from a write that worked.

Fifty-three checks behind a fake registrar, a fake foreground reader and
the existing fake devices. No test registers a real hotkey or touches a
real display. Every check was proven by breaking the line it guards -
including two that were rewritten after review because they exercised a
copy of the logic rather than the logic, which the whole suite could not
distinguish.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0187tGqyEw4frZzDYPPJfUMd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant