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

Alchemy #118

Merged
merged 45 commits into from
Sep 3, 2024
Merged

Alchemy #118

merged 45 commits into from
Sep 3, 2024

Conversation

jbw3
Copy link
Contributor

@jbw3 jbw3 commented Aug 12, 2024

Alchemy!
I've added all the Alchemy cards.

Notable changes:

The big change was adding the potion cost to cards. I added a new Cost class with money and potions properties. Now all cards cost property is of type Cost. I overloaded a lot of operators for the Cost class (==, !=, <, <=, >, >=, +, -) to make logic easier, and, in most cases, this made the cost comparison logic for existing cards backwards compatible. The Cost class could also be modified to handle debt costs someday.

Alchemist and Herbalist used the Effect logic.

Possession was, of course, the most challenging to implement. I had to make a few changes to the Player class to support it.

@evanofslack
Copy link
Owner

Holy hell I missed this, checking it out now!

Comment on lines +246 to +247
# if player played Possession while being possessed, take the extra turn
# before their main turn
Copy link
Owner

Choose a reason for hiding this comment

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

Copy link
Owner

@evanofslack evanofslack left a comment

Choose a reason for hiding this comment

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

Actually amazing work, this is so cool.

My only comment would be I'd prefer if the formatting/tidying changes were in a separate PR (i.e. the List -> list type changes). It doesn't really matter though, would just make it easier to review!

@evanofslack evanofslack merged commit d39b4fd into evanofslack:master Sep 3, 2024
1 check 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