1. Dlaczego na Twoim repozytorium wszystkie pliki wrzucone są do jednego katalogu? Bardzo ciężko jest przez to zorientować się z czego zbudowany jest projekt.
2. Dlaczego niektóre nazwy plików w ogóle mi nie mówią na temat ich funkcjonalności? "AK" to nie jest dobra nazwa dla pliku.
3. Dlaczego nie stosujesz jednolitej metody nazywania plików? "Ai_manager", "GameButton", "Zombie Defence". Według mnie powinieneś wybrać jedną metodę i się jej do końca trzymać, ulepsza to czytelność i estetykę.
Przejdę teraz do pliku: "Zombie Defence"
a) Zmienne screenWidth oraz screenHeight są typu int, tymczasem konstruktor sf::VideoMode przyjmuje unsigned int.
b) Byłoby według mnie czytelniej i jaśniej, gdybyś stworzył zamiast dwóch zmiennych, jedną typu sf::Vector2u która opisywałaby wymiary ekranu.
c) Nie wiem jaki był Twój plan przy tworzeniu gry, ale ustaliłeś z góry szerokość i wysokość okna. W tym momencie włączenie Twojej gry na pewnych platformach może być przez to niemożliwe. Spróbuj w przyszłości napisać projekt, który uwzględni fakt, że ludzie korzystają z urządzeń o różnych rozdzielczościach. Metody sf::VideoMode::getDesktopMode oraz sf::VideoMode::getFullscreenModes są kluczowe w tym celu, albowiem informują cię one o parametrach ekranu osoby grającej, dając możliwość odpowiedniego zareagowania. Oczywiście musiałbyś również na nowo napisać wiele algorytmów. DOPISEK: Zauważyłem, że dajesz grającemu możliwość zmiany rozdzielczości, to dobrze z Twojej strony.
d) Zamiast makra NULL, powinieneś wykorzystywać nullptr z C++11. Zauważ, że makro NULL to tak naprawdę 0, jeżeli poślesz do przeładowanej funkcji coś z wartością NULL może to zostać błędnie zinterpretowane jako liczba całkowita, zamiast wskaźnika.
e) W twojej pętli głównej korzystasz z "głupich" wskaźników (Survival*), czyli musisz samodzielnie martwić się o zwolnienie pamięci. W standardowej sytuacji to żaden kłopot, ale zastanów się co stanie się, gdy na przykład funkcja Game::run() wyrzuci wyjątek? Pamięć nie zostanie zwolniona i powstaną przecieki. Zamiast surowych wskaźników powinieneś używać "mądrych" wskaźników ze standardu C++11. Czyli np. std::unique_ptr<Survival>. Jaka jest zaleta takiego rozwiązania? W momencie, gdy program wyjdzie poza zasięg "mądrego" wskaźnika, obiekt ten zostanie automatycznie zwolniony, nie musisz nigdzie zabezpieczać się operatorami delete czyli delete[].
f) Osobiście nie użyłbym tego: "using namespace sf;". Podobnie z przestrzenią nazw std. Dlaczego? Zauważ, że pisząc grę będziesz często tworzyć klasy, które będą nazywać się podobnie do tych np. z SFML. Takie jak okno czy wydarzenie. Wykorzystując operator zasięgu dajesz jasny sygnał skąd pochodzą obiekty, których planujesz użyć.
g) Twoje przyciski nie wykorzystują polskich znaków. Próbowałeś coś na to poradzić? Najprostsza metoda to wykorzystanie obiektów klasy std::wstring które obsługują nasze rodzime znaki. Poczytaj o tym więcej, a całkiem łatwo zrozumiesz jak dodać obsługę dla różnych języków w swojej grze.
To jeszcze nie koniec, będę co jakiś czas dopisywał kolejne uwagi i propozycje, pozdrawiam Lafoniz.
