Why doesn't my DirectX program recognize that I've released the escape key? (C++)

Question

EDIT: After even more code modification, the error is still there, modified code shown:

KeyDown():

const int input_bit_num = 0x8000;
char keys[256];
bool KeyDown(int key)
{
    return (keys[key] & input_bit_num) != 0;
}

PollKeyboard():

LPDIRECTINPUTDEVICE8 di_keyboard;
void PollKeyboard()
{
    long result = di_keyboard->GetDeviceState(sizeof(keys), (LPVOID)&keys);
    char para[16];
    itoa(result, para, 17);
        if(result != DI_OK) MessageBox(NULL, para, "ERROR", MB_OK);

}

When I try to put MessageBox within a KeyDown() if statement (as seen below in the game loop), the MessageBox simply coninues to appear even if I stop pressing the key, ie: I press , the "Would you like to quit?" message box appears, I say no, It disappears and then instantly reappears, as if I were still holding the key.

This is my loop:

void GameRun(HWND hWnd) //called once every frame
{
    PollKeyboard();
    if(GetTickCount - start >= 30)
    {
        if(KeyDown(DIK_LEFT))
            MoveLeft();
        if(KeyDown(DIK_RIGHT))
            MoveRight();
    }

    if(d3ddev->BeginScene())
    {
        //rendering
    }

    if(KeyDown(DIK_ESCAPE))
    {
        //any MessageBox()
        int result = MessageBox(hWnd, "I'm causing so much trouble!", "IMMORTAL", MB_YESNOCANCEL);
        if(result == IDYES)
            //end
    }
}

EDIT: The catch in PollKeyboard() displays the sequence 53gd6bcc, I could not, however, find the error code it corresponds to.

EDIT: After another test, I saw that even if the MessageBox is not within a KeyDown() if statement, the glitch still occurs.

EDIT: After a bit more testing, it appears that MessageBox itself is causing the glitch.

Answer 1

Because the sample code works, something else in your program is causing the bug. Try moving bits of the code below into your own until it works, then you will know which section of code was culprit.

Sample Code

Alright, huge code block coming up. This code works correctly for me. (Escape, along with all other keys successfully activate and deactivate). It's large, commented, and explains things fairly well. Try this, if it works, we'll examine other parts of your program, if not, I can only leave you with "Good luck", and take what you want:

// DirectInput
#define DIRECTINPUT_VERSION 0x0800
#include<dinput.h>

// Standard stuff
#include <iostream>
#include <stdexcept>
#include <sstream>
#include <string>

// Link from code, MSVC specific, could be done in project settings
#pragma comment(lib, "dinput8.lib")
#pragma comment(lib, "dxguid.lib")

// Utility lexical_cast, use Boost if possible.
// Simple replacement, converts a stream-able `T`
// to a string
template <typename T>
const std::string lexical_cast(const T& pValue)
{
    std::stringstream ss;
    ss << pValue;

    return ss.str();
}

// Utility function + macro to execute DirectX code with exceptions.
// Kinda ugly, but helpful for us.
void check_error(HRESULT pResult, const std::string& pFuncName)
{
    // DI_OK == S_OK, but S_OK is more general, so we'll use that
    if (pResult != S_OK)
    {
     throw std::runtime_error("Error executing: " + pFuncName +
           "! Returned: " + lexical_cast(pResult));
    }
}

// Macro, makes calling the function easier. It is wrapped in
// an `if` statement for reasons outlined in:
// http://www.parashift.com/c++-faq-lite/misc-technical-issues.html#faq-39.5
#define CHECK_ERROR(x) if (true) { check_error(x, #x); } else (void)0

// The above gives the warning:
// "warning C4127: conditional expression is constant", disable below:
#pragma warning(disable: 4127)

// Manages input
class input_manager
{
public:
    // Constants
    static const int NumberKeys = 256;

    // Creation
    input_manager(void)
    {
     // Create input and keyboard (like I said, ugly macro, but helpful :] )
     CHECK_ERROR(DirectInput8Create(GetModuleHandle(NULL), DIRECTINPUT_VERSION,
      IID_IDirectInput8, reinterpret_cast<void**>(&_input), 0));


     CHECK_ERROR(_input->CreateDevice(GUID_SysKeyboard, &_keyboard, 0));
     CHECK_ERROR(_keyboard->SetDataFormat(&c_dfDIKeyboard));
     CHECK_ERROR(_keyboard->Acquire());
    }

    ~input_manager(void)
    {
     // Free resources. Note: Many programmers
     // unnecessarily wrap this stuff in
     // `if (_keyboard !=0)`, and then
     // `_keyboard = 0`. This is completely unnecessary,
     // because destructors are only run one time.

     // Also, I can assume they are valid, because if they
     // weren't, we wouldn't be here (an exception would have
     // been thrown)

     _keyboard->Unacquire();
     _keyboard->Release();
     _input->Release();

     // Also, if we wrapped this into a nice RAII class, we wouldn't
     // be forced to write a destructor, but this is outside the scope.
     // Feel free to ask how; additionally, since we're on the topic, if you'd
     // like more tips handling input (I've written PLENTY of input managers)
     // I'm free for asking about things like testing for triggers rather than pressed
     // ("was it pressed, regardless if it's being held now" versus
     // "is it being pressed"), etc.
    }

    // Operations
    void update(void)
    {
     CHECK_ERROR(_keyboard->GetDeviceState(NumberKeys, reinterpret_cast<void*>(&_keys)));
    }

    // Query
    bool key_pressed(int pKey) const
    {
     return test_key(pKey);
    }

    // Might wrap into an operator[] for convenience.

private:
    // Constants
    static const int PressMask = 0x80;

    // Sorry for the confusion, but indeed, with
    // `char`s the mask is simply 0x80.

    // Utility
    bool test_key(int pKey) const
    {
     return (_keys[pKey] & PressMask) != 0;
    }

    // Members
    LPDIRECTINPUT8 _input;
    LPDIRECTINPUTDEVICE8 _keyboard;

    char _keys[NumberKeys];
};

void test_keys(const input_manager& input)
{
    bool anyPressed = false;

    for (unsigned i = 0; i < input_manager::NumberKeys; ++i)
    {
     if (input.key_pressed(i))
     {
      std::cout << "Pressing: " << i << std::endl;

      anyPressed = true;
     }
    }

    if (!anyPressed)
    {
     std::cout << "No keys pressed." << std::endl;
    }
}

void execute(void)
{
    input_manager input;

    std::cout << "Press Q to quit." << std::endl;

    bool running = true;
    while (running)
    {
     input.update();

     if (input.key_pressed(DIK_Q))
     {
      running = false;
     }

     test_keys(input);

     Sleep(0); // give some processor time
    }
}

int main(void)
{
    // Place real code in an execute function, so main
    // is clean and ready to catch exceptions:
    try
    {
     execute();
    }
    catch (const std::exception& e)
    {
     // Error!
     std::cerr << "Unhandled exception:" << e.what() << std::endl;
    }
}

Old suggestion:

Try catching the return value from GetDeviceState:

HRESULT result =                              // v Prefer C++-style casts
    di_keyboard->GetDeviceState(sizeof(keys), reinterpret_cast<void*>(&keys);

if (result != DI_OK)
{
    // uh-oh
    std::cout << result << std::endl;
}

Compare it against the table here.

Old Semi-Answer:

Shortly after editing in the code in the Extra Stuff section, I realized the error, sorry I didn't catch it earlier. You're testing the wrong bit :)

Observe:

//                                     v HERE! Should be 0x8000, not 0x80.
return (GetAsyncKeyState(pKeyCode) & 0x8000) != 0;

Try that:

int KeyDown(int key)
{
    return (keys[key] & 0x8000);
}

Also, this should be moved into a constant to avoid magic numbers:

// somewhere, probably in the private section of the class or in a detail namespace:
static const int PushedMask = 0x8000;

// code reads better:
int KeyDown(int key)
{
    return (keys[key] & PushedMask);
}

Lastly, in C++ you have a bool type, so take advantage of it!

// v here
bool KeyDown(int key)
{
    return (keys[key] & PushedMask);
}

I know Visual Studio will warn about this conversion from int to bool, so you can get rid of it while also making your intents clearer:

bool KeyDown(int key)
{
    return (keys[key] & PushedMask) != 0; // or == 1, your choice
}

Extra Stuff:

Try the following code:

#include <iostream>
#include <windows.h>

bool key_pressed(int pKeyCode)
{
    return (GetAsyncKeyState(pKeyCode) & 0x8000) != 0;
}

void test_keys(void)
{
    for (unsigned i = 0; i < 255; ++i)
    {
     if (key_pressed(i))
     {
      std::cout << "Pressing: " << i << std::endl;
     }
    }
}

int main(void)
{
    bool running = true;
    while (running)
    {
     if (key_pressed(VK_ESCAPE))
     {
      running = false;
     }

     test_keys();

     Sleep(0);
    }
}

This works for me (responds to all keys, quits on escape). Minimal test case for GetAsyncKeyState. If this does not work, please add OS, Keyboard, etc, in your comment.

Answer 2

If you create a MessageBox(Null,...) you won't have any control over the window after it's creation. IE, the window won't disappear when you depress the key.

As for why it keeps on appearing, seems to have something to do with this:

const int input_bit_num = 0x8000;
char keys[256];
bool KeyDown(int key)
{
    return (keys[key] & input_bit_num) != 0;
}

keys consits of 1 byte long characters, while input_bit_num is a 2 byte value. While I don't honestly know which bit it is that you're looking for (0xff - 0x00 is the domain of 1 byte).

Honestly, I'm surprised that your code runs, unless the & operation is carrying over into keys[key-1] in which case any KeyDown is undefined, and KeyDown(...) when key is 0 is particularly dangerous.