@betzi1985: Was so beim drüberschauen aufgefallen ist: Das Hauptprogramm ist nicht in einer Fuktion und damit gibt es globale Variablen was nicht sein sollte. Das verdeckt im Moment einen Fehler weil eine der Methoden in `Spiel` auf eine globale Variable zugreift.
Defaultwerte für Argumente die gar nicht Optional sind ist falsch.
Man muss auch nicht für jedes Attribut ein Argument haben. Und beim Aufrufen muss man auch nicht alle Argumente per Schlüsselwort angeben.
`alles_aufräumen()` sollte es nicht geben. Hier haben wir wieder das Thema irgendwas leeren/aufräumen, statt einfach das alte Objekt wegzuwerfen und mit einem neuen anzufangen. Es ist einfach fehleranfälliger zu versuchen etwas wieder in einen sauberen Anfangszustand zu versetzen, als einfach reinen Tisch zu machen und einfach mit einem neuen Anfangszustand zu beginnen.
`Spiel` übernimmt Aufgaben die nicht zur Spiellogik gehören. Die Benutzerinteraktion würde man davon trennen.
Nur eine der Methoden von `Spiel` bekommt ein Argument übergeben. Das ist ein riecht komisch, als wenn ein Programm mit globalen Variablen einfach in eine Klasse verschoben wurde. Damit sind dann zwar formal die globalen Variablen weg, aber das Problem, dass das alles ein bisschen unübersichtlich ist, und damit fehleranfälliger, bleibt ja. Ich denke zum Beispiel nicht das `karte` ein Attribut von der Klasse sein sollte. Das gehört zum Vorgang der den Zustand des Spiels in den nächsten Zustand überführt, aber nicht zum Zustand selbst.
`karte_gezogen` sieht auch komisch aus. Wird das tatsächlich über Spielzüge hinweg verwendet? Ich glaube nicht.
`erlaubter_zug()` prüft ob der Zug erlaubt ist *und* ändert den Zustand des Spiels, und zwar so das man die Methode in dem Fall nicht mehr als einmal aufrufen kann! Das ist überraschend und damit fehleranfällig.
Es gibt da auch Redundanzen. Man braucht eigenltich nicht die Informationen „Sonderfunktion Bube“ wenn es gleichzeitig ein Attribut `wunschfarbe` gibt. Wenn das einen anderen Wert als `None`/`Kartenfarbe.NONE` hat, dann ist das ja auch ein Zeichen das die „Sonderfunktion Bube“ aktiv ist.
`NONE` ist keine Kartenfarbe von Skatkarten. „Keine Farbe“ würde ich hier als `None` modellieren. Dann muss man die Farben die nicht `NONE` sind, nicht noch mal extra als Liste erstellen und herum reichen.
Die Sonderfunktionen sollten keine Kartenwertnamen haben, sondern einen Namen an dem man die Sonderfunktion erkennen kann. Also `AUSSETZEN`, `KARTEN_ZIEHEN`, und so weiter.
`Spieler.spielernummer` sollte nur `Spieler.nummer` heissen. Das es die Nummer eines Spielers ist, wird ja schon durch den Datentyp `Spieler` klar. Wobei das Attribut komplett weg kann. Genau wie das `aktiv`-Attribut. Wenn es nur zwei Spieler gibt, sollte die `Spiel`-Klasse einfach zwei Attribute `aktiver_spieler` und `inaktivier_spieler` haben. Das macht den Code an vielen Stellen einfacher. Da wird dann auch einiges an Code verschwinden wo in zwei Zweigen immer fast das gleiche steht, und sich im Grunde nur ein Indexwert für den jeweiligen Spieler unterscheidet.
`Spieler.lege_Karte()` gibt immer noch ein leeres Tupel oder eine Karte zurück. Die Methode wird aber auch nirgends aufgerufen!
In `loesche_console()` wird eine anonyme Funktion an einen Namen gebunden und dann auch gleich in der nächsten Zeile aufgerufen. Das macht beides keinen Sinn. *Und* der Inhalt der Funktion ist dann auch noch fragwürdig. Es werden Farben per ANSI-Escapes ausgegeben, dann kann man auch die Konsole mit der entsprechenden ANSI-Escape-Sequenz löschen und muss dafür dann nicht plötzlich nach Betriebssystem unterschiedliche externe Programme aufrufen.
`random.shuffle()` mehrfach hintereinander aufzurufen ist Zeitverschwendung. Das wird dadurch nicht ”zufälliger” als wenn man nur einmal mischt.
Die Schreibweise von Namen entspricht noch nicht überall den Konventionen. Und auch inhaltlich sind die Namen manchmal nicht gut bis falsch. `Spieler.get_Karte()` beispielsweise: Methoden die mit `get_*` anfangen holen einen Wert *von* dem Objekt auf dem sie Aufgerufen werden, und übergeben da keinen Wert der dann dem internen Zustand *hinzugefügt* wird. Das sollte `fuege_karte_hinzu()` heisssen. Ich würde an der Stelle auch ganz gerne für Englisch plädieren, weil dort einiges weniger sperrig klingt (`add_card()`) und weil im Englischen der Plural nur sehr selten wie die Einzahl eines Begriffs geschrieben wird. Im Englischen ``for player in players:`` im Deutschen ``for spieler in spieler:`` → Problem! Und das tritt halt recht häufig auf.
Man macht keine Vergleiche mit literalen Wahrheitswerten. Bei dem Vergleich kommt doch nur wieder ein Wahrheitswert bei heraus. Entweder der, den man sowieso schon hatte; dann kann man den auch gleich nehmen. Oder das Gegenteil davon; dafür gibt es ``not``.
Dieses Zusammenstückeln von Zeichenketten, teilweise mit `str()` für Werte, ist unübersichtlich und eher BASIC denn Python. In Python gibt es dafür Zeichenkettenformatierung mit der `format()`-Methode oder f-Zeichenkettenliteralen.
`neues_spiel` kann man sich sparen. Einfach eine ”Endlosschleife” (``while True:``), die an entsprechender Stelle dann mit ``break`` verlassen wird.
Ich hatte ja schon geschrieben das die Karten keine einfachen Tupel sein sollten, denn das die Zahlen 0 und 1 ab und zu mal Kartenwert oder -farbe bedeuten ist total undurchsichtig.
Zeilenlängen: Ich bin da ja konvervativ und bleibe bei maximal 80 Zeichen pro Zeile wo der Style Guide mittlerweile bis 120 geht, aber 264 Zeichen ist echt zu lang.
Python kann zwar mit Umlauten in Namen umgehen, aber nicht alle anderen Werkzeuge die mit Python-Quelltext oder allgemein Quelltext arbeiten. Das würde ich deshalb auf ASCII beschränken.
Sooo das reicht erst mal.