"Show related card" option added to ingame context menu #3115
Conversation
|
I'm concerned that "Show card" might be misleading to users. It's very similar to "reveal card" |
|
Yeah, that really may be so. Do you have a suggestion what to rename it to? |
|
"Display card info"? Maybe? @ZeldaZach @tooomm thoughts? |
cockatrice/src/player.cpp
Outdated
| cardMenu->addSeparator(); | ||
| auto *signalMapper = new QSignalMapper(this); | ||
| for (const CardRelation *relatedCard : relatedCards) { | ||
| QAction *viewCard = cardMenu->addAction("Show Card: \"" + relatedCard->getName() + "\""); |
There was a problem hiding this comment.
...->addAction(tr("View Card") + ":\"" + relatedCard->getName() + "\"");
There was a problem hiding this comment.
Note especially the tr, that's important so the string gets translated for other languages
cockatrice/src/player.cpp
Outdated
| @@ -2685,6 +2686,28 @@ void Player::updateCardMenu(const CardItem *card) | |||
| cardMenu->addAction(aClone); | |||
| } | |||
| } | |||
| addRelatedCardView(card, cardMenu); | |||
There was a problem hiding this comment.
While this location is good for cards in hand, it's really inconvenient for cards in play. Either we should make this feature only work in hand, or we should place it somewhere better when the card's in play. Similar issues on the stack.
There was a problem hiding this comment.
For cards in play I'd actually put it near the token menu, maybe above or below
There was a problem hiding this comment.
My thought was to view related card images > submenu it
|
I left the option at the end of the context menu, but changed its name ("View related card info:") and added it as a submenu. Also added the |
| @@ -2612,6 +2612,8 @@ void Player::updateCardMenu(const CardItem *card) | |||
|
|
|||
| if (card->getZone()) { | |||
| if (card->getZone()->getName() == "table") { | |||
| // Card is on the battlefield | |||
There was a problem hiding this comment.
I'm not sure this comment adds any clarity; the line above says the same thing.
There was a problem hiding this comment.
Added since I added to all of them
cockatrice/src/player.cpp
Outdated
| cardMenu->addAction(aPlay); | ||
| cardMenu->addAction(aPlayFacedown); | ||
| cardMenu->addSeparator(); | ||
| cardMenu->addAction(aClone); | ||
| cardMenu->addMenu(moveMenu); | ||
| } else { | ||
| // Card is in hand |
There was a problem hiding this comment.
in hand or any custom zone that the server is using which the client isn't aware of, would be more accurate.
|
Screenshots Updated |
|
I see... Can you add it in for that case? |
|
I just have to say, with the tokens, it seems really weird to see the same card twice. Maybe we can move the create tokens to a sub dialog? What do y'all think? |
|
We probably shouldn't nest the menu too deeply, since that makes it more
annoying to use, and related cards are such a common interaction
…On Sat, Feb 17, 2018 at 3:03 AM Zach H ***@***.***> wrote:
I just have to say, with the tokens, it seems really weird to see the same
card twice. Maybe we can move the create tokens to a sub dialog? What do
y'all think?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#3115 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAA5NCBCuZMvzVaGVf5hHGo2F8w05yvhks5tVoe2gaJpZM4SHmES>
.
|
Related Ticket(s)
Short roundup of the initial problem
There was no way to know the details of a card's reverse side without actually creating that card in-game.
What will change with this Pull Request?
Screenshots