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

Replace class indication in opponent's deck name with class icon in decktracker #264

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

piedpiper358
Copy link
Contributor

Also add default values for opponent name and class

Before:

2022-10-15_21-53-45 2
2022-10-16_01-50-07 2
2022-10-16_02-16-27

After:

2022-10-16_22-47-58

2022-10-18_00-01-58

@sebastientromp
Copy link
Contributor

Could you explain a bit what motivates the change? Are there cases where it's difficult to read the opponent's class from the game itself, or needed to recap it? How would you handle dual-class heroes?

@piedpiper358
Copy link
Contributor Author

piedpiper358 commented Oct 24, 2022

@sebastientromp, The phrase '{{playerName}} as {{class}}' has no short equivalent in Russian, perhaps in some non-English languages ​​too. So I translate it as '{{playerName}} | {{class}}' which doesn't look very nice.

It seems to me that class information is only useful on mulligan, when it is difficult to see the opponent due to offered cards. In this case, player would probably like to see opponent class at a glance.

For duels, it would probably be more useful to display the opponent's hero name rather than his class.

@piedpiper358 piedpiper358 removed their assignment Nov 6, 2022
@sebastientromp
Copy link
Contributor

Thanks. Right now I'm not convinced that showing the class icon is more important than showing the name (to be sure, none of which are really important).
Having an icon looks nice visually though (it adds more color), so let me think about it.

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.

2 participants