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

refactor: TargetCreatureOrPlayer inheritance #13199

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

xenohedron
Copy link
Contributor

closes #11161

updates TargetCreatureOrPlayer (rarely used) to extend TargetPermanentOrPlayer (like similar classes do) rather than reimplement all the code

@github-actions github-actions bot added the cards label Jan 4, 2025
@JayDi85
Copy link
Member

JayDi85 commented Jan 4, 2025

Not for PR

Looks like miss filter settings, (2 params instead 4), so player part will ignore filters like controlled. Must use 4 params as much as possible.

IMG_1042

@JayDi85
Copy link
Member

JayDi85 commented Jan 4, 2025

Looks like canChoose miss logic of isNotTarget. It must call canBeTargeted check for “!isNotTarget” only like canTarget do above:
IMG_1043

P.S. Potential bugs: player can’t choose permanents with protection for non targeted effects. CanChoose is rare directly used in custom effects, so it must be a rare reported bug.

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.

Refactor: have TargetCreatureOrPlayer extend TargetPermanentOrPlayer
2 participants