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

When two players have the same score, show Draw result #4

Merged
merged 2 commits into from
Oct 16, 2017
Merged

When two players have the same score, show Draw result #4

merged 2 commits into from
Oct 16, 2017

Conversation

Spidy88
Copy link
Contributor

@Spidy88 Spidy88 commented Oct 16, 2017

In my opinion, a better solution would be to modify the api results. Returning the "players" in order from highest to lowest score, means you can't map the score back to the original player order. It's an odd user experience to plugin a player 1 and a player 2, and then always have the highest score on the left side (essentially swapping player 1 and player 2 when player 2 is the winner). If the players were calculated and returned back in the original order they were submitted, then the scores could easily be mapped back to the original UI, resulting in a better UX (in my opinion of course).

draw-state

Closes #3

UPDATE
I ended up doing exactly as I suggested above and reworking the API to not over process the results. If for some reason you disagree, I can drop the last commit and only have the Draw state update.

player-positions-dont-change

@Harkindey Harkindey merged commit 529d6b8 into Harkindey:master Oct 16, 2017
@Harkindey
Copy link
Owner

Harkindey commented Oct 16, 2017

@Spidy88 I love this. Thanks. please do you work with React-Native i have an issue i havn't been able to work out. The Issue

@Spidy88
Copy link
Contributor Author

Spidy88 commented Oct 16, 2017

@Harkindey I will take a look at that issue. Probably won't have time til tomorrow to submit code for 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.

None yet

2 participants