Page 1 of 1

keyboard mappings bugs

Posted: Mon Nov 05, 2012 2:55 am
by maze
I found 3 bugs with keyboards mappings. They are actually pretty minor except for the last one.

1) Shall you map "q" to an action different from the default one, by the harsh mistress shall you be mislead.

2) If you map a key other than "M" to the "Cockpit::MapKey" action, pressing the key will result in what is shown in the attached file. If it showed your actual system map, or the path to the selected target, that would actually be feature! But since it shows always the same thing, it's a bug.

3) the ctrl modifier does not work for me.

Re: keyboard mappings bugs

Posted: Mon Nov 05, 2012 5:16 pm
by loki1950
And this is under what OS :?:

Enjoy the Choice :)

Re: keyboard mappings bugs

Posted: Mon Nov 05, 2012 7:48 pm
by maze
I'm using GNU/Linux (Debian testing).

Re: keyboard mappings bugs

Posted: Wed Nov 07, 2012 4:13 am
by pheonixstorm
I think I remember a bug with the ctrl key but thought it was under OSX not linux.

Solution to 1.. don't allow remapping q or change the text from static to dynamic and take the key commands off the mistress screen. You could add ; to 1 as well since ; is reload and q is quit.

Re: keyboard mappings bugs

Posted: Wed Nov 07, 2012 5:56 am
by klauss
I think 1 is expected. There's no in-game key remapping anyway, so if you change Q you already know what you're doing.

The ctrl thing I've noticed. Not sure how to fix it, but I have a ticket for it waiting for some attention.

Re: keyboard mappings bugs

Posted: Wed Nov 07, 2012 1:17 pm
by pheonixstorm
If ctrl isn't working check the SDL forums and see if anyone else is having that issue. What else do we use SDL for besides Joystick, mouse, and keyboard handling?

Re: keyboard mappings bugs

Posted: Wed Nov 07, 2012 4:37 pm
by klauss
SDL also sets the GL context, which in windows is a big deal (I remember all the hoops one had to jump through to get a GL context back when I coded in windows)

Re: keyboard mappings bugs

Posted: Wed Nov 07, 2012 8:50 pm
by maze
klauss wrote:I think 1 is expected. There's no in-game key remapping anyway, so if you change Q you already know what you're doing.

The ctrl thing I've noticed. Not sure how to fix it, but I have a ticket for it waiting for some attention.
Oh and regarding 2), I realize I was simply confused. The actual action is Cockpit::NavScreen, and this one works. Cockpit::MapKey might be leftover or whatever, but it is commented out in the default config file.

Re: keyboard mappings bugs

Posted: Sat Nov 10, 2012 5:09 am
by maze
klauss wrote:The ctrl thing I've noticed. Not sure how to fix it, but I have a ticket for it waiting for some attention.
Rather than "not sure how to fix it," you should have written "not sure how I've fixed it," because you did. My ctrl key works using the development version, as I've just pulled it from svn.

However... any idea how and why it could be that my joystick does not work with the development version? Maybe you know after all. If none of you devs can tell, I'll try to pinpoint the svn commit which broke it and I'll let you know what I find.

(I'm trying the development version under gdb, that's because I've found a course of action which basically makes me always reproduce a segfault within a few minutes; that's when I select and destroy subunits of an agesipolis; it always ends up segfaulting before I'm finished with its turretpds)

Re: keyboard mappings bugs

Posted: Sat Nov 10, 2012 6:14 am
by klauss
maze wrote:
klauss wrote:The ctrl thing I've noticed. Not sure how to fix it, but I have a ticket for it waiting for some attention.
Rather than "not sure how to fix it," you should have written "not sure how I've fixed it," because you did. My ctrl key works using the development version, as I've just pulled it from svn.

However... any idea how and why it could be that my joystick does not work with the development version? Maybe you know after all. If none of you devs can tell, I'll try to pinpoint the svn commit which broke it and I'll let you know what I find.
Frankly, we haven't touched any of that code since the 0.5.1 release. Probably even before that.

Re: keyboard mappings bugs

Posted: Mon Nov 12, 2012 1:21 am
by maze
klauss wrote:
maze wrote:
klauss wrote:The ctrl thing I've noticed. Not sure how to fix it, but I have a ticket for it waiting for some attention.
Rather than "not sure how to fix it," you should have written "not sure how I've fixed it," because you did. My ctrl key works using the development version, as I've just pulled it from svn.

However... any idea how and why it could be that my joystick does not work with the development version? Maybe you know after all. If none of you devs can tell, I'll try to pinpoint the svn commit which broke it and I'll let you know what I find.
Frankly, we haven't touched any of that code since the 0.5.1 release. Probably even before that.
Err... Right... I could see that finally. In fact nothing changes from 0.5.1.r1 to r13439 in terms of control key and joystick status.

Nevertheless some progress has been made:
In fact the (see above) difference I was experiencing between the released binary of 0.5.1.r1 and the compiled-at-home development version was purely a compile issue, due to the fact that the SDL headers were not present on my system. Since there's a fallback on GLUT, they aren't strictly a requirement and thus not listed as such here. The configure script evaluates that by compiling a test program, but tell me, who reads those little configure warnings anyways? It's lucky I don't, actually, because it's been giving me a lead on the issue I'm having with the control key.

Hence the situation remains the same from 0.5.1.r1 to r13439, and is the following:
  1. compilation with HAVE_SDL defined:
    • joystick working
    • control modifier not working (SDL failure)
  2. compilation with HAVE_SDL not defined:
    • joystick not working (GLUT fallback failure)
    • control modifier working (presumably through GLUT, but haven't read the code yet)
I'd like to understand more what causes the control key to be ineffective, and to propose you a patch if my programming skills let me.
At the very worse, should there be an SDL bug impossible to work around from the higher level, we know that there is a working fallback that can be used...
Given that I now have something to bite on, I expect to sort this out completely within reasonable time. I'll let you know within a week from now.

Re: keyboard mappings bugs

Posted: Wed Nov 21, 2012 12:52 am
by maze
Some progress has been made by your servitor on this topic.
gdb is my friend.

It appears that forcing is_unicode to false just after line 447 of winsys.cpp restores the function of the control key.
By the same token, disabling unicode in the config file also restores the correct function of the control key.

This being said, I can't say I've reached full understanding yet.

Here's where I am.
The decisive mishaps leading to the bug ought to happen in winsys.cpp, in the keepRunning loop.

When the modifier is none or alt, is_unicode tests false and *keyboard_func) gets called with the intended keysym integer as its parameter, the keysym in question being the ASCII code corresponding to the letter that was pressed on a US layout keyboard...

When the modifier is control, and the key is an ASCII character, is_unicode is triggered for a reason which I have yet to figure out. An incorrect keysym is fed to *keyboard_func. When the key pressed is a key corresponding to a lowercase letter on a US layout keyboard, it seems that this incorrect value corresponds to the order number of said lowercase letter in the alphabet.

Why and how the status of the control key would impact the keysym_to_unicode array for a totally different key still has to be investigated by your servitor.

Re: keyboard mappings bugs

Posted: Wed Nov 21, 2012 4:11 am
by klauss
maze wrote:Some progress has been made by your servitor on this topic.
gdb is my friend.

...

Here's where I am.
The decisive mishaps leading to the bug ought to happen in winsys.cpp, in the keepRunning loop.

When the modifier is none or alt, is_unicode tests false and *keyboard_func) gets called with the intended keysym integer as its parameter, the keysym in question being the ASCII code corresponding to the letter that was pressed on a US layout keyboard...
Amazing word maze! (wow... puntastic).

I managed to find the problem with your detective work. I'll be committing the fix any time now :D :D :D :D

Re: keyboard mappings bugs

Posted: Thu Nov 22, 2012 1:59 am
by maze
After your commit it nearly works. Keys modified by control are indeed not ignored anymore.
However, all the ASCII characters between 0x00 and 0x1a get translated into alphabetic characters... which should probably be considered a serious issue given that you have to kiss Backspace, Tab and Carriage_return goodbye, be it in flight or in the save/load dialog.

Try this if you don't mind:
svn diff

Code: Select all

Index: src/gldrv/winsys.cpp
===================================================================
--- src/gldrv/winsys.cpp        (revision 13448)
+++ src/gldrv/winsys.cpp        (working copy)
@@ -441,7 +441,7 @@
                     bool is_unicode = maybe_unicode && event.key.keysym.unicode;
                     
                     //Fix up ctrl unicode codes
-                    if (is_unicode && event.key.keysym.unicode <= 0x1a)
+                    if (is_unicode && event.key.keysym.unicode <= 0x1a && (event.key.keysym.sym&0xFF) > 0x1a)
                         event.key.keysym.unicode += 0x60; // 0x01 (^A) --> 0x61 (A)
                         
                     //Translate untranslated release events
(the diff is from your very last commit)

Still not completely working, since you won't be able to modify ASCII characters between 0x00 and 0x1a with the control key. I wouldn't consider that a major drawback since, unless I'm mistaken, such combinations do not exist in the default bindings. I don't know if you'll find it useful to add support for this corner case, but I guess it could be done (haven't thought it through though).

Other than this, the above seems to work as intended.

P-S: sorry I don't know how to formally submit a patch, I believe the process described in the Code contribution forum announcement is broken as of today.

Re: keyboard mappings bugs

Posted: Thu Nov 22, 2012 3:30 pm
by klauss
maze wrote:After your commit it nearly works. Keys modified by control are indeed not ignored anymore.
However, all the ASCII characters between 0x00 and 0x1a get translated into alphabetic characters... which should probably be considered a serious issue given that you have to kiss Backspace, Tab and Carriage_return goodbye, be it in flight or in the save/load dialog.
Argh... You're right.
maze wrote: Try this if you don't mind:
svn diff

Code: Select all

Index: src/gldrv/winsys.cpp
===================================================================
--- src/gldrv/winsys.cpp        (revision 13448)
+++ src/gldrv/winsys.cpp        (working copy)
@@ -441,7 +441,7 @@
                     bool is_unicode = maybe_unicode && event.key.keysym.unicode;
                     
                     //Fix up ctrl unicode codes
-                    if (is_unicode && event.key.keysym.unicode <= 0x1a)
+                    if (is_unicode && event.key.keysym.unicode <= 0x1a && (event.key.keysym.sym&0xFF) > 0x1a)
                         event.key.keysym.unicode += 0x60; // 0x01 (^A) --> 0x61 (A)
                         
                     //Translate untranslated release events
I added something to your patch and committed. Try it out now.

Re: keyboard mappings bugs

Posted: Fri Nov 23, 2012 12:45 am
by maze
I just tried and it looks good. I'm going to declare the thread fixed just after posting this.

This being said, I just briefly tried a session with a french keyboard layout and enable_unicode set, let's say the result was far from brilliant... I can't say though if we've made things worse than they're supposed to be, being that IIRC non-us layouts were never supported.

Last remark: if one were to rewrite the whole keyboard event handling code, for example as part of an attempt to bring support to non-us layouts, I wonder if the best approach wouldn't be to write straightforward code in winsys.cpp, and repel the inevitable hackish corner-caseish stuff in config_xml.cpp.

For one, it would reduce the clutter in a loop that is constantly gone through in real time, and place it in code that is only executed once at startup.

I've googled a little bit and I understand that libSDL has some pretty severe limitations when it comes to internationalization, but it seems that if we'd ever manage to parse a correct [modifier][unicode] array at startup, then we'd be good. By this I mean that making it work on one particular layout would pretty much guarantee that it works under all layouts.

However right now as of today I'd like to take a look at this torpedo bug 3552768 which is bothering me quite a bit when I play and see if I can do something, who knows... Hopefully I'll come back in a few days with some clues or some code.

Re: keyboard mappings bugs

Posted: Fri Nov 23, 2012 12:54 am
by klauss
Actually, the real solution is what's said somewhere around that code in a comment: OIS