From 93967db096f6d39c9d9a992e433a72f3002519b3 Mon Sep 17 00:00:00 2001 From: "Lucas Greuloch (greluc)" Date: Mon, 7 Sep 2026 21:27:29 +0200 Subject: [PATCH 1/2] Stop a shortcut firing the moment it is assigned Assigning the mute or deafen shortcut fires it immediately, and the same mechanism can leave the microphone open until the app restarts. The key watcher polls. `AddKeyHandler` seeds a key as up, and the thread compares that seed against GetAsyncKeyState every 60ms, emitting a transition. So wiping its map has two effects that both look like input nobody gave: * a key that is physically down at that moment is re-seeded up, so the next sweep reports a keydown the user never made, and the real release then completes a press the app invents. That is what fires mute or deafen the instant one of them is bound, because binding is committed on keydown while the key is still held and immediately sends RESET_KEYHOOKS. * a key the watcher was already tracking loses the keyup that would have closed it. Nothing decrements `speaking`, PUSH_TO_TALK false is never sent again, and the microphone stays open for the rest of the session. Three changes, and they only work together. `resetKeyHooks` no longer calls `clearKeyHooks`. addKeyHook ignores a key already in the map, so re-applying the shortcuts leaves every key exactly as the watcher last saw it and nothing is fabricated or lost. The cost is that a key which stops being a shortcut keeps being polled until exit; it matches no binding, so nothing acts on it. The addon has no way to unhook one key -- RemoveKeyHandler is an empty function -- which is why this used to wipe everything. The settings field commits on the DOM keyup instead of the keydown. The renderer is the only part of the app that knows the key is up; the main process cannot ask. Hooking it in a state the watcher agrees with is the whole point. And every release is now answered from what its press was granted, recorded per key at keydown, instead of by asking the bindings again. The answer can change while a key is held -- a round ending, or a binding reassigned to the very key being held, which is exactly what assigning a shortcut does. Verified against a model of the watcher transcribed from keyhandler.cpp, driving the real handler bodies extracted from this file. Eight scenarios: on nightly four fail, including the reported bug and two ways to strand the microphone; on this branch all eight pass. A search over 20000 random gestures that ends by releasing everything and settling finds 54 runs on nightly where the microphone is left open, and none here. One visible change beyond the fix: holding a key over the field and clicking away without releasing it now binds nothing, where before it bound the key and fired the shortcut. Co-Authored-By: Claude Opus 5 Signed-off-by: Lucas Greuloch (greluc) --- src/main/hook.ts | 76 ++++++++++++------- .../settings/sections/KeybindsSection.tsx | 26 ++++++- 2 files changed, 73 insertions(+), 29 deletions(-) diff --git a/src/main/hook.ts b/src/main/hook.ts index 9764f005a..359de5f36 100644 --- a/src/main/hook.ts +++ b/src/main/hook.ts @@ -27,7 +27,18 @@ function resetKeyHooks(): void { deafenShortcut = store.get('deafenShortcut', 'RControl') as K; muteShortcut = store.get('muteShortcut', 'RAlt') as K; impostorRadioShortcut = store.get('impostorRadioShortcut', 'F') as K; - keyboardWatcher.clearKeyHooks(); + // Deliberately no clearKeyHooks() here. The watcher polls, and its map is what it + // compares against: clearing re-seeds every key as up, so the next poll invents a + // keydown for any key that is physically down at that moment, and loses the keyup for + // any key it was already tracking. The invented press is what fires mute or deafen the + // instant one of them is assigned; the lost release is what leaves `speaking` above + // zero with the microphone open until the app restarts. + // + // addKeyHook ignores a key already in the map, so re-applying the shortcuts leaves + // every key exactly as the watcher last saw it. The cost is that a key which stops + // being a shortcut keeps being polled until exit; it matches no binding, so nothing + // acts on it. The addon offers no way to unhook a single key -- its RemoveKeyHandler + // is an empty function -- which is why this used to wipe everything. addKeyHandler(pushToTalkShortcut); addKeyHandler(deafenShortcut); addKeyHandler(muteShortcut); @@ -69,26 +80,39 @@ ipcMain.handle(IpcHandlerMessages.START_HOOK, async (event) => { if (!readingGame) { readingGame = true; let speaking: number = 0; - // Whether the impostor radio is currently held down and was granted. The keyup - // branch below used to re-evaluate `isImpostor` instead, and that answer can change - // while the key is still down -- a round ending leaves `lastState.players` without - // this client in it. When it did, the decrement and the release were both skipped, - // `speaking` stayed above zero, and PUSH_TO_TALK false was never sent afterwards. - let impostorRadioHeld = false; + // What each held key was granted when it went down. Every release is answered from + // this record rather than by asking the bindings again, because the answer can + // change while the key is still down: a round can end, leaving `lastState.players` + // without this client in it, and a binding can be reassigned -- including to the + // very key being held, which is what happens when a player assigns a shortcut. + // Re-asking at keyup makes the release answer a different question than the press + // did, which either fires a shortcut the press never claimed or skips the decrement + // and leaves `speaking` above zero with the microphone open until the app restarts. + const heldGrants = new Map(); resetKeyHooks(); keyboardWatcher.on('keydown', (keyId: number) => { - if (keyCodeMatches(pushToTalkShortcut!, keyId)) { - speaking += 1; - } - if ( + // Already held: the watcher re-reporting a key it is polling, or auto-repeat. + // Counting it again would need two releases to undo one press. + if (heldGrants.has(keyId)) return; + + const talk = keyCodeMatches(pushToTalkShortcut!, keyId); + const radio = keyCodeMatches(impostorRadioShortcut!, keyId) && - !impostorRadioHeld && - gameReader.lastState.players?.find((value) => { + gameReader?.lastState.players?.find((value) => { return value.clientId === gameReader.lastState.clientId; - })?.isImpostor - ) { - impostorRadioHeld = true; + })?.isImpostor === true; + heldGrants.set(keyId, { + talk, + radio, + mute: keyCodeMatches(muteShortcut!, keyId), + deafen: keyCodeMatches(deafenShortcut!, keyId), + }); + + if (talk) { + speaking += 1; + } + if (radio) { speaking += 1; event.sender.send(IpcRendererMessages.IMPOSTOR_RADIO, true); } @@ -103,22 +127,22 @@ ipcMain.handle(IpcHandlerMessages.START_HOOK, async (event) => { }); keyboardWatcher.on('keyup', (keyId: number) => { - if (keyCodeMatches(pushToTalkShortcut!, keyId)) { + const grant = heldGrants.get(keyId); + heldGrants.delete(keyId); + + if (grant?.talk) { speaking -= 1; } - if (keyCodeMatches(deafenShortcut!, keyId)) { + if (grant?.radio) { + speaking -= 1; + event.sender.send(IpcRendererMessages.IMPOSTOR_RADIO, false); + } + if (grant?.deafen) { event.sender.send(IpcRendererMessages.TOGGLE_DEAFEN); } - if (keyCodeMatches(muteShortcut!, keyId)) { + if (grant?.mute) { event.sender.send(IpcRendererMessages.TOGGLE_MUTE); } - // Released on the fact that it was granted, not on whether it would be granted - // again now. - if (keyCodeMatches(impostorRadioShortcut!, keyId) && impostorRadioHeld) { - impostorRadioHeld = false; - speaking -= 1; - event.sender.send(IpcRendererMessages.IMPOSTOR_RADIO, false); - } // Cover weird cases which shouldn't happen but just in case if (speaking < 0) { diff --git a/src/renderer/settings/sections/KeybindsSection.tsx b/src/renderer/settings/sections/KeybindsSection.tsx index 69fa2e036..dff5b1d4c 100644 --- a/src/renderer/settings/sections/KeybindsSection.tsx +++ b/src/renderer/settings/sections/KeybindsSection.tsx @@ -1,4 +1,4 @@ -import React, { useState } from 'react'; +import React, { useRef, useState } from 'react'; import { TFunction } from 'i18next'; import Alert from '@mui/material/Alert'; import Box from '@mui/material/Box'; @@ -80,6 +80,8 @@ const ShortcutField: React.FC = function ({ onStopRecording, onCapture, }) { + // What the last key press captured, held until that key is released. + const pendingCapture = useRef(null); const unset = !value || value === 'Disabled'; return ( = function ({ aria-label={label} onFocus={onStartRecording} onBlur={onStopRecording} + // Captured on the way down, committed on the way up. The key watcher polls + // GetAsyncKeyState against a map it seeds as up, so hooking a key while it is + // physically held makes the next poll report a press the user never gave -- and + // its release then fires the shortcut that was just assigned. Waiting for the + // release is the only way to hook it in a state the watcher will agree with, + // because the main process cannot ask whether a key is down. onKeyDown={(ev) => { if (ev.key === 'Tab') return; ev.preventDefault(); - const captured = keyFromEvent(ev); + pendingCapture.current = keyFromEvent(ev) ?? null; + }} + onKeyUp={(ev) => { + if (ev.key === 'Tab') return; + ev.preventDefault(); + const captured = pendingCapture.current; + pendingCapture.current = null; if (captured) onCapture(captured); }} + onMouseUp={(ev) => { + if (!recording || ev.button <= 2) return; + ev.preventDefault(); + onCapture(`MouseButton${ev.button + 1}`); + }} onMouseDown={(ev) => { if (recording && ev.button > 2) { + // Committed on mouseup, for the reason above -- the extra mouse buttons + // are polled by the same watcher. ev.preventDefault(); - onCapture(`MouseButton${ev.button + 1}`); return; } if (ev.button !== 0) return; From 1df869d3f4db4c9c6c06609a3ccb6c05ebc0ce53 Mon Sep 17 00:00:00 2001 From: Guus van der Meer Date: Tue, 8 Sep 2026 21:43:10 +0200 Subject: [PATCH 2/2] Remove verbose keybind comments --- src/main/hook.ts | 16 ---------------- .../settings/sections/KeybindsSection.tsx | 8 -------- 2 files changed, 24 deletions(-) diff --git a/src/main/hook.ts b/src/main/hook.ts index d692f1a86..e36f9577e 100644 --- a/src/main/hook.ts +++ b/src/main/hook.ts @@ -25,10 +25,6 @@ let impostorRadioShortcut: K | undefined; let keySender: WebContents | undefined; let pushToTalkHeld = false; let impostorRadioHeld = false; -// Whether the key that is currently down was bound to mute or deafen at the moment it went -// down. The toggles are fired from this rather than from the bindings as they stand at -// keyup, because a binding can be reassigned while its key is still held -- which is -// exactly what happens when a player assigns one of these shortcuts. const toggleGrants = new Map(); function releaseHeldKeys(): void { @@ -48,18 +44,6 @@ function resetKeyHooks(): void { deafenShortcut = store.get('deafenShortcut', 'RControl') as K; muteShortcut = store.get('muteShortcut', 'RAlt') as K; impostorRadioShortcut = store.get('impostorRadioShortcut', 'F') as K; - // Deliberately no clearKeyHooks() here. The watcher polls, and its map is what it - // compares against: clearing re-seeds every key as up, so the next poll invents a - // keydown for any key that is physically down at that moment, and loses the keyup for - // any key it was already tracking. The invented press is what fires mute or deafen the - // instant one of them is assigned. (The lost release used to leave the microphone open - // as well; releaseHeldKeys above now covers that.) - // - // addKeyHook ignores a key already in the map, so re-applying the shortcuts leaves - // every key exactly as the watcher last saw it. The cost is that a key which stops - // being a shortcut keeps being polled until exit; it matches no binding, so nothing - // acts on it. The addon offers no way to unhook a single key -- its RemoveKeyHandler - // is an empty function -- which is why this used to wipe everything. addKeyHandler(pushToTalkShortcut); addKeyHandler(deafenShortcut); addKeyHandler(muteShortcut); diff --git a/src/renderer/settings/sections/KeybindsSection.tsx b/src/renderer/settings/sections/KeybindsSection.tsx index dff5b1d4c..71c03f525 100644 --- a/src/renderer/settings/sections/KeybindsSection.tsx +++ b/src/renderer/settings/sections/KeybindsSection.tsx @@ -90,12 +90,6 @@ const ShortcutField: React.FC = function ({ aria-label={label} onFocus={onStartRecording} onBlur={onStopRecording} - // Captured on the way down, committed on the way up. The key watcher polls - // GetAsyncKeyState against a map it seeds as up, so hooking a key while it is - // physically held makes the next poll report a press the user never gave -- and - // its release then fires the shortcut that was just assigned. Waiting for the - // release is the only way to hook it in a state the watcher will agree with, - // because the main process cannot ask whether a key is down. onKeyDown={(ev) => { if (ev.key === 'Tab') return; ev.preventDefault(); @@ -115,8 +109,6 @@ const ShortcutField: React.FC = function ({ }} onMouseDown={(ev) => { if (recording && ev.button > 2) { - // Committed on mouseup, for the reason above -- the extra mouse buttons - // are polled by the same watcher. ev.preventDefault(); return; }