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

[Alex__r]

Merci beaucoup pour tes remarques !!

J'ai quelques questions..

Balle::getDirection n'est pas const-correct,

Qu'est ce que tu entends par const correct et pourquoi ?

Accessoirement, préfère une référence à un pointeur

Il faudrait donc une référence comme ça ?

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

void getCollision(Collision &test);

C'est plus gros que 2 int un Vector2f?

Comment je peux savoir ça ?

Pour les balles, tu aurais dû employer des std::unique_ptr je soupçonne.

Je n'ai pas encore très bien saisi la différence entre les pointeurs et les unique_ptr mais je suis en train d'y travailler

Essaie de factoriser si tu peux

J'ai déjà essayé de factoriser au maximum mais le problème est que certaines fonctions prennent des paramètres différents et je ne peux donc pas ... Si tu vois des solutions je veux bien que tu me dises


En tout cas merci encore de m'avoir éclairé sur ces points je vais reprendre un peux mon code !!

[Luc Hermitte]

- A propos de const-correct: ta fonction déclare qu'elle pourrait modifier l'objet sous-jacent. Ca doit exister, mais je n'ai pas de bon lien sous la main
- https://cpp.developpez.com/faq/cpp/?...apres-leur-nom
- https://isocpp.org/wiki/faq/const-co...overview-const

- Oui pour la référence, mais j'ai même envie de dire que le plus efficace serait probablement

Code c++ :

Sélectionner tout - Visualiser dans une fenêtre à part

Collision getCollision() const;

- Au sujet du type plus gros que 2 int: en lisant la doc, en faisant des test, en affichant son sizeof, en regardant s'il porte une charge utile de type mémoire allouée, etc

- > "J'ai déjà essayé de factoriser au maximum mais le problème est que certaines fonctions prennent des paramètres différents et je ne peux donc pas ... Si tu vois des solutions je veux bien que tu me dises "

tout le code qui est identique devrait être factorisable d'une façon ou d'un autre. Cela va très probablement signifier que tu vas devoir extraire plusieurs fonctions: des fonctions pour les choses identiques, et des fonctions pour les choses différentes.


PS: avec réflexion, que certains de tes objets dérivent des classes de sprites est très bizarre. Que la balle contiennent un sprite pour la représenter OK, qu'elle soit un sprite, c'est plus étrange.

[koala01]

Salut,

PS: avec réflexion, que certains de tes objets dérivent des classes de sprites est très bizarre. Que la balle contiennent un sprite pour la représenter OK, qu'elle soit un sprite, c'est plus étrange.

Et meme le fait que la balle contienne un sprite, c'est très largement discutable, car la notion de sprite est une notion qui intervient dans un type de vue bien particulier, alors que la notion de balle en elle-même représente une donnée purement métier : c'est "quelque chose" qui se trouve, à un instant T à un endroit donné, mais qui se déplace dans n'importe quel sens et qui rebondit lorsqu'elle touche un obstacle.

Maiis tout ce que l'on doit savoir sur elle, en définitive, c'est sa position à un instant T

Quant à la notion de collision en elle-même, ce serait sans doute plutôt du ressort d'un controleur quelconque, qui se fout pas mal de savoir quelle forme peuvent prendre la balle ou les briques, mais qui doit juste pouvoir déterminer, quelle brique la balle touche, à un instant T, pour pouvoir mettre les données sur les briques à jour

[Alex__r]

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

Collision getCollision() const;

Cette solution semble en effet bien plus logique... Mais je me disais que c'était peu être plus rapide et moins lourd de passer par des réferences

-Pour cette histoire de sprite je comprend effectivement que faire hérité la classe de Sprite ne soit pas très correct... Il vaudrait mieux la mettre comme attribut de la classe tout simplement.

Et meme le fait que la balle contienne un sprite, c'est très largement discutable

Mais comment je peux lui appliquer mon Sprite donc? Il faudrait le créer dans le main?

[Bousk]

Dans l'absolu tu aurais une classe Balle qui contient tous les éléments de la balle (position, etc), et une BalleRendu qui se chargerait d'afficher la Balle.
En pratique tu peux embarquer le sprite dans Balle, ca facilitera les choses et tu changes rarement la méthode de rendu lors d'un projet. Tu peux aussi avoir un moteur de rendu qui propose une classe Sprite abstraite qui permet de changer le rendu sans toucher au Sprite dans Balle.

[Alex__r]

Je comprend et dans ce cas la il faudrait que la classe RenduBalle ai comme attribut une Balle pour pouvoir determiner sa position etc?

[Bousk]

Par exemple. Il peut aussi s'agir d'une seule fonction libre qui prend une Balle ou un tableau de Balle pour les afficher.

[Alex__r]

D'accord merci beaucoup pour toutes ces réponses et pour votre aide !