Change: Make Command a valid modifier on OS-X

[leezer3]

Purpose of this PR

• Adds the Command key to be a valid key modifer.
• OS-X native window only.

Testing status

• Untested- I've got no physical hardware, but assuming the NSEventModifierMask for Command is correct it's a simple change. See below.

Comments

• This is Mac native platform specific only. This may make it too narrow to be merged into the main library, but on Mac, the Command modifier is the standard, and hence what users expect......

#757

[leezer3]

Hmm, just thought of a (mild) flaw in this :/
https://github.com/opentk/opentk/blob/develop/src/OpenTK/Input/Key.cs
Our key enum contains no definition for the Command key. As we're directly pulling the flags when one or more keys other than Command are pressed this doesn't (or at least shouldn't) matter.
However, when the command key is pressed on it's own, it'll be mapped here https://github.com/opentk/opentk/blob/develop/src/OpenTK/Platform/MacOS/MacOSKeyMap.cs#L243 to the Left Windows key.

Now, the obvious answer to that is just to add it to the enum, but I don't particularly like adding something at the start of the enum (Within the existing modifiers section) when it'll be a nightmare to check that our codebase isn't using the int representation for a key somewhere, let alone what third parties may or may not be doing......

Need some input from the maintainers here:
It's easy enough for me to add the enum value and do some basic testing on Windows and Linux, but this is an area where an edge case can easily explode on us.
So, is this worth doing (IMHO yes), and how do we go about it?

[leezer3]

The second commit adds the Command key to the enum, and actually makes this work.

Still need some maintainer input as to whether this is the best route mind :)

Testing status:

• OS-X: High Sierra VM, standard Windows keyboard (Logitech) appears to return all keys correctly, including the new Command key.
• Windows: All keys appear to work correctly.

Still concerned about any edge cases though. Whilst I can't think of any off the top of my head, the Key enum is used everywhere in the keyboard handling, and is a perfect candidate for biting us.
Need some other people to test this!

[Nihlus]

Thank you for you contribution! Currently, the develop branch is frozen while we work on the 4.0 release, and we are not accepting pull requests to it. All current pull requests should be based on that branch, or should be updated to be based on it.

In order to avoid clutter in our pull requests, we're going to be closing any pull request that isn't targeting 4.0 on the 31st of July, 2018. Please rebase your pull request before then.

[leezer3]

Rebased & squelched into a single commit.

[Nihlus]

Is the use of the alt left key here intentional?

[leezer3]

It's intentional, copying the pattern from those above.

Modifiers on OS-X / Cocoa don't generate a keydown state event per-se, but rather are passed as flags in a window event message (Including ALT).
If I understand the intent of the original code correctly, the virtual LeftAlt key is being used to define that any given modifier was valid at the last keypress event, and the modifiers flags are used to determine which.

[tyronx]

That.... doesn't seem to make much sense to me. Why would a modifier not be valid? Also this method seems only called by the cocoanative window and I still cant get CMD+V to work. This seems like a typo to me tbh, and the reason why CMD+V doesnt work. Testing now...

[leezer3]

Because a modifier isn't a key & generates no events.

We can therefore hold one key and a modifier. Release the modifier while holding the key & press another.
The first is now invalid, but no keypress events have been generated.

If this don't make sense it's because its from the phone....

[tyronx]

Oh, but I just realize that this second argument just sets the KeyDownArgs.IsRepeat field to determine if its a key repeat event when holding down the key for a longer time.

The first is now invalid, but no keypress events have been generated.

Keypress events do not consider the modifiers anyway though (at least not OpenTK, the Operating system does). Only the keydown events pass on the modifiers. And opentk registers any cocoa keydown/keyup events so even your case should be handled correctly.

[leezer3]

It's marginally crappy code, but rehacking the keypress events for the native window isn't something I'm going after on my own.

[leezer3]

It's intentional, copying the pattern from those above.

Modifiers on OS-X / Cocoa don't generate a keydown state event per-se, but rather are passed as flags in a window event message (Including ALT).
If I understand the intent of the original code correctly, the virtual LeftAlt key is being used to define that any given modifier was valid at the last keypress event, and the modifiers flags are used to determine which.

[Nihlus]

Alright, seems fine. I'll merge it, and we can test it as a part of the general 4.0 overview.

[tyronx]

Argggh, this took me like 4 hours to figure out.
I tried your patch on my fork. This one enum entry broke all keyboard inputs on mac os for my game because it caused all subsequent enum indices to be shifted by 1 which caused my game internal keycode representation to be incorrectly mapped.
This is not a bug per se but perhaps its a good idea to move this enum entry to the very bottom for backwards compatibility?

[leezer3]

Dunno.

I would have thought most people are using the actual enum values, rather than the index directly?
It was deliberately added there so it was grouped with the other OS-X modifiers rather than elsewhere.

You've absolutely proved my point about this biting us though!

[tyronx]

I have an abstraction layer to abstract away OpenTK itself so I can switch to another graphics library / toolkit should the need ever arise, hence the mapping

[tyronx]

Hokay, more tests revealed that the keydown event for the command key itself is fired and works.

But when you do a key combination like Command + V the KeyboardKeyEventArgs for that keydown event does not have the Command modifier flag set to on because you forgot to patch KeyboardKeyEventArgs.cs

[tyronx]

Found even more bugs. Using Command+V also triggers a Keypress event for 'V', which it shouldn't.

Judging by the lack of information on that in the online apple cocoa documentation we have to literally hardcode those special cases.

The windows implementation handles this by listening to a CHAR event so neatly does the work for us when handling hotkeys (as in, not trigger a keypress when such shortkey is used).

[Perksey]

@tyronx Should report those bugs in a new issue ;)