Making cards with ability require a target-selection #115
Conversation
|
This commit is really wrong, but is the first approach I managed to do for solving the issue.
Any help with this two problems will be appreciated. |
Masclins
changed the title
|
Now the only problem is making the card green when clicking an owned card. ...and, of course, the tests. All the tests... |
frontend/src/game/board/builder.js
Outdated
| @@ -40,6 +42,12 @@ module.exports = { | |||
| newCard.classList.remove("col-1"); | |||
| newCard.classList.add("col-2"); | |||
| newCard.innerHTML = cardData["power"]; | |||
| newCard.addEventListener('click', function() { | |||
| if (status.clickedCard !== undefined && | |||
There was a problem hiding this comment.
if (status.clickedCard !== undefined)
is equivalent to
if (status.clickedCard)
frontend/src/game/board/builder.js
Outdated
| newCard.addEventListener('click', function() { | ||
| if (status.clickedCard !== undefined && | ||
| status.clickedCard.hasAttribute("row-played")) { | ||
| play.playCard(true) |
There was a problem hiding this comment.
If I click a card on the board and had already selected and played a card (that is, I have assigned it a row-played I call play.playCard to play status.clickedCard.
The true value can be any anything but undefined right now. When we work on passing a target, this value will be the target.
There was a problem hiding this comment.
then i feel this true should be in a 'target' variable, and that variable would be set to true for now
| newCard.innerHTML += showAddPower(cardInHand["add-power"]) | ||
| newCard.innerHTML = cardInHand["power"]; | ||
| newCard.setAttribute("add-power", cardInHand["add-power"]); | ||
| newCard.innerHTML += showAddPower(cardInHand["add-power"]); |
There was a problem hiding this comment.
The ability add-power is set as an attribute, so we can easily check it for a card.
Maybe this two lines can be simplified.
frontend/src/game/play/play.js
Outdated
| target: target, | ||
| }; | ||
| document.querySelector('.card[index="'+cardindex+'"]').style.background = "yellow"; | ||
| playCard(target) { |
There was a problem hiding this comment.
Since we keep track of the status.clickedCard for marking it and handling abilities, passing the playing info here is redundant.
frontend/src/game/play/play.js
Outdated
| if (target === undefined && | ||
| status.clickedCard.getAttribute("add-power")) { | ||
| status.clickedCard.style.background = "yellow"; | ||
| } else { |
There was a problem hiding this comment.
I go ahead with the playing only if the card doesn't have add-power or has a defined target. Otherwise I simply mark the card in yellow.
|
Last change solves a bug where clicking on an owned card for playing a card gave a bug, since it has no |
frontend/src/game/play/play.js
Outdated
| target: target, | ||
| }; | ||
| document.querySelector('.card[index="'+cardindex+'"]').style.background = "yellow"; | ||
| playCard() { |
There was a problem hiding this comment.
I don't like this function "magically" getting its info from somewhere that isn't even in the same file (status.clickedCard).
It should take the clicked card as an argument or it will grow unmantainable in the future, as this project grows.
|
We need a test that checks that on clicking to play a card with an ability, a target selection is requested. |
|
Unit tests are failing 🙃 |
This is the first commit on an attempt to resolve #77.
As I understand, the only thing that should be achieved to solve this Issue is the game requiring a target to be selected when playing a card with an ability. Doing something with that target will be solved by #78.