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

feat: Add global shortcuts #326

Draft
wants to merge 15 commits into
base: main
Choose a base branch
from
Draft

feat: Add global shortcuts #326

wants to merge 15 commits into from

Conversation

tuxinal
Copy link

@tuxinal tuxinal commented Jan 7, 2024

i'm using electron's globalShortcuts api here which doesn't have a way to detect tell the difference between keyup and keydown. this means the push to talk/mute/deafen/etc actions don't really work. we'd probably have to switch to something else at some point
currently implementing venbind to use instead of electron's api

resolves #18

src/renderer/patches/keybinds.ts Outdated Show resolved Hide resolved
src/renderer/patches/keybinds.ts Outdated Show resolved Hide resolved
@tuxinal tuxinal changed the title Add global shortcuts feat: Add global shortcuts Jan 7, 2024
@tuxinal
Copy link
Author

tuxinal commented Jan 7, 2024

Okay so electron's globalShortcuts api is not at all a good fit for this. the keyup/keydown events are rather crucial for discord's functionality. and also it "consumes" inputs (i have no idea what the correct term for this is. basically if you set anything as a keybind nothing else can use it, for example if you have the tab key as push to talk tab won't do anything everywhere else).

@Vendicated
Copy link
Member

yes. as mentioned in the issue you linked and as discussed on discord, using a native library would be better. however all the current options don't seem to support wayland at all, so we will most likely have to write our own module

@D3SOX D3SOX mentioned this pull request Feb 1, 2024
tuxinal added 2 commits March 8, 2024 23:27
convert id to number
enable desktop only shortcut actions
Co-Authored-By: exhq <infidelLOler@gmail.com>
tuxinal added 2 commits May 4, 2024 14:19
@klhrt
Copy link

klhrt commented May 31, 2024

Could you do a quick write-up of how this works? E.g. is vesktop --keybind 2 supposed to execute the keybind of id 2 set inside the app, or is the cli used to attach a bind to a key? It's pretty difficult for me to parse the code to figure out how to use it.

@tuxinal
Copy link
Author

tuxinal commented Jun 1, 2024

when you make a new keybind in discord asigns it an id. currently i have the id for each keybind written next to the keybind in the client.
you can use this id in the cli as: (vesktop) -- --keybind (id) (mind the two sets of dashes)
or if it's ptt you can add a keyup/keydown thing at the end of it.

@alterNERDtive

This comment was marked as spam.

@klhrt
Copy link

klhrt commented Jul 3, 2024

this doesn't seem to work on my setup on ubuntu with i3+x11, unless I still don't get how to use it. not sure if it's just not compatible with my setup or i'm just stupid, but if it doesn't work due to a window manager or display server problem it's something to look into.

if i set a toggle mute keybind with an arbitrary key and it gets assigned an id of 3, running vesktop --keybind 3 doesn't toggle mute.

@tuxinal
Copy link
Author

tuxinal commented Jul 4, 2024

an update seems to have borked it
you'll have to wait until i figure out how to fix it

@tuxinal
Copy link
Author

tuxinal commented Jul 15, 2024

@klhrt try this commit

@klhrt
Copy link

klhrt commented Jul 15, 2024

UnhandledPromiseRejectionWarning: Error: Script failed to execute, this normally means an error was thrown. Check the renderer console for the error.
    at node:electron/js2c/renderer_init:2:16470
    at IpcRendererInternal.<anonymous> (node:electron/js2c/renderer_init:2:10723)
    at IpcRendererInternal.emit (node:events:519:28)
    at Object.onMessage (node:electron/js2c/renderer_init:2:8837)
(node:1605447) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 33)

Whenever I run the keybind command, no response in-app and this error pops up. Not sure if the error is responsible/related though. I also don't know if it was happening in the earlier commit as I wasn't running vesktop from a terminal window.

for reference this is the resulting printout from the terminal where i run the keybind command:

~> vesktop --keybind 2
Checking for beta autoupdate feature for deb/rpm distributions
Found package-type: deb
Checking for update
~>

@Covkie
Copy link
Collaborator

Covkie commented Jul 15, 2024

@klhrt ATM the command is vesktop -- --keybind id 🥴

@klhrt
Copy link

klhrt commented Jul 15, 2024

@klhrt ATM the command is vesktop -- --keybind id 🥴

and it works! thanks

@tuxinal
Copy link
Author

tuxinal commented Aug 18, 2024

it would be nice if someone could test this commit on their system. the binary is technically built on nix and idk if that affects anything

@Covkie
Copy link
Collaborator

Covkie commented Aug 18, 2024

Can confirm X11 works! (Arch here)

@rbutera
Copy link

rbutera commented Aug 27, 2024

anyone know how hard it would be to get this to work on Hyprland?

@tuxinal
Copy link
Author

tuxinal commented Aug 27, 2024

you'd have to implement xdg-desktop-portal's GlobalShortucts api for hyprland on their portal implementation (which they might have implemented already i'm not sure)
i'm working on the wayland side of things as of right now and it's proving somewhat difficult. no one really has a complete implementation of GlobalShortcuts

@condmaker
Copy link

Doesn't appear to be working on Hyprland or wayland in general. I can't even start the instance. Here's the full stack trace if that helps:

thread '<unnamed>' panicked at src/lib.rs:16:44:
called `Result::unwrap()` on an `Err` value: Message("todo")
stack backtrace:
   0:     0x7e1f3eae23d5 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h3692694645b1bb6a
   1:     0x7e1f3eb0661b - core::fmt::write::h5131d80b4c69b88d
   2:     0x7e1f3eadff8f - std::io::Write::write_fmt::h1fb327a7d8b0eb36
   3:     0x7e1f3eae21ae - std::sys_common::backtrace::print::h998d75b840f75a73
   4:     0x7e1f3eae36f9 - std::panicking::default_hook::{{closure}}::h18ec7fe6a38b9da0
   5:     0x7e1f3eae349a - std::panicking::default_hook::hfb3f22c2e4075a6a
   6:     0x7e1f3eae3b93 - std::panicking::rust_panic_with_hook::h51af00bcb4660c4e
   7:     0x7e1f3eae3a74 - std::panicking::begin_panic_handler::{{closure}}::h39f76aa863fbe8ce
   8:     0x7e1f3eae2899 - std::sys_common::backtrace::__rust_end_short_backtrace::h4d10fc2251b89840
   9:     0x7e1f3eae37a7 - rust_begin_unwind
  10:     0x7e1f3e9cea13 - core::panicking::panic_fmt::h319840fcbcd912ef
  11:     0x7e1f3e9ceea6 - core::result::unwrap_failed::haccb9aaa604e1e21
  12:     0x7e1f3e9dfa8c - std::sys_common::backtrace::__rust_begin_short_backtrace::hfc2e22cd1f52e7a4
  13:     0x7e1f3e9dd1ff - core::ops::function::FnOnce::call_once{{vtable.shim}}::h3001236f03d016af
  14:     0x7e1f3eae5ecb - std::sys::pal::unix::thread::Thread::new::thread_start::h3b8e81128811868f
  15:     0x7e1fd0c4439d - <unknown>
  16:     0x7e1fd0cc949c - <unknown>
  17:                0x0 - <unknown>
[arRPC > ipc] listening at /run/user/1000/discord-ipc-0
[arRPC > websocket] listening on 6463
[arRPC > process] started
thread '<unnamed>' panicked at src/lib.rs:20:44:
called `Result::unwrap()` on an `Err` value: Message("todo")
stack backtrace:
   0:     0x7e1f3eae23d5 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h3692694645b1bb6a
   1:     0x7e1f3eb0661b - core::fmt::write::h5131d80b4c69b88d
   2:     0x7e1f3eadff8f - std::io::Write::write_fmt::h1fb327a7d8b0eb36
   3:     0x7e1f3eae21ae - std::sys_common::backtrace::print::h998d75b840f75a73
   4:     0x7e1f3eae36f9 - std::panicking::default_hook::{{closure}}::h18ec7fe6a38b9da0
   5:     0x7e1f3eae349a - std::panicking::default_hook::hfb3f22c2e4075a6a
   6:     0x7e1f3eae3b93 - std::panicking::rust_panic_with_hook::h51af00bcb4660c4e
   7:     0x7e1f3eae3a74 - std::panicking::begin_panic_handler::{{closure}}::h39f76aa863fbe8ce
   8:     0x7e1f3eae2899 - std::sys_common::backtrace::__rust_end_short_backtrace::h4d10fc2251b89840
   9:     0x7e1f3eae37a7 - rust_begin_unwind
  10:     0x7e1f3e9cea13 - core::panicking::panic_fmt::h319840fcbcd912ef
  11:     0x7e1f3e9ceea6 - core::result::unwrap_failed::haccb9aaa604e1e21
  12:     0x7e1f3e9d82bc - venbind::js::napi_register_keybind::hee504851f7cfee42
  13:     0x5c87ef3a456c - <unknown>
fatal runtime error: failed to initiate panic, error 5
[1]    79116 IOT instruction (core dumped)  RUST_BACKTRACE=full ./vesktop

@lillithkt
Copy link

Doesn't appear to be working on Hyprland or wayland in general. I can't even start the instance. Here's the full stack trace if that helps:
looks like venbind isnt setup for wayland yet, id just wait
/~https://github.com/tuxinal/venbind/blob/885e0dd7658bb2e25f1ad45f8d6581b0e330febb/src/linux.rs#L30

@tuxinal
Copy link
Author

tuxinal commented Aug 27, 2024

oh none of the wayland support stuff is pushed yet. currently venbind only supports linux x11

@OrHy3
Copy link

OrHy3 commented Nov 26, 2024

Over the weekend I started writing a C library (based on libdbus) to ease interactions with XDG GlobalShortcuts. The said library is libgsbus, it's in a very basic release state, but it's complete.
As of now it lacks documentation and a build system, but it's a single source-header files pair, the latter being commented. The project should now be ready.

If anyone is willing to test it around, it'll be greatly appreciated.

@checkraisefold
Copy link

checkraisefold commented Jan 9, 2025

As an update for anyone with the PR watched: further work on Venbind has begun to support Windows and Wayland (through xdg-desktop-portal DBus), and is underway.
See tuxinal/venbind#2, tuxinal/venbind#1

@checkraisefold
Copy link

An alternative to XDP if at all possible may need to be explored at some point for Venbind considering there is currently 3 different (2 confirmed) bugs blocking global shortcuts in KDE and Hyprland, which afaik are the only two actually usable global shortcuts XDP implementations

hyprwm/xdg-desktop-portal-hyprland#310
https://bugs.kde.org/show_bug.cgi?id=492992

@OrHy3
Copy link

OrHy3 commented Jan 9, 2025

Last time I worked with GlobalShortcuts on KDE it reused the previous session just fine (also tested right now).
Regarding your pull request, seems like it already got merged.

I honestly think the D-Bus portal is the way to go, mostly because will likely be the next standard for Wayland, but also because (from what I get) there is no intention here to use libevdev or similar input-permission related methods, even if it's just gonna be temporary.

@checkraisefold
Copy link

Last time I worked with GlobalShortcuts on KDE it reused the previous session just fine (also tested right now). Regarding your pull request, seems like it already got merged.

I honestly think the D-Bus portal is the way to go, mostly because will likely be the next standard for Wayland, but also because (from what I get) there is no intention here to use libevdev or similar input-permission related methods, even if it's just gonna be temporary.

Yeah, the hyprland xdg PR got merged in 10 minutes apparently. However, the KDE bug was blocking testing of Venbind's XDG implementation by the Venbind maintainer - I couldn't test it on Hyprland because of that bug, but I'll give it a try with the latest commit of xdg-desktop-portal-hyprland.

@StayBlue
Copy link

StayBlue commented Jan 9, 2025

I think it's important to note that the Venbind maintainer hasn't had any Github activity over the past couple of months, so it might be necessary to make a hard fork of it if he doesn't reply soon. That decision is probably best left in the hands of @checkraisefold, as he is the one who is bringing back momentum into this PR.

@checkraisefold
Copy link

I think it's important to note that the Venbind maintainer hasn't had any Github activity over the past couple of months, so it might be necessary to make a hard fork of it if he doesn't reply soon. That decision is probably best left in the hands of @checkraisefold, as he is the one who is bringing back momentum into this PR.

On that note, the maintainer is active in the Vencord Discord server #vesktop-development channel (where I have been very active in regards to looking at the Wayland/XDG issues, I pinged you in there). Shouldn't need to make a hard fork, I think they'll be able to merge the XDG pr soon once I test it on Hyprland.

@checkraisefold
Copy link

Basic Windows and Wayland (via xdg-desktop-portal) support have been merged into Venbind! 🥳

There won't be another tagged version of Venbind until it is at least somewhat usable (confirmed with maintainer) - key release events aren't implemented and there's issues with the existing implementation that need to be fixed. Once that's done, and Darwin support is done (shouldn't be too hard - libuiohook has it inbuilt) then Venbind should be pretty ready, and then everything's on the JavaScript side to implement.

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.

Custom Keybinds