Une class Sprite

**FabaCoeur** · 21/04/2007, 21h48

Bonjour,

J'ai programmé une class Sprite pour gérer plus simplement les feuilles de sprite. La class fonctionne, et j'aimerai avoir votre avis pour la perfectionner si besoin est et pour savoir si ce que j'ai fait est correct (et pourquoi pas la mettre en source par la suite).

Voila le code:
- sprite.h
- sprite.cpp

Un modèle de fichier de configuration:
- sprites/spr_perso.txt

Explications : Les grandes lignes
La class contient :
- une surface SDL (sur laquelle on va charger l'image par la suite)
- un pointeur vers une couleur transparente
- un ensemble de couple string/SDL_Rect (autrement dit une map de STL)

Ainsi une animation est un couple "nom de l'animation" de type string et un ensemble de coordonnées (x, y, w et h) stockée dans une structure SDL_Rect. J'ai codé les principaux accèsseurs (même s'il ne serviront presque jamais) tandis que le constructeur par recopie est vide (je ne pense pas qu'il soit possible à faire à cause du pointeur sur la SDL_Surface*, quoiqu'il en soit je ne pense pas qu'il soit nécessaire de recopier une feuille de sprite mais cela risque d'être problématique pour l'utilisation de feuille de sprite en tant qu'argument de fonction).

Les méthodes principales:
- charger(string fichier) : charge le fichier image sur la surface
- definir_coul_trans(r,v,b) : définit une couleur transparente
- ajouter() : permet d'ajouter un couple animation/coordonnées. La fonction est surchargée pour permettre une souplesse d'utilisation.
- blitter(surface, nom_anim, x, y) : blitte un sprite sur la surface

Class FeuilleSprite : Extension de la class
J'ai codé aussi une class FeuilleSprite qui hérite de Sprite.
Je l'ai ajouté pour pouvoir gérer plus simplement la définition des animations; ainsi on va aller charger les infos du sprite et les animations (nom et coordonnées) dans un fichier texte externe (c'est là qu'intervient le sprites/spr_perso.txt). J'ai fait en sorte de pouvoir commenter le fichier texte.

La class contient une liste chainée d'erreur (au format string) c'est pourquoi j'ai ajouté deux accèsseurs pour lire les messages par la suite (et un alias de l'iterateur). Ce système de message d'erreur est assez lourd à gérer, si vous avez une meilleur idée je suis preneur !

Sinon la class ne contient qu'une méthode en plus: charger_feuille(string chemin) qui permet d'aller lire le fichier .txt pour en extraire les informations et ensuite définir entièrement le Sprite.

Merci à tous ceux qui auront pris la peine de lire mes sources

**fearyourself** · 23/04/2007, 10h59

Bonjour voici mes remarques :

le constructeur par recopie est vide (je ne pense pas qu'il soit possible à faire à cause du pointeur sur la SDL_Surface*, quoiqu'il en soit je ne pense pas qu'il soit nécessaire de recopier une feuille de sprite mais cela risque d'être problématique pour l'utilisation de feuille de sprite en tant qu'argument de fonction).

- Si on pourrait créer une nouvelle surface et faire la copie mais c'est un gaspillage mémoire.
- Copier simplement le pointeur en ajoutant un booléen pour savoir qui doit libérer la feuille de style -> dangereux, faut que la désallocation se fasse dans le bon ordre.
- Ajouter un gestionnaire de ressources qui centralise le tout et qui gére l'allocation/désallocation des ressources...

- Je ne vois pas pourquoi mettre la couleur transparente en tant que membre de la classe Sprite ou même pourquoi la mettre en pointeur...
Du coup, ta fonction get_coul_trans est dangereuse, tu ne testes pas si ton pointeur est NULL.

- Pas de test après SDL_DisplayFormat.

Code :

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

1
2
3
4
5
6
7
8
// Indique si on a définit une couleur transparente
bool Sprite :: a_coul_trans() {
 
	if( this->couleur_clee != NULL)
		return true;
	else
		return false;
}

Peut être simplifié en :

Code :

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

1
2
3
4
5
6
 
// Indique si on a définit une couleur transparente
bool Sprite :: a_coul_trans() {
 
	return (this->couleur_clee != NULL);
}

ou encore :

Code :

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

1
2
3
4
5
6
 
// Indique si on a définit une couleur transparente
bool Sprite :: a_coul_trans() {
 
	return (couleur_clee != NULL);
}

Code :

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

1
2
3
 
	// On mémorise la couleur
	this->couleur_clee = new SDL_Color;

Sympa la fuite de mémoire

Personnellement, je ne vois pas pourquoi stocker cette valeur, cela ne devrait regarder personne quelle est la couleur transparente sauf la fonction de blit. Or, on stocke la valeur de la couleur transparente grâce à la fonction SDL_SetColorKey...

Code :

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

1
2
3
 
if( iter != this->coord.end() ) return true;
	else return false;

peut devenir :

Code :

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

1
2
 
return (iter != coord.end());

Code :

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

1
2
3
4
5
// Supprimer un couple identifiant/coordonnée
bool Sprite :: supprimer(string id) {
 
	if( !this->existe(id) )
		return false;

Pas la peine de tester l'existence de l'élément, au pire la suppression ne fera rien.

Code :

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

1
2
 
	nbanim = atoi( s.c_str() );

beurk atoi, déjà en C on n'utilise pas atoi alors en C++...
http://c.developpez.com/faq/cpp/?pag...RINGS_strtonum

La class contient une liste chainée d'erreur (au format string) c'est pourquoi j'ai ajouté deux accèsseurs pour lire les messages par la suite (et un alias de l'iterateur). Ce système de message d'erreur est assez lourd à gérer, si vous avez une meilleur idée je suis preneur !

Je ne vois pas vraiment mieux. J'ai une tendance de sortir le plus proprement possible à la première erreur que je vois pas à faire un iterateur comme toi. Question de goût je pense.

Attention tout de même :
- Ta solution est sympathique mais est-elle vriament pratique. Lorsqu'on va vouloir gérer les animations, comment passer d'une animation à l'autre ? S'il faut recréer la chaîne clé à chaque image, cela ne va pas être pratique.

- Limiter le nombre de recherche dans ta map. Ta fonction existe est pratique dans le code de debuggage mais généralement, ce sera plus intelligent de simplement chercher l'élément est le comparer à l'élément end...

Jc

**Laurent Gomila** · 23/04/2007, 11h14

Si on veut autoriser la copie, le mieux pour ce genre de classes est de séparer en deux : une classe interne "implémentation", et une classe publique qui l'encapsule avec un pointeur intelligent à base de comptage de référence.

Si la copie n'a aucun sens ou est déconseillée, il faut l'interdire en mettant le constructeur par copie et l'opérateur d'affectation en accès privé et ne pas les implémenter.

Autre chose : je trouve l'utilisation de l'héritage pas très appropriée pour l'objectif recherché. Mieux vaut faire une fonction non membre qui prend en paramètre le nom du fichier et qui renvoie un sprite. Ou au pire une classe s'il y a des traitements complexes à faire et des données à stocker, mais pas dérivée de sprite.

**FabaCoeur** · 24/04/2007, 21h05

Tout d'abord, merci de vos réponse.
Attaquons le vif du sujet !

Envoyé par fearyourself

Si on pourrait créer une nouvelle surface et faire la copie mais c'est un gaspillage mémoire.

Je retiendrais cette solution comme la meilleur. Quoiqu'il en soit, la recopie d'un objet de type Sprite n'a vraiment presque aucun interêt de toute façon, en réalité seulement lors de l'appel à une fonction prenant un Sprite en paramètre quoi.

Je ne voie pas comment réaliser cette solution, peut-être faire une copie de surface ?
Puisque nous utilisons des SDL_Surface*, faire sprite1.surface = sprite2.surface reviendrait à copier le pointeur (et donc la libération d'une surface de mémoire renderait le second objet non-intègre).

Pour ce qui est de la couleur transparente, en réalité je réalise que c'est un mauvais choix de la conserver. A la base je l'ai ajouté dans la class pour que l'on puisse, à tout moment, connaitre la couleur de la feuille ce qui n'est en réalité pas nécessaire. J'avais aussi penser pouvoir changer de couleur et de feuille, mais à ce compte là autant créer un nouvel objet... il n'est pas pas du tout nécessaire de la stocker en réalité.

En revanche, comment supprimer la couleur transparente d'une surface ?
Il y a-t-il une fonction inverse à SDL_SetColorKey() ?

Je vais améliorer un peu le code avec les simplifications, et surtout ne plus utiliser de atoi() et itoa() car apparement c'est vraiment beurk, erf !

Je pense aussi que la liste chainée d'erreur est un bon modèle, puisque ca permet de garder les erreurs dans l'ordre sur lequelle elles sont arrivées et d'en avoir plusieur.
Comme tu l'as dit c'est une question de gout après, mais si une erreur arrive puisqu'il est impossible de définir la couleur transparente (ce qui n'altère en rien le déroulement du programme) je préfère continuer. Et puis, l'utilisateur verra bien qu'il y a un problème !

Le gros problème c'est que le traitement derrière est assez lourd... utiliser un itérateur déjà, puis ajouter deux accèsseurs c'est pas top.

Ta solution est sympathique mais est-elle vriament pratique. Lorsqu'on va vouloir gérer les animations, comment passer d'une animation à l'autre ? S'il faut recréer la chaîne clé à chaque image, cela ne va pas être pratique.

A la base j'ai créer cette classe pour simplifier un peu ma class de type Personnage justement. Je préférais manier des noms d'animations en string, plutot que de constantes provenant de listes enum{} ou autres.
Lors d'une animation (plusieurs images) l'utilisation ressemble à:

Code :

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

1
2
3
4
5
6
7
8
switch( /* vecteur vitesse */ ) {
    case ...: anim = "haut"
    case ...: anim = "bas"
}
 
if( /* gestion des animations par exemple */ ) {
   anim += framecounter;
}

Ainsi je retombe sur le nom de l'animation haut0, haut1,... je trouve cela assez parlant à la relecture du code. Mais l'utilisation de string est parfois un peu plus lourde à gérer.

Envoyé par fearyourself

Limiter le nombre de recherche dans ta map. Ta fonction existe est pratique dans le code de debuggage mais généralement, ce sera plus intelligent de simplement chercher l'élément est le comparer à l'élément end...

Je n'ai pas bien compris sur ce point.
Puis la fonction existe n'apparait que très peu non ?

Envoyé par Laurent Gomila

Si la copie n'a aucun sens ou est déconseillée, il faut l'interdire en mettant le constructeur par copie et l'opérateur d'affectation en accès privé et ne pas les implémenter.

Mais si un developpeur veut utiliser ma class et créer une fonction qui la prends en paramètre, il y aura forcement une copie non? Du coup si je ne l'autorise pas, il sera impossible de passer un Sprite en argument (à moins de le passer par référence avec &), je me trompe ?

Envoyé par Laurent Gomila

Autre chose : je trouve l'utilisation de l'héritage pas très appropriée pour l'objectif recherché. Mieux vaut faire une fonction non membre qui prend en paramètre le nom du fichier et qui renvoie un sprite. Ou au pire une classe s'il y a des traitements complexes à faire et des données à stocker, mais pas dérivée de sprite.

J'ai fait une class héritée FeuilleSprite pour séparer en deux les deux class; la première plus générale et la seconde plus ciblée. En effet, dans la seconde la structure du fichier que l'on va charger est fixe et déterminée et à moins de mettre des méthodes pour gérer le format du fichier (ce qui peut sembler assez compliqué!) j'ai préféré faire une autre class, utilisant un schéma de donnée fixe (nom de l'image;couleur transparentes;animations).

**fearyourself** · 24/04/2007, 21h24

Envoyé par FabaCoeur

Je ne voie pas comment réaliser cette solution, peut-être faire une copie de surface ?

Il faut créer une surface avec SDL_CreateRGBSurfaceFrom de même taille et copier les pixels... Par contre, pour vraiment faire une copie, il faudrait utiliser la couleur pour la transparence.

Pour ce qui est de la couleur transparente, en réalité je réalise que c'est un mauvais choix de la conserver. A la base je l'ai ajouté dans la class pour que l'on puisse, à tout moment, connaitre la couleur de la feuille ce qui n'est en réalité pas nécessaire. J'avais aussi penser pouvoir changer de couleur et de feuille, mais à ce compte là autant créer un nouvel objet... il n'est pas pas du tout nécessaire de la stocker en réalité.

Donc en effet, si on veut faire une copie, faudrait garder la couleur de la transparence.

En revanche, comment supprimer la couleur transparente d'une surface ?
Il y a-t-il une fonction inverse à SDL_SetColorKey() ?

Il suffit de passer 0 au deuxième paramètre.

Je pense aussi que la liste chainée d'erreur est un bon modèle, puisque ca permet de garder les erreurs dans l'ordre sur lequelle elles sont arrivées et d'en avoir plusieur.
Comme tu l'as dit c'est une question de gout après, mais si une erreur arrive puisqu'il est impossible de définir la couleur transparente (ce qui n'altère en rien le déroulement du programme) je préfère continuer. Et puis, l'utilisateur verra bien qu'il y a un problème !

Le gros problème c'est que le traitement derrière est assez lourd... utiliser un itérateur déjà, puis ajouter deux accèsseurs c'est pas top.

Suffit de tout internaliser pour ne pas laisser l'utilisateur s'en servir et laisser juste une fonction pour afficher les erreurs.

A la base j'ai créer cette classe pour simplifier un peu ma class de type Personnage justement. Je préférais manier des noms d'animations en string, plutot que de constantes provenant de listes enum{} ou autres.
Lors d'une animation (plusieurs images) l'utilisation ressemble à:

Code :

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

1
2
3
4
5
6
7
8
switch( /* vecteur vitesse */ ) {
    case ...: anim = "haut"
    case ...: anim = "bas"
}
 
if( /* gestion des animations par exemple */ ) {
   anim += framecounter;
}

Ainsi je retombe sur le nom de l'animation haut0, haut1,... je trouve cela assez parlant à la relecture du code. Mais l'utilisation de string est parfois un peu plus lourde à gérer.

Oui ce sera largement plus long qu'utiliser directement des entiers. Si tu veux garder cette solution, utilise au moins un tableau directement qui donnera le string correspondant.

Je n'ai pas bien compris sur ce point.
Puis la fonction existe n'apparait que très peu non ?

Apparaît très peu mais dans des fonctions qui seront appelées souvent.

Mais si un developpeur veut utiliser ma class et créer une fonction qui la prends en paramètre, il y aura forcement une copie non? Du coup si je ne l'autorise pas, il sera impossible de passer un Sprite en argument (à moins de le passer par référence avec &), je me trompe ?

Justement, il ne faut jamais passer directement la classe mais passer une référence ou le pointeur.

Jc

**Laurent Gomila** · 24/04/2007, 21h38

Mais si un developpeur veut utiliser ma class et créer une fonction qui la prends en paramètre, il y aura forcement une copie non? Du coup si je ne l'autorise pas, il sera impossible de passer un Sprite en argument (à moins de le passer par référence avec &), je me trompe ?

D'où l'intérêt d'avoir interdit la copie, comme ça il sera obligé de passer par référence ou pointeur comme dit par JC, au lieu de faire plein de copies lourdingues silencieusement

J'ai fait une class héritée FeuilleSprite pour séparer en deux les deux class; la première plus générale et la seconde plus ciblée. En effet, dans la seconde la structure du fichier que l'on va charger est fixe et déterminée et à moins de mettre des méthodes pour gérer le format du fichier (ce qui peut sembler assez compliqué!) j'ai préféré faire une autre class, utilisant un schéma de donnée fixe (nom de l'image;couleur transparentes;animations).

Tout ce que fait ta classe dérivée c'est créer un sprite d'une manière particulière. Mais une fois créé, c'est juste un sprite. Il n'y a rien qui justifie que l'instance se trimballe en tant que FeuilleSprite tout au long du programme.

Une class Sprite

SDL

Vue hybride

Discussions similaires

Partager

Partager