-
Notifications
You must be signed in to change notification settings - Fork 0
Conversation
make GenreTemporary.kt a super class, let typing game and lyrics game inherit from it.
add progress bar use game manager
It seems that the duplication is because of import statements... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some minor comments but overall good job!
app/src/androidTest/java/ch/sdp/vibester/activity/LyricsBelongGameActivityTest.kt
Show resolved
Hide resolved
app/src/androidTest/java/ch/sdp/vibester/activity/TypingGameActivityTest.kt
Outdated
Show resolved
Hide resolved
The separation of a game manager for all games I like! Thank you very much! And the lyrics game is coming soon! yayyy As you said in your description, the app crashes in the end when displaying wrong songs. There is also a little weird behaviour:
Looking forward to working game! |
getAndCheckLyrics(songName, artistName, speechInput, gameManager) | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
} | |
} | |
h.post(runnable!!) |
Now the progressbar should work!
@@ -70,6 +66,7 @@ class GameManager : Serializable { | |||
/** | |||
* Get a current playing song | |||
*/ | |||
@JvmName("getCurrentSong1") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Question] what is the reason to apply JvmName here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's Android Studio's suggestion, otherwise I got compile error saying "Platform declaration clash: The following declarations have the same JVM signature (getCurrentSong()Lch/sdp/vibester/model/Song;):
public open fun (): Song defined in ch.sdp.vibester.helper.GameManager
public final fun getCurrentSong(): Song defined in ch.sdp.vibester.helper.GameManage"
ignoreCase = true | ||
) | ||
) "res: correct" else "res: too bad" | ||
private fun checkLyrics(lyricToBeCheck: String, lyrics: String, gameManager: GameManager) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What will happen if the speech input is null? Will the sonf be added to incorrect?
In general, if speech input is null, is there a need to fetch song, or we can just continue within the next sonf?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now, I don't have the answer for your second question. If the speech input from SpeechRecognizer is null, the SpeechRecognizer itself will crash the app, if we call it without initialize, it will crash, and if the speech input is "" (empty) the check pass as correct.
This is something I need to fix in my next PR.
speechInput, | ||
lyrics | ||
) // be sure the lyrics is ready when checking | ||
checkLyrics( speechInput, lyrics, gameManager) // be sure the lyrics is ready when checking | ||
} else { | ||
findViewById<TextView>(R.id.lyricMatchResult).text = | ||
"No lyrics found, try another song" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the progress bar should be stopped if lyrics are not found
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for spotting this out, I will replace all the use of the next button to the progress bar and redesign the switch logic in my next PR.
} else { | ||
gameManager.addWrongSong() | ||
findViewById<TextView>(R.id.lyricMatchResult).text = "res: too bad" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, then the result arrives the progress bar is not stopped
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM now!
Thanks for your feedback, I will take them into account in a separate PR. (The reason is to make my time estimation more accurate, familiarize myself with the progress bar, and come up with a better solution) |
Code Climate has analyzed commit 4b24ba5 and detected 2 issues on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 87.0% (80% is the threshold). This pull request will bring the total coverage in the repository to 91.2% (0.6% change). View more on Code Climate. |
turn lyricsBelongToActivity into a proper game:
- add a progress bar
- add genre menu
- add game ending screen
refactor common elements from the lyrics game and the typing game
- extract genre
- extract game activity
- extract game manager
Issues in the lyrics game need to be fixed (In another PR):
- progress bar doesn't decrease, need to manually click the next button
- the logic of next, playRound and checkAnswer need to be reconsidered:
currently, we can go to the end of the game by clicking the next button, but it won't save the wrong song which results crash on the game-ending screen
Walk-around: say anything and check the answer then click the next button