Optimisierung von einem kleinen Programm mit Pygame

Wenn du dir nicht sicher bist, in welchem der anderen Foren du die Frage stellen sollst, dann bist du hier im Forum für allgemeine Fragen sicher richtig.
Antworten
Daniel Schreiber
User
Beiträge: 21
Registriert: Sonntag 14. Januar 2018, 15:03

Hallo, ich habe ein sehr kleines Programm geschrieben, das Regen simuliert. Ich habe es schon profiliert, konnte aber nichts signifikant verbessern. Manche Kommentare mögen Sinnlos wirken, die brauche ich aber später wenn ich mehr Inhalt hinzufüge. Danke für jede Hilfe
https://www.file-upload.net/gal-264090/9lmcsl/1.html
__deets__
User
Beiträge: 14494
Registriert: Mittwoch 14. Oktober 2015, 14:29

Die beschissene Upload-Seite versucht einem hartnäckig etwas unterzuschieben. Man kann bei github umsonst und ohne so einen Mist Code hosten. Du solltest dir das wirklich überlegen. Oder den Code gleich hier (in den notwendigen Tags) Posten. Denn nochmal tue ich mir diese Seite nicht an....

Auf den ersten Blick sehe ich nichts dramatisches in Bezug auf Laufzeit. Was also genau tut nicht?
Daniel Schreiber
User
Beiträge: 21
Registriert: Sonntag 14. Januar 2018, 15:03

Ah ja das werde ich berücksichtigen. Ich habe mich gefragt ob ich noch effizienter drawen kann, da das Zeichnen viel Zeit benötigt
__deets__
User
Beiträge: 14494
Registriert: Mittwoch 14. Oktober 2015, 14:29

Woher weisst du das? Braucht es zu VIEL Zeit? Fuer wieviele Baelle braucht es das?

Und wie gesagt, wenn du das auf github packst, Grafiken inklusive etc so das man es einfach nachvollziehen kann, um so besser. Dann kann man das ausprobieren.
Benutzeravatar
__blackjack__
User
Beiträge: 13004
Registriert: Samstag 2. Juni 2018, 10:21
Wohnort: 127.0.0.1
Kontaktdaten:

Wobei das zeichnen hier letztlich auch von der Grafikhardware und ob Pygame davon irgend etwas nutzen kann, abhängen könnte.
“Most people find the concept of programming obvious, but the doing impossible.” — Alan J. Perlis
Daniel Schreiber
User
Beiträge: 21
Registriert: Sonntag 14. Januar 2018, 15:03

https://github.com/DanielSchreiberMendes/Rain
Hab keine eigenen Grafiken drinnen. Hätte noch eine Frage, ich habe die Klasse Test(), welche alles verwaltet. Die Klasse Hud() wird von ihr referenziert und die Klasse Slider wieder von dieser. Ich möchte, dass man dem Silder eine beliebige Variable(bool) geben kann und er diese entsprechend verändert. Das funktioniert soweit auch.

Code: Alles auswählen

#In der Klasse Hud, self.test.raining soll geändert werden
	#....
	self.rainSlider = Slider((120, 500), self.test.rain.raining, test)

        def update(self):
       		self.test.rain.raining = self.rainSlider.update()
       		
#In der Klasse Slider
#  self.test.rain.raining heißt hier self.stateToChange
	def update(self):
	#.....
		return self.stateToChange
       
Ich möchte aber, dass ich self.test.raining, bzw. stateToChange noch im Slider ändere. Ich hab einige Wege probiert, ohne Erfolg. Ich glaube dasliegt daran, dass self.stateToChange unabhängig von self.test.raining ist und es nicht verändert. Wie kann ich das ändern, aber so, dass der Silder flexibel bleibt, also beliebige Aufgaben übernehmen kann, entsprechend der Eingabevariable?
__deets__
User
Beiträge: 14494
Registriert: Mittwoch 14. Oktober 2015, 14:29

Also es laeuft nach ein paar kleinen Anpassungen. Der Hud-Import ist falsch, und die unnoetige Windows-Spezifische Abfrage ist raus. Danach tropft's.

Das Ding macht, was es soll. Und wenn man so viele Partikel macht, dann brauchen die halt Zeit. Ich zaehle knapp 1000. Wenn du das schneller machen willst, musst du ggf. mit einer (oder 10) zerplatz-Animationen arbeiten, die du dann abspielst. Statt selbst Pixel-Level-Manipulation zu betreiben.

Ansonsten gibt's ne Menge zu bemerken: das die Klasse Rain von aussen die Partikel fuer die Tropfen steuert, statt den Tropfen beim zerplatzen selbst die Kontrolle darueber uebernehmen zu lassen ist nicht gut. Das du raindrops veraenderst indem du Tropfen rausnimmst waehrend du ueber die Liste iterierst ist ein Fehler. Dadurch springt der Iterator quasi mit 2 Schritten durch die Liste. Stattdessen sammelt man die zu entfernenden Items auf, und tut das am *Ende*. Die vielen hard-kodierten Zahlen wie die Kollisions-Konstante "140", 50, 0.2 usw.

Und zu deinem Slider-Problem: es gibt mehrere Loesungen dafuer. Ein Klassiker ist zB eine Liste von Funktionen zu verwalten, die bei Aenderung des Sliders mit dem neuen Wert aufgerufen werden. Dann kannst du durch zB ein lambda festlegen, was dann genau passieren soll. Alternativ kann man "property"-Objekte programmieren, ala tkinters StringVar, IntVar etc. Damit kannst du einen Wert praktisch rumreichen, und durch Aufruf von "var.get()" bekommst du den aktuellen Wert.
Daniel Schreiber
User
Beiträge: 21
Registriert: Sonntag 14. Januar 2018, 15:03

Was soll ich statt der jetzigen Windows abfrage benutzen? Gibt Viele verschiedene, ich will, dass das Programm für jede Auflösung funktioniert. Der Hud-Import ist falsch, weil ich bei GitHub keine Ordner hochladen konnte, sondern nur Dateien.

Wie kann ich ohne Bilder zu benutzen, eine Animation erstellen?; Bilder brauchen so lange zu laden. Sollte ich die Klasse Rain behalten oder ganz löschen?

Danke dein Kommentar ist sehr hilfreich, ich werde das versuchen.
__deets__
User
Beiträge: 14494
Registriert: Mittwoch 14. Oktober 2015, 14:29

Ich verstehe den Zusammenhang von Aufloesungen und Windows nicht. pygame kann fullscreen, und regelt das fuer dich ohne das du dich um die Platform kuemmern must.

Und wenn du sagst, du willst es so machen, wie du es machen willst, dann bleibt's halt wie es ist. Mit "moeglichst schnell" hat das allerdings nichts zu tun

Die Klasse Rain kannst du durchaus behalten, da sie das spawing von Regentropfen regelt. Mein Kommentar bezieht sich auf das Verhalten des Tropfens selbst. DAS solltest du nicht unnoetig von aussen micro-managen. Das widerspricht dem Sinn der OO.
Benutzeravatar
__blackjack__
User
Beiträge: 13004
Registriert: Samstag 2. Juni 2018, 10:21
Wohnort: 127.0.0.1
Kontaktdaten:

@Daniel Schreiber: Git kann auch Ordnerstrukturen (solange etwas darin enthalten ist).
“Most people find the concept of programming obvious, but the doing impossible.” — Alan J. Perlis
Daniel Schreiber
User
Beiträge: 21
Registriert: Sonntag 14. Januar 2018, 15:03

Ich brauche die Auflösung, um zB feststellen zu können, wann der Tropfen den Screen überschreitet. Ich würde schon eine Animation benutzen, die Partikel sollen später allerdings mit anderen Objekten interagieren können
__deets__
User
Beiträge: 14494
Registriert: Mittwoch 14. Oktober 2015, 14:29

Ich habe nicht gesagt, dass du die Aufloesung nicht benutzen sollst. Es gibt nur keinen Grund, dafuer den von dir gezeigten Code zu verwenden. Du kannst jederzeit den screen nach seiner Aufloesung befragen.
Benutzeravatar
__blackjack__
User
Beiträge: 13004
Registriert: Samstag 2. Juni 2018, 10:21
Wohnort: 127.0.0.1
Kontaktdaten:

@Daniel Schreiber: Die Auflösung kannst Du aber vorgeben. Oder eine aus `pygame.display.list_modes()` auswählen, oder über ein `pygame.display.Info`-Exemplar die aktuelle Breite und Höhe abfragen. Hauptsache nix plattformabhängiges wenn man mit Pygame/SDL schon etwas plattformunabhängiges verwendet.
“Most people find the concept of programming obvious, but the doing impossible.” — Alan J. Perlis
Benutzeravatar
__blackjack__
User
Beiträge: 13004
Registriert: Samstag 2. Juni 2018, 10:21
Wohnort: 127.0.0.1
Kontaktdaten:

@Daniel Schreiber: Der `Hud`-Import ist auch deswegen falsch weil der gar nicht zusätzlich noch einmal in einem Package stecken sollte. Du hast da ein Package `Hud` in dem *ein* Modul `Hud` steckt, in dem *eine* Klasse `Hud` definiert wird. Das ist auf jeden Fall ein `Hud` zu viel, und für meinen Geschmack sogar zwei `Hud` zu viel. Das ganze Programm ist auf viel zu viele Module aufgeteilt. Ein einziges wäre wahrscheinlich sogar schon genug. Dann würde sich das mit den Sternchen-Importen auch gleich erledigen – die sind Böse™. Das macht das Programm unübersichtlich, es besteht die Gefahr von Namenskoventionen, und wenn man am Ende sowieso überall, alles per Sternchen importiert, wo liegt denn dann der Sinn das auf Module aufzuteilen? Du hast am Ende im Hauptmodul sowieso alles importiert. Ein *-Import importiert alles aus einem Modul – auch das was dieses Modul selbst irgendwo her importiert hat.

Die `Test`-Klasse repräsentiert keinen Test, sondern das Spiel bzw. die Simulation.

Der `gameloop()`-Aufruf sollte nicht am Ende der `__init__()` stehen, weil man so kein Exemplar erstellen kann mit dem man nach dem Erstellen und *vor* dem Ablaufen der `gameloop()` irgend etwas anstellen kann.

Die `Main`-Klasse ist komisch. Ich sehe keinen Grund `Clock` und das `Surface` für die Anzeige in eine eigene Klasse zu kapseln die sonst nichts macht ausser sich selbst an die eigentliche Klasse für die Simulation zu übergeben. Klassen die nur aus einer `__init__()` bestehen sind in der Regel ein Alarmzeichen, es sei denn es sind wirklich reine, passive Datenklassen, die dort nichts weiter tun als Attribute zu initialisieren. Wobei man dafür aber auch eher so etwas wie das `attr`-Modul verwenden sollte und die `__init__()` nicht selber schreiben sollte.

Einige der Namen halten sich nicht an den Style Guide for Python Code.

Abkürzungen sollte man vermeiden. Das das `m` bei `mCollision` für `mouse` steht, sollte man nicht raten müssen. Für diese Funktion ein eigenes Modul ist übrigens auch extrem übertrieben.

Die Funktion sollte auch nicht `True` und `None` zurückgeben, sondern `True` und `False`. Dazu braucht man auch kein ``return True``/``return False``, denn die Bedingung die jetzt im ``if`` steht wird ja bereits zu `True` oder `False` ausgewertet:

Code: Alles auswählen

def mouse_collision(rect):
    return (
        rect[0] < pygame.mouse.get_pos()[0] < (rect[0] + rect[2])
        and rect[1] < pygame.mouse.get_pos()[1] < (rect[1] + rect[3])
    )
Das geht aber noch viel einfacher. Dazu muss man als erstes aufhören an Namen wie `rect` eine Liste zu binden, statt eines `pygame.Rect`. Denn die Objekte haben bereits eine Testmethode für diesen Zweck:

Code: Alles auswählen

def mouse_collision(rect):
    return rect.collidepoint(pygame.mouse.get_pos())
Die Frage ob man dafür eine eigene Funktion in einem eigenen Modul braucht, wird damit noch grösser.

Wenn man ein Programm auf mehrere Module verteilt, was ich hier wie gesagt nicht als zwingend gegeben sehe, sollte man die auch alle in einem Package zusammenfassen, damit die eigenen Modulnamen nicht mit anderen Modul- und Packagenamen in Konkurrenz stehen.
“Most people find the concept of programming obvious, but the doing impossible.” — Alan J. Perlis
Benutzeravatar
__blackjack__
User
Beiträge: 13004
Registriert: Samstag 2. Juni 2018, 10:21
Wohnort: 127.0.0.1
Kontaktdaten:

@Daniel Schreiber: Ich habe es jetzt auch mal ausprobiert und die erste Kleinigkeit, die mir aber deutlich aufgefallen ist: man kann das nicht mit der Maus beenden in dem man das X in der Fensterleiste klickt.

Ein Weg das zu Beschleunigen ist den Regen hinter dem HUD nicht zu berechnen. Das verdeckt ja einen guten Teil des Bildschirms. Alternativ würde ich das graue Rechteck einfach nicht zeichnen, damit man den Regen an der Stelle auch sehen kann.

`pygame.init()` und `pygame.quit()` haben auf Modulebene nichts zu suchen. Man sollte jedes Modul importieren können, ohne das mehr passiert als das der Modulinhalt (Konstanten, Funktionen, Klassen) definiert wird. `pygame.init()` versucht Hardware zu initialisieren.

Und `pygame.quit()` am Ende möchte man vielleich auch in einen ``finally:``-Zweig schreiben.

Der Name `Slider` ist irreführend. Darunter verstehen alle GUI-Rahmenwerke die ich kenne einen Schieberegler der in aller Regel mehr als nur zwei Positionen hat. Die vorliegende Klasse hätte eher den Namen `Switch` verdient.

`stateToChange` könnte auch einfach nur `state` heissen. Das Zustand veränderbar ist, steckt in „Zustand“ im Grunde schon drin.

Ich würde auch versuchen die Abhängigkeit von dem `Test`-Objekt zu minimieren. In Deinem Entwurf muss ja wirklich jedes Objekt dieses Exemplar kennen, und über das Test-Exemplar kommt man auch an jedes andere Objekt heran. Das ist für meinen Geschmack alles etwas zu stark aneinander gekoppelt. Aus den `draw()`-Methoden kann man es beispielsweise ganz einfach heraus bekommen, in dem man das `Surface` auf dem gezeichnet werden soll als Argument übergibt. Aus `Raindrop` und `Rain` kann man es dann schon rausnehmen, weil deren `update()`-Methode es nicht verwenden.

Wenn man die beiden Farben von dem Schalter-Objekt in einer Liste speichert, lässt sich das schalten kompakter ausdrücken.

Statt sich in `mKlicked` ein Flag zu merken ob mit der Maus geklickt wurde oder nicht, könnte man sich dort entweder `None` oder die Position der Maus beim Klick merken. Und auch den `update()`-Methoden könnte man Argumente mitgeben, damit die Objekte nicht alle das komplette `Test`-Exemplar kennen müssen. Wenn man bei `Hud.update()` die Klickposition übergibt und die dort an `Slider.update()` weiterreicht, dann braucht auch der `Slider` das `Test`-Objekt gar nicht mehr zu kennen.

Nun braucht man nur noch die Verbindung zwischen `Slider` und `Rain` über eine Rückruffunktion realisieren und die in `Test` verbinden, so das `Hud` gar nichts davon wissen muss, und schon muss auch `Hud` das `Test`-Objekt nicht mehr kennen.

Kommentare gehören über den Code den sie kommentieren. An mindestens einer Stelle hast Du das umgekehrt gemacht.

`Raindrop` hat ein paar unbenutze Attribute und `self.particles` wird in der `__init__()` zweimal definiert.

Die Konstante `MTOP` braucht einen besseren Namen. Was soll der aktuelle Name denn bedeuten?

Literale Zeichenketten sind nicht dazu da um Code auszukommentieren oder generell Kommentare zu schreiben.

Bei den Partikeln ist noch ein zweiter Fehler bei den Zuständigkeiten. Diesmal nicht zwischen Objekten, sondern zwischen Methoden: Die `draw()`-Methode sollte wirklich nur zeichnen und nicht auch gleichzeitig noch Partikelpositionen aktualisieren. Das gehört in die `update()`-Methode.

Wobei ich die Listenrepräsentation eines Partikels für unschön halte. Das gehört IMHO auch in eine Klasse.

Das der Inhalt von `particles` mit ``del`` gelöscht wird ist eine überflüssige Aktion. Das passiert kurz bevor das `Raindrop`-Exemplar insgesamt zur Speicherbereinigung freigegeben wird, weil es aus der Liste mit den Regentropfen entfernt wird.

Ein weiterer Fehler befindet sich in `gameloop()`. Eine `pygame.error`-Ausnahme wird mit einem `pygame.quit()` behandelt und danach geht aber alles weiter wie vorher, man kann aber keine Pygame-Funktionalität mehr verwenden wenn man `pygame.quit()` aufgerufen hat.

Der `pygame.quit()`-Aufruf in der `input()`-Methode sollte auch nicht sein, denn danach wird das am Programmende ja noch einmal aufgerufen. Einmal am Programmende reicht.

Ich lande dann ungefähr hier:

Code: Alles auswählen

#!/usr/bin/env python3
import random

import pygame

SIZE = (1024, 786)  # Set to `None` for fullscreen.

MTOP = 50  # TODO Needs a better name.
FPS = 60

WHITE = (255, 255, 255)
BLACK = (0, 0, 0)
BLUE = (0, 0, 255)
LIGHTGREY = (160, 160, 160)


class Switch:
    COLORS = [BLACK, BLUE]
    
    def __init__(self, position, state=False, callback=lambda state: None):
        self.state = state
        self.callback = callback
        self.rect = pygame.Rect(position, (50, 20))

    @property
    def color(self):
        return self.COLORS[self.state]

    def update(self, click_position):
        if click_position and self.rect.collidepoint(click_position):
            self.state = not self.state
            self.callback(self.state)

    def draw(self, screen):
        pygame.draw.rect(screen, self.color, self.rect)


class Hud:
    
    def __init__(self, width, height):
        self.width = width
        self.height = height
        self.rain_switch = Switch((120, 500))

    def update(self, click_position):
        self.rain_switch.update(click_position)

    def draw(self, screen):
        # pygame.draw.rect(screen, LIGHTGREY, [0, 0, self.width, self.height])
        self.rain_switch.draw(screen)


class Particle:
    
    def __init__(self, position, velocity):
        self.position = list(position)
        self.velocity = velocity
    
    def update(self):
        self.position[0] += self.velocity[0]
        self.position[1] += self.velocity[1]
    
    def draw(self, screen):
        pygame.draw.line(screen, BLUE, self.position, self.position, 1)


class Raindrop:
    
    def __init__(self, x, y, max_y):
        self.x = x
        self.y = y
        self.max_y = max_y
        self.particles = []
        # Tropfendurchmesser in mm * 2 = m/s
        # 1m = 100pxl
        diameter = 4 # eigentlich mm, wäre zu klein
        self.velocity = (
            random.uniform(-0.2, 0.2) / FPS * MTOP,
            (diameter * 2  + random.random()) / FPS * MTOP
        )
        self.particles = []

    def update(self):
        self.y += self.velocity[1]
        self.x += self.velocity[0]
        if self.y >= self.max_y:
            for particle in self.particles:
                particle.update()
            self.particles.append(
                Particle(
                    (self.x, self.max_y),
                    (
                        random.uniform(-50, 50) / FPS,
                        random.uniform(-50, 0) / FPS)
                    )
            )
            if self.y > self.max_y + 140:
                return False
        
        return True

    def draw(self, screen):
        pygame.draw.line(
            screen, BLUE, [self.x, self.y], [self.x, self.y + 4], 1
        )
        for particle in self.particles:
            particle.draw(screen)


class Rain:
    
    def __init__(self, size):
        self.width, self.height = size
        self.raindrops = []
        self.raining = False

    def switch(self, state):
        self.raining = state

    def update(self):
        if self.raining:
            for _ in range(3):
                self.raindrops.append(
                    Raindrop(random.randint(0, self.width), 0, self.height)
                )
        raindrops = []
        for raindrop in self.raindrops:
            if raindrop.update():
                raindrops.append(raindrop)
        self.raindrops = raindrops

    def draw(self, screen):
        for raindrop in self.raindrops:
            raindrop.draw(screen)


class Simulation:
    
    def __init__(self, screen):
        self.screen = screen
        self.running = True
        self.click_position = None
        self.clock = pygame.time.Clock()
        self.rain = Rain(self.screen.get_size())
        self.hud = Hud(300, self.screen.get_height())
        self.hud.rain_switch.callback = self.rain.switch

    def input(self):
        for event in pygame.event.get():
            if (
                event.type == pygame.KEYDOWN and event.key == pygame.K_ESCAPE
                or event.type == pygame.QUIT
            ):
                self.running = False
            elif event.type == pygame.MOUSEBUTTONUP:
                self.click_position = pygame.mouse.get_pos()

    def update(self):
        self.rain.update()
        self.hud.update(self.click_position)
        self.click_position = None

    def draw(self):
        self.screen.fill(WHITE)
        self.rain.draw(self.screen)
        self.hud.draw(self.screen)
        pygame.display.flip()

    def run(self):
        while self.running:
            self.clock.tick(FPS)
            self.input()
            self.update()
            self.draw()

   
def main():
    pygame.init()
    try:
        if SIZE:
            screen = pygame.display.set_mode(SIZE)
        else:
            screen = pygame.display.set_mode((0, 0), pygame.FULLSCREEN)
        pygame.display.set_caption('Rain')
        simulation = Simulation(screen)
        simulation.run()
    finally:
        pygame.quit()


if __name__ == '__main__':
    main()
“Most people find the concept of programming obvious, but the doing impossible.” — Alan J. Perlis
Antworten