-
Notifications
You must be signed in to change notification settings - Fork 39
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/targets #108
Fix/targets #108
Conversation
@SamuelRott thoughts? This overlaps a bit with your new |
@@ -54,7 +54,7 @@ export const cards = [ | |||
energy: 1, | |||
damage: 5, | |||
block: 5, | |||
target: 'self and enemy', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this wasn't even implemented
public/ui/dragdrop.js
Outdated
} | ||
|
||
onAdd(cardElement.dataset.id, target, cardElement) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename onAdd
to afterRelease
?
Hello, I dont think this overlap the conditions logic. They both concern different part of the game and rely on different logic. For the long run I would say that card.target is not going to be enough. We are going to need card.allowedTarget = []. |
All cards have a
target
property that specifies which targets the card can be played at. Allowed ones are currentlyplayer
,enemy
andall enemies
.Before this PR, it wasn't really enforced e.g. a "Strike" card with
target: enemy
could also be played on yourself, the player.This PR changes that and makes sure the
card.target
is respected.If we later want to allow attacking yourself, we can now do that in a cleaner way by creating a new target
player and enemies
or something like that. But that's for later, at least now it's "clean".Closes #104 , fixes #106.