52
« am: 20. Dezember 2013, 20:25:41 »
Auch auf die Gefahr hin, dass einiges pampig rüber kommt …
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:
>> Welche Version genau nimmt du? Das ist nicht ersichtlich. 2.7x oder 3.1.2?
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?
>> Was möchtest du denn gerne kommentiert haben? Die Raum-Funktionen, bei den die Namen schon genau sagen, was passiert?
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.
>> Bisher gab es nie die Verwendung, für andere Bilder. Aber da gebe ich dir recht, das könnte man besser lösen.
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.
>> Gib mir bitte dazu mal ein Beispiel, das kann ich mir gerade gar nicht vorstellen.
>> Ah, jetzt verstehe ich, du nimmt die Verison 2.7x der Villa, da du das Plugin verwendest.
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.
>> Das ist Ansichtssache. Ich habe bei einer Firma gearbeitet, da wurde das konsequent so durchgezogen, sodass man gleich sieht, was für Typ die Funktion oder die Variable hat. Genau so könnte ich an der Form und Art der Einrückung meckern. Fakt ist, jeder macht es so, wie er es gewohnt ist und es ihm gefällt.
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.)
>> Siehe Kommentar davor. Schreibe es um und veröffentliche eine Version der Villa, die deinen Bedürfnissen und Wünschen entspricht.
Die Namen sind meistens nichtssagend. Wieso steht an Funktionsnamen der Rückgabewert dran, aber nicht, was sie eigentlich tut? ('v_Flashlight' - Flashlight was?)
>> Da fehlt mir jetzt der Zusammenhang, um dir die Frage richtig zu beantworten.
Englisch und Deutsch wird in den Variablen vermischt (gl_LichtBibliothek vs. b_LightOff)
>> Das stimmt, da war ich beim Überarbeiten wohl nicht ganz konsequent.
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.
>> Die Features waren schon im vorherigen SP so drin, die habe ich nur übernommen. Sie dienen dazu, kleinere Episoden mit den Villa zu erstellen und so viele Räume nicht begehbar zu machen, da sich der Spielen sonst die Hacken abläuft.
>> Was ist arkanes Verhalten?
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.
>> Schön für dich, aber der war bereits deaktiviert und ist nur eine option in den globalen Einstellungen von AGS.
>> Du bist der erste, der sich über diese kleine Spielerei überhaupt auslässt.
>> Ich denke, da lässt du nur deinen Frust raus, das hört man.
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.
>> Über das Flash-Plugin kann ich nichts sagen. Diese Dll stammt nicht von mir.
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?
>> Ich nehme nicht gerne an solchen Quiz’ teil, gebe dir aber recht.
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.
>> Und wieso willst du hier keinen Kommentar. Du scheinst mir nur die Beispiele zu bringen, die du nicht versteht und meckerst dann rum. Wenn, dann musst du schon konsequent sein und auch den if-Teil mit reinnehmen und nicht schreiben: … scheint relativ eindeutig dafür da zu sein …
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?
>> Deswegen sind diese Präfixe da. Damit man gleich sieht, dass es sich um eine GUI handelt. Oben hast du sie bemängelt und jetzt kommtes dir zugute, wenn du einmal das System verstanden hast. Ich hätte zwar auch GuiFlashlight schreiben können, aber das andere ist kürzer. (Mach einen Vorschlag für zukünftige oder Anfänger-geeignete Startepacks.)
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.
>> Jetzt mutmaßt du wieder und versuchst deshalb diese zeile nicht zu ändern und schaust was passiert?
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:
>> Warum sollte ich den Aufruf einer Funktion kommentieren? Normalerweise kommentiert man die Implementiertung einer Funktion, also die Stelle, an der der Code geschrieben ist. Aber gut, das kann man auch hier dahinter schreiben.
[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.
>> Wenn ich den Code selber und nicht die Funktion verstehen möchte und mich in AGS einarbeiten möchte, dann lese ich den Code.
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).
>> Du regst dich über eine Zeile auf, die vermutlich übersehen wurde. Wenn sie nichts bewirkt, dann lösche sie und schicke einen Hinweis ein.
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]
>> Da gebe ich dir recht, schreibe die Funktionen um sie wandern in das SP.
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.
>> Dann nimm dir mal die Zeit und mach dir die Mühe und vergleiche das 'alte' Villa-SP mit dem 'neuen' Villa-SP. Du stellst fest, dass beide nicht viel Unterschied haben.
>> Von welchen Fehlern, die keines der alten SP hatte, sprichtst du. Du lässt uns hier wieder am langen Arm verhungern. Reiche Beispiele ein, damit man sich dieser annehmen kann und die Fehler beseitigen kann. Einfach nur von alten und neuen Fehlern ohne konkretes zu nennen, sprechen, reicht mir nicht.
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...?)
>> Du hast keine Ahnung, oder?
>> Nicht, dass da etwas falsch rüberkommt, ich habe nichts gegen Kritik, aber nur Kritik und keine anderen oder eigenen Vorschläge zu machen prallen an mir einmal ab. Ich habe versucht sachlich da ran zu gehen, möchte mich aber entschuldigen, falls es falsch rüber kommen sollte.
Grüße
Rulaman
Beiträge anzeigen
