Reformatting my script from v1 to v2

[chinagreenelvis]

In actuality you'd write some Loops to read monitor/column info and push them in the array.

Starting on a simpler part first - what am I doing wrong here? I'm told MonRight is an invalid index for the MsgBox.

Code: Select all

Mon := []
Mon.Push([])
NumMonitors := SysGet(80)
Loop NumMonitors
{
	ActualN := MonitorGet(A_Index, &Left, &Top, &Right, &Bottom)
	Mon[A_Index].Push({MonLeft:Left, MonTop:Top, MonRight:Right, MonBottom:Bottom})
}
MsgBox Mon[1]["MonRight"]

[Descolada]

@chinagreenelvis, your code pushes a regular Object, but the square bracket notation MsgBox Mon[1]["MonRight"] is for Arrays and Maps. You need to access it with MsgBox Mon[1].MonRight. If you want to use brackets then use a Map: Mon[A_Index].Push(Map("MonLeft", Left, "MonTop", Top, "MonRight", Right, "MonBottom", Bottom)).

Perhaps something like this:

Code: Select all

#requires AutoHotkey v2

Mon := []
NumMonitors := SysGet(80), NumColumns := 3
Loop NumMonitors {
	ActualN := MonitorGet(A_Index, &Left, &Top, &Right, &Bottom)
	Mon.Push({MonLeft:Left, MonTop:Top, MonRight:Right, MonBottom:Bottom, Columns:[]})
    Loop NumColumns {
        Mon[-1].Columns.Push({Width:1, Height:2}) ; Mon[-1] accesses last item
    }
}

MsgBox Mon[1].MonRight
MsgBox Mon[1].Columns[1].Width

@boiler this thread could possibly be moved to v2 Help section?

[boiler]

@boiler this thread could possibly be moved to v2 Help section?

Done. Merged posts from the "Wish List" thread that OP created into this original "Ask for Help" thread -- starting with the one identified with the red note earlier in the thread and ending with this one.

[geek]

Instead of

Code: Select all

Global Mon1Col1 := Map()
Mon%MonNum%Col%ColNumber%["X"] := WhateverVal

Would I just use

Code: Select all

MonCol := [[Map()]]
MonCol[MonNum][ColNumber]["X"] := WhateverVal

Depending on your intended use case, lots of nesting is not always the best answer. Having to create intermediate maps or arrays for every level of nesting can really complicate your code. Instead, you can merge multiple data into a single string the same way you would have generated a variable name in v1. Using that technique, the most direct rewrite of this code could be

Code: Select all

Mons := Map()
Mons["Mon1Col1"] := Map()
Mons["Mon" MonNum "Col" ColNumber]["X"] := WhateverVal

You can think of Mons here as the direct replacement for global variable scope. You can do all the same dynamic "variable" creation that you would have done to the global scope, but instead of creating global variables you create map keys.

[chinagreenelvis]

Depending on your intended use case, lots of nesting is not always the best answer. Having to create intermediate maps or arrays for every level of nesting can really complicate your code. Instead, you can merge multiple data into a single string the same way you would have generated a variable name in v1. Using that technique, the most direct rewrite of this code could be

Code: Select all

Mons := Map()
Mons["Mon1Col1"] := Map()
Mons["Mon" MonNum "Col" ColNumber]["X"] := WhateverVal

You can think of Mons here as the direct replacement for global variable scope. You can do all the same dynamic "variable" creation that you would have done to the global scope, but instead of creating global variables you create map keys.

Thanks everyone for all of the help and attention. This right here specifically is an elegant solution and I think fits the format of my previous code the best.

As a test, I have a working script here that stores the resolutions of each monitor into maps and runs a loop to detect when a change in monitor numbers or resolutions has happened. It looks like it might still be a bit clunky, though, so if there is any way of further simplifying it, feedback is appreciated. (I definitely need the maps for use in a different part of the bigger script, so they might look superfluous here but they aren't. I'm mostly trying to figure out if there's a better way than using the whole LoopRunOnce variable.)

Code: Select all

#Requires AutoHotkey v2
#SingleInstance Force
Persistent
SetWorkingDir A_ScriptDir
Thread "NoTimers", True

SetTimer MonitorTimer, 300

SM_CMONITORS := 80
LoopRunOnce := 0
Monitors := Map()
OldMonitors := Map()

MonitorTimer()
{
	Global
	Monitors.Clear()
	NumMonitors := SysGet(SM_CMONITORS)
	If (LoopRunOnce)
	{
		If NumMonitors != OldNumMonitors
		{
			MsgBox "Number of Monitors Changed"
			LoopRunOnce := 0
			GoTo EndOfTimer
		}
	}
	Global OldNumMonitors := SysGet(SM_CMONITORS)
	Loop NumMonitors
	{
		MonitorNumber := A_Index
		Monitors["Mon" MonitorNumber] := Map()
		ActualN := MonitorGet(MonitorNumber, &Left, &Top, &Right, &Bottom)
		Monitors["Mon" MonitorNumber]["Left"] := Left
		Monitors["Mon" MonitorNumber]["Top"] := Top
		Monitors["Mon" MonitorNumber]["Right"] := Right
		Monitors["Mon" MonitorNumber]["Bottom"] := Bottom
				
		If (LoopRunOnce)
		{
			If (OldMonitors["Mon" MonitorNumber]["Left"] != Left ||
					OldMonitors["Mon" MonitorNumber]["Top"] != Top ||
					OldMonitors["Mon" MonitorNumber]["Right"] != Right ||
					OldMonitors["Mon" MonitorNumber]["Bottom"] != Bottom)
			{
			
			;; Do a thing here
			
			SoundBeep 300, 500
			
			;	MsgBox
			;	(
			;		"Resolution changed on monitor #" MonitorNumber "`n"
			;		"Old Left: " OldMonitors["Mon" MonitorNumber]["Left"] "`n"
			;		"Old Top: " OldMonitors["Mon" MonitorNumber]["Top"] "`n"
			;		"Old Right: " OldMonitors["Mon" MonitorNumber]["Right"] "`n"
			;		"Old Bottom: " OldMonitors["Mon" MonitorNumber]["Bottom"]
			;	)
			
				LoopRunOnce := 0
				GoTo EndOfTimer
			}
		}
		
		OldMonitors["Mon" MonitorNumber] := Map()
		OldMonitors["Mon" MonitorNumber]["Left"] := Left
		OldMonitors["Mon" MonitorNumber]["Top"] := Top
		OldMonitors["Mon" MonitorNumber]["Right"] := Right
		OldMonitors["Mon" MonitorNumber]["Bottom"] := Bottom
	}
	If (!LoopRunOnce)
	{
		LoopRunOnce := 1
	}
	EndOfTimer:
}

[Mod edit: Added missing opening quote tag.]

[Descolada]

@chinagreenelvis, depending on what the rest of your code does, it might be easier to use indeed. One downside is that it's more difficult to change the number of columns, as both monitor AND column info are tied together. Whether that matters to you depends on your code.

I opted to change Maps to Objects as you are defining properties for monitors, and in such situations my personal preference is using object notation. Also Maps are case-sensitive by default, so Monitor["Left"] is not the same as Monitor["left"], whereas Monitor.Left == Monitor.left. See further discussion of Maps vs Objects here: viewtopic.php?style=7&t=120410.

Code: Select all

#Requires AutoHotkey v2
#SingleInstance Force
Persistent
SetWorkingDir A_ScriptDir
Thread "NoTimers", True

SetTimer MonitorTimer, 300
Monitors := []

MonitorTimer() {
    static SM_CMONITORS := 80
    global Monitors
	NumMonitors := SysGet(SM_CMONITORS)

    ; Get current status of monitors
    CurrentMonitors := []
    Loop NumMonitors {
        MonitorNumber := A_Index
        ActualN := MonitorGet(MonitorNumber, &Left, &Top, &Right, &Bottom)
        CurrentMonitors.Push({Left:Left, Top:Top, Right:Right, Bottom:Bottom, Columns:[]})
    }

    if NumMonitors != Monitors.Length {
        if Monitors.Length
		    MsgBox "Number of Monitors Changed"
        else
            MsgBox "First run (or no monitors)"
	} else {
        for i, Monitor in CurrentMonitors {
            Monitor.Columns := Monitors[i].Columns
            if Monitor.Left != Monitors[i].Left ||
               Monitor.Top != Monitors[i].Top ||
               Monitor.Right != Monitors[i].Right ||
               Monitor.Bottom != Monitors[i].Bottom 
            {
                ;; Do a thing here
                SoundBeep 300, 500
            }
        } 
    }

    ; Rewrite old monitor info with new monitor info
    Monitors := CurrentMonitors
}

[chinagreenelvis]

@chinagreenelvis, depending on what the rest of your code does, it might be easier to use indeed. One downside is that it's more difficult to change the number of columns, as both monitor AND column info are tied together. Whether that matters to you depends on your code.

I opted to change Maps to Objects as you are defining properties for monitors, and in such situations my personal preference is using object notation. Also Maps are case-sensitive by default, so Monitor["Left"] is not the same as Monitor["left"], whereas Monitor.Left == Monitor.left. See further discussion of Maps vs Objects here: viewtopic.php?style=7&t=120410.

Code: Select all

#Requires AutoHotkey v2
#SingleInstance Force
Persistent
SetWorkingDir A_ScriptDir
Thread "NoTimers", True

SetTimer MonitorTimer, 300
Monitors := []

MonitorTimer() {
    static SM_CMONITORS := 80
    global Monitors
	NumMonitors := SysGet(SM_CMONITORS)

    ; Get current status of monitors
    CurrentMonitors := []
    Loop NumMonitors {
        MonitorNumber := A_Index
        ActualN := MonitorGet(MonitorNumber, &Left, &Top, &Right, &Bottom)
        CurrentMonitors.Push({Left:Left, Top:Top, Right:Right, Bottom:Bottom, Columns:[]})
    }

    if NumMonitors != Monitors.Length {
        if Monitors.Length
		    MsgBox "Number of Monitors Changed"
        else
            MsgBox "First run (or no monitors)"
	} else {
        for i, Monitor in CurrentMonitors {
            Monitor.Columns := Monitors[i].Columns
            if Monitor.Left != Monitors[i].Left ||
               Monitor.Top != Monitors[i].Top ||
               Monitor.Right != Monitors[i].Right ||
               Monitor.Bottom != Monitors[i].Bottom 
            {
                ;; Do a thing here
                SoundBeep 300, 500
            }
        } 
    }

    ; Rewrite old monitor info with new monitor info
    Monitors := CurrentMonitors
}

Much appreciated!