Ich muss verzeihen, dass ich gerade etwas frustriert schreibe, aber ich habe grade ein eigentlich kleines Problem mit einem der MMM-Starterpacks gelöst und musste mir dort den Code einmal zu gemüte führen. Die Starterpacks sind furchtbar geschrieben, der Code ist unübersichtlich und alles andere als selbsterklärend, was es dank einer nicht vorhandenen Dokumentation schwer macht, einige Aufgaben zu lösen. Ich kann nun nur über das Villa-Starterpack reden, aber hier mal so meine Eindrücke und größten Beschwerden:
Generell:
- Die Autoren haben es fertig gebracht auf über 17.500(!) Zeilen Code grade mal 700 Zeilen Kommentare zu schreiben, davon die meisten reine Trenner. Das heißt, über 95% des Quellcodes sind unkommentiert. Wenn ich großzügigerweise davon ausgehe, dass 75% der Skripts den immer gleich aussehenden Raumskripts entsprechen, bleibt es trotzdem bei grade mal 17% kommentierten Zeilen in GlobalScripts und Modulen. Und da soll man sich zurecht finden?
- Das Starterpack macht stille Vorraussetzungen. Die Taschenlampen-Inventory-Items werden in den Raumskripts benutzt, können nicht durch andere Objekte überschrieben werden ohne dass es irgendwo gesagt wird.
- Das Konzept von Modulen wurde ein bisschen arg missbraucht, da sie in den Skripts exzessiv genutzt werden. Während ich das bei einigen wie playerExtends(sic) ja noch einsehe, frag ich mich ernsthaft, wieso das für die meisten Spiele eher nicht benötigte Flashlight-Plugin so tief verwoben im Quelltext drin steckt. (siehe unten) Das macht es nur schwerer, den Code zu entfernen, wenn ich ihn nicht brauche.
Namen:
- Die Namenskonvention ist furchtbar. 'b_On', 'l_StairsUsed', 'v_Flashlight' - es liest sich nicht nur furchtbar, sondern enthält auch unnötige Informationen. Mischung von unterstrich_als_trennzeichen vs. camelCase im selben Namen? Geht's hier um Ordnung bei der Autovervollständigung? Da gibt's bestimmt auch schönere Varianten.
- Wieso muss von allen Sachen, die man als Präfix nehmen könnte ausgerechnet der Datentyp explizit dran stehen, macht das überhaupt irgendein größeres Projekt ebenfalls so? (Gut, AGS macht es selbst bei Charakteren, Objekten, usw., aber da kann man es noch damit rechtfertigen, dass damit Namenskollisionen vermieden werden. Bei Funktionen ist es definitiv unnötig.)
- Die Namen sind meistens nichtssagend. Wieso steht an Funktionsnamen der Rückgabewert dran, aber nicht, was sie eigentlich tut? ('v_Flashlight' - Flashlight was?)
- Englisch und Deutsch wird in den Variablen vermischt (gl_LichtBibliothek vs. b_LightOff)
Features (im Sinne von Flashlight, konfigurierbaren Räumen...)
- Wem sollen denn all diese zusätzlichen Funktionen eigentlich was bringen? Anfänger? Die werden eher von den Unmengen an Code erschlagen, den sie nicht verstehen. Fortgeschrittene? Die wollen sich genauso wenig durch den Spaghetticode wühlen sich in einer Tiefensuche verlieren, weil sie keine Ahnung haben, as eine bestimmte Funktion tut und auch der Code dieser Funktion nur sehr arkanes Verhalten darstellt. Ein einzeiliger Kommentar über der Funktion, der so ungefähr beschreibt was sie tut, wär schon hilfreich.
- Es hat mich 15 Minuten gebraucht, den Debug-Modus so einzustellen, wie ich es von jedem anderen AGS-Spiel gewohnt bin. NIEMAND AUSSER DEM ENTWICKLER BRAUCHT DEN DEBUG-MODUS und niemand braucht ihn in einem Spiel-Release! Ich will keine Cheats per default, keine Datei, die ihn nach Release wieder aktiviert. Wenn ich es will, kann ich es in 10 Minuten selbst einbauen oder eine Zeile auskommentieren. Und selbst wenn ich es will, wenn ich den Debug-Mode explizit einschalte im Editor, sollen die Hotkeys funktionieren, ohne dass ich ne extra Datei anlege. Das Standardverhalten ist unglaublich sinnlos und fehleranfällig.
- Das Flashlight-Plugin hat ein unüberschaubares Innenleben, das so einfache Aufgaben wie das Ein- und Ausschalten des Lichts in einer Cutscene unheimlich schwer macht.
Das letzte Beispiel führe ich mal etwas aus, weil es genau der Grund ist, wieso ich mich mit dem Starterpack näher beschäftigen musste.
Wenn man im Keller der Edisons möchte, dass ein Charakter automatisch das Licht einschaltet, wird man unweigerlich die OnRoomLoad-Funktion anschauen und stößt über dieses Fragment:
[ags] /* Jetzt kommt die Taschenlampe */
if ( gi_LichtKeller == 0 || gi_LichtKeller == 2 )
{
gFlashlight.Visible = true;
v_Flashlight(false);
Flashlight.Enabled = true;
if ( player.InventoryQuantity[iTaschenlampeOff.ID] )
{
v_Flashlight(true);
Flashlight.Enabled = true;
player.LoseInventory(iTaschenlampeOff);
PlaySound(60);
player.AddInventory(iTaschenlampeOn);
oLichtschalter.Clickable = true;
}
}[/ags]
Quizfrage: Welche dieser Zeilen ist diejenige, die dafür sorgt, dass der Raum schwarz wird?
Gut, den if-Zweig können wir ignorieren, der scheint relativ eindeutig dafür da zu sein, dass der Spieler die Taschenlampe automatisch einschaltet, wenn er den Raum betritt (warum auch immer) und ist darüber hinaus etwas redundant.
Bleiben also noch diese Zeilen:
[ags]gFlashlight.Visible = true;
v_Flashlight(false);
Flashlight.Enabled = true;[/ags]
Die erste sieht doch nach etwas aus, oder? Wenn die Funktionalität über ein GUI erreicht wird, kann man dieses einfach abschalten, richtig?
Falsch. Zumindest bringt ein Ausschalten dieser Zeile keine Änderung.
Was bleibt noch? Flashlight.Enabled = true klingt danach, dass die Lampe eingeschaltet wird. Wahrscheinlich wird diese Änderung irgendwo global überschrieben und die Zeile macht genauso wenig Sinn wie die erste, also stellen wir das erstmal zurück.
Bleibt noch v_Flashlight. Wie oben schon angedeutet, sagt eine Zeile wie v_Flashlight(true) nicht viel mehr aus, als dass es eine Void-Funktion ist, die wohl irgendwas mit der Taschenlampe zu tun hat. Da die Zeile nicht kommentiert ist, bleibt uns also auch dort nicht viel mehr, als einfach mal reinzuschauen:
[ags]void v_Flashlight(bool b_On)
{
if ( b_On )
{
// Taschenlampenlichtkegel
FlashSprite = DynamicSprite.CreateFromExistingSprite(169, true);
Flashlight.Transparency = 1;
}
else
{
// leeres Sprite (Dunkelheit)
FlashSprite = DynamicSprite.CreateFromExistingSprite(26, true);
Flashlight.Transparency = 2;
}
Flashlight.SetGUIMode(gFlashlight);
Flashlight.SetBeamSprite(FlashSprite);
Flashlight.SetFollowMouse();
}[/ags]
Eine Funktion mit 19 Zeilen und 2 Kommentaren (die nur die Spritenummern entschlüsseln), von den Zeilen 7 Aufrufe an das Modul, die wiederum untersucht werden müssten. Wenn ich wissen will, was Flashlight.Transparency macht, muss ich den gesamten Modulcode darauf untersuchen, wo diese Variable benutzt wird und eventuell dort nochmal Funktionen untersuchen. Das ist kein guter Ausgang dafür, wenn ich verstehen will, was dieser Code macht.
Die Lösung für das Problem liegt übrigens nicht in diesem Code. Tatsächlich ist diese Zeile das einzige, was relevant ist:
[ags]Flashlight.Enabled = true;[/ags]
Entgegen meiner ursprünglichen Vermutung steuert diese Variable nämlich global, ob die Flashlight-FUNKTIONALITÄT eingeschaltet ist, nicht ob die Taschenlampe gerade an ist. Ein Flashlight.Enabled = false; schaltet das Licht an, sorgt auch automatisch dafür, dass das GUI abgeschaltet wird (warum es dann nochmal da steht, versteh ich nicht).
Hier mal ein Vorschlag, wie es aussehen könnte
[ags] /* Jetzt kommt die Taschenlampe */
if ( lightBasement == eOff || lightBasement == eDisabled )
{
Flashlight.lightState = eOff; // Raumbeleuchtung aus
Flashlight.flashlightState = eOff; // Taschenlampe aus
if ( player.InventoryQuantity[iTaschenlampeOff.ID] )
{
Flashlight.flashlightState = eOn; // Taschenlampe an
player.LoseInventory(iTaschenlampeOff);
PlaySound(60);
player.AddInventory(iTaschenlampeOn);
oLichtschalter.Clickable = true;
}
}[/ags]
Deutlich lesbarer, oder? Vielleicht sogar ein paar neue Funktionen, die dann auch benutzt werden:
[ags]Flashlight.LightOff();
Flashlight.FlashlightOn();[/ags]
Was ich viel schlimmer finde: KEINES der alten Starterpacks hatte diese Probleme. Das Globalskript war ein paar tausend Zeilen lang, in denen hat man sich schnell zurecht gefunden, das meiste musste man gar nicht anrühren. Die Funktionen hatten alle eine klare Aufgabe, es war ein minimaler Ansatz. Gut, einige Funktionen waren noch überflüssigerweise aus MMD-Zeiten über, aber die konnte man ja ignorieren. Die paar GS-Funktionen, die benutzt wurden wie UsedAction, MovePlayer, InitDoor, wurden ständig benutzt und man wusste irgendwann, was sie tun. Dieser haufenweise undokumentierte Code im Villastarterpack macht einem einfach nur das Leben schwer, insbesondere, wenn er ständig genutzt wird.
Ich möchte die Starterpack-Entwickler doch noch einmal dringend bitten, ihren Code zu kommentieren, zu überarbeiten und ordentliche Namen zu vergeben, zuallermindest mal in den von ihnen neu eingeführen Codes. Ausdrucksstarke Namen, mit einer sinnvollen Angabe davon, wofür Variablen und Funktionen gut sind und was sie tun. Überlegen, was Funktionalität ist, die der Spieler vielleicht braucht. Und sinnvolles Default-Verhalten wählen (lasst das Licht doch am Anfang einfach überall an...?)
Bughunter
