Projekt SpaceInvaders

Franjo G Anregungen? Hey, Du hast gefragt. 😈

Du hattest ja gestern am Stammtisch gefragt was man gegen dieses leichte ”Gedränge” am Anfang machen kann: Entsteht das nicht hauptsächlich weil die Startgeschwindigkeit der Gegner (eine Konstante) höher ist als die hart kodierte Geschwindigkeit nach dem der Gegner die Richtung wechselt? Da sollte auch die Konstante verwendet werden. Und das ”reflektieren” lässt sich dann auch einfacher schreiben, denn der Richtungswechsel ist dann ja einfach nur ein Vorzeichenwechsel a la self.x_speed = -self.x_speed. Man braucht dann auch keine zwei Abfragen für die Ränder, sondern kann das in einer Abfrage zusammenfassen.

        if not 0 < self.x < 736:
            self.y += self.y_speed
            self.x_speed = -self.x_speed

Mein Lieblingsthema die Vorsilbe my in Namen: Wenn's den gleichen Namen nicht auch mit our, their, marys, peters, … gibt, dann bringt das my so gar keinen zusätzlichen Informationswert.

Ich bin so gar kein Fan von kryptischen Abkürzungen. Wenn man image meint, sollte man nicht nur img schreiben. Quelltext wird deutlich öfter gelesen als geschrieben, darum sollte man auf leichte Lesbarkeit optimieren, nicht auf wenig tippen. Zumal letzteres ja heute nicht mehr das ”Problem” ist, was es vor Editoren mit Autovervollständigung war. Man tippt ja heute wie früher bgimg, und dann Tab um background_image einzugeben. Statt da wie früher bgimg überall im Quelltext stehen zu haben und beim ersten mal einen Kommentar „background image“ dazu zu schreiben.

Ausser Konstanten, Funktionen, und Klassen sollte man nichts auf Modulebene definieren. game wird da aber definiert. Darum sollte auch das Hauptprogramm in einer Funktion stehen. Die heisst üblicherweise main().

Die __init__()-Methode ist dazu da ein Objekt zu initialisieren, so dass man es verwenden kann. Da gehört kein Programmablauf rein, so dass man das Objekt an der Stelle wo man es erstellt gar nicht braucht.

Die Attribute width und height beim Game-Objekt werden nirgends wirklich gebraucht. Auch clock und running müssen nicht an das Objekt gebunden werden, das sind einfach nur lokale Variablen. running kann man sich eigentlich auch sparen wenn man die Methode einfach mit return verlässt, statt das auf False zu setzen.

Beim erstellen der enemy-Liste bietet sich eine „list comprehension“ an.

Code und Daten sollte man nicht wiederholen. Das macht unnötig Arbeit und ist fehleranfällig beim schreiben und beim warten/verändern. Der Code um die Zufallskoordinaten für einen Gegner festzustellen sollte beispielsweise nur einmal im Quelltext stehen. Wenn man den Bereich mal ändern will, muss man das sonst nicht nur an einer Stelle ändern, sondern überall wo das gemacht wird, und dass dann auch überall gleich(wertig) ändern. Im Grunde ist das Problem ja bereits vorhanden: Y zwischen 30 und 130 oder zwischen 50 und 150? Warum unterscheidet sich das an den beiden Stellen?

Sowohl event.type als auch event.key können jeweils nur einen Wert haben, deshalb macht es keinen Sinn da alle Werte unabhängig zu prüfen. Da sollten die entsprechenden ifs eigentlich elifs sein.

change_x wäre ein guter Name für eine Methode die x ändert, weil der Name eine Tätigkeit beschreibt. Das ist kein guter Name für X-Geschwindigkeit/Distanz die pro Frame zurückgelegt wird.

Spaceship.move() macht extrem überraschendes. Da würde man erwarten, dass das einmal um die entsprechende Distanz bewegt wird und nicht, dass da eine permanente Bewegung gestartet wird. Und selbst dann ist es auch überraschend, dass die Distanzangabe nicht absolut ist, sondern sich auf die aktuelle Richtung und Geschwindigkeit auswirkt. Also das move(0) überhaupt keinen Einfluss hat, statt die Bewegung zu stoppen. Das macht die Tastenauswertung unnötig schwer verständlich. Also das dort beim Drücken die Bewegung startet und beim loslassen stoppt, darauf wird kaum jemand kommen, so wie das geschrieben ist. Das wäre so viel offensichtlicher was da passiert:

                elif event.type == pygame.KEYDOWN:
                    if event.key == pygame.K_LEFT:
                        self.spaceship.x_speed = -SPACESHIP_MOVE_SPEED
                    elif event.key == pygame.K_RIGHT:
                        self.spaceship.x_speed = SPACESHIP_MOVE_SPEED
                    ...

                elif event.type == pygame.KEYUP:
                    if event.key in [pygame.K_LEFT, pygame.K_RIGHT]:
                        self.spaceship.x_speed = 0

Der Test ob etwas in einem Container-Objekt enthalten ist, wird normalerweise in Python einfach über den Wahrheitswert des Containerobjekts gemacht. Also nicht if len(things) > 0: sondern einfach nur if things:. Und einfach nur eine for-Loop ”schützt” man damit auch nicht, denn das macht ja gar keinen Unterschied ob man die nur ausführt wenn etwas in things enthalten ist, oder ob man die einfach ausführt.

Da wo mir das aufgefallen ist, hast Du auch einen Fehler: Man löscht nichts aus Datenstrukturen über die man gerade iteriert. Das funktioniert nicht wirklich:

In [76]: xs = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]

In [77]: for x in xs:
    ...:     xs.remove(x)
    ...: 

In [78]: xs
Out[78]: [2, 4, 6, 8, 10]

Wenn man die Datenstruktur verändert, kann das immer auch Auswirkungen auf die Schleife haben. Bei den Geschossen hast Du Glück, dass das a) wahrscheinlich funktioniert, und b) das wenn es nicht funktioniert, das im nächsten Frame dann wahrscheinlich funktioniert, und es der Benutzer nicht merkt. Aber so ganz grundsätzlich: Keine Strukturen verändern über die man gerade iteriert, also nicht so, dass sich deren Grössse ändert, oder anderweitig Auswirkungen auf das iterieren entstehen können.

Übliches Vorgehen ist einfach eine neue Datenstruktur ohne die unerwünschten Elemente zu erstellen. Oder die unerwünschten Elemente sammeln und danach in einer extra Schleife löschen. Beziehungsweise würde man sich bei einer Liste die Indizes merken und die dann in umgekehrter Reihenfolge aus der Liste entfernen. Mit list.remove() hätte man nämlich eine quadratische Laufzeit die man nicht haben möchte, denn remove() enthält seinerseits eine Schleife, die die Liste Element für Element durchgeht um das zu entfernende zu suchen. An einfachsten und leicht verständlich wäre hier eine „list comprehension“ die eine neue Liste erstellt, die nur aktive Geschosse enthält.

An der Stelle wo Raumschiff und Geschosse aktualisiert werden, fällt auch ein Zuständigkeitsproblem bezüglich Objektorientierung auf. Wenn die Geschosse in die Zuständigkeit des Raumschiffs gehören würden, dann hätte man da nicht den update()-Aufruf auf dem Raumschiff und danach eine Schleife die durch die Geschosse vom Raumschiff laufen und auf denen dann update() aufruft, sondern diese Schleife wäre in der update()-Methode vom Raumschiff. Ich würde aber stark infrage stellen ob das Raumschiff überhaupt für die Geschosse verantwortlich ist. Die sind abgefeuert und bewegen sich wie Raumschiff und Gegner in der Spielwelt, unabhängig vom Raumschiff. Da ist das Game-Objekt für zuständig. Noch krasser ist das bei der Kollisionsprüfung im Enemy-Objekt. Das kennt das Spiel, weiss, dass es da ein Raumschiff gibt, und das dieses Raumschiff die Geschosse verwaltet (faktisch nicht wirklich). Das ist viel zu viel wissen und ein viel zu weiter Durchgriff durch die Objektstruktur für ein Enemy-Objekt. Das wäre auch eher etwas für das Objekt das alle Beteiligten (Gegner und Geschosse) kennt, und die Regeln der Spielwelt durchsetzen muss: das Game-Objekt.

Zusätzlicher Bonus: Man braucht weder Raumschiff, noch Geschosse oder Gegner, als Attribute auf irgendeinem Objekt. Das sind dann alles lokale Variablen in der Funktion oder Methode mit der Hauptschleife. Letztlich kann da vielleicht sogar die Game-Klasse wegfallen, weil die nicht mehr wirklich etwas sinnvolles verwaltet.

print_score() und print_game_over() wären dann Funktionen, und die sind sich so ähnlich, dass das auch eher nur eine Funktion ist, die halt Argumente bekommt.

sequence[len(sequence) - 1] ist einfach sequence[-1]. Negative Indizes greifen ”von hinten” auf die Sequenz zu.

Wo das verwendet wird, könnte man sich das aber sparen wenn Geschosse gleich im abgefeuerten Zustand erzeugt werden. Nicht abgefeuerte Geschosse gibt es letztlich ja auch gar nicht um Programm, weil dort wo ein Geschoss erzeugt wird, immer auch gleich fire() aufgerufen wird.

i als Name, insbesondere in Schleifen, nur für ganze Zahlen, nicht für komplexere Objekte. Das ist verwirrend, das erwartet niemand.

Attributnamen sollten nicht noch mal den Klassennamen enthalten. spaceship.spaceship_image ist unnötig redundant und verrät dem Leser nicht mehr als spaceship.image.

Die Position der Gegner auf 1000 setzen, damit sie nicht mehr sichtbar sind, ist keine so gute Idee. Man muss das ändern wenn das Spielfeld mal grösser wird, und man muss beim lesen auch darüber nachdenken, was damit gemeint ist beziehungsweise bezweckt wird. self.enemies = [] sagt das sehr viel deutlicher aus. Wech mit den Dingern. Und dann diesen Fall auch explizit im Code behandeln, denn momentan werden ja auch die nicht-sichtbaren Gegner immer und immer alle abgearbeitet und gegen alle Geschosse geprüft.

Das verschleiert auch so ein bisschen die Gemeinsamkeiten zwischen Deinen Klassen. Denn Raumschiff, Geschosse, und Gegner haben ja alle einen gemeinsamen Satz an Daten: das Spiel, die Koordinaten, und ein Bild sind für alle gleich. Da alle beweglich sind, haben auch alle eine X- und eine Y-Geschwindigkeit, auch wenn die für einige Komponenten 0 sein kann. Bewegen, Bild laden, und sich selbst zeichnen ist auch für alle gleich. Das sind also alles Sachen die man einmal in eine Basisklasse schreiben kann, statt das in jeder der drei Klassen erneut zu programmieren.

Was aus OOP-Sicht auch komisch riecht ist die enge Kopplung zwischen Spiel und den beweglichen Objekten. Die Attribute eines Objekts beschreiben woraus sich das zusammensetzt. Ein Spiel besteht aus dem Spielfeld, dem Raumschiff, den Gegnern, den Geschossen. Die Beschreibung klingt okay. Aber zu sagen ein Raumschiff besteht aus den Koordinaten, einem Bild, und dem Spiel klingt komisch/falsch. Das Spiel ist nicht Bestandteil des Raumschiffs. Wenn sich Objekte gegenseitig kennen müssen ist das oft ein Warnzeichen. Eine Lösung besteht darin die eine Seite der anderen als Argument zu übergeben. Also beispielsweise Spaceship.update() das Game-Objekt beim Aufruf zu übergeben.

Das ganze mal nach den genannten Punkten überarbeitet:

#!/usr/bin/env python3
import math
from pathlib import Path
from random import randint

import pygame

SELF_PATH = Path(__file__).parent

ENEMY_COUNT = 15
ENEMY_X_SPEED = 5
ENEMY_Y_SPEED = 50
BULLET_SPEED = 10
SPACESHIP_MOVE_SPEED = 10


class Movable:
    IMAGE_FILENAME = ""

    def __init__(self, x=0, y=0, x_speed=0, y_speed=0):
        self.x = x
        self.y = y
        self.x_speed = x_speed
        self.y_speed = y_speed
        self.image = pygame.image.load(SELF_PATH / self.IMAGE_FILENAME)

    def draw(self, surface):
        surface.blit(self.image, (self.x, self.y))

    def update(self, _surface):
        self.x += self.x_speed
        self.y += self.y_speed


class Bullet(Movable):
    IMAGE_FILENAME = "bullet.png"

    def __init__(self, x, y):
        Movable.__init__(self, x, y, y_speed=-BULLET_SPEED)
        self.is_active = True

    def update(self, surface):
        if self.is_active:
            Movable.update(self, surface)
            if self.y < 0:
                self.is_active = False
            else:
                self.draw(surface)


class Spaceship(Movable):
    IMAGE_FILENAME = "spaceship.png"

    def update(self, surface):
        Movable.update(self, surface)
        self.x = min(max(0, self.x), 736)
        self.draw(surface)


class Enemy(Movable):
    IMAGE_FILENAME = "alien.png"

    def place_randomly(self):
        self.x = randint(0, 736)
        self.y = randint(50, 150)

    def update(self, surface):
        self.draw(surface)
        Movable.update(self, surface)
        if not 0 < self.x < 736:
            self.x = min(max(0, self.x), 736)
            self.y += ENEMY_Y_SPEED
            self.x_speed = -self.x_speed
        self.draw(surface)

    @classmethod
    def randomly_placed(cls):
        enemy = cls(x_speed=ENEMY_X_SPEED)
        enemy.place_randomly()
        return enemy


def display_text(surface, text, font_size, position):
    text = pygame.font.Font("freesansbold.ttf", font_size).render(
        text, True, (255, 255, 255)
    )
    surface.blit(text, position)


def main():
    pygame.init()
    screen = pygame.display.set_mode((800, 600))
    pygame.display.set_caption("Space Invaders")
    score = 0
    background_image = pygame.image.load(SELF_PATH / "stars.png")
    spaceship = Spaceship(370, 515)
    enemies = [Enemy.randomly_placed() for _ in range(ENEMY_COUNT)]
    bullets = []
    clock = pygame.time.Clock()
    while True:
        clock.tick(60)
        screen.blit(background_image, (0, 0))
        for event in pygame.event.get():
            if event.type == pygame.QUIT:
                return

            elif event.type == pygame.KEYDOWN:
                if event.key == pygame.K_LEFT:
                    spaceship.x_speed = -SPACESHIP_MOVE_SPEED
                elif event.key == pygame.K_RIGHT:
                    spaceship.x_speed = SPACESHIP_MOVE_SPEED
                elif event.key == pygame.K_SPACE:
                    bullets.append(Bullet(spaceship.x, spaceship.y))

            elif event.type == pygame.KEYUP:
                if event.key in [pygame.K_LEFT, pygame.K_RIGHT]:
                    spaceship.x_speed = 0

        spaceship.update(screen)
        for bullet in bullets:
            bullet.update(screen)
        bullets = [bullet for bullet in bullets if bullet.is_active]

        if enemies:
            for enemy in enemies:
                enemy.update(screen)
                for bullet in bullets:
                    distance = math.sqrt(
                        (enemy.x - bullet.x) ** 2 + (enemy.y - bullet.y) ** 2
                    )
                    if distance < 35:
                        bullet.is_active = False
                        score += 1
                        enemy.place_randomly()

                if enemy.y > 480:
                    print(enemy.y)
                    enemies = []
                    break
        else:
            display_text(screen, "GAME OVER", 64, (200, 250))

        display_text(screen, f"Punkte: {score}", 24, (8, 8))
        pygame.display.update()


if __name__ == "__main__":
    main()

Was da noch wirklich unschön ist, und was man dringend angehen sollte, sind die vielen hart kodierten Zahlen die dazu auch nicht unabhängig voneinander sind. Viele Startpositionen, Grenzen usw. hängen ja beispielsweise von der Spielfeld- und Spritegrösse ab. Da sollte dann beispielsweise nicht 742 oder so stehen, sondern an der Stelle sollte entweder die Sprite-Breite von der Spielfeldbreite abgezogen werden, denn dann stimmt der Wert auch wenn man Spielfeldgrösse und/oder Sprite-Grösse ändert, ohne das man den Code überall ändern muss. Oder man arbeitet mehr mit dem was Pygame bietet. Zum Beispiel Rect-Objekte die eine Position haben und viele nützliche Methoden und Properties. Man vergleiche if self.x > surface.get_width() - self.width: mit if self.rect.right > surface.get_width(). Rect-Objekte kann man auch verschieben, braucht also nicht X und Y als Einzelwerte. Und man von Surface-Objekten ein Rect relativ zu einer anderen Koordinate abfragen. Beispielsweise wenn man ein Surface mit Text hat, das man genau mittig auf einem anderen blitten möchte:

screen.blit(text, text.get_rect(center=screen.get_rect().center))

Und dann hat Pygame schon fertiges für Sprites und Spritegruppen und Kollisionen zwischen denen.

Quote from __blackjack__

Mein Lieblingsthema die Vorsilbe my in Namen: Wenn's den gleichen Namen nicht auch mit our, their, marys, peters, … gibt, dann bringt das my so gar keinen zusätzlichen Informationswert.

Das MY_PATH kam gestern von mir. Und in diesem Zusammenhang finde ich das "my" passend.

Das Script denkt: "Wo bin ich?". Steht halt in MY_PATH...

Franjo G ”Meine” Version ist ja aber eigentlich Dein Code, halt überarbeitet. Ist ja nicht was komplett anderes, von Grund auf neu geschrieben.

Und wirklich hart falsch/fehlerhaft war auch nur das löschen aus der Liste in der Schleife über die Liste. Auch wenn das letztlich beim Spielen niemandem aufgefallen wäre, ist das halt wirklich so eine Stolperfalle die man kennen muss, weil das ist wohl eher selten, dass es unwichtig ist wenn in einem Durchgang Elemente da bleiben können, die eigentlich raus sollten.

Hallo,

zur Inspiration was die angesprochene `Sprite`-Klassen angeht, ich hatte die "damals" verwendet:

SpaceInvader/SpaceInvaders.py at master · Dennis-89/SpaceInvader

Some Code Refactoring. Contribute to Dennis-89/SpaceInvader development by creating an account on GitHub.

github.com

Grüße
Dennis

Es geht nicht um die Größe, sondern um `Sprite`-Klasse die pygame bietet.

pygame.sprite — pygame v2.6.0 documentation

Grüße
Dennis

Franjo G Naja, es ist halt relativ einfach das so zu schreiben, dass die Grösseninformation von den Bilddateien genommen werden, also eine Grössenänderung keine Änderungen am Code nach sich ziehen, sondern einfach durch austauschen der Bilddateien passieren.

Und was ist mit der anderen Richtung? Wenn man an den Raspi ein 320×240 Pixel Display anschliesst? Dann könnten die momentanen Sprites ein bisschen zu gross sein.

Franjo G Du hast einen Fan:

http://www.bildschirmarbeiter.com/content/images/picdump-16-08-05/picdump-16-08-05-029.jpg

MfG

Jürgen

Was muss ich machen wenn ich das auf meinen Pi5 mit grafischem raspiOS spielen will?

Quote from momefilo

Was muss ich machen wenn ich das auf meinen Pi5 mit grafischem raspiOS spielen will?

Herunterladen aus #8, auspacken der zip-Datei, Starten von spaceInvaders.py

Also damit hast du eine Ausgangsbasis, die Gegner sollten langsamer, zufälliger oder "ästhetischer" bewegt werden. Grafisch find ich es gut, würd ich als spielen wenn die Gegner besser bewegt wären (Ich mag einfache Ballerspiele aus den 80ern)