Quake's mouse code is written stupidly

[Baker]

Quake's mouse code is written stupidly.

The function names are terrible. The variables tell you little and are so vague as to require exploration. Some of the if statements are very long roads to do what could be express so much more condensely.

DirectQ, for example, has far better function names and DirectQ has better input code than all but a couple of engines or maybe better than all of them.

(ezQuake's mouse code can be expected to do a lot of things and be very well tested, but I think they go a bit crazy with the number of options but I guess their userbase wants that and it matters quite a bit for the physics in Quakeworld. Still, they stick with all the poorly worded functions and the maze of code that doesn't have to be).

I would just steal MH's in_win.cpp verbatim except I need to make some modifications to make it "modular" so I can possibly use it eventually with a stripped down engine shell I do OpenGL testing in.

[mh]

Ha ha - I guess most people just notice the renderer and stop right there, but this is actually where probably the most work ended up being done.

Really nice input code that works at least reasonably consistently on all of your target platforms is surprisingly difficult (but interesting and rewarding). It took me so many false starts to get to what you see today, and a good chunk of the end result must be attributed to a lot of time spent trying to fix what I thought was jerkiness in the input code (it turned out to be using a frametime instead of cl.time somewhere in the view.c stuff).

There are a lot of little gotchas in that code by the way. It uses the frame time from the previous frame for rebalancing movement events (quite intentional because there is some latency between when an event actually happens and when the message arrives to your window procedure - this seems to average things out better). It relies on Sys_SendKeyEvents being run every pass through your main loop, even if you're not running a frame. There's additional code in the window procedure for handling the mouse wheel in menus and the console (because WM_INPUT messages are no longer recieved when in menus or the console in an SP game and a windowed mode).

[daemonicky]

Is not it counter productive that Quake code is without official documenation

I would just steal MH's in_win.cpp verbatim except I need to make some modifications to make it "modular" so I can possibly use it eventually with a stripped down engine shell I do OpenGL testing in.

It is great that you try to make it modular and do unit testing. This unit testing code might be interesting to this community.

Are you making it object oriented (module == object) or just "C"

[Baker]

Is not it counter productive that Quake code is without official documenation

I would just steal MH's in_win.cpp verbatim except I need to make some modifications to make it "modular" so I can possibly use it eventually with a stripped down engine shell I do OpenGL testing in.

It is great that you try to make it modular and do unit testing. This unit testing code might be interesting to this community.

Are you making it object oriented (module == object) or just "C"

My code is mostly for my personal learning and experimentation, I'm not sure that anyone else would find it interesting.

It is just "C", and they aren't reusable objects (I couldn't run 2 of them for example) ... I ultimately want to use the same exact code for both my Quake engine and other little experiments.

[Baker]

Ok .... I'm just trying to figure out what sense this could possibly make:

snd_dma.c

Code: Select all

void S_ExtraUpdate (void)
{

#ifdef _WIN32
	IN_Accumulate (); <----- why are we checking mouse here?
#endif

	if (snd_noextraupdate.value)
		return;		// don't pollute timings
	S_Update_();
}

For reference, DirectQ's S_ExtraUpdate ...

Code: Select all

void S_ExtraUpdate (void)
{
}

As far as I know, S_ExtraUpdate is to update the sound during really long frames like slow fps due to lots of stuff on the screen or a slow video card or the engine is having a harder time rendering the section of the map.

I guess using that logic, capturing the mouse position a few extra times is things are lagging is good.

Still, the code is weird and effectively doesn't even apply if DirectInput is on because IN_Accumulate ignores dinput.

/End weird observation

[Baker]

Code: Select all

	if (m_filter.value)
	{
		mouse_x = (mx + old_mouse_x) * 0.5;
		mouse_y = (my + old_mouse_y) * 0.5;
	}
	else
	{
		mouse_x = mx;
		mouse_y = my;
	}

	old_mouse_x = mx;
	old_mouse_y = my;

	mouse_x *= sensitivity.value;
	mouse_y *= sensitivity.value;

At first glance to me, this looks like it would have little different between reducing the sensitivity by half, but what it is actually doing is spreading mouse position updates over multiple frames.

DirectQ stripped out m_filter, unsurprisingly.

I really wonder if the above could help even with a very jerky mouse. I guess the main reason to keep this is to keep people from complaining. Or maybe stealthily comment the code out and see if anyone really notices.

[mh]

For m_filter, have a read of this: http://www.team5150.com/~andrew/carmack ... #d19980608

It's not a problem in DirectQ owing to the way it collects and accumulates mouse input samples, so I got rid of it.

Quake's DirectInput code is quite primitive (no mousewheel even) so I wouldn't use that as a guideline for reasons why things are done, but in general your reading of IN_Accumulate is the same as mine.

Regarding DirectInput, some observations.

On reasonably modern systems (DX8+, WinXP or higher) DirectInput just uses raw input in a separate thread behind the scenes; it doesn't do anything special in and of itself.

You'll commonly find advice that you should no longer use DirectInput, but instead bypass the middle man and write your own raw input code.

If, on the other hand, you're going to collect input data in a separate thread, then you may as well just use DirectInput. Why write (and test/debug) the extra code when the work has already been done for you?

There's therefore nothing to be gained from running DirectInput in it's own thread, if you're thinking of going down that route. Just use standard DirectInput in the same thread as everything else and it will give you that extra thread without you needing to do any work.

As an added bonus, documentation for raw input is extremely bad, whereas documentation for DirectInput is very good.

[goldenboy]

Carmack wrote:

It is a bit amusing how after the QuakeArena anouncement, I got flamed by lots of people for abandoning single player play (even though we aren't, really) but after I say that Quake 2 can't forget that it is a single player game, I get flamed by a different set of people who think it is stupid to care about single player anymore when all "everyone" plays is multiplayer. The joy of having a wide audience that knows your email address.

Ahah hah hah, this amuses me for some reason. Heh. *sniff*

Awesome.

[Baker]

Old thread. Pain revisited. Trying to fix up FitzQuake's input code.

My "mistake" was changing FitzQuake to set windowed or fullscreen mode once on startup. This busts the mouse code at least on some setups.

For instance, the set video mode code deactivates and reactivates the mouse. And this is before mouse initialization :roll: . Also Windows ShowCursor () is not kind ... if you do ShowCursor (TRUE) twice, you better call ShowCursor (FALSE) twice -- otherwise you will attempt to "hide the mouse" (but you didn't) or "show the mouse cursor" (but you didn't).

The single pass video mode startup didn't break anything. Starting it with -window is enough to cause it fits apparently for some versions/configurations of Windows. I just made it far more obvious since starting in windowed mode is easier to do.

Input code in my humble opinion is probably the least fun thing to have to deal with. Startup, switching away from the active app, switching back, suspending it, re-enabling it ... no fun. And just to get back to even. There isn't much of a "win" because the time invested just gets it back to "properly functional".

[r00k]

I've been through this as well, personally as a player I prefer raw input, oddly with m_filter 1. I can feel the graininess of the poll rate with m_filter 0. Which might just be a timing issue...

[mh]

I love input code.

Regarding ShowCursor, Doom 3 has this (forgive the reformatting, I'm physically unable to read K&R):

Code: Select all

	for (i = 0; i < 10; i++)
	{
		if (::ShowCursor (false) < 0)
		{
			break;
		}
	}

And this:

Code: Select all

	for (i = 0; i < 10; i++)
	{
		if (::ShowCursor (true) >= 0)
		{
			break;
		}
	}

For activating/deactivating the mouse I eventually just gave up and did it this way:

Each frame I check the mouse state. If it's what it should be for the current mode, I do nothing. Otherwise I put it the way it should be. Deactivating (which includes going to the console or menu in a windowed mode) is a full deactivation - including destroying DirectInput, flushing out all states, etc. Likewise activating is a full re-init.

The beauty of this is that I just have one place where this needs to be checked, and everything just happens the way it should completely automatically - I don't need to add special-case code to any other event handlers, for example. If the mouse isn't the way it should be in one frame I don't even bother worrying about it, because it will be in the next one. It works.

[Baker]

I love input code.

...
Each frame I check the mouse state. If it's what it should be for the current mode, I do nothing.

If the mouse isn't the way it should be in one frame I don't even bother worrying about it, because it will be in the next one. It works.

Seems like a considerably less messy method. I have something I want to do that I've put off because I'd have to mess with the input code.

[mh]

It turned out incredibly clean. I'll probably wrap it in a nice class sometime, but for now here's the main function:

Code: Select all

/*
===================
IN_SetMouseState

sets the correct mouse state for the current view; if the required state is already set it just does nothing;
called once per frame before beginning to render
===================
*/
void IN_SetMouseState (bool fullscreen)
{
	// no mouse to set the state for
	if (!mouseinitialized) return;

	if (keybind_grab)
	{
		// non-negotiable, always capture and hide the mouse
		IN_AcquireMouse ();
	}
	else if (!ActiveApp || Minimized)
	{
		// give the mouse back to the user
		IN_UnacquireMouse ();
	}
	else if (key_dest == key_game || fullscreen || cl.maxclients > 1)
	{
		// non-negotiable, always capture and hide the mouse
		IN_AcquireMouse ();
	}
	else
	{
		// here we release the mouse back to the OS
		IN_UnacquireMouse ();
	}
}

You could probably have figured most of what's needed yourself, but there are a few tricksy edge cases that this might help you with. The acquire/unacquire functions just run through a full startup/shutdown if needed; nothing fancy there.

[Spike]

IN_Accumulate is because the mouse is clipped to be within the window. consider how fast you can move your mouse 160 pixels across - can you do that within the course of one frame? probably. what happens when you reach the side? it gets clamped. which is bad. its even worse if windows doesn't honour ClipRect for some reason, which can result in clicking on other windows!
you don't need it for dinput/rawinput because those are movement deltas already, rather than absolute cursor coords.

q3 supposedly has a much cleaned up input system. events are shoved into a queue by whatever windowing system code is active, and handled in a single unified place. instead of different input code on every single platform.
which reminds me that I need to do the same thing myself some time. hrmph.

[mh]

...and S_ExtraUpdate is obviously being (mis)used as a more generic "stuff anything else that we want to bring up to date if we're running slow in here" routine...