If Statement Being Stepped Into When It Shouldn't.

First, I posted the code at http://pastebin.com/AqRvzb0V because it's somewhat large (225 lines). I posted the whole code just to be thorough.

I am having an issue where an if statement is being stepped in to without evaluating to true. Here is the if statement you will find in the code:

 
if ((GetKeyState(VK_F8) == -128) || (GetKeyState(VK_F8) == -127))


This clearly states that if F8 is pressed (either toggled or not), then execute the statements inside the curly brackets. Well, soon as I fire up the game and am loaded into single player, I am spammed with the message defined by char NotInNetwork[100], without even pressing a single key on my keyboard yet. The GetKeyState API was originally the matching GetAsyncKeyState in the sibling if statement above, however, I changed it because I thought it was perhaps the culprit.

How this code works:
Basically it's a DLL that is loaded in to the game, and the RunScript() is executed indefinitely until you exit the game. The overall project is pretty simple, and what I am not getting is that the sibling hotkey that is defined above my issue, which is Hotkey F9, does NOT have this problem.

Any and all help will be appreciated.. thank you
i didnt look at the full source code, but the only possible problem i see is the double ||'s. its only supposed to one | between the too, but since you have 2 different numbers i might be wrong, im not very experienced in game making
The || is the logical OR operator. Meaning if one of those two return true, then the if statement evaluates true. The only way for a false evaluation is if they are both false.

Edit: forgot to mention, the | is the bitwise OR operator when working with bits.. this is a simple true/false if statement
Last edited on
looking at [url=http://msdn.microsoft.com/en-us/library/windows/desktop/ms646301(v=vs.85).aspx]MSDN[/url] it states that GetKeyState() returns a short (16 bits) and the high order bit represents keydown state and the low order bit represents toggle state (caps lock etc).

you need to test the specific bit and not the entire return value

1
2
3
4
if (GetKeyState(VK_F8) & 0x8000)  // msb set?
{
  // key is down
}


1
2
3
4
if (GetKeyState(VK_F8) & 0x0001)  // toggle set?
{
  // key was toggled
}


although VK_F8 is a bad example for the toggle.

edit:
Notice the single & that is for bit operations, don't confuse it with && for logical operations (bools)


Last edited on
+1 to Jaybob66.


You should always use the & operator to isolate the MSB when using GetKeyState/GetAsyncKeyState.
I didn't take the bit checking route because I was inexperienced with doing so and I had checked the MSDN, but they failed to provide an example of bit checking for the inexperienced. (I have no idea where you pulled 0x8000 and 0x0001 from)

However, shouldn't my if statement work as well despite not being a bit check? I ran tests on my keyboard and the following seems to be true on what GetKeyState returns:

Returns 0: Not down, not toggled
Returns -127: Down, not toggled
Returns 1: Not down, toggled
Returns -128: Down and toggled

Is there a downfall to my check versus the bit check?

BTW: I connected the two if statements in the source code by "else if" and all seems to be OK now, but it was still odd that it was stepping in to the if statement when by itself without keys being pressed. Is this the drawback of not using bits or is something else awry?
(I have no idea where you pulled 0x8000 and 0x0001 from)


It's basic binary:

GetKeyState returns a 'SHORT' which is 16-bits wide. This means the highest bit.. or the "most significant bit" (MSB) is bit 15, and the lowest/least significant bit is bit 0.

To calculate the value of a specific bit, you do (1<<n), where 'n' is the bit number (ie: 0 for LSB, and 15 for MSB in this case).

(1<<15) = 32768 = 0x8000 (hex)
(1<<0) = 1 = 0x0001 (hex)

However, shouldn't my if statement work as well despite not being a bit check?


Not necessarily. GetKeyState only guarantees the state of bits 0 and 15. The other bits could have completely arbitrary/random values. Just because it works on your machine doesn't mean it will work everywhere.

Is there a downfall to my check versus the bit check?


Yes. You could potentially fail to acknowledge keypresses that happen. For example, GetKeyState may return -4. Since -4 has the MSB set and the LSB clear, this would indicate the key is down, but not toggled. However your code would not detect it as such.

-4 is also a perfectly legal value for the function to return. Just because you don't get it on your machine for this key doesn't mean it's the same for everyone.

it was still odd that it was stepping in to the if statement when by itself without keys being pressed. Is this the drawback of not using bits or is something else awry?


It's hard to say. I find it doubtful that it was caused by the & issue.... since if anything your code would fail to detect actual keypresses -- not detect keypresses that never happened.

What might be happening is GetKeyState buffering weirdness. Like you pressed F8 at one point but didn't process the WM_KEYUP event, so GetKeyState still thinks F8 is pressed even though the user released it a long time ago. Using GetAsyncKeyState instead of GetKeyState would solve that issue if that was indeed the problem.

Further reading on the difference between the two can be found here:
http://blogs.msdn.com/b/oldnewthing/archive/2004/11/30/272262.aspx
This whole thing is for a game add-on. In my opinion I would rather NOT use either GetKeyState or GetAsyncKeyState (because of queuing) and have researched Raw Input but haven't found enough relevent information on the topic to implement it or even if it would bypass key queuing. When I accidentally hold a key for more than a split second and the action performs numerous times, it is quite annoying, therefore the need to bypass queuing.

Im thinking perhaps a list of sorts that holds the VK_KEY and a GetTickCount() DWORD value and not execute certain hotkeys before a certain amount of time has passed. I have also researched DirectInput since it is a 3D game, however, a lot of negativity surrounds that subject about reliability so I haven't studied that yet, not to mention an entire framework to learn is daunting.

Thank you for Disch, Jaybob66 and JayzisLazy for enlightening me on bit usage. Your fountains of knowledge are amazing :D
When I accidentally hold a key for more than a split second and the action performs numerous times, it is quite annoying, therefore the need to bypass queuing.


Well that's not really "queuing". What's happening is that GetAsyncKeyState gives you the real-time status of a key. So if you are checking that status 60 times per second... if the user holds that key for longer than 1/60th of a second, you are going to get its status as "pressed" multiple times.

What you want is to check the status when the key is first pressed. You can do this by keeping track of its previous state, and only taking the action when the state transitions from released to pressed.

Example:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
// elsewhere in the code:
bool F8down = false;


// where you want to check if F8 was pressed:
if(GetAsyncKeyState(VK_F8) & 0x8000)
{
    // F8 is currently held down, but was it just pressed?
    if( !F8down )  // if it was previously released.... then yes
    {
        F8down = true;
        // <- .... do whatever stuff you want to do when F8 is pressed here
    }
}
else  // else, F8 is not being held down
    F8down = false;
Last edited on
Topic archived. No new replies allowed.