obsproject
UI: Split toggle preview program hotkey into hotkey pair
Description
Split toggle preview program hotkey into hotkey pair
Motivation and Context
To be sure it is enabled or disabled when the hotkey is pressed.
How Has This Been Tested?
set the same and different keys for enable and disable
Types of changes
- New feature (non-breaking change which adds functionality)
Checklist:
- [x] My code has been run through clang-format.
- [x] I have read the contributing document.
- [x] My code is not on the master branch.
- [x] The code has been tested.
- [x] All commit messages are properly formatted and commits squashed where appropriate.
Just for clarification, this is in addition to the toggle hotkey, correct? It does not remove the single toggle?
@Fenrirthviti It replace the toggle hotkey, with a hotkey pair. If you set both hotkeys of the hotkey pair to the same key you can still get the toggle.
I am personally opposed to this paradigm in the hotkey codex, as there is no indication that doing so will create a toggle, and in my personal experience, using this method to create a toggle is sketchy and unreliable at best. I would prefer to leave the toggle separate still.
Note: this was requested on Fider https://ideas.obsproject.com/posts/769/split-studio-mode-hotkey-into-enable-studio-mode-disable-studio-mode-to-avoid-toggle
If you want to bind macros for working in studio mode, and want to make sure it's enabled when pressed - this isn't possible because you'd end up toggling things.
I'm not opposed to adding the new hotkeys, but I think we should leave the single toggle as well. Honestly, we should have a toggle for many other functions that's bound as a single hotkey, or come up with a better/cleaner way of communicating these pairs are treated as a toggle, and clean up the inconsistencies with their function. Might be a bit larger than this PR for that, but I would like to see the single toggle hotkey stay otherwise. However, this is just my opinion.
If we split a single toggle into separate hotkeys that can be bound together into a toggle, then I don't think we should keep the single toggle hotkey. It would be redundant, and wouldn't match what we are doing elsewhere. For the sake of adding the ability to separate the actions, I think switching from a single toggle hotkey into separate hotkeys that are bindable together as a toggle is perfectly fine from the perspective of it not doing anything new in the program.
If we want to get rid of bindable toggles altogether, or replace the binding mechanism with a separate single-button toggle entry in addition to the separate entries, that is a different question that should probably be addressed in a different context.
tl;dr I'm fine with this change as-is, assuming the code itself is good.
I understand it's not something we do anywhere else, but I think you're missing my point where I think it's a bad paradigm to have. There's very little indication in the UI that pairing hotkeys like that creates a toggle, and honestly, I'm not convinced that it does actually create a true toggle as I have encountered plenty of issues when setting hotkeys like this where it is unreliable. Again probably a discussion for another thread, but I want to make sure it's at least called out.
On topic to this PR, if we're removing the single toggle hotkey we also need a fallback/catch for people who already have this key set as to not break workflow.
The pair system was sort of designed to be used like a toggle. That's why it's a separate designated hotkey type. That being said I'm not entirely sure how I feel about this. I'll think about it.
The pair system was sort of designed to be used like a toggle. That's why it's a separate designated hotkey type. That being said I'm not entirely sure how I feel about this. I'll think about it.
What is there to feel about? It's in line with how the rest of OBS works, and would give the added functionality of macroing Studio states. Currently there's no way to tell if the hotkey is toggled on or off.
It's essentially just a feature increase, while keeping previous functionality?
That's true, I suppose I understand how it would be useful on a single monitor setup.
In general I agree that explicit is better than implicit, but this change seems to re-use the paradigm already in use by all the other hotkey pairs.
Speaking of "implicit": Do we need/want to read the old "toggle" hotkey and set it for these new hotkeys automatically to not break current setups as pointed out by @Fenrirthviti?
@PatTheMav replacing the old hotkey to not break existing setups was added Mar 25 2020 to this PR, Is there something wrong or missing in the way I implemented that?
@PatTheMav replacing the old hotkey to not break existing setups was added Mar 25 2020 to this PR, Is there something wrong or missing in the way I implemented that?
Got it, saw that commit but wasn't sure it was doing what I thought it was doing, so wanted to make sure.