Skip to content

WorldBuffer/WorldPacket et QByteArray #2

Description

@scalexm

L'utilité de la classe QByteArray ici est totalement nulle (0), étant donné que la seule chose intéressante est la méthode

WorldBuffer &operator<<(QString)

et cela oblige donc à faire des trucs assez ignobles du style

queuePosition << QString::number(AuthQueue::Instance()->GetClientPosition(this)).toAscii().data()

alors qu'avec un SIMPLE ostringstream, on aurait juste à écrire:

queuePosition << AuthQueue::Instance()->GetClientPosition(this)

et ce, pour n'importe quel type de valeur! (attention toutefois aux int8_t/uint8_t qui seront interprétés comme des char)

Activity

  1. KevinSupertramp commented on Apr 15, 2013

    @KevinSupertramp
    Member

    Bonjour, merci pour ton commentaire. Je n'ai pas tout à fait compris comment tu proposes d'utiliser ostringstream. Le prendre en paramètre comme ceci ? :

    WorldBuffer &operator<<(ostringstream)

    En faite actuellement la ligne est totalement fausse, c'est stupide de passer via une QString, on peut simplement écrire comme tu l'as proposé car il y a un opérateur pour prendre les int.

    Merci

  2. KevinSupertramp commented on Apr 15, 2013

    @KevinSupertramp
    Member

    Après je pourrais également utiliser un opérateur avec QVariant qui doit être plus ou mois pareil que ostringstream.

  3. scalexm commented on Apr 15, 2013

    @scalexm
    Author

    Remplacer WorldBuffer/WorldPacket qui actuellement ne sert à rien par un ostringstream

    std::ostringstream packet;
    packet << "Af|" << 5 << ";" << 7.0 ...

    Et si tu tiens à garder les SMSG_/CMSG_ (ce que je conseille d'ailleurs), remplace les par de simples const char* (dans un namespace ou non, c'est selon) et tu pourras alors te passer de la table de conversion enum <-> string qui disons le ne sert à rien.

    Si t'as besoin, tu peux aussi faire un truc du genre:

    class WorldPacket
    {
    private:
       std::ostringstream _buffer;
    public:
       WorldPacket(const std::string & opcode)
       { _buffer << opcode; }
    
       template<class T>
       WorldPacket & operator <<(const T & value)
       {
           _buffer << value;
          return *this;
       }
    };

    Ou quelque chose de ce genre comme ça tu peux spécialiser l'opérateur << ou rajouter des trucs dans le constructeur etc

  4. scalexm commented on Apr 15, 2013

    @scalexm
    Author

    Et si tu parles de la surcharge de WorldBuffer::operator << pour prendre les int, j'ai plutôt l'impression qu'elle écrit la représentation sous forme de tableau d'octets de ton int dans le buffer, donc ça ne vas pas avoir l'effet que tu attends.

  5. KevinSupertramp commented on Aug 24, 2013

    @KevinSupertramp
    Member

    Hello, merci encore pour ton commentaire, j'ai suivi ton conseil et implémenté un QTextStream qui fait un peu prêt la même chose mais qui supporte les types propre à Qt.

    Voir 0c76a17

    Bonne soirée,
    Kevin

  6. scalexm commented on Aug 24, 2013

    @scalexm
    Author

    Ouais voilà ça c'est cool :) mais pourquoi allouer dynamiquement, tu n'en as pas besoin là une allocation sur la pile suffit

  7. KevinSupertramp commented on Aug 24, 2013

    @KevinSupertramp
    Member

    Pour QTextStream et QByteArray ? J'ai essayé mais sans succès, apparemment QTextStream interdit la copie de constructeur (tout comme ostringstream) et le seul fait de le déclarer donne une erreur...

  8. scalexm commented on Aug 24, 2013

    @scalexm
    Author

    Le fait de passer par un pointeur ne va pas résoudre le problème, car si tu veux copier un WorldPacket le pointeur interne pointera vers le même QTextStream. De toutes façons tu ne devrais jamais avoir à copier un WorldPacket, et l'utilisation d'un QTextStream alloué sur la pile (qui désactivera donc le constructeur par copie) évitera toutes erreurs silencieuses qui pourraient apparaître avec l'utilisation d'un pointeur (et donc d'un constructeur par copie)

  9. KevinSupertramp commented on Aug 25, 2013

    @KevinSupertramp
    Member

    En faite du moment que je fais un :

    private:
    QTextStream m_stream;

    J'obtiens une erreur comme quoi "QTextStream is private". Du coup je ne sais pas comment faire ^^

  10. scalexm commented on Aug 25, 2013

    @scalexm
    Author

    .h:

        const QString & GetPacket() const
        {
            return m_buffer;
        }
    private:
        QString m_buffer;
        QTextStream m_stream;
        quint8 m_opcode;

    .cpp:

    WorldPacket::WorldPacket(quint8 opcode) : m_stream(&m_buffer), m_opcode(opcode)
    {
        *this << GetOpcodeHeader(m_opcode);
    }
  11. KevinSupertramp commented on Aug 25, 2013

    @KevinSupertramp
    Member

    Merci mais toujours le même problème :). En faite même avec une simple classe comme celle-ci j'obtiens l'erreur :

    class WorldPacket
    {
    public:
        WorldPacket(quint8/* opcode*/)
        {
        }
    
    private:
        QTextStream m_stream;
    };

    Me retourne l'erreur : erreur : 'QTextStream::QTextStream(const QTextStream&)' is private

  12. scalexm commented on Aug 25, 2013

    @scalexm
    Author

    C'est parce qu'à un endroit dans ton code tu copies un WorldPacket (et normalement tu ne devrais jamais à avoir à faire ceci).

  13. scalexm commented on Aug 25, 2013

    @scalexm
    Author

    Exemple: void SendPacket(WorldPacket data); dans shared/servers/SocketHandler
    Il faut ici passer ton WorldPacket par const référence

  14. KevinSupertramp commented on Aug 25, 2013

    @KevinSupertramp
    Member

    Ah d'accord merci bien pour les explications ! J'ai pu l'intégrer comme ceci : 2948b4b . Par contre pour les const j'ai pas vraiment compris comment les utiliser pour le coup, car j'avais des problèmes ensuite avec les fonctions "SendPacket" que j'aurais également dû passer en const ? (discards qualifiers)

  15. scalexm commented on Aug 25, 2013

    @scalexm
    Author

    Le problème vient de WorldPacket::GetPacket. Il faut que la méthode soit const comme dans le code que je t'ai donné, et pour ça il ne faut pas appeler m_stream.flush. Donc il faut passer par un QString et non un QByteArray, car les QString n'ont pas besoin d'être flush

  16. scalexm commented on Aug 25, 2013

    @scalexm
    Author

    Et donc dans ta méthode SocketHandler::SendPacket tu fais:
    m_socket->write(data.GetPacket().toAscii() + (char)0x0)

  17. scalexm commented on Aug 25, 2013

    @scalexm
    Author

    En fait le mieux (même si on sort un peu du sujet) c'est d'éviter au maximum les allocations dynamiques. Il ne faut les utiliser que quand tu veux éviter de lourdes copies, ou bien quand tu veux partager un objet entre plusieurs endroits du code et que tu ne peux pas utiliser de références. Et dans l'idéal si tu dois utiliser des allocations dynamiques, utilise des pointeurs intelligents (Qt en contient plusieurs).

  18. KevinSupertramp commented on Aug 26, 2013

    @KevinSupertramp
    Member

    Okay merci bien pour les explications ! Je ne pensais pas que cela venait du flush ! Voilà la correction, on devrait être bon maintenant : 1a088cf .

    J'avais jamais checké s'il y avait des smart pointer avec Qt, merci de m'avoir donné les infos, j'ai fait quelques recherches et je vais voir quand les implémenter.

    Bonne journée ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions