Input poller/system patch

Coordinator
Mar 29, 2008 at 8:32 AM
Edited Apr 1, 2008 at 6:51 AM
== Uploaded ==

This patch includes changes to the way input systems send their messages. All messages now only send messages during state changes as opposed to current state each frame.

Keyboard Handler:
The keyboard handler was a hybrid setup, it would only send up/down messages when a button was pressed or released, but would send held messages every frame. This was somewhat of a waste, but I believe it was also done a temporary measure because the camera couldn't move each frame based on a single message passed to it one time, this is now possible through the input poller which I will describe later. Also the keyboard system written by Sturm was likely sufficient, but I found the way I was used to to be shorter (code-wise). As far as performance between the two, they were likely very similar, although my system only has a single list of keys, and an array of bools, versus the original handler having 4 lists of keys, this isn't much of a memory difference as they're such small lists, but I find it to be slightly less confusing.

There is no further need to keep track of specific keys like ctrl, shift, and alt. The input polling system is made to keep track of multiple key states, so you can easily ask if there are combinations of keys down, like CTRL + Enter. You can see an example of this change in the QuickStartSampleGame.cs.

Mouse Handler:
The input poller serves no advantage in storing mouse movement values, as they're the same as would be received by event-based anyway. If a user wants to use mouse movement they should do it through listening for mouse move messages, not through the input poller. Mouse buttons can still be polled for however. The up/down messages were only being sent on-event, however, held was being sent every frame. This has been changed, held is now only sent once, when the button is first held in.

GamePad Handler:
- Changed all gamepad input and message to be based of polling methods rather than event based methods. This means not having to store nearly as much information in the handler for each gamepad because we can query the states in the input poller.

InputPollingHandler:
This system stores all information about any buttons/keys requested. Any system in the game can use this object, and it will simply listen to messages from the input system and store the info, and gives an easy way to access that information. The main purpose of this system, is that it allows the owner of it to use input in a polling-based fashion. This means if you perform an action (like moving the camera forward) which requires a key to be held down each frame, then you can easily do this through the handler. The alternatives to this method would've meant we either had to send all input messages every single frame for every single button pressed/released, or we'd have to use an event-based system (which is good), but require all systems that need polling-style methods to store all of their own information about every button/key they needed polling for, and then require them to write their own methods for using polling-style things (which is bad). This system does all the work, so noone has to write their own code to do polling-style actions in our new event-based input system.
Mar 29, 2008 at 3:15 PM
Looks good. The only request I have is to make sure this system is completely thread-safe. In the future, we may want to have several processing threads accessing the input state.
Coordinator
Mar 30, 2008 at 5:28 AM
Well, using the input poller, you'll simply recieve the state it was in the last time it recieved a message. I'm fairly unexperienced with multi-threading. Are there any specific requirements for "thread-safe"?

Threads won't actually access the input state anymore. There are two options:

1.) Recieve input messages, and act on them (event-based).
2.) The input poller receives messages, and stores the info. Any object can have its own instance of an input poller. Object requests information about any button/key that the poller knows about.
Coordinator
Mar 31, 2008 at 7:08 AM
Shaw, probably wont get this patch in tonight (Sunday night). Commenting is taking far longer than I thought, and I got a late start. Took me an hour almost to comment one file, 800 lines almost. Might have it up tomorrow. Just thought I'd let you know....work continues.
Coordinator
Apr 1, 2008 at 6:55 AM
I've merged with the newest commit, and uploaded the patch. Seems to run fine on my end, but as terrible as cpc is, you should make sure I didn't break anything you recently commited....just in case. Thanks in advance for the review. There are quite a bit of changes floating around. Not much of it is easily noticable while running. The biggest difference is the amount of messages we send. Previously, for every button/key held down, we were sending it every frame. Messages are indeed fast, but as I stated long ago when starting this system, this should cut down on input messages by about 95-98%. Not to mention we can easily design game systems around input OR polling based styles, which the freecamera.cs does exactly.
Apr 1, 2008 at 3:36 PM
Looks very nice!

  • Xbox Compatibility:
    • GamePadHandler.cs - line 314: structs must be initialized when declared:
GamePadThumbStick thumbstickData = new GamePadThumbStick();  // We need to use "new" even though this is a value-type
    • Don't forget to add your new files to the Xbox projects. :)
  • Why are you deriving GamePadThumbStick from IPoolItem? The object pool is designed for reference-types, not stack-allocated value-types.
  • Style:
    • Are we still using "this." in front of all class members? If we are, there are quite a few missing.
    • It may be beneficial to change all methods that do not access class state to static methods.
Coordinator
Apr 1, 2008 at 3:45 PM
I'll try and get those changes in when I can. I will add my files to the xbox project, but I have no gaurantee it'll run, only that it will compile :).

I was deriving from IPoolItem because that is how Sturm was doing it with his mouse stuff. I can certainly remove that.

What do you mean by class state?
Apr 1, 2008 at 6:10 PM
I mean accessing member variables. If a method only references its parameters, it should be able to be called without a class instance.
Apr 1, 2008 at 11:46 PM
It may be beneficial to change all methods that do not access class state to static methods.
This is an interesting point which is often raised during coding standards meetings...

I agree with Shaw, I think that each method that do NOT use a member field ( = a class-level variable that is NOT static) should BE declared as STATIC.

I think we should always properly balance the usage of static methods/variables and non-static ones, and going to pure-OO or pure-procedural is too extreme. We don't need the overhead of instantiating an object when it contains no state.


Regarding the way we call methods, I fell confortable with the following :
- use 'this.' if it is a non-static method, systematically
- use the type name instead of this to call a static method

Thus we immediately figure out if the method is static or not by looking at the code.

FYI, below are my personal prefered naming standard :
- prefix private class members (non-static) with underscore
- start method parameters with lowercase letter
- start property name with uppercase letter

- start private/internal methods names with lowercase letter
- start public method with uppercase letter (or lowercase if we want to differenciate them from the .NET framework methods)

- avoid static properties (replace by static methods or fields).
Coordinator
Apr 2, 2008 at 12:41 AM
Edited Apr 2, 2008 at 12:42 AM
I'm not a big fan of changing the coding guidelines this far in. I agree with the 'static' stuff we've been talking about, and that 'this.' should only come in front of member variables, but not methods, unless preferred.

I will use static where necessary before I commit. Thanks guys.
Coordinator
Apr 2, 2008 at 5:12 AM
Ok, maybe I'm blind, but I'm not seeing any opportunities in my recent changes for static :).
Coordinator
Apr 2, 2008 at 5:40 AM
Edited Apr 2, 2008 at 5:47 AM
I've commited my changes. I forgot to update the Xbox project. I will do that now and commit it as well asap.
EDIT: Done.
Apr 2, 2008 at 2:38 PM
Cool, now can we move to SVN? :)
Coordinator
Apr 2, 2008 at 4:01 PM
Indeed. You want to scout out sourceforge a little bit? I'll try and take a look at it tonight.