search

giawa / opengl4csharp

OpenGL 4 Bindings (partially based on OpenTK) for C#
Other
234 stars 61 forks source link

SDL Key support in Platform.Input #42

Closed gepbird closed 4 years ago

gepbird commented 4 years ago

It's possible to subscribe a key event to any key. Including special keys like shift, control and alt. It's very similar to the old names, you just add a Raw at the end of the method name and use SDL.SDL_Keycode instead of char as parameter.

This solves the issue https://github.com/giawa/opengl4csharp/issues/41

TheAIBot commented 4 years ago

Looking over the code again i've found more threading issues. Basically any method that uses Keybindings or KeyBindingsRaw are not thread safe. I think this pr is fine if you change subqueueRaw from using a Dictionary to using a ConcurrentDictionary. That's the only change i would recommend right now because technically it's possible for a Dictionary to return values from other keys when multiple threads are at play.

I will make a pr to fix the other threading issues when this pr is merged.

giawa commented 4 years ago

Thanks for working on this. I'll take a look once you have implemented the suggestion from @TheAIBot regarding using a ConcurrentDictionary.

gepbird commented 4 years ago

Looking over the code again i've found more threading issues. Basically any method that uses Keybindings or KeyBindingsRaw are not thread safe. I think this pr is fine if you change subqueueRaw from using a Dictionary to using a ConcurrentDictionary. That's the only change i would recommend right now because technically it's possible for a Dictionary to return values from other keys when multiple threads are at play.

I will make a pr to fix the other threading issues when this pr is merged.

Okay, I changed the KeyBinding's and its raw version's type to ConcurrentDictionary. https://github.com/giawa/opengl4csharp/pull/42/commits/5ed337bfe9b1d07ba2f3426e47eb31c2d52c00b5

TheAIBot commented 4 years ago

@giawa Did you take a look that this yet. I think it looks fine as is right now.