Skip to content
This repository has been archived by the owner on Jan 31, 2024. It is now read-only.

margaux/typing game score #257

Merged
merged 4 commits into from
May 7, 2022
Merged

margaux/typing game score #257

merged 4 commits into from
May 7, 2022

Conversation

zwierski
Copy link
Collaborator

@zwierski zwierski commented May 7, 2022

This PR updates the Typing Game by showing the player's current score on the game screen.

pr_round_score
pr_round_score_2

@codeclimate
Copy link

codeclimate bot commented May 7, 2022

Code Climate has analyzed commit 6963833 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (80% is the threshold).

This pull request will bring the total coverage in the repository to 82.3% (0.0% change).

View more on Code Climate.

@zwierski zwierski linked an issue May 7, 2022 that may be closed by this pull request
@zwierski zwierski marked this pull request as ready for review May 7, 2022 12:40
@zwierski zwierski self-assigned this May 7, 2022
@MaximeZmt MaximeZmt self-requested a review May 7, 2022 14:38
Copy link
Collaborator

@jiabaow jiabaow left a comment

Choose a reason for hiding this comment

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

I feel like having the game size displayed along with the score is more indicative for the player (like 3/5, 2/6), what do you think?

@@ -223,6 +223,8 @@ class TypingGameActivity : GameActivity() {
findViewById<LinearLayout>(R.id.displayGuess).removeAllViews()
findViewById<EditText>(R.id.yourGuessET).text.clear()
findViewById<EditText>(R.id.yourGuessET).isEnabled = true
val score = gameManager.getCorrectSongs().size
findViewById<TextView>(R.id.playerScore).text = "@string/your_score_dot_dot $score"
Copy link
Collaborator

Choose a reason for hiding this comment

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

it shows @string/your_score_dot_dot instead of "your score" when I tried it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will fix

@zwierski
Copy link
Collaborator Author

zwierski commented May 7, 2022

I feel like having the game size displayed along with the score is more indicative for the player (like 3/5, 2/6), what do you think?

I think having something else right after the score will look cluttered. Maybe 2 lines, one with the song number (in the game) on the top and one with the score underneath?

not showing directly 3/5, just the idea of having the score and at which song the player is currently playing.
It's true that he knows the size of the game he chose but he might forget which song he is currently at (the 4th or the 5th for example).

@@ -223,6 +223,8 @@ class TypingGameActivity : GameActivity() {
findViewById<LinearLayout>(R.id.displayGuess).removeAllViews()
findViewById<EditText>(R.id.yourGuessET).text.clear()
findViewById<EditText>(R.id.yourGuessET).isEnabled = true
val score = gameManager.getCorrectSongs().size
findViewById<TextView>(R.id.playerScore).text = "@string/your_score_dot_dot $score"
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
findViewById<TextView>(R.id.playerScore).text = "@string/your_score_dot_dot $score"
findViewById<TextView>(R.id.playerScore).text = "@string/your_score_dot_dot $score"

Great changes 😃
Small comment:

  • Can you change the string name according to the convention here [https://github.com/MaximeZmt/SDP_2022-Vibester/wiki] (We try to keep things ordered for new string like that 😉) It would be typingGame_yourScore (and can also change its place in the string.xml at the top of the file in the section if it exists or create TypingGame)

Copy link
Owner

@MaximeZmt MaximeZmt left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Collaborator

@jiabaow jiabaow left a comment

Choose a reason for hiding this comment

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

LGTM

@zwierski zwierski merged commit 0adf82a into main May 7, 2022
@zwierski zwierski deleted the margaux/typing-game-score branch May 7, 2022 16:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a visible score to the typing game
3 participants