Mac OS command key issues

[tyronx]

Description

As requested by DylanFPS a seperate issue entry for the issues i observed when testing out pull request #759.

• While pressing Command alone a key down event is correctly triggered, it is however not being set as a modifier during a hotkey event such as Cmd+V. This is because KeyboardEventArgs.cs was not patched to include the modifier. My fork has a fix for it

• A new entry to the Key enum has been added. Any game that stored key bindings or uses its own abstracted away key enum will have its compatibility broken from that. I'd suggest moving the Command key entry to the bottom of the list or explicitly numbering the enum entries to retain backwards compatibility

• Using special key combinations such as Command+V trigger a keypress event for 'V'. Obviously the user does not intend to type 'V' in this case. (the windows implementation also does not trigger a keypress in this case)

• The IsARepeat flag for the command key (and also the shift and ctrl key) is set using the keyboard state of the left alt key. While technically incorrect, this may be irrelevant as the command/ctrl/alt keys do not have a repeat function (or is it just missing?)

[leezer3]

With regards to the second issue, as noted in the patch discussion, this only affects those using the underlying enum value.
I'd argue that grouping the key in the 'expected' place in the enum is more correct than placing it at the end.

Also, 4.0 is already a mass of breakages, and if I remember has been specifically declared to not be backwards compatible?

Easy to move, but need maintainer input as to where we want it :)

---

The first can just be PR'd in, thanks (either you or me is fine)

---

Checking for 'special' key combinations I think is going to be a pain in the neck.
Anyone with clever ideas is more than welcome to share :P

[Perksey]

A new entry to the Key enum has been added. Any game that stored key bindings or uses its own abstracted away key enum will have its compatibility broken from that. I'd suggest moving the Command key entry to the bottom of the list or explicitly numbering the enum entries to retain backwards compatibility

Lol 4.0 is so breaking that this'll be the least of the developer's worries.

I think we also need to discuss how we'll go about this. At the moment (as I understand it), we're sending keypress events for Control and V where Ctrl+V is used. This is a slippery slope, and there's multiple ways we can integrate this. For example:

• We could add some extra properties like IsControlKeyDown and similar to the key press event.
• We could let the user deal with the above
• We could alter the key press event so that multiple keys can be sent at once
• ...and many other angles that this issue can be tackled.

I think my first option is pretty neat, but I'm open to suggestions as always :)

[varon]

Lol 4.0 is so breaking that this'll be the least of the developer's worries.

We're okay with breaking changes, but the issue is that this is a really subtle change that's not discoverable at all. If we want to keep it, someone needs to start a migration guide make explicit numbered references to things like this. A good heuristic for 'if it's okay to break stuff' is "Will this produce a compile error when someone migrates?". It makes it obvious, discoverable, and most importantly fixable.

@tyronx Already lost about 4 hours of work apparently due to the change. A very common problem I imagine people would encounter is if they've serialized a keybinding map to disk using an earlier version of OpenTK; they'd either need to write upgrade code or break their users setup.

To the end user, there's zero benefit to placing it higher up in the list.

For everything else suggested by @tyronx - It's great feedback, thanks!

@DylanFPS - The first is pretty nice for the event driven approach.

[Perksey]

Yeah I think we should go with the first option I suggested and, yeah, we should make sure that the enum positions are correct to ensure compatibility

[varon]

(Should be) Fixed by 4.x