NickeManarin / ScreenToGif

🎬 ScreenToGif allows you to record a selected area of your screen, edit and save it as a gif or video.
http://www.screentogif.com
Microsoft Public License
23.06k stars 2.15k forks source link

[Bug] Can't set shortcut with window button #1250

Open TheMikeyRoss opened 7 months ago

TheMikeyRoss commented 7 months ago

Look at this PR https://github.com/NickeManarin/ScreenToGif/pull/955

It seems to work but without the window button.

I'm trying to set win + shift + g as a shortcut but I cant put more than 2 keys (when a window button is part of the combination). My version is the latest 2.39

pawlos commented 7 months ago

@TheMikeyRoss As far as I checked, in the current code, Windows key is not treated as a modifier (similar to ctrl, alt or shift) but as a regular key (also I can see .NET doesn't return it in modifiers collection).

So you can have shift+windows in the same way as shift+g but you can't have win+shift+g. I can also see that win, key is also partially excluded by the property AllowAllKeys that is set to false for the shortcut controls.

I think that the check and the comment in the lines https://github.com/NickeManarin/ScreenToGif/blob/master/ScreenToGif/Controls/KeyBox.cs#L286C1-L287C76 are invalid (stating and treating Windows as a modifier) as in this particular case IsWindowsDown won't be true. This is the part of the code where AllowAllKeys = false (else for https://github.com/NickeManarin/ScreenToGif/blob/master/ScreenToGif/Controls/KeyBox.cs#L248) and thus this didn't happen https://github.com/NickeManarin/ScreenToGif/blob/ccd9b0d49bdbd180beebc1175ca2ef51e9755374/ScreenToGif/Controls/KeyBox.cs#L234C8-L235C90 so IsWindowsDown will have the default value.

Additionally many of the win shortcuts are used in the windows itself and that might be causing the issue.

Maybe with some major rewrite (or in V3) it will be possible to cover such case. Again, this is just my few thought looking at my aforementioned PR #955 and the code.