Bonjour, comme demandé par godrik ( https://www.jeuxvideo.com/forums/1-47-46404-44-0-1-0-printf-blabla.htm#message_62359 ) je viens poster ici ma demande.
"Bonjour, étant débutant en programmation (j'ai commencé en septembre 2011) je voudrais savoir si quelqu'un pourrait m'aider à corriger mon code (enfin juste me dire ce qui est mal fait, ce qu'il faut changer, après je pourrais chercher moi-même comment faire).
En fait, j'ai fais un petit jeu avec la sdl en c++, un jeu de combat un contre un (sans ordi) et j'aimerais bien savoir quelles sont les erreurs que j'ai fais et comment l'améliorer.
Merci d'avance de m'aider, et désolé si c'est mal venu comme demande, mais j'aimerais bien avoir quelque chose de propre. "
Lien de l'archive avec tout les fichiers sources : http://dl.dropbox.com/u/40795884/Autre/SourceRedVsBlue.rar (j'ai pas mis les dll et les images du jeu).
En ajoutant les commentaires j'ai eu quelques idées pour améliorer le code, elles sont écrites sous forme de commentaire, j'ai pas changé le code source (pour avoir un avis dessus).
Bon, je le redis mais merci d'avance à ceux qui m’aideront.
Le plus simple pour partager ton code serait de le poster sur un truc genre http://pastebin.com/
Le truc c'est qu'il y
Le truc c'est qu'il y a plusieurs fichiers, enfin si ça dérange vraiment je le fais.
Désolé pour le post raté au-dessus.
Si il y a des images et des dlls, alors je prefere une archive histoire que l'on voit ce qui se passe.
ArgentComptant, je vais jetter un oeil.
http://dl.dropbox.com/u/40795884/Autre/ExeRedVsBlue.rar
DLL, images, polices et .exe, pour windows.
Je précise juste que c'est moi.
Je veux bien la même mais sans les dll et surtout pas les exe. Juste le code ![]()
Bah l'archive contenant juste le code est dans mon 1er post.
Je viens de regarder le code, voici donc mes remarques.
La premiere chose que j'ai vu c'est l'absence de test de controle. Tu ne test pas les infos que te retrounent les appels a, par exmple, SDL_SetVideoMode ou TTF_OpenFont. A la moindre tuile ou fichier manquant c'est le segfault ![]()
Ensuite, l'abence d'information dans les entetes est assez genant. Le mieux est de decrire les méthodes, les parametres d'entree et de sortie dans l'entete facon Doxygen : http://www.stack.nl/~dimidimitri/doxygen/docblocks.html
Il y a également la boucle principale qui m'a fait tiquer, pourquoi ne pas utiliser quelques usleep de courtes durées le temps de tes 40 ms ?
Enfin, j'ai vu beaucoups de constante en dur dans le code. Bon, pour un mini projet c'est pas tres genant, mais perso j'aime bien regrouper mes constantes pour les retrouver rapidement en cas de modif (le plus souvent les sortirs en config).
Salut. Desole je n'avais pas vu le premier post. Tres rapidemment car je suis au boulot.
D'une petite remarque : le francais c'est tabou on en viendra tous a bout. Les commentaires passent encore mais ecrire du code en anglais de A a Z c'est beaucoup plus conseille. D'une parce que des que tu as un probleme tu peux poster ton code sur n'importe quel forum du monde, de deux parce que l'anglais est generalement plus pratique comme grammaire pour le code et la concision, de trois car c'est un peu plus harmonieux avec les langages qui sont generalement tous deja en anglais de toute maniere... parce que setProfondeurDeChamp c'est quand meme pas tres joli joli :D
Deuxieme remarque. Je suis assez connu pour etre anti-commentatisation. Pour moi un commentaire ca ne doit pas etre un reflexe, des methodes et des noms self-explanatory ca par contre ce doit. Ya des commentaires totalement inutiles (genre "boucle", "variable globale") osef. Un commentaire tu le mets dans une partie sensible, un truc change suite a un gros bug qui indique bien pourquoi tu fais ca, ou oui en chaque definition de classe c'est pas mal de definir le propos de la classe.
De trois, grossierement ce que j'avais ecrit un jour ici s'applique a ton code donc je te renvoies en parfait flemmard directement a : https://www.jeuxvideo.com/forums/1-31-8600925-1-0-1-0-sfml-probleme-modification-de-tableau.htm#message_8600939
Bonne continuation et bienvenu sur le forum !
+1 pour le hardcodage itinerant. C'est bien de ne pas avoir de magic numbers ici et la dans du code.
Bon, je voulais prendre le temps de faire une review complète, mais je me sens obligé de faire quelques remarques maintenant (pour le cas où une autre archive verrais le jour) :
1/ En C++, les fichiers header ont une extension en .H ou en .hpp.
2/ Pour éviter les problèmes de compilation, il vaut mieux inclure (par exemple) <SDL/SDL.h> plutôt que juste <SDL.h>. En effet, le fichier SDL.h et ses compains sont tous placés dans un dossier SDL qui lui est (au moins sous Linux/BSD/Mac) dans un répertoire standard.
3/ Un Makefile, ou à défaut un fichier pour dire dans quel ordre compiler les fichiers, ce n'est jamais du luxe.
(c'est peut-être dans la deuxième archive, moi j'ai récupéré seulement la première)
4/ clang++ me donne de nombreux warning à la compilation. Tu sembles comparer des entiers de tailles différentes, ce qui est parfois (souvent) synonyme d'une comparaison entre un indice de boucle et une case d'un tableau. À voir. De même, tu dois faire un switch non exhaustif et sans cas par défaut à un endroit. Dans tous les cas, ajouter des options de compilation (-Wall -Wextra) pour avoir des avertissement n'est pas un luxe.
PS: je n'ai pas ouvert les fichiers de code (sauf pour corriger les points 1/ et 2/ ). Je donne donc ici simplement mon retour après compilation du code.
Je ne suis pas tout a fait d'accord avec toi Chris sur les points 1 et 2.
Concernant les extensions, ainsi que le nom des fichiers, je ne peux que recommander de tout mettre en minuscule. Mettre des majuscule dans les noms de fichiers entraîne parfois des problèmes de portabilité.
Par exemple, il y a une erreur de compilation sous linux car <SDL_Image.h> est utilisé au lieu de <SDL_image.h>.
Je déconseille également d'utiliser <SDl/xxx>, ainsi que pour tout les autres libs. Utiliser `sdl-config --cflags` dans les options de compilations (et `sdl-config --libs` pour le linkage) on s’évite bien des ennuis dans le futur.
Je rejoins Chris sur l'importance d'inclure un makefile dans les sources, ainsi que l'utilisation intessive des -Wall --Wextra. Cela aurait permis de voir tout de suite les const manquant, les erreur d'initialisations et autres.
« Mettre des majuscule dans les noms de fichiers entraîne parfois des problèmes de portabilité. »
déjà, on n'est plus en 1970 (la portabilité, ok, mais il ne faut pas pousser non plus). Mais surtout, elles sont où les majuscules dans .hpp ?
Pour sdl-config, tu ne fais que déporter le problème. La solution pkg-config (parce que sdl-config c'est rien qu'un vieux script pour imiter mollement pkg-config) apporte son lot de problèmes :
et si j'ai plusieurs versions de la SDL ? (et ça fait parfaitement sens d'avoir la SDL v1.2 et v2.0)
et si je veux/je dois changer quelque chose par rapports aux flags fournis ?
pire, et si ça te donne un flag qui est incompatible avec une autre lib ?
Bref, moi j'aime bien fixer les choses moi-même. J'aurais pu mettre le -I/usr/include/SDL qui manquait, mais dans ce genre de cas je préfère mettre le path complet du fichier tel qu'il est fourni par la bibliothèque (ce qui en prime permet d'éviter les conflits... bien qu'il faille le vouloir pour avoir un autre SDL.h ailleurs dans le include path
).
Par contre, tu as raison pour SDL_Image. Il faut bien changer la casse ici. Après, l'OP n'y peut rien sur ce coup là. ![]()
"et ça fait parfaitement sens d'avoir la SDL v1.2 et v2.0"
C'est terrible quand on pense à une expression en anglais alors qu'on écrit en français ![]()
En se qui concerne les majuscules, ce n'est pas une question d'époque, mais d'OS différencient ou non la casse. Je réagissait au ".H". Si on peut assurer la portabilité a moins frais, autant le faire.
Je fais référence a sdl-config car il vient avec le package, simple d'utilisation et qu'il donne le bon chemin. Solution simple, efficace et fourni avec la lib.
Apres, si tu as envi de bidouiller, ça empêche rien hein
Mettre les chemins en dur c'est une très très très mauvaise habitude. Forcement, sur ce projet, ca va juste faire echouer la compile sur les gens chez qui la lib est dans /usr/local, mais je trouve que c'est bien d'avoir l'habitude de faire la le flexible. J'ai perdu de nombreuses heures à remettre d'aplomb des makefiles avec des chemins en dur, maintenant je suis psycho-rigide la dessus.
Et j'ajouterai :
http://www.stroustrup.com.com/Programming/PPP-style.pdf
Aldebran: c'est encore plus terrible de se faire traiter d'anglissiste parce qu'on a utilisé une expression on ne peut plus française.
L'expression incorrecte que des gens utilise parfois, c'est "faire du sens", triste mixte entre "faire sens" et "avoir du sens".
KouicKouic: « Mettre les chemins en dur c'est une très très très mauvaise habitude. »
s'il t'arrive de mettre #!/bin/whatever au début d'un script, tu n'es pas en droit de me faire cette remarque.
En vrai, je considère que ce n'est pas la même chose ici dans la mesure où le chemin, ce n'est pas la partie "SDL/SDL.h" qui est ce que te fournira la librairie ***quelque soit l'endroit où elle installe ses fichiers***, mais la partie "/usr/include" (enfin, plutôt $LIBDIR dans le Makefile même).
Joker pour le #!bin/truc, c'est pas vraiment le même problème
Si on a, par exemple, la version 1.2 et 2.0 de SDL. Nous devrions inclure <SDL/SDL.h> pour la 1.2 ou <SDL2/SDL.h>.
Je trouve que la solution la plus propre et simple est d'inclure juste <SDL> et de passer le bon chemin avec -I.
Oula, fatigué moi... donc, inclure <SDL2/SDL.h> pour la 2.0... <SDL/SDL.h> pour la 1.2... Ou juste <SDL.h> et passer le chemin avec flag -I grâce a, SDL-config ou SDL2-config, mais la encore il faut choisir
http://forums.libsdl.org/viewtopic.php?t=5997&sid=73840727929628eced1704a39bdc7fba
Ouais... et dès que ça parle de Mac OS X, on voit ce genre de réponse :
« Solution A) Create your own sdl-config. It's really simple.
Solution B) Write a little more code to include the necessary headers. Here's how I accomplished that:
»
ou encore
« as mentioned by others, #include "SDL.h" will work for most cases on most platforms. »
Ça fait rêver.
Le seul problème de "SDL/SDL.h" et que ça cassera le jour où ils changent le nom du dossier. D'un autre coté, c'est sans doute une bonne idée que ça casse ce jour là car les changements dans la lib seront tels que le code ne compilera pas de toute façon. Autant être prévenu d'emblée. ![]()