Das deutsche Python-Forum

Hallo zusammen,

ich habe mein erstes, kleines Pygame-Spiel "Catch the Squares" fertiggestellt und würde mich um Feedback dazu freuen. Ist das Programm effizient umgesetzt oder gibt es grundsätzliche Dinge, die ich verbessern könnte?

Code: Alles auswählen

#!/usr/bin/env python

# Catch the Squares
# by Pythoguras

# Catch as much brown (good) squares (+1 point) as possible and avoid the
# radioactive (bad) squares (-1 point). Each collected radioactive square
# will increase the the spawn chance of following radioactive ones

# The game is lost when this chance reaches 100 % so the score cannot
#increase anymore.

import pygame
import sys
import os
import math
import random

from pygame.locals import *

os.environ['SDL_VIDEO_CENTERED'] = '1' 

IMAGE_PATH = 'Images'
SOUND_PATH = 'Sounds'

FPS = 30

SCREEN_WIDTH = 500
SCREEN_HEIGHT = 700
SCREEN_SIZE = SCREEN_WIDTH, SCREEN_HEIGHT
BOTTOM_MARGIN = 30

BGCOLOR = (0, 190, 255)
FONTCOLOR = (255, 0, 0)


class Player(pygame.sprite.Sprite):
    def __init__(self, image, position):
        pygame.sprite.Sprite.__init__(self)
        self.image = image
        self.rect = image.get_rect()
        self.rect.center = position

    def get_rect(self):
        return self.rect
    
    def draw(self, surface):
        surface.blit(self.image, self.rect)

    def update(self, player_direction, player_speed):
        if player_direction != 0:
            for step in range (player_speed):
                if (self.rect.left + player_direction >= 5 
                and self.rect.right + player_direction < SCREEN_WIDTH - 5):
                    self.rect.x += player_direction

class Square(pygame.sprite.Sprite):
    def __init__(self, image, spawn_point, is_bad_square):
        pygame.sprite.Sprite.__init__(self)
        self.image = image
        self.rect = image.get_rect()
        self.rect.topleft = spawn_point
        self.is_bad_square = is_bad_square

    def is_bad(self):
        return self.is_bad_square
    
    def draw(self, surface):
        surface.blit(self.image, self.rect)

    def update(self, square_speed):
        if self.rect.bottom <= SCREEN_HEIGHT + self.rect.height:
            for step in range(square_speed):
                self.rect.bottom += 1
        

def main():
    pygame.init()
    
    clock = pygame.time.Clock()

    screen = pygame.display.set_mode(SCREEN_SIZE)
    pygame.display.set_caption('Catch the Squares')
    
    player_image = pygame.image.load(os.path.join(IMAGE_PATH, "player.png"))
    good_square_image = pygame.image.load(os.path.join(IMAGE_PATH,
                                                       "good_square.png"))
    bad_good_square_image = pygame.image.load(os.path.join(IMAGE_PATH,
                                                           "bad_square.png"))
    collision_sound = pygame.mixer.Sound(os.path.join(SOUND_PATH,
                                                      "collision.wav"))

    font = pygame.font.Font(None, 28)

    player = Player(player_image, (screen.get_rect().center[0],
                                   SCREEN_HEIGHT - BOTTOM_MARGIN))
    player_speed = 30
    player_direction = 0
    player_score = 0

    squares = pygame.sprite.Group()
    square_size = good_square_image.get_height()
    square_speed = 17
    bad_squares_chance = 15
    
    frames_between_squares = 30 - square_speed
    frame_counter = 0
    frames_until_next_square = frames_between_squares

# Game Loop
    while True:
        screen.fill(BGCOLOR)

        # Spawn a square
        if frame_counter == frames_until_next_square:
            spawn_point = (random.randint(0, SCREEN_WIDTH - square_size),
                           - square_size)
            if random.randint(0, 100) <= bad_squares_chance:
                square = Square(bad_good_square_image, spawn_point, 1)
            else:
                square = Square(good_square_image, spawn_point, 0)
            squares.add(square)
            frame_counter = 0
            
            variation = random.randint(- frames_between_squares / 3,
                                       frames_between_squares / 3)

            frames_until_next_square = frames_between_squares + variation

        # Player colliding with square
        colliding_square = pygame.sprite.spritecollideany(player, squares)
        if colliding_square:
            if colliding_square.is_bad() and player_score > 0:
                player_score -= 1
                bad_squares_chance += 5
            else:
                player_score += 1
            collision_sound.play()
            squares.remove(colliding_square)

        squares.update(square_speed)
        squares.draw(screen)

        player.update(player_direction, player_speed)
        player.draw(screen)

        player_score_text = font.render("Score: " + str(player_score),
                                        1, FONTCOLOR)
        screen.blit(player_score_text, (0, 0))
        
        pygame.display.update()

        for event in pygame.event.get():
                if event.type == QUIT:
                    pygame.quit()
                    sys.exit()
                elif event.type == KEYUP:
                    if event.key in (K_RIGHT, K_d):
                        if left_pressed():
                            player_direction = -1
                        else:
                            player_direction = 0
                    elif event.key in (K_LEFT, K_a):
                        if right_pressed():
                            player_direction = 1
                        else:
                            player_direction = 0
                elif event.type == KEYDOWN:
                    if event.key in (K_RIGHT, K_d):
                        if not left_pressed():
                            player_direction = 1
                        else:
                            player_direction = 0
                    elif event.key in (K_LEFT, K_a):
                        if not right_pressed():
                            player_direction = -1
                        else:
                            player_direction = 0
                
        clock.tick(FPS)
        frame_counter += 1

    
def left_pressed():
    if pygame.key.get_pressed()[pygame.K_a] or\
       pygame.key.get_pressed()[pygame.K_LEFT]:
        return True
    else:
        return False
    
def right_pressed():
    if pygame.key.get_pressed()[pygame.K_d] or\
       pygame.key.get_pressed()[pygame.K_RIGHT]:
        return True
    else:
        return False


if __name__ == '__main__':
    main()

Hier das komplette Spiel (Code, Bilder & Sounds) zum Download: Catch-the-Squares.rar

Vielen Dank im Voraus!

Mfg
Pythoguras

@Pythoguras: Okay, ein paar Anmerkungen:

Das `math`-Modul wird importiert, aber nicht benutzt.

Die Zuweisung der Fenstergrössen könnte man zu einer Zeile zusammenfassen:

Code: Alles auswählen

SCREEN_WIDTH = 500
SCREEN_HEIGHT = 700
SCREEN_SIZE = SCREEN_WIDTH, SCREEN_HEIGHT

# ->

SCREEN_SIZE = SCREEN_WIDTH, SCREEN_HEIGHT = 500, 700

`Player.get_rect()` ist überflüssig. Zum einen kann man auch direkt auf das Attribut zugreifen und zum anderen wird diese Methode im Programm gar nicht verwendet.

Die Bewegungen in den `Sprite.update()`-Methoden sind ein wenig ineffizient. Beim `Square` ist das ganz einfach zu lösen: Statt in einer Schleife über alle Zahlen von 0 bis `square_speed` 1 zu addieren könnte man auch *einmal* ohne Schleife `square_speed` addieren. Beim `Player` sollen gewisse Grenzen nicht über- beziehungsweise Unterschritten werden. Da könnte man einfach die Bewegung berechnen und dann mit `min()` und `max()` dafür sorgen, dass das Sprite nicht über die Grenzen geht und damit auch wieder die Schleife sparen. Oder man gibt dem `Player`-Objekt ein `Rect` mit in dem es sich bewegen kann/darf und verwendet `Rect.clamp()` nachdem man die Position verändert hat. Die Argumentnamen bei den beiden Funktionen brauchen auch nicht unbedingt den Typ auf dem die Methode definiert ist als Präfix.

`Square.is_bad()` ist überflüssig, da kann man direkt auf das `is_bad_square`-Attribut zugreifen beziehungsweise das auch umbenennen zu `is_bad`.

`Square.draw()` wird nicht verwendet, kann also weg.

`player_speed`, `square_speed`, und `frames_between_squares` sind Konstanten, können also aus der `main()` heraus gezogen werden.

Beim erzeugen eines neuen Quadrats ist der Inhalt zwischen den beidne ``if``/``else``-Zeigen sehr ähnlich. Das könnte man heraus ziehen und erst bestimmen ob das Quadrat „böse” ist, anhand der Information dann das Bild aussuchen und den `Square()`-Aufruf danach nur einmal hinschreiben. Wenn man das alles ein wenig Umstellt kann man die Grössen auch dynamischer gestalten, also zum Beispiel `square_size` einsparen und vom konkreten ausgewählten Bild die Daten verwenden. Dann können beide Bilder unterschiedlich gross sein und sogar nicht-quadratisch, ohne das man etwas am Quelltext ändern müsste.

Bei der Bestimmung der Startposition ist ja eigentlich nur die X-Position interessant, darum würde ich auch nur die an den `Square()`-Aufruf übergeben.

Der Körper der Ereignisschleife ist eine Ebene zu tief eingerückt.

Die Tastenverarbeitung ist zu kompliziert. Einfacher wäre es sich zu merken welche Tasten gerade gedrückt sind, und ein Wörterbuch mit Werten pro Taste zu führen und dann die Werte für alle gedrückten Tasten zusammen zu rechnen. Wenn links und rechts gleichzeitig gedrückt sind, dann ergibt sich aus +1 und -1 automatisch der Stillstand.

Die Squares die nicht mit dem Spieler kollidieren und unten verschwinden, werden nicht aus `squares` entfernt. Diese Spritegruppe wird also mit der Zeit immer grösser durch Sprites die man nie mehr zu Gesicht bekommt, die aber trotzdem weiterhin „berechnet” werden.

Die `main()`-Funktion ist ein wenig zu lang. Man kann davon sicher das ein oder andere in eine Funktion auslagern, als Methode auf bestehenden Typen definieren, oder vielleicht auch ein oder zwei neue Typen einführen. `Square`\s könnte man zum Beispiel in eine eigene Unterklasse von `SpriteGroup` stecken und das aktualisieren der Gruppe als Ganzes dort machen. Zum Beispiel in der `update()` sich sowohl um neu zu erzeugende als auch am unteren Ende zu löschende Sprites kümmern.

Ohne aufteilen der `main()`-Funktion lande ich bei dem hier:

Code: Alles auswählen

#!/usr/bin/env python
 
# Catch the Squares
# by Pythoguras
 
# Catch as much brown (good) squares (+1 point) as possible and avoid the
# radioactive (bad) squares (-1 point). Each collected radioactive square
# will increase the the spawn chance of following radioactive ones
 
# The game is lost when this chance reaches 100 % so the score cannot
# increase anymore.
 
import os
import random
import sys
 
import pygame
from pygame.locals import *
 
os.environ['SDL_VIDEO_CENTERED'] = '1'
 
IMAGE_PATH = 'Documents/Images'
SOUND_PATH = 'Documents/Sounds'
 
FPS = 30
 
SCREEN_SIZE = SCREEN_WIDTH, SCREEN_HEIGHT = 500, 700
BOTTOM_MARGIN = 30
 
BACKGROUND_COLOR = (0, 190, 255)
FONT_COLOR = (255, 0, 0)

PLAYER_SPEED = 30
SQUARE_SPEED = 17
FRAMES_BETWEEN_SQUARES = 30 - SQUARE_SPEED

KEY2DIRECTION = { K_LEFT: -1 , K_a: -1 , K_RIGHT: 1, K_d: 1 }


class Player(pygame.sprite.Sprite):
    def __init__(self, image, position, move_restriction_rect):
        pygame.sprite.Sprite.__init__(self)
        self.image = image
        self.rect = image.get_rect()
        self.rect.center = position
        self.move_restriction_rect = move_restriction_rect
 
    def draw(self, surface):
        surface.blit(self.image, self.rect)
 
    def update(self, direction, speed):
        if direction != 0:
            self.rect.x += direction * speed
            self.rect.clamp_ip(self.move_restriction_rect)


class Square(pygame.sprite.Sprite):
    def __init__(self, image, spawn_x, is_bad):
        pygame.sprite.Sprite.__init__(self)
        self.image = image
        self.rect = image.get_rect()
        self.rect.bottomleft = (spawn_x, 0)
        self.is_bad = is_bad
 
    def update(self, speed):
        if self.rect.bottom <= SCREEN_HEIGHT + self.rect.height:
            self.rect.bottom += speed


try:
    shrink_rect = pygame.Rect.shrink
except AttributeError:
    def shrink_rect(rect, x, y):
        result = rect.move((x // 2, y // 2))
        result.width -= x
        result.height -= y
        return result

 
def main():
    pygame.init()
   
    clock = pygame.time.Clock()
 
    screen = pygame.display.set_mode(SCREEN_SIZE)
    pygame.display.set_caption('Catch the Squares')
   
    player_image = pygame.image.load(os.path.join(IMAGE_PATH, 'player.png'))
    good_square_image = pygame.image.load(
        os.path.join(IMAGE_PATH, 'good_square.png')
    )
    bad_good_square_image = pygame.image.load(
        os.path.join(IMAGE_PATH, 'bad_square.png')
    )
    collision_sound = pygame.mixer.Sound(
        os.path.join(SOUND_PATH, 'collision.wav')
    )
 
    font = pygame.font.Font(None, 28)
 
    screen_rect = screen.get_rect()
    player = Player(
        player_image,
        (screen_rect.centerx, screen_rect.bottom - BOTTOM_MARGIN),
        shrink_rect(screen_rect, 10, 0)
    )
    player_direction = 0
    player_score = 0
 
    squares = pygame.sprite.Group()
    bad_squares_chance = 15
   
    frame_counter = 0
    frames_until_next_square = FRAMES_BETWEEN_SQUARES
 
    pressed_keys = set()

    # Game Loop
    while True:
        screen.fill(BACKGROUND_COLOR)
 
        # Spawn a square
        if frame_counter == frames_until_next_square:
            is_bad = random.randint(0, 100) <= bad_squares_chance
            image = [good_square_image, bad_good_square_image][is_bad]
            spawn_x = random.randint(0, screen_rect.width - image.get_width())
            squares.add(Square(image, spawn_x, is_bad))
           
            frame_counter = 0
            variation = random.randint(
                -FRAMES_BETWEEN_SQUARES / 3, FRAMES_BETWEEN_SQUARES / 3
            )
            frames_until_next_square = FRAMES_BETWEEN_SQUARES + variation
 
        # Player colliding with square
        colliding_square = pygame.sprite.spritecollideany(player, squares)
        if colliding_square:
            if colliding_square.is_bad and player_score > 0:
                player_score -= 1
                bad_squares_chance += 5
            else:
                player_score += 1
            collision_sound.play()
            squares.remove(colliding_square)
            print len(squares)
 
        squares.update(SQUARE_SPEED)
        squares.draw(screen)
 
        player.update(player_direction, PLAYER_SPEED)
        player.draw(screen)
 
        player_score_text = font.render(
            'Score: ' + str(player_score), 1, FONT_COLOR
        )
        screen.blit(player_score_text, (0, 0))
       
        pygame.display.update()
 
        for event in pygame.event.get():
            if event.type == QUIT:
                pygame.quit()
                sys.exit()
            elif event.type == KEYDOWN:
                pressed_keys.add(event.key)
            elif event.type == KEYUP:
                pressed_keys.discard(event.key)
        
        player_direction = sum(KEY2DIRECTION.get(k, 0) for k in pressed_keys)

        clock.tick(FPS)
        frame_counter += 1


if __name__ == '__main__':
    main()

Das deutsche Python-Forum

Erstes Spiel - Catch the Squares

Erstes Spiel - Catch the Squares

Re: Erstes Spiel - Catch the Squares