UPDATE
Ignore all patches in this thread. A patch has been submitted to the developer mailing list.
END OF UPDATE

Patch is attached. ^[1]

Reproducing/Demonstrating the problem without patch:

1.) Run ex_keyboard_events in Windows.

2.) Press and hold SHIFT+P in the main Window and see how the modifier flags and the keyboard events are correctly displayed in the log.

3.) While still holding the keys from step 2, switch away from the main window by clicking something else.

4.) Release all keys.

5.) Click back into the main window and press any key and see how the SHIFT modifier flag is incorrect.

Patch Description:
The patch causes the entire keyboard state and modifier and toggle modifier flags to be reread and translated from a Windows API call upon switching back into the window to fix the key state (especially the modifier flags) from being out of sync with the current physical keyboard state.

The patch does not clear the keyboard state on switching out to achieve consistency with an event based handling (where you would not receive KEY_UP messages either if you switch away while you're still holding down something).

I also added some modifier flags which seemed to be missing in the function which handled modifier flag updates (special L/RWIN keys and MENU key).

Demonstrating the fix after patching:
Do everything as in pre-patch except in step 5 observe how the modifier flag is correct.

Someone should test and confirm, before committing this.
I tested on Windows 7. It may not work correctly in Wine.

Sigh. Let me try to explain.

So if the numpad Enter was pressed, I get the other Enter as a bonus? That's just wrong. I don't want to special case the Enter keys in my game, I want Allegro to take care of it. And currently it does that just fine.

From reading MSDN docs, I'm pretty sure that neither GetKeyboardState, GetKeyState, nor GetAsyncKeyState can actually do what you want here, as there is no bit for the extended key flag. In other words, this cannot be done properly.

What I think you should do is this:

1. Clear all keys.

2. Update the toggle modifier keys.

Nothing more. Don't update the non-toggle modifiers either, just clear everything. It's consistent, less code, easier to maintain. And games don't use the modifier keys as modifiers, they just use them as any other key.

Another thing you can do: Update the toggle modifiers in the keyboard driver initialization, this is missing. And, yes add the missing keys in update_modifiers.

torhu, you're quoting information I wrote about the old, wrong patch, which is why I won't respond to that here.

This patch is entirely new and does not do what the old patch did.

It only addresses what is described in the first post of THIS thread. Nothing more, nothing less.

If there are other issues, I need a clean description of the problem and the steps necessary to reproduce the erroneous behavior before I can patch it.

I did read your new patch before replying, and it does update the keys in basically the same way. So what I said still applies.

Your examination of the new patch can not have been very thorough then as it clearly does additional things and some things different and also, what's more important, does it only on switching-in and not all the time like the old wrong patch (which was addressing different issues that this one is not addressing).

Clearing the state on switching out would cause an inconsistency with the event system:
If your keyboard handling code is based on events then if you switch away, you do not get a key-up message for any keys being depressed while switching away.
This is why that is not addressed in this patch. If the state would be cleared on switching out, keeping consistency would mean to also generate key-up messages on switching out.

I repeat again: This patch only fixes the wrong modifier flags as described in the OP.

I also repeat again: The information from the old thread you quoted does NOT apply to this patch. The behavior is different.

Unless you generate "fake" events to sync it up, the state and the events will be out of sync anyway. This is not a big deal. Just forget about this.

I think that you need to stop for a bit, go back and actually read and understand the code, starting here.

And then do some testing with ex_keyboard_events, pressing different kinds of keys on your keyboard. See what ALLEGRO_KEY_* codes they generate. Especially the extended keys, and the numpad keys, with Num Lock on and off, etc. Because your patch does not give the same ALLEGRO_KEY constants as the event system does.

Please make some effort to understand how things work. It's hard to create a good patch if you don't know what you are doing. I improved and bugfixed some of the Allegro Windows code, and to do that I had have a good understanding of it. I am not making things up when I say that your patch has problems.

Maybe since you have a better understanding of the problems involved, you could make the necessary fixes?

I guess, but I shouldn't be coding since my left shoulder is fucked up for the time being

I thought I had given pretty exact intructions how to do this, but I don't seem to get through

torhu: Are you saying when returning after the switch, we should not use GetKeyboardState() but another function to update the state?

No. The state cannot be properly updated, so don't try either. See this.

EDIT:
Well, there is the possibility that the array that GetKeyboardState returns actually contain all the keys, even the ones that don't have their own VK codes. But someone would need to figure this out first. In that case, it can be done.

From reading the patch it looks like that's what the GetKeyboardState() is for.

The problem here is that in my opinion, you're displaying quite the annoying and immature attitude and I'm tempted to simply ignore you (I am aware of the fact that this makes me seem immature as well but you're not exactly making this easy).

You also seem to fail at understanding the very limited scope and purpose of this small patch in this thread.

I also doubt you tested the new patch because if you had, you would have noticed that the state is being correctly reported by al_get_keyboard_state after switching back in because after the fire-just-once-on-switch-in update it is immediately based on the event maintained state afterwards which already was correct before the patch (before the old one which has already been reverted).

I explained why the state can not be cleared on switching out.

There are other issues. Those can be addressed in another patch outside of the scope of this one as described in the OP.

The state can be properly updated in that case, because the respective function to do so to acquire the state from the Windows API is called within the correct thread. Please stop spreading false information.

The only case where it would not work correctly (by system design) is when the Window does not have input focus. That's consistent with the event driven system however so it's a non-issue.

This is from your patch. Note how it loops over all the 256 elements. Is this not the patch you are talking about? This loop will cause the key state to be wrong, like I have been repeatedly trying to tell you. What you need to to is to update only some of the keys. I suggest only Scroll Lock, Num Lock and Caps Lock. The update_toggle_modifiers function does this for you already. Updating the other modifiers, is an option, but not much point IMHO. Since you can't update all the keys correctly, I think it's better to make all keys (except the toggle modifiers) behave consistently.

I think we should just stop this here. I will do it later this summer.

No, it does cause the key state to be synchronized with the key state that the windowing system (Windows) intends for the application.

An application is not intended/supposed (by the system) to know of any keys being pressed while it does not have input focus. It is also not supposed to get key up messages while it does not have input focus. It is only supposed to know the last state as it was when it lost focus.

When it gets focus back it is only natural that you need to press a key again to get another key down message or in this case an updated state (which you will get correctly even by al_get_keyboard_state because the Windows events will update the state after that initial correction).

It's not optional. If shift is not pressed anymore after switching back in, pressing just P should not still report shift. That's what this patch fixes.

So you are suggesting that in the initial example, when you press P, the .modifiers field of the event should have the SHIFT bit set, even though shift is not pressed?

It's a very minor issue of course, but it seems to make more sense to not set it (and it's also what we do under X11).

There are other issues (which I repeat, this patch does not address, I lost count how many times I stressed that).

For example, if a key is being held while switching in, the event system never gets a key-down and thus it can't be correctly reported in the state.
To fix that the hw_to_mycode array would have to be further decoded/filled but then al_get_keyboard_state would be more accurate than the event system causing another inconsistency, unless we'd also use the information from them to generate a couple of fake (Allegro)events after switching in.

This would then not be consistent with the default behavior of Windows applications only relying on (Windows)event (not Allegro Events) based input.

But again, that will have to be fixed in a separate patch.

So wait, if you hold a key down, switch away, and switch back in, that key is now no longer pressed according to allegro or the app? I don't think that makes as much sense as you think it does. Broken key state ftl.

Okay, looks like I'll be up all night anyway...

Is this your original issue? Like said earlier: just clear the state (as in the_state) on switch in. With a single call to memset(). That's it. In addition, I would call update_toggle_modifiers().

Now I think you're just looking for mistakes in my posts Like I said: Windows does not seem to give you enough information to do this through the keyboard state API. Only through window (WM_KEYDOWN etc.) messages. Would be nice if it did, but finding out requires testing as it's not documented. And I think just clearing all keys (except the 3 toggle modifiers) is a good solution, instead of updating some keys but not others.

Just for fun I tried confusing the Allegro 4 game Saucelifter by switching in/out while holding keys, etc. And it didn't take much to mess it up. This is just for illustrating the problem, I know we are talking about Allegro 5. It's just not going to be perfect no matter what we do. But it can be made a bit less noticeable.

Not in your posts, but in what you think might be a good solution.

So the state API doesn't give you the current state of the keyboard? That seems a bit broken. Maybe I'm misunderstanding what you're talking about here.

I must be misunderstanding what you're talking about. As WM_KEY events are very broken afaik. It won't send key up events if a key was released outside of the app. Where as, the key state stuff should tell you what the current state is.

But that leads to very broken state. I think we should fix it, rather than leaving it broken.

In either case, I don't think allegro should be reporting broken state at all. If allegro's view of the state doesn't match the actual state of the keyboard (regardless of what windows may say it is at any given time via WM_KEY events), its just broken, and needs to be fixed. Even if that means patching up the key state, and sending synthesized events.

To be honest, I think the Allegro 5 key state api was a bad idea to begin with.

Of course it does. It's just that Allegro presents a different view of the keyboard, which requires some extra information to create (I have already explained this, please don't ask again). The Windows keyboard state stuff is probably very old, and they haven't updated the API to match the capabilites of the window message system. Maybe because they expect people to use one of the event/message systems instead. Which is what both A4 (yes, it does) and A5 does.

It's not really broken. It's just that you can't hold down a key while switching to your A5 game and expect it to register. You switch to it, then start playing. Not a big deal, and I wouldn't be surprised if many non-Allegro games behave like this. It's just that you don't notice, because you generally don't do this unless you are testing you keyboard input system

What you would notice is keys that get "stuck" in the pressed state. Clearing the key state fixes that. Which is why I keep suggesting it.

Does other GUI systems do this? I have no idea. That's a different discussion than this anyway. At least it's a different patch.

If you're going to be as antagonistic as you have been in the past couple threads, why should I bother go read your rather negative posts?

Especially considering you conveniently push aside points others bring up and claim they aren't an issue, but YOUR problems are all important

Give us a calm and thorough explanation of all the issues, and maybe we'll take your word for it.

This code is taken from the 'key pressed' event handler in wkeyboard.c that torhu linked to:

   /* Check for an extended key first. */
   my_code = 0;
   if (extended)
      my_code = extkey_to_keycode(vcode);

   /* Map a non-extended key.  This also works as a fallback in case
      the key was extended, but no extended mapping was found. */
   if (my_code == 0) {
      if (vcode == VK_SHIFT) /* Left or right Shift key? */
         vcode = MapVirtualKey(scode, MAPVK_VSC_TO_VK_EX);
      my_code = hw_to_mycode[vcode];
   }

Allegro aims to represent ALLEGRO_KEY_ENTER and ALLEGRO_KEY_PAD_ENTER as two entirely separate keys. But let's look at how Allegro does that:

• We get ALLEGRO_KEY_PAD_ENTER from the 'extkey_to_keycode' function in the case where vcode is 'VK_RETURN'. This only happens if 'extended' is set.

• We get ALLEGRO_KEY_ENTER from the 'hw_to_mycode' array, which is only used if the 'extkey_to_keycode' part didn't work correctly. So it doesn't happen if my_code has already been set to ALLEGRO_KEY_PAD_ENTER.

• ALLEGRO_KEY_ENTER appears in slot 0x0D. I checked, and VK_RETURN is 0x0D. So Windows must provide both Enter keys via the same VK_ constant, but with an 'extended' boolean to differentiate.

This is from Dennis's patch:

+        for (i = 0; i < 256; i++) {
+           if (win_key_state[i] & 0x80)
+              _AL_KEYBOARD_STATE_SET_KEY_DOWN(the_state, hw_to_mycode[i]);
+           else
+              _AL_KEYBOARD_STATE_CLEAR_KEY_DOWN(the_state, hw_to_mycode[i]);
+        }

When that loop hits i==0x0D, it will update ALLEGRO_KEY_ENTER. But it's possible that Windows will have set win_key_state[0x0D] if ALLEGRO_KEY_PAD_ENTER is held down. If so, then ALLEGRO_KEY_ENTER will be set incorrectly. Have you checked for this?

Even if the loop doesn't introduce an error here, it is still incomplete, as it will never set the entry for ALLEGRO_KEY_PAD_ENTER (because ALLEGRO_KEY_PAD_ENTER doesn't appear in the hw_to_mycode array).

(The other thing I was going to mention is that the loop indiscriminately writes garbage to the_state[0] for all the keys with no entries in hw_to_mycode, but then, the existing code does that too.)

Right - now that all that is said:

torhu, you're in danger of driving away a possible contributor. You could tweak your explaining-fu as follows:

You've stated that Allegro "takes care of it" so you don't have to "special case the Enter keys". You should say exactly what Allegro does, which is to treat the Enter keys as separate unrelated keys.

In order for this to make sense, you have to explain what the extended key flag is.

If you don't get through, don't shout louder. On the contrary, if you make the effort to explain, then Dennis will either reach the same conclusion or win you over to his view, and no one will need to use any underlines.

You're absolutely right to defend Allegro 5's best interests - don't stop doing that - just find a more diplomatic way to do it.

[APPEND]
Tomasu, Dennis and elias, on the subject of broken state:

If I'm playing a game and something like Windows Update or a calendar reminder pops up and steals focus, then there are a number of things I'd like the game to do:

• Pause if it's a real-time game - this is ideal and can be accomplished if the game uses events. It can hook the 'focus lost' event.

• Release all keys if it's a non-real-time game - then I don't run the risk of a 'use resource' operation auto-repeating, a puzzle map scrolling way further than I wanted it to, etc.

When I go back to the game, I will happily release and re-press a key if I was holding it and it doesn't respond the first time. This is accepted behaviour, as is very rare anyway since the default physical state of a keyboard is 'all keys up' anyway.

On the other hand, if my player is moving to the right the whole time, it isn't intuitive to have to press the right arrow again to stop the movement. I usually naturally try to fix it by pressing the left arrow first a few times, which you can imagine accomplishes nothing and is quite frustrating.

So pragmatically speaking, the 'broken state' torhu is recommending is a very practical option that will make all real users happy. It's a perfectly good solution if Dennis's approach can't be implemented safely, and it's definitely better than accidentally setting state for a key that isn't held down.

(Nothing wrong with the state API. Just fix the bugs, as you would in any other piece of code. Forcing your users to worry about mismatched KEY_DOWN and KEY_UP events, just because Windows does, is on the other hand quite negligent. I would advocate synthetic KEY_UP events on focus loss, perhaps optional but enabled by default, or perhaps flagged as synthetic inside the event object. Oh yeah, another thing that might be good if it's not currently possible - the ability to get the full keyboard state as it was at the time of the current event, as opposed to the latest possible state.)

Thanks for your input, Bruce. This thread needed it.

To be fair, this discussion started in another thread, where I already have explained what "extended" keys are, probably more than once. And I didn't start "shouting" until having posted the same information multiple times and having it seemingly ignored. I was hoping that underlining the main points would help. As for the Enter keys, I think he already knew that Allegro treated them as two different keys. I'm posting this clarification in the hope that more people won't start to "gang up on me". Please treat this as a technical discussion, not a political one.

Dennis: If my suggested solution is not the right one, please explain the original issue.

And you can do that yourself. I just don't think we should make it impossible to get correct keyboard state/events in the case of switch outs.

Here's how to reproduce this with the current code (using the patch from this thread):

1. Modify ex_keyboard_events to report the ALLEGRO_KEY_ENTER state. E.g. like this:

static void log_key(char const *how, int keycode, int unichar, int modifiers)
{
   char multibyte[5] = {0, 0, 0, 0, 0};
   const char* key_name;
   ALLEGRO_KEYBOARD_STATE s;

   al_utf8_encode(multibyte, unichar <= 32 ? ' ' : unichar);
   key_name = al_keycode_to_name(keycode);
   log_printf("%-8s  code=%03d, char='%s' (%4d), modifiers=%08x, [%s]\n",
      how, keycode, multibyte, unichar, modifiers, key_name);
   
   al_get_keyboard_state(&s);
   log_printf("Enter is pressed: %s\n", al_key_down(&s, ALLEGRO_KEY_ENTER) ? "yes" : "no");
}

2. Run the example, highlight the log window and press and hold the numpad enter key
3. While holding the numpad enter key, switch to the main window
4. You'll note that Allegro is erroneously reporting the enter key is pressed
5. Release the numpad enter key and press some other keys (not the enter key though)
6. You'll note that the enter key is still reported as pressed

Thanks, SiegeLord. Let me note that this is just the tip of the iceberg, there are about 15 keys that are problematic. The End key and the numpad End key is another example of keys that have this relationship.

Ok, so to summarize my understanding:

• One minor problem in current SVN is that the .modifiers field of ALLEGRO_KEY_DOWN messages has wrong information (SHIFT appears pressed when it is not) when returning after a switch. The patch in this thread fixes that. Except for modifiers which are extended keys, I don't know whether any of them are - does anyone know?

• al_get_keyboard_state() has stale information when returning after a switch. The patch fixes this, except for extended keys, as explained by entheh and as seen in SiegeLord's example. For those keys the behavior is the same as before the patch.

So, I'd say the patch should be modified to not touch al_get_keyboard_state at all - only fix the event modifiers issue for now.

A separate patch may do something about al_get_keyboard_state afterwards, but according to torhu that may not be possible for extended keys.

(The event generation and switching behavior are separate issues but already work as intended anyway.)

I'll also try to summarize all the issues at hand again (even though this patch was never meant to fix ALL of them).

The misunderstanding here was that it was assumed that the patch in this thread was meant to fix more than the wrong modifiers for now.

It was repeatedly wrongly assumed that the patch was also meant to fix ALL the other issues from the old thread.

The other misunderstanding is that we have different opinions about how the application should behave and what key state changes and key events it should be aware of, depending on Window state.

The one camp want's to simply clear the whole internal state(this should include all key state also for modifiers) on switching out.

The other camp argues that the internal state and the event based approach should display consistent behavior and that since no key-up event is generated on switching away, the state should not get cleared either because that would be the equivalent to a key-up event.

Before further (correct by definition of what the system should do) patching to fix other issues can be made, we need to reach consensus on a few things:

1. Should usage of event based keyboard handling and state based keyboard handling be consistent?
1.a) yes
1.b) no

If 1.a) (keep consistency) then:
2.) How should consistency be achieved?
2.a) keep everything as it is: state will keep last known state before switching away (consistent with no extra events as they are currently not generated, so a program re-acting to just key-down will just as a state based program still think the key is being held)
2.b) clear state and generate (Allegro)key_up events for all keys being held while switching away
2.c) keep state and update state after switching in as good as possible (technically not possible for some keys, maybe possible but needs further investigation of the data returned by the Windows API calls) and generate key-down events for keys being held while switching in

If 1.b) (give up consistency) then:
3.) Should the state simply be cleared on...?
3.a) switching away
3.b) switching back in
3.c) switching away and switching back in

This patch could be best described as a part-solution on the way to achieve 1.a) with option 2.c).

Without using this patch and making a different patch, the technically easiest to achieve behavior would be 1.b) with 3.c).

Another technically possible behavior would be 1.a) in combination with 2.b) to get consistency for switching-away behavior.
That would not fix inconsistencies on switching in though except if in addition to 2.b), 2.c) would also be implemented.

So, in short, the most we could do would be 1.a) with 2.b) AND 2.c) in an attempt to keep consistency.
The easiest we could do is 1.b) with 3.c) while throwing consistency overboard.

I'd say consistency between events and state is not important. Events should always correspond to WM_ messages 1:1, for keyboard as well as switch events. We do not and should not create any fake messages.

We do want correct .modifers fields for events though.

The state API has a different purpose, it's supposed to give a snapshot of all keyboard keys at the time you call the function. So there is no concept of consistency between the two APIs, they are not meant to be (but if the implementation makes them, it's not a problem).

Why not? We paper over issues in the underlying platforms all the time. This would just be another case of us working around an issue in the underlying platform.

Hm, maybe. We should first find agreement on applying a fix for the .modifers field though since that's all I'm interested in

There are these two (inside extkey_to_keycode).

case VK_CONTROL:  return ALLEGRO_KEY_RCTRL;
case VK_MENU:     return ALLEGRO_KEY_ALTGR;

Still, there are VK_RCONTROL and VK_RMENU too... I don't know if Windows gives you the correct data for those keycodes, or not.

No, it's not. Before the patch you couldn't get numpad enter to cause enter to be pressed. This new behavior is not an omission of a fix, but an introduced bug. The only way to get this quoted statement to be true would be to ignore the keys that have extended key variants, which this patch does not do.

And on Windows the only way to guarantee this is to make it consistent with the WM_ messages, and thus consistent with Allegro events (not fake events though).

I think this is the best solution (/if/ it's even possible, given the beginning of my post) given the current understanding of what's possible under Windows. This decouples modifiers from al_get_keyboard_state (as they are already on Linux, for example) and keeps events consistent with the keyboard state.

In current SVN if you keep any key pressed while switching out, it will be reported as stuck. With the new version, only if you keep one of the extended keys presse it will still be reported as stuck. So the new version does not completely fix the problem, but certainly not introduce a new one. (Note that al_get_keyboard_state is not modified except for the re-sync on switch-in.)

No. You press the numpad enter and the regular enter key gets stuck. In my example you never press the regular enter key, and yet it's reported as pressed.

SiegeLord, you're right. It is an introduced bug due to the patch being incomplete and not fixing all the other issues yet.

Elias, I have attached a new patch^[1] to this post which only attempts to read in the modifier flags and the toggle modifiers after a switch in, without attempting to get the states for all other keys. This fixes the wrong modifier flags in events.

Ok, now we have progress

Is the _al_win_wnd_call_proc stuff needed? I don't think so, but maybe you know something I don't. Removing it would simplify a lot.

But what is your actual goal here? It started out being only updating the modifier keys in the state returned by al_get_keyboard_state(). You wanted to get rid of stuck keys, I assumed. The solution is simple, just clear the state on switch in, call update_toggle_modifiers(). This fixes the actual problem: stuck keys. Nobody will know the difference, and the code is simple.

But now the char event modifiers field has entered the picture. The only reliable fix for that is to update the modifiers each time a char event is created in _al_win_kbd_handle_key_press(). GetKeyboardState is already being called there, since ToUnicode requires it.

If you want to fix both issues, just do both fixes.

There are some other things I should mention, but that can wait.

EDIT: Allright, I'm working on a patch...

UPDATE: Patch attached, please comment. Needs some more testing, and I can probably combine update_toggle_modifiers and update_modifiers into a single function, etc.

It does basically two things:

1. On keyboard initialization and at switch in it clears the keyboard state, but updates the toggle keys so that they match the lights on your keyboard. No stuck keys, but no complete keyboard state update either.

2. ALLEGRO_EVENT_KEY_CHAR's modifiers field is completely updated every time a char event is sent. This should be 100% reliable, but more testing still needed.

A question: the mouse grab key stuff. Is that still supported? Currently my patch breaks that.

UPDATE2: Just realized, one part of the patch is bogus

I don't think we should patch anything other than the incorrect modifier flags in the events until there is consensus about the behavior options I described above in the second half of my longest post.

We should have consistency across different systems. How is switching out/switching in handled on other platforms with regard to the keyboard state?

(and by keyboard state I mean everything: key states, modifier states, toggle key states)

Yeah, does anyone feel like running ex_keyboard_events on Linux and OS X and tell us if you get key up events on switch in or switch out? Other platforms could be interesting too.

You only get keyboard events when you have focus in X11.

Ok and what about the al_get_keyboard_state() function?
Does that still report keys as pressed which were held down while losing focus while you do not have focus?

Looks like nobody cares enough to bother testing. Attached is a simpler patch that just clears the state and the modifiers on switch in. And it doesn't break the mouse grab keys or anything else.

After the information that there are no "fake" events on Linux, this is how I see the relationship between state and events: If you use events for keyboard input in your game, you have to assume that anything can happen while your game does not have input focus, and you have to deal with that. But if you use al_get_keyboard_state, it's reasonable to expect an updated state each time you call the function.

With this patch, al_get_keyboard_state will much less frequently report keys that are up as being down. You can still trick it if you really want to. Anyway, I feel that reporting keys that are down as being up is less of a problem that the other way around, which is what we have now.

Discuss

Dude it's been two days.

That said I really don't care about the state api. IMO it's a crutch. To me, its like someone setting up a bitmap to do double buffering in A5 the way they did in A4.

Why is it reasonable to asume that the state api doesn't give you incorrect info if the events do?

Well, the docs for al_get_keyboard_state do say "at the time the function is called".

I used to use al_get_keyboard_state in one of my projects. But switched to maintaining my own state based on events, to avoid the occasional stuck keys on switch ins. The problem is you don't know if the state Allegro gives you might have been messed up by a switch out while keys were held down or not. And a stuck key is stuck until you happen to press it again, it doesn't go away by itself.

By the way, I had a look at the X and the OS X keyboard code, and I couldn't find any clearing of the state at switch in or switch out there. So now we know.

Ok, so all we should do is fix the modifier flags for now without touching anything else. ( this patch does that: http://www.allegro.cc/files/attachment/606198 )
([edit] To clarify: If on other platforms the state is never cleared, neither should the Windows implementation do so. [/edit])

That's technically impossible in Windows if it is called while the window does not have input focus (But we already knew that, which is why we starting having this discussion about clearing it in the first place to prevent stuck keys.)

So... what are we to do:
A reasonable thing to do is, leave everything as is (except for patching the modifier problem) and add a user level function 'al_clear_keyboard_state' which users can call themselves to achieve any key state behavior they consider most suitable for their application.
If they don't call that function on switch-out, al_get_keyboard_state, will continue to report keys which were pressed while switching away (consistent with never receiving a key-up event).

Tomasu, keyboard state is a very useful thing - I don't think it's reasonable to view it as a crutch.

On the other hand, I would be in favour of being able to get the state at the time of the event most recently pulled from the queue, as opposed to the most up-to-date state at the time of the call. That way you could mix the two mechanisms perfectly. Especially if you're using timer events to drive your main loop.

[APPEND]

Why don't you clear them on switch-out?

If something steals focus while you're scrolling a map, and that map continues scrolling, that's extremely stressful. No one will ever want keys reported held while there's no focus. Holding a key down is an active behaviour. No one wants that if they're not looking at the window right now. Ever.

That's very easy to do already (it's exactly how my wrapper works). And it never was the idea of the state API. I'd completely agree to having it work like that (or dropping it completely, as I often said on the mailing list in the past) though

But the state API as it is now is for when you want imprecise input like in A4 or for the rare cases where the events API does not work - right at startup, and after a switch-in - since you could not have got a key down event for keys pressed while the application was not running. (It seems this very use-case is broken in the Windows implementation though.)

Yes, that's probably better.

Well, I guess it's better than nothing. I still think just clearing them on switch in would fix the problem, though.

I have some comments on the patch in any case:

1. Is the _al_win_wnd_call_proc stuff needed? I don't think so, but maybe you know something I don't. What does it achieve?

2. If you don't use _al_win_wnd_call_proc, there's no point in finding the foreground window etc.

3. You could just call GetKeyState for the keys you care about, instead of GetKeyboardState. No big deal, though.

4. init_keyboard() doesn't update the modifiers, should it?

5. The call to update_toggle_modifiers is not needed, as it's called in _al_win_kbd_handle_key_press every time a char event is created.

6. You forgot VK_APPS (ALLEGRO_KEY_MENU)

You're right. I left most of the stuff in in preparation for a future attempt at fixing the other problems but since that's not possible, none of that is needed anymore and just clearing the modifier state is sufficient.

Have a look at this minimal modifier state patch then ^[1].

The patch looks fine as far as I'm concerned.

al_install_keyboard() is supposed to support being called multiple times, and I assume it's supposed to be threadsafe, but that's for another day

I have submitted that patch to the mailing list (in GIT format for the current 5.1 branch from the recently set up new GIT repository).