Bug 886

Summary: mouse vs keyboard binds
Product: Odamex Reporter: JKist3 <jkist3>
Component: Server & ClientAssignee: Odamex Bug Reporter <odamex-bug-reporter>
Status: RESOLVED FIXED    
Severity: major CC: grandpachuck187, ijgjgr
Priority: P1    
Version: 0.6.x   
Hardware: All   
OS: All   
Attachments: WIP (dirty hack to repeat mousedown events)
Disable keyrepeat unless chatting or in console

Description JKist3 2012-07-29 06:00:51 UTC 
When holding down a key on your keyboard, odamex calls it over and over. see bug 470:

http://odamex.net/bugs/show_bug.cgi?id=470

However, when holding down a mouse button, odamex only calls the action once.  It should repeatedly call the action just like on the keyboard.
Comment 1 Amateur Spammer 2012-07-29 06:24:12 UTC 
What did you bind to the mouse button that it needs repeating?
Comment 2 JKist3 2012-07-29 07:14:50 UTC 
Im helping some other zdaemon players play odamex. And one of them has an sr50 bind on mouse2, that fucks up due to the current behavior.  Either way the mouse buttons and keyboard buttons should behave the same to avoid this and other problems.
Comment 3 Amateur Spammer 2012-07-29 08:12:40 UTC 
Okay, can u explain what exactly doesn't work?

alias "+sr50" "+forward; +strafe; +moveright; +right"
alias "-sr50" "-forward; -strafe; -moveright; -right"

and then binding it seems to work for me.
Comment 4 Amateur Spammer 2012-07-29 18:21:52 UTC 
[2012.07.29, 21:46:26]	JKist3	so its not just sr50
[2012.07.29, 21:46:32]	JKist3	regular keys repeat actions
[2012.07.29, 21:46:35]	JKist3	mice keys dont
[2012.07.29, 21:46:46]	JKist3	mice keys should repeat as well
[2012.07.29, 21:47:00]	HeX9109	you mean if you fire with mouse1 it doesnt rapid fire?
[2012.07.29, 21:47:11]	JKist3	no it does...
[2012.07.29, 21:47:14]	JKist3	just do this
[2012.07.29, 21:47:29]	JKist3	bind "k" "echo keyboard"
[2012.07.29, 21:47:44]	JKist3	bind "mouse2" "echo mouse"
[2012.07.29, 21:47:49]	JKist3	and u will see the issue
[2012.07.29, 21:47:57]	Ralphis_	try using + on the command?
[2012.07.29, 21:47:59]	Ralphis_	+ and -
[2012.07.29, 21:48:01]	JKist3	mouse and keyboard buttons behave differently and this is not good
[2012.07.29, 21:48:24]	JKist3	yeah ralphis thats not really the issue
[2012.07.29, 21:48:31]	JKist3	i submitted a bug report on it last night
[2012.07.29, 21:48:57]	mnbvzxc	but why is it needed
[2012.07.29, 21:49:13]	JKist3	http://odamex.net/bugs/show_bug.cgi?id=886
[2012.07.29, 21:49:28]	JKist3	mnbvzxc do you actually code odamex at all
[2012.07.29, 21:50:37]	mnbvzxc	nope
[2012.07.29, 21:51:31]	JKist3	well its important to have mouse and keyboard keys behave the same so ppl dont get confused
[2012.07.29, 21:51:43]	JKist3	liek if you bound something to mouse2
[2012.07.29, 21:51:53]	JKist3	ud expect it to function the same way as if u bound it to k right
[2012.07.29, 21:54:29]	mnbvzxc	yes, but the keyboard repeating is nod handled in odamex code, odamex just requests the key to be repeated from SDL
[2012.07.29, 21:54:48]	mnbvzxc	and btw it was me who talked with you on the bug
[2012.07.29, 21:56:45]	mnbvzxc	can be done of course, would be nice to have, but u still haven't explained how it breaks sr50
[2012.07.29, 21:58:54]	JKist3	it depends on how u bind it
[2012.07.29, 21:59:07]	Ralphis_	why don't you show how you are trying to bind it
[2012.07.29, 21:59:10]	Ralphis_	on the bug
[2012.07.29, 21:59:17]	-->|	TheSpider (~TheSpider@pc-163-72-47-190.cm.vtr.net) has joined #odamex
[2012.07.29, 21:59:24]	JKist3	Ralphis this is not for me u know...
[2012.07.29, 21:59:32]	JKist3	hang on ill show u
[2012.07.29, 21:59:53]	Ralphis_	well then have the person who you are helping explain their bind
[2012.07.29, 21:59:59]	JKist3	alias "+sr50l" "+strafe\;+left\; echo +sr50l"
[2012.07.29, 22:00:00]	JKist3	alias "+sr50r" "+strafe\;+right\; echo +sr50r"
[2012.07.29, 22:00:01]	JKist3	alias "-sr50l" "-strafe\;-left\; echo -sr50l"
[2012.07.29, 22:00:02]	JKist3	alias "-sr50r" "-strafe\;-right\; echo -sr50r"
[2012.07.29, 22:00:21]	JKist3	bind "mouse2" "+sr50r"
[2012.07.29, 22:00:28]	JKist3	bind "s" "+sr50l"
[2012.07.29, 22:00:31]	Ralphis_	you should put that in the bug (unless you already have today)
[2012.07.29, 22:00:52]	JKist3	no i didnt... but its really irrelevant... i mean there are many other binds that could do something totally different
[2012.07.29, 22:00:54]	JKist3	and still fuck up
[2012.07.29, 22:01:05]	JKist3	it doesnt have to be sr50
[2012.07.29, 22:01:09]	Ralphis_	except it gives the team a way to test exactly your problem
[2012.07.29, 22:01:16]	JKist3	sure
[2012.07.29, 22:01:20]	Ralphis_	so it is useful
[2012.07.29, 22:01:39]	Ralphis_	what is the echo for
[2012.07.29, 22:01:50]	JKist3	so show you whats happening
[2012.07.29, 22:02:05]	JKist3	has no in game behavior
[2012.07.29, 22:02:13]	JKist3	just prints to the console which command is being called
[2012.07.29, 22:02:28]	JKist3	which is why i said
[2012.07.29, 22:02:29]	JKist3	12:47 <JKist3>	bind "k" "echo keyboard"
[2012.07.29, 22:02:30]	JKist3	12:47 <JKist3>	bind "mouse2" "echo mouse"
[2012.07.29, 22:02:39]	JKist3	that will show u the problem clearly
[2012.07.29, 22:02:46]	JKist3	if u just test that out in a game
[2012.07.29, 22:03:19]	mnbvzxc	aha, so when you want to switch from left sr50 to right, and first press the mouse button, then release "s", it doesn't work?
[2012.07.29, 22:03:41]	JKist3	yeah it starts turning u in a circle
[2012.07.29, 22:03:51]	mnbvzxc	i have an idea
[2012.07.29, 22:04:18]	Ralphis_	is this even something that can be fixed prior to sending the input to sdl
[2012.07.29, 22:04:33]	JKist3	i have no clue...
[2012.07.29, 22:04:42]	JKist3	all i know is it u hold down a keyboard key
[2012.07.29, 22:04:48]	JKist3	the action gets sent over and over and over again
[2012.07.29, 22:04:53]	JKist3	with a mouse it only gets sent once
[2012.07.29, 22:05:20]	mnbvzxc	er, ralphis. Input is what odamex *gets* from sdl
[2012.07.29, 22:05:23]	JKist3	this bind works on zdaemon.... but not on odamex bc of this behavior
[2012.07.29, 22:05:42]	mnbvzxc	and as I said, I have an idea
[2012.07.29, 22:05:45]	mnbvzxc	let me try
[2012.07.29, 22:07:36]	Ralphis_	that's what I meant
[2012.07.29, 22:07:45]	Ralphis_	I'm off my rocker atm
[2012.07.29, 22:09:02]	JKist3	is there any reason why mouse commands cant be repeated?
[2012.07.29, 22:09:12]	JKist3	you had submitted the original bug for the keyboard repeating ralphis
[2012.07.29, 22:09:55]	Ralphis_	If there is a reason why it couldn't be repeated, my gut tells me it would be an sdl limitation
[2012.07.29, 22:10:06]	Ralphis_	but I don't have the technical background to be able to answer that off the top of my head
[2012.07.29, 22:10:15]	JKist3	yeah
[2012.07.29, 22:10:22]	JKist3	hpefully its just an easy fix
[2012.07.29, 22:19:25]	|<--	JohnZombie has left quakenet (Quit: Quitting)

sorry for the badly formatted log
Comment 5 Amateur Spammer 2012-07-29 19:27:17 UTC 
Created attachment 391 [details]
WIP (dirty hack to repeat mousedown events)

Piece of shit, saved for posterity!
Repeats events probably every tic, didn't check yet
Code is repetitive and duplicates some other logic from the same function.
Input while not in focus, when the mouse is off, etc. needs to be tested.
Comment 6 Amateur Spammer 2012-07-30 08:04:21 UTC 
I tried to post on the forums, but my post was "classified as spam". 
Oh well. I'll just comment here.

http://filesmelt.com/dl/client-inputhack.7z has this patch. 
This build may or may not work. Please test. You should probably extract this in a separate directory. 

Also this has nothing to do with server. This is strictly a client issue.
Comment 7 Dr. Sean 2012-07-31 20:45:13 UTC 
A quick test shows that the client does not in fact repeat a command bound to the keyboard every tic. In the below data, the +forward key was bound to E and held down constantly.  Perhaps the results are indicative of the keyboard repeat rate configured in the OS or BIOS...

**** tic 1519, +forward
**** tic 1554, +forward
**** tic 1575, +forward
**** tic 1578, +forward
**** tic 1581, +forward
**** tic 1584, +forward
**** tic 1587, +forward
**** tic 1590, +forward
**** tic 1593, +forward
**** tic 1596, +forward
**** tic 1599, +forward
**** tic 1602, +forward
**** tic 1605, +forward
**** tic 1608, +forward
**** tic 1611, +forward
**** tic 1614, +forward
**** tic 1617, +forward
**** tic 1620, +forward
**** tic 1623, +forward
**** tic 1626, +forward
Comment 8 Amateur Spammer 2012-08-01 19:55:03 UTC 
The bug was about commands bound to *mouse* buttons not repeating at all.

As ot the keyboard Odamex configures low repeat rate using SDL_EnableKeyRepeat() function. I don't know if SDL generates repeated events internally or uses OS ones.
I saw a comment that key repeat was somehow bad and code disabling key repeats; but it was also commented out and replaced with setting a low repeat rate.
Comment 9 Dr. Sean 2012-08-13 23:26:58 UTC 
Created attachment 393 [details]
Disable keyrepeat unless chatting or in console
Comment 10 Dr. Sean 2012-08-13 23:27:31 UTC 
I think that key repeat should only be enabled when the client is entering text, such as in-game chat or using the console. Repeating actions, while harmless, seems like bad policy.

I've attached a quick & dirty patch that disables key repeating unless the player is in the console or typing a message.
Comment 11 Amateur Spammer 2012-08-14 18:31:22 UTC 
(In reply to comment #10)
> I think that key repeat should only be enabled when the client is entering
> text, such as in-game chat or using the console. Repeating actions, while
> harmless, seems like bad policy.
It has legitimate uses described in this bug.
See comment 3; absence of key repeating creates problem with the SR-50 bindings there: if you want to switch from SR-50 to ordinary running, you must *first* release key bound to SR-50 and *then* press "forward" key for the binding to work. Otherwise the player stops.
If this example isn't convincing, consider two strafe-50 binds for left and right straferunning respectively; you can't switch between them without event repeating, as the player will start spinning.
No such behavior exists for forward & backward keys: if you want to start moving backward instead of forward, you can press "backward" key and release "forward" key in any order. So is for "strafe left" and "strafe right", etc. It's the intuitively expected behavior.
Comment 12 Dr. Sean 2012-09-03 17:37:06 UTC 
(In reply to comment #11)
absence of key repeating creates problem with the SR-50 bindings
> there: if you want to switch from SR-50 to ordinary running, you must *first*
> release key bound to SR-50 and *then* press "forward" key for the binding to
> work. Otherwise the player stops.

I did not experience any awkwardness with the transition from sr-50 to running straight with +forward with my patch. I simply release the +sr50l key while still holding down +forward and keep moving.
Comment 13 Amateur Spammer 2012-09-03 18:11:48 UTC 
With the turnspeeds-based bind there's indeed no problem, if you're willing to sacrifice keyboard turning (which I only use when debugging in conjunction with -nomouse so that Odmex keyboard and mouse grabs). Probably not an issue for most people, and I understand that you may not want this patch.
Comment 14 Dr. Sean 2012-12-10 18:58:58 UTC 
A fix for this bug was committed in r3499. It removes the key-repeating code from ZDoom 1.22 entirely, which previously had run in conjunction with SDL's key-repeating. Key-repeating is disabled during game play but is enabled for menus, the console, and for entering chat messages.

Please test and confirm.
Comment 15 Mike Lightner 2013-01-17 02:57:29 UTC 
Doesn't this mean it's in 0.6.2?  Can we get a confirmation please jkist?
Comment 16 JKist3 2013-01-18 23:43:30 UTC 
appears to be working correctly now.  Thanks.
Comment 17 Dr. Sean 2013-03-04 20:48:56 UTC 
Confirmed by JKist3 so marking fixed.