[crossfire] Key repeat and keybindings

[Mark Wedel]

On 10/25/13 04:59 PM, Arvid Brodin wrote:
> 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.)
>

  Interesting.  So I'm guessing that gtk must capture some of the events.  If I
run xev, I do see key press/key release events for the keys.

  But even in the above example, it potentially means a more complicated setup,
in that the client (or something) has to keep track if the release events has 
occurred.

  Note that focus can mess things up.  Start running and then have the client 
lose focus - your character will continue to run, even after you release the 
keys.  However, this is more a special case I'm not too concerned with.

>
>> 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'd really prefer this not be in the server, and instead be in the client. 
the fire_on and run_on are really pretty terrible hacks as is, and I'd rather 
not add more of them.

  The protocol has improved since those were put in.  The client should be able 
to better watch the number of acknowledge commands and send or not send commands 
as appropriate.  Note per my previous mail, the next enhancement that would be 
nice would be for the server to look at all commands, and in addition to having 
priority, there could be something like 'cancel are north commands', such that 
when the key is released, that is sent to the server and the server does just 
that.  That said, even this gets tricky - you don't want to cancel the a single 
command that was sent, which might mean the client has to know if multiple 
commands have been sent, and which ones.

  This reduces the server having to remember more state.

  In terms of what commands should repeat and what should not, this could 
probably be an option in the keybind menu, eg, 'repeat?' which notes if the 
command should be repeated or not.  Probably also allow it in the keybindings 
itself, because for some commands, there may be desire to repeat them in certain 
circumstances and not other (apply immediately comes to mind in certain cases, 
like eating a lot of low value food, but not in others, like using exits)

>
>
> ----
>
> 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?

  Not sure, although I could imagine depending on keyboard layout, etc, there 
may be desire to do so.

>
> 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.

  Yes, that seems like a good fix.

>
> 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...

  I'd certainly say it is worth it to post it or upload it to sourceforge on the 
tracker.