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

Added player's choice for playing pawn #2

Merged
merged 4 commits into from
Apr 22, 2024
Merged

Conversation

daniPclos
Copy link
Contributor

In the original version the board's pawns are iterated over each turn so players can't choose what pawn they want to use and have to alternate between the pawns. In this change set I have added the functionality that enables players to choose what pawn they want to use in addition to the move and build choices. The players now only receive the board when playing their moves (I kept the old functionality for placing the pawns) and need to return the pawn number, in addition to the move and build positions. I have adapted the existing players to play accordingly.
For your information, this is my first pull request so I'm a bit uncertain how does this process work, so any guidance and advice will be welcomed! :)

@Tomansion
Copy link
Owner

Hi @daniPclos, thank you for contributing to this project !

I'm playing Santorini mostly in 2v2 with everyone playing a pawn and playing one after the other, so I was surprised to know that you can select the pawn that you want to play. When I realized, the project was already well started and making an AI is simpler when you don't have to select the pawn you need to play, so I didn't change it in this project.

Creating a pull request with a nice comment is the best you can do for a pull request.

However, I can't accept the changes until :

  • The code is formatted with Python Black (this project code linter)
  • The tests are passing
  • The documentation (the Readme) is up to date with the changes

However, those are not the most fun changes to make, so I've made them in the pull request #3. I've also made other changes that I hope you won't mind, fill free to copy this branch and test it, leave a review if you want or to ignore it ( ͡ᵔ ͜ʖ ͡ᵔ) .

Thank you again for your contribution and good luck with whatever you are doing with this project.

@Tomansion Tomansion merged commit f14c603 into Tomansion:main Apr 22, 2024
0 of 4 checks passed
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