search

quadra-game / quadra

An addictive action puzzle game with single player and multiplayer capabilities (Internet or LAN).
GNU Lesser General Public License v2.1
28 stars 18 forks source link

Input code/design is awful #125

Closed pphaneuf closed 10 years ago

pphaneuf commented 10 years ago

It should be clear when the games wants keycodes or wants keysyms. The skelton shouldn't have big ugly translation tables like in skelton/svgalib/input_x11.cpp, this stops us from porting to other "easy" platforms like Solaris or whatever, where the keycode are different (and believe me, they are).

It would be nice to change the "Linux" to "POSIX" in the Trove categorization...

pphaneuf commented 10 years ago

Should this code cleanup goes only in SDL version branch? I would suggest so.

pphaneuf commented 10 years ago

On general principles, code should go only go on trunk if there is a very good reason not to put it in the SDL branch.

pphaneuf commented 10 years ago

Uh, there's an extra word in there, sorry!

What I meant is this: most of the work should be going on quadra-sdl, only what's needed for the soonish stable release should go on trunk (for example, fixing a bug that would prevent Quadra from working on Vista).

Stuff like this issue is definitely quadra-sdl material. :-)

pphaneuf commented 10 years ago

This issue is irrelevant: our input subsystem is correctly using SDLKey which is multi-platform and pre-translated. There ain't no ugly translation table nowhere to be found. Thus Quadra should detect and display key names appropriately on all SDL- supported platforms. I have fixed a small bug allowing the Escape key to be used in player key config.

pphaneuf commented 10 years ago

There's more than one level of translation, unfortunately, and the ickiness is quite present. The keycodes have been standardised on those of SDL, so that translation table is now gone (that issue was pretty old, I did fix that), but the confusion between keycodes and keysyms is still present.

We're using our own Input::keys array, instead of the SDL_GetKeyState, but note that we use the keysym as the index instead of the keycode, as it would make sense. When we're interested in event-based keysym input, we use the last_key and basically hope that people don't type too fast. The issues might not be there on Windows or OS X, I'm not sure, but I remember that dead keys gave me quite some grief on Linux (see the "unicode" stuff).

I think that the input during the game itself is probably okay, but could be use SDL_GetKeyState. For input fields, though, whoa, it's wacky. We basically assume that every character value that we get has a virtual key, because we don't make the difference between keys and input characters. For example, if you can point me at the ò key on your keyboard, or tell me which character is produced by pressing the Alt key... ;-)

pphaneuf commented 10 years ago

Our own input->keys[] array could be replaced by direct call to SDL_GetKeyState, this shouldn't be hard but isn't necessary either. It's up to us, but I might give it a shot. The fact that we are using keysyms instead of hardware scancodes as index isn't that much of an issue, but then again this would be removed by using SDL_GetKeyState.

Input field are perfectly working. They are using our own keyboard buffer and this buffer keeps unicode translated characters (along with SDL key syms, this is something I added recently to allow cursor movement inside a text field). So the ALT key you mention does NOT push a character in an input field since it returns an ASCII value of 0, and any meta-accentued-whatever-composed character are correctly handled. By the way there might be no 'ò' key on your keyboard, but my Mac has a 'ù' key and it works. * Note: only ASCII character less than 256 are handled, our font doesn't support more than that anyway.

last_key isn't exactly 'great' I admit, but it is used to process special things like Escape or TAB to move keyboard focus. We really don't care if the user pressed Escape twice in a single frame tick so this mecanism is up to its designed task. The only time 'people can type fast' is inside an Input Field, and as I said we already implement a 32-characters key buffer. If you find someone who can type over 32 characters in 1/100th of a second then we would have to file a bug request ;)

pphaneuf commented 10 years ago

Ok, I have worked a bit on this issue and fixed some things in revision: http://code.google.com/p/quadra/source/detail?r=466 and http://code.google.com/p/quadra/source/detail?r=468

So basically everything is working correctly, at least on Windows and Mac OS. However, removing our 'input->keys[]' array is a big no. The game input system relies on this array for adequate detection of key release, continuous down feature, etc. Replacing it with tons of polling with SDL_GetKeyState() would be hard if even possible, along with being suboptimal. (Since our key array is event-driven).

So I would recommend leaving our Input system as is, unless of course new issues are detected on specific platforms.