virtual machine - How can my AutoHotKey script launch batch scripts accordingly?

Question

This script is supposed to switch me from Windows to Linux(VM) and vice versa:

Pause::vmStart()
return

runVM := false
linux := false

vmStart()
{
    If (!runVM and !linux) {
        Run, C:Userspatrickdev-vmPS.cmd
        runVM := true
        sleep, 18000
    }
    If (!linux and !WinExist("DevVM - 127.0.0.1:23389 - RDP")) {
        Run, C:Userspatrickdev-vmRDP.cmd
    }
    if (!linux) {
        WinShow, DevVM - 127.0.0.1:23389 - RDP
        WinActivate, DevVM - 127.0.0.1:23389 - RDP
    }
    Send ^!{CtrlBreak}
    linux := !linux
}

• When I am in Windows, it needs to activate my RDP window first (what works fine).
• When I am in Windows and the VM was not started with PS.cmd, then it should launch it (what also works and takes about 18 sec)
• Using ^!{CtrlBreak} is the normal switch that also works.

I think that there is something wrong with my brackets/function/boolean definition. Do you find the mistake?

Answer 1

Has a few problems, but just one that matters.
Firstly, your variable definitions are unreachable code.

Pause::vmStart()
return

runVM := false
linux := false

Code execution stops at the first hotkey that is met.
In addition you also have a Return in there, which would also stop code execution. So you really are making sure code execution will never reach the variable definitions haha.
Luckily AHK is super forgiving and if you reference any variable that hasn't been declared yet, it's created with the default value of nothing, which also evaluates to false.

So that wasn't the actual problem, but still something that needs fixing. Move the definitions to be above your hotkey, or just remove them, they're not needed due to how forgiving AHK is, as explained above.
Then onto the next problem, variable scope.

In that function's scope the variables you reference don't exist, and they're created and freed every time you run the function.
You have a few options you can do. You can either define the variables as global, static, or super global (super global is bad practice and not recommended).

Defining them as global means you reference a variable that's found outside of the function's scope and its value will be stored there. To define the variables as global, you'd make the first line(s) of your function do that like this:

vmStart()
{
    global runVM, linux
    ...

Or you could just make the first line of function be nothing but the keyword global, and that means the function assumes all variables are global.

To define the variables as static, you'd do the same as for global (with the keyword static). Defining them as static basically means they aren't freed after the function completes its execution. So next time you call the function their value is what you last set it to be in the function.

To define them as super global, you'd define the variables outside of the function (at the very top of your script) with the keyword global like this:

global runVM := false
global linux := false
Pause::vmStart()
...

This would mean any scope anywhere that tried to reference a variable by that name would use your super global variable. This is bad practice and can be dangerous to do, especially if you use external libraries. Wouldn't be too hard to accidentally break something.

Of course when you have a little script like that, is makes no difference whatsoever what method you use. Even I confess to sometimes using super global on my personal scripts, just because it's quite convenient to not have to worry about scopes.

If you'd like to hear my recommendation, I'd say go for static variables.
It's pretty much exactly intended for what you're doing here.

Here's your finished product (with some miscellaneous changes) in case I didn't explain something well enough:

Pause::vmStart()
;the 'return' here did nothing for us, removed

vmStart()
{
    ;using the static keyword to make the function assume
    ;all variables are static, and also skipped even 
    ;declaring the variables, not needed due to how forgiving AHK is
    static

    ;got rid of the 'and' keyword in your if statements
    ;that's legacy syntax, big ew, it's not 2005
    If (!runVM && !linux) {
        Run, C:Userspatrickdev-vmPS.cmd
        runVM := true
        sleep, 18000
    }
    ;removed braces, one-liner statments don't need them
    ;just personal preference though, of course
    If (!linux && !WinExist("DevVM - 127.0.0.1:23389 - RDP")) 
        Run, C:Userspatrickdev-vmRDP.cmd
    if (!linux) {
        WinShow, DevVM - 127.0.0.1:23389 - RDP
        WinActivate, DevVM - 127.0.0.1:23389 - RDP
    }
    ;switched to SendInput, it's faster and more reliable
    SendInput, ^!{CtrlBreak}
    linux := !linux
}