[crossfire] Key repeat and keybindings

[Arvid Brodin]

On 2013-10-24 07:19, Mark Wedel wrote:
> On 10/23/13 06:53 PM, David Hurst wrote:
>> Hi Arvid,

Hi!

[]
> 
> A lot of this has to down how keys are handled, at least on
> x-windows.
> 
> If you could down the control, shift, or other modifier keys, a
> single event is generated - that the key has been pressed, and
> another event is generated when the key is released.  This is all
> done in the x-server - at a level beyond the client.
> 
> For other keys, autorepeat kicks in.  So if you hold an arrow key,
> the x-server is generating numerous key press/key release events,
> which the client then sees and takes as different key presses.  Best
> I know, there isn't an easy way to know if multiple key presses are a
> result of autorepeat or if the player is actually pressing and
> releasing the key (as a side note, for myself, for things like
> search, I would just hit the s key 5 times to know how much I'm
> searching)

I did a test of this. In the client key press events are handled by keyfunc() and key releases are handled by keyrelfunc() (both in gtk-v2/src/keys.c). They are not x-server events but rather GDK events. I added a simple printf() at the beginning of each to see what kind of events are generated for different keys.

For Ctrl, the result is as you say:

key pressed: Control_L
<held key generates no new events>
key released: Control_L

For auto-repeating keys though, there are no release events between the key press events:

key pressed: KP_2
key pressed: KP_2
key pressed: KP_2
key pressed: KP_2
key pressed: KP_2
key pressed: KP_2
key pressed: KP_2
key released: KP_2

So keeping state for each button would be easy. It's also easy to see if a key is actually pressed several times vs being held down. (Maybe focus in/out can make things a bit more difficult, but not a lot, I guess.)


> Given this background, run and fire are handled differently, since
> these are modifiers - when the client gets that the key is pressed,
> it just sends the appropriate fire_on or run_on to the server, and
> when key is release, send fire_off/run_off.

And here of course lies the problem: which commands should be of the type start/stop, and which should be of the type "do once"? It would be pretty bad if one sent a command to drink a potion and the PC drank _all_ potions of the type because the key was held too long. But other commands - search, disarm, run, maybe fire? - may be better suited as start/stop?

It could be as simple as adding a few new start/stop type commands to the server - walk (without attacking), search, and disarm, to begin with? - and updating the client to use them. Assuming the client key handling is in place, of course.


----

I've done some coding on the other, related issue of key combinations/modifiers and key bindings. 

Today the client is extremely flexible: you can actually re-bind the run, fire, alt and meta modifier keys! Is this a popular feature?

Also, it is unclear how combinations of modifiers work: if you check both Run and Fire in the Keybindings dialog, does the key work only when they are both depressed, or with any of them? If you select no modifiers, does the key then work regardless of modifier keys or only when no modifier keys are held?

The code is a bit ambivalent on this, it seems (maybe this works systematically by chance, but I found at least one obvious mistake in the code, as well as code comments leading me to this conclusion).


I reworked the code a bit and added an "Any" checkbox in the Keybindings dialog. This way you can choose whether the key should ignore modifiers, or set the same key to different commands depending on the combination of modifier keys held.

At the same time I removed the code that treats default key bindings specially, so now you can easily unbind/change them in the dialog box or using the unbind command. (You could do this via the "unbind -g" command before, but how would you know without looking at the code?) 

I also removed the possibility to re-bind the Ctrl, Shift, Alt and Meta keys since that just seems overly complex.

I can post the patch if anyone is interested. It also contains quite a few style fixes though (mostly remove space before function '(', space after comma, space around '='/'==' etc). I tried to use the most prevalent style already in use in the file, but I should probably have done it in a separate patch...



-- 
Arvid