Custom autoclick Python script

Question

I'm a newbie Python programmer and I just made my first script which allows autoclicking into various positions of the screen (3 for now) an X number of times with an Y interval.

I'd like to know what do you think about my code, specifically if there is a more efficient way to do the same task.

No need to extend functionalities and such. I'm just curious to hear what more experienced developers think.

#import libraries
import pyautogui
from tkinter import *
import time
import subprocess
#settings
pyautogui.PAUSE = 1
pyautogui.FAILSAFE = True
pyautogui.size()
width, height = pyautogui.size()
initialStr = "Screen Size: " + str(width) +" - " +str(height)
print(initialStr)
x,y = pyautogui.position()
positionStr = 'X: ' + str(x).rjust(4) + ' Y: ' + str(y).rjust(4)
print(positionStr)
# displays screen size
def function():
 print(initialStr)
#saves mouse position 1
def position():
 time.sleep(2)
 global xmouse, ymouse
 xmouse, ymouse = pyautogui.position()
 print(str(xmouse)+","+str(ymouse))
 w2 = Label(ro, text="Position 1 set: "+str(xmouse)+","+str(ymouse)).grid(row=2,columnspan=2)
#saves mouse position 2
def position2():
 time.sleep(2)
 global xmouse2, ymouse2
 xmouse2, ymouse2 = pyautogui.position()
 print(str(xmouse2)+","+str(ymouse2))
 w3 = Label(ro, text="Position 2 set: "+str(xmouse2)+","+str(ymouse2)).grid(row=4,columnspan=2)
#saves mouse position 3
def position3():
 time.sleep(2)
 global xmouse3, ymouse3
 xmouse3, ymouse3 = pyautogui.position()
 print(str(xmouse3)+","+str(ymouse3))
 w4 = Label(ro, text="Position 3 set: "+str(xmouse3)+","+str(ymouse3)).grid(row=6,columnspan=2) 
#saves number of cycles
def sel():
 selection = "Value = " + str(iterations.get())
 label = Label(ro, text="Number of cycles: "+str(iterations.get())).grid(row=11,columnspan=2)
#saves execution interval
def sel2():
 selection = "Value = " + str(parametro_timer.get())
 label2 = Label(ro, text="Execution interval set at: "+str(parametro_timer.get())+" seconds").grid(row=14,columnspan=2)
#starts autoclicking
def gogo():
 #checks for unset variables, if one or more are unset it returns an error
 try:
 xmouse,xmouse2,xmouse3
 except NameError:
 label_error = Label(ro, foreground="red", text="ERROR: Some parameters are not set").grid(row=16,columnspan=2)
 #if all settings have been set then the program can start autoclicking
 else:
 time.sleep(2)
 timer=int(parametro_timer.get())
 parametro_range=int(iterations.get())
 for i in range(0,parametro_range):
 pyautogui.click(xmouse, ymouse)
 time.sleep(timer)
 pyautogui.click(xmouse2, ymouse2)
 time.sleep(timer)
 pyautogui.click(xmouse3, ymouse3)
 time.sleep(timer)
#GUI 
ro = Tk()
ro.wm_title("AutoClicker1.0")
#scale variables
iterations = DoubleVar()
parametro_timer = DoubleVar()
w1 = Label(ro, text=initialStr).grid(row=0,columnspan=2, pady=15) 
w2 = Label(ro, text="Position 1 is unset").grid(row=2,columnspan=2)
w3 = Label(ro, text="Position 2 is unset").grid(row=4,columnspan=2)
w4 = Label(ro, text="Position 3 is unset").grid(row=6,columnspan=2) 
button = Button(ro, text="Set Position 1 [2 seconds to hover into position]", command=position).grid(row=1,columnspan=2)
button = Button(ro, text="Set Position 2 [2 seconds to hover into position]", command=position2).grid(row=3,columnspan=2)
button = Button(ro, text="Set Position 3 [2 seconds to hover into position]", command=position3).grid(row=5,columnspan=2)
scale = Scale( ro, variable = iterations, orient=HORIZONTAL, from_=5, to=100 ).grid(row=9,columnspan=2)
button = Button(ro, text="Set number of cycles", command=sel).grid(row=10,columnspan=2)
label = Label(ro, text="Cycles are unset (Default=5)").grid(row=11,columnspan=2)
scale1 = Scale( ro, variable = parametro_timer, orient=HORIZONTAL, from_=2, to=15 ).grid(row=12,columnspan=2)
button = Button(ro, text="Set execution interval", command=sel2).grid(row=13,columnspan=2)
label2 = Label(ro, text="Execution interval is unset (Default=2)").grid(row=14,columnspan=2)
button = Button(ro, text="Start", command=gogo).grid(row=15,columnspan=2,padx=110,pady=5)
label_error = Label(ro, text="").grid(row=16,columnspan=2)
ro.mainloop()

Answer 1

Review

PEP 8

There are rules in Python, how you should format your code. If you want to write proper code, you should follow it: https://www.python.org/dev/peps/pep-0008/

Use an IDE

I suggest using a good IDE for Python, since it will give you hints about errors in your code. As I opened your code, my IDE found 6 warnings and 79 weak warnings. Here is a list of IDEs: https://realpython.com/python-ides-code-editors-guide/

Remove unused code

If there is any unused code, please remove it. See line 22 to 24:

# displays screen size
def function():
 print(initialStr)

Organize Imports

• You should remove unused imports, like import subprocess.
• In your case, it may be okay to use from tkinter import *, but in general, you shouldn't. See here why: https://stackoverflow.com/questions/2386714/why-is-import-bad Instead, write import tkinter or import tkinter as tk (see example).

Use f-Strings

In line 12 to 13 you built a String this way:

width, height = pyautogui.size()
initialStr = "Screen Size: " + str(width) +" - " +str(height)

Instead, you could use f-Strings, which make it way easier:

width, height = pyautogui.size()
initialStr = f"Screen Size: {width} - {height}"

More information: https://realpython.com/python-f-strings/

Use IntVar for iterations

Since all possible values for iterations are integers, you should use an IntVar instead of DoulbeVar here (line 97, iterations = DoubleVar()). Your Iterations Scale will still work fine.

Use if name == "main"

Wrap your main code with if __name__ == "__main__":. See here why: https://stackoverflow.com/questions/419163/what-does-if-name-main-do

Use constants

It's nice to declare constants, especially inside classes, so they are not in global scope. This way, you can modify simple values easily, no need for searching them inside your code. See example.

Merge similar functions in one

There is a programmer inside joke that says "no self respecting ethical programmer would ever consent to write a bomb Baghdad function. They would write a bomb city function to which Baghdad could be passed as a parameter." (source: https://codereview.stackexchange.com/a/204210/179357).

From line 26 to 52 you wrote the following code:

#saves mouse position 1
def position():
 time.sleep(2)
 global xmouse, ymouse
 xmouse, ymouse = pyautogui.position()
 print(str(xmouse)+","+str(ymouse))
 w2 = Label(ro, text="Position 1 set: "+str(xmouse)+","+str(ymouse)).grid(row=2,columnspan=2)
#saves mouse position 2
def position2():
 time.sleep(2)
 global xmouse2, ymouse2
 xmouse2, ymouse2 = pyautogui.position()
 print(str(xmouse2)+","+str(ymouse2))
 w3 = Label(ro, text="Position 2 set: "+str(xmouse2)+","+str(ymouse2)).grid(row=4,columnspan=2)
#saves mouse position 3
def position3():
 time.sleep(2)
 global xmouse3, ymouse3
 xmouse3, ymouse3 = pyautogui.position()
 print(str(xmouse3)+","+str(ymouse3))
 w4 = Label(ro, text="Position 3 set: "+str(xmouse3)+","+str(ymouse3)).grid(row=6,columnspan=2)

As you can see, you wrote nearly the same code three times. Whenever you see this, you should write one function and pass a parameter to determine, which position you address. I fixed this in my example below.

Avoid global

Using globals like you did in line 29 global xmouse, ymouse is bad programming style. See here why: https://stackoverflow.com/questions/423379/using-global-variables-in-a-function

Shadowing scope variables

It seems like you have been trying to declare variables and to modify them iside your functions. For example: You declare label in line 117:

label = Label(ro, text="Cycles are unset (Default=5)").grid(row=11,columnspan=2)

In function sel you try to override the label (line 57):

label = Label(ro, text="Number of cycles: "+str(iterations.get())).grid(row=11,columnspan=2)

But this way you cannot address label from outer scope, instead you define a new variable, which is useless. Now your workaround by using global might come into mind. There are two solutions to this problem:

1. Pass the variable as a parameter. E.g. def sel(label):
2. Use classes, define members and reference them inside your methods. I suggest this way, as I did in my example below.

Reuse objects

Now since we understand how to reference variables, we might manipulate them instead of creating new ones. So instead of creating new Labels and placing them over other labels (you can see that in your GUI, yikes!), you can simply modify them by using label.config(text="foo").

Don't block your GUI

time.sleep(int) blocks your whole GUI. It may work for your 2-second freeze to find mouse positions, but it's not nice for executing your autoklick event. Instead, you should start new threads. Simply look at my example, you can find an implementation there.

Add a way to cancel autoclicking

It's a very important feature for your Autoclicker to be able to interrupt the script. See example

---

Example

autoclicker.py

import tkinter as tk
import pyautogui
import time
import threading
# inherits from Tk class, so each autoclicker is a Tk itself
class Autoclicker(tk.Tk):
 POSITION_COUNT_DEFAULT = 3
 TITLE = "AutoClicker1.0"
 ITERATIONS_MIN = 5
 ITERATIONS_MAX = 100
 ITERATIONS_RESOLUTION = 1
 TIMER_MIN = .2
 TIMER_MAX = 5
 TIMER_RESOLUTION = .2
 DELAY = 2 # in s
 # creates all widgets, variables...
 def __init__(self, pos_count=POSITION_COUNT_DEFAULT):
 tk.Tk.__init__(self)
 self.title(self.TITLE)
 self.pos_count = pos_count
 self.positions = [None] * pos_count
 # variables for scales and checkbuttons
 self.iterations = tk.IntVar()
 self.timer = tk.DoubleVar()
 self.doubleclick = tk.BooleanVar()
 width, height = pyautogui.size()
 self.label_dim = tk.Label(self, text=f"Screen Size: {width} x {height}")
 self.labels_position = []
 self.buttons_position = []
 for i in range(pos_count):
 self.labels_position.append(tk.Label(self, text=f"Position {i + 1} is unset"))
 self.buttons_position.append(tk.Button(self, text=f"Set Position {i + 1}",
 command=lambda i_cpy=i: self.position(i_cpy)))
 self.scale_iterartions = tk.Scale(
 self,
 variable=self.iterations,
 orient=tk.HORIZONTAL,
 from_=self.ITERATIONS_MIN,
 to=self.ITERATIONS_MAX,
 resolution=self.ITERATIONS_RESOLUTION,
 label="Iterations:"
 )
 self.scale_timer = tk.Scale(
 self,
 variable=self.timer,
 orient=tk.HORIZONTAL,
 from_=self.TIMER_MIN,
 to=self.TIMER_MAX,
 resolution=self.TIMER_RESOLUTION,
 label="Timer:"
 )
 self.checkbutton_doubleclick = tk.Checkbutton(self, variable=self.doubleclick, text="Doubleclick")
 self.button_start = tk.Button(self, text="Start", command=self.start)
 self.label_error = tk.Label(self, foreground="red")
 # layout
 self.label_dim.pack()
 for i in range(pos_count):
 self.labels_position[i].pack()
 self.buttons_position[i].pack()
 self.scale_iterartions.pack()
 self.scale_timer.pack()
 self.checkbutton_doubleclick.pack()
 self.button_start.pack()
 self.label_error.pack()
 # set keyboard events
 self.bind("<Key-Escape>", self.cancel)
 # init cancel var, needed to interrupt autoclicking
 self.cancel = False
 # saves mouse positions
 def position(self, i):
 self.set_busy(True) # lock ui
 def callback():
 time.sleep(self.DELAY)
 self.positions[i] = pyautogui.position()
 xmouse, ymouse = self.positions[i]
 self.labels_position[i].config(text=f"Position {i + 1} set: {xmouse}, {ymouse}")
 self.set_busy(False) # unlock ui
 threading.Thread(target=callback).start()
 # locks/unlocks all widgets
 def set_busy(self, busy):
 state_new = tk.DISABLED if busy else tk.NORMAL 
 for button in self.buttons_position:
 button.config(state=state_new)
 self.scale_iterartions.config(state=state_new)
 self.scale_timer.config(state=state_new)
 self.checkbutton_doubleclick.config(state=state_new)
 self.button_start.config(state=state_new)
 # starts autoclicking
 def start(self):
 def callback():
 for _ in range(self.iterations.get()):
 for position in range(self.pos_count):
 if not self.cancel:
 if self.doubleclick.get():
 pyautogui.doubleClick(self.positions[position])
 else:
 pyautogui.click(self.positions[position])
 time.sleep(self.timer.get())
 self.cancel = False
 self.set_busy(False) # unlock ui
 # checks for unset variables, if one or more are unset it returns an error
 if None in self.positions:
 self.label_error.config(text="ERROR: Some positions are not set")
 # if all settings have been set then the program can start autoclicking
 else:
 self.set_busy(True) # lock ui
 self.cancel = False
 self.label_error.config(text="")
 threading.Thread(target=callback).start()
 # interrupts autoclicking (press Escape-Key)
 def cancel(self, _):
 self.cancel = True
if __name__ == "__main__":
 # change the parameter to any number of positions you want to have
 Autoclicker(2).mainloop()

• John Wilver: Thanks for spending your time improving my code. There's much I don't understand yet but I'll keep studying it. I'll make sure to vote once I have enough reputation.
• saucecode: You can still accept this answer if it solves your problem. If not, you don't have to.
• John Wilver: Right, I've noticed that the escape key doesn't work to stop the script though, is it an OS related problem?
• John Wilver: I found out why. The bind() method only works within the mainloop. So I changed it with FocusIn, that way the script will halt once I re open the program's gui.
• saucecode: U may also try to execute self.focus_set() in Autoclicker.init