search

bartq98 / tetris

Simple clone of good, old Tetris with few little improvements. Written with Python and PyGame
1 stars 1 forks source link

Uwagi #4

Closed MarcinCiura closed 3 years ago

MarcinCiura commented 4 years ago

Jeden z lepiej zrobionych projektów, jakie w tym semestrze widziałem. Duży plus za sensowne komentarze.

  1. BOARD_WITH_BORDER_COORDS i GAME_BOARD_COORDS lepiej by wyglądały jako collections.namedtuple.
  2. Analogicznie przy COLORS można użyć enum.Enum, jeśli Pan lubi nowości. Ogólne zalecenie brzmi: stałe napisy jako klucze słowników nie wyglądają poważnie, że nie wspomnę o naruszaniu zakazu stosowania magicznych napisów (vide 2. rozdział Poradnika).
  3. Funkcja pre_configure_window() raczej nie powinna leżeć w config.py.
  4. W klasie Tetromino lepiej wprowadzić nową metodę na dwa pierwsze wiersze wspólne dla draw_bufor() i debug_draw_bufor(), zwracającą parę (x, y), oraz nową funkcję lokalną dla tego modułu (wg Google lepsze od @staticmethod) dla pygame.draw.rect(...) w środku ich pętli. Nawet jeśli chce Pan kiedyś wyrzucić wersję debug..., to, co zostanie, będzie lepiej wyglądać.
  5. Bufor to po angielsku buffer.
  6. Ten wiersz jest niepotrzebny, vide 3. rozdział.
  7. Podobnie jak w punkcie 4. wyciągnąć powtarzającą się część z Gameboard.draw_gameboard().
  8. Wyjąć z Game.py game_single_frame i time_steps_to_fall_bufor jako stałe. Dodatkowa złota myśl na przyszłość: wszelkie zmienne i stałe dotyczące czasu powinny mieć przyrostek, określający jednostkę czasu, w której są mierzone. Dla sekund może być _sec.
MarcinCiura commented 4 years ago

Aaa. Czy pytania: "czy sterowanie (lewo/prawo/obrót itd.) aktualnie spadającego klocka (bufora) zaimplementować w klasie Tetromino? czy detekcję bufora z planszą zaimplementować w klasie Board?" są jeszcze aktualne?

bartq98 commented 4 years ago

Dziękuję za Pana dokładne uwagi i miłe słowa; zachęcają do dalszej pracy :smiley:

Co do poszczególnych uwag:

  1. Zrobione :heavy_check_mark:
  2. Zrobione :heavy_check_mark:
  3. Zrobione. :heavy_check_mark: - jedynym miejscem, gdzie funkcja pre_configure_window() będzie kiedykolwiek użyta jest moduł game.py, stąd przeniosłem ją tam. Myślę, że to dobry wybór.
  4. Zrobione :heavy_check_mark:
  5. Zrobione :heavy_check_mark:
  6. Zrobione :heavy_check_mark:
  7. Zrobione :heavy_check_mark:
  8. Zrobione :heavy_check_mark:

Z drobniejszych, estetycznych poprawek:

W odpowiedzi do Pańskiej drugiej wiadomości - tak, nadal aktualne. Póki co nie implementowałem jeszcze memento ani nie zmieniałem niczego w funkcji Tetromino.move(). Zastanawiam się nad zmianą event.type == pygame.KEYDOWN na pygame.key.get_pressed() by z każdą kolejną klatką w grze nie mieć potrzeby wciskania przycisku np, ↓ kolejny raz, a byłby on odczytywany ciągle. Próbowałem to zaimplementować, ale nie chciało współpracować.

Mam małe pytanie - wiem, że Python to nie Java, ale uważam za estetyczne i czytelne wyrównywanie deklaracji zmiennych w pionie (przykładowo jak tutaj). Czy jest to do przyjęcia, czy raczej powinenem zdać się także w tej kwesti na Pylinta?

MarcinCiura commented 4 years ago

Dziękuję za zmiany. Na razie mam tylko dwie kosmetyczne uwagi.

  1. Te stałe ponazywać.
  2. Zostało jeszcze sporo bufor-ów.

Ponieważ sterowanie wpływa na położenie, czyli na atrybuty Bufora/Klocka, naturalniej będzie je zrobić wewnątrz tej klasy, bo tam jest nam wolno zmieniać wszelkie atrybuty. Alternatywa wiązałaby się z koniecznością wystawienia na zewnątrz zmieniaczy do tych atrybutów, myślę, że niepotrzebnego. Wykrywanie kolizji chyba będzie wyglądać podobnie, czy w Buforze, czy w Planszy, więc proszę zrobić, jak Panu wygodniej. Na odczytywaniu klawiszy w pygame niestety się nie znam. Z dokumentacji get_pressed() wygląda, jakby powinno działać. Wyrównywanie deklaracji mi nie przeszkadza. Może Pan nad LIGHTBLUE dopisać # pylint: disable=to-co-trzeba-żeby-go-uciszyć

bartq98 commented 4 years ago

Co do Pańskich ostatnich uwag:

  1. Zrobione :heavy_check_mark:
  2. Zrobione :heavy_check_mark: - cały kod zrefactoruję, gdy sama gra będzie już technicznie skończona. Póki co wprowadziłem kilka drobniejszych poprawek w kodzie. Zdaję sobie sprawę, że gameboard.draw_gameboard(screen) mogę zastąpić gameboard.draw(screen) itd.

    minmax

    Na branchu evaluating:

Co do minmax - mogę się mylić, ale ten algorytm jest stosowany dla gier dwuosobowych (a moja gra taka nie jest). W każdym razie - stworzyłem nowy moduł evalutaor.py który zawiera klasę o takiej samej nazwie, a w niej - cztery statyczne metody.

Ewaluacja planszy ma miejsce, gdy aktualny klocek już spadł na planszę i potrzeba wygenerować nowy (odpowiada za to metoda generate_tetromino(board: gameboard.Gameboard, malice_level: int)).

Idea działania poniżej:

  1. generuje wszystkie dopuszczalne tetromina (każdy kształt · każdy obrót · przesunięcie na całej szerokości) dzięki funkcji test_all_cases(board: gameboard.Gameboard)
  2. symuluje upadek tego tetromina na istniejącą planszę w simulate_falling(board: gameboard.Gameboard, buffer: tetromino.Tetromino). W sytuacji, gdy dane tetromino na starcie koliduje z krawędziami board funkcja zwraca False i ten przypadek jest wykluczany z puli. Jeśli wszystko odbyło się pomyślnie funkcja zwraca planszę z opadniętym klockiem
  3. liczymy ilość punktów dla wszystkich tak wygenerowanych plansz: iteruję po każdej kolumnie, w każdej kolumnie wierszami od góry do dołu i liczę najwyżej napotkany klocek (jako max_height) oraz ilość pustych klocków pod nim (jako gaps_found) a potem mnożę obie wartości przez wagi i dodaję do score danej planszy.
  4. sortuję po ilości score (im większy tym trudniejszy dla gracza przypadek) i zwracam klocek odpowiedni dla poziomu trudności.

Sprawdziłem i działa - dla poziomu malice = 0 gra generuje głównie tetromina "I" oraz "O", które są banalne do układania. Dla poziomu 9 pierwszy nieprzemyślany ruch gracza bardzo utrudnia mu dalsze życie :wink:

testy

(również branch evaluating, gdyż jest najświeższy)

W module tetromino_test.py sprawdzam działanie wszystkich metod z klasy Tetromino. Póki co mam tam sprawdzanie, czy metoda rotate(buffer) prawidłowo obraca dany klocek. Pozostałe testy też wydają się proste (i najpewniej zrobię je jeszcze dziś), za wyjątkiem metody test_draw, która przyjmuje argument screen bibloteki pygame. Pytanie - czy https://docs.python.org/3/library/unittest.mock.html#quick-guide nadaje się do mockowania klas z pygame, czy mogę tę metodę pominąć?

Z góry dziękuję za Pana czas i pomoc :smiley:

MarcinCiura commented 4 years ago

Na dzisiejszych konsultacjach odpowiedziałem na Pana pytania, prawda?

bartq98 commented 4 years ago

Tak, zgadza się. W ciągu najbliższego dnia/dwóch wdrożę Pana sugestie. Dziękuję

bartq98 commented 4 years ago

Od ostatniej wymiany korespondencji, dodałem:

Mam nadzieję, że cały mój kod jest zrobiony poprawnie i gotowy do oddania.

Porszę o kontakt i pozdrawiam

MarcinCiura commented 4 years ago

LGTM. Gratuluję, jest Pan dwudziesty czwarty. W wolnej chwili: użyć w testach .assertTrue() i .assertFalse(), gdy pasują.

Dodatkowa prośba: jeśli nikt tego przed Panem nie zrobi, proszę ogłosić konsultacje w niedzielę o 20:30 i w poniedziałek o 12:00 pod standardowym linkiem. Dziękuję!

bartq98 commented 4 years ago

Dziękuję za informację, a także za pomoc i uwagi przy projekcie - miałem okazję nauczyć się kilku praktycznych rzeczy

Co do zmian - postaram się poprawić testy w wolnej chwili.

Co do Pańskiej prośby - z tego co widziałem to jeden z kolegów opublikował już post dotyczący konsultacji