Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add player numbers to in game #2435

Merged
merged 8 commits into from
Mar 4, 2017
Merged

add player numbers to in game #2435

merged 8 commits into from
Mar 4, 2017

Conversation

ZeldaZach
Copy link
Member

@ZeldaZach ZeldaZach commented Mar 2, 2017

Fix #2417

@tooomm
Copy link
Member

tooomm commented Mar 2, 2017

Works very nice on game room join and displays the count in the title of the widget or undocked window. Also works in tabbed mode.

But if players join it doesn't update at all and keeps the initial value:
untitled

So what's the difference, that it instantly updates in the game view list but not here?

Copy link
Member

@Daenyth Daenyth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea why the count isn't correct - the code looks right to me. I'd have to dig deeper. If I have time later tonight I'll clone the branch and try it out locally.

Also your editor is configured for tabs - please recheck all files for tabs->spaces

}
playerListWidget->addPlayer(playerInfo);

qDebug() << "eventJoin retranslate w/ count = " << playerCountInRoom;
retranslateUi();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think emitUserEvent causes a retranslateUi to fire already - can you check?

@Daenyth Daenyth requested a review from ctrlaltca March 2, 2017 21:54
@@ -420,6 +421,8 @@ TabGame::TabGame(TabSupervisor *_tabSupervisor, QList<AbstractClient *> &_client

this->installEventFilter(this);
QTimer::singleShot(0, this, SLOT(loadLayout()));

playerCountInRoom = gameInfo.player_count();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tabs => spaces


cardInfoDock->setWindowTitle(tr("Card Info") + (cardInfoDock->isWindow() ? tabText : QString()));
playerListDock->setWindowTitle(tr("Player List") + (playerListDock->isWindow() ? tabText : QString()));
playerListDock->setWindowTitle(tr("Player List") + userCountInfo + (playerListDock->isWindow() ? tabText : QString()));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can get the number of players from the PlayerListWidget, just adding a method like:

int PlayerListWidget::getPlayersCount() { return players.size(); }

@ZeldaZach
Copy link
Member Author

@ctrlaltca Can you take over this PR? I'm not sure how to get it to work properly as nothing I've done suffices to get the correct data.

@ctrlaltca
Copy link
Contributor

@ctrlaltca Can you take over this PR? I'm not sure how to get it to work properly as nothing I've done suffices to get the correct data.

Please look https://github.com/ZeldaZach/Cockatrice/pull/3

@ZeldaZach
Copy link
Member Author

ZeldaZach commented Mar 4, 2017

This works as expected now. Thanks @ctrlaltca. Care to explain why your method works?
Can somebody review?

@ctrlaltca
Copy link
Contributor

ctrlaltca commented Mar 4, 2017

Care to explain why your method works?

The TabGame object already takes care of maintaining a list of the actual players and spectators. I just get the information from it when needed, instead of maintaining another counter and handling all the exceptions.

@tooomm
Copy link
Member

tooomm commented Mar 4, 2017

Working now. It also just counts real players, no spectators! 👍

@ZeldaZach ZeldaZach merged commit c850ae9 into Cockatrice:master Mar 4, 2017
@ZeldaZach ZeldaZach deleted the fix_2417 branch March 4, 2017 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants