BASE/MOTEUR 3D EN QT/OPENGL (COMPLET ET FONCTIONNEL!) POUR UN TRÈS PROCHAIN JEU 3D

Commentaire de CptPingu le 25/07/2011 11:13:55 administrateur CS

Bonjour.

Le projet est sympa. J'aime bien l'idée de la minicarte. C'est un très bon début. Si tu veux faire un moteur, peut être qu'il serait pratique d'en faire une bibliothèque, en essayant de bien séparer C++/OpenGL et Qt ?

Je n'ai malheureusement pas encore pu compiler et tester cette source. En revanche, j'ai lu le code source, et j'ai quelques critiques à te proposer.
A noter que j'ai vais écrire beaucoup de choses négatives, mais ça ne va absolument pas dire que ton projet n'est pas bien. Bien au contraire. Pour progresser, il faut pointer ce qui ne va pas, pas ce qui va bien.

- Attention code mort (code en commentaire non utilisé)
- Des vrais commentaires en doxygen aurait été sympas (http://0217021.free.fr/portfolio/axel.berardino/articles/ecrire-de-bons-commentaires)
- Lorsque tu initialises des valeurs dans un constructeur de classe, utilise la liste d'initialisation plutôt que de mettre à jour les valeurs dans le corps de ton constructeur.
- Tu peux déclarer des variables où tu veux. Contrairement au C (non 98) tu peux déclarer une variable au milieu du code. Des fois tu utilises aussi des variables temporaires inutiles.
Ex: "distanceDroiteAuPoint" (réécriture)
float Coord3D::distanceDroiteAuPoint(const Coord3D& pointDeLaDroite, const Coord3D& point) const
{
  Coord3D vectDeuxPoints(pointDeLaDroite, point);
  Coord3D prod = crossProduct(vectDeuxPoints);
// On pourrait même mettre:
//  Coord3D prod = crossProduct(Coord3D(pointDeLaDroite, point));
  prod = prod - prod * 2;

  return prod.length() / this->length();
}

- Le "this->" n'est pas nécessaire. Il n'est nécessaire qu'en PHP (et souvent utilisé en Java), mais pas en C++.
- Lorsque tu passes en argument un type pod (int, float, char, etc...), inutile de mettre const devant, vu que la valeur est passée par copie. const float <=> float lors d'un passage d'argument.
- Par convention, on essaie de différencier une variable locale, d'un attribut. Il est d'usage d'ajouter un signe distinctif. (Par exemple: float _x; pour ta classe 3DCoord).
- Les membres en public, c'est moyen... Si tu fais un "getter" et un "setter" inlinés, tu gardes l'encapsulation sans perte de performance.
- Évite les "using namespace", voir: http://0217021.free.fr/portfolio/axel.berardino/articles/bon-usage-using-namespace
- Lorsque tu veux créer une énumération, n'utilise pas de #define ! Utilise un "enum" ! Normalement en C++, tu n'as jamais à utiliser un define. Si tu as besoin d'une valeur constante, on créé généralement une variable "static const" correctement typée.
- En génie logiciel, si tu créés une classe "Objet", c'est que tu as soucis de conception. Ici, tu ne fais heureusement pas tout hériter de cette classe. Le nom n'est en revanche pas très judicieusement choisie.
- vector< vector< float > > violent en terme de performance vu que tu connais la taille exact de tes tableaux. Deux solutions:
* Tu fais du C++ "normal" => utilise une classe templatée qui encapsule un tableau statique dont tu fixes la taille.
* Tu fais du C++0X => utilise un std::vector<std::array<float, 3>>
- vector.resize => Attention: resize change la taille externe du tableau, mais pas sa capacité. Ici, vu que tu ne le fais qu'au chargement, il n'y a pas de souci. Mais si tu fais la chose suivante:
- std::vector<float> tab;
tab.resize(1000);
// puis
tab.resize(1);
// Le tableau prend toujours de la place en mémoire.
Pour éviter cela, deux solutions:
* Tu fais du C++ "normal" => utilise le "swap trick" après ton resize: tab.swap(tab);
* Tu fais du C++0X => utilise tab.shrink_to_fit();

- Style: Les booléens ne s'utilisent pas comme des entiers:
* if (_isDead == true) => if (_isDead)
* if (_isDead == false) => if (!_isDead)

- Librairie => Bibliothèque (librairie est une traduction incorrecte de "library")

- if (_typeDeCamera == MINICARTE) [...] if (_typeDeCamera == VUEPRINCIPALE): Quand tu as des enum, utilise un switch plutôt que plein de if.
- gestionArme(Objet *listeObjets[], int nombreObjets) => C'est bizarre de déclarer utiliser un tableau d'Objet, pour n'en utiliser que le premier.
- Objet *liste_objets[50]; => Pourquoi 50 ? Si tu ne connais pas le nombre par avance, un tableau dynamique serait peut être adapté.

Commentaire de envi33 le 25/07/2011 20:14:11

@CptPingu
J'ai pu modifier tout le code (et apprendre pas mal de chose en passant) pour la prochaine version, mais je bute sur les vector :

- Pourquoi y a-t-il un soucis avec la déclaration :
vector< vector< float > > _tableauMesh;
Je pensais que cette ligne ne consommait rien en mémoire étant donné que je lui donne pas le nombre de case?

Commentaire de envi33 le 26/07/2011 10:49:28

Ok j'ai compris! Je continue les modifs!

Commentaire de opossum_farceur le 26/07/2011 17:07:17

Hi!

Compilation réussie avec Qt4.70+minGW sous Win7 32 bits.
Exécution ok!

Le code :
Mon intérêt s'est focalisé sur la méthode "OpenGLWidget::paintGL", qui est le centre opérationnel de toute application reposant sur QGLWidget.
Je vois que la scène est entièrement recalculée à chaque image : pour l'instant ton application ne fait pas grand chose, les ressources sont donc suffisantes, mais plus tard?
Les moyens d'optimiser ces calculs existent, l'un d'entre eux s'appelle " Vertex Buffer Objects" ou "VBO", dont la mise en oeuvre est rationalisée dans la classe de Qt "QGLBuffer".
Ces techniques d'optimisation reposent cependant sur l'existence de tableaux/vecteurs à une dimension.
Des déclarations telles que :
std::vector< std::vector< std::vector< float > > > _tableauMesh
sont donc satisfaisantes pour l'esprit mais impropres à un usage orienté "optimisation".
Sur ce sujet, un document récent à potasser :
http://gbelz.developpez.com/remi-achard/gpu-avance-avec-qt/
Ah, je vois que tu as créé une classe "Coord3D", c'est tout à ton honneur, mais connais-tu l'existence de la classe " QVector3D" ? (et sa fonction static "QVector3D::normal", qui fait des merveilles pour le calcul des normales).
Bonne continuation...

Commentaire de LeFauve42 le 01/08/2011 13:46:54

Bonjour,

J'ai survole une partie du code, tres clair et tres bien commente :o) mais j'ajoute ma petite remarque :
Dans ton whellEvent(), tu verifies que la nouvelle vitesse est entre 0 et 6 avant d'appeler joueur->setVitesse().
En toute rigueur, si tu respectes le paradigme de la programmation objet c'est dans setVitesse() que tu devrais faire ce test. C'est en effet l'objet joueur qui sait quelles vitesses sont valides ou pas pour lui (et si tu fais du polymorphysme avec plusieurs classes derivees de joueur (par exemple si tu peux jouer un bonhomme, un tank ou un avion), chacune peut avoir un comportement different sans que tu ais desoin d'adapter le code principal du programme).

J'ai aussi une petite remarque sur une des remarques de CptPingu :
> Lorsque tu passes en argument un type pod (int, float, char, etc...), inutile de mettre
> const devant, vu que la valeur est passée par copie. const float <=> float lors d'un
> passage d'argument.

Je ne suis pas d'accord. Const est surtout la pour faciliter la maintenance et la relecture du code. Quand je vois qu'une fonction prend un argument "const", je sais qu'il est constant, sans avoir a reflechir si dans ce language la il est de toutes facons passe en recopie ou pas.
Ca aide aussi si par erreur quelqu'un essaie de modifier cet argument dans la fonction.
Avec le const, ca va generer une erreur de compilation, et il sera donc facile de trouver/corriger l'erreur. Sans le const, ca va compiler correctement, mais la valeur ne sera pas modifiee dans la fonction appelante. D'ou un bug bizarre lors de l'execution qui a toutes les chances de passer inapercu jusqu'a la livraison :o)

Bref, si techniquement c'est vrai que ca ne change rien avec ces types, ce n'est pas inutile d'utiliser const quand meme.

Eric

Commentaire de tatarus le 04/08/2011 13:39:40

bonjour
Compilation OK

mais a lexecution :
     + Support OpenGL : OK
     ----->ERREUR 01 ; Chargement du fichier map : FAILED
     ..\Desktop\Envi3D-build-desktop\debug\Envi3D.exe s'est terminé avec le code -1073741819

Commentaire de Actares1456 le 15/08/2011 09:56:21

Bonjour envi33.
Enfait la question peut parraître bête mais j'ai aucunne idéée de ce que je dois mettre en".exe" et non plus comment le faire. Enfait je suis un débutant débutant mais je m'interesse beaucoup au millieux de la programation donc ça serait sympa que tu m'explique! Sinon j'ai jeté un oeil à tes codes. Je vais pas te cacher que la plupart des codes sont trops complex pour moi,mais de ce que je comprend, pour moi, c'est juste trop "balaize"!
Continue c'est extra!

Commentaire de Actares1456 le 16/08/2011 12:19:43

Merci enfiniment, si j'ai pris un peu de temps avant de te répondre c'est que j'était sur un projet. Enfait j'étrait entrain de concevoir un petit code en c++ et Managed pour un détécteur de mouvement avec la webcam. J'en suis encore qu'a la création du squelette mais comme je suis encore un débutant débutant, ça me prend un peu de temps pour trouver/écrire/... Si non j'ai regarder comme tu m'as dit et je trouve ça toujour trop "balaize"!XD