Abilities to fe #165
Abilities to fe #165
Conversation
|
Ehhh, i don't think #154 discussion is finished... |
| @@ -6,18 +6,27 @@ | |||
| "Returns cards as seen by a player" | |||
| [game-state player-id] | |||
| (vec (map | |||
There was a problem hiding this comment.
This function is becoming very long and hard to read... do we really need nested ifs?
I feel like cond would be clearer, even if the conditions become a little more verbose, and maybe even split off the different cases into functions?
There was a problem hiding this comment.
Let's see if it's easier to read now
backend/src/rules/abilities.clj
Outdated
| [:cards target :power] | ||
| #(+ % increase))) | ||
| ([] | ||
| {:description (str "Enhance " increase) :target 1}))) |
There was a problem hiding this comment.
"Enhance" doesn't seem very appropriate for a negative value
| ; Correctly returns cards with abilities | ||
| (expect | ||
| [{:power 1 :location [:hand] :owner "me" :description "Enhance 100" :target 1} | ||
| {:power 17 :location [:hand] :owner "me" :description "Enhance -1" :target 1} |
There was a problem hiding this comment.
Again, I feel like "Enhance -1" just looks very weird. Diminish or Weaken maybe? It might even then be positive "Weaken 1"
| @@ -2,31 +2,31 @@ | |||
| (:require [rules.victory-conditions :as victory] | |||
| [configs.messages :as messages])) | |||
|
|
|||
| (defn ^:private partial-card | |||
| "Returns only certain keys of a card and change the :owner" | |||
There was a problem hiding this comment.
I know that this could be a more generic function (no need to call it "card") but since it changes the :owner, I felt like it should be like that.
Also, considering it's private, I see no reason to make it more generic.
There was a problem hiding this comment.
agree with no need to be more generic
| @@ -2,31 +2,31 @@ | |||
| (:require [rules.victory-conditions :as victory] | |||
| [configs.messages :as messages])) | |||
|
|
|||
| (defn ^:private partial-card | |||
| "Returns only certain keys of a card and change the :owner" | |||
There was a problem hiding this comment.
agree with no need to be more generic
| (defn get-cards | ||
| "Returns cards as seen by a player" | ||
| [game-state player-id] | ||
| (vec (map | ||
| #(cond | ||
| (= (:owner %) player-id) | ||
| (assoc % :owner "me") | ||
| (and (and (= (:owner %) player-id) |
There was a problem hiding this comment.
double and is redundant. In clojure and takes any amount of arguments, so you can just do (and cond1 cond2 cond3 ... condN)
Description
Backend responding API requests only gives relevant information about cards.
Issues Resolved
Resolves #154
Details
All cards abilities (which are functions) must be created in a way that calling them without parameters gives the frontend relevant information. That's a
:descriptionand:target, as proposed by @kenan-rhoton on #154.Hotspots
(api.player_view_functions/get-cardsshould be carefully reviewed. I tried to make it the most readable by dividing the 4 possible scenarios regarding card's info that must be shown and write every time what should be send. An alternative would be abuse(mergeto avoid repetitions, but I felt that would be less redeable.Checklist for contribution requirements
(I leave the checks for reviewers)