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

Judge mode #3531

Merged
merged 17 commits into from
Feb 21, 2019
Merged

Judge mode #3531

merged 17 commits into from
Feb 21, 2019

Conversation

basicer
Copy link
Member

@basicer basicer commented Jan 27, 2019

Short roundup of the initial problem

  • When judges enter a game to fix problems, it can be difficult to get the players to do the right thing.
  • Some non-magic involve having a deal, banker, or game master that should be able to manipulate all the cards.
  • When cockatrice to players, it can be helpful to just do some things for them while they are still learning.

What will change with this Pull Request?

  • Adds a new flag to users on the server, "Judge".
  • Admins can add/remove judges the same as moderators.
  • Adds a new flag to players in a game "Judge". Judes can be both players and spectators.
  • A judge can compel a player to take an action by using that player's player area (this is similar to how offline multiplayer work).
  • A judge has a special gavel indicator in the player list.
  • If a judge uses this power, a log message is generated telling players the judge is using their special ability.
  • Server Judges can join as a judge by holding down shift when the join a game.
  • A host can create a game as a judge by holding down shift as they create the game, but only if the server has a special flag set (defaults to off).

Screenshots

image

@ZeldaZach
Copy link
Member

I'm reading this over and the code seems decent, just have some questions.

  1. Who are "judges?" Are they defined by the server operator, similar to the Donator, Vip, Moderator etc. stuff, or are the defined by the game creator?

  2. The shift key thing is a bit awkward for me. Judges should be allowed in any room, and not be allowed to bypass passwords. Maybe have a checkbox on by the "Join Room" button that is similar to [ x ] Judge Mode would be more helpful

  3. I see moderators can bypass with shift, which is fine by me as moderators are defined as more advanced users of the program. Up to you if you want to include the judge mode checkbox I defined above for them. Might be clever, and make sure to incorporate it into the mod powers enabled/disabled menu so if they are not normally a judge, it would grey out / uncheck if they are not on mod power mode (albeit this doesn't happen much anymore, but still)

Overall, this is awesome!! Keep up the great work, and welcome to Cockatrice development :)

@basicer
Copy link
Member Author

basicer commented Jan 27, 2019

I'm actually not sure who judges are yet, which is why I didn't add the checkbox to the create_game dialog. I see the primary users of this being the judges of MagicLeague if that ever gets going again or some future similar thing

One thing I don't want for example is people creating their games with judge mode for no reason, and using it to cheat or troll players who don't yet understand judge mode.

@ZeldaZach
Copy link
Member

Sounds like we'll need to add a new rank to the system. Can you update the database in some way to allow for people to be a judge in addition to other titles. Currently we have an Admin/Moderator/None option and a VIP/Donator/None option

@basicer
Copy link
Member Author

basicer commented Jan 27, 2019

I'm just wondering if it is ever the case you trust people enough to be a judge but not a moderator.

@skwerlman
Copy link
Contributor

skwerlman commented Jan 28, 2019

I'm just wondering if it is ever the case you trust people enough to be a judge but not a moderator.

a judge is a different role from a moderator. a moderator is delegated authority over an entire server; a judge should only be given authority over the games on the server people want judges in.

Maybe have a checkbox on by the "Join Room" button that is similar to [ x ] Judge Mode would be more helpful

imo a "join game as judge" button that appeared for judges on to games that allow judges would be better.

Adds a new flag to players in a game "Judge". Judes can be both players and spectators.

i'm not a fan of judges being allowed to both play and judge in the same game at the same time; it should really be an either-or kind of thing just to mitigate the potential for abuse

Some non-magic involve having a deal, banker, or game master that should be able to manipulate all the cards.

I think this case deserves a separate descriptor, and should be per-game, rather than per-server. creating a game with "judge" permissions should be visibly distinct from a user the server op has explicitly granted judge authority to

aside from those notes, i really like this proposal; having a formal system for judging in trice is a big improvement over the current state, where one has to find a judge elsewhere (assuming they even know where to look)


(ancient) related discussion: #206

@ctrlaltca
Copy link
Contributor

Suggestion: use a libra icon

@ZeldaZach
Copy link
Member

Seeing if there are any updates on this :)

@basicer
Copy link
Member Author

basicer commented Feb 10, 2019

image

@basicer basicer changed the title [RFC] Judge mode Judge mode Feb 10, 2019
@basicer basicer force-pushed the judge-mode branch 3 times, most recently from 6975615 to ebed247 Compare February 11, 2019 10:13
@tooomm
Copy link
Member

tooomm commented Feb 11, 2019

  1. I think the little icons in the pawns are not ideal. They are really difficult to recognize.

  2. Also, I had the idea to make the judge actions/log messages a different color in the log.

  3. Do you want to stick with the hammer, or do you prefer that judge/law scale instead?

@basicer
Copy link
Member Author

basicer commented Feb 12, 2019

  1. I think the little icons in the pawns are not ideal. They are really difficult to recognize.

For sure. We've been talking about reworking the pawn system completely. Ideally we would unify the language, and do more things procedually (like how the gavel is burned in now).

  1. Also, I had the idea to make the judge actions/log messages a different color in the log.

What color were you thinking?

  1. Do you want to stick with the hammer, or do you prefer that judge/law scale instead?

No preference, whatever looks better.

@tooomm
Copy link
Member

tooomm commented Feb 12, 2019

What color were you thinking?

Maybe just black as all log is colored currently?

@tooomm
Copy link
Member

tooomm commented Feb 18, 2019

Can you post a screenshot of how the log and player area are looking in the current version, please?


I would also consider the "Scale" icon over the gavel: https://icons8.com/icon/61021/measurement-scale

While the gavel is a perfectly fine symbol for that purpose in itself, it looks too similar to the other weapon (sword) which is used for a "Ready Player" in my opinion.

@basicer
Copy link
Member Author

basicer commented Feb 19, 2019

image

@basicer
Copy link
Member Author

basicer commented Feb 19, 2019

image

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.

Great PR! I noticed a few minor ones that you may want to check, thank you

cockatrice/src/playerlistwidget.cpp Show resolved Hide resolved
cockatrice/src/tab_game.cpp Outdated Show resolved Hide resolved
common/server_player.cpp Outdated Show resolved Hide resolved
common/server_player.cpp Show resolved Hide resolved
common/server_player.cpp Show resolved Hide resolved
common/server_protocolhandler.cpp Show resolved Hide resolved
servatrice/src/serversocketinterface.cpp Outdated Show resolved Hide resolved
- Fix sort order
@ZeldaZach
Copy link
Member

I'm somewhat ready to roll this in and see what happens. Worst comes to worst, 2.7.1 :)

cc @ctrlaltca

@ctrlaltca
Copy link
Contributor

Before merging, please document the new option in servatrice.ini.example

Copy link
Member

@ZeldaZach ZeldaZach left a comment

Choose a reason for hiding this comment

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

Seems great. Can't wait to put this into place :)

@ZeldaZach
Copy link
Member

Is there any database migration required for this? Or does it work with the current layout of the DB?

@basicer
Copy link
Member Author

basicer commented Feb 21, 2019

admin is an int field so it fires right up.

@ZeldaZach ZeldaZach merged commit ea8201a into Cockatrice:master Feb 21, 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

5 participants