Ocena projektu

Question

Projekt (jeszcze nie) jest grą, w której prawdopodobnie będziemy hodowali/opiekowali się pixelowymi stworzonkami i otaczającym go światem.

Czymś ważnym co wdrożę w kolejnej wersji jest ekwipunek oraz większa interakcja, choć jednak prosiłbym o ocenę stanu obecnego.

Tak, gra jest renderowana w DOM, nie canvas, to musi boleć :c

(pomniejsz ekran a postacie przyjdą na widoczny teren)

Jest napisana głównie w JS ES6+ wraz z Jquery i na ocenie tego głównie mi zależy, bo grafiki są tymczasowe a obecnie grywalnością (posiada taką?) i wyglądem wrażenia nie robi.

 

używając w konsoli funkcji

example

możemy spawnić (pojawiać!(?)) postacie.

JS:

https://tiny.pl/gv7b2

 

Będę wdzięczny za opinie <3

Comments

Proszę przywrócić kod.

---

Gotowe, robiłem porządki

Answer 1

Jest napisana głównie w JS ES6+

To co tu robi "var" :)?

Parę propozycji zmian w zakresie kodu JS:

1. j.w. var pozamieniać na const, ewentualnie let jeśli będzie taka potrzeba,
2. klasa entity: constructor(z,x,c,v,b,n,m,a,s,d,f,g,h) błędny konstruktor. Technicznie teoretycznie jest oki, ale przy tak dużej liczbie parametrów nie robi się ich jako listę, bo łatwo się w tym pogubić. Przerób to na obiekt konfiguracyjny. 
3. "creature[id].left==true" unikaj takich zapisów... po pierwsze to lepszy jest i bezpieczniejszy operator identyczności "===", a po drugie to w sumie wystarczyło tutaj (jeśli creature.left jest typu boolean) po prostu podanie tej wartości jako warunku dla IF.
4. podepnij sobie np. eslinta aby Ci sprawdzał kod, głównie formatowanie, bo masz strasznie porozwalane.
5. Staraj się unikać nazw właściwości w obiektach, które po za obiektem mają specjalne przeznaczenie, np. " class: `creatureimg`,". Nie jest to technicznie błąd w ES, ale osobiście uważam to za złą praktykę bo może narobić z czasem zamieszania w kodzie.
6. Z tego co widzę, to jQuery używasz tylko do pobierania referencji DOM i obróbki tyli CSS. W tym momencie pytanie po co w ogóle Ci jQuery? Referencję pobierzesz bardzo szybko np. querySelector, getElementById itp., a style ładnie obsłużysz classList.
7. "spawncreature(spwtable[0],spwtable[1],spwtable[2],spwtable[3],spwtable[4],spwtable[5],spwtable[6],spwtable[7],spwtable[8],spwtable[9],spwtable[10],spwtable[11],spwtable[12]);" taki kod nie ma prawa istnieć :) Tak samo jak to pobierasz wartości powyżej tej linijki. Poczytaj o zasadach pisania kodu, aby nie powielać wielokrotnie tych samych czynności. Nawet jeśli z jakiś powodów miałbyś pobrać taką listę wartości to pomyśl nad jakimś przeiterowaniem np. po rodzicu itp.
8. a tak na marginesie to w ogóle proponuję zagłębić się w tematykę tzw. wzorców projektowych, bo akurat przy temacie gry można by się fajnie nimi pobawić. Teraz kod jest bardzo trudny w rozczytaniu i zrozumieniu co gdzie się dzieje...
9. Rozbij ten kod na małe pliki co zwiększy czytelność i łatwość utrzymania kodu. Do złączenia na sam koniec na wersję prod. możesz użyć np. webpack czy inne tego typu narzędzia, które od razu pomogą Ci np. z kompilacją SCSS, prefixami css, minimalizacją kodu html/css/JS itp. itd.

Comments

Jak zawsze super wszystko wypisane. Lecz wydaje mi się ze zapis w warunkach wartości logicznych ma sens. Na przykład zdecydowanie wole === false niż negacje którą można przeoczyć ;) Dodatkowo zapis === true niesie to ze sobą informacje ze oczekujemy wartości logicznej a nie np null/instancja. Dlatego też przychylił bym się bardziej do pierwszej propozycji z === ;)

Co do radzenia sobie z dużą ilością parametrów warto też wspomnieć o innym sposobie. Możemy dodać wartości domyślne a następnie funkcje ustawiające. 

Np w konstruktorze masz jeden, dwa parametry które musza z jakiegoś powodu być. Następnie w konstruktorze dopisujesz wartości domyślne do wszystkiego na stałe. 

this.speed = 10;
this.size = 3;

i tak dalej. Ostatnim krokiem jest stworzenie funkcji ustawiających w środku klasy 

setSpeed(speed){
    this.speed = speed;
}

 

---

Co do tych warunków IF to w sumie kwestia indywidualna, ale co by nie mówić to tak jak napisałeś i Ty i ja, czyli "===" :). Osobiście od pewnego czasu siedzę dużo w TypeScript i pracując z silnym typowaniem i interfejsami mam większą kontrolę nad typami, ale w czystym JS może być jak piszesz, kwestia wyboru co komu bardziej odpowiada.

Co do tych wartości pól ustawianych jako stałe to pomyślałbym jeszcze nad ich jakąś ochroną przed "światem zewnętrznym" jeśli mają to być faktycznie stałe. Można by wyrzucić je po za zakres deklaracji klasy albo pobawić się w set/get, bo na razie nie mamy jeszcze natywnych modyfikatorów dostępu w klasach JS jak private, protected itp.

---

1. Gotowe, var było roboczo przez obawy z dostępnością funkcji

2. Stworzylem takie coś:

http://jsfiddle.net/7eruvk6c/

Lecz nie wiem czy jest to poprawnie zrobionie, czy do konstruktora się powinno także przekazać obiekt/ domyślne parametry ustawić w klasie.

4. Mialem problem z jslintem bo np. kazal mi brać w cudzyslow "class" itp.

5. Zatem jak nadać atrybut class dla tworzonego diva przy pomocy obiektu?

6. Przyda się w przyszłości do eventów z którymi będzie po prostu łatwiej, choc masz w zasadzie rację w większości, to jednak chyba z przyzwyczajenia korzystam z Jquery.

Reszta jest jasna i już się doszkalam, dziękuje bardzo za poświęcony czas :)