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

Margaux/setup-to-gamescreen-connection #105

Merged
merged 16 commits into from
Mar 30, 2022
Merged

Conversation

zwierski
Copy link
Collaborator

@zwierski zwierski commented Mar 24, 2022

This PR provides the functionality of choosing the number of players and entering their names in a buzzer game. The spinner in the setup screen will make the correct number of editable name fields appear, and these names + the number of players are used to create the scoreboard and buzzers in the game screen.
pr_stgsc_2
pr_stgsc_1
pr_stgsc_3
pr_stgsc_4

* n: number of players selected in the spinner
*/
fun updatePlayerNameVisibility(n: Int) {
findViewById<EditText>(R.id.namePlayer2).visibility = if (n>=2) android.view.View.VISIBLE else android.view.View.INVISIBLE
Copy link

Choose a reason for hiding this comment

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

Similar blocks of code found in 3 locations. Consider refactoring.

*/
fun updatePlayerNameVisibility(n: Int) {
findViewById<EditText>(R.id.namePlayer2).visibility = if (n>=2) android.view.View.VISIBLE else android.view.View.INVISIBLE
findViewById<EditText>(R.id.namePlayer3).visibility = if (n>=3) android.view.View.VISIBLE else android.view.View.INVISIBLE
Copy link

Choose a reason for hiding this comment

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

Similar blocks of code found in 3 locations. Consider refactoring.

fun updatePlayerNameVisibility(n: Int) {
findViewById<EditText>(R.id.namePlayer2).visibility = if (n>=2) android.view.View.VISIBLE else android.view.View.INVISIBLE
findViewById<EditText>(R.id.namePlayer3).visibility = if (n>=3) android.view.View.VISIBLE else android.view.View.INVISIBLE
findViewById<EditText>(R.id.namePlayer4).visibility = if (n>=4) android.view.View.VISIBLE else android.view.View.INVISIBLE
Copy link

Choose a reason for hiding this comment

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

Similar blocks of code found in 3 locations. Consider refactoring.

@zwierski zwierski marked this pull request as ready for review March 30, 2022 12:44
@zwierski zwierski changed the title Margaux/buzzer game logic Margaux/setup-to-gamescreen-connection Mar 30, 2022
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.

👍 Nice !

Copy link
Collaborator

@kamilababayeva kamilababayeva left a comment

Choose a reason for hiding this comment

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

I am so happy it finally worked! I tried it myself, works like a charm.
I have added some comments, up to you if you wanna change them!

Also, add the pr description/screenshots for next PRs. I think it is easier to understand the picture of your task. thanks!


public class BuzzerScoreUpdaterTest {

// FIXME: Uncomment this when the infinite loop issue in the class is fixed
Copy link
Collaborator

Choose a reason for hiding this comment

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

The infinite loop problem! you forgot to increment i in your buzzerCode:

var i = 0
while (i < scores.size) {
theMap.put(ids[i], scores[i])
}

Happens to all of us!!! I hope it helped you!

fun proceedToGame(view: View) { //FILLER INTENT
val intent = Intent(this, GamescreenActivity::class.java)
intent.putExtra("Number of players", text)
//intent.putExtra("Number of players", text)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
//intent.putExtra("Number of players", text)


val allPoints = arrayOf(1, 2, 3, 4)
println(nPlayers)
print(players) // test
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would remove these print statements

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, they're test statements I forgot to remove. Will do!

val buzIds = players?.let { fetchBuzIdArray(it.size) }

if (players != null && allPoints != null) { buildScores(players, allPoints) }
if (players != null && buzIds != null) { buildBuzzers(players, buzIds, answer) }
setAnswerButton(answer, findViewById(R.id.buttonCorrect))
setAnswerButton(answer, findViewById(R.id.buttonWrong))
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe instead of setting setAnswerButton function for buttons here, you can set them in xml file. For each button have android:onclick and set setAnswerButton function there.

}

/*
Programmatically builds the buzzers according to the number and names of players.
*/
private fun buildBuzzers(players: Array<String>, answer: LinearLayout) {
// the correct number of buzzers are built but the names don't show up...
Copy link
Collaborator

Choose a reason for hiding this comment

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

The names are displayed correctly in my screen:
image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I forgot to remove this comment after fixing that issue. 😅 It's all good now

buildBuzzers(players, answer)
val buzIds = players?.let { fetchBuzIdArray(it.size) }

if (players != null && allPoints != null) { buildScores(players, allPoints) }
Copy link
Collaborator

Choose a reason for hiding this comment

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

for buildScore function: I am not sure if you need allPoints here, since it is initialization and it is always 0


answerText.text= "The song was $song by $artist"
val playersFull = getIntent?.getStringArray("Player Names")
val players = nPlayers?.let { playersFull?.copyOfRange(0, it) }
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would move getting intent extras at one place. Is there a reason why you copy from playerfull to players? I would assume the lengthof player names is according to number of players

@codeclimate
Copy link

codeclimate bot commented Mar 30, 2022

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

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

This pull request will bring the total coverage in the repository to 90.9%.

View more on Code Climate.

@zwierski zwierski merged commit 5402c89 into main Mar 30, 2022
@zwierski zwierski deleted the margaux/buzzer-game-logic branch March 30, 2022 16:45
@zwierski zwierski linked an issue Mar 30, 2022 that may be closed by this pull request
@zwierski zwierski self-assigned this Mar 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants