0

For two days now I've been trying to understand why different slots are being called by the same button on my application. I've googled a lot about the subject but mostly what I found was regarding a unique SLOT being called multiple times by the same button which is solved adding a fifth parameter Qt::UniqueConnection on connect call i.e.:

connect(obj, SIGNAL(signal()), obj2, SLOT(slot()), Qt::UniqueConnection);

Moving on, my problem is that a same button is calling different slots when it shouldn't. I created a generic code to setup all my buttons:

void KHUB::btSetupInt(QPushButton **button, const QString name, int posX, int posY, int width, int height, void (KHUB::*fptr)(int parameter), int value, int handler) {
  *button = new QPushButton(name, this);
  (*button)->setGeometry(QRect(QPoint(posX, posY), QSize(width, height)));

  signalMapper->setMapping(*button, value);
  connect(*button, SIGNAL(clicked()), signalMapper, SLOT(map()));
  //qDebug() << "My value: " + QString::number(value);

  switch (handler) {
    case (int) ButtonHandler::hl_Register:
      connect(signalMapper, SIGNAL(mapped(int)), this, SLOT(handleRegister(int)), Qt::UniqueConnection);
      break;
    case (int)ButtonHandler::hl_UpVote:
      connect(signalMapper, SIGNAL(mapped(int)), this, SLOT(handleUpVote(int)), Qt::UniqueConnection);
      break;
    case (int) ButtonHandler::hl_OpenUrl:
      //connect(signalMapper, SIGNAL(mapped(int)), this, SLOT(handleUrl(int)), Qt::UniqueConnection);
      break;
    case (int)ButtonHandler::hl_DownVote:
      connect(signalMapper, SIGNAL(mapped(int)), this, SLOT(handleDownVote(int)), Qt::UniqueConnection);
      break;
    case (int) ButtonHandler::hl_DisposeBrowser:
      connect(signalMapper, SIGNAL(mapped(int)), this, SLOT(handleDispose(int)), Qt::UniqueConnection);
      break;
    }
}

Within it I have my signalMapper which is declared in my header file. This button setup is being called by:

void KHUB::handleSearch() {
  //DO THE MATH
  for (int pos = 0; pos < localUrl.size(); pos++){
    QLabel *link = new QLabel();
    link->setText("<a href=\"" + localUrl.at(pos) + "\">" + localUrl.at(pos) + "</a>");
    link->setTextFormat(Qt::RichText);
    link->setTextInteractionFlags(Qt::TextBrowserInteraction);
    link->setOpenExternalLinks(true);

    QPushButton *upArrow = new QPushButton();
    btSetupInt(&upArrow, "Up Vote", 225, 300, 100, 25, &KHUB::handleUpVote, pos*147, (int)ButtonHandler::hl_UpVote);

    QPushButton *open = new QPushButton("Open");
    //btSetupInt(&open, "Open", 225, 300, 100, 25, &KHUB::handleUrl, pos, (int) ButtonHandler::hl_OpenUrl);

    QPushButton *downArrow = new QPushButton();
    btSetupInt(&downArrow, "Down Vote", 225, 300, 100, 25, &KHUB::handleDownVote, pos*163, (int)ButtonHandler::hl_DownVote);

    QLabel *separator = new QLabel();
    QLabel *guider = new QLabel("<----------------------------------------------------------------------------------------------------------------------------------------------------->");

    gridLayout->addWidget(link, componentsPos, 0, 1, -1);
    gridLayout->addWidget(guider, componentsPos, 1, 1, -1);
    gridLayout->addWidget(upArrow, componentsPos - 1, 2, 1, 1, Qt::AlignBottom);
    gridLayout->addWidget(open, componentsPos, 2, 1, 1);
    gridLayout->addWidget(downArrow, componentsPos + 1, 2, 1, 1, Qt::AlignTop);
    gridLayout->addWidget(separator, componentsPos + 2, 1, 1, 1);

    componentsPos = componentsPos + 5;
  }
  //KEEP DOING THE MATH
}

Commented lines are as it is for debugging purposes. Anyway, this code was fully working when I had only the open QPushButton. When added upVote and downVote I expected the programm to replicate its logic success, instead, now when I click for Up voting ou Down voting the programm triggers both slots for handling Up and Down votin and also "Open" if not commented. Code for handling Up and Down Vote:

void KHUB::handleUpVote(int reference) {
    qDebug() << "This is my reference for upvoting: " + QString::number(reference/147);
}

void KHUB::handleDownVote(int reference) { 
    qDebug() << "This is my reference for DOWNVOTING: " + QString::number(reference/163);
}

According to Qt QSignalMapper documentation (Topic "Advanced Signals and Slots Usage" at page's ending) I'm doing everything as expected. You also must have noticed that I multiplied pos at btSetupInt call by 147 and 163 (to produce a "random" number) since every button must have its only integer ID [see setMapping for integers void QSignalMapper::setMapping(QObject * sender, int id)].

Days ago I was using the same position (pos) for the three of them (buttons) and whether multiplying or not the produced result is equal. That leads me to my first question: This ID is a unique identifier for each button or a unique parameter that can be passed by the button? I Still didn't get this considering the context I gave above

This is my UI:

Application UI

This is my output clicking (random link choosen) Up Vote at us.battle.net/wow/en/blog/19913791/patch-623-preview-10-14-2015:

"This is my reference for upvoting: 3"
"This is my reference for DOWNVOTING: 3"

This is my output clicking (random link choosen) Down Vote at http://www.swtor.com/holonet/companions/blizz:

"This is my reference for upvoting: 6"
"This is my reference for DOWNVOTING: 6"

I used breakpoints to see if the handler switch at btSetupInt was being called correctly and it is. Even not using my member function btSetupInt to call connections I'm still getting the same wrong results. To finish, my second and main question: How is even possible, in the given context, that a same button is calling different slots? What am I missing here?

Community
  • 1
  • 1
Victor Oliveira
  • 3,293
  • 7
  • 47
  • 77
  • before connected signal to the new slot you need to disconnect it with the previous one. Then all will work fine. – johngull Oct 16 '15 at 14:12
  • enlighten me please, I got your words but no the idea at all - How am I supposed to do this? Quick google research would be something like `button->disconnect(SIGNAL(signal))` at the end of my switch? – Victor Oliveira Oct 16 '15 at 14:24

2 Answers2

1

Because, you are connecting the signal mapper multiple times i.e., for each button instance. You are creating a button and then depending on the handler you are connecting to a different slot. But the signal mapper's mapped(int) signal is connected to multiple slots. Hope you got what I'm saying here.

Another issues with your code is memory leak.

QPushButton *upArrow = new QPushButton();
btSetupInt(&upArrow, "Up Vote", 225, 300, 100, 25, &KHUB::handleUpVote, pos*147, (int)ButtonHandler::hl_UpVote);

btSetupInt() is creating new button. Again.

*button = new QPushButton(name, this);

This is untested code. check whether it works.

// custom button claas
class VotingButton : public QPushButton
{
    Q_OBJECT

public:
    VotingButton(const QString& url, int voteType, QObject* prnt=0)
        : QPushButton(prnt)
          m_url(url),
          m_voteType(voteType)
    {}

    QString getUrl() const { return m_url; }
    int getVoteType() const { return m_voteType; } 

private:
    QString     m_url;
    int         m_voteType;
}

in your KHUB class

connect(signalMapper, SIGNAL(mapped(QObject*)), this, SLOT(handleButtonPress(QObject*)));

void KHUB::btSetupInt(QPushButton* button, const QString& name, int posX, int posY, int width, int height, const QString& url, int handler) {
    button = new VotingButton(url, handler, this);
    button->setText(name);
    button->setGeometry(QRect(QPoint(posX, posY), QSize(width, height)));
    signalMapper->setMapping(button, this);
    connect(button, SIGNAL(clicked()), signalMapper, SLOT(map()));
}

void KHUB::handleSearch() {
  //DO THE MATH
  for (int pos = 0; pos < localUrl.size(); pos++){
    QLabel *link = new QLabel();
    link->setText("<a href=\"" + localUrl.at(pos) + "\">" + localUrl.at(pos) + "</a>");
    link->setTextFormat(Qt::RichText);
    link->setTextInteractionFlags(Qt::TextBrowserInteraction);
    link->setOpenExternalLinks(true);

    QPushButton *upArrow;
    btSetupInt(upArrow, "Up Vote", 225, 300, 100, 25, localUrl.at(i), (int)ButtonHandler::hl_UpVote);

    QPushButton *open;
    btSetupInt(open, "Open", 225, 300, 100, 25, localUrl.at(i), (int)ButtonHandler::hl_OpenUrl);    

    QPushButton *downArrow;
    btSetupInt(downArrow, "Down Vote", 225, 300, 100, 25, localUrl.at(i), (int)ButtonHandler::hl_DownVote);

    QLabel *separator = new QLabel();
    QLabel *guider = new QLabel("<----------------------------------------------------------------------------------------------------------------------------------------------------->");

    gridLayout->addWidget(link, componentsPos, 0, 1, -1);
    gridLayout->addWidget(guider, componentsPos, 1, 1, -1);
    gridLayout->addWidget(upArrow, componentsPos - 1, 2, 1, 1, Qt::AlignBottom);
    gridLayout->addWidget(open, componentsPos, 2, 1, 1);
    gridLayout->addWidget(downArrow, componentsPos + 1, 2, 1, 1, Qt::AlignTop);
    gridLayout->addWidget(separator, componentsPos + 2, 1, 1, 1);

    componentsPos = componentsPos + 5;
  }
  //KEEP DOING THE MATH
}

void KHUB::handleButtonPress(QObject* obj)
{
    VotingButton* voteButton = static_cast<VotingButton*>(obj);
    if (voteButton) { // now you have voteType and the url assigned to the button
        switch (voteButton->getVoteType())
        {
            case ...
        }
    }
}
ramtheconqueror
  • 1,907
  • 1
  • 22
  • 35
  • I really appreciate your answer, thank you; But I will need some hours until I cant test it, I have an exam tomorrow so as soon as I can I will come back to reply what happened – Victor Oliveira Oct 16 '15 at 15:20
0

As far as I understood based on ramtheconqueror's answer, you can't use the same map to connect different slots. In my code I was using signalMapper to connect to different handlers (slots) and, to solve that, I just added a QSignalMapper parameter at btSetupInt. I created three different maps for each button type, to provide which map I wanted to connect a slot.

My code looks like this now:

void KHUB::btSetupInt(QPushButton **button, const QString name, int posX, int posY, int width, int height, void (KHUB::*fptr)(int parameter), QSignalMapper* map, int value, int handler) {
  *button = new QPushButton(name, this);
  (*button)->setGeometry(QRect(QPoint(posX, posY), QSize(width, height)));

  map->setMapping(*button, value);
  connect(*button, SIGNAL(clicked()), map, SLOT(map()));
  //qDebug() << "My value: " + QString::number(value);

  switch (handler) {
    case (int) ButtonHandler::hl_Register:
      connect(map, SIGNAL(mapped(int)), this, SLOT(handleRegister(int)), Qt::UniqueConnection);
      break;
    case (int)ButtonHandler::hl_UpVote:
      connect(map, SIGNAL(mapped(int)), this, SLOT(handleUpVote(int)), Qt::UniqueConnection);
      break;
    case (int) ButtonHandler::hl_OpenUrl:
      //connect(signalMapper, SIGNAL(mapped(int)), this, SLOT(handleUrl(int)), Qt::UniqueConnection);
      break;
    case (int)ButtonHandler::hl_DownVote:
      connect(map, SIGNAL(mapped(int)), this, SLOT(handleDownVote(int)), Qt::UniqueConnection);
      break;
    case (int) ButtonHandler::hl_DisposeBrowser:
      connect(map, SIGNAL(mapped(int)), this, SLOT(handleDispose(int)), Qt::UniqueConnection);
      break;
    }
}

  void KHUB::handleSearch() {
  //DO THE MATH
  upVoteMap = new QSignalMapper(this);
  downVoteMap = new QSignalMapper(this);
  openMap = new QSignalMapper(this);

   for (int pos = 0; pos < localUrl.size(); pos++){
    QLabel *link = new QLabel();
    link->setText("<a href=\"" + localUrl.at(pos) + "\">" + localUrl.at(pos) + "</a>");
    link->setTextFormat(Qt::RichText);
    link->setTextInteractionFlags(Qt::TextBrowserInteraction);
    link->setOpenExternalLinks(true);

    QPushButton *upArrow;
    btSetupInt(&upArrow, "Up Vote", 225, 300, 100, 25, &KHUB::handleUpVote, upVoteMap, pos, (int)ButtonHandler::hl_UpVote);

    QPushButton *open;
    btSetupInt(&open, "Open", 225, 300, 100, 25, &KHUB::handleUrl, openMap, pos, (int) ButtonHandler::hl_OpenUrl);

    QPushButton *downArrow;
    btSetupInt(&downArrow, "Down Vote", 225, 300, 100, 25, &KHUB::handleDownVote, downVoteMap, pos, (int)ButtonHandler::hl_DownVote);

    QLabel *separator = new QLabel();
    QLabel *guider = new QLabel("<----------------------------------------------------------------------------------------------------------------------------------------------------->");

    gridLayout->addWidget(link, componentsPos, 0, 1, -1);
    gridLayout->addWidget(guider, componentsPos, 1, 1, -1);
    gridLayout->addWidget(upArrow, componentsPos - 1, 2, 1, 1, Qt::AlignBottom);
    gridLayout->addWidget(open, componentsPos, 2, 1, 1);
    gridLayout->addWidget(downArrow, componentsPos + 1, 2, 1, 1, Qt::AlignTop);
    gridLayout->addWidget(separator, componentsPos + 2, 1, 1, 1);

    componentsPos = componentsPos + 5;
  }
  //KEEP DOING THE MATH
}

I believe ramtheconqueror's solution would work and for sure it's fancier than mine, although it wasn't applicable in my case since I'm working on a Thesis and to prove my POV I need to create a limited number of classes that were pre-established.

Anyway, I will assume his answer as correct because without him I wouldn't realise my own answer.

Unfortunately I still don't have the answer for my first question but I hope in general this helps someone in the future.

Zoe
  • 27,060
  • 21
  • 118
  • 148
Victor Oliveira
  • 3,293
  • 7
  • 47
  • 77