Prośba o ocenę kodu gry w szachy

0

Witam,
Napisałem grę w szachy w Pythonie z użyciem Pygame. Postanowiłem że napisze do nich jeszcze grę z botem ale przed tym chciałbym aby ktoś ocenił mój kod. Starałem się napisać ten program obiektowo. Proszę o ocenę :)

Mój kod na github: https://github.com/Suchy702/szachy/tree/master

3

Pierwsze co bym zmienił, to:

  1. ujednolicenie nazewnictwa zmiennych do jednego języka - angielskiego.
  2. skasowanie komentarzy - nie są one potrzebne w jednak prostym kodzie.
  3. zastosuj takie nazewnictwo, żebyś wiedział co robi metoda
  4. program nie jest obiektowy, poza użyciem obiektowych słów kluczowych jak 'class'
  5. zacznij rozbijać ten kod na krótkie metody, które robią 1 (!sic) rzecz, nie strukturalny kod na 300 linijek
  6. staraj się deklarować zmienne na "górze" modułu / klasy, chyba że w pythonie nie ma takiej konwencji
  7. OOP - zacznij refactor do kodu obiektowego: każdy obiekt w grze to ma być właśnie obiekt! tj. plansza, pole na planszy, bierka, ruch
    I na przykład, plansza ma kolekcję pól, pole z kolei ma referencję do bierki, która się na nim znajduje, każda bierka ma obiekt, który definiuje jej ruch. Każda bierka ma def move() etc etc

Jak coś to jak poprawisz, to możesz dać znać ;)

0
NeutrinoSpinZero napisał(a):

Pierwsze co bym zmienił, to:

  1. ujednolicenie nazewnictwa zmiennych do jednego języka - angielskiego.
  2. skasowanie komentarzy - nie są one potrzebne w jednak prostym kodzie.
  3. zastosuj takie nazewnictwo, żebyś wiedział co robi metoda
  4. program nie jest obiektowy, poza użyciem obiektowych słów kluczowych jak 'class'
  5. zacznij rozbijać ten kod na krótkie metody, które robią 1 (!sic) rzecz, nie strukturalny kod na 300 linijek
  6. staraj się deklarować zmienne na "górze" modułu / klasy, chyba że w pythonie nie ma takiej konwencji
  7. OOP - zacznij refactor do kodu obiektowego: każdy obiekt w grze to ma być właśnie obiekt! tj. plansza, pole na planszy, bierka, ruch
    I na przykład, plansza ma kolekcję pól, pole z kolei ma referencję do bierki, która się na nim znajduje, każda bierka ma obiekt, który definiuje jej ruch. Każda bierka ma def move() etc etc

Jak coś to jak poprawisz, to możesz dać znać ;)

Dziękuje bardzo !
Zgodnie z twoimi poradami to chyba lepiej będzie jak napiszę to od początku :P.

2
plansza.B_szach = False
plansza.C_szach = False
plansza.B_r_szach = 0
plansza.C_r_szach = 0

To jest mało obiektowe. Jak widzisz takie coś, to powinieneś od razu napisać klasę z odpowiednimi polami.

if not W and U is False:

Jednoznakowe zmienne zaciemniają kod. Ponadto konstrukcja tego warunku nie ułatwia jednoznacznego odczytania.

is pewnie zadziała dla booleana, ale poprawniej w tej sytuacji byłoby użyć tego operatora ==. ( http://net-informations.com/python/iq/is.htm ).
W ogóle najlepiej zapomnij o is, to unikniesz wielu nieprzewidzianych sytuacji :]

plansza.screen.blit(pygame.image.load('ustawienia.png'), (0, 0))

Nigdy tak nie rób. Obrazek załaduj gdzieś wcześniej w aplikacji, przed pętlą, żeby siedział w pamięci. W blit używaj referencji do uprzednio załadowanego obrazka.

        if self.rodzaj == 'pion':
            if self.poz[1] == '8' or self.poz[1] == '1':
                self.rodzaj = 'hetman'
                return ruchy_hetmana(self, slownik, T)
            else:
                return ruchy_piona(self, slownik, self.kolor, T)
        elif self.rodzaj == 'kon':
            return ruchy_konia(self, slownik, T)
        elif self.rodzaj == 'wieza':
            return ruchy_wiezy(self, slownik, T)
        elif self.rodzaj == 'goniec':
            return ruchy_gonca(self, slownik, T)
        elif self.rodzaj == 'hetman':
            return ruchy_hetmana(self, slownik, T)
        else:
            return ruchy_krola(self, slownik, T)

To jest bardzo nieobiektowe. Aż się prosi, żeby zrobić klasę dla każdej bierki, nadpisującą "wirtualną" metodę ruchy().

No i obiekty wsadzamy w odpowiednie pliki *.py.

Obiektowe szachy z dwoma plikami źródłowymi to przykry żart...

1
NeutrinoSpinZero napisał(a):
  1. staraj się deklarować zmienne na "górze" modułu / klasy, chyba że w pythonie nie ma takiej konwencji

A gdzie jest? W Pascalu? PL/SQL?

0
stivens napisał(a):
NeutrinoSpinZero napisał(a):
  1. staraj się deklarować zmienne na "górze" modułu / klasy, chyba że w pythonie nie ma takiej konwencji

A gdzie jest? W Pascalu? PL/SQL?

ANSI C?

1
Spine napisał(a):

ANSI C?

To chyba legacy (c89) z powodu prymitywnych kompilatorow w tamtych czasach. Teraz to raczej nie obowiazuje.

0
Spine napisał(a):
plansza.B_szach = False
plansza.C_szach = False
plansza.B_r_szach = 0
plansza.C_r_szach = 0

To jest mało obiektowe. Jak widzisz takie coś, to powinieneś od razu napisać klasę z odpowiednimi polami.

if not W and U is False:

Jednoznakowe zmienne zaciemniają kod. Ponadto konstrukcja tego warunku nie ułatwia jednoznacznego odczytania.

is pewnie zadziała dla booleana, ale poprawniej w tej sytuacji byłoby użyć tego operatora ==. ( http://net-informations.com/python/iq/is.htm ).
W ogóle najlepiej zapomnij o is, to unikniesz wielu nieprzewidzianych sytuacji :]

plansza.screen.blit(pygame.image.load('ustawienia.png'), (0, 0))

Nigdy tak nie rób. Obrazek załaduj gdzieś wcześniej w aplikacji, przed pętlą, żeby siedział w pamięci. W blit używaj referencji do uprzednio załadowanego obrazka.

        if self.rodzaj == 'pion':
            if self.poz[1] == '8' or self.poz[1] == '1':
                self.rodzaj = 'hetman'
                return ruchy_hetmana(self, slownik, T)
            else:
                return ruchy_piona(self, slownik, self.kolor, T)
        elif self.rodzaj == 'kon':
            return ruchy_konia(self, slownik, T)
        elif self.rodzaj == 'wieza':
            return ruchy_wiezy(self, slownik, T)
        elif self.rodzaj == 'goniec':
            return ruchy_gonca(self, slownik, T)
        elif self.rodzaj == 'hetman':
            return ruchy_hetmana(self, slownik, T)
        else:
            return ruchy_krola(self, slownik, T)

To jest bardzo nieobiektowe. Aż się prosi, żeby zrobić klasę dla każdej bierki, nadpisującą "wirtualną" metodę ruchy().

No i obiekty wsadzamy w odpowiednie pliki *.py.

Obiektowe szachy z dwoma plikami źródłowymi to przykry żart...

Dziękuję bardzo za ocenę !
"Starałem się" napisać to obiektowo...

No i obiekty wsadzamy w odpowiednie pliki *.py.

Dla każdej klasy mam robić osobny plik ?

2
Suchy702 napisał(a):

Dla każdej klasy mam robić osobny plik ?

Tak. Zobacz sobie inne projekty na GitHubie. Niekoniecznie pythonowe...

0

Cześć,
Zgodnie z waszymi radami napisałem moje szachy od nowa i dodałem prymitywnego bota :), teraz też proszę o ocenę ale teraz mam jeszcze dodatkowo kilka pytań i proszę o odpowiedź, a zatem:

  1. Za każdym razem gdy pisze (jakiś obiekt) == None, Pycharm sugeruje mi że powinienem użyć is, dla bezpieczeństwa zostawiać == czy tylko gdy upomni mnie Pycharm pisać is?
  2. Pycharm sugeruje mi że kilka metod klas powinny być zwykłymi funkcjami, czy robić metody do klasy tylko wtedy gdy korzysta ona z atrybutów tej klasy, czy kiedy?
  3. Czy mój algorytm minimax jest dobrze napisany ?
  4. Pliki zaczynać z dużych czy małych liter ? :D
    Mój projekt na githubie:
    [https://github.com/Suchy702/Szachy]
0

Jak to odpalić? Z pythonem nie mialem nigdy do czynienia.

0
kzkzg napisał(a):

Jak to odpalić? Z pythonem nie mialem nigdy do czynienia.

Musisz mieć interpreter
(Możesz go pobrać z tej strony [https://www.python.org/downloads/])
ściągasz wszystko co mam z Githuba i klikasz dwa razy na main.py, i wszystkie pliki .py muszą być wraz z folderem images w jednym folderze
(Szerze mówiąc nie wiem czy zadziała na innym komputerze niż mój :P)

Zapomniałem dodać że jeszcze trzeba zainstalować pygame
W konsoli wpisujesz
pip install pygame

4

Ok, odpaliłem.
Kodu nie oceniam bo nie wiem co tam sie dzieje.
Masz buga, można zrobić roszadę przez pole szachowane.

0

Kodu nie oceniam bo nie wiem co tam sie dzieje.

Aż tak źle napisany ? :'(

Masz buga, można zrobić roszadę przez pole szachowane.

Dzięki
Okazuje się że nie za dobrze znam zasady gry w szachy :P

2

Kilka uwag:

  1. Zbyt długo czeka się na ruch komputera - w tym czasie program nie reaguje. Może warto obliczenia z Engine przenieść do osobnego wątku?
  2. Czarny król jest nieprawidłowo ustawiony.
  3. Czasami czarny król porusza się na pole będące pod biciem białych figur.
1

Cześć
Moje szachy zostały poprawione zgodnie z waszymi radami:
poprawiłem mechanikę roszady
ustawiłem dobrze czarnego króla
do tego jeszcze zmieniłem grafikę dodałem animację i możliwość "chwytania" pionka a do tego zapis gry :)
Mój program:
[https://github.com/Suchy702/Szachy]

0

Gra łamie zasady gry, min:

  • można przesunąć króla w pole szachowane
  • można pozostawić króla szachowanego

1 użytkowników online, w tym zalogowanych: 0, gości: 1