Discussion :

[Alex__r]

Bonjour a tous !

J'étais en terminale S cette année avec option info et j'ai appris le C++ tous seul depuis plusieurs années maintenant.
Pour mon projet de fin d'année j'ai fait un jeu en C++ qui m'a permis de décrocher un petit 20 coef 2

Mon projet se trouve ici https://github.com/wistifer/jeu_isn

Cependant, malgré la bonne note que j'ai eu je voulais me tourner vers des développeurs en C++ plus expérimentés pour avoir un avis sur mon programme qui est sans doute un peu maladroit

J'aimerais donc savoir ce que vous pensez de ce que j'ai fait sur l'organisation, les classes et si vous avez des conseils pour optimiser mon code

J’espère que github marchera bien je n'ai pas encore l'habitude de l'utiliser ...

Merci d'avance pour tous vos conseils n’hésitez pas !!

Alex

[Invité]

Salut

J'ai testé vite fait sur linux.

À la compilation, il y a quelques erreurs :
- pour les include c'est / et non \ donc il faut mettre #include <SFML/graphics.hpp> et non #include <SFML\graphics.hpp> dans les .h
- il faut ajouter #include <cmath> au début de decor.cpp

Pour l'exécution, il manque les dossiers Jeu/Jeu/graphics/hero et Jeu/Jeu/graphics/ennemi.

Je n'ai pas trop regardé le code sinon...

[Alex__r]

Effectivement merci ...
J'ai corrigé ces petites erreurs c'est bon !

[Invité]

Ok, ça compile mieux.

Au niveau du gameplay, c'est un peu étrange : il y a 3 ennemis au début et plus rien ensuite.

Au niveau du code, je n'ai pas trop regardé l'organisation des classes mais déjà tu as des fuites mémoires. Par exemple dans Ennemi::updatePerso tu fais un Collision * test = new Collision; mais tu ne fais pas de delete. Et au final, je pense que tu n'as pas besoin de pointeur, un Collision test; suffit. Pareil pour vector<Ennemi*> * tabPerso dans loadPerso : pas besoin de pointeur. Déclare une variable statique vector<Ennemi> et passe-la en paramètre de fonction par référence vector<Ennemi> & tabPerso.

Mais sinon, pour un lycéen qui a appris tout seul, c'est pas mal du tout. Continue comme ça.

[Alex__r]

Merci de t'être penché sur mon programme et pour ces conseils !!

Au niveau du gameplay, c'est un peu étrange : il y a 3 ennemis au début et plus rien ensuite.

effectivement je vais changer ça

un "Collision test;" suffit.

ça semble mieux pour gérer les fuites je comprend je pensais qu'il ballait mieux créer un pointeur dès qu'on en avait l'occasion mais ça semble plus logique ainsi

Déclare une variable statique "vector<Ennemi>"

je n'y avait pas pensé c'est plus pratique merci beaucoup !

[Luc Hermitte]

Hello, très rapidement en vrac côté code C++

- Balle::getDirection n'est pas const-correct, pareil pour l'autre getter. Pareil dans les autres classes.
- Balle::getCollision. C'est bizarre de renvoyer le résultat par paramètre sortant. En plus, ce n'est pas nécessairement plus performant. Accessoirement, préfère une référence à un pointeur : quid si l'utilisateur se plante et te passe un pointeur nul ? Avec une référence tu dis clairement: "you shall not pass". Ce problème de plein de pointeurs qui auraient dus être des références est assez récurrent.
- Constructeur par défaut de Balle: est-ce que cela a vraiment du sens à ce que ce constructeur existe
- Balle::Balle(sf::Vector2f const a, bool dir): C'est plus gros que 2 int un Vector2f? Si oui, il peut être préférable de le passer par référence constante. A voir.

- typedef.h: Ne jamais mettre de using namespace dans un .h. Cf la FAQ
- typedef struct {...} Nom;: C'est du C ça. En C++, c'est plus simple: struct Nom{...};
- préfère les enum class du C++11 aux enum

- beaucoup trop d'includes inutiles dans Decor.h (et ailleurs) -- ça fait inclusions magiques comme si tu ne savais pas pourquoi tu inclus quel fichier.
- vector<vector<int>> n'est pas une structure efficace pour stocker une structure carrée. Préfère une classe dédiée avec une seule allocation et où tu redistribues les coordonnées pour "délinairiser" si je puis dire. Exemple ici: https://github.com/LucHermitte/ticta...actoe.cpp#L182

- Decor::getMap n'a aucun effet: tu modifies une variable locale (un paramètre pris par copie est une variable locale) pour lui donner une adresse, qui restera donc locale. La fonction est-elle bien nécessaire? Puisqu'elle ne marche pas, c'est qu'elle ne doit pas être utilisée. Ne fait pas des getter pour le plaisir d'en faire. Fais-en uniquement si tu en as besoin ailleurs.

- lecture des fichiers: toujours tester les résultats des lectures et des ouvertures de fichiers en cas d'erreurs. Sinon tu vas traiter des données qui n'ont aucun sens et ne rien y comprendre. La FAQ en parle je crois.

- Decor::loadMap() mériterait un découpage en sous-fonctions. Typiquement le switch devrait être une fonction à part.

- Ennemi: beaucoup trop de pointeurs bruts: Résultat tu as une fuite de mémoire derrière la variable test -- cela aurait dû être un objet sur la pile. Pour les balles, tu aurais dû employer des std::unique_ptr je soupçonne. Même chose pour les Hero, d'ailleurs cela sens la duplication de code à plein nez. Essaie de factoriser si tu peux -- les fonctions virtuelles t'aideront pour permettre de spécialiser les points de variation.
- Pas sûr que tu aies besoin d'un constructeur par défaut pour Ennemi