markdwags / Razor

Razor is a free tool designed to help with simple tasks while playing Ultima Online.
https://www.razorce.com
GNU General Public License v3.0
139 stars 97 forks source link

[BUG] ClassicUO: Function keys (F1-F12) as Hot Keys overlap with non-function keys #153

Closed dallenwilson closed 3 years ago

dallenwilson commented 3 years ago

Client: ClassicUO 0.1.7.49 targeting client.exe v4.0.6a Razor: v1.6.8.15 Razor installed as a ClassicUO Plugin. OS is Gentoo Linux 64-bit.

When setting a hotkey in Razor, the function keys (F1 through F12) appear to be overlapping with non-function keys.

Example: I set "Undress Both Hands" to F4, and F4 does unequip both hands. But so does the 's' key. The 's' is not passed to the chat input, regardless of whether "Pass to UO" is checked. Unset the "Undress Both Hands" and 's' works as expected.

F3 overlaps 'r'. F7 overlaps 'v'. F9 overlaps 'x'.

There may be more, but those are what I found. There are no macros set up in the ClassicUO client beyond the default (alt-p, alt-r, etc).

dallenwilson commented 3 years ago

/usr/include/X11/keysymdef.h shows #define XK_s 0x0073 /* U+0073 LATIN SMALL LETTER S */, while this microsoft documentation shows vbKeyF4 | 0x73 | F4 key.

Similar patterns exist for the other overlaps. I'm still getting acquainted with Razor's codebase, but it looks like Razor/Platform.cs does change it's key-getting technique depending on which OS it's running on (GetAsyncKeyState()). Once I can get it to actually compile under mono, I'll be able to test a bit and try to find out why this is happened.

dallenwilson commented 3 years ago

The root of the problem is that X11 keysyms don't match up nicely to Windows's Virtual Key Codes. In Razor/HotKeys/HotKeys.cs under OnKeyDown(), it checks the passed variable key against the hotkey list, and if it doesn't find it it does a second check, comparing the whole hotkey list against every key currently being pressed. Under X11 but using Windows's key codes, pressing 's' matches both 's' and 'F4'. X11 has separate keysyms for upper and lowercase, windows apparently does not.

I'm going to try to fix this first in GetAsyncKeyState() under Razor/Platform.cs by ignoring lowercase keycodes, but another approach would be to scrap the second check in OnKeyPressed(). Either approach could have unintended consequences, but the second would affect every Razor user; The first only affect's linux users.

markdwags commented 3 years ago

We've definitely had some issues pop up similar to what you're experiencing in the past as you can see in Razor/Platform.cs where there is some manually mapping.

That's mapping SDL to Winforms. In ClassicUO, keypresses send an SDL code to Razor, and Razor uses that mapping to convert it to the Winforms code. Unfortunately the mapping doesn't always cover all keyboards layouts (usually effects folks with a non-standard US keyboard) so Razor also does it's own platform-specific "is this key pressed?" check.

@roxya has done a bunch of work in this space and might be able to assist more, we're both on the ClassicUO Discord server too.

dallenwilson commented 3 years ago

I noticed the mapping, but didn't pay it much mind. I'm not surprised SDL means yet a third system to manage, though.

I seem to have found a quick fix. I changed the return of LinuxPlatform.GetAsyncKeyState() from return r == s ? (ushort) 0xFF00 : (ushort) 0;

to

if (pressed)
return r == s ? (ushort) 0xFF00 : (ushort) 0;
else
return (ushort) 0;

The bool pressed variable was already there but unused other than being assigned. It seems to work for the few keys I found problems with. It's such an obvious thing to try that I'm sure there's a reason it wasn't being used, though.

I'll run it locally for a bit and see if I can find out what it breaks, before I submit any PR's.

dallenwilson commented 3 years ago

I've fixed things to my satisfaction. The changes are available here if anyone wants to look them over.

I'm not submitting it as a PR at this time, as it's not a very good quality fix. But it works for me.

Edit: It's better quality now. I'm pretty happy with it.

markdwags commented 3 years ago

PR for the fix is here

https://github.com/markdwags/Razor/pull/156