Moja gra napisana w C++

Question

Cześć, stworzyłem grę w której w kosmosie strzelamy do przeciwników. Nazwałem ją bardzo oryginalnie Space Shooter xd. Ale nie w tym rzecz. Chciał bym abyście w nią zagrali i dali mi jak najwięcej feedbacku. Oraz co dla mnie jeszcze ważniejsze żebyście ocenili mój kod.

Wersja na windows :WINDOWS
Wersja na linuxa: LINUX
Oraz kod

Liczę na dużo konstruktywnej krytyki, nawet jeśli będzie ona surowa :)

EDIT: JEŚLI TWÓJ EKRAN NIE OBSŁUGUJE ROZDZIELCZOŚCI 1440x900 NIE WŁĄCZAJ TEGO PROGRAMU. 
Błędnie załorzyłem że każdy ekran obsługuję taką rozdzielczość. Przepraszam za to, postaram sie to naprawić jak najszybciej

Comments

Tak na Linuksie. Niestety taki błąd

---

teraz nie jestem wstanie ci na to odpowiedzieć bo nie jestem w tym momencie na linuxie. Podejrzewam że zapomniałem wrzucić jakiegoś pliku. Postaram się to jak najszybciej naprawić.

---

Wydaje mi się że wszystko powinno być wporządku, mi noramlnie działa z tego linku na linuxie.

---

Bo masz zainstalowaną bibliotekę sfml

Doinstalowałem sfml ale nadal to samo

---

nie wiedziałem że to tak działa. Co muszę zrobić żeby odpaliło to na każdym linuxie nawet jak nie ma sfml? A tobie pewnie nie działa po zainstalowaniu bo masz inną wersje sfml. ja mam 32-bitową.

Jeśli chcesz to możesz poprostu sam skompilować program

Answer 1

I. Rozgrywka

Co mi się podoba:

• Gra jest dynamiczna. Wszędzie latają wrogie pociski i statki, które dodatkowo odbijają się od siebie
• Muzyka +
• Własna grafika
• Wymaga zręczności

Co mi się nie podoba

• Po przegranej gra wyłącza i włącza się
• Poruszanie się statkiem jest strasznie wolne w porównaniu do np kursora w menu. Niby mogę zmienić DPI na myszce, ale coś wyraźnie spowalnia statek w grze. Może to jakiś problem w kodzie?
• Dopiero w kodzie dowiedziałem się, że do czegoś można używać lewego i prawego przycisku shift; jak również o istnieniu punktów doświadczenia i ulepszeń

II. Uwagi dotyczące kodu

1. Pierwsze co rzuciło mi się w oczy (ba nawet uniemożliwiło grę i bardzo zdenerwowało!) to bug w momencie gdy zadana rozdzielczość nie jest dostępna. Słusznie zauważyłeś, że problemem jest pętla gry. To właśnie ona jest też przyczyną dla której gra ponownie uruchamia się po przegranej. Lepsze podejście: link 
   Natomiast można usunąć ten problem tak, aby zwykły hipopotam mógł zagrać w Twoją grę:

   	sf::VideoMode videoMode(1440, 900);
   	if (!videoMode.isValid()) // sprawdzamy czy rozdzielczość jest dostępna
   	{
   		auto validModes = sf::VideoMode::getFullscreenModes(); // pobiera wszystkie dostępne rozdzielczości
   		for (auto& i : validModes)
   		{
   			if (i.width >= videoMode.width && i.height >= videoMode.height) // większe ponieważ chcemy, aby wszystko się zmieściło
   			{
   				videoMode = i;
   				break;
   			}
   		}
   
   		if (!videoMode.isValid())
   		{
   			std::cerr << "VideoMode " << videoMode.width << "x" << videoMode.height << " is not available\n";
   			return 0;
   		}
   	}
   
       sf::RenderWindow window(videoMode, "My window",sf::Style::Fullscreen);

   Mógłbyś również ustawić rozdzielczość taką jaką użytkownik ma domyślną (statyczna funkcja - sf::VideoMode::getDesktopMode) i następnie przeskalować całą grę korzystając sf::View żeby zachować odpowiednie proporcje w grze.

2. Wincyj OOP, wincyj funkcji. Nigdy nie wrzucamy wszystkiego do jednej. 
3. Przyciski i sprawdzanie kolizji:

   if(pozycja.x>=510&&pozycja.x<=910&&pozycja.y>=150&&pozycja.y<=350)
       return 1;//po kliknięciu start program zwraca 1 co rozpoczyna grę
   if(pozycja.x>=280&&pozycja.x<=1130&&pozycja.y>=500&&pozycja.y<=700)
       return 0; // po kliknięciu wyjdź z gry program zwraca 0 co kończy pracę programu
   

   Załóżmy pomylisz się gdzieś o 10px i co zrobisz? Przyciski też mogą zostać zrobione w ramach klasy sf::Sprite czy sf::RectangleShape. Ustawiasz położenie, rozmiar, teksturę, a później tylko sprawdzasz kolizję z myszką bez żadnego zastanowienia:

   if(sf::Mouse::isButtonPressed(sf::Mouse::Left))
   {
       sf::Vector2i mousePos = sf::Mouse::getPosition(window);
       if (startButton.getGlobalBounds().contains(window.mapPixelToCoords(mousePos))
   	return 1;
       /* 
       dodatkowo metoda mapPixelToCoords dla poprawnego działania z sf::View 
       */
   }

4. Nazwy powinny być czytelne (najlepiej od razu po angielsku). Trudno się domyślić co to jest i gdzie tego szukać w projekcie:

   window.draw(st);
   window.draw(kc);

   Podobnie tutaj:

   int licznik = 0;//licznik dzięki któremu pojawiają się przeciwnicy
   int licz = 0;// licznik dzięki któremu przeciwnicy strzelają
   int licz2 = 0;//licznik dzięki któremu mini boss strzela
   int licz3 = 0;//licznik dzięki któremu boss strzela
   
   bool bs = false;//// zmienna bs ma wartość true gdy walczysz z bossem
   

   Przykładowo zamiast zmiennej bs mogłoby być bossFightEnabled. Tak powinieneś robić z każdą zmienną. Zalety takiego rozwiązania:
   - komentarz jest zbędny, bo nazwa dokładnie mówi jakie jest zadanie tej zmiennej.
   - ułatwiasz innym zrozumienie kodu (obowiązek jak pracujesz w grupie)
   - jak wrócisz do projektu za rok nadal będziesz rozumiał co się w kodzie dzieje i jak go modyfikować (przynajmniej w małym stopniu)
5. W przypadku klasy Enemys (dlaczego nie Enemy?)
6. Dla poprawienia czytelności możesz użyć enum (np dla Enumys::Type).
7. Nie zawsze korzystać z iteratorów. Możesz sobie ułatwić zadanie korzystając z range-based for (C++11) z dedukcją typu (auto):

   for(std::vector<std::unique_ptr<Weapon>>::iterator itr = bron.begin(), koniec = bron.end(); itr != koniec; ++itr)
   {
       (*itr)->draw(window);
       (*itr)->move();
   }
   

   Dodatkowo: Nazwa nie sugeruje, ze 'bron' to kontener - lepiej nazwać 'bronie' (albo od razu po angielsku).
   Przykład użycia:

   for(auto& weapon: weapons) 
   {
       weapon->draw(window);
       weapon->move();
   }
   

8. Operatory logiczne zwracają wartość typu bool:

   bool Enemys::poza_mapa()
   {
       if(y>900) return true;
       else return false;
   }

   Od razu krócej:

   bool Enemys::poza_mapa()
   {
       return y>900;
   }
   

9. Do kodu wyżej: Czy to, aby na pewno powinno się znajdować klasie Enemy?

10. Puste metody i brakujące destruktory dla klasy Medium:

    void Medium::attack()
    {
        ;
    }
    

11. Bardzo często powtarzasz sekwencje kodu. Zamiast tego korzystaj z pętli i tablic. Dane możesz również pobierać z pliku redukując powtórzenia. Zaoszczędzisz sobie setki linii kodu. Np ta instrukcja switch w monstrualnej funkcji first_mission ma ponad 600 linii. Zdecydowanie o co najmniej 590 za dużo. 

 

Możliwe, że coś jeszcze dopiszę. Niektórym elementom jeszcze nie zdążyłem się przyjrzeć. Ogólnie największy problem to brak dobrej obiektowej konstrukcji programu,

Comments

Dziękuje za tak obszerną opinie.

Najpierw odniosę się do uwag dotyczących gry.

-gra po przegraniu (jak również po wygraniu ) wyłącza się i włącza. Jest to dość skomplikowany problem. Chciałem go rozwiązać inaczej tworząc funkcję która usunie wszystkie zbędne obiekty oraz przywróci atrybuty statku kosmicznego do takich jakie są zapisane w konstruktorze tego obiektu. A na końcu dodałem funkcję startGame(). Problem był jednak taki że gra przy restarcie się crashowała. Nie potrafiłem rozwiązać problemu inaczej więc postanowiłem że cała funkcja first_mission() zostanie zamnięta i wywołana na nowo co omnie cały problem i nie będę musiał tworzyć wielu nowych lini kodu. Ale zdaję sobie sprawę z tego że nie jest to rozwiązanie optymalne i muszę nad tym popracować
- Ja zazwyczaj w myszce mam ustawione bardzo wysokie DPI przez co uznałem że najlepiej będzie spowolnić ruch statku względem myszki. Ale faktycznie takie rozwiązanie nie jest optymalne jak ktoś ma niższe DPI. Mam zamiar dodać możliwość zmiany szybkości staku.
- Prawy przycisk muszy nie służy do niczego, a wydawało mi się że strzelanie LPM będzie oczywiste, ale postaram się dodać w menu opcje sterowanie która będzie wszystko wyjaśniała.

A jeżeli chodzi o uwagi dotyczące kodu to:

1. To jest problem który muszę w pierwszej kolejności rozwiązać, dzięki za zaproponowanie rozwiązania problemu.
2. Racja, muszę podzielić kod w funkcji first_mission() na kilka mniejszych funkcji, tym bardziej że duża część tego kodu przyda mi się jeśli stworzę kontynuację, więc lepiej mieć go w osobnej funkcji.
3. Nie wiedziałem że można tak zrobić z myszką, to napewno lepsze rozwiązanie więc na pewno to zmienie.
4. Masz w 100% rację. Większość kodu tworzyłem w lipcu i nie przykładałem zbyt dużej wagi do nazw. W sierpniu nie zrobiłem zbyt dużo, a gdy wróciłem we wrześniu żeby skończyć projekt sam się gubiłem w nazwach. Wtedy było jeszcze gorzej bo nie było połowy komentarzy.
5. Akurat uważam że to jest nieistotna pierdoła. Nazwałem ją Enemys ponieważ to klasa matka z której dziedziczą inne klasy wrogów. A więc klasa Enemys służy to tworzenia wielu wrogów różnego typu stąd to s na końcu.
6. Szczerze mówiąc nie lubie tego enum, ale pewnie masz rację powinienem to zmienić.
7. Nie wiedziałem że tak się da więc dzięki za poradę.
8. Myślałem że bool może zwracać tylko true/false, ewentualnie 0/1 więc dzięki za info
9. Wydaje mi się że tak
10. Pusta metoda jest po to aby móc przechowywać wszystkich wrogów w jednym kontenerze. Pewnie można to zrobić lepiej ale nie wiem jak. I nie mam pojęcia po co w tej klasie destruktor.
11. Tego switcha można by fatycznie trochę skrócić ale napewno nie do 10 lini kodu (a przynajmniej nie jestem sobie tego wstanie wyobrazić). Nowo tworzone obiekty nie zawsze są identyczne. Np. same meteory zajmują więcej niż 10 linii i chyba nie da się tego skrócić.

Postaram się zastosować do twych porad i usprawnić grę. Jak to zrobię to poinformuję tutaj.

---

0b. Możesz również ustawić stałą prędkość statku, która podążałaby za myszką, ale to już kwestia preferencji ;)

0c. Pomyliłem się - chodziło mi o lewy i prawy klawisz shift.

5. Bool zwraca faktycznie zwraca tylko true/false (1/0), ale chodzi o to, że operator logiczny > to funkcja przyjmująca dwa argumenty (y, 900) i zwracająca również bool (y > 900 zwraca 1 w przeciwnym wypadku 0).

11. Cały poziom możesz wczytać z pliku. W funkcji pobierałbyś kolejno np nazwę poziom, ilość wrogów, a następnie ich dane za pomocą pętli. Wewnątrz pętli byłby switch, który w zależności od typu wroga (jakiegoś oznaczenia liczbowego) tworzyłby obiekty klasy Hard, Easy itp.

Answer 2

Jeśli mogę coś wtrącić to poczytaj o systemie kontroli wersji git. Wtedy będziesz miał porządek w projekcie, sam sie nie pogubisz. Jeśli będziesz chciał komuś to pokazać wrzucisz na githuba i nie będziesz musiał bawić się z dyskiem google.

Comments

tak wiem, w przyszłości tak zrobię

Answer 3

Mi się zawsze lepiej patrzy jak każda klasa jest w osobnym pliku który nosi nazwę tej klasy.

Nie podoba mi się plik first.cpp, wszystko walnąłeś do jednej funkcji na pałę.

Kolejna sprawa, że nie powinieneś w funkcji ładować tekstury a potem je dawać do klas tylko zrobić jakiś manager zasobów który przy starcie aplikacji wszystko załaduje z danego katalogu i poprzypisuje do kluczu odpowiadajacego nazwie pliku, a w klasach które potrzebują odpowiedni zasób po prostu by sobie go z niego pobierały po kluczu.

Zrób jakąś klasę typu GameManager i tam niech będzie uniwersalny sposób gry i opcja załadowania poziomu z pliku. Przydałby się wtedy także dodatkowy programik do tworzenia poziomów.

Comments

Dzięki odpowiedź, ten pomysł z klasą Game Manager jest nawet fajny. Mogłem też klasy SpaceShip i Weapon rozdzielić na 2 pliki. Ale w przypadku klas dziedziczących z klasy Enemys to wydaje mi się że lepiej jak są w jednym pliku bo łatwiej można zerknąć na atrybuty dziedziczone. Funkcję first.cpp faktycznie mogłem lepiej podzielić.

Myślałem też nad stworzeniem edytora poziomów co bardzo przyspieszyło by pracę nad ewentualną rozbudową gry. Ale szczerze mówiąc nie mam pojęcia jak się za to zabrać.

A co sądzisz o samiej grze?