Discussion :

[AbsoluteLogic]

Bonjour,

Je fais face aux joies du c++ avec des problèmes que je considère comme des "effets de bord". En effet une variable change de valeur sans raison. Je fais donc du débogage pas à pas avec gdb, en suivant la valeur d'une de mes variables "FILE* file" (Je sais que je devrais utiliser fstream en c++, mais j'obtiens le même problème, j'ai donc utilisé FILE* pour le mettre en évidence). En déboguant avec gdb, j'obtiens:
- initialisation de file dans le constructeur d'une classe Replay:

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
Breakpoint 1, Replay::Replay (this=0x7ffffffed9d8, pixels_per_unit=32, width=24, height=24)
    at ../SimplePvP/Replay.cpp:38
38      Replay::Replay(int pixels_per_unit, float width, float height)
(gdb) next
40              file = fopen("0.replay", "wb");
(gdb) next
42              WriteUint16(pixels_per_unit);
(gdb) p file
$16 = (FILE *) 0x84d2c20
(gdb) p &file
$17 = (FILE **) 0x7ffffffed9e0
(gdb) p *(FILE**)0x7ffffffed9e0
$18 = (FILE *) 0x84d2c20
(gdb) next
43              WriteFloat32(width);
(gdb) next
44              WriteFloat32(height);
(gdb) next
45      }

- bazar dans la classe de base dont file est l'attribut:

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
(gdb) next
GameManager::GameManager (this=0x7ffffffed9c0, nb_players_=3, pixels_per_unit=32, width=24, height=24)
    at ../SimplePvP/GameManager.cpp:10
10              unsigned int seed = time(NULL);
(gdb) next
11              srand(seed);
(gdb) next
12              std::cout << "Seed: " << seed << std::endl;
(gdb) next
Seed: 1567342599
13              turn = -1;
(gdb) next
14              current_player = -1;
(gdb) p *(FILE**)0x7ffffffed9e0
$19 = (FILE *) 0x84d2c20
(gdb) next
15              scores = new int[nb_players];
(gdb) next
16              for (int playerId = 0; playerId < nb_players; playerId++)
(gdb) next
17                      scores[playerId] = 1;
(gdb) next
16              for (int playerId = 0; playerId < nb_players; playerId++)
(gdb) next
17                      scores[playerId] = 1;
(gdb) next
16              for (int playerId = 0; playerId < nb_players; playerId++)
(gdb) next
17                      scores[playerId] = 1;
(gdb) next
16              for (int playerId = 0; playerId < nb_players; playerId++)
(gdb) next
18      }
(gdb) p *(FILE**)0x7ffffffed9e0
$20 = (FILE *) 0x84d2c20

- et dès que je quitte le constructeur de la classe de base (GameManager) pour revenir dans le constructeur de la classe fille qui l'avait appelée (SimplePvPGameManager), le pointeur change par magie:

Code :

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

1
2
3
4
5
(gdb) next
SimplePvPGameManager::SimplePvPGameManager (this=0x7ffffffed9c0) at ../SimplePvP/SimplePvPGameManager.cpp:27
27              for (int i = 0; i < NB_WALLS; i++) {
(gdb) p *(FILE**)0x7ffffffed9e0
$21 = (FILE *) 0x60000000a

Sachant que ces trois parties s'enchaînent, et qu'il ne s'est donc rien passé entre, auriez-vous une idée d'où peut venir un tel problème? (Vous me redonneriez la foi dans le c++ si vous dites que c'est ma faute, mais pour le moment, c'est mieux le c# )

Merci d'avance pour votre expertise!


EDIT: Je mis ici en annexe le code correspondant au débogage, si vous souhaitez avoir une vue d'ensemble:

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
Replay::Replay(int pixels_per_unit, float width, float height)
{
	file = fopen("0.replay", "wb");
	//file.open("0.replay", ios::binary | ios::out);
	WriteUint16(pixels_per_unit);
	WriteFloat32(width);
	WriteFloat32(height);
}
 
GameManager::GameManager(int nb_players_, int pixels_per_unit, float width, float height) : 
	nb_players(nb_players_),
	replay(pixels_per_unit, width, height)
{
	unsigned int seed = time(NULL);
	srand(seed);
	std::cout << "Seed: " << seed << std::endl;
	turn = -1;
	current_player = -1;
	scores = new int[nb_players];
	for (int playerId = 0; playerId < nb_players; playerId++)
		scores[playerId] = 1;
}
 
SimplePvPGameManager::SimplePvPGameManager() : GameManager(NB_PLAYERS, 32, SIZE_MAP, SIZE_MAP)
{
    ...
}

(Dommage qu'il n'y ait pas la possibilité de créer de spoiler pour cacher les choses facultatives, et ainsi moins encombrer les posts...)

[jo_link_noir]

Caster des adresses en FILE n'a aucun sens et ta liste d'initialisation est foireuse: l'initialisation doit se faire dans le même ordre que la déclaration et les classes héritées sont avant n'importe quel membre. En plus, la moitié des membres ne sont pas initialisés, mais modifier dans le corps de la fonction, l'ensemble n'est pas cohérent.

Perso, avant d'utiliser le moindre débuggeur, je passe par les sanitizers: -fsanitize=address au minimum et éventuellement valgrind.

Pour le code en lui-même:

- score devrait être un std::vector, nb_score ne devrait pas exister
- l'utilisation de rand/srand en plein milieu d'un constructeur est en mon sens bien foireux. C'est intestable et le C++ offre des générateurs de nombre aléatoire qui ne dépendent pas d'état global.

[koala01]

Salut,

Dejà, il y a un fameux problème avec ta classe Replay, car son constructeur veut automatiquement écrire des données, du moins, si on s'en tient au nom des fonctions que le constructeur appelle

Or, si l'on sen tient au nom de ta classe (Replay) et au contexte dans lequel elle est utilisée -- le constructeur de GameManager (Quel horrible nom pour une classe !! -- on se rend compte qu'il y a beaucoup plus de chance pour que, ce qu'elle doive faire au moment de sa création soit plutot de la lecture,(appel de fonctions "ReadXXX) de manière à pouvoir ... charger des informations précédemment enregistrées.

Pire encore (si j'ai tiré les bonnes conclusions sur base du nom de cette classe "Replay") : on se rend compte que GameManager ne devrait -- a priori -- faire appel à une instance de cette classe qu'à quelques moments clés tels que:

• au lancement du programme (s'il souhaite charger automatiquement "la dernière sauvegarde")
• juste avant de quitter le programme (s'il souhaite sauvegarder automatiquement le dernier état du jeu)
• à quelques moments choisis par le joueur pour effectuer une sauvegarde "de sécurité" (des fois que, par la suite, la partie soit mal engagée, pour pouvoir reprendre "sur de bonnes bases")
• à quelques moments choisis par le joueur pour charger la sauvegarde "de sécurié" (quand, justement la partie est mal engagée, et qu'il souhaite reprendre sur "de bonne bases")

Le mieux de l'histoire étant que le point (1) s'accorde tout à fait avec le point (4) et que le point (2) s'accorde lui tout à fait avec le point (3), et qu'il ne sera donc même pas nécessaire de dupliquer du code

Mais, l'important, c'est que, en dehors de ces périodes bien particulières, il n'y a absolument aucune raison de maintenir une instance de Replay en mémoire (et donc, d'en faire une donnée membre de la classe GameManager ).

Un code proche de

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
GameManager::GameManager(/* paramètres */):/*initialisation des membres */{
    LoadGame();
    /* le reste du constructeur */
}
GameManager::~GameManager(){
    SaveGame();
}
void GameManager::LoadGame(){
    /* nettoyage éventuel des données de la partie "abandonnée" */
    Replay r{/* paramètres nécessaire*/ };
    r.Load(/* paramètres spécifiques */);
}
void GameManager::SaveGame(){
    Replay r{/* paramètres nécessaire*/ };
    r.Save(/* paramètres spécifiques */);
}

faisant parfaitement l'affaire

De même, il n'y a absolument aucune raison de vouloir déclarer un membre (de type fstream "simple", qui plus est) au niveau de la classe Replay : en ouvrant un fichier d'entrée (std::ifstream) dans la fonction Load (ou Read, comme tu préfères), qui le transmettra (sous forme de référence, bien sur) à toutes les fonctions ReadXXX spécifiques ou un fichier de sortie (std::ofstream) dans la fonction Save (ou Write, comme tu préfères) qui le transmettra (sous forme de référence, toujours) à toutes les fonctions WriteXXX spécifiques, tu n'auras plus aucun problème

[AbsoluteLogic]

Merci pour ta réponse rapide. J'avais déjà essayé Valgrind, mais il signalait seulement qu'il y avait une corruption lorsque j'essayais d'écrire dans le fichier dont le pointeur était modifié. Pas comment le pointeur a été modifié. Bref, j'ai donc maintenant utilisé -fsanitize=address comme tu me l'as suggéré (que je ne connaissais pas), et je ne sais pas quoi dire:
Premièrement, le programme compilé avec cette option fonctionne, plus de changement de pointeur. J'ai cru comprendre que cette option servait à détecter les erreurs de mémoire, pas à les corriger . Bon, je peux comprendre que le programme doit gérer la mémoire différemment avec cette option, et que de cette manière, l'erreur n'arrive plus. Au passage, j'obtenais l'erreur sous Linux, mais sous Windows, ça fonctionnait. Je n'ai pas ressenti le besoin de le dire puisque je me suis dit que c'est juste que Windows gère la mémoire d'une manière faisant que "par hasard", mes effets de bord n'ont pas fait de dégâts.
Ensuite, j'ai donc de nouveau retiré l'option sanitize pour retrouver mon erreur, et contre toute attente, ça fonctionne toujours! Là, je ne vois pas comment l'expliquer...

Bon, donc d'une certaine manière, le problème est réglé. Mais je laisse le sujet ouvert par peur que le bug revienne par la même magie qui a fait qu'il est parti. Je vais donc faire les améliorations que tu m'as suggéré, et si le bug revient avec ses améliorations, je me remanifesterai.

Donc merci beaucoup pour ton aide!

[AbsoluteLogic]

Merci pour l'attention que tu as apporté à mon sujet, mais malheureusement, tu te méprends sur le rôle de mes classes.
- GameManager est une classe "vaste" qui gère les actions générales de jeu (tour par tour), comme la gestion des tours, le score des joueurs, ou l'interface avec les joueurs. D'où son nom en français "gestionnaire de jeu",je ne vois pas mieux
- SimplePvPGameManager est une classe qui en hérite, implémentant toutes les fonctionnalités d'un jeu en particulier.
- Replay est un outil que j'ai créé permettant de créer facilement des animations, qu'il enregistre alors dans un fichier (la lecture se fait par un logiciel à part). Cela permet à SimplePvPGameManager de sauvegarder le replay du jeu courant en quelques lignes.
Désolé donc si les noms ont porté à confusion

[koala01]

- GameManager est une classe "vaste" qui gère les actions générales de jeu (tour par tour), comme la gestion des tours, le score des joueurs, ou l'interface avec les joueurs. D'où son nom en français "gestionnaire de jeu",je ne vois pas mieux

Mais, justement: cette classe est donc très loin de respecter le premier des principes SOLID (le S, mis pour SRP ou Single Responsability Principle) ce qui est déjà en soi un très gros problème de conception qui ne tardera pas à t'enfoncer sous "des tonnes de m...de"...

- SimplePvPGameManager est une classe qui en hérite, implémentant toutes les fonctionnalités d'un jeu en particulier.

Et qui ne sert donc à rien, à moins que tu ne prévoie de permettre au joueur de choisir le genre de jeu auquel il veut jouer

- Replay est un outil que j'ai créé permettant de créer facilement des animations, qu'il enregistre alors dans un fichier (la lecture se fait par un logiciel à part). Cela permet à SimplePvPGameManager de sauvegarder le replay du jeu courant en quelques lignes.

Mais donc, nous en revenons toujours au même point de base : il n'y a aucune raison que l'instance de Replay soit créée dés la création du GameManager et qu'elle soit "maintenue en vie" aussi longtemps qu'il existe : L'instance de Replay ne devrait, en tout état de cause, n'être créée que ... lorsque l'utilisateur décide se sauvegarder sa partie (ou son morceau de partie), et n'exister que ... le temps nécessaire à la création du fichier de sauvegarde.

En outre, le nom du fichier de sauvegarde ne devrait en aucun cas être codé en dur mais :

1. soit déterminé automatiquement en fonction des noms des fichiers de sauvegardes précédants et / ou sur base d'un timestamp quelconque
2. soit fournis directement par l'utilisateur

Désolé donc si les noms ont porté à confusion

Oh, tu n'as pas à être désolé, moi, je n'en soufirai absolument pas Toi, par contre...

[AbsoluteLogic]

Je ne sais pas si ces remarques avaient un lien avec mes fuites mémoires, mais je me sens obligé de continuer à me défendre, dans la mesure où tu interprètes les classes sans leur contexte... que je vais te donner de ce pas :
Mon projet consiste à confronter des IAs (programmé dans divers langages) programmées par des tiers, sur différents jeux. Mon programme prend en entrée deux IAs et quelques paramètres. Il simule la confrontation et renvoie le score résultant, plus un replay (systématiquement!) pour que les programmeurs des IAs puissent voir ce qui s'est passé. Mon programme effectue la simulation sur un objet GameManager qui est une sorte d'interface. Ensuite, les IAs sont confrontés sur un jeu dont le fonctionnement est défini par la "surcharge" de GameManager, ici avec la classe fille SimplePvPGameManager. Et donc oui, je m'arrange pour qu'il puisse y avoir plusieurs jeux,et que le travail pour la création de ce nouveau jeu soit au maximum mâché (de plus, l'héritage m'assure la compabilité avec mon programme).
Donc avec ce contexte, je peux répondre:
- GameManager a pour rôle de fournir le fonctionnement de base du jeu, et des fonctions "à surcharger". Ca ne respecte peut-être pas le principe du SOLID, mais c'est ce que j'ai trouvé de mieux pour simplifier au maximum la tâche des programmeurs de jeu.
- Le replay reste ouvert pendant toute la simulation pour que lorsque une IA demande au "SimplePvPGameManager" de déplacer le personnage dans une direction, le replay enregistre cette action dans un fichier, etc... Encore une fois, la classe Replay est là pour mâcher un maximum le travail à celui qui programmera un nouveau jeu.
- Le fait que le nom soit en dur est évidemment juste pour les tests. Quand ça fonctionnera, je l'améliorerai!

[jo_link_noir]

Bon, donc d'une certaine manière, le problème est réglé. Mais je laisse le sujet ouvert par peur que le bug revienne par la même magie qui a fait qu'il est parti.

C'est le genre de bug qui, s'il n'est pas corrigé, va créer plein d'autres bugs de manière aléatoire et subtile. Je suis assez surpris du comportement de asan, mais ça peut arriver. Il faudrait voir plus de code pour déterminer la cause, les plus fréquentes étant dépassement de tableau et utilisation d'un pointeur invalide.