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

Fix additional credit costs not being ignored by :ignore-costs :rez-costs #7216

Merged
merged 2 commits into from
Nov 15, 2023

Conversation

butzopower
Copy link
Contributor

Fixes #7214
Fixes #6824

@butzopower
Copy link
Contributor Author

Not sure if this is better with the refactor or was better with the remove filtering out the credit costs.

(get-effects state side :rez-additional-cost card))))
([state side card] (rez-additional-cost-bonus state side card (constantly true)))
([state side card pred]
(filter pred (merge-costs
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd prefer (let [costs (merge-costs ...)] (if pred (filterv pred costs) costs)). No need to do extra work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

happy with either, kinda took the first one to be more functional / less having to declare extra state but also think the if does read more straightforward

Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel you. Applying a single function to everything is nice. Conditionals make logic harder to follow.

I don't think of items in let bindings as state because they don't persist. Immutable data means we can let bind all we want for free.

@@ -26,7 +26,8 @@
(= :all-costs ignore-cost) [:credit 0]
alternative-cost alternative-cost
:else (let [cost (rez-cost state side card {:cost-bonus cost-bonus})
additional-costs (rez-additional-cost-bonus state side card)]
additional-costs (rez-additional-cost-bonus state side card #(or (nil? ignore-cost)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't like this here. Instead, (when ignore-cost #(= :credit ...)). Along with the change above, no need to check every step.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah ya this is much better and makes sense then for the above to use nil instead of a true function

@butzopower butzopower force-pushed the tread-lightly-ignore-costs branch from 5c5ada4 to e002551 Compare November 15, 2023 13:19
@NoahTheDuke NoahTheDuke merged commit 6dd0371 into mtgred:master Nov 15, 2023
3 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
2 participants