Why you should avoid while(GetKeyState("a", "P")) to detect release of key a

Put simple Tips and Tricks that are not entire Tutorials in this forum
User avatar
evilC
Posts: 4822
Joined: 27 Feb 2014, 12:30

Why you should avoid while(GetKeyState("a", "P")) to detect release of key a

30 Jun 2016, 05:21

Because it only works for one key.

I think we should avoid advising people to use this technique, as it has limitations that are not immediately obvious to most users.

Notice how with this code, if you hold a, then b, then release a, you do not get a "A RELEASED" tooltip.

Code: Select all

~a::
	tooltip A PRESSED
	while (GetKeyState("a", "P")){
		sleep 10
	}
	tooltip A RELEASED
    return
 
~b::
	tooltip B PRESSED
	while (GetKeyState("b", "P")){
		sleep 10
	}
	tooltip B RELEASED
    return
Use this technique instead:

Code: Select all

~a::
	tooltip A PRESSED
	return

~a up::
	tooltip A RELEASED
	return

~b::
	tooltip B PRESSED
	return
	
~b up::
	tooltip B RELEASED
	return
Note that KeyWait without a timeout is the same thing - the following code also exhibits this issue:

Code: Select all

~a::
	tooltip A PRESSED
	KeyWait, a
	tooltip A RELEASED
    return
 
~b::
	tooltip B PRESSED
	KeyWait, b
	tooltip B RELEASED
    return
Last edited by evilC on 14 May 2020, 05:42, edited 1 time in total.
User avatar
Nextron
Posts: 1391
Joined: 01 Oct 2013, 08:23
Location: Netherlands OS: Win10 AHK: Unicode x32

Re: Why you should avoid while(GetKeyState("a", "P")) to detect release of key a

30 Jun 2016, 06:14

To those who wonder why this is, read up on threading. Basically, when you press (and hold) a its hotkey fires, when you then also press b the execution of the hotkey of a is interrupted and the hotkey of b starts. Only when it finishes, will the hotkey of a resume where it left off.

Making the up-event a separate hotkey, makes an up-event interrupt any running hotkey too (although in this example the down-events will already have finished executing).

A side-effect of using an up-event hotkey is that it (and its corresponding down-event) will use the keyboard hook instead of registering the hotkey, which may be of concern.

On a side note, there are useful situations to wait for a key to be released by halting the execution of the hotkey like in the first example, for example to prevent key-repeat from executing a hotkey ~50 times per second as it would in the second example. So I wouldn't dismiss it outright, but instead of using the While GetKeyState() Sleep construct, you might as well use a more versatile and shorter KeyWait.
User avatar
boiler
Posts: 16767
Joined: 21 Dec 2014, 02:44

Re: Why you should avoid while(GetKeyState("a", "P")) to detect release of key a

30 Jun 2016, 13:29

Useful insight. Thanks for posting. I know I've had GetKeyState not work as I expected when waiting for a key release, and that's probably why.
User avatar
PipeDreams
Posts: 165
Joined: 19 Dec 2015, 00:20

Re: Why you should avoid while(GetKeyState("a", "P")) to detect release of key a

30 Jun 2016, 17:48

guest3456 wrote:uh, KeyWait ?
Is no good, it too has limitations.

Take for example, an early version of a SprintAssist script for KingsAndHeros I made. Just a bit of background about why I even made this at all, is because in order to sprint, you have to hold W and Shift simultaneously. I hate that, it's truly annoying. I wanted something that would allow me to hold W and quickly tap Shift to sprint, and this script works fine for that, and it will release W and Shift on the up stroke of W. However, the issue comes along when you are still holding W down after a short time, your stamina bar has depleted, and after it goes back up, I want to just tap LShift again while having never let up on the W key to initiate the original action, Holding W and tapping LShift to sprint. This happens because AHK is not multi thread, it is single thread, and the script is waiting for W to be released before it can do anything else. So basically it's stuck.

Code: Select all

~w & LShift::GoSub, SprintAssist
SprintAssist:
{	SendInput, {LShift DownTemp}
	KeyWait, W
	SendInput, {LShift Up}
} Return

What is the solution? To get this working properly you must do as evilC said, and use the "Key Up" command and you'll get a fully functional script like the one below:

Code: Select all

*$~w up::GoSub, w_up
~w & LShift::GoSub, SprintAssist
 
SprintAssist:
{	SendInput, {LShift Up}
	SendInput, {LShift DownTemp}
} Return
 
w_up:
{ SendInput, {LShift Up}
} Return

You can take it a bit farther by adding a double tap to the mix like so:

Code: Select all

*$~w::GoSub, DP_SprintAssist

DP_SprintAssist:
If W_Presses > 0
{	W_Presses += 1
	Return
}W_Presses = 1
SetTimer, W_Counter,
Return

W_Counter:
SetTimer, W_Counter, off
If W_Presses = 2
{	SendInput, {LShift DownTemp}
}W_Presses = 0
Return ;<<< DP_SprintAssist


Good post evilC! :bravo:
Last edited by PipeDreams on 14 Jul 2016, 19:44, edited 2 times in total.
User avatar
Masonjar13
Posts: 1555
Joined: 20 Jul 2014, 10:16
Location: Не Россия
Contact:

Re: Why you should avoid while(GetKeyState("a", "P")) to detect release of key a

01 Jul 2016, 10:28

evilC wrote:Because it only works for one key.

Code: Select all

i:=0

t::
while(getKeyState("t","P") && getKeyState("LCtrl","P"))
    tooltip,% i++
return
And what about if you're trying to loop an action?

Code: Select all

LButton::
while(getKeyState("LButton","P")){
    click
    sleep 10
}
return
Because of this, I thoroughly disagree with your statement to avoid it. If it is just sleeping, then yes, using a paired hotkey would be better, but that's not universal. Each method has it's uses; one shouldn't be preferred over the other, they should just be used appropriately. Looping an action while a key is held, yes, use a while-loop. Waiting for it to be released: if the code is strictly procedural, or does not need more than one running hotkey at a time, keywait is sufficient. If it's not, or if it's expected that hotkey execution will overlap, then using a paired hotkey would be best.

Different methods for different instances. I thoroughly discourage the mindset of there being one "right" way of doing something, it can ruin a creative mind.
OS: Windows 10 Pro | Editor: Notepad++
My Personal Function Library | Old Build - New Build
User avatar
Xtra
Posts: 2744
Joined: 02 Oct 2015, 12:15

Re: Why you should avoid while(GetKeyState("a", "P")) to detect release of key a

01 Jul 2016, 17:36

Masonjar13 wrote: Different methods for different instances. I thoroughly discourage the mindset of there being one "right" way of doing something, it can ruin a creative mind.
+1
User avatar
evilC
Posts: 4822
Joined: 27 Feb 2014, 12:30

Re: Why you should avoid while(GetKeyState("a", "P")) to detect release of key a

12 Oct 2016, 08:57

Masonjar13 wrote: And what about if you're trying to loop an action?

Code: Select all

LButton::
while(getKeyState("LButton","P")){
    click
    sleep 10
}
return
Am I missing something or is there no difference?
Your code exhibits the same problem.

Code: Select all

#SingleInstance force

~LButton::
while(getKeyState("LButton","P")){
    Send a
    sleep 10
}
ToolTip % "LButton Released"
return

~RButton::
while(getKeyState("RButton","P")){
    send b
    sleep 10
}
ToolTip % "RButton Released"
return
Hold LButton, Hold RButton, release LButton.
You do NOT see "LButton Released".
Furthermore, if you hold LButton, it starts sending A, but then as soon as you hold RButton, it stops sending A and only sends B.
Even if you release RButton, it does not start sending A again.

In order for this code to work, you would need to do something like:

Code: Select all

#SingleInstance force

F11::
	SetTimer, SendA, 10
	Gosub, SendA
	return

F11 up::
	SetTimer, SendA, Off
	return

SendA:
	send a
	return

F12::
	SetTimer, SendB, 10
	Gosub, SendB
	return

F12 up::
	SetTimer, SendB, Off
	return

SendB:
	send b
	return
User avatar
evilC
Posts: 4822
Joined: 27 Feb 2014, 12:30

Re: Why you should avoid while(GetKeyState("a", "P")) to detect release of key a

12 Oct 2016, 09:35

Masonjar13 wrote:Because of this, I thoroughly disagree with your statement to avoid it. If it is just sleeping, then yes, using a paired hotkey would be better, but that's not universal. Each method has it's uses; one shouldn't be preferred over the other, they should just be used appropriately. Looping an action while a key is held, yes, use a while-loop. Waiting for it to be released: if the code is strictly procedural, or does not need more than one running hotkey at a time, keywait is sufficient. If it's not, or if it's expected that hotkey execution will overlap, then using a paired hotkey would be best.

Different methods for different instances. I thoroughly discourage the mindset of there being one "right" way of doing something, it can ruin a creative mind.
My point is that if you teach people one method - it should work in all instances.
Many people simply do not understand the issues at hand, so by teaching a method that only works when you do it with one hotkey at a time, but as soon as you move to more than one hotkey then it starts behaving in an unpredictable manner, you just shot yourself in the foot and needlessly confused users.

https://autohotkey.com/docs/commands/GetKeyState.htm
There is absolutely no mention of this technique on this page, nor any mention of the limitations of GetKeyState loops for detecting up events.
If users are trying to get release events for joystick buttons, then they are basically screwed - the average newb will have absolutely no idea how to detect up events for more than one joystick button - hell even my technique does not work for joystick buttons because up hotkeys do not properly work for stick buttons. You must for example build an array holding names of held joystick buttons, then iterate through them.
User avatar
Masonjar13
Posts: 1555
Joined: 20 Jul 2014, 10:16
Location: Не Россия
Contact:

Re: Why you should avoid while(GetKeyState("a", "P")) to detect release of key a

12 Oct 2016, 15:41

evilC wrote: Am I missing something or is there no difference?
Your code exhibits the same problem.
I completely didn't remember your first post. But of course two loops will never run at the same time, AHK being single-threaded. I multi-thread when multiple, constant loops are needed. Timer works relatively well to allow a multi-loop-like environment, but if the loops are too large or take too much time, the other one won't be as timely as needed, usually. Being that, I can't agree that this method should be used quite like that.
evilC wrote: My point is that if you teach people one method - it should work in all instances.
I completely disagree. This is an unreasonable statement. For achieving accurate timing, a while-loop will be significantly better than having a bunch of timers running. Take an auto-clicker for example. You wouldn't want it to be interrupted by another timer that, say, couldn't perform a specific operation due to error and takes significantly longer to move past that, after the click sub has already been interrupted. Your clicker is then useless for an amount of time. While this isn't a high-chance thing, it still causes inconsistency. I've never actually had a need for what your talking about, and most shouldn't if they designed their scripts properly, but actual multi-threading would work in such a situation, quite well.

That is, using timers isn't a solution, it simply makes people think it is, until they have subs taking too long. It works in some cases, and is tragic in others. Which, brings me back to "Different methods for different instances." There simply isn't one way to do anything, in any case. Code structure should be completely dependent on how it needs to operate, as to get the expected outcome.
OS: Windows 10 Pro | Editor: Notepad++
My Personal Function Library | Old Build - New Build
User avatar
evilC
Posts: 4822
Joined: 27 Feb 2014, 12:30

Re: Why you should avoid while(GetKeyState("a", "P")) to detect release of key a

02 Mar 2018, 11:55

Masonjar13 wrote:For achieving accurate timing, a while-loop will be significantly better than having a bunch of timers running. Take an auto-clicker for example. You wouldn't want it to be interrupted by another timer that, say, couldn't perform a specific operation due to error and takes significantly longer to move past that, after the click sub has already been interrupted. Your clicker is then useless for an amount of time. While this isn't a high-chance thing, it still causes inconsistency. I've never actually had a need for what your talking about, and most shouldn't if they designed their scripts properly, but actual multi-threading would work in such a situation, quite well.

That is, using timers isn't a solution, it simply makes people think it is, until they have subs taking too long. It works in some cases, and is tragic in others. Which, brings me back to "Different methods for different instances." There simply isn't one way to do anything, in any case. Code structure should be completely dependent on how it needs to operate, as to get the expected outcome.
Please give me an example of how you can have a while loop that encounters an error and deals with it fine, but a timer executing the same code that does not.
Also demonstrate to me how you can work around the issue I mention in the OP using a while loop instead of timers.
User avatar
evilC
Posts: 4822
Joined: 27 Feb 2014, 12:30

Re: Why you should avoid while(GetKeyState("a", "P")) to detect release of key a

02 Mar 2018, 12:09

Also, I take issue with the statement "For achieving accurate timing, a while-loop will be significantly better"

Consider the following examples:

Code: Select all

while (true){
	Tooltip % A_TickCount
	Sleep 100
}

Code: Select all

SetTimer, DoLoop, 100
return

DoLoop:
	Tooltip % A_TickCount
	return
Which more reliably fires at 100ms intervals?
The SetTimer, because the while loop does not take into account the time that the body of the while loop takes to execute.

Also consider this code:

Code: Select all

#SingleInstance force
#Persistent
OutputDebug DBGVIEWCLEAR

lastT := 0

while (true){
	OutputDebug % "AHK|" A_TickCount - lastT
    lastT := A_TickCount
    Sleep 100
}

F12::
    OutputDebug AHK| KEY PRESSED
    Sleep 1000
    return
Result:

Code: Select all

[45212] AHK|109
[45212] AHK|109
[45212] AHK|110
[45212] AHK|109
[45212] AHK|110
[45212] AHK| KEY PRESSED
[45212] AHK|1031			; EPIC FAIL!
[45212] AHK|109
[45212] AHK|110
Oh dear, not what we wanted!

Now with SetTimer:

Code: Select all

OutputDebug DBGVIEWCLEAR

lastT := 0
SetTimer, DoLoop, 100
return

DoLoop:
	OutputDebug % "AHK|" A_TickCount - lastT
    lastT := A_TickCount
    return


F12::
    OutputDebug AHK| KEY PRESSED
    Sleep 1000
    return
Result:

Code: Select all

[31476] AHK|109
[31476] AHK|109
[31476] AHK|110
[31476] AHK|109
[31476] AHK|109
[31476] AHK| KEY PRESSED
[31476] AHK|110			; CORRECT!
[31476] AHK|109
[31476] AHK|110
[31476] AHK|109
[31476] AHK|109
[31476] AHK|110
Exactly what was intended
guest3456
Posts: 3454
Joined: 09 Oct 2013, 10:31

Re: Why you should avoid while(GetKeyState("a", "P")) to detect release of key a

02 Mar 2018, 15:56

how would you prefer to write this example?

Code: Select all

if !WinExist("Untitled - Notepad")
{
   Run, notepad.exe
   WinWait, Untitled - Notepad
}
notepadhwnd := WinExist()
return

q::
   MouseGetPos,,, qdownhwnd
   if (qdownhwnd = notepadhwnd)
   {
      KeyWait, q
      MouseGetPos,,, quphwnd
      if (quphwnd != notepadhwnd)
         MsgBox, mouse was over notepad at Q downpress`n and moved outside notepad by Q up release
   }
return

User avatar
evilC
Posts: 4822
Joined: 27 Feb 2014, 12:30

Re: Why you should avoid while(GetKeyState("a", "P")) to detect release of key a

02 Mar 2018, 16:42

Something like this

Code: Select all

if !WinExist("Untitled - Notepad")
{
   Run, notepad.exe
   WinWait, Untitled - Notepad
}
notepadhwnd := WinExist()
return

inNotepadWhileDown := 0

q::
   MouseGetPos,,, qdownhwnd
   inNotepadWhileDown := (qdownhwnd = notepadhwnd)
return

q up::
	if (inNotepadWhileDown){
      MouseGetPos,,, quphwnd
      if (quphwnd != notepadhwnd)
         MsgBox, mouse was over notepad at Q downpress`n and moved outside notepad by Q up release
	}
	return
guest3456
Posts: 3454
Joined: 09 Oct 2013, 10:31

Re: Why you should avoid while(GetKeyState("a", "P")) to detect release of key a

02 Mar 2018, 17:32

i see. i think thats much less readable and harder to follow.

and the only benefit to doing it that way is if you have other hotkeys which might interrupt the thread?

User avatar
evilC
Posts: 4822
Joined: 27 Feb 2014, 12:30

Re: Why you should avoid while(GetKeyState("a", "P")) to detect release of key a

03 Mar 2018, 06:58

FYI, this is not really aimed at you, you clearly know enough to know when and when not to use a blocking wait.
This is more aimed at trying to stop noobs being taught the blocking wait pattern.
Consider this example from the documentation

Code: Select all

*NumpadAdd::
MouseClick, left,,, 1, 0, D  ; Hold down the left mouse button.
Loop
{
    Sleep, 10
    GetKeyState, state, NumpadAdd, P
    if state = U  ; The key has been released, so break out of the loop.
        break
    ; ... insert here any other actions you want repeated.
}
MouseClick, left,,, 1, 0, U  ; Release the mouse button.
return
Take that example and extend it to also do the same for Right Mouse.

Code: Select all

*NumpadAdd::
MouseClick, left,,, 1, 0, D  ; Hold down the left mouse button.
Loop
{
    Sleep, 10
    GetKeyState, state, NumpadAdd, P
    if state = U  ; The key has been released, so break out of the loop.
        break
    ; ... insert here any other actions you want repeated.
}
MouseClick, left,,, 1, 0, U  ; Release the mouse button.
return

*NumpadSub::
MouseClick, right,,, 1, 0, D  ; Hold down the left mouse button.
Loop
{
    Sleep, 10
    GetKeyState, state, NumpadSub, P
    if state = U  ; The key has been released, so break out of the loop.
        break
    ; ... insert here any other actions you want repeated.
}
MouseClick, right,,, 1, 0, U  ; Release the mouse button.
return
It doesn't work properly and buttons can get stuck.

People will use the code in the examples as templates - and we are giving them bad templates simply to try and save one or two lines of code.
I can find a grand total of three code snippets in the AHK docs which use an up event hotkey - two on this page and a side note here. All other examples in the docs, from what I can see, use blocking waits to detect key release. Why do you think so many people do not seem to even realize that up event hotkeys even exist? Because the docs never use them as an example, even when they are the most appropriate way to do it.
kyuuuri
Posts: 340
Joined: 09 Jan 2016, 19:20

Re: Why you should avoid while(GetKeyState("a", "P")) to detect release of key a

14 Mar 2018, 01:53

Hello, sry to revive this thread but what if i have something like this:

Code: Select all

key:="q"
Hotkey, $%key%, Fkey
FKey:
StringTrimLeft thishotkey, a_thisHotkey, 1
Loop
{
	if not GetKeyState(ThisHotkey, "P")
		break
	loop
	{
		Sendinput {%thisHotkey%}
		Delay(0.005)
		asd:=Readmemory(skill_address)
	} until asd<>0
		Delay(0.005)
		sendinput {Click}
}
return
In that script i can't use settimer because it slows things down, and i need it to be as fast as possible. This loop gets stuck a lot of times and it's annoying because sometimes when you press Q again it doesn't stop. You have to reload the script.
I've been thinking about something like:

Code: Select all

key:="q"
Hotkey, $%key%, Fkey
Hotkey, $%key% up, Fkey2
FKey:
stop:=0
StringTrimLeft thishotkey, a_thisHotkey, 1
Loop
{
	if stop=1
		break
	loop
	{
		Sendinput {%thisHotkey%}
		Delay(0.005)
		asd:=Readmemory(skill_address)
	} until asd<>0
		Delay(0.005)
		sendinput {Click}
}
return

FKey2:
stop:=1
return
But i think that there might be a better way. What do you think?
Thank you.
User avatar
nnnik
Posts: 4500
Joined: 30 Sep 2013, 01:01
Location: Germany

Re: Why you should avoid while(GetKeyState("a", "P")) to detect release of key a

14 Mar 2018, 03:22

kyuuri what evilC tried to say in this post is that using any Loop will prevent AutoHotkey from switching to another thread easily.
You are already loooping though so I think getKeyState is the best option here.
SetTimer starts a new thread to execute the code in the GoSub label so SetTimer is not an option here.
And I think the Up Event Hotkey is just the same as GetKeyState in your case.
Recommends AHK Studio
kyuuuri
Posts: 340
Joined: 09 Jan 2016, 19:20

Re: Why you should avoid while(GetKeyState("a", "P")) to detect release of key a

14 Mar 2018, 04:57

nnnik wrote:kyuuri what evilC tried to say in this post is that using any Loop will prevent AutoHotkey from switching to another thread easily.
You are already loooping though so I think getKeyState is the best option here.
SetTimer starts a new thread to execute the code in the GoSub label so SetTimer is not an option here.
And I think the Up Event Hotkey is just the same as GetKeyState in your case.
Thank you for the answer, my problem is that it gets stuck all the time. And i don't know what to do to solve it.
User avatar
nnnik
Posts: 4500
Joined: 30 Sep 2013, 01:01
Location: Germany

Re: Why you should avoid while(GetKeyState("a", "P")) to detect release of key a

14 Mar 2018, 05:01

I think you should ask in Ask for Help and or inform yourself about threads and their limitations.
Recommends AHK Studio

Return to “Tips and Tricks (v1)”

Who is online

Users browsing this forum: No registered users and 52 guests