Fixing dir, shift-dir and ctrl-dir keys part 1: Win client

[PowerWyrm]

The problem: shift-dir and ctrl-dir keys (keypad) don't work with numlock on

The solution (main-win.c):

static bool handle_keydown(WPARAM wParam, LPARAM lParam)
{
	keycode_t ch = 0;

	bool kp = FALSE;
	
	int mods = 0;
	static bool has_shift = FALSE;
	static bool has_ctrl = FALSE;
	
#define check_ctrl(K) \
    if (has_ctrl) \
    { \
        ch = (K); \
        mods |= KC_MOD_CONTROL; \
    }

#ifdef USE_SAVER
	if (screensaver_active)
	{
		stop_screensaver();
		return TRUE;
	}
#endif /* USE_SAVER */

	/* for VK_ http://msdn.microsoft.com/en-us/library/dd375731(v=vs.85).aspx */
	switch (wParam) {
		case VK_F1: ch = KC_F1; break;
		case VK_F2: ch = KC_F2; break;
		case VK_F3: ch = KC_F3; break;
		case VK_F4: ch = KC_F4; break;
		case VK_F5: ch = KC_F5; break;
		case VK_F6: ch = KC_F6; break;
		case VK_F7: ch = KC_F7; break;
		case VK_F8: ch = KC_F8; break;
		case VK_F9: ch = KC_F9; break;
		case VK_F10: ch = KC_F10; break;
		case VK_F11: ch = KC_F11; break;
		case VK_F12: ch = KC_F12; break;
		case VK_F13: ch = KC_F13; break;
		case VK_F14: ch = KC_F14; break;
		case VK_F15: ch = KC_F15; break;

		case VK_INSERT: ch = KC_INSERT; break;
		case VK_DELETE: ch = KC_DELETE; break;
		/* Backspace is calling both backspace and delete
		   Removed the backspace call, so it only calls delete */
		case VK_BACK: break;
		/* Tab is registering as ^i; don't read it here*/
	        case VK_TAB: break;
		
        /* Ensure that shift-dir works with NUMLOCK on */
        case VK_PRIOR: ch = KC_PGUP; if (has_shift) mods |= KC_MOD_SHIFT; break;
		case VK_NEXT: ch = KC_PGDOWN; if (has_shift) mods |= KC_MOD_SHIFT; break;
		case VK_END: ch = KC_END; if (has_shift) mods |= KC_MOD_SHIFT; break;
		case VK_HOME: ch = KC_HOME; if (has_shift) mods |= KC_MOD_SHIFT; break;
		case VK_LEFT: ch = ARROW_LEFT; if (has_shift) mods |= KC_MOD_SHIFT; break;
		case VK_RIGHT: ch = ARROW_RIGHT; if (has_shift) mods |= KC_MOD_SHIFT; break;
		case VK_UP: ch = ARROW_UP; if (has_shift) mods |= KC_MOD_SHIFT; break;
		case VK_DOWN: ch = ARROW_DOWN; if (has_shift) mods |= KC_MOD_SHIFT; break;

		case VK_CLEAR: ch = '5'; kp=TRUE; break;
		case VK_PAUSE: ch = KC_PAUSE; break;
		
		case VK_SHIFT: has_shift = TRUE; return TRUE;
		case VK_CONTROL: has_ctrl = TRUE; return TRUE;
		
		/* Ensure that ctrl-dir works with NUMLOCK on */
		case VK_NUMPAD9:
            check_ctrl(KC_PGUP)
            break;
        case VK_NUMPAD3:
            check_ctrl(KC_PGDOWN)
            break;
        case VK_NUMPAD1:
            check_ctrl(KC_END)
            break;
        case VK_NUMPAD7:
            check_ctrl(KC_HOME)
            break;
        case VK_NUMPAD4:
            check_ctrl(ARROW_LEFT)
            break;
        case VK_NUMPAD6:
            check_ctrl(ARROW_RIGHT)
            break;
        case VK_NUMPAD8:
            check_ctrl(ARROW_UP)
            break;
        case VK_NUMPAD2:
            check_ctrl(ARROW_DOWN)
            break;
	}
	
	has_shift = FALSE;
	has_ctrl = FALSE;

	/* we could fall back on using the scancode */
	/* obtained using LOBYTE(HIWORD(lParam)) */
	/* see http://source.winehq.org/source/include/dinput.h#L468 */

	if (ch) {
		mods |= extract_modifiers(ch, kp);
		/* printf("ch=%d mods=%d\n", ch, mods); */
		/* fflush(stdout); */
		Term_keypress(ch, mods);
		return FALSE;
	}
	return TRUE;
}

This works with Win7 and an AZERTY keyboard (my tests didn't show any regression when I tried different combinations of keys). More tests with different configurations will be needed, but I think this fixes the problem once and for all.

[fizzix]

I'll try this out on my QWERTY keyboard tonight (or tomorrow).

[fizzix]

It seems your patch produces some undesirable behavior. Pressing shift and then releasing it and then pressing a direction key will cause you to run in that direction. I don't think this should be expected behavior.

[PowerWyrm]

Strange... I don't have the same problem.

The code I use for my variant is slightly different, since handle_keydown for me returns TRUE when the key is handled, and not FALSE (return value is reversed). And the function is called like this:

if (handle_keydown(wParam, lParam)) return 0;
break;

which is not the case in current Angband code (probably incorrect).

[ekolis]

Fizzix, you don't happen to have StickyKeys enabled, do you?

edit: in case that's not it, didn't someone add a "laptop mode" feature to Angband wherein you can move diagonally by pressing two arrow keys in quick succession? Maybe this is involved here?

[david3x3x3]

With this patch I think that has_shift gets set when you press the shift key, but it is not cleared when the key is released without pressing another key. I think it needs to be cleared on WM_KEYUP. In order to do this I think that has_shift needs to be a global variable.

[PowerWyrm]

For me it works because:
- when you press shift-dir with NUMLOCK on, the key is in the range VK_PRIOR-VK_DOWN and has_shift is applied
- when you press SHIFT then dir with NUMLOCK on, the key is in the range VK_NUMPAD1-VK_NUMPAD9 and has_shift is reset

This only happens when pressing shift-dir with NUMLOCK off.

Unfortunately, it seems that WM_KEYUP is called once before WM_KEYDOWN, so clearing has_shift on WM_KEYUP doesn't work...

[PowerWyrm]

The problem is that pressing shift+key triggers the following events:
- keydown (shift)
- keyup (shift)
- keydown (key)
- keyup (key)

So you have no means to check if you pressed shift+key or shift and then a key...

[david3x3x3]

I think that's because Windows is trying to insure that pressing shift-numlock-keypad key has the same effect as pressing the keypad key with no modifiers. This is by design and is probably a good thing. Rather than coding to handle this I'd suggest we either automatically disable num-lock when the app starts or rely on the player setting num-lock to the correct setting.

[PowerWyrm]

I think that's because Windows is trying to insure that pressing shift-numlock-keypad key has the same effect as pressing the keypad key with no modifiers. This is by design and is probably a good thing. Rather than coding to handle this I'd suggest we either automatically disable num-lock when the app starts or rely on the player setting num-lock to the correct setting.

Ah ok... that explains the behavior of keypresses on SDL and GCU clients, so the same behavior should indeed be applied with Windows. NUMLOCK off should be the default and shift-keypad with numlock off should work as expected.

I know TomeNET disables numlock by default, maybe V should do the same...

[fizzix]

I know TomeNET disables numlock by default, maybe V should do the same...

I'm thinking this is the way to go also.

[PowerWyrm]

Ah ok... that explains the behavior of keypresses on SDL and GCU clients, so the same behavior should indeed be applied with Windows. NUMLOCK off should be the default and shift-keypad with numlock off should work as expected.

So the correct behavior should be:
- press keypad with numlock off: virtual key (left, right, down, up, home, end, pgup, pgdn)
- press keypad with numlock on: numeric key (1, 2, 3, 4, 6, 7, 8, 9)
- press shift+keypad with numlock off: shift+virtual key (shift+left, shift+right, shift+down, shift+up, shift+home, shift+end, shift+pgup, shift+pgdn)
- press shift+keypad with numlock on: virtual key (left, right, down, up, home, end, pgup, pgdn)

The game should then ensure than numlock is off and that the virtual keys are mapped to the numeric keys in pref.prf (which should be the case).

[PowerWyrm]

This also implies fixing pref.prf to map the virtual key VK_DELETE to the numeric '.' so that running is possible with numlock off by pressing the game command.

[PowerWyrm]

Hmm what's the expected behavior of control+numpad? Currently, pressing control+numpad does nothing when numlock is on, since the VK_NUMPAD keys are not handled in handle_keydown().

Note that handling VK_NUMPAD keys in handle_keydown() leads to the old bug of double keypresses and should be avoided...

[half]

PowerWyrm,

I just wanted to say thanks for working on this. When you've stabilised on something, I definitely want to incorporate it into Sil. I just don't know enough about Windows to do it myself. I was actually pretty shocked that these things don't work properly on Windows (or Unix) and that it was been decades without it being fixed.