Au, toll, ein Programm verreissen.
Das ist aber auch echt schlimm.
Sehr viele Zeilen sind zu lang (>80 Zeichen). Dann fehlen Leerzeichen vor und nach Operatoren und nach Kommata zur besseren Lesbarkeit. Es gibt inkonsistente Einrückung z.B. in den Zeilen 337-338 und 348-349.
Schlechter Stil, der zu schlechter Wartbarkeit führt
Ein paar allgemeine Anmerkungen die für jedes Programm gelten.
Die vielen `from xxx import *` führen zu ganz vielen unbenutzten Namen, die den Namensraum des Moduls zumüllen. Hier sind mal alle Namen die Du importierst, aber nie benutzt:
setrecursionlimit, TypeType, mainloop, MacOS, prefix, maxint, Template, showwarning, LAST, TclVersion, copyright, Wm, RETRYCANCEL, strptime, NUMERIC, mktime, WARNING, DictProxyType, rstrip, splitfields, exc_value, ObjectType, OptionMenu, EXTENDED, OFF, maxunicode, askfloat, lowercase, Scrollbar, MethodType, ModuleType, image_names, atoi_error, capwords, stderr, ClassType, LambdaType, XRangeType, getfilesystemencoding, Y, lstrip, Misc, CodeType, Label, PAGES, index_error, interrupt_main, excepthook, LabelFrame, BaseWidget, image_types, index, YESNO, clock, UnicodeType, exc_type, version, Grid, callstats, Line, api_version, FLAT, CanvasText, joinfields, ABORTRETRYIGNORE, getboolean, start_new, atof_error, S, COMMAND, Bitmap, ACTIVE, path, FrameType, BASELINE, digits, tkinter, struct_time, setdlopenflags, rfind, Tcl, maketrans, LockType, askretrycancel, getrefcount, Group, setcheckinterval, replace, WRITABLE, FixTk, SEPARATOR, NSEW, Rectangle, rjust, executable, platform, rindex, NotImplementedType, getrecursionlimit, path_hooks, CHECKBUTTON, ANCHOR, RIDGE, PanedWindow, Tributton, SOLID, N, SEL_FIRST, TkVersion, meta_path, Dialog, UNITS, TupleType, join, Text, SW, OUTSIDE, expandtabs, IGNORE, YESNOCANCEL, error, localtime, wantobjects, SE, hexversion, ascii_lowercase, DictType, EXCEPTION, IntType, Polygon, DOTBOX, SINGLE, octdigits, getdefaultencoding, INSERT, BROWSE, punctuation, BuiltinFunctionType, SliceType, BEVEL, version_info, PIESLICE, FileType, sys, BuiltinMethodType, CANCEL, capitalize, Message, byteorder, BufferType, center, displayhook, stdin, getcheckinterval, exc_traceback, PROJECTING, ComplexType, getdouble, getdlopenflags, MULTIPLE, OKCANCEL, altzone, AtSelLast, ListType, get_ident, tzset, ascii_uppercase, TRUE, Widget, showerror, exec_prefix, Toplevel, call_tracing, CallWrapper, accept2dyear, exit_thread, rsplit, FIRST, ON, OK, ctime, allocate_lock, ABORT, GROOVE, Event, Scale, uppercase, BUTT, exc_clear, daylight, AtInsert, whitespace, FloatType, askinteger, atol_error, Spinbox, Arc, allocate, asctime, StringTypes, GeneratorType, ImageItem, VERTICAL, MITER, warnoptions, argv, swapcase, Place, EllipsisType, ERROR, EW, Pack, CHORD, Image, BitmapImage, AtSelFirst, RADIOBUTTON, exc_info, HIDDEN, os, DictionaryType, letters, askquestion, NoDefaultRoot, CHAR, path_importer_cache, BooleanType, HORIZONTAL, ljust, TclError, MOVETO, SUNKEN, READABLE, NE, builtin_module_names, NS, InstanceType, NW, RAISED, AtEnd, stdout, BooleanVar, settrace, setprofile, Variable, atoi, atol, atof, INFO, LongType, SEL, FALSE, SEL_LAST, getint, Radiobutton, tzname, Oval, Listbox, RETRY, ascii_letters, DoubleVar, Window, timezone, UnboundMethodType, CanvasItem, translate, QUESTION, ROUND, UNDERLINE, ARC, FunctionType, TracebackType, E, StringType, printable, Entry, hexdigits, CENTER, CASCADE, Studbutton, INSIDE, Menubutton, modules, StringVar, At, zfill, SCROLL, NoneType
Die ganzen `global` Deklarationen sorgen dafür, das die Funktionen nicht unabhängig voneinander sind und man sie damit auch nicht einzeln verstehen kann.
`time_w`, `time_s`, `time_count_w`, `time_count_s`, `mouse_lock`, `zug_50` und noch einige andere sind in Funktionen als ``global`` markiert aber auf Modulebene nicht an einen Wert gebunden. Die werden auf Modulebene erstellt wenn das erste mal so eine Funktion aufgerufen wird. Sollte das eine sein, die den Wert nur lesen will, dann gibt's einen `NameError`.
Ausnahmen nur mit ``except:`` behandeln ohne den konkreten Typ der Ausnahme anzugeben ist schlecht, weil man damit Ausnahmen unterdrückt, die einem wichtige Informationen bei der Fehlersuche bringen können. So geschehen zum Beispiel in `zeit_w()`.
Zuviele lokale Variablen und/oder Verzweigungen machen Funktionen unübersichtlich und sind ein Zeichen dafür, das man vielleicht zuviel in einer Funktion macht die besser auf mehrere Funktionen aufgeteilt werden sollte. Betrifft zum Beispiel `brett_init()` oder `start_zug()` und `zug_verlauf()`. Wenn es unübersichtlich wird, übersieht man auch leicht unbenutzte Variablen. In `brett_init()` werden die Variablen `mutex` und `zeile` beispielweise gar nicht benutzt. Das gleiche gilt für `mutex` in `setze_figur()` oder `i` und `inv` in `test_verlauf()`.
Anstelle von 0 und 1 sollte man `False` und `True` benutzen wenn man Wahrheitswerte meint, das macht das Programm lesbarer.
Und in Python muss man um einfache Bedingungen keine Klammern setzen. Das sollte man nur machen wenn es sonst nicht auf den ersten Blick ersichtlich wird, was zuerst geprüft wird.
Dokumentieren mit `docstrings` wo beschrieben wird was die Funktion tut, wofür die Parameter stehen und in etwa welchen Wertebereich sie haben und was zurückgegeben wird, falls es einen Rückgabewert gibt, ist immer eine gute Idee wenn man das Programm auch später noch verstehen möchte oder es andere verstehen sollen.
Konkrete Probleme
Bei `callback()` steht im Kommentar, das die Funktion nicht benutzt wird, aber falls sie das mal sollte, dann ist der Name `eingabe` undefiniert.
Den Kommentar beim `feld`, dass das 2D-Feld ineffizient ist, würde ich nicht unterschreiben. Dein Programm geht einfach nur an einigen Stellen ziemlich ineffizient damit um. (Das Wort "Performanz" gibt's im deutschen übrigens nicht.)
Du mischt Spiellogik mit GUI-Code. Das macht die Funktionen unübersichlicher weil zwei verschiedene Dinge in einer Funktion passieren. Sie werden damit auch automatisch länger und verwenden mehr Variablen. Ausserdem kann man die GUI dann nicht einfach austauschen, z.B. weil man ein anderes Toolkit benutzen möchte, oder einen Netzwerk-Modus einbauen möchte wo die GUI nicht (nur) lokal vorhanden ist.
Bei der Zeitmessung hast Du `copy'n'paste` Programmierung verwendet. Die beiden Funktionen unterscheiden sich nur minimal und sollten deshalb zu einer zusammengefasst werden, die sich nur durch entsprechende Parameter unterscheiden. Wenn Du die Funktion verändern möchtest, dann musst Du das dann nicht mehr an zwei Stellen parallel machen, wobei man schnell mal vergessen kann *beide* Funktionen anzupassen. Macht halt doppelt Arbeit.
In der Funktion `logging()` stecken IMHO zuviele Informationen in einer einfachen Zahl (`t`). Ob es ein Zug mit "normalen" Start- und Endpositionen oder eine Rochade ist, ob ein Zug mit oder ohne Schlagen vorliegt und ob Schach oder sogar Schachmatt gegeben ist und dann das ganze nochmal in der Variante, das ein Bauer umgewandelt wurde. Die ``elif``-Kaskade sieht auch wieder nach copy'n'paste aus.
"Magische" Argumente wie bei `feld_bedroht()`, wo (x,y)==(-1,-1) plötzlich was anderes als Koordinaten bedeuten sind schlecht. Warum gibt's für das Testen auf `Schach` keine eigene Funktion? Die Funktion ist auch viel zu lang. Die Tests für die einzelnen Figuren würde ich in eigene Funktionen auslagern.
Zum Quelltext von `rochade()` fehlen mir gerade die Worte. Ich denke den müsste ich 'ne Woche lang anschauen um ihn zu verstehen. Soo kompliziert ist mir eine Rochade gar nicht in Erinnerung.
In den `test_*()` Funktionen für die Figuren sind auch einige diskussionswürdige verschachtelte `if-else` und Schleifen die das Programm wahrscheinlich langsamer machen als es sein müsste.
`start_zug()` ist wieder reichlich lang und vermischt das freie Setzen von Figuren mit dem normalen Spiel.
Es gibt auch wieder copy'n'paste, das durch einfachere und lesbarere Konstrukte ersetzt werden kann:
Code: Alles auswählen
if (feld[v1][v2][1]=='B') and (int(feld[v1][v2][0])==(count%2)):
booli=test_bauer(v1,v2,n1,n2,0,count)
elif (feld[v1][v2][1]=='T') and (int(feld[v1][v2][0])==(count%2)):
booli=test_turm(v1,v2,n1,n2,0,count)
elif (feld[v1][v2][1]=='S') and (int(feld[v1][v2][0])==(count%2)):
booli=test_springer(v1,v2,n1,n2,0,count)
elif (feld[v1][v2][1]=='L') and (int(feld[v1][v2][0])==(count%2)):
booli=test_laufer(v1,v2,n1,n2,0,count)
elif (feld[v1][v2][1]=='D') and (int(feld[v1][v2][0])==(count%2)):
booli=test_dame(v1,v2,n1,n2,0,count)
elif (feld[v1][v2][1]=='K') and (int(feld[v1][v2][0])==(count%2)):
booli=test_konig(v1,v2,n1,n2,0,count)
else:
booli=0
Alles hängt von dem sich immer wiederholenden Teil nach dem ``and`` ab, d.h. diese Bedingung kann man *einmal* am Anfang überprüfen. Und die variablen Teile sind die Figur und die dazugehörige Testfunktion. Die kann man in ein Dictionary stecken und daraus dann auswählen:
Code: Alles auswählen
tests = { 'B': test_bauer, 'T': test_turm, 'S': test_springer,
'L': test_laufer, 'D': test_dame, 'K': test_konig }
figur = feld[v1][v2][1]
farbe = int(feld[v1][v2][0])
booli = 0
if farbe == count % 2:
booli = tests[figur](v1, v2, n1, n2, 0, count)
Ähnlicher Code findet sich in `mouse_click()`.
Noch etwas zur Ineffizienz: Das Programm berechnet viel zu oft alle möglichen Züge von Figuren um auf der einen Seite mögliche Züge und auf der anderen Seite Bedrohungen für die einzelnen Felder zu ermitteln. Effizienter wäre es die Zugmöglichkeiten *einmal* für jede Figur zu ermitteln und zum einen als Liste bei der entsprechenden Figur zu speichern und zum anderen bei jedem Feld zu vermerken, von welchen Figuren es betreten werden kann oder bedroht wird. Dann kann man sofort abfragen, wohin man eine Figur ziehen kann und bei einem Zug braucht man nur die Figuren neu zu berechnen die entweder auf das alte oder das neue Feld ziehen konnten.
Und das Spiel schreit geradezu nach objektorientierter Programmierung.