Dungeon Starter + JUnit: Allow spaces or special characters in paths

[tgrothe]

Problem

In

dungeon/src/contrib/crafting/Crafting.java und
game/src/core/components/DrawComponent.java

werden Resources (Assets) und Test-Resources jeweils nach bestimmten Dateien durchsucht. Das funktioniert in manchen Fällen nicht, wenn Umlaute im Pfad zum Projekt sind. Zudem funktioniert ein Test in

dungeon/test/dslToGame/TestDslFileLoader.java

nicht immer.

Dieser Pull-Request soll diese Dateien fixen, so dass das Dungeon auch mit Umlauten im Pfad funktioniert.

closes #1325

closes #1361

[tgrothe]

Ich denke, ich bin hier jetzt einen Schritt weiter ... Aber soll das so aussehen (die Dialoge funktionieren auch)?:

[cagix]

keine ahnung 🙃 erklär mal, was du machst?

[tgrothe]

keine ahnung 🙃 erklär mal, was du machst?

das Schlimme ist, ich weiß gerade nicht genau, woher diese Grafiken kommen...

ich habe die zwei Methoden loadAnimationsFromJar und loadAnimationsFromIDE durch eine Methode ersetzt:

loadAnimationAssets

Diese Methode soll alle Assets finden, die in pathToDirectory liegen, unabhängig davon, ob sich das directory in einer jar befindet oder im normalen Dateisystem.

[tgrothe]

@cagix Es funktioniert jetzt... Ich hab mir noch einmal genau durchgelesen, was die vorherige Methode von @AMatutat gemacht hatte...

Aber es ist noch nicht "schön"... Ich möchte die Helper-Clazz noch auflösen.

@AMatutat Weißt du zufällig, ob man in der Klasse Crafting irgendwie auf DrawComponent zugreifen könnte? Dann wäre alles in DrawComponent und an richtiger Stelle.

[AMatutat]

@AMatutat Weißt du zufällig, ob man in der Klasse Crafting irgendwie auf DrawComponent zugreifen könnte? Dann wäre alles in DrawComponent und an richtiger Stelle.

Ohne genau zu wissen was du machen willst: Ich glaube nicht und wenn doch, ziehst du dir eine ganz unschöne Verbindung die du nicht haben willst. Crafting und DrawComponent haben nix miteinander zu tun, und das sollte sie auch nicht

[AMatutat]

Ich denke, ich bin hier jetzt einen Schritt weiter ... Aber soll das so aussehen (die Dialoge funktionieren auch)?:

das sind die Missing-Textures, die werden immer dann verwendet, wenn eine Textur nicht gefunden werden kann. Im Kontext des PR-Titles würde ich mal vermuten, dass da was mit den Pfaden nicht passt(e).

[tgrothe]

Gute Nachricht, "run" und "java -jar build/libs/Starter.jar" funktionieren (mit/ohne Leerzeichen im Pfad).

Schlechte Nachricht, 51 Tests schlagen fehl... 😵‍💫

Wie jetzt weiter?

[cagix]

@AMatutat Weißt du zufällig, ob man in der Klasse Crafting irgendwie auf DrawComponent zugreifen könnte? Dann wäre alles in DrawComponent und an richtiger Stelle.

Ohne genau zu wissen was du machen willst: Ich glaube nicht und wenn doch, ziehst du dir eine ganz unschöne Verbindung die du nicht haben willst. Crafting und DrawComponent haben nix miteinander zu tun, und das sollte sie auch nicht

sehe ich genau so. hier muss analysiert werden, wo überall dateizugriffe passieren und warum und wie, und dann muss ein konzept her. ich sehe da eine zentrale helperklasse mit einer einfachen api, die das für die nutzenden klassen transparent erledigt. ich will nicht jedesmal mir wieder neu gedanken machen müssen, ob und wie ich nun unterscheide, in welcher situation ich grad stecke (jar, jar im jar, lokales fs, ...) - das muss einmal zentral angeboten werden. und ich will das auch nicht aus einer anwendungsklasse ziehen, also ich will nicht in Wuppie auf den code in DrawComponent zugreifen (müssen).

[tgrothe]

@cagix hmm, unabhängig davon, dass das genau dokumentiert werden muss... in welcher Utility-Klasse und Paket könnte ich das denn hinzufügen? filesystem-util/FilesystemUtil.java?

wir brauchen nur zwei Funktionen:

a) durchsuche ein Asset-Verzeichnis nach allen Unterdateien (das Verzeichnis kann auch nur / sein), gib eine String-Map zurück

b) durchsuche ein Asset-Verzeichnis nach allen Unterdateien mit einer bestimmten Dateiendung, gib eine String-Map zurück

( a0) bestimme, wie das Dungeon gestartet wurde (jar oder normal) )

[cagix]

@cagix hmm, unabhängig davon, dass das genau dokumentiert werden muss... in welcher Utility-Klasse und Paket könnte ich das denn hinzufügen? filesystem-util/FilesystemUtil.java?

klingt nach einem ort, wo man util-funktionen im zusammenhang mit dateien suchen könnte.

aber ist der name nicht doppeltgemoppelt? würde nicht utils/FileSystemUtils reichen?

wir brauchen nur zwei Funktionen:

a) durchsuche ein Asset-Verzeichnis nach allen Unterdateien (das Verzeichnis kann auch nur / sein), gib eine String-Map zurück

b) durchsuche ein Asset-Verzeichnis nach allen Unterdateien mit einer bestimmten Dateiendung, gib eine String-Map zurück

( a0) bestimme, wie das Dungeon gestartet wurde (jar oder normal) )

muss nicht auch eine bestimmte datei gefunden werden können? was ist mit dem laden der inhalte?

[tgrothe]

@cagix hmm, unabhängig davon, dass das genau dokumentiert werden muss... in welcher Utility-Klasse und Paket könnte ich das denn hinzufügen? filesystem-util/FilesystemUtil.java?

klingt nach einem ort, wo man util-funktionen im zusammenhang mit dateien suchen könnte.

aber ist der name nicht doppeltgemoppelt? würde nicht utils/FileSystemUtils reichen?

ja

wir brauchen nur zwei Funktionen:
a) durchsuche ein Asset-Verzeichnis nach allen Unterdateien (das Verzeichnis kann auch nur / sein), gib eine String-Map zurück
b) durchsuche ein Asset-Verzeichnis nach allen Unterdateien mit einer bestimmten Dateiendung, gib eine String-Map zurück
( a0) bestimme, wie das Dungeon gestartet wurde (jar oder normal) )

muss nicht auch eine bestimmte datei gefunden werden können? was ist mit dem laden der inhalte?

Den Mechanismus (das Darstellen eines Assets/das Laden einer Animation) würde ich unverändert lassen, zum einen funktioniert er, zum anderen müsste zu viel umgeschubst werden... und zum Dritten betrifft das nicht diesen PR...

aber einige Tests müssten ggf. angepasst werden, ich hab noch nicht herausgefunden, weshalb

[cagix]

Den Mechanismus (das Darstellen eines Assets/das Laden einer Animation) würde ich unverändert lassen, zum einen funktioniert er

that remains to be seen, doesn't it?

ich bin mir nicht sicher. auf den ersten blick stimme ich dir zu. aber auf den zweiten blick willst du strings zurück geben, die pfade o.ä. repräsentieren und die andere öffnen und einlesen sollen. muss ich dabei dann nicht auch wieder unterscheiden zw. jar, local fs, jar in jar?

und mir gefällt es überhaupt nicht, wenn statt vernünftiger und aussagekräftiger datentypen wieder string zum einsatz kommen soll. das war im alten dungeon immer schon ein problem, und ich würde gern wieder wie ein informatiker denken und arbeiten...

[tgrothe]

Sooo, bitte bei euch einmal schauen/testen, ob beides (run und .jar) mit Leerzeichen im Pfad bei euch funktioniert. Bei mir (Win 11) funktioniert es.

Dann muss ich die neue Klasse noch dokumentieren und die Tests anpassen. 😬

[tgrothe]

Den Mechanismus (das Darstellen eines Assets/das Laden einer Animation) würde ich unverändert lassen, zum einen funktioniert er

that remains to be seen, doesn't it?

ich bin mir nicht sicher. auf den ersten blick stimme ich dir zu. aber auf den zweiten blick willst du strings zurück geben, die pfade o.ä. repräsentieren und die andere öffnen und einlesen sollen. muss ich dabei dann nicht auch wieder unterscheiden zw. jar, local fs, jar in jar?

und mir gefällt es überhaupt nicht, wenn statt vernünftiger und aussagekräftiger datentypen wieder string zum einsatz kommen soll. das war im alten dungeon immer schon ein problem, und ich würde gern wieder wie ein informatiker denken und arbeiten...

Ich stimme dir zu. Aber String wäre gar nicht so übel, weil die LibGdx-Funktionen ja auch String haben möchten.

[cagix]

Den Mechanismus (das Darstellen eines Assets/das Laden einer Animation) würde ich unverändert lassen, zum einen funktioniert er

that remains to be seen, doesn't it?

ich bin mir nicht sicher. auf den ersten blick stimme ich dir zu. aber auf den zweiten blick willst du strings zurück geben, die pfade o.ä. repräsentieren und die andere öffnen und einlesen sollen. muss ich dabei dann nicht auch wieder unterscheiden zw. jar, local fs, jar in jar?

und mir gefällt es überhaupt nicht, wenn statt vernünftiger und aussagekräftiger datentypen wieder string zum einsatz kommen soll. das war im alten dungeon immer schon ein problem, und ich würde gern wieder wie ein informatiker denken und arbeiten...

Ich stimme dir zu. Aber String wäre gar nicht so übel, weil die LibGdx-Funktionen ja auch String haben möchten.

@tgrothe nur weil libgdx einen string haben möchte, bedeutet das noch lange nicht, dass wir dateien und pfade auch so anfängerhaft als string repräsentieren. lass uns endlich anfangen, wie informatiker zu handeln.

[tgrothe]

@cagix Der eine Test funktioniert nicht, wenn Leerzeichen/ümläüts im absoluten Pfad der Testumgebung sind (habe TODO dran geschrieben). Danke schon mal fürs Review.

[cagix]

kannst bitte nochmal zusammenfassen, welche fälle bzgl. start auftreten/zu beachten sind und warum? und welche klassen sind davon wie betroffen (matrix)? und was ist dein lösungsansatz (konzept)?

erst dann kann ich beurteilen, was dein code/pr löst und was nicht.

---

davon unabhängig ein paar erste fragen:

• ich sehe im code, dass du noch irgendwas mit junit und einem stacktrace machst?
• warum muss ich deiner filesystem-methode mal ein dummy-objekt mitgeben und mal nicht?
• warum ist der filehandler nicht static, also warum muss ich für den aufruf der helper-method erst ein objekt instantiieren?
• warum kriege ich ein hash zurück, was repräsentieren die keys, was genau stelle ich mir unter dem jeweiligen value vor (liste von strings)?
• warum kriege ich nicht was zurück, wo ich nicht erst mit verschachtelten iterationen nochmal drüber muss?
• warum kann ich der filesystem-hilfsmethode nicht aus der aufrufenden klasse eine methodenreferenz mitgeben, um mir die doppelte iteration zu sparen?

All review suggestions of this review are out of date now...