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

Unconcede #3515

Merged
merged 1 commit into from
Jan 27, 2019
Merged

Unconcede #3515

merged 1 commit into from
Jan 27, 2019

Conversation

basicer
Copy link
Member

@basicer basicer commented Jan 23, 2019

Short roundup of the initial problem

  • In a multiplayer game, if a player concedes but wants to reenter the game, all the remaining players in the game must also concede, restart the match, and then get their cards back how they were.
  • When playing non-magic games like poker, a player can concede to sit out for a few hands, and then unconcede to resume playing.

What will change with this Pull Request?

  • Clicking concede when a player has already conceded will prompt them asking if they would like to rejoin the game.

@basicer basicer force-pushed the unconcede branch 2 times, most recently from 4ac4838 to 096ee71 Compare January 23, 2019 09:48
@ebbit1q
Copy link
Member

ebbit1q commented Jan 23, 2019

Testing this would require a test server, did you test this already?

@basicer
Copy link
Member Author

basicer commented Jan 23, 2019

Yeah I tested it.

I also have some patches to test it in offline mode by adding the buttons to the player menu.

@ctrlaltca
Copy link
Contributor

ctrlaltca commented Jan 23, 2019

It works, but it's not that different from actually quitting the game and re-joining, since your deck and zones contents will be reset to the initial state.
The nice part is that you can rejoin the game (and start over) without forcing other users to quit the game too, re-choose their decks, etc..
Two problems:

  • the concede menu entry should probably be renamed from "concede" to "unconcede" after a player conceded the game;
  • in a 2 player game, when you concede you get back to the deck/sideboard view. From there, you can click "concede" and the game will ask if you want to unconcede, but it won't do nothing. Maybe the menu entry should be disabled when game is not being played (in the deck/sideboard view).

@basicer
Copy link
Member Author

basicer commented Jan 23, 2019

the concede menu entry should probably be renamed from "concede" to "unconcede" after a player conceded the game;

True, this is nicer, but wasn't sure it was worth managing the state, especially since tab_game doesn't know much about the state of the player that happens to be localPlayer. At one point I had an extra menu item for Unconcede, but it seemed like a waste of space.

in a 2 player game, when you concede you get back to the deck/sideboard view. From there, you can click "concede" and the game will ask if you want to unconcede, but it won't do nothing. Maybe the menu entry should be disabled when game is not being played (in the deck/sideboard view).

This matches more or less what happens now. You get a concede button, and you can click it.. get the prompt.. and nothing happens. I guess ideally you'd remove the entire game menu minus "Leave Game" and "Game Information" during side-boarding.

void MessageLogWidget::logUnconcede(Player *player)
{
soundEngine->playSound("player_concede");
appendHtmlServerMessage(tr("%1 has unconceded the game.").arg(sanitizeHtml(player->getName())), true);
Copy link
Member

Choose a reason for hiding this comment

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

Not a big fan of the term "unconceded" maybe "rejoined"?

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought about rejoin, but that has some overlap with rejoining the room. I hate the term unconceded so Im down to change everything including the proto command names if you think rejoin is clear.

Copy link
Contributor

@ctrlaltca ctrlaltca left a comment

Choose a reason for hiding this comment

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

I'm good with merging this.
Some existing problem with the menu have been noticed when testing this PR, but being unrelated they can be fixed in a following PR.

@ZeldaZach ZeldaZach merged commit 7cd9b9c into Cockatrice:master Jan 27, 2019
@tooomm tooomm mentioned this pull request May 22, 2019
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