Pac-Man sur ecran Oled

Bonjour a tous,

J'ai découvert le monde Arduino depuis un an et demi et je m’éclate.
Je suis partie de zéro, zéro en électronique, code ect...

Je viens de finir de coder le jeu Pac-man sur un écran oled.

J'aimerai beaucoup avoir votre avis sur mon code.

Je cherche a m’améliorer, donc n’hésitez pas.

J'ai codé qu'avec des variables globales, mon prochain challenge Tetris en C++ (en utilisant les class).

Merci pour m'avoir aidé a arriver jusque là.

et Merci pour les motivés a commenter mon code.

global.h (2.29 KB)

image.h (3.53 KB)

pacman_V13.ino (23.5 KB)

Personne de motivé ?
Ma demande est peut être incongru, mais j aimerai progresser en code.
Un regarde expert pourrai m y aider.
Merci

c’est difficile ce que vous demandez … votre code est long et vous n’avez pas de question précise… les bénévoles qui trainent sur le forum ne vont pas tout décortiquer…

deux trois trucs comme ça en lisant en diagonal

le code est structuré, indenté et commenté, et vous dites que ça fonctionne donc c’est un super bon début :slight_smile:

bon la critique constructive:

appeler votre port série logiciel ss n’est pas top… outre la référence historique douteuse, si c’est un port série dites nous ce qui est connecté au bout par exemple serialSoundBoard serait un bien plus joli nom :slight_smile:

vos enum, genre enum {NONE, INTRO, EATBEACON, DEATH, EATPHANTOM, START};prennent deux fois trop de mémoire car par défaut le langage utilise des int. n’oubliez donc pas de déclare un type plus courtenum :byte {NONE, INTRO, EATBEACON, DEATH, EATPHANTOM, START}; vous fera gagner 1 octet par variable de ce type.

quand on définit et initialise des tableaux à plusieurs dimensions, généralement on ne donne pas la première dimension (le compilateur va la calculer pour vous) et on fait apparaître la structure en 2D dans la définition. par exemple on n’écrit pasconst byte elbowRightDown[4][2] = {12, 47, 2, 2, 87, 37, 72, 2};mais plutôtconst byte elbowRightDown[][2] = {{12, 47}, {2, 2}, {87, 37}, {72, 2}};

On évite les nombres magiques

//Crossroads
const byte elbowRightDown[4][2] = {{12, 47}, {2, 2}, {87, 37}, {72, 2}};
const byte elbowLeftDown[4][2] = {92, 47, 17, 37, 32, 2, 102, 2};
const byte elbowLeftUp[4][2] = {32, 57, 102, 57, 17, 22, 92, 12};
const byte elbowRightUp[4][2] = {2, 57, 72, 57, 12, 12, 87, 22};
const byte teeDown[10][2] = {32, 47, 72, 47, 17, 12, 27, 12, 42, 12, 62, 12, 77, 12, 87, 12, 12, 2, 92, 2};
const byte teeUp[10][2] = {17, 47, 27, 47, 42, 47, 62, 47, 77, 47, 87, 47, 12, 57, 92, 57, 32, 12, 72, 12};
const byte teeRight[2][2] = {2, 22, 2, 37};
const byte teeLeft[2][2] = {102, 22, 102, 37};
boolean authorizedMovement = true;
byte crossroadsCoord[2] = {0}; //[0]=X - [1]=Y
byte pac_manOrPhantom = 0;
boolean intersectionExitFlag[4] = {false, false, false, false};

//Beacon & bonus
boolean bonusEaten = false;
boolean beacon[4] = {false, false, false, false};
boolean bonus[4] = {0};

→ tous ces 2, 4 et 10 je suppose sont reliés. Définissez des constantes avec un nom appropriés.

Je suppose que le 2 veut dire X et Y, des coordonnés; Quand on représente des coordonnées au lieu d’un tableau à 2 dimension on crée une structure un truc du genre

struct coord_t {
  byte x;
  byte y;
};

coord_t pac_manCoordOrigine = {52, 47};

est bien plus propre. comme ça à la lecture du code on ne verrait plus

void pac_manDisplay() {

  //Check the available directions
  crossroadsCoord[0] = pac_manCoord[0];
  crossroadsCoord[1] = pac_manCoord[1];

mais

void pac_manDisplay() {

  //Check the available directions
  crossroadsCoord.x = pac_manCoord.x;
  crossroadsCoord.y = pac_manCoord.y;

ce qui est bien plus compréhensible.

et comme j’imagine que vos tableaux sont des coordonnés au lieu deconst byte elbowRightDown[][2] = {{12, 47}, {2, 2}, {87, 37}, {72, 2}};on écrirait

const coord_t elbowRightDown[] = {{12, 47}, {2, 2}, {87, 37}, {72, 2}};

pour pouvoir y avoir accès avec .x et .y

le score ne peut pas aller au dessus de 255
byte score = 0;je vois que vous faites

  if (score == 129) {
    endGame = true;
    endGameAnimation();
  }

donc c’est bon, mais si un jour vous vous dites qu’on peut aller au delà de 255 vous risquer de rater le typage sur 1 octet

la vitesse peut-elle être négative? int speedGame = 400;si non, alors un unsigned est mieux, ça vous laisse plus de place. et si elle ne change pas, alors la déclarer en const n’est pas plus mal.

dans cette partie il y a 2 trucs louches

void checkButtons() {
  buttonUpValue = (digitalRead(buttonUpPin) == LOW);
  buttonDownValue = (digitalRead(buttonDownPin) == LOW);
  buttonRightValue = (digitalRead(buttonRightPin) == LOW);
  buttonLeftValue = (digitalRead(buttonLeftPin) == LOW);

  if (buttonUpValue == HIGH) pac_manDirection = UP;
  else if (buttonDownValue == HIGH) pac_manDirection = DOWN;
  else if (buttonRightValue == HIGH) pac_manDirection = RIGHT;
  else if (buttonLeftValue == HIGH) pac_manDirection = LEFT;

D’une part buttonUpValue et ses amis sont bien des booléens, donc vous les initialisez correctement en faisant une condition. mais ensuite dans le if vous les comparez à une valeur qui représente une tension… donc il faut écrire if (buttonUpValue) pac_manDirection = UP;… de plus comme vous utilisez des else ça veut dire que si vous appuyez sur 2 boutons en même temps, vous avez défini un ordre de préférence et UP est favorisé par rapport aux autres ! (pas possible d’aller en bais? :slight_smile: ).

même problème sémantique dans le code qui suit - et comme vous avez des PULLUP c’est même une erreur de logique, le bouton appuyé n’est pas HIGH mais vous avez de la chance que TRUE et HIGH ce soit pareil (1)

//Click button to start
  if (startGame == false) {
    if (buttonUpValue == HIGH || buttonDownValue == HIGH || ....

il ne faut pas comparer avec HIGH

quand on a des booléens avec un nom parlant, pas la peine de faire == true  if (authorizedMovement == true) {autant écrire   if (authorizedMovement) {

évitez de faire faire des calculs inutiles quand vous connaissez la réponse:

  //Click button to start
  if (startGame == false) {
    if (buttonUpValue == HIGH || buttonDownValue == HIGH || buttonRightValue == HIGH || buttonLeftValue == HIGH) {
      startGame = !startGame;  // <<== SI ON EST ICI C'ÉTAIT QUE STARTGAME == FALSE, DONC AUTANT LE METTRE À TRUE PLUTÔT QUE DE FAIRE UN CALCUL
      gameBoardDisplay();
      pac_manDirection = DONOTMOVE; // <<== ET DONC POURQUOI ON S'EST ENNUYE A LES INITIALISER JUSTE AVANT? --> faire un else
    }
  }

Quand vous faites

    switch (pac_manDirection) {
      case UP:
        pac_manCoord[1] += - 5;
        break;
      case DOWN:
        pac_manCoord[1] += + 5;
        break;
      case RIGHT:
        pac_manCoord[0] += + 5;
        break;
      case LEFT:
        pac_manCoord[0] += - 5;
    }

les +5 et -5 devraient être des constantes (une vitesse de déplacement X et Y) et au lieu de faire += d’un nombre négatif, autant faire -= c’est plus compréhensible

    switch (pac_manDirection) {
      case UP:
        pac_manCoord.y -= vitesseY;
        break;
      case DOWN:
        pac_manCoord.y +=  vitesseY;
        break;
      case RIGHT:
        pac_manCoord.x += vitesseX;
        break;
      case LEFT:
        pac_manCoord.x -= vitesseX;
    }

Là encore des constantes au lieu de nombres magiques seraient pertinents. pourquoi 4? pourquoi 100?

  for (phantomNumero = 0; phantomNumero < 4; phantomNumero++) {
    if (millis() - previousMillisPhantomBlink >= 100) {

(et pas sûr que vous ayez besoin que la variable phantomNumero soit globale, si ??)

  if (millis() - previousMillisBonusEaten >= 6000) bonusEaten = false;pourquoi 6000? devrait aussi être une constante

la fonction checkAvailableDirection() est plein de chiffres magiques crossroadsCoord[0] == [color=red]52[/color] && crossroadsCoord[1] == [color=red]12[/color]

le dessin du tableau est statique. pas de niveaux possibles ?

voilà - à la louche quelques commentaires; j’espère que ça aide :slight_smile:

Je suis bien conscient que l’exercice de vérifier un code est compliqué et fastidieux.

Mais tu as parfaitement su trouver la bonne formule. Sans allez trop loin dans le détail, tes commentaires sont très constructif.

Un grand MERCI