FEAT(client): Add shortcut to toggle whispers for voice activation/continuous transmission modes#7219
FEAT(client): Add shortcut to toggle whispers for voice activation/continuous transmission modes#7219darustam-1 wants to merge 4 commits into
Conversation
WalkthroughThis PR introduces a Whisper Hold feature to Mumble. It extends the type system by adding 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/mumble/MainWindow.cpp`:
- Around line 3272-3284: Current code calls removeWhisperHoldTarget() which
performs an immediate updateTarget(), causing a brief fallback to the default
when switching held targets; instead, when switching from one held target to
another, clear the old target without triggering an update and only call the
update once after the new target is set. Change the block around
stWhisperHoldTarget/bWhisperHoldActive so that when stWhisperHoldTarget != st
you call a non-updating removal (either an existing no-update variant or add a
boolean parameter to removeWhisperHoldTarget, e.g.,
removeWhisperHoldTarget(false)) to clear the old target, then assign
stWhisperHoldTarget = st, set bWhisperHoldActive = true and call
refreshWhisperHoldTarget() once to apply the new target; keep the existing
immediate-remove path when stWhisperHoldTarget == st to clear and update
immediately.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0b944f76-9c93-4a9d-9440-fea964d32146
📒 Files selected for processing (7)
src/mumble/EnumStringConversions.cppsrc/mumble/GlobalShortcutTypes.hsrc/mumble/Log.cppsrc/mumble/Log.hsrc/mumble/MainWindow.cppsrc/mumble/MainWindow.hsrc/mumble/Settings.cpp
Hartmnt
left a comment
Hartmnt
left a comment
There was a problem hiding this comment.
Please squash the first 3 commits into one. You will need to force-push your branch after that which is fine. Leave the translation commit as is for now.
This commit changes the earlier test where whisper/shout was set up as a global toggle, which while it worked for my purposes in current channel scope, it seemed an improperly implemented feature that could add a lot of useful functionality if properly fleshed out. This is replacing the toggle with a shortcut action "Whisper Hold." Whisper Hold acts much like the Whisper/Shout shortcut, but for voice activity and continuous transmission instead of requiring push to talk. It is disabled when push-to-talk is in use, as that functionality is handled by the existing whisper/shout shortcut. It operates as a latched target selector for those transmission modes, where pressing the shortcut sends a console message to the user by default and then sends all voice activity to the assigned whisper target until either unlatched by a repeated press, or latching to another whisper target if an alternative binding is simultaneously bound, much like having multiple whisper/shout PTS keybinds set up at once. It operates under the existing whisper/shout framework and thus the old PTS binding overrides the whisper hold, up until the pttHold delay ends. Implements mumble-voip#6754 with additional functionality
darustam-1
force-pushed
the
whisper-shout-toggle
branch
from
dee8310 to
5200b54
Compare
|
Squashed the existing commits. The two whitespace issues are vscode default clang-format artifacts (when I use "format document") that I didn't catch; I will review the other notes soon and respond to them respectively. |
Refactored some settings validation code into the respective ConfigDialog and added ShortcutTarget conversion guards.
Identified this bug later on--changing channel while whisper holding to the "Current" channel setting works on the channel when the hold is enabled, but changing channels keeps you whispering to that channel. This is because UserModel:moveUser() emits userMoved before moveItem() updates the user's channel. Thus, just adding an updateTarget() doesn't work because ClientUser::get()->cChannel still points to the old channel. Thus, an emitter localUserChannelChanged() was added to emit after moveItem(), so cChannel is updated, and it doesn't intrude on existing code and functions. More lines than I would like to add but it does the trick.
2 participants
Currently, whispering requires setting a whisper/shout shortcut in settings for a designated target, then using that key as a push-to-talk key that overrides the current transmission target. This is fine in push-to-talk mode as it operates in a similar fashion with different key bindings set for other targets, but in automatic transmit modes (continuous/voice activation), this means you are forced to push-to-whisper, rather than just being able to latch to a target for some time while keeping your fingers free.
This branch implements a feature that allows the user to bind "Whisper Hold" keys for automatic transmission modes. In automatic modes it allows the user to latch the transmission target until the key is either pressed again, returning to default state (shout mode) or another whisper hold key is pressed, changing the target once again.
In addition, this feature rests beneath existing whisper/shout functionality, meaning that while latched to a whisper target, holding down a whisper/shout shortcut to any target will activate a temporary override, like a push-to-talk key.
The implementation reuses Mumble's existing whisper target routing functionality without any new audio packet path or server protocol. Whisper Hold stores its own latch state using:
bWhisperHoldActive(user has latched a target)bWhisperHoldTargetApplied(latched target is currently inqmCurrentTargetsIWhisperShoutTargetsActive(regular W/S PTT targets are active, includingpttHoldShortcutTarget(selected latch target)In push-to-talk it has no functionality and is therefore disabled. Switching to PTT through the toolbar, settings, or audio wizard clears the latch. Switching between continuous and voice activation does not clear the latch. The state is also automatically reset on app close, disconnect, or modifying shortcut settings.
The feature also adds a message type for whisper hold. It has no default sound but console is enabled, thus pressing the "Whisper Hold" shortcut for a given target will inform the user in the console that they are being held to a target or if the hold is cleared.
This is my first open-source contribution. It builds on Windows and passes my runtime testing in multiple servers.
Implements #6754 with additional functionality.
Checks