Journal Petit Framework de jeu 2d en C++

Posté par  . Licence CC By‑SA.
Étiquettes :
10
20
sept.
2013

Cher journal,

Au chômage depuis quelques temps, il fallait bien m'occuper un peu, je me suis alors renseigné sur le développement des jeuxvideos.
Comme c'est la mode depuis quelques temps de développer pour smartphone, je me suis lancé dans la création d'un petit framework pouvant fonctionner aussi bien sous ios que android, mais sur pc. J'avais la flemme d'apprendre le C# et de faire encore du Java. C'était donc l'opportunité de re-apprendre le C++.

J'ai donc créer un petit framework en C++ du nom de Castellum, disponible à cette adresse : http://castellum.adrienleroy.com/
Il dispose déjà d'un support des fichiers ogg,png (avec possibilité d'utiliser un atlas),des polices avec freetype,box2d….
Il reste plein de fonctionnalités à développer et corriger mais le but est avant tout pédagogique pour moi.
Pour l'instant,il n'y a pas de licence particulière, je pensais a une licence MIT.

Afin de tester un peu Castellum, j'ai crée un petit jeu : "Cube" disponible sur le Playstore et bientôt sur l'AppStore : http://cube.adrienleroy.com/.
Les sources ne sont pas encore disponibles et un éditeur de niveau existe en Qt, et je pense implémenter la possibilité de pouvoir soumettre ses propres niveaux.
Il consiste à amener un petit cube rose dans un autre cube vert en évitant certains obstacles.
Pour cela le cube est soumis à la gravité déterminé par la position du téléphone qui lit les données en provenance de l'accéléromètre.

  • # Très rapide review

    Posté par  (site web personnel) . Évalué à 10.

    J'ai jeté un coup d'oeil au code (très vite) et voici quelques retours:

    • JAMAIS de using namespace xxx dans un header, JAMAIS, JAMAIS! Ça casse l'intérêt principal d'avoir des namespace
    • L'encapsulation est brisée un peu partout via des getter (non constant) renvoyant des pointers sur des choses internes de tes classes. À quoi ça sert de faire de l'encapsulation si tu renvoie un pointeur nue que n'importe qui peut modifier et briser ainsi tous les invariants ? Pour la grande majorité, ces getters devraient être marqués const.
    • De manière plus générale, ton API manque beaucoup de modifiers const.
    • Les pointeurs nus, c'est souvent mal. On préfère utiliser au maximum les références quand c'est possible, ou un pointeur intelligent à défaut.
    • Le pattern singleton, c'est un peu le pattern sur-utilisé et presque toujours mal. Cf http://stackoverflow.com/questions/137975/what-is-so-bad-about-singletons pour une discussion sur les singletons.
    • Il faut utiliser les listes d'initialisation dans les constructeurs
    • Les objets "un peu gros" doivent (en général) être passé par référence constante plutôt que par valeur
    • Quand tu manipule des pointeurs, gare aux constructeurs par copie, et n'oublie pas d'avoir un destructeur qui libère correctement les ressources (bon évidemment pour l'instant, il n'y a quasiment que des singletons).

    Beaucoup d'autres choses à dire encore, mais il faudrait que je regarde de plus prêt.

    • [^] # Re: Très rapide review

      Posté par  (site web personnel) . Évalué à 2.

      Les variables membres sont préfixés par un underscore. Ce n'est pas réservé au compilo?

      Sinon, ce qui manque vraiment c'est de la doc et un petite page qui explique pourquoi utiliser ce cadriciel plutôt qu'un autre.

      Le post ci-dessus est une grosse connerie, ne le lisez pas sérieusement.

      • [^] # Re: Très rapide review

        Posté par  . Évalué à 2.

        Comme je l'explique, ce framework est d'abord pédagogique, il y a plein de framework ayant la meme vocation qui sont plus aboutis pour le moment (Moai,Corona….).
        Pour la doc effectivement, ca manque un peu, je suis en plein dedans.

    • [^] # Re: Très rapide review

      Posté par  . Évalué à 3.

      C'est marrant, ca me fait penser a beaucoup de conseils que j'ai pu retrouver dans le livre :

      "Effective C++: 55 Specific Ways to Improve Your Programs and Designs (3rd Edition)"

      Par le tres tres tres bon Scott Meyers.
      J'adore la maniere d'ecrire dedans (je pense que vous l'aurez deviner…)
      Du coup je te conseil de le lire :)

    • [^] # Re: Très rapide review

      Posté par  . Évalué à 2.

      Les objets "un peu gros" doivent (en général) être passé par référence constante plutôt que par valeur

      Comment sait-on si l'object est "un peu gros" à part faire un benchmark ?

      • [^] # Re: Très rapide review

        Posté par  (site web personnel) . Évalué à 5.

        Comment sait-on si l'object est "un peu gros" à part faire un benchmark ?

        Je vais plutôt répondre à : Quand passer un objet par référence constante plutôt que par valeur (recopie) ?

        La réponse rapide : Si l'objet n'est pas modifié: tout le temps. Sinon, passage par adresse (pas par référence).

        La réponse longue :

        Un passage par référence coute la même chose qu'un passage par adresse : la copie de l’adresse de l'objet (4 octets en 32 bits, 8 en 64 bits). D'un point de vu mémoire, un objet faisant très souvent plus de 4 octets, il doit donc quasiment tout le temps être passé par référence.

        De plus la recopie d'un objet (même petit) n'est pas obligatoirement gratuite. Le constructeur par recopie peut faire des opérations relativement longue. Un passage par référence évite la recopie.

        Une recopie perd le "type réel" de l'objet et casse le polymorphisme:

        #include <iostream>


struct Obj
{
        virtual int getvalue() const { return 1; }
};


struct Obj2: public Obj
{
        virtual int getvalue() const { return 2; }
};


void test1(const Obj toto)
{
        std::cout << "test1 " << toto.getvalue() << std::endl;
}

void test2(const Obj& toto)
{
        std::cout << "test2 " << toto.getvalue() << std::endl;
}


int main()
{
        Obj2 toto;
        test1(toto); // affiche "test1 1"
        test2(toto); // affiche "test2 2"
        return 0;
}

        Le cassage du polymorphisme peut être voulu, mais ça surprendra probablement l'utilisateur. Et je conseille un appelle explicite à la méthode d'origine si c'est voulu : "toto.Obj::getvalue()"

        Par contre, je déconseille très fortement le passage par référence non constante:

        • À la lecture d'un code déjà écrit, impossible de savoir si l'objet sera modifié par la fonction sans aller voir la définition de celle-ci. Si une fonction modifie un argument, c'est passage par adresse. point. Au moins c'est clair (à la lecture) : je passe l'objet=>pas de modif, je passe l'adresse => modif.
        • Impossible de passer des objets temporaires. Ce code ne compile pas :
        struct Obj
{
    int getvalue() const { return 1; }
};

void test(Obj& toto)
{
    std::cout << toto.getvalue() << std::endl;
}

int main()
{
    test(Obj());
    return 0;
}

        Avec "void test(const Obj& toto)" ça compile.

        Matthieu Gautier|irc:starmad

        • [^] # Re: Très rapide review

          Posté par  . Évalué à 1.

          Par exemple dans mon code j' ai actuellement la fonction suivante (qui est donc un passage par adresse) :

          void CastellumLayer::addBrick( CastellumBox* castellumBox) {
    _bricks.push_back(castellumBox);
    _bricks.sort(box_zindex_compare);
}

          Est il bien nécessaire d'optez pour un passage par référence de la sorte :

          void CastellumLayer::addBrick( CastellumBox &castellumBox) {
    _bricks.push_back(&castellumBox);
    _bricks.sort(box_zindex_compare);
}

          En effet mes objets CastellumBox sont toujours initialisé de manière dynamique, et non statique afin d'éviter qu'ils soient détruit dès qu'ils sortent de portée
          J'ai relu mon code et a première vue je ne fais pas de passage par valeur pour de "gros objets", seulement des types simples.

          • [^] # Re: Très rapide review

            Posté par  . Évalué à 2.

            Tout dépend si CastellumLayer deviens propriétaire de l'objet ou non (autrement dit, si il deviens responsable de la destruction du CastellumBox)`.

            Si avant l'appel, le CastellumBox appartient à l'appelant et qu'après l'appel il n'appartient plus à l'appelant, alors il faut utiliser ni pointeurs nus ni références, mais des pointeurs intelligents comme std::unique_ptr (ou std::auto_ptr en C++03). Parce que tel que le code est écrit, si push_back balance une exception (genre std::bad_alloc), alors tu peut potentiellement laisser fuiter un castellumBox.

            Si CastellumLayer ne deviens jamais propriétaire de l'objet, et que la durée de vie de CastellumBox est correctement gérée (aucun pointeur traîne vers l'objet lors de sa destruction, y compris dans un CastellumLayer::_bricks), alors je dirait qu'il faudrai mieux prendre un pointeur nu, mais ça peut se discuter.

            Si CastellumLayer à toujours été propriétaire de l'objet, et que l'appelant à récupéré le CastellumBox depuis CastellumLayer, alors y a peut-être des choses à améliorer dans l'API.

          • [^] # Re: Très rapide review

            Posté par  (site web personnel) . Évalué à 3.

            En fait il ne faut pas voir comment ta fonction est définie mais comment elle va être utilisée.

            Si tu reviens au c, tu as deux solutions :

            int a;
function1(a);
function2(&a);

            Sans connaitre la définition des fonctions, tu sais (le langage te le garantie) que a ne sera pas modifier par function1. Je sais que je peut faire ce que je veux avec a après, je sais que je casserai rien.

            Pour function2, tu sais pas. Ça peut être modifié, ou pas. Ça peut être stocké (pour lecture ou modification future) ou pas. Mais tu sais qu'il faut faire attention.

            Si tu es en C++ et que ta as des références

            int a;
function1(a);

            Ben là… Mais le c++ ne garantie rien car function1 peut prendre une référence.
            L'usage (le mien) voudrait que tu ais la garantie que a soit pas modifié. C'est pour ça que je déconseille très fortement la référence non constante. Le but est de garder la sémantique des passages d'argument du c.

            Dans ton premier cas, la fonction sera utilisée de la sorte:

            CastellumBox box1;
CastellumBox* p_box2 = new CastellumBox();

layer.addBrick(&box1);
layer.addBrick(p_box2);

            C'est clair, tu attends une adresse parce que tu veux faire un truc avec. L'utilisateur doit faire attention à ce qu'il fait.

            Dans ton deuxième cas :

            CastellumBox box1;
CastellumBox* p_box2 = new CastellumBox();

layer.addBrick(box1);
layer.addBrick(*p_box2);

            Là, c'est le drame. Tu "demandes" une valeur à l'utilisateur alors que justement tu veux une adresse. Donc : jamais de référence non constante comme argument de fonction. Reste avec un passage d'adresse.

            À noter que dans ton cas, il ne faut pas utiliser les références constante non plus (même si tu ne modifie pas castellumBox).

            class CastellumLayer{
    void addBrick(const CastellumBox& castellumBox);

    std::vector<const castellumBox*> _bricks;
};
void CastellumLayer::addBrick(const CastellumBox& castellumBox) {
    _bricks.push_back(&castellumBox);
    _bricks.sort(box_zindex_compare);
}

            Ton code compile et tu ne modifie pas castellumBox mais tu stockes l’adresse pour l'utiliser plus tard. Tu casses le contrat (purement implicite) de la fonction vers le code appelant : "Fait ce que tu veux avec, moi j'en ai fini, je l'utilise plus".
            Si tu fais un delete après l'appel de la fonction:

            CastellumBox* p_box = new CastellumBox();
layer.addBrick(*p_box);
delete p_box;

            Le code semble correcte, tu passes une valeur et tu supprime le contenant après utilisation, mais ton programme va se taper un segfault bien méchant (car pas obligatoirement tout de suite et au même endroit)

            Avec un passage par adresse :

            CastellumBox* p_box = new CastellumBox();
layer.addBrick(p_box);
delete p_box;

            Rien ne t’empêche de faire l'erreur, mais c'est moins clean : tu passes le contenant et tu le supprimes après. Ça mais la puce à l'oreille.
            Après on peut jouer avec les les smart pointer, shared_pointer et unique_ptr pour rajouter une sémantique de "propriété de l'objet" mais ça fait des posts trop long :)

            Il reste un cas (le seul?), où le passage par recopie est nécessaire : Quand tu veux explicitement une copie de l'objet (pour la modifier ou la stocker sans interférer avec l'objet d'origine)

            Matthieu Gautier|irc:starmad

            • [^] # Re: Très rapide review

              Posté par  . Évalué à 2.

              Concernant les pointers nus, je me souviens pourquoi j'ai opté pour cette solution :
              Le support de la norme C++0x par le ndk (native development kit d'android), quand j'ai commencé mon projet le support n'était pas terrible, j ai donc exclu l'utilisation de la norme et donc des pointers intelligents.
              Il restait auto_ptr, mais j'avais lu que auto_ptr et les containers de la STL ne fonctionne pas bien ensemble :
              http://www.gotw.ca/gotw/025.htm

              D'une manière générale j'ai l'impression quand dans mon api, le vrai problème n'est pas tant le passage de mes paramètres mais plutôt le cycle de vie de mes objets qui n'est pas très clair.

        • [^] # Re: Très rapide review

          Posté par  (site web personnel) . Évalué à 3.

          L'un des gros avantages des références par rapport aux pointeurs, ça reste la garantie de non-nullité. On peut toujours arriver à avoir des danglings références dans du code un peu poilu (genre avec des callback), mais ça reste une sécurité non négligeable.

    • [^] # Re: Très rapide review

      Posté par  . Évalué à 1.

      JAMAIS de using namespace xxx dans un header, JAMAIS, JAMAIS! Ça casse l'intérêt principal d'avoir des namespace

      Je vois pas le problème avec les using namespace dans les headers, c'est bien pratique parfois dans du métacode. Ça permet d'éviter du code plus poilu.

      Faut juste pas le faire dans le namespace global.

      • [^] # Re: Très rapide review

        Posté par  . Évalué à -1.

        Merci pour les remarques.
        Pour le namespace dans le header comme je l'ai fait, cela pose problème si je créer une fonction/classe…. qui existe déjà dans la STL.
        http://www.cplusplus.com/forum/beginner/25538/
        Je trouve cette remarque justifié, mais c'est vrai que par fainéantise il est facile de le déclarer en header.

      • [^] # Re: Très rapide review

        Posté par  (site web personnel) . Évalué à 2.

        Oui tu as raison, pas de using namespace globaux est plus juste. P

        Personnellement, je préfére éviter les using namespace en général, quitte à avoir du code un peu plus long ou utiliser un alias local. Ça permet de savoir toujours quoi tu appelle rien qu'en lisant le code, sans aller chercher loin d'où vient ce vector (std::, boost::container::, my_cool_implementation:: , qt quand ils auront compris les namespace :D).

Suivre le flux des commentaires

Note : les commentaires appartiennent à celles et ceux qui les ont postés. Nous n’en sommes pas responsables.