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

Add button for user to choose their kingdom color #73

Merged
merged 11 commits into from
Apr 8, 2024

Conversation

AnthonyClayton12
Copy link
Contributor

I added a button for the user to select kingdom color. In doing so, the game preference kingdom color becomes the new selected color and the colors get swapped around accordingly. This maintains the balance and randomness so that the user can not select the kingdom with the best advantage, they can only change the color of the kingdom given to them. I will attach below information about the process I took.

Screenshot 2024-03-16 181520

US card - kingdom color

new button
new button pressed
new color selected
new green kingdom selected

all changed class commit

@AnthonyClayton12
Copy link
Contributor Author

I have implemented these changes into the preferences as well so that the last color the user played as will be the color already selected by default. This addition has caused no issues, gameplay unaffected, no crashes thus far. If you have any more questions, please feel free to reach out to me.

@Sesu8642
Copy link
Owner

Sesu8642 commented Apr 4, 2024

Thank you very much! I will try to review the code soon.
You also documented this very well. Is it for some university assignment?

@Sesu8642 Sesu8642 mentioned this pull request Apr 4, 2024
@AnthonyClayton12
Copy link
Contributor Author

AnthonyClayton12 commented Apr 5, 2024 via email

@Sesu8642
Copy link
Owner

Sesu8642 commented Apr 5, 2024

Please make sure to add your name to the CLA.

@Sesu8642
Copy link
Owner

Sesu8642 commented Apr 5, 2024

Other than that, there are some minor discrepancies from the code style i configured but I can fix that easily before merging.

.vscode/launch.json Outdated Show resolved Hide resolved
@Sesu8642
Copy link
Owner

Sesu8642 commented Apr 7, 2024

As discussed in #24, I'd prefer to have this setting in the preferences page. However, the current solution is functional and can be merged. Let me know if you'd like to put in some more work and move the setting or not.

@AnthonyClayton12
Copy link
Contributor Author

@Sesu8642 I do not have much time to change it right now as college finals are around the corner and I have to focus my time on that at the moment. However, I would be able to change it next month when the semester is over unless you are okay with the current implementation. It allows the user to select the color before the game and it will save the last choice so that they would not have to choose it again until they close the application. Let me know your preference, otherwise, I am okay with the current solution being merged if you allow it. Thank you.

@Sesu8642
Copy link
Owner

Sesu8642 commented Apr 8, 2024

Let's merge it now. If you want to contribute more after your finals, just let me know 😃

@Sesu8642 Sesu8642 merged commit 288358e into Sesu8642:master Apr 8, 2024
@Sesu8642
Copy link
Owner

Sesu8642 commented Apr 8, 2024

Thanks for your contribution!

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