Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UI: Add Search by Hotkey filter and Hotkey Duplicate Detection #5297

Merged

Conversation

furious
Copy link
Contributor

@furious furious commented Sep 18, 2021

Description

Implemented a duplicate detection that shows a warning indicator inside the Hotkey field, clicking on that icon will filter all actions with the same Hotkey.
Also added a standalone search field so users can find anything by Hotkeys.
Improved the hotkey listing, now all hotkeys are inside a scroll area so the filters stay on the top always.
image

Motivation and Context

Hotkey Duplicate Detection

Fixes #5081

How Has This Been Tested?

Windows 10, compiled and run

On the hotkeys settings, put the same hotkey in random actions, it shows a warning icon inside the field telling its being used by other actions.
Clicking on the icon will only show actions with the same hotkey. Same effect as you search by the hotkey filter.

Types of changes

New feature (non-breaking change which adds functionality)
Tweak (non-breaking change to improve existing functionality)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

@derrod derrod added the Seeking Testers Build artifacts on CI label Sep 18, 2021
@WizardCM WizardCM added the Enhancement Improvement to existing functionality label Sep 18, 2021
@dodgepong dodgepong requested a review from Warchamp7 September 20, 2021 14:35
@furious furious force-pushed the feature/hotkey-duplicate-detection branch from fa4899f to 7e8dd7a Compare September 20, 2021 14:49
@ghost
Copy link

ghost commented Sep 20, 2021

I've found an issue: First filter by a hotkey, then enter a search term into the normal filter.
If you do it in that order, you will see that it will show all items again matching the normal filter, ignoring the hotkey filter.
When you then re-enter the filtered hotkey, everything works correctly.

@furious
Copy link
Contributor Author

furious commented Sep 26, 2021

Fixed the issue mentioned by @Vainock, now both filters work together.
Change the warning text as @Fenrirthviti suggested on the discussion thread.

@jp9000 jp9000 added this to the OBS Studio 27.2 milestone Oct 16, 2021
@jp9000 jp9000 force-pushed the feature/hotkey-duplicate-detection branch from 19a16a0 to 79f4ae6 Compare October 24, 2021 12:52
@jp9000 jp9000 force-pushed the feature/hotkey-duplicate-detection branch from 79f4ae6 to 793eb40 Compare October 24, 2021 12:54
@jp9000
Copy link
Member

jp9000 commented Oct 24, 2021

I made some minor modifications before merging.

  • I reduced the iteration count within ScanHotkeys. It was O(n2) before which was really not optimal.
  • It now disregards duplicates from hotkey pairs, because pairs were intended to share hotkeys
  • The icon is now only created once in OBSBasicSettings, then shared among all the hotkeys, which will help create hotkey widgets much faster
  • Every hotkey widget was being redrawn every single time ScanHotkeys was called, which is really not optimal. Instead, I changed it so that it checks to see if the duplicate state has changed since last update, and only update the widget in that specific case.
  • I made the code a little bit less indented/squishy. If you run clang-format and you see your code becoming really squished, that probably means you need to split the code you're writing off into a separate function

Keep in mind that there are some users that have a very large number of scenes/sources, thus they can have a lot of hotkeys, so the hotkeys window was already unoptimal as it was. Even with my added optimizations, it will still have an impact on users who have a large number of hotkeys. This is not really your fault or anything, just kind of a symptom of creating/showing every single hotkey all in one go.

I need to note again to everyone else that we need to switch the hotkeys section to a listview or gridview of some sort where we can add hotkeys to that box contextually rather than show every potential hotkey all at once. The current method is just not optimal for people with large number of sources/etc.

@jp9000 jp9000 merged commit 0c1524b into obsproject:master Oct 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Improvement to existing functionality Seeking Testers Build artifacts on CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hotkey Duplicate Detection
4 participants