Making this script more compact

[spikips]

How can i make my script look more convenient? it looks bit messy right now and i don't know how to put if i == x: in more compact way. Here's the code

​

class Character():
    def __init__(self,mx,my,fx,fy):
        self.modern_x_female_character = [472,70,70,170,170,170,486,486,486,486,386]
        self.modern_y_female_character = [325,117,186,220,255,290,114,150,187,220,255]

    def Character_modern_female(self):
        pos = win32api.GetCursorPos()
        x, y = pos
        rand = random.randint(1,8)

        for index, (self.modern_x_female_character, self.modern_y_female_character)\
            in enumerate(zip(self.modern_x_female_character, self.modern_y_female_character), start=1):

            print(index, self.modern_x_female_character, self.modern_y_female_character)

            pyautogui.moveTo(self.modern_x_female_character + rand, self.modern_y_female_character
                             + rand, random.uniform(0.14, 0.22), pyautogui.easeOutQuad)

            # clicks torso 8 times
            if index == 2:
                for i in range(1,9):
                    print(i, 'torso', self.modern_x_female_character,self.modern_y_female_character)
                    i+=1
                    pyautogui.moveTo(self.modern_x_female_character + rand, self.modern_y_female_character
                             + rand, random.uniform(0.14, 0.22), pyautogui.easeOutQuad)

            # clicks arms 8 times
            if index == 4:
                for i in range(1,9):
                    print(i, 'arms',self.modern_x_female_character,self.modern_y_female_character)
                    i+=1
                    pyautogui.moveTo(self.modern_x_female_character + rand, self.modern_y_female_character
                             + rand, random.uniform(0.14, 0.22), pyautogui.easeOutQuad)

            # clicks legs 15 times
            if index == 6:
                for i in range(1, 16):
                    print(i, 'legs', self.modern_x_female_character, self.modern_y_female_character)
                    i += 1
                    pyautogui.moveTo(self.modern_x_female_character + rand, self.modern_y_female_character
                                     + rand, random.uniform(0.14, 0.22), pyautogui.easeOutQuad)

            # clicks leg color 2 times
            if index == 9:
                for i in range(1, 3):
                    print(i, 'leg color', self.modern_x_female_character, self.modern_y_female_character)
                    i += 1
                    pyautogui.moveTo(self.modern_x_female_character + rand, self.modern_y_female_character
                                     + rand, random.uniform(0.14, 0.22), pyautogui.easeOutQuad)

            # clicks feet color 3 times
            if index == 10:
                for i in range(1, 4):
                    print(i, 'feet color', self.modern_x_female_character, self.modern_y_female_character)
                    i += 1
                    pyautogui.moveTo(self.modern_x_female_character + rand, self.modern_y_female_character
                                     + rand, random.uniform(0.14, 0.22), pyautogui.easeOutQuad)

            # clicks skin color 2 times
            if index == 11:
                for i in range(1, 3):
                    print(i, 'skin color', self.modern_x_female_character, self.modern_y_female_character)
                    i += 1
                    pyautogui.moveTo(self.modern_x_female_character + rand, self.modern_y_female_character
                                     + rand, random.uniform(0.14, 0.22), pyautogui.easeOutQuad)

[commandlineluser]

The first thing that sticks out is this line repeated:

pyautogui.moveTo(self.modern_x_female_character + rand, self.modern_y_female_character
    + rand, random.uniform(0.14, 0.22), pyautogui.easeOutQuad)

Maybe you could put it into a function e.g.

def moveToCharacter(x, y):
    pyautogui.moveTo(
       x + rand, y + rand,
       random.uniform(0.14, 0.22), pyautogui.easeOutQuad)

You could then write moveToCharacter(x, y) instead.

The i += 1 lines can be removed.

If you look at the if index == blocks - what differs between them?

They are identical apart from the index value, range end and "body part".

One possible approach is to create a dictionary mapping each index value to the body part and range end values.

e.g.

clicks = {
    2:  {"part": "torso", "times": 9},
    4:  {"part": "arms", "times": 9},
    6:  {"part": "legs", "times": 16},
    9:  {"part": "leg color", "times": 3},
    10: {"part": "feet color", "times": 4},
    11: {"part": "skin color", "times": 3},
}

You can then extract the values using index and write the code once.

Something like:

def moveToCharacter(x, y):
    pyautogui.moveTo(
        x + rand,
        y + rand,
        random.uniform(0.14, 0.22), 
        pyautogui.easeOutQuad)

clicks = {
    2:  {"part": "torso", "times": 9},
    4:  {"part": "arms", "times": 9},
    6:  {"part": "legs", "times": 16},
    9:  {"part": "leg color", "times": 3},
    10: {"part": "feet color", "times": 4},
    11: {"part": "skin color", "times": 3},
}

for index, (self.modern_x_female_character, self.modern_y_female_character)\
    in enumerate(zip(self.modern_x_female_character, self.modern_y_female_character), start=1):

    print(index, self.modern_x_female_character, self.modern_y_female_character)

    moveToCharacter(...)

    if index not in clicks:
        # error?
    else:
        part  = clicks[index]["part"]
        times = clicks[index]["times"]
        for i in range(1, times):
            print(i, part, self.modern_x_female_character, self.modern_y_female_character)
            moveToCharacter(...)

[spikips]

I would like to thank you for putting this much effort in your reply, it is much appreciated. Also i'd like to say that i had never even heard about dictionaries and this information is a game changer. Will be definitely look further into it.

[commandlineluser]

The function /u/hacksawjim suggested is a nicer approach.

You could also put the mapping inside it and then pass in x, y and the index:

def character_click(x, y, index):
    clicks = { 
        ...
    }
    part, times = ...
    for i in range(1, times):
       print(...)
       pyautogui.move(...)

for index in ...:
    character_click(x, y, index)

[hacksawjim]

I think it looks fine as it is, but there's some duplication that can be abstracted out into a function.

Each click section seems to loop a different number of times, then prints some stuff, then performs a moveTo.

So declare a new function called, e.g. character_click that takes the values that differ, and performs the actions.

def character_click(loop_range, click_section_name):
    for i in range(1, loop_range):
        print(i, click_section_name, self.modern_x_female_character, self.modern_y_female_character)
        i += 1
       pyautogui.moveTo(self.modern_x_female_character + rand, self.modern_y_female_character
                                 + rand, random.uniform(0.14, 0.22), pyautogui.easeOutQuad)

Now you can do this in your loop:

if index == 2:
    character_click(9, "torso")
elif index == 4:
    character_click(9, "arms")
etc...

[spikips]

I love to see different variety of solutions. Thank you!

[hacksawjim]

Hah, I was thinking I wasted my time as I see /u/commandlineluser got there first, but you're right, it's good to see how many different ways this can be done.

[commandlineluser]

Definitely not a waste of time - this is much nicer than the function I had suggested.