1
\$\begingroup\$

Currently I am trying to teach myself more about functions with parameters and return values that connects with one to another.

In this code I created a square object with graphics.py that moves to a direction in loop whenever you press an arrow key.

from graphics import *
import keyboard, time
width = 200
height = 200
win = GraphWin("MOVEMENTS", width, height)
win.setBackground(color_rgb(25,25,25))

key = ""

def movement(key):
    if keyboard.is_pressed("Right"):
        key = "d"
    if keyboard.is_pressed("Left"):
        key = "a"
    if keyboard.is_pressed("Down"):
        key = "s"
    if keyboard.is_pressed("Up"):
        key = "w"
    return key

def horizontal_movement(x):
    global key
    key = movement(key)
    if key == "d":
        x += 20
    elif key == "a":
        x -= 20
    return x

def vertical_movement(y):
    global key
    key = movement(key)
    if key == "s":
        y += 20
    elif key == "w":
        y -= 20
    return y


def main():
    x = 0
    y = 0
    radius = 10
    player = Rectangle(Point((width/2)-radius+x,(height/2)-radius+y), Point((width/2)+radius+x, (height/2)+radius+y))

    while(True):
        player.undraw()
        player = Rectangle(Point((width/2)-radius+x,(height/2)-radius+y), Point((width/2)+radius+x,(height/2)+radius+y))
        player.setFill("green")
        player.setWidth(2)
        player.draw(win)

        x = horizontal_movement(x)
        y = vertical_movement(y)

        update(10)


main()

I want to know if there is a better code design that can move the movement(key) function into the horizontal_movement(x) and vertical_movement(y) function because right now I feel like I have typed some unecessary coding (the movement function and global key) into the script and I want it be as efficient as possible.

If there is an alternative that makes the script more efficient or perform better, please let me know so I can improve further.

\$\endgroup\$
  • \$\begingroup\$ Did you use this graphics.py? \$\endgroup\$ – Graipher Aug 16 '19 at 14:43
  • \$\begingroup\$ yes I am using the Zelle graphics.py \$\endgroup\$ – ifsoMarcus Aug 16 '19 at 21:18
1
\$\begingroup\$

While you seem to be on a good way, your code still hast a global variable it does not need. You can simplify that code a lot by using something like this:

OFFSETS = {"w": (0, -1), "Up": (0, -1),   # weird coordinate system...
           "a": (-1, 0), "Left": (-1, 0),
           "s": (0, 1), "Down": (0, 1),
           "d": (1, 0), "Right": (1, 0)}

def get_offsets():
    for key, offset in OFFSETS.items():
        if keyboard.is_pressed(key):
            return offset
    return 0, 0

This just returns a tuple of changes for each defined key, and zero otherwise.

You should do as little as possible inside your loop (since that affects performance). You can set the graphic options once before the loop if you afterward continue modifying the object(s) in place. With the Rectangle that is easy, since you can just overwrite the p1 and p2 attributes. The two Point objects have an internal _move method that you can use (or you code it yourself):

class Point:
    ...
    def _move(self, dx, dy):
        self.x += dx
        self.y += dy

Another small performance optimization is to do nothing if the rectangle does not move. I would also add a if __name__ == "__main__": guard to allow importing from this module without running the game and follow Python's official style-guide, PEP8, which recommend adding spaces around operators. It also recommends putting imports from different modules on separate lines and not using unnecessary parenthesis.

With these changes you get this for your main:

def main():
    x = 0
    y = 0
    radius = 10
    speed = 20
    lower_left = Point((width / 2) - radius, (height / 2) - radius)
    upper_right = Point((width / 2) + radius, (height / 2) + radius)
    player = Rectangle(lower_left, upper_right)
    player.setFill("green")
    player.setWidth(2)
    player.draw(win)

    while True:
        dx, dy = get_offsets()
        if dx != 0 or dy != 0:
            player.undraw()
            dx, dy = dx * speed, dy * speed
            lower_left._move(dx, dy)
            upper_right._move(dx, dy)
            player.p1, player.p2 = lower_left, upper_right
            player.draw(win)
        update(10)

if __name__ == "__main__":
    main()

In addition, you might want to look for a different keyboard event handler, the keyboard module requires administrator rights (at least on UNIX). I know I certainly wouldn't just run a game that needs that. In other words, I would at least read the whole source code to make there is no exploit hidden in it (which was still possible with the code you have so far), but probably stay on the safe side and not play it at all.

| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ Hello I just read your comment and I have been self teaching python/coding for almost 3 months so I am a bit overwhelmed by this, although I really appreciate the effort you put into this to help me out. Do you have any advice on improving coding for beginners? I know that the perfect solution is to practice everyday and don't give up, but I want to gather as much information as I continue to learn coding to give myself enough resources to improve. Although I am currently overwhelmed, I do believe that in future as I learn more I will be able to understand this and other challenges. \$\endgroup\$ – ifsoMarcus Aug 16 '19 at 21:32
  • 1
    \$\begingroup\$ @ifsoMarcus We all started somewhere. One big improvement is knowing what is in the standard library. The other thing that helped me getting better in algorithms, data structures and optimization is Project Euler. The first challenges can be solved with a one-liner in Python, but the later problems (20+) start to get more mathematically involved. If that is too much, any other coding challenge website will do. As you said, practice is everything. Answering questions here also helped me a lot :-) \$\endgroup\$ – Graipher Aug 16 '19 at 22:31
  • 1
    \$\begingroup\$ Hello I am glad you replied to my comment. I had actually been doing online challenges including project euler before I started giving myself some projects to do, although I later dropped project euler because it began to become difficult to find answers without writing a smarter and more efficient script. Again thank you for the reply and I have actually considered to revisit some online challenges again because I feel stuck from just giving myself a project to do. \$\endgroup\$ – ifsoMarcus Aug 16 '19 at 22:41
  • 1
    \$\begingroup\$ @ifsoMarcus Yeah, in the end almost nothing beats needing to use it in a real life problem...What you can (and should) do, though, is solve the problem as proper as possible. I.e. add docstrings and unittests, upload it to GitHub or similar and regularly push your updates (something I have not been doing diligently enough...). This way you already have a portfolio for if you want to apply to programming jobs. \$\endgroup\$ – Graipher Aug 16 '19 at 22:46

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.