Engine Code: Improving Key Releasing

[Baker]

Summary:

This short tutorial fixes a couple of really annoying problems in Quake that have to do with all sorts of aliases and keybinds being triggered inappropriately in situations in-game, in the console and when using ALT-TAB or changing video mode.

Walkthrough:

This part fixes the ALT-TAB and video mode changing issues. Later on, the console and messagemode fixes are addressed.

For starters, some of the code is not really in the right place in Quake.

In ProQuake, I moved Key_ClearAllStates from gl_vidnt.c/vid_win.c into keys.c. There is no Windows-only code in Key_ClearAllStates, so it is just a more sensible location.

Here is the ClearAllStates from GLQuake:

/*
================
ClearAllStates
================
*/
void ClearAllStates (void)
{
int i;

// send an up event for each key, to make sure the server clears them all
for (i=0 ; i<256 ; i++)
{
Key_Event (i, false);
}

Key_ClearStates ();
IN_ClearStates ();
}

And the Key_ClearStates from GLQuake:

/*
===================
Key_ClearStates
===================
*/
void Key_ClearStates (void)
{
int i;

for (i=0 ; i<256 ; i++)
{
keydown[i] = false;
key_repeats[i] = 0;
}
}

Note that the above is setting the down state to false.

And here is the IN_ClearStates from GLQuake in in_win.c:

/*
===================
IN_ClearStates
===================
*/
void IN_ClearStates (void)
{

if (mouseactive)
{
mx_accum = 0;
my_accum = 0;
mouse_oldbuttonstate = 0;
}
}

This is reseting the mouse coordinates.

What is wrong with the way GLQuake does it

First, it is triggering -aliases FOR all keys. It doesn't matter if the key was depressed or not. This can have undesirable effects when ALT-TAB is done or if the client supports video mode changing.

Second, only ClearAllStates in gl_vidnt.c/vid_win.c calls Key_ClearStates. Why have 2 procedures separate and 1 of them in clearly the wrong source file.

What I did to fix this ...

Only ClearAllStates calls Key_ClearStates and it's in the wrong place. I deleted ClearAllStates from gl_vidnt.c and vid_win.c. Then renamed Key_ClearStates to Key_ClearAllStates.

And instead of triggering every key with a release event, it only triggers key events for keys that were actually down.

/*
==================
Key_ClearAllStates
==================
*/
void Key_ClearAllStates (void)
{
	int		i;

	for (i=0 ; i<256 ; i++)
	{
		// If the key is down, trigger the up action if, say, +showscores or another +bind is activated
		if (keydown[i])
			Key_Event (i, false);

	}
	IN_ClearStates ();
}

The Key_Event with the down=false is going to set keydown[key] to false and keyrepeats[key] to 0 anyway.

Extra Credit

The above doesn't address another issue. Minus -aliases are triggered in the console or even in messagemode! This is annoying.

My console, for instance, gets spammed with "-hook isn't a valid alias" and weird impulses gets trigger when I'm in message mode because I have a weapon changing binds with both +aliases and -aliases. I've had other oddball things happen while typing, I even ruined a tournament game once when logged in as admin from this because all sorts of things can get triggered by -aliases being executed while chatting.

The Fix For That

First, we are going to mark whether a key was depressed while in-game.

//At the top of keys.c

qboolean keygamedown[256];

Second, in Key_Event we need a temp variable to store this value. This is because Key_Event will need to set the down state to false if exiting.

//keys.c in Key_Event
qboolean wasgamekey = false;

And we need to set this variable immediately after "keydown[key] = down;"

//keys.c in Key_Event 

	wasgamekey = keygamedown[key]; // Baker: to prevent -aliases being triggered in-console needlessly		
	if (!down) {
		keygamedown[key] = false;
	}

Now we need to find this to make it only execute the -alias if the key was triggered in the game:

if (!down)
{
// Baker: we only want to trigger -alias if appropriate
// but we ALWAYS want to exit is key is up
if (wasgamekey) {
kb = keybindings[key];
if (kb && kb[0] == '+')
{
Q_snprintfz (cmd, sizeof(cmd), "-%s %i\n", kb+1, key);
Cbuf_AddText (cmd);
}
if (keyshift[key] != key)
{
kb = keybindings[keyshift[key]];
if (kb && kb[0] == '+')
{
Q_snprintfz (cmd, sizeof(cmd), "-%s %i\n", kb+1, key);
Cbuf_AddText (cmd);
}
}
}
return;
}

And finally, we need to have it set the keygamedown ONLY when the +alias is executed:

if ((kb = keybindings[key]))
{
// Baker: if we are here, the key is down
// and if it is retrigger a bind
// it must be allowed to trigger the -bind
//
keygamedown[key]=true; // Let it be untriggered anytime
if (kb[0] == '+')
{ // button commands add keynum as a parm
Q_snprintfz (cmd, sizeof(cmd), "%s %i\n", kb, key);
Cbuf_AddText (cmd);
}
else
{
Cbuf_AddText (kb);
Cbuf_AddText ("\n");
}
}
return;

[Bam]

Sweet, I found that same bug annoying. (getting errors while typing)

Now you just gotta fix pq_moveup.

[R00k]

Sweet, I found that same bug annoying. (getting errors while typing)

Now you just gotta fix pq_moveup.

I always wanted to make +moveup or +watermove mouse viable, meaning
if I hold a button and move the mouse (ie forward mouse moves me UP in water, back down, left/right etc..) u get the meaning...

[Bam]

That would be neat! I say you implement that, temporarily even, to see how well it will play.

[=peg=]

that means moving the mouse will turn and strafe at same time? hack if you can make that work then you might as wall replace the +strafe command with an improved version

[R00k]

lol,

not trying to steal the thread but

if ((in_strafe.state & 1) && (noclip_anglehack) || (cl.inwater))
cmd->upmove -= m_forward.value * mouse_y;
else
cmd->forwardmove -= m_forward.value * mouse_y;

adding || (cl.inwater)) up there allows me to bind mouse2 "+strafe", and use m_forward 99, m_side 99 and when I'm in water i can hold mouse2 down and move the mouse to strafe side to side and up and down

[=peg=]

yeah but +strafe wont let you turn as well.. i have been playing for over 10 years using my own +mystrafe (i dint use +moveleft and +moveright ) with m_side 25

i managed to be pretty competitive with this but it was a big handicap as it did not allow me to turn and strafe at the same time (no bunny / circlestrafing) but i compensated by (strafe)jumping and turning in air (after releasing the +mystrafe key).

alias +mystrafe "lookstrafe 1"
alias -mystrafe "lookstrafe 0"

/end of little sidetrack from this thread

[spooker]

&& (noclip_anglehack) ||

rook i always knew u were haxing. u mop the floor with me in CTF far too often! =:]

[R00k]

lol u silly druid, that variable is triggered by "noclip" single player, thats standard.

any i will 1 on you ctf any day, and teach you a thing or 3
you're a good man charlie brown...

[R00k]

Extra Credit

Minus -aliases are triggered in the console or even in messagemode! This is annoying.

Actually a quicker fix would be, find this part and add the highlightened if statement..

// key up events only generate commands if the game key binding is
// a button command (leading + sign).  These will occur even in console mode,
// to keep the character from continuing an action started before a console
// switch.  Button commands include the kenum as a parameter, so multiple
// downs can be matched with ups
	if (!down)
	{
		[COLOR="Yellow"]if ((key_dest == key_menu && menubound[key]) || (key_dest == key_console && !consolekeys[key]) || (key_dest == key_game && (!con_forcedup || !consolekeys[key])))
		{[/COLOR]
			kb = keybindings[key];
			if (kb && kb[0] == '+')
			{
				sprintf (cmd, "-%s %i\n", kb+1, key);
				Cbuf_AddText (cmd);
			}
			if (keyshift[key] != key)
			{
				kb = keybindings[keyshift[key]];
				if (kb && kb[0] == '+')
				{
					sprintf (cmd, "-%s %i\n", kb+1, key);
					Cbuf_AddText (cmd);
				}
			}
			return;
		}
	}

I guess id forgot to add that...

[Baker]

Actually a quicker fix would be, find this part and add the highlightened if statement..

// key up events only generate commands if the game key binding is
// a button command (leading + sign).  These will occur even in console mode,
// to keep the character from continuing an action started before a console
// switch.  Button commands include the kenum as a parameter, so multiple
// downs can be matched with ups
	if (!down)
	{
		[COLOR="Yellow"]if ((key_dest == key_menu && menubound[key]) || (key_dest == key_console && !consolekeys[key]) || (key_dest == key_game && (!con_forcedup || !consolekeys[key])))
		{[/COLOR]
			kb = keybindings[key];
			if (kb && kb[0] == '+')
			{
				sprintf (cmd, "-%s %i\n", kb+1, key);
				Cbuf_AddText (cmd);
			}
			if (keyshift[key] != key)
			{
				kb = keybindings[keyshift[key]];
				if (kb && kb[0] == '+')
				{
					sprintf (cmd, "-%s %i\n", kb+1, key);
					Cbuf_AddText (cmd);
				}
			}
			return;
		}
	}

I guess id forgot to add that...

Hmmmm.

My idea is that a -alias should always be triggered if a +alias is triggered.

The problem as I see it is not that the -alias is triggered in the console (sometimes it should be!) but that it is triggered in the console even when it should not be.

My method assures a -alias is triggered in the console if the +alias is triggered.

Case in point, let's say I have +zoom bound to a key.

If I go to the console and release the key, -zoom needs to be triggered. Otherwise after I exit the console, the zoom will still be FOV 10 or whatever and since the keyup event was ignored (as in your example).

/I'm pretty sure I don't see how your method would allow the -zoom to occur

[R00k]

ah very true, then ill just do this

void Con_ToggleConsole_f (void)
{
key_lines[edit_line][1] = 0; // clear any typing
key_linepos = 1;

if (key_dest == key_console)
{
if (cls.state == ca_connected)
{
key_dest = key_game;
}
else
{
M_Menu_Main_f ();
}
}
else
{
Key_ClearAllStates();// If a +alias was interrupted by toggleconsole, then execute the -alias.
key_dest = key_console;
}

SCR_EndLoadingPlaque ();
memset (con_times, 0, sizeof(con_times));
}

[Baker]

ah very true, then ill just do this

I think that would mean you will stop moving/firing/jumping/hooking/close the scoreboard if you pull up the console, even if you didn't release the key.

If you are playing a CA game and running around and need to quickly toggle the console to see what your teammate said, I don't think that means you want to stop running.

[PapaSmurf]

Hmmmm.

My idea is that a -alias should always be triggered if a +alias is triggered.

The problem as I see it is not that the -alias is triggered in the console (sometimes it should be!) but that it is triggered in the console even when it should not be.

My method assures a -alias is triggered in the console if the +alias is triggered.

Case in point, let's say I have +zoom bound to a key.

If I go to the console and release the key, -zoom needs to be triggered. Otherwise after I exit the console, the zoom will still be FOV 10 or whatever and since the keyup event was ignored (as in your example).

/I'm pretty sure I don't see how your method would allow the -zoom to occur

wouldn't this have an effect on +mlook?

[R00k]

im not totally sure, i only tested by creating an alias, then in console trying to execute the alias.

I havent tested it with all the variables.
I dont receive the "unkown command" in console or message mode typing anymore.

Maybe I'm overlooking the problem...

I will re-evaluate, as this just an optimized repress

edit

I think that would mean you will stop moving/firing/jumping/hooking/close the scoreboard if you pull up the console, even if you didn't release the key.

actually this is exactly what happens, do u want to run valiantly in to the abyss while in the console??
use con_notifytime to increase the duration of the console messages, so u dont have to toggle to read team messages.
m_look cvar replaces +mlook long ago...