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

[ACR] Implement Eivor, Wolf-Kissed #12499

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

theelk801
Copy link
Contributor

Opening this as a PR in case anyone takes issue with the interface I added or anything else

@ssk97
Copy link
Contributor

ssk97 commented Jun 21, 2024

CardTypeAssignment is now just a strict subset of your TypeAssignment, right? Should probably just be one.

@xenohedron
Copy link
Contributor

xenohedron commented Jun 21, 2024

Have you seen PredicateCardAssignment and TargetCardAndOrCard? #11497

I think it's best to use a common target class and not a custom target (for example, that includes the possibleTargets method which your implementation is missing).

I also think it's probably cleaner to just pass in predicates rather than add a new interface and method. Since predicates are more general and thus can be extended to things that aren't types (e.g. colors) as well as and/or/not combinations.

So basically, I think all you'd have to do is change a constructor from protected to public and use:

new TargetCardAndOrCard(SubType.SAGA.getPredicate(),
        CardType.LAND.getPredicate(),
        "a Saga card and/or a land card")

@theelk801
Copy link
Contributor Author

yeah I didn't realize that exists, I'll adapt it to use that

@xenohedron xenohedron marked this pull request as draft June 25, 2024 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants