Sesu8642 / FeudalTactics

Strategy game with countless unique and challenging levels.
GNU General Public License v3.0
79 stars 20 forks source link

Add button for user to choose their kingdom color #73

Closed AnthonyClayton12 closed 7 months ago

AnthonyClayton12 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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

AnthonyClayton12 commented 7 months ago

Thank you, yes it is a university assignment. We had to find an open source to contribute on to become more familiar with the process from planning to implementation on adding features or fixing bugs.

On Thu, Apr 4, 2024 at 4:51 PM Sesu8642 @.***> wrote:

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

— Reply to this email directly, view it on GitHub https://github.com/Sesu8642/FeudalTactics/pull/73#issuecomment-2038195637, or unsubscribe https://github.com/notifications/unsubscribe-auth/A3K7H2HIBUIXAWKDZ2F5I4DY3W4NNAVCNFSM6AAAAABFXQ6IHKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMZYGE4TKNRTG4 . You are receiving this because you are subscribed to this thread.Message ID: @.***>

Sesu8642 commented 7 months ago

Please make sure to add your name to the CLA.

Sesu8642 commented 7 months ago

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

Sesu8642 commented 7 months ago

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 commented 7 months ago

@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 commented 7 months ago

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

Sesu8642 commented 7 months ago

Thanks for your contribution!