[Pygame/Python]Anfänger will sich verbessern :D

Hier werden alle anderen GUI-Toolkits sowie Spezial-Toolkits wie Spiele-Engines behandelt.
Antworten
cruzz
User
Beiträge: 16
Registriert: Sonntag 5. Juli 2015, 14:55

Hallo ich habe mit heute Pygame geladen und habe auch gleich ein paar Stunden programmiert ... Hat Spaß gemacht :D ...
Aber da ich nur hiermi t" http://pygame.org/tags/ " gearbeitet habe fehlt da natürlich noch ganz viel...
Meine eigentliche Frage ist:
"Meincodeistderschlechtestedenesgibtweilesmeinerstesprogrammistundichhoffeihrkönntmirverbesserungstippsgebenodermirweiterhelfenweildanochganzvielebugssind"
:roll:

Also bitte einfach mal rein gucken und sagen was ich vereinfachen kann (ich bin mir sicher alles geht einfacher :K ). Ich hab es so programmiert wie es mit meinem Wissen ging.

und bitte jetzt nicht damit angekommen das ich ja so schlecht coden kann ... Das weiß ich selber :mrgreen:


https://mega.nz/#!x1EziZ7Y!gDcrpDaD4qH7 ... PsXMHHGb7w
(Steuerung: oben, unten, links, rechts, space(schießen), enter)
BlackJack

@cruzz: Ein Blick in den Style Guide for Python Code wäre nicht schlecht was die Quelltextformatierung angeht.

Auf Modulebene sollte nur Code stehen der Konstanten, Funktionen, und Klassen definiert. Das Hauptprogramm gehört in eine Funktion die üblicherweise `main()` genannt wird und durch folgendes Idiom aufgerufen wird wenn man das Modul als Programm ausführt:

Code: Alles auswählen

if __name__ == '__main__':
    main()
Dann ist das ganze viel zu lang für eine Funktion. Da steckt ja wirklich alles drin. Mindestens das Intro, das Menü, und das Spiel sollte man trennen. Aber da sind sicherlich noch mehr sinnvolle Funktionen möglich um nicht so einen Monstercode zu haben. Da verliert man auch sehr leicht mal den Überblick über die verwendeten Variablen. Oder über die nicht verwendeten. `coords` wird nämlich definiert, dann aber überhaupt nicht benutzt. Wenn man das in Funktionen auftrennt werden wahrscheinlich auch noch mehr Namen überflüssig die jetzt nur dazu da sind sich zu merken welcher Code eigentlich gerade ausgeführt werden muss und welcher nicht. Wobei ich es da komisch finde warum das auf zwei Namen aufgeteilt ist `zahler` (schlechter Name!) und `game`. Und `zahler` kann die Werte 0, 1, 5, und 4 annehmen. Warum gerade *die*? Warum nicht 2 oder 3? Das sieht sehr willkürlich aus und man muss den Code genau studieren um zu verstehen was die Werte bedeuten. Deswegen sind solche magischen Zahlenwerte schlecht und man sollte mindestens Konstanten dafür definieren.

Wenn man die drei Bereiche in eigene Funktionen herauszieht, dann werden die Kommentare welche die Bereiche im Quelltext ”trennen” überflüssig. Der Kommentar ``# Nachrichtenschleife`` steht meilenweit von einer Schleife entfernt. Weg damit.

`laufzeit` wird nur einmal auf 1 gesetzt und dann nie wieder verändert, der Name ist überflüssig und kann weg. Wenn man Wahrheitswerte meint, sollte man ausserdem nicht 0 und 1 nehmen sondern die Werte `False` und `True`. Dann weiss der Leser das es sich um einen Wahrheitswert handelt und nicht um eine Zahl.

Innerhalb einer Endlosschleife immer wieder die gleiche Funktion zu definieren macht keinen Sinn. Da `musik()` (kein guter Name für eine Funktion) keine Namen aus der Umgebung benötigt, kann das eine ganz normale Funktion auf Modulebene sein. Fünfmal hintereinander den gleichen Aufruf hinzuschreiben ist unschön, das kann man mit einer Schleife kürzer und flexibler lösen. Wobei das hier aber völlig unsinnig ist `play()` fünfmal aufzurufen denn auch beim ersten Aufruf startet die Musik bereits.

Das erste Bild für das Intro wird bei jedem durchlauf der Hauptschleife konvertiert. Immer und immer wieder. Das ist für das Intro schon überflüssig, aber das passiert auch beim Menü und im Spiel obwohl das Bild dort überhaupt gar nicht verwendet wird.

Namen durchnummerieren ist meistens ein Zeichen dafür das man eigentlich eine Datenstruktur verwenden möchte. Oft eine Liste. Das wäre bei den Introbildern beispielsweise angesagt. Wobei der Grundname `bild` schon viel zu generisch ist. Den Leser interessiert bei den ganzen `bild*`-Namen doch wofür dieses Bild steht. So muss man erst im Quelltext nach der Stelle wo der jeweilige Name definiert wird schauen und hoffen das der Dateiname aussagekräftiger ist, oder schauen wo und wie `bildx` verwendet wird und hoffen das man am Code leicht erkennt wofür dieses Bild verwendet wird.

Beim Intro ist die Kombination aus `blit()`, `delay()`, und `flip()` teilweise komisch. Es macht keinen Sinn eine Wartezeit auf meherere `delay()`-Aufrufe zu verteilen und das dann auch noch so zwischen `blit()` und `flip()` einzustreuen das man erst nachdenken muss welche `delay()`-Zeiten man zusammenrechnen muss um zu wissen welches Bild wie lange angezeigt wird.

Das ganze Intro ist blöd; da wird ein Ladebalken gezeigt der alles verzögert aber es wird in Wirklichkeit *gar nichts* **getan**. Ladebalken sind nicht da weil das irgendwie ”cool” ist einen zu haben, sondern um dem Benutzer bei *unvermeidbaren* Wartezeiten anzuzeigen das das Programm nicht abgestürzt ist sondern im Hintergrung etwas tut.

Die Behandlung von `K_ESCAPE` ist komisch. Es macht keinen Sinn ein `QUIT`-Ereignis zu `post()`\en wenn die nächste Aktion das beenden des Programms ist, weil die Ereignisschleife die das Ereignis verarbeiten könnte, dann ja überhaupt nicht mehr durchlaufen werden kann.

Beim Menü hängt `zahler2` (schlechter Name) immer von `zahler` ab, ist also überflüssig.

Es wird ingesamt zu oft die `flip()`-Funktion aufgerufen. Das macht man einmal am Ende des Bildaufbaus und nicht ständig zwischendurch. Für die Schüsse braucht man das bei dem Code leider weil die an der falschen Stelle im Code gezeichnet werden. Der Code gehört eigentlich nach dem Blitten des Hintergrunds. Dazu könnte man eine neues Flag einführen das gesetzt wird wenn die Leertaste gedrückt wurde.

So einige der ``if``\s könnte man durch ``elif``\s ersetzen.

Die Namen die für das Spiel selbst verwendet werden sind auch zum Grossteil schlecht gewählt. Ausserdem sollte man Abkürzungen vermeiden, weil das im Zweifelsfall beim Leser dann nur zu Ratespielchen führt. Was Du Dir bei `rel` gedacht hast, habe ich zum Beispiel nicht herausfinden können. Weil das irgendwie keinen Sinn macht was mit dieser Variablen gemacht wird, fällt mir da auch kein besserer Name ein.

Das skalieren eines Bildes sollte immer vom Originalbild passieren und nicht mit einem bereits skalierten Bild, weil bei jedem Skaliervorgang ein wenig Quelitätsverlust einhergeht, insbesondere beim hochskalieren weil da Pixel ”erfunden” werden müssen die es in den Daten gar nicht gibt. Es sieht auch nicht besonders gut aus einen Konstanten Wert für Höhe und Breite zu verwenden weil das Seitenverhältnis beim Skalieren dann nicht berücksichtigt wird. Das Flugzeug wird wenn man nach vorne fliegt immer flacher. Die Laserstrahlen passen nach dem skalieren auch nicht mehr zu Grösse und Position des Flugzeugs.

So wie die Tastatureingabe gelöst ist kann man nicht gleichzeitig das Flugzeug bewegen und schiessen. Du hast versucht das über die Tastenwiederholungsrate zu lösen, was man eigentlich über `KEYDOWN`- und `KEYUP`-Ereignisse machen würde.

Der `sys.stdin.readline()`-Aufruf wird niemals erreicht weil der hinter einer (echten) Endlosschleife steht.

Ich lande dann als Zwischenergebnis bei so etwas:

Code: Alles auswählen

#!/usr/bin/env python
import sys
from operator import add

import pygame


def load_image(filename):
    result = pygame.image.load(filename)
    if result.get_alpha():
        return result.convert_alpha()
    else:
        return result.convert()


def show_intro(screen):
    pygame.time.delay(5)
    for i, delay in enumerate([1000, 1500, 4000, 2000, 2000], 1):
        image = load_image('ladt{0}.jpg'.format(i))
        screen.blit(image, (0, 0))
        pygame.display.flip()
        pygame.time.delay(delay)
    

def show_menu(screen, clock, fps):
    pygame.mixer.music.load('menu.mp3')
    pygame.mixer.music.play()

    images = [load_image('menu{0}.jpg'.format(i)) for i in xrange(1, 3)]
    item_index = 0
    key = None
    while True:
        clock.tick(fps)
        for event in pygame.event.get():
            if (
                event.type == pygame.QUIT or
                event.type == pygame.KEYDOWN and event.key == pygame.K_ESCAPE
            ):
                pygame.quit()
                sys.exit()

            if event.type == pygame.KEYDOWN:
                if event.key in [pygame.K_UP, pygame.K_DOWN, pygame.K_RETURN]:
                    key = event.key
                else:
                    key = None

        if key == pygame.K_UP:
            item_index = 0
        elif key == pygame.K_DOWN:
            item_index = 1
        elif key == pygame.K_RETURN:
            if item_index == 0:
                return
            elif item_index == 1:
                pygame.quit()
                sys.exit()

        screen.blit(images[item_index], (0, 0))
        pygame.display.flip()


def play_game(screen, clock, fps):
    background_surface = load_image('gameground.jpg')
    ship_base_surface = load_image('ship.png')
    left_shot_surface = right_shot_surface = load_image('lazer.png')
    ship_coordinates = [300, 320]
    ship_left_gun_offset = [70, 110]
    ship_right_gun_offset = [325, 110]
    left_shot_coordinates = [0, 0]
    right_shot_coordinates = [0, 0]
    ship_size = [400, 190]
    ship_gun_active = False
    while True:
        clock.tick(fps)
        for event in pygame.event.get():
            if (
                event.type == pygame.QUIT or
                event.type == pygame.KEYDOWN and event.key == pygame.K_ESCAPE
            ):
                pygame.quit()
                sys.exit()

            elif event.type == pygame.KEYDOWN:
                if event.key == pygame.K_UP:
                    if ship_size[0] >= 0 and ship_size[1] >= 28:
                        ship_size[0] -= 20
                        ship_size[1] -= 20
                        ship_coordinates[1] -= 5

                elif event.key == pygame.K_DOWN:
                    if ship_size[0] <= 1000 and ship_size[1] <= 1000:
                        ship_size[0] += 20
                        ship_size[1] += 20
                        ship_coordinates[1] += 5

                elif event.key == pygame.K_RIGHT:
                    ship_coordinates[0] += 4

                elif event.key == pygame.K_LEFT:
                    ship_coordinates[0] -= 4

                elif event.key == pygame.K_SPACE:
                    ship_gun_active = True


        ship_surface = pygame.transform.smoothscale(
            ship_base_surface, ship_size
        )
        screen.blit(background_surface, (0, 0))
        if ship_gun_active:
            left_shot_coordinates = map(
                add, ship_coordinates, ship_left_gun_offset
            )
            right_shot_coordinates = map(
                add, ship_coordinates, ship_right_gun_offset
            )
            for _ in xrange(30):
                left_shot_coordinates[1] -= 10
                left_shot_coordinates[0] += 3
                right_shot_coordinates[1] -= 10
                right_shot_coordinates[0] -= 3
                screen.blit(left_shot_surface, left_shot_coordinates)
                screen.blit(right_shot_surface, right_shot_coordinates)
            ship_gun_active = False
        screen.blit(ship_surface, ship_coordinates)

        pygame.display.flip()


def main():
    pygame.init()
    screen = pygame.display.set_mode((1000, 563))
    pygame.display.set_caption('MeteorKiller')
    pygame.mouse.set_visible(1)
    pygame.key.set_repeat(30)
    clock = pygame.time.Clock()
    fps = 30

    #show_intro(screen)
    show_menu(screen, clock, fps)
    play_game(screen, clock, fps)


if __name__ == '__main__':
    main()
Die `play_game()`-Funktion ist noch zu lang und zu komplex. Statt der Listen mit den Koordinaten sollte man da mindestens mit `pygame.Rect`-Objekten arbeiten. Noch besser wäre es `pygame.Sprite` zu verwenden und Code in Methoden zu verschieben.
cruzz
User
Beiträge: 16
Registriert: Sonntag 5. Juli 2015, 14:55

Wow erstmal gaaanz großes lob an dich. Hast dich echt durch den ganzen codehaufen durchgekämpft.
DANKE DANKE DANKE! Das hilft mir richtig gut weiter!
Wenn nochmal Fragen auftauchen melde ich mich nochmal :)
cruzz
User
Beiträge: 16
Registriert: Sonntag 5. Juli 2015, 14:55

Hier mal ein kleines update:

Code: Alles auswählen

#!/usr/bin/env python
import sys
from operator import add
import pygame
import random
 
 
def load_image(filename):
    result = pygame.image.load(filename)
    if result.get_alpha():
        return result.convert_alpha()
    else:
        return result.convert()
 
 
def show_intro(screen):
    pygame.time.delay(5)
    for i, delay in enumerate([1000, 1500, 4000, 2000, 2000], 1):
        image = load_image('ladt{0}.jpg'.format(i))
        screen.blit(image, (0, 0))
        pygame.display.flip()
        pygame.time.delay(delay)
   
 
def show_menu(screen, clock, fps):
    pygame.mixer.music.load('menu.mp3')
    pygame.mixer.music.play()
 
    images = [load_image('menu{0}.jpg'.format(i)) for i in xrange(1, 3)]
    item_index = 0
    key = None
    while True:
        clock.tick(fps)
        for event in pygame.event.get():
            if (
                event.type == pygame.QUIT or
                event.type == pygame.KEYDOWN and event.key == pygame.K_ESCAPE
            ):
                pygame.quit()
                sys.exit()
 
            if event.type == pygame.KEYDOWN:
                if event.key in [pygame.K_UP, pygame.K_DOWN, pygame.K_RETURN]:
                    key = event.key
                else:
                    key = None
 
        if key == pygame.K_UP:
            item_index = 0
        elif key == pygame.K_DOWN:
            item_index = 1
        elif key == pygame.K_RETURN:
            if item_index == 0:
                return
            elif item_index == 1:
                pygame.quit()
                sys.exit()
                
 
        screen.blit(images[item_index], (0, 0))
        pygame.display.flip()
 
 
def play_game(screen, clock, fps):
    background_surface = load_image('gameground.jpg')
    ship_base_surface = load_image('ship.png')
    left_shot_surface = right_shot_surface = load_image('lazer.png')
    ship_coordinates = [300, 320]
    ship_left_gun_offset = [70, 110]
    ship_right_gun_offset = [325, 110]
    left_shot_coordinates = [0, 0]
    right_shot_coordinates = [0, 0]
    ship_size = [400, 190]
    ship_gun_active = False

    meteor_img = load_image('m.png')                                            #new
    random_x = random.randint(0, 1000)                                          #new
    random_y = random.randint(-20, -10)                                         #new
    meteor_coordinates = [random_x,random_y]                                    #new
    while True:
        clock.tick(fps)
        for event in pygame.event.get():
            if (
                event.type == pygame.QUIT or
                event.type == pygame.KEYDOWN and event.key == pygame.K_ESCAPE
            ):
                pygame.quit()
                sys.exit()
 
            elif event.type == pygame.KEYDOWN:
                if event.key == pygame.K_UP:                                    #transformationen entfert 
                        ship_coordinates[1] -= 5
 
                elif event.key == pygame.K_DOWN:
                        ship_coordinates[1] += 5
 
                elif event.key == pygame.K_RIGHT:
                    ship_coordinates[0] += 4
 
                elif event.key == pygame.K_LEFT:
                    ship_coordinates[0] -= 4
 
                elif event.key == pygame.K_SPACE:
                    ship_gun_active = True
 
 
        ship_surface = pygame.transform.smoothscale(
            ship_base_surface, ship_size
        )
        screen.blit(background_surface, (0, 0))
        if ship_gun_active:
            left_shot_coordinates = map(
                add, ship_coordinates, ship_left_gun_offset
            )
            right_shot_coordinates = map(
                add, ship_coordinates, ship_right_gun_offset
            )
            for _ in xrange(30):
                left_shot_coordinates[1] -= 10
                left_shot_coordinates[0] += 3
                right_shot_coordinates[1] -= 10
                right_shot_coordinates[0] -= 3
                screen.blit(left_shot_surface, left_shot_coordinates)
                screen.blit(right_shot_surface, right_shot_coordinates)
            ship_gun_active = False
        if meteor_coordinates[1] <= 500:                                        #new
            meteor_coordinates[1] += 5                                          #new
        else:                                                                   #new
            meteor_coordinates[0] = random.randint(0, 1000)                     #new hier ist mein derzeitiges Problem, habe ich irgendwas in deiner rechnung vergessen?
            meteor_coordinates[1] = random.randint(-20, -10)                    #new
        if left_shot_coordinates[1] == meteor_coordinates[1] or right_shot_coordinates[1] == meteor_coordinates[1]:                   #new
            meteor_coordinates[0] = random.randint(0, 1000)                     #new
            meteor_coordinates[1] = random.randint(-20, -10)                    #new
            print("works")                                                      #ignorieren
        screen.blit(ship_surface, ship_coordinates)                             
        screen.blit(meteor_img, meteor_coordinates)                             #new
        pygame.display.flip()
        
        

 

 
def main():
    pygame.init()
    screen = pygame.display.set_mode((1000, 563))
    pygame.display.set_caption('MeteorKiller')
    pygame.mouse.set_visible(1)
    pygame.key.set_repeat(30)
    clock = pygame.time.Clock()
    fps = 30
 
    #show_intro(screen)
    show_menu(screen, clock, fps)
    play_game(screen, clock, fps)
 
if __name__ == '__main__':
    main()
Habe alles markiert.
Das "random" muss ich nicht unbedingt zweimal haben oder?
BlackJack

@cruzz: Man könnte die Meteorkoordinaten so initialisieren das sie unten ausserhalb des `screen` liegen, dann wird gleich im ersten Durchlauf der Schleife eine neue Zufallsauswahl getroffen. Die ``if``\s und Bedingungen kann man so umschreiben das man nur einmal die `random.randint()`-Aufrufe schreiben muss.

Bei der Bedingung mit den Schusskoordinaten fehlt zum einen noch die Bedingung das auch gerade ein Schuss aktiv sein sollte. Dann bleiben da allerdings noch zwei Probleme: Der die linke obere Ecke wo der Meteor geblittet wird muss sich pixelgenau in einer vertikalen Linie mit der linken oberen Ecke einer der beiden zuletzt gezeichneten Laser-Segmente befinden. Das ist nicht nur schwierig sondern mit einer Wahrscheinlichkeit von 3:1 sogar gar nicht machbar weil sich das Flugzeug nur in Vier-Pixel-Schritten bewegen kann, der Meteor aber an jeder horizontalen Pixelposition herunterfallen kann. Um das besser zu lösen möchte man dann aber wirklich langsam `Sprite`-Objekte und `SpriteGroup`\s benutzen.

Code: Alles auswählen

def play_game(screen, clock, fps):
    background_surface = load_image('gameground.jpg')
    ship_surface = load_image('ship.png')
    ship_rect = ship_surface.get_rect().move(300, 320)
    ship_left_gun_offset = (70, 110)
    ship_right_gun_offset = (325, 110)
    left_shot_surface = right_shot_surface = load_image('lazer.png')
    ship_gun_active = False
    meteor_surface = load_image('m.png')
    meteor_rect = meteor_surface.get_rect().move(
        screen.get_rect().bottom, 0
    )

    while True:
        clock.tick(fps)
        for event in pygame.event.get():
            if (
                event.type == pygame.QUIT or
                event.type == pygame.KEYDOWN and event.key == pygame.K_ESCAPE
            ):
                pygame.quit()
                sys.exit()
 
            elif event.type == pygame.KEYDOWN:
                if event.key == pygame.K_UP:
                    ship_rect.top -= 5
 
                elif event.key == pygame.K_DOWN:
                    ship_rect.top += 5
 
                elif event.key == pygame.K_RIGHT:
                    ship_rect.left += 4
 
                elif event.key == pygame.K_LEFT:
                    ship_rect.left -= 4
 
                elif event.key == pygame.K_SPACE:
                    ship_gun_active = True
 
        screen.blit(background_surface, (0, 0))
        if ship_gun_active:
            left_shot_rect = ship_rect.move(ship_left_gun_offset)
            right_shot_rect = ship_rect.move(ship_right_gun_offset)
            for _ in xrange(30):
                left_shot_rect.move_ip(3, -10)
                right_shot_rect.move_ip(-3, -10)
                screen.blit(left_shot_surface, left_shot_rect)
                screen.blit(right_shot_surface, right_shot_rect)
            ship_gun_active = False
        if (
            meteor_rect.top > 500 or (
                ship_gun_active and meteor_rect.left in [
                    left_shot_rect.left, right_shot_rect.left
                ]
            )
        ):
            meteor_rect.topleft = (randint(0, 1000), randint(-20, -10))
        else:
            meteor_rect.top += 5
        screen.blit(meteor_surface, meteor_rect)
        screen.blit(ship_surface, ship_rect)
        pygame.display.flip()
Edit: Den Code von Koordinatenangaben auf `Rect`-Objekte umgestellt. Nun kann man anfangen das in `Sprite`\s zu verpacken.
Antworten