Rolisteam Evolution

The function evolution…

I would like to show the evolution of one function in rolisteam code.
This function is called when the user (Game Master) closes a map or image.

The function must close the map or image and send a network order to each player to close the image or plan.

At the beginning, the function looked like that.
Some code line and comments were written in French. No coding rules were respected.
Low-level code for networking. The function was very long.

 // On recupere la fenetre active (qui est forcement de type CarteFenetre ou Image, sans quoi l'action
        // ne serait pas dispo dans le menu Fichier)
        QWidget *active = workspace->activeWindow();

        // Ne devrait jamais arriver
        if (!active)
        {
                qWarning("Close map action called when no widget is active in the workspace (fermerPlanOuImage - MainWindow.h)");
                return;
        }

        // On verifie pour le principe qu'il s'agit bien d'une CarteFenetre ou d'une Image
        if (active->objectName() != "CarteFenetre" && active->objectName() != "Image")
        {
                qWarning("not expected type of windows (fermerPlanOuImage - MainWindow.h)");
                return;
        }

        // Creation de la boite d'alerte
        QMessageBox msgBox(this);
        msgBox.addButton(QMessageBox::Yes);
        msgBox.addButton(QMessageBox::Cancel);
        msgBox.setIcon(QMessageBox::Information);
        msgBox.move(QPoint(width()/2, height()/2) + QPoint(-100, -50));
        // On supprime l'icone de la barre de titre
        Qt::WindowFlags flags = msgBox.windowFlags();
        msgBox.setWindowFlags(flags ^ Qt::WindowSystemMenuHint);
        // M.a.j du titre et du message
        if (active->objectName() == "CarteFenetre")
        {
                msgBox.setWindowTitle(tr("Close Map"));
                msgBox.setText(tr("Do you want to close this map?\nIt will be closed for everybody"));
        }
        else
        {
                msgBox.setWindowTitle(tr("Close Picture"));
                msgBox.setText(tr("Do you want to close this picture?\nIt will be closed for everybody"));
        }
        msgBox.exec();

        // Si l'utilisateur n'a pas clique sur "Fermer", on quitte
        if (msgBox.result() != QMessageBox::YesRole)
                return;

        // Emission de la demande de fermeture de la carte
        if (active->objectName() == "CarteFenetre")
        {
                // Recuperation de l'identifiant de la carte
                QString idCarte = ((CarteFenetre *)active)->carte()->identifiantCarte();

                // Taille des donnees
                quint32 tailleCorps =
                        // Taille de l'identifiant de la carte
                        sizeof(quint8) + idCarte.size()*sizeof(QChar);

                // Buffer d'emission
                char *donnees = new char[tailleCorps + sizeof(enteteMessage)];

                // Creation de l'entete du message
                enteteMessage *uneEntete;
                uneEntete = (enteteMessage *) donnees;
                uneEntete->categorie = plan;
                uneEntete->action = fermerPlan;
                uneEntete->tailleDonnees = tailleCorps;

                // Creation du corps du message
                int p = sizeof(enteteMessage);
                // Ajout de l'identifiant de la carte
                quint8 tailleIdCarte = idCarte.size();
                memcpy(&(donnees[p]), &tailleIdCarte, sizeof(quint8));
                p+=sizeof(quint8);
                memcpy(&(donnees[p]), idCarte.data(), tailleIdCarte*sizeof(QChar));
                p+=tailleIdCarte*sizeof(QChar);

                // Emission de la demande de fermeture de la carte au serveur ou a l'ensemble des clients
                emettre(donnees, tailleCorps + sizeof(enteteMessage));
                // Liberation du buffer d'emission
                delete[] donnees;

                // Suppression de la CarteFenetre et de l'action associee sur l'ordinateur local
                ((CarteFenetre *)active)->~CarteFenetre();
        }

        // Emission de la demande de fermeture de l'image
        else
        {
                // Recuperation de l'identifiant de la carte
                QString idImage = ((Image *)active)->identifiantImage();

                // Taille des donnees
                quint32 tailleCorps =
                        // Taille de l'identifiant de la carte
                        sizeof(quint8) + idImage.size()*sizeof(QChar);

                // Buffer d'emission
                char *donnees = new char[tailleCorps + sizeof(enteteMessage)];

                // Creation de l'entete du message
                enteteMessage *uneEntete;
                uneEntete = (enteteMessage *) donnees;
                uneEntete->categorie = image;
                uneEntete->action = fermerImage;
                uneEntete->tailleDonnees = tailleCorps;

                // Creation du corps du message
                int p = sizeof(enteteMessage);
                // Ajout de l'identifiant de la carte
                quint8 tailleIdImage = idImage.size();
                memcpy(&(donnees[p]), &tailleIdImage, sizeof(quint8));
                p+=sizeof(quint8);
                memcpy(&(donnees[p]), idImage.data(), tailleIdImage*sizeof(QChar));
                p+=tailleIdImage*sizeof(QChar);

                // Emission de la demande de fermeture de l'image au serveur ou a l'ensemble des clients
                emettre(donnees, tailleCorps + sizeof(enteteMessage));
                // Liberation du buffer d'emission
                delete[] donnees;

                // Suppression de l'Image et de l'action associee sur l'ordinateur local
                ((Image *)active)->~Image();
        }

A big step forward, the networking code has been reworked.
So the readability is improved. It is also possible to see the beginning of polymorphism. MediaContener is used but not everywhere.

QMdiSubWindow* subactive = m_mdiArea->currentSubWindow();
QWidget* active = subactive;
MapFrame* bipMapWindow = NULL;

if (NULL!=active)
{

    QAction* action=NULL;

    Image*  imageFenetre = dynamic_cast(active);

    QString mapImageId;
    QString mapImageTitle;
    mapImageTitle = active->windowTitle();
    bool image=false;
    //it is image
    if(NULL!=imageFenetre)
    {
        m_pictureList.removeOne(imageFenetre);

        mapImageId = imageFenetre->getMediaId();
        image = true;
        action = imageFenetre->getAction();
    }
    else//it is a map
    {
        bipMapWindow= dynamic_cast(active);
        if(NULL!=bipMapWindow)
        {
            mapImageId = bipMapWindow->getMediaId();
            action = bipMapWindow->getAction();

        }
        else// it is undefined
        {
            return;
        }
    }

    QMessageBox msgBox(this);
    msgBox.setStandardButtons(QMessageBox::Yes | QMessageBox::Cancel );
    msgBox.setDefaultButton(QMessageBox::Cancel);
    msgBox.setIcon(QMessageBox::Information);
    msgBox.move(QPoint(width()/2, height()/2) + QPoint(-100, -50));
    Qt::WindowFlags flags = msgBox.windowFlags();
    msgBox.setWindowFlags(flags ^ Qt::WindowSystemMenuHint);

    if (!image)
    {
        msgBox.setWindowTitle(tr("Close Map"));
    }
    else
    {
        msgBox.setWindowTitle(tr("Close Picture"));
    }
    msgBox.setText(tr("Do you want to close %1 %2?\nIt will be closed for everybody").arg(mapImageTitle).arg(image?tr(""):tr("(Map)")));

    msgBox.exec();
    if (msgBox.result() != QMessageBox::Yes)
        return;

    if (!image)
    {
        NetworkMessageWriter msg(NetMsg::MapCategory,NetMsg::CloseMap);
        msg.string8(mapImageId);
        msg.sendAll();

        m_mapWindowMap.remove(mapImageId);
        m_playersListWidget->model()->changeMap(NULL);
        m_toolBar->changeMap(NULL);
    }
    else
    {
        NetworkMessageWriter msg(NetMsg::PictureCategory,NetMsg::DelPictureAction);
        msg.string8(mapImageId);
        msg.sendAll();
    }

    MediaContainer*  mediaContener = dynamic_cast(subactive);
    if(NULL!=mediaContener)
    {
        CleverURI* cluri = mediaContener->getCleverUri();
        cluri->setDisplayed(false);
        if(NULL!=m_sessionManager)
        {
            m_sessionManager->updateCleverUri(cluri);
        }
    }

    delete action;
    delete subactive;
}
Version Intermédiaire

Then, we implements a solution to describe map, vmap (v1.8) or image as mediacontener.
They share a large part of their code, so it becomes really easy to do it.

QMdiSubWindow* subactive = m_mdiArea->currentSubWindow();
    MediaContainer* container = dynamic_cast(subactive);
    if(NULL != container)
    {
        CleverURI::ContentType type = container->getContentType();
        if(CleverURI::VMAP == type)
        {
            removeVMapFromId(container->getMediaId());
        }
        else if(CleverURI::MAP == type)
        {
            removeMapFromId(container->getMediaId());
        }
        else if(CleverURI::PICTURE == type )
        {
            removePictureFromId(container->getMediaId());
        }
    }
Actuel

Leave a Reply

Your email address will not be published. Required fields are marked *