Mouse/Keyboard GetState()/IsKeyDown()/IsKeyUp() methods

[ghost]

I would like to suggest that the methods listed in the subject line be made static, thereby eliminating the need to instantiate an instance of Mouse/Keyboard for most uses. For now, I'm wrapping instances of these classes in a class that has equivalent static methods, so I can achieve the effect I'm going for, but it would be nice if the DirectXTK more closely matched the XNA API right out of the box. If there is a technical reason why they shouldn't be made static I'd like to know why, if just for educational purposes. I'm humbled by the quality of the code and I'm still a ways away from mastering C++.

Another suggestion, and this is just a stylistic thing, I think it would be better to have the first letter of the Mouse::State struct members capitalized. This matches the style of the Keyboard::State struct and Microsoft conventions in general...at least in the clean and consistent world of .NET. :)

Thanks guys!

[pgalasti]

In the future, would there be a case where you would want to distinguish separate keyboards/mice connected to the system, and have each instance assigned to each individual device?

[ghost]

Hmm, I just looked at the Windows API docs regarding keyboard/mouse messages and I don't see anything in there that would even support being able to differentiate between multiple keyboards/mice plugged in. Did I overlook it in the docs?

[walbourn]

RE: Static vs. singleton

I indeed had started by making Keyboard a static class like the XNA Game Studio, but once I added support for Windows Store / universal Windows apps it became apparent I needed the full pImpl pattern to do it right and that the initialization was not as trivial as one might imagine from Windows desktop development. I'm also working on a few more features for the Mouse class which leverages this design even further.

As it is now, Keyboard, Mouse, and GamePad are singletons which greatly simplifies the handling for all supported platforms .That said, I could certainly add a static GetState method since it is a singleton... I'll take a look at that.

RE: IsKeyUp/IsKeyDown

These methods are actually on the State object which you get back from GetState, so the pattern of usage is (if I had the static above):

auto state = Keyboard::GetState();
if ( state.IsKeyDown( VK_ENTER ) )
...
if ( state.Back )
...

Again, I could add a static IsKeyUp/IsKeyDown which checks the state internally through the singleton, but I honestly think the state.Key pattern is much easier to read code. I have IsKeyUp and IsKeyDown supported for loops and rebinding support, but I hadn't expected it to be the dominate usage pattern.

RE: Naming conventions

The .NET world has very strong conventions, largely because the FXCop tool is a pedantic monster enforcing the PascalCase naming convention well beyond the public interfaces. For DirectX Tool Kit I've adopted PascalCase for methods/functions/classes, camelCase for member variables in structs/classes and enums, and UPPERCASE for #defines and nameless enums. This pattern matches what Win32 APIs C++ generally use that aren't so old that they are still using the long-deprecated Hungarian notation. The result is something consistent that retains the flavor of XNA Game Studio, but is still C++.

For the enum Keyboard::Keys I used PascalCase as normal, and I used the exact same name as the bit field member in Keyboard::State. This ends up making Keyboard::State member variables a little different than the other structs in the toolkit (PascalCase rather than camlCase) but I deemed this to improve usability.

[pgalasti]

Robert,

You're right, Windows doesn't differentiate between different keyboard/mice devices. My mistake!

[ghost]

Chuck: You're right, it doesn't make much sense to make IsKeyUp/Down static and actually I didn't mean to suggest it in the first place. The code I use now only wraps Mouse::GetState() / Keyboard::GetState() with equivalent static methods, resulting in an API that looks like this BInput::Mouse::GetState() / BInput::Keyboard::GetState(). If you make those methods static I'll be able to get rid of my BInput class.

As for the naming conventions, as long as there is a method to the madness, and it seems as though there is, I can rest easy! I started my career in .NET, so I can't help but to prefer Pascal for public members. Whenever I look at code, I think my brain spawns a thread dedicated to running FXCop :)

pgalasti: No worries mate! In the event it was possible, my follow up response would have been to overload the static method (or specify a default value) and have it accept an enum/int to signify which keyboard/mouse you want the state for. :)

[walbourn]

C++ doesn't let you overload a function between static & non-static, so it would have to be a different name for the static method than GetState. I'll have to think about that one...

[ghost]

This really is a tricky one to solve (without adding a static method with an ugly or confusing name), and the Mouse class is already in a workable, "pure" state and I'm probably violating a best practice of some sort, but here is an idea none-the-less! Tested with Win32 Desktop and Windows 8.1 Store.

Mouse.h line 80:

static State __cdecl GetState();

Mouse.h line 90:

static void __cdecl SetWindow(Windows::UI::Core::CoreWindow^ window);

Mouse.cpp line 292:

Mouse::Impl* Mouse::Impl::s_mouse = new Impl;

Mouse.cpp line 299:

Mouse::Impl::s_mouse->SetWindow(iwindow);

Mouse.cpp line 351:

Mouse::Impl* Mouse::Impl::s_mouse = new Impl;

Mouse.cpp line 431:

Mouse::Impl* Mouse::Impl::s_mouse = new Impl;

Mouse.cpp line 522:

 // : pImpl( new Impl() )

Mouse.cpp line 548:

Mouse::State Mouse::GetState()

Mouse.cpp line 551:

Mouse::Impl::s_mouse->GetState(state);

[walbourn]

I think a more consistent solution is going to be to add a Get to the singletons:

static Mouse& Mouse::Get()

static GamePad& GamePad::Get()

static Keyboard& Keyboard::Get()

[ghost]

So to get Mouse state, for example, it will be:

Mouse::State mouseState = Mouse::Get().GetState();

?

[walbourn]

Yes, that would work. I still need to implement it, but those three classes are internally singletons already.

[ghost]

Sounds good. Thanks!

[walbourn]

Submitted.

6a4fd0dcef5f04d302e1c2691f0679eb7b1cf34c