search

Jarr0d161 / TeamFightChaticts

TwitchChat spielt TFT über Chatbot
Apache License 2.0
18 stars 1 forks source link

Refactoring #3

Closed Bonifatius94 closed 2 years ago

Bonifatius94 commented 2 years ago

Änderungsvorschläge:

@DooomiT: Ich sehe, dass du auch einen Fork erstellt hast. Du kannst gerne bei mir mitmachen, wenn du willst ;)

ghost commented 2 years ago

@Bonifatius94 Gute vorschläge, bin dabei. Mein Python ist etwas eingerostet aber sollte ich hinbekommen. Ich denke aus dem Projekt kann man für die Streamer und Zuschauer echt was cooles machen :D Bei der Zahlenerkennung kann man ziemlich sicher einen low level classification algorithmus (regularized logical regression NN) verwenden der auf jedem Rechner laufen sollte.

Maihoernchen commented 2 years ago

@Bonifatius94 ich bin kompletter rookie dödel, aber ich kann soviel sagen: tesseract ist purer Schmerz. Wir benutzen das ganze um Daten von einer Seite auszulesen und die json die ausgespuckt wird ist unvollständig und unordentlich... Ich würde tesseract auf jeden Fall ersetzen

Bonifatius94 commented 2 years ago

@Maihoernchen Jaja das werd ich auch machen, wenn ich Zeit finde. Ich hab mir nicht umsonst das letzte Jahr lang harte KI Theorie im Informatik Master reingezogen. Aber ja, ist wirklich nicht so schwierig, denk ich mal. Wir wollen ja Druckschrift erkennen und keine extrem unleserliche Handschrift ^^

Jarr0d161 commented 2 years ago

Hallo zusammen, vielen Dank für den Input. Der Code ist eher eine Bastelei, da ich mir nicht sicher war, wie oft er verwendet wird. Wenn das Projekt auf solch Interesse stößt, freu ich mich natürlich, wenn wir es vorantreiben. Den Code säubern und strukturieren steht hier definitiv an erster Stelle. Ich hatte mir auch noch einige Gedanke zur Weiterentwicklung gemacht, auch hinsichtlich neuer Features (z.B. Interface für Zuschauer*Innen - damit das mtimachen leichter wird). Mir fehlt aktuell nur etwas die Zeit dafür, wenn hier jemand unterstützen will, bin ich sehr dankbar!

Da das Thema Tesseract hier diskutiert wurde. Ich hatte mich dafür entschieden, weil es schnell implementiert ist. Ich hatte anfangs auch im Hinterkopf, dass man die Champion Namen aus dem Shop lesen könnte, weshalb eine gut funktionierende Texterkennung für mich wichtig war. Aber klar, gern eine effizientere Lösung implementieren!

ghost commented 2 years ago

Champion Namen aus dem shop kannst du auch gut mit ner eigenen Lösung lesen. Ist nur Blockschrift. Das schafft auch eine vektorisiertet log reg wenn du den richtigen pixel bereich übergibst.
Das wird wohl unser job @Bonifatius94.

Danke fürs akzeptieren der Pr @Jarr0d161 . Wärs okay wenn ich eine cidc Pipelines implementiere und ein pytest force?

Bonifatius94 commented 2 years ago

Ich hab auf meinem Fork schon gute Fortschritte gemacht. Die Module sind größtenteils in den funktionalen Kontext abstrahiert, wie wir es eigentlich haben wollen. Schaus dir am besten mal an.

Ich hab eigentlich alles in meinen Commit Messages reingeschrieben, was anders ist (bis auf die erste).

@DooomiT: Wart mal lieber noch ein bisschen mit deinen Änderungen, sonst bekomm ich Merge Konflikte, dass es nimmer schön ist. Deine Übersetzungen können wir auch noch zu einem späteren Zeitpunkt einarbeiten.

ghost commented 2 years ago

Ich hab auf meinem Fork schon gute Fortschritte gemacht. Die Module sind größtenteils in den funktionalen Kontext abstrahiert, wie wir es eigentlich haben wollen. Schaus dir am besten mal an.

Ich hab eigentlich alles in meinen Commit Messages reingeschrieben, was anders ist (bis auf die erste).

@DooomiT: Wart mal lieber noch ein bisschen mit deinen Änderungen, sonst bekomm ich Merge Konflikte, dass es nimmer schön ist. Deine Übersetzungen können wir auch noch zu einem späteren Zeitpunkt einarbeiten.

wurde leider schon gemerged, ich kann mich um die konflikte kümmern 👍

ghost commented 2 years ago

@Bonifatius94 Wenn du ne PR aufmachst, könnte ich die Kommentare dort anfügren. Bis jetzt fin ich die PR ganz gut. Was das Refactoring angeht würde ich persönlich noch etwas weiter gehen.

Beispiele:

Um bessere contributions / weniger merge conflikte zu verursachen

Um die UX etwas zu verbessern:

ghost commented 2 years ago

Hallo zusammen, vielen Dank für den Input. Der Code ist eher eine Bastelei, da ich mir nicht sicher war, wie oft er verwendet wird. Wenn das Projekt auf solch Interesse stößt, freu ich mich natürlich, wenn wir es vorantreiben. Den Code säubern und strukturieren steht hier definitiv an erster Stelle. Ich hatte mir auch noch einige Gedanke zur Weiterentwicklung gemacht, auch hinsichtlich neuer Features (z.B. Interface für Zuschauer*Innen - damit das mtimachen leichter wird). Mir fehlt aktuell nur etwas die Zeit dafür, wenn hier jemand unterstützen will, bin ich sehr dankbar!

Hast hier echt was cooles angefangen, vielen dank an Dich und Maxim natürlich :) Wäre cool wenn du für die features ein paar stories anlegst, dann kann man sich stück für stück darum kümmern. Scheint hier echt schon eine coole community zu sein.

Da das Thema Tesseract hier diskutiert wurde. Ich hatte mich dafür entschieden, weil es schnell implementiert ist. Ich hatte anfangs auch im Hinterkopf, dass man die Champion Namen aus dem Shop lesen könnte, weshalb eine gut funktionierende Texterkennung für mich wichtig war. Aber klar, gern eine effizientere Lösung implementieren!

Cool das du da so offen bist, war ein guter Ansatz. Eine schnelle Lösung ist auch eine Lösung, optimieren kann man ja immer noch. Dann braucht man vielleicht auch kein high end setup mehr um das ganze zu verwenden :D

ghost commented 2 years ago

@Jarr0d161 Füge uns gerne als contributors zu dem repo hinzu, dann können wir hier etwas agiler arbeiten. Wenn dir das zu riskant ist forken wir halt weiter, skaliert nur nicht so gut 🙃

Bonifatius94 commented 2 years ago

@Bonifatius94 Wenn du ne PR aufmachst, könnte ich die Kommentare dort anfügren. Bis jetzt fin ich die PR ganz gut. Was das Refactoring angeht würde ich persönlich noch etwas weiter gehen.

Beispiele:

* Die _magic numbers_ in der init würde ich errechnen und nicht statisch setzen

* Ein Logger für die messages  / prints verwenden bevor man ihn selbst implementiert

* IRC Verbindung -> Connection class | function

* Die gesammte init des twitch bot etwas übersichtlicher gestalten

* nested if / else -> Cyclomatic Complexity gering halten

* Für die reihen / listen eine art game map class erstellen. die dann auch die relevanten Funktionen enthält.

Um bessere contributions / weniger merge conflikte zu verursachen

* Pylint / black pep8 conform (functions names -> sss_sss_sss etc.

* Unittesting framework & tdd

* Code of conduct

* Contribution guidelines

* Pipenv

  * scripte für start / dev / test
  * dependencies für prod / dev

Um die UX etwas zu verbessern:

* Config in gui einlagern

* Setup script für verschiedene OS

* Container execution oder exe build

* cicd -> versioned releases

Ich kümmere mich gerne um den ganzen contribution / cicd spaß, bei allem anderen bin ich auch gerne dabei. Habe schon einiges an Erfahrung in den Bereichen gesammelt.

Freut mich, wenn dir mein Refactoring bisher gefällt.

Deine ganzen Anmerkungen sind natürlich richtig und das hatte ich auch alles mehr oder weniger so vorgehabt. Ich bin halt nur praktisch nonstop von 15.00 bis um 02.00 (also 11 Stunden) drangesessen und irgendwann hat die Konzentration nachgelassen. Außerdem musste ich heut früh raus 😂

Aber bitte nix überstürzen. Ich hab das Ding kein einziges Mal angestartet. Man muss erstmal schauen, ob immer noch alles so funktioniert, wie es soll. Sind ja paar Tausend Zeilen Änderungen. Und die CoC, CI/CD, Unit Tests, Linter usw jetzt schon anzugehen, ist eher suboptimal, weil wir noch viel an den Schnittstellen ändern werden. Containerisierung brauchen wir auch nicht, weil es ne Python Desktop App werden soll. Multi-Plattform Setups sind erstmal nicht nötig; Python läuft eh praktisch identisch auf allen OS und der bisherige Code braucht sowieso Windows 😉

Ich glaub das womit du am leichtesten beitragen kannst, ohne dass es mega mit mir kollidiert, wäre das Ersetzen von Tesseract (pref mit TensorFlow). Ich würd außerdem das NN nicht so overengineeren, weil wir nur Zahlen erkennen müssen, also nicht mal Text und max. 2 Digits. Lieber ne schnelle, einfache, verständliche Lösung als ne Raketenwissenschaft 😉

Jarr0d161 commented 2 years ago

Ich habe euch beide eingeladen - Mi casa es su casa! Tobt euch gerne aus.

Da euch das Texterkennungsthema so beschäftigt und hier vorher schon viel diskutiert wurde. Tesseract ist für Druckschrift konzipiert, nicht für Handschrift - ergo die vortrainierten Modelle haben Druckschrift gesehen, weshalb es hier auch gut funktioniert. Wenn ihr wirklich nur die Zahlen auslesen wollt und das möglichst mit wenig rechenaufwand. Dann würde ich weder Tesseract noch TensorFlow (Neuronale Netze) anfassen, sondern mit Pixelmasken und Regressionsverfahren arbeiten (opencv, scikit-learn). Aber das hat @DooomiT auch schon erwähnt, also könnt ihr das gern umsetzen!

Bonifatius94 commented 2 years ago

Ich hab auf meinem Fork schon gute Fortschritte gemacht. Die Module sind größtenteils in den funktionalen Kontext abstrahiert, wie wir es eigentlich haben wollen. Schaus dir am besten mal an. Ich hab eigentlich alles in meinen Commit Messages reingeschrieben, was anders ist (bis auf die erste). @DooomiT: Wart mal lieber noch ein bisschen mit deinen Änderungen, sonst bekomm ich Merge Konflikte, dass es nimmer schön ist. Deine Übersetzungen können wir auch noch zu einem späteren Zeitpunkt einarbeiten.

wurde leider schon gemerged, ich kann mich um die konflikte kümmern 👍

Alles gut, ich merge gerade deine Sachen bei mir rein und werde es dann auf dem Main-Repo pushen. Ich hab mir außerdem die Freiheit erlaubt, deine Übersetzungen in mein settings.py zu überführen, weil es dort besser passt.

Bitte mach solange nix, damit wir wieder auf einem gemeinsamen Stand sind. Ich versuche es innerhalb der nächsten halben Stunde hochzuladen ;)

Bonifatius94 commented 2 years ago

Ich habe euch beide eingeladen - Mi casa es su casa! Tobt euch gerne aus.

Danke dir @Jarr0d161. Ja das bekommen wir schon hin. Meine Änderungen sind schon auf nem guten Weg :)

Da euch das Texterkennungsthema so beschäftigt und hier vorher schon viel diskutiert wurde. Tesseract ist für Druckschrift konzipiert, nicht für Handschrift - ergo die vortrainierten Modelle haben Druckschrift gesehen, weshalb es hier auch gut funktioniert. Wenn ihr wirklich nur die Zahlen auslesen wollt und das möglichst mit wenig rechenaufwand. Dann würde ich weder Tesseract noch TensorFlow (Neuronale Netze) anfassen, sondern mit Pixelmasken und Regressionsverfahren arbeiten (opencv, scikit-learn). Aber das hat @DooomiT auch schon erwähnt, also könnt ihr das gern umsetzen!

Alles klar, ja dann wäre das doch die richtige Aufgabe für dich @DooomiT, wenn du dich eh schon damit auskennst. Ich wollte halt TensorFlow benutzen und würde den Forward-Pass mit den vortrainierten Modellgewichten dann in NumPy durchführen (muss ja nicht wesentlich komplexer als MNIST mit paar Dot-Produkten und ReLU / Sigmoid Aktivierungen werden für den Anfang). Bei meinem Vorschlag bräuchten wir halt gar keine neuen Abhängigkeiten mehr in unser Projekt hinzufügen, weil NumPy haben wir ja schon. Aber macht wie ihr meint. Ich bin niemandem böse, wenn ihr ne bessere Idee habt. Dann lern ich vielleicht noch was dazu ^^

Bonifatius94 commented 2 years ago

Also die Änderungen von @DooomiT sind gemerged. Allerdings waren 3 Settings gar nicht in den Übersetzungseinstellungen enthalten. Die müsste man noch bei Gelegenheit nachtragen.

Bonifatius94 commented 2 years ago

@Jarr0d161: Was ist eigentlich die gewünschte Verhaltensweise deines Chatbots?

Meinem Verständnis nach wartet der Chatbot doch, bis von einem Kommando eine gewisse Anzahl an Messages eingegangen ist und führt dann das häufigste Kommando aus. Alle weiteren bis dahin eingegangenen Messages werden weggeworfen und dann geht das Spiel wieder von vorne los.

Soweit so gut, aber dann kommen ja noch die Flags ins Spiel wie zum Beispiel der Chaos-Modus. Da werden einfach alle Messages ungefiltert ausgeführt, wie sie gerade kommen, wobei ein identisches aufeinanderfolgendes Kommando ignoriert wird. Stimmt das?

Bonifatius94 commented 2 years ago

Ich bin mit meinen Refactorings sehr zufrieden. Der Code ist sehr modular, sodass man anfangen kann, Unit Tests zu schreiben.