@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:
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.