Pygame kollisionen

[Fire Spike]

Wirst du das mal noch unter die nehmen oder soll ich die Hoffnung auf eine Antwort aufgeben?

[__deets__]

Ich schaue es mir mal an. Aber garantiere keine Antwort Zeit!

[__blackjack__]

@Fire Spike: Mal schauen: `sys` wird importiert, aber nirgends verwendet.

Da ist einiges an Kommentaren drin die einfach nur *sehr* offensichtliches ”erklären”, die also absolut überflüssig sind.

Es gibt auch falsche Kommentare. `screenx` und `screeny` sind keine Koordinaten sondern die Breite und Höhe des Fensterinhalts. Das könnte man auch im Namen besser zum Ausdruck bringen. Ausserdem fehlen dort Unterstriche, weil es schwerer ist Wörterzulesenundzuverstehenwenndiedirektaneinanderkleben. Und das sind Konstanten, sollten also KOMPLETT_GROSS geschrieben werden. Und last but not least sind hier X und Y vertauscht, was sehr verwirrend ist. Das Problem scheint sich durch den gesamten Code zu ziehen. X ist die horizontale Position und Y ist die vertikale Position.

`icon_paths` und `icon_sizes` sind auch Konstanten. Unschön ist hier, dass das ”parallele” Datenstrukturen sind, also beide die gleichen Schlüssel haben und die auf Werte abbilden. Es wäre sinnvoll(er) die Daten zu einer Datenstruktur zusammen zu fassen. Wobei mir `icon_sizes` auch nicht so wirklich sinnvoll erscheint, weil die Zahlen die dort stehen den Grössen der Bilder entsprechen und man diese Information hier also nicht fehleranfällig redundant manuell ins Programm schreiben muss.

Auf Modulebene sollte nur Code stehen, der Konstanten, Funktionen, und Klassen definiert. Das Hauptprogramm steht üblicherweise in einer Funktion die `main()` heisst. Daraus folgt dann das Funktionen und Methoden alles was sie ausser Konstanten benötigen als Argument(e) übergeben bekommen. Das betrifft `Spike.down()` und `Cube.update()` die beide `screen` übergeben bekommen müssen.

`pygame.init()` beinhaltet alle Subsysteme, `pygame.mixer.init()` danach ist also überflüssig.

Wenn es um das messen von Zeitdifferenzen geht ist `time.monotonic()` in der Regel die bessere Wahl als `time.time()`.

Die hart kodierte 650 für die X-Koordinate des Spielers ist nicht gut. Warum überhaupt 650 und nicht mittig bei 800?

`pygame.quit()` steht da zu oft im Programm. Das gehört einmal hinter die Hauptschleife. Und wenn man die Musik explizit stoppen will, dann *muss* man das *vor* `pygame.quit()` machen, und *sollte* das auch in *jedem* Fall machen, und nicht nur in einem der drei Fälle wo das Programm beendet werden soll.

Wobei man die drei Fälle auf zwei reduzieren kann in dem man die beiden ``if``\s in der ``for``-Schleife über die Ereignisse zusammenfasst. Da dann dort nur noch ein Flag aufgrund der Bedingung gesetzt wird, kann man sich sogar das ``if`` noch sparen.

Die `collide_mask()`-Funktion liefert laut Dokumentation einen Wahrheitswert. Es ist nicht nett von der Funktion im `False`-Fall `None` zu liefern, aber man sollte nicht auf `None` vergleichen wenn die Dokumentation das nicht garantiert.

Wenn man eine Kollision festgestellt hat, kann man die Schleife über die Spikes abbrechen.

Den folgenden Code muss ich einfach mal zitieren:

Code: Alles auswählen

            if not spike.is_visible():
                visible_spikes.append(spike)

Lies das mal bitte laut jemanden vor. Das kommt Dir nicht irgendwie komisch vor Spiles zu `visible_spikes` hinzuzufügen wenn sie *nicht* „visible“ sind? Hier ist doch offensichtlich etwas falsch benannt oder falsch implementiert. Und zwar `Spike.is_visible()` was genau das Gegenteil von dem liefert was der Name suggeriert.

Die Methode hat auch noch einen weiteren Fehler weil der ``else``-Zweig einen Ausdruck enthält der nichts tut und die Methode dadurch nicht `True` oder `False` liefert, sondern `True` oder `None`.

Man braucht hier auch gar kein ``if``/``else`` denn das Ergebnis hat man ja schon durch die Bedingung die da ausgewertet wird.

Die hart kodierte 800 in der Methode sollte man durch die Konstante mit der Fensterhöhe ersetzen.

Das mit dem zwei Listen `all_spikes` und `visible_spikes` ist unnörig kompliziert. Man würde da besser nur *eine* Liste führen und das am Schleifenende einfach immer alle aus dem Bild gefallenen Spikes raus filtern.

In den Klassen läuft viel zu viel Code unnötigerweise immer wieder aufs neue ab, obwohl der immer das gleiche Ergebnis liefert.

Die Spikes sehen alle gleich aus, aber Du malst nicht nur für jeden Spike immer das gleiche Bild sondern das auch noch jedes mal wenn sich der Spike nach unten bewegt. Es reicht das Bild *einmal* für *alle* Spikes zu malen. Das gleiche gilt für die Maske, die sich ja auch nicht ändert.

Beim auswürfeln der X-Koordinate sollte man die hart kodierte 1600 durch die Konstante ersetzen, welche die Fensterbreite festlegt.

Die X- und Y-Koordinate braucht man nicht speichern weil die bereits im `rect`-Objekt drin steckt.

`Cube.__init__()` ruft die `__init__()` der Basisklasse nicht auf.

Beim `Cube` wird `mask` mehrfach neu berechnet, obwohl sich der Wert dabei nie ändert.

`Cube.update()` heisst IMHO falsch und hat auf jeden Fall einen falschen Docstring, weil das was dort steht in der Methode gar nicht gemacht wird. Zudem wird damit die `Sprite.update()`-Methode überschrieben und das mit einem Inhalt der nicht dazu passt, denn das Blitten wird eigentlich nicht von `Sprite`-Objekten selbst gemacht, sondern von `SpriteGroup`-Objekten.

Zwischenstand:

Code: Alles auswählen

#!/usr/bin/env python3
import random
import time

import pygame

WINDOW_TITLE = "Game"

WHITE = (255, 255, 255)
RED = (255, 0, 0)

FPS = 60

WIDTH = 1600
HEIGHT = 800

PLAYER_Y = HEIGHT - 100
assert PLAYER_Y > 0

ICON_SIZE_TO_PATH = {
    "small": "Images/Icons/Icon_small.png",
    "medium": "Images/Icons/Icon_medium",
    "large": "Images/Icons/Icon_large",
}


def create_spike_image(size):
    image = pygame.Surface((size, size), pygame.SRCALPHA)
    pygame.draw.polygon(image, RED, ((0, 0), (size, 0), (size // 2, size)))
    return image


class Spike:
    STEP = 20
    SIZE = 100
    IMAGE = create_spike_image(SIZE)
    MASK = pygame.mask.from_surface(IMAGE)

    def __init__(self):
        self.image = self.IMAGE.convert_alpha()
        self.mask = self.MASK
        self.rect = self.image.get_rect(
            center=(random.randrange(0, WIDTH), -self.SIZE)
        )

    def down(self, screen):
        """
        Move the spike down.
        """
        self.rect = self.rect.move(0, self.STEP)
        screen.blit(self.image, self.rect)

    def is_visible(self):
        return self.rect.top < HEIGHT


class Cube(pygame.sprite.Sprite):
    STEP = 10
    def __init__(self, x, icon_size):
        pygame.sprite.Sprite.__init__(self)
        self.image = pygame.image.load(
            ICON_SIZE_TO_PATH[icon_size]
        ).convert_alpha()
        self.rect = self.image.get_rect(center=(x, PLAYER_Y))
        self.mask = pygame.mask.from_surface(self.image)

    def left(self):
        if self.rect.right > 0:
            self.rect = self.rect.move(-self.STEP, 0)

    def right(self):
        if self.rect.left < WIDTH:
            self.rect = self.rect.move(self.STEP, 0)

    def blit(self, screen):
        screen.blit(self.image, self.rect)


def main():
    pygame.init()
    pygame.display.set_caption(WINDOW_TITLE)
    screen = pygame.display.set_mode((WIDTH, HEIGHT))
    screen.fill(WHITE)
    clock = pygame.time.Clock()

    pygame.mixer.music.load("Sounds/Death-Moon.mp3")
    pygame.mixer.music.play()
    #
    # TODO Use `SpriteGroup` for spikes and player.
    #
    spikes = list()
    last_time = time.monotonic()
    player = Cube(WIDTH // 2, "small")
    player_is_alive = True
    while player_is_alive:
        screen.fill(WHITE)
        for event in pygame.event.get():
            player_is_alive = not (
                event.type == pygame.KEYDOWN
                and event.key == pygame.K_ESCAPE
                or event.type == pygame.QUIT
            )

        pressed_keys = pygame.key.get_pressed()
        if pressed_keys[pygame.K_RIGHT]:
            player.right()
        if pressed_keys[pygame.K_LEFT]:
            player.left()

        player.blit(screen)

        if time.monotonic() - last_time > 0.1:
            spikes.append(Spike())
            last_time = time.monotonic()

        for spike in spikes:
            spike.down(screen)

        pygame.display.flip()

        for spike in spikes:
            if pygame.sprite.collide_mask(spike, player):
                player_is_alive = False
                break

        spikes = [spike for spike in spikes if spike.is_visible()]

        pygame.display.set_caption(str(clock.get_fps()))
        clock.tick(FPS)

    pygame.mixer.music.stop()
    pygame.quit()


if __name__ == "__main__":
    main()

Hier funktioniert das mit der Kollision dann auch richtig. Frag nicht an welcher Änderung es lag.

[Fire Spike]

Danke für deine Hilfe __blackjack__.
Diese Frage hat weniger mit meinem Spiel zu tun, aber ich stelle sie jetzt einfach hier.
Ich hatte mein Spiel(oben) auf dem Raspberry Pi 4 geschrieben und getestet.
Da waren die FPS immer zwischen 22 und 27.
Als ich jetzt den Raspberry auf 2 GHz übertaktet habe erreicht es plötzlich bis zu 140 FPS.
Was hat das für einen Grund?

[__deets__]

Ich kann nur raten, aber ich vermute da kommt irgendwie die clock durcheinander.

[Fire Spike]

Ich kann nur raten, aber ich vermute da kommt irgendwie die clock durcheinander.

Was meinst du damit?

[__deets__]

Was ich gesagt habe. Wenn ich deine Kuechenuhr manipuliere, dann werden deine Fruehstueckseier auch nicht mehr wie gewuenscht. Denn dann sind eben 3 Minuten keine 3 Minuten mehr. Ob das wirklich so ist, kann ich hier mangels PI4, overclocking etc. nicht nachvollziehen. Aber du kannst ja mal nachschauen, wie genau clock.tick(FPS) implementiert ist, um dem auf die Schliche zu kommen.

[Fire Spike]

Leider ist das ein SO Datei

[__deets__]

Ja und? Du kannst doch in den Quellcode schauen, das ist doch alles open source.