kreuvf / roll20-character-sheets

Kreuvf's fork of the Roll20 Character sheet templates. Do not work with my branches prefixed with "pre", I rebase and force-push them all the time. Check out the YouTube channel for videos on my pull requests.
https://www.youtube.com/user/Isurandil
MIT License
7 stars 6 forks source link

Codeverbesserungen: Templating/Buildsystem/SCSS #139

Open Triqueon opened 8 months ago

Triqueon commented 8 months ago

Nachdem @kreuvf dankenswerterweise den Code in leichter lesbare und zuordenbare Segmente aufgeteilt hat, hier eine Grundsatzfrage:

Ist es eine sinnvolle Idee, eventuell das makefile zu erweitern und ein paar Features einzubauen wie:

Vorteile:

Nachteile:

kreuvf commented 8 months ago

Ist es eine sinnvolle Idee, eventuell das makefile zu erweitern und ein paar Features einzubauen wie:

Erweiterung der Makefile sinnvoll: ja. Eine Funktion, die aus den fertigen HTML- und CSS-Dateien die Einzeldateien zurückstückelt wäre praktisch.

  • Templating: auch wenn das HTML deutlich besser lesbar ist nach der Aufteilung, sind immer noch viele Dinge extrem repetitiv, beispielsweise die diversen Dropdowns an den Talenten. Hier könnte eine Templatesprache Abhilfe schaffen indem ein entsprechender Block einmal definiert wird und dann nur noch gerufen wird.

Das wäre wirklich schön, aber: Es hätte auch einen entscheidenden Nachteil: Es vereinfacht die Erzeugung dieses Bloats enorm. Wenn die 330 Zauber, die der Bogen aktuell enthält, auf eine repeating section umgestellt werden können, fallen auf einen Schlag über 500 KiB der 1,8 MiB der HTML-Datei weg. Lässt sich dieses HTML bequem erzeugen, haben wir doch gar keinen Leidensdruck etwas daran zu verändern ^_^ Prinzipiell wäre das auch für Talente möglich und vielleicht sogar sinnvoll ...

(Komme übrigens mit regulären Ausdrücken ganz gut hin, solche Wiederholungen automatisiert zu bearbeiten ^^)

Roll20 schlägt PUG vor: https://wiki.roll20.net/Sheet_Author_Tips#Frameworks . Das erscheint mir hier unhilfreich, da man das komplette HTML überarbeiten müsste, weil die Syntax komplett anders ist. Persönlich habe ich in so Fällen gute Erfahrungen mit Twig (https://twig.symfony.com/) gemacht, weil sich dabei Templating Elemente in das existierende HTML einfügen lassen, ohne selbiges komplett ändern zu müssen.

  • SCSS/SASS: die CSS Selektoren sind teilweise echt hart umständlich zu lesen und prinzipiell komprimierbar (mit vielleicht der ein oder anderen minimalen Änderung an den Klassen im HTML. SCSS könnte hier helfen, selbige zu strukturieren und zu verkürzen

Beim CSS bin ich tatsächlich eher bei dir und habe daher auch schon vor Jahren geschaut, welche Präprozessoren es gibt, aber mir waren die Abhängigkeiten so groß. Wenn das nur so eine einfache C/C++-CLI-Anwendung wäre, aber das war mir alles zu fett. Bei PUG sehe ich es auch so und PHP ist nochmal eine eigene Hausnummer ^.^

Vorteile:

  • kürzerer, leichter zu pflegender Code mit weniger Duplikation (die auch inhärent fehleranfällig ist, sobald man irgendwas dran ändern will/muss)
  • Übersichtlicher beim Hinzufügen neuer Features

Nachteile:

  • 2 Zusätzliche Dependencies für den Buildprozess (php und npm)
  • Jemand muss die initiale Arbeit machen (dafür melde ich mich gerne freiwillig)
  • Höhere Voraussetzungen für Leute die Beiträge leisten wollen als "nur" HTML/CSS/vanilla JS zu können

Sehr gute Zusammenfassung. Ich würde allerdings PHP und npm eher als "zwei" (also in Anführungszeichen) Dependencies bezeichnen, weil die ja schon ordentlich fett sind und im Falle von npm auch sicherheitstechnisch eher am unteren Ende der Vertrauensskala agieren.

Die Nachteile überwiegen für mich daher aktuell eindeutig. Ich habe ja bei der Einführung der Makefile schon ewig gehadert und befürchte, dass das Helfer abschrecken könnte :sweat_smile:

Triqueon commented 8 months ago

Allgemein hab ich den Eindruck, von der Menge an Aktivität die es hier neben den Sachen von mir gab, dass die Aufteilung mit dem Makefile eher Helfer motiviert als abgeschreckt hat, und ich würde vermuten dass mehr Strukturierung da tendenziell mehr hilft.

Ich nehme ansonsten daraus folgendes mit:

SCSS kann man mittels eines standalone packages/CLI Anwendung auch ohne npm zu CSS kompilieren, siehe https://github.com/sass/dart-sass/releases/tag/1.72.0 . Das könnte man im Build-README erwähnen, Leute die eine Umgebung nutzen in der git und make gehen, sollten in der Lage sein, einen tar-Ball runterzuladen, zu entpacken und auszuführen, alternativ kann man auch ein optionales zusätzliches bash-script mitliefern dass das für einen macht (In das makefile einbauen würde ich das nicht, weil automatisierte Downloads von remotequellen ohne dass sie explizit gemacht werden immer ein bisschen fragwürdig sind, eventuell kann man im Makefile einen Test einbauen ob der relevante Befehl verfügbar ist und wenn nicht eine Info ausgeben wie hier weiter zu verfahren ist).

Das wäre für mein Verständnis eine akzeptabel schlanke Dependency (die noch dazu für einige Leute die mehr als nur dieses Projekt an Webentwicklung machen sowieso schon verfügbar ist), und ich wäre so frei irgendwann in näherer Zukunft einen entsprechenden PR zu stellen.

Gewichtigkeit von php/npm sehe ich nicht ganz so kritisch wie du, vor allem weil bei vielen Linuxdistributionen (und von denen gehe ich meistens aus, wenn wir schon make benutzen), php sowieso schon vorinstalliert ist, aber da will ich gar keine Diskussion anfangen, du bist hier Maintainer und entscheidest was geht.

Was die Repeating Sections angeht, werd ich mich damit bei Gelegenheit mal mehr beschäftigen. Mein erster Eindruck war allerdings dass es das recht schwierig machen würde, die jeweils assoziierten Attribute irgendwo fest vorzuhalten. Allgemein ist es, sofern möglich, natürlich wünschenswert, den Bloat nicht nur in den Buildfiles, sondern auch im fertigen HTML zu reduzieren, das soll durch das Templating gar nicht umgangen werden, und wenn das mit den Repeating-Sections sinnvoll geht, ist das natürlich die bessere Lösung.

Hauptsächlich störend für mich und Auslöser der Diskussion sind in der Tat die Talente und die paar Hundert Kopien des Attributdropdowns, das ich einfach liebend gerne durch eine Zeile der Art ` ersetzen würde. Vielleicht täte es, sofern das nicht über repeating sections geht (und zumindest für die Talente, bei denen diverse ja per Default aktiviert sind, sehe ich das kritisch), hier auch einfach ein kurzer sed oder awk Befehl im Makefile, das würde die Notwendigkeit von komplexeren Dependencies reduzieren.

Regexes sind in der Tat hilfreich, so hatte ich auch die Dropdowns für die Repräsentation in #136 eingebaut, lösen aber nicht das grundlegende Problem dass mein Hirn bei ein paar tausend Zeilen HTML sich einfach weigert den Überblick zu behalten was wie zusammengehört.

kreuvf commented 8 months ago

Allgemein hab ich den Eindruck, von der Menge an Aktivität die es hier neben den Sachen von mir gab, dass die Aufteilung mit dem Makefile eher Helfer motiviert als abgeschreckt hat, und ich würde vermuten dass mehr Strukturierung da tendenziell mehr hilft.

Möglich, kann aber auch einfach nur Zufall sein :)

SCSS kann man mittels eines standalone packages/CLI Anwendung auch ohne npm zu CSS kompilieren, siehe https://github.com/sass/dart-sass/releases/tag/1.72.0 . Das könnte man im Build-README erwähnen, Leute die eine Umgebung nutzen in der git und make gehen, sollten in der Lage sein, einen tar-Ball runterzuladen, zu entpacken und auszuführen, alternativ kann man auch ein optionales zusätzliches bash-script mitliefern dass das für einen macht (In das makefile einbauen würde ich das nicht, weil automatisierte Downloads von remotequellen ohne dass sie explizit gemacht werden immer ein bisschen fragwürdig sind, eventuell kann man im Makefile einen Test einbauen ob der relevante Befehl verfügbar ist und wenn nicht eine Info ausgeben wie hier weiter zu verfahren ist).

Das wäre für mein Verständnis eine akzeptabel schlanke Dependency (die noch dazu für einige Leute die mehr als nur dieses Projekt an Webentwicklung machen sowieso schon verfügbar ist), und ich wäre so frei irgendwann in näherer Zukunft einen entsprechenden PR zu stellen.

Mhh, interessant, das muss ich mir definitiv mal anschauen. ABER: Auch wenn die CSS-Datei eleganter geschrieben werden könnte, auch dort könnte man mit etwas Anstrengung viele Zeilen einsparen, auch ganz ohne CSS-Präprozessor.

Gewichtigkeit von php/npm sehe ich nicht ganz so kritisch wie du, vor allem weil bei vielen Linuxdistributionen (und von denen gehe ich meistens aus, wenn wir schon make benutzen), php sowieso schon vorinstalliert ist, aber da will ich gar keine Diskussion anfangen, du bist hier Maintainer und entscheidest was geht.

Okay, die Distributionen, die ich bisher genutzt habe, hatten weder PHP noch npm vorinstalliert, dafür in der Regel aber Python.

Was die Repeating Sections angeht, werd ich mich damit bei Gelegenheit mal mehr beschäftigen. Mein erster Eindruck war allerdings dass es das recht schwierig machen würde, die jeweils assoziierten Attribute irgendwo fest vorzuhalten. Allgemein ist es, sofern möglich, natürlich wünschenswert, den Bloat nicht nur in den Buildfiles, sondern auch im fertigen HTML zu reduzieren, das soll durch das Templating gar nicht umgangen werden, und wenn das mit den Repeating-Sections sinnvoll geht, ist das natürlich die bessere Lösung.

Nun, für die Initialisierung braucht man eine Funktion, die die Basistalente hinzufügen kann. Den Rest könnte der User dann unterstützt durch eine data-list und Script im Hintergrund, das bei bekannten Talenten die Attribute etc. pp. ausfüllt, selber machen. Keine endlosen Abhaklisten mehr und im Grunde könnte man dann sogar die Talentkategorieaufteilung wegfallen lassen - wie oft fragt jemand "Ist $talent unter 'Handwerk' oder 'Wissen'?", so könnte man einfach durchscrollen. Gut, etwas radikal und hier nicht Thema, aber so könnte man halt viel Wiederholung einsparen.

Hauptsächlich störend für mich und Auslöser der Diskussion sind in der Tat die Talente und die paar Hundert Kopien des Attributdropdowns, das ich einfach liebend gerne durch eine Zeile der Art ` ersetzen würde. Vielleicht täte es, sofern das nicht über repeating sections geht (und zumindest für die Talente, bei denen diverse ja per Default aktiviert sind, sehe ich das kritisch), hier auch einfach ein kurzer sed oder awk Befehl im Makefile, das würde die Notwendigkeit von komplexeren Dependencies reduzieren.

S. o. :D

Regexes sind in der Tat hilfreich, so hatte ich auch die Dropdowns für die Repräsentation in #136 eingebaut, lösen aber nicht das grundlegende Problem dass mein Hirn bei ein paar tausend Zeilen HTML sich einfach weigert den Überblick zu behalten was wie zusammengehört.

136 habe ich leider noch nicht angeschaut (s. Verweis auf akuten Zeitmangel gerade, schaffe es ja nicht mal eine ordentliche Antwort hierzu zu schreiben :cry: ).

kreuvf commented 1 month ago

Ich denke, dass SCSS eine gute Idee ist. Mir geistern aktuell noch ein, zwei größere Sachen im Kopf herum, aber danach würde ich dann gern ein CSS-Refactoring angehen. Das ist dann IMHO auch der ideale Zeitpunkt SCSS in den Workflow einzubauen. Das CSS benötigt nämlich mal eine Komplettüberarbeitung und dann lässt sich das gleich kombinieren. Danke nochmals für den Hinweis auf SCSS :blush:

Warum nicht schon vorher? Wenn ich Talente, Zauber, Rituale und Liturgien auf das "neue" System umgestellt habe, fallen nicht nur etliche Attribute weg, sondern auch viel CSS (und noch mehr HTML). In repeating sections sollten sich via Sheetworker Script neue Zeilen erstellen lassen, sodass es auch möglich sein sollte, zu migrieren bzw. initialisieren.