akrzyz / nerthusaddon Goto Github PK

Czy w każdym pliku potrzebne są try{}catch(){}? Jak będziemy mieli w danym pliku 100% code coverage można by pomyśleć nad usunięciem ich, jako że nie powinno być żadnych problemów. A jeżeli już to chyba nie takich, które by wywaliły error który by catch złapał.

Jako że trudno mi się skontaktować gdzie indziej, napisałem tutaj.
Będąc osobą, którą stworzyła cały dodatek na NI oraz regularnie wprowadza do niego wymagane poprawki, chciałbym prosić o możliwość zostania maintainerem głównego dodatku. Inne strony (to jest rada oraz administracja) nie widzą przeciwskazań do tego. Zgoda @akrzyz jest jedyną, którą potrzebuje.

Już nie raz pomagałem osobom ze świata w ich problemach technicznych. Do tego stworzyłem w pełni* działającą wersję dodatku na NI. Co więcej, stworzyłem również cały dodatek dla narratorów, którego to teraz przepisuję na webpacka, z racji, że przerósł moje oczekiwania co do ilości funkcji. Myślę, że lepszej i bardziej zaangażowanej w dodatek osoby nie można znaleźć na tą funkcję.

Jako maintainer pierwszymi moimi krokami będzie wprowadzenie do głównego dodatku wszystkich zmian z #35 , które to działają prawie bez zarzutu na obu interfejsach, a w szczególności na SI, co do którego działa to lepiej i zawiera dodatkowe funkcje (jak ukrywanie npców zależne od mapy, jeżeli się id mapy wpisze w komendę czy też ukrywanie potworków z mapy jeżeli narrator odpowienio to ustawi, a jednocześnie gracz ma włączone to w ustawieniach), a także dodanie obsługi niektórych rzeczy, których nie mogłem dodać mając na uwadze, że są jednocześnie w użyciu dwie wersje dodatku.

Dodatkowo wszelkie znalezione błędy będą czym prędzej poprawiane, tak jak ostatnio znaleziony przeze mnie błąd, który uniemożliwia ustawianie zwojów czerwonego smoka, mając zainstalowaną obecną wersję dodatku globalnego

Końcowym krokiem jest cały aspekt nowego dodatku, który to zapewne będzie po części przepisaniem obecnego systemu (najlepiej na coś pokroju webpacka) wraz z połączeniem go z serwerem, który tworzy @mikkielt, z którym się kontaktuje co do tego. To jednak zaplanowane jest na przyszłość.

Więcej informacji: https://www.margonem.pl/?task=forum&show=posts&id=502252

Wyrażenie które wygląda w ten sposób:

nerthus.loadQueue = []

Jest parsowana przez dodatek na:

(function() {
    [native code]
})

Przez co źle się wczytuje i uniemożliwia poprawne odczytanie dodatku z localstorage:

Narratorzy i radni mają obecnie dostęp do własnego, ukrytego repozytorium w którym opisane są wątki wszystkich postaci. Można by stworzyć zaraz obok kolejne repozytorium, widoczne dla wszystkich, w których będą zawarte NPC i mapy które narratorzy i radni chcą podmieniać.

Obecnie rozwiązanie jest na gitlabie (z racji darmowych repozytoriów prywatnych) więc repozytorium z mapami i NPC również by na nim było (zaraz obok by narratorzy nie musieli logować się na oddzielną stronę)

Plusy:

• Narratorzy nie muszą czekać kilka dni na dodanie jednego NPCa czy mapy na stałe.
• Jeżeli do dodatku dla narratorów dodamy możliwość wyświetlenia stworzonego pliku json z mapy na której jesteśmy to jest to najbliższe rozwiązanie postawionemu serwerowi który obsługuje jakąś bazę danych z npcami (a przy tym darmowe).

Mogę tym się zająć, potrzebuję tylko zielonego światełka.

npc created with addon has no collision and you can stand on them .

Move addon to different CDN as rawgit is not working anymore. This require changes in NN_start.js and global addon update.
Jsdeliver is an option to replace rawgit or find some real deployment server.
Without this changes future addon releases are blocked!

Add to npc presence time, that allow to display npc only in given time period.
ex. "time" : "22-5"

You don't have to stay next to npc to open dialog.

Obecnie wciąż ładuje się stary, co powoduje błędy jak dodatek próbuje wczytać nowe funkcje logowania. Musiałem dodać tymczasowego fixa , bo inaczej cały dodatek nie będzie działać.

Obecny intefejs testów Mocha (qunit) jest niedorobiony i brakuje mu kilku podstawowych funkcji, takich jak nestowanie kolejnych suitów i before. Do tego sprawia same problemy, jak z wyciekaniem globalnych zmiennych do innych suitów.

Proponuję zmianę interfejsu na taki, który owe funkcje posiada i nie sprawia problemów. Z moich krótkich testów przykładowo TDD nie wycieka globalnych zmiennych (no bo suiteSetup zamyka już w funkcji), a dodatkowo zawiera nestowanie, a także podobny syntax co qunit (suite,test) z drobnymi różnicami (suiteSetup zamiast before, setup zamiast beforeEach)

Mając wybrać nowy interfejs najbardziej skłaniałbym się właśnie do TDD. Jeżeli jednak będzie taka potrzeba, to mogę się odpowiednio dostosować. Po wybraniu odpowiedniego nowego interfejsu mogę sam przepisać wszystkie testy na niego.

Można by dodać jego togglowanie jako akcje na kliknięcie ikone panelu. Teraz się tylko otwiera, myślę, że było by spoko jakby umiał się też zamknąć w ten sam sposób.

Originally posted by @akrzyz in #58 (comment)

test/npc_suite.js Jest źle napisany i wycieka before() naokoło, dlatego nie mogę np. dodać testów do test/night_suite.js bo wykrywa before() z npc_suite i nie mogę testować niczego w pliku.

Z tego co patrzyłem to w Mocha w interfejsie Qunit nie można robić tak jak jest teraz zrobione, to jest dać jedno before a potem kilka suite. Ja to bym podzielił to po prostu na pliki poszczególne, i w każdym w before wpisał tylko to, co jest potrzebne.

Jeżeli chcesz przetestować błąd to dodaj w teście w night-cie console.log(nerthus), a ci wypisze obiekt który powstałby po before z npc_suite
Możesz też spojrzeć na 7e2b118 tutaj

npc.url support now only global url or url from margonem.pl.

Support for url pointing to repository is required with nerthus.addon.fileUrl(url)

more info and some graphics at
https://www.margonem.pl/?task=forum&show=posts&id=264553&ps=5#post40857512
https://www.margonem.pl/?task=forum&show=posts&id=264553&ps=5#post40866371

Osobiście do SI mam działający kod oparty na silniku, do NI będę musiał prowizorkę trochę zrobić bo nie można tak łatwo dostać się jak na SI.

Ale nw czy 'zgodzisz się' na taki kod (linie 771 - 853, i kilka parserów niżej). Najwyżej samemu dorób jakieś sprawdzania dziwne jeżeli nie chcesz by tak mocno ingerować w silnik i window.npcTalk, bo to działa na prawdziwym klikaniu w npce i aktywowaniu ich dialogu, a nie na sprawdzaniu kto jest wokoło itd. i ogólnie robi wiele rzeczy inaczej niż kod który jest teraz. Przez to też może wydawać się trochę długie. (oczywiście nie cały plik to to jedno .-. tylko wybrane linie które wskazałem)

Warto by było gdzieś zawrzeć minimalne wymagania dodatku co do wersji najczęściej używanych przeglądarek. Wtedy nie będzie problemu z używaniem nowych rozwiązań, które mogą nie pasować do starszych przeglądarek. Bardziej by była to informacja dla edytorów jeżeli by chcieli skorzystać z jakiejś funkcji która może wydawać się nowa.

Można by dodać jakiś dodatkowy, boczny panel, możliwe że wysuwany za pomocą przycisku czy coś w tym stylu zawierający dane o zmianach w danej mapie dodawanych przez narratorów/radnych, na podobnej zasadzie co mapy na stałe (osoby folder z plikami typu json z id mapy). Jedyne co to nie jestem pewien jak dokładnie miałoby coś takiego graficznie wyglądać od strony klienta.

Trzeba zmienić link w http://serwery.margonem.pl/ by miał on w sobie najnowsze zmiany co do linków, bo obecnie wczytuje za każdym razem starą wersję, bez względu na to czy localstorage jest włączony czy nie.

Obecnie wersja dodatku jest hashem ostatniego commita. O wiele ładniej wyglądałoby to, gdyby była ona w formacie human-readable. Na forku zastosowałem prosty trzystopniowy system: [ogromne aktualizacje z dużą ilością nowej treści].[dodanie znaczących funkcji].[pomniejsze poprawki] [nazwa aktualizacji], gdzie przy pomniejszych poprawkach nazwa się nie zmienia. Tak naprawdę dowolny format byłby dobry, to jest jeden z przykładów jak można to zrobić i jak ja to zaimplementowałem.

Po co ta zmiana? Wersję dodatku widać przy każdym updacie. O wiele łatwiej domyślić się jak wiele zmieniło się od wersji 2.9.3 do wersji 2.9.4 niż z 42880f572ee023da0f662ccb47b7ccf1d297b1f7 do 396cfbcc7d3debd6cf0b5142292994195d516208. Dodatkowo taka wersja jest łatwiejsza do zapamiętania w razie czyichś problemów z dodatkiem oraz łatwiej posługiwać się takową, np. ogłaszając zmiany na forum czy też na discordzie (a poza tym daje to przyjemne poczucie spełnienia jak przestawia się cyferkę z 9 na 10).

Kiedy wczytujemy starą wersję z localStorage, nowa wersja pliku NN_start z githuba nie zapisuje się

localStorage.getItem(nerthus) po bugu
Należy zwrócić uwagę na nową wersję:

"version":"9fa78fb24bdff0e20a91c68d28e796cd478bbc08"

podczas gdy jednocześnie funkcje z NN_start są stare:

"fileUrl":"(function(filename)\r\n{\r\n    return [[this.filesPrefix, this.version].join(this.version_separator), filename].join('/')\r\n})"

(brak obecnego w wersji 9fa78fb24bdff0e20a91c68d28e796cd478bbc08 encodeURI())

Dla porównania: localStorage.getItem(nerthus) po kompletnym ręcznym usunięciu obiektu w localStoragu
Mimo tej samej wersji co wcześniej funkcja z NN_start jest już nowa:

"fileUrl":"(function(filename)\r\n{\r\n    return encodeURI([[this.filesPrefix, this.version].join(this.version_separator), filename].join('/'))\r\n})"

Jednocześnie w obydwu przypadkach zmiany w innych plikach dobrze zaakceptowało (przykładowo zmiany w mapsArr), jedyne co to przez tego buga nie zmieniło znaków specjalnych na znaki do URI, więc większości osób nie działają teraz nowe mapy.

Jako rozwiązanie bym zapisywał wersję w osobnym localstoragu, i jego najpierw wczytywać. Jak się nie zgadza - usunąć wszystko i kazać pobrać aktualną wersję, usuwając przy tym po sobie śmieci.
O wiele prostsze to niż jakieś dziwne próby najpierw załadowania całego dodatku by dopiero wtedy sprawdzić wersję.

Dodatkowo (raczej nie ma to związku z tym błędem), 17sta linijka w NN_start:

nerthus.addon.version = nerthus.addon.consts.MASTER

jest błędna, ponieważ nigdzie wcześniej nie ma deklaracji nerthus.addon.consts.MASTER, więc nerthus.addon.version pozostaje niezdefiniowany.

Można by dodać przy pojawianiu się deszczu jeżeli nie było go wcześniej powolnego przesuwania opacity z 0 na 1, dzięki czemu pojawienie się go będzie wyglądać naturalniej.

Do tego możliwie zmienienie części deszczów które nie są burzami na deszcze z mniejszym opacity (np. 0.7) tworząc zróżnicowane opady.

opacity 1:
https://i.imgur.com/sP1VAZy.png
opacity 0.7:
https://i.imgur.com/kRTI6id.png

Jak na tą chwilę: wszystko. Dodatek w ogóle się nie wczytuje na NI.

Tak to:
Tarcza powinna być jako button taki jak np. konfiguracja.
Komendy powinny ładnie wyglądać.
Okienko konfiguracji tak samo.

lista niekompletna.

Ogólnie najpierw trzeba sprawdzić jak podłącza się dodatek pod NI - czy osobnym linkiem czy tym samym. (obecnie nie ma w ogóle podłączonego) Wtedy będzie można stworzyć plan działania.