3
\$\begingroup\$

This little chunk of code is run every 10 seconds on my desktop. It generates a PNG with up to 6 player faces based on the list of online players.

Is there any way for this code to execute faster? Any comments?

Each player has a PNG of their avatar's face with the title: "username_face.png" These are pasted into the main image when that player is online.

I wrote this script to compile their faces into a single image for display in a program called conky, allowing me to display the online players as part of my desktop.

from mcstatus import MinecraftServer
import os
from PIL import Image

#Query the server and get player names.
server = MinecraftServer("123.123.123.123", 25565)
query = server.query()
nameList = query.players.names


#Sort the list of player names.
nameList.sort()

#Open our temporary file for storing player names.
tempFile = open('server1_previous_players.txt', 'r')
oldNameList = tempFile.read().splitlines()
tempFile.close()

def createImage(players):
    #create a blank transparent image with the dimensions: 212x147px.
    baseImage = Image.new("RGBA", (212, 147))
    slotList = [(17, 17), (82, 17), (147, 17), (17, 82), (82, 82), (147, 82)]

    if len(players) > 0:
        for x in range(0, 6):
            try:
                face = Image.open(str(nameList[x]).lower() + "_face.png", 'r')
                baseImage.paste(face, slotList[x])
            except:
                break

    baseImage.save("server1_online.png")

#If nothing changed, pass!
if nameList == oldNameList:
    print("same thing!")
    pass

#Else, if no-one is on, but the text file says people are on, empty the text file.
elif len(nameList) == 0 and len(oldNameList) > 0:
    print("everyone left :'(")
    tempFile = open('server1_previous_players.txt', 'w')
    createImage(nameList)

#Else, if at least 1 person is on, generate a new image accordingly.
elif len(nameList) > 0:
    tempFile = open('server1_previous_players.txt', 'w')
    print('\n'.join(nameList), file=tempFile)
    createImage(nameList)

else:
    pass
\$\endgroup\$
4
\$\begingroup\$

Building on @ovs answer. I would use the for loop approach (it is almost always nicer in Python, unless you do an infinite while True loop.

def create_image(players):
    # create a blank transparent image with the dimensions: 212x147px.
    base_image = Image.new("RGBA", (212, 147))
    slot_list = [(17, 17), (82, 17), (147, 17), (17, 82), (82, 82), (147, 82)]

    for slot, player in zip(slot_list, players):
        with Image.open("{}_face.png".format(player.lower())) as face:
            base_image.paste(face, slot)
    base_image.save("server1_online.png")

This works, because if players is shorter than 6 elements, zip will stop there. If it is longer than 6 elements it will stop after slot_list is empty, which is exactly 6 elements long.

Here I also changed the names to conform with Python's official style-guide, PEP8, as well as used with open(...) as to ensure the file is closed again. Also note that "r" is the default mode of open.

The same can be done for the second part. We can also use the fact that empty lists are falsey, so evaluate as False in an if context:

if name_list == old_name_list:
    # nothing changed, pass!
    print("same thing!")
else:
    # generate image and write new player list to file
    if not name_list and old_name_list:
        print("everybody left")
    with open('server1_previous_players.txt', 'w') as temp:
        # writes "" for an empty player list
        temp.write('\n'.join(name_list))  
    create_image(name_list)
\$\endgroup\$
  • \$\begingroup\$ Your improvements to the readability and structure of the code are both intuitive and robust. Thank you, I actually gained some FPS while on my server! \$\endgroup\$ – Ethen Crowl Jan 27 '17 at 20:29
4
\$\begingroup\$

Maybe it will not increase the performance, but I have some general tips:
Use while loops instead of for-loops with break. It will make your code more readable. I have rewritten the createImage function for you:

def createImage(players):
    # create a blank transparent image with the dimensions: 212x147px.
    baseImage = Image.new("RGBA", (212, 147))
    slotList = [(17, 17), (82, 17), (147, 17), (17, 82), (82, 82), (147, 82)]

    x = 0
    while x < len(players) and x < 6:
        face = Image.open(str(players[x]).lower() + "_face.png", 'r')
        baseImage.paste(face, slotList[x])
        x += 1
    baseImage.save("server1_online.png")

If you prefer for over while, you can use:

def createImage(players):
    # create a blank transparent image with the dimensions: 212x147px.
    baseImage = Image.new("RGBA", (212, 147))
    slotList = [(17, 17), (82, 17), (147, 17), (17, 82), (82, 82), (147, 82)]

    for slot, player in zip(slotList, players):
        face = Image.open(str(player).lower() + "_face.png", 'r')
        baseImage.paste(face, slot)
    baseImage.save("server1_online.png")

Read more about zip here

You need to close the files you open:

# If nothing changed, pass!
if nameList == oldNameList:
    print("same thing!")

# Else, if no-one is on, but the text file says people are on, empty the text file.
elif len(nameList) == 0 and len(oldNameList) > 0:
    print("everyone left :'(")
    tempFile = open('server1_previous_players.txt', 'w')
    tempFile.write("")
    tempFile.close()
    createImage(nameList)

# Else, if at least 1 person is on, generate a new image accordingly.
elif len(nameList) > 0:
    tempFile = open('server1_previous_players.txt', 'w')
    # write should be used instead of print
    tempFile.write('\n'.join(nameList))
    tempFile.close()
    createImage(nameList)

An if-statement can be used without an else, so if there is only pass in there you don't need it.

If you want to create an empty / transparent image when nobody is on, you can combine both elif-statements:

# If nothing changed, pass!
if nameList == oldNameList:
    print("same thing!")


# else generate image and write new player list to file
else:
    if len(nameList) == 0 and len(oldNameList) > 0:
        print("everybody left")
    tempFile = open('server1_previous_players.txt', 'w')
    tempFile.write('\n'.join(nameList))  # writes "" for an empty player list
    tempFile.close()
    createImage(nameList)
\$\endgroup\$
  • 2
    \$\begingroup\$ For me, the while loop in Python never feels right unless it is while True: :) \$\endgroup\$ – яүυк Jan 27 '17 at 16:07
  • \$\begingroup\$ Thank you for your reply. I modified my code to close the files as I forgot to do that initially. \$\endgroup\$ – Ethen Crowl Jan 27 '17 at 20:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.