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

[DSK] Implement Kaito, Bane of Nightmares #13187

Merged
merged 3 commits into from
Jan 3, 2025
Merged

Conversation

jackd149
Copy link
Contributor

@jackd149 jackd149 commented Dec 27, 2024

This PR implements Kaito, Bane of Nightmares

@jackd149 jackd149 changed the title [DSK] Implement Kaito, Bane of Monsters [DSK] Implement Kaito, Bane of Nightmares Dec 27, 2024
@jackd149
Copy link
Contributor Author

Part of #12534

Copy link
Contributor

@xenohedron xenohedron left a comment

Choose a reason for hiding this comment

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

a couple nits but looks fine otherwise

}
}

enum NumberOfOpponentsWhoLostLife implements DynamicValue {
Copy link
Contributor

Choose a reason for hiding this comment

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

rename to KaitoBaneOfNightmaresCount - we don't use generic class names if they're specific to a single card like this

return this.calculate(game, sourceAbility.getControllerId());
}

public int calculate(Game game, UUID controllerId) {
Copy link
Contributor

Choose a reason for hiding this comment

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

don't bother with this extra sub method, it's not standard style. Put the logic directly in the overridden method, it's okay that it doesn't use all its params

@jackd149
Copy link
Contributor Author

jackd149 commented Jan 2, 2025

Thanks for the review. I made the changes as requested.

Copy link
Contributor

@xenohedron xenohedron left a comment

Choose a reason for hiding this comment

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

thanks for the contribution

@xenohedron xenohedron merged commit fd4b826 into magefree:master Jan 3, 2025
3 checks passed
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.

2 participants