Sunday, December 20, 2015

Cartagena, part 2

This week brought "interesting" progress on the implementation of Cartagena.  A note before we begin, however.  Tim's version of pirates is NOT a Cartagena implementation as I'd originally thought.  My apologies if this caused any confusion.  Tim's game is actually WAY MORE COOL, as you get to sail your pirate ship to all the ports in the Caribbean.  In any case, it's a different game (at least, for now).

OK, now on with the show.  I started this week by refactoring the board to "pieces" instead of "cards."  I also added stubbed tests to outline my thoughts of how the implementation should go from that point forward.  This is almost like taking notes and can help you think through a problem, which I definitely need help with.

(deftest shuffle-cards-test
  (testing "Returns a random collection of N cards"))

(deftest initialize-player-test
  (testing "Returns a player data structure full of initial state data")
  (testing "Each player should have six pirates")
  (testing "Each player should have six cards"))

(deftest new-game-test
  (testing "Player count should be equal to the number to which it was initialized")
  (testing "All players should be in jail")
  (testing "Active player should be player 1"))

(deftest player-move-test
  (testing "Player can play card and move pieces")
  (testing "Player can move backward and receives cards")
  (testing "Player without cards must move backward")
  (testing "Player with no available spaces behind cannot move backward")
  (testing "Player with pirate on ship can move that pirate backward"))

(deftest game-over-test
  (testing "Game ends when a player has all pirates on the ship"))

The next bits involved managing some state.  I created atoms for managing the card draw and discard piles while writing tests and implementing card shuffling.  Once cards are shuffled, a player could be initialized.  While implementing that function, I realized I hadn't implemented a function for drawing cards, so I paused the player initialization in favor of writing a function for drawing cards from the draw pile.  After that, I finished out the player initialization.

(def draw-pile (atom []))
(def discard-pile (atom []))
(defn place-cards!
  "Puts the full set of cards into the discard pile"
  (reset! discard-pile (->> icons
                            (map #(repeat 17 %))

(defn shuffle-cards!
  "Shuffles the card in the discard pile, placing them in the draw pile"
  (when (and (not (empty? @discard-pile))
             (empty? @draw-pile))
    (reset! draw-pile (vec (shuffle @discard-pile)))
    (reset! discard-pile [])))

(defn initialize-board!
  "Returns a vector populated with icons from the 6 of the board pieces concatenated."
  (->> all-board-pieces
       (take 6)

(defn draw-cards!
  "Takes n cards off of the top of the draw pile"
  (let [cards (take n @draw-pile)]
    (reset! draw-pile (drop n @draw-pile))
    (vec cards)))

(defn initialize-player
  "Initializes a player data structure"
  [{:keys [name color]}]
  {:name name :color color :pirates [-1 -1 -1 -1 -1 -1] :cards (draw-cards! 6)})

Now that we have cards and players, it seemed like it was time to try to initialize a new game.  I created yet another atom to hold general game state.  At this point, I had several atoms, which is a TERRIBLE smell.  I stopped forward progress in order to refactor all of the state storage into a single atom.  This caused a fair amount of refactoring at each function's level, mainly due to removal of state management from those functions.  This actually felt REALLY nice, and I was more pleased with the implementation at that point.

(def game-state (atom {}))

(defn initialize-board
  "Generates a board from 6 random pieces"
  (->> all-board-pieces
       (take 6)

(defn shuffle-cards
  "Shuffles and returns passed cards"
  (vec (shuffle cards)))

(defn initialize-cards
  "Puts the full set of cards into the discard pile"
  (->> icons
       (map #(repeat 17 %))

(defn initialize-player
  "Initializes a player data structure"
  [{:keys [name color]}]
  {:name name :color color :pirates [-1 -1 -1 -1 -1 -1] :cards []})

(defn draw-cards
  "Pulls cards off the top of the draw pile, returning a map of the new hand and what remains in the draw pile"
  [n player draw-pile]
  {:player (assoc player :cards (apply conj (:cards player) (take n draw-pile)))
   :draw-pile (vec (drop n draw-pile))})

(defn new-game!
  "Initializes a new game"
  (let [board (initialize-board)
        players-draw-pile (loop [ps (vec (map initialize-player players))
                                 cards (initialize-cards)
                                 acc []]
                            (if (empty? ps)
                              (let [player-draw-pile (draw-cards 6 (first ps) cards)]
                                (recur (rest ps) (:draw-pile player-draw-pile) (conj acc player-draw-pile)))))
        init-players (vec (map :player players-draw-pile))
        draw-pile (:draw-pile (last players-draw-pile))]
    (reset! game-state {:board-spaces board
                        :players init-players
                        :current-player 0
                        :draw-pile draw-pile
                        :discard-pile []}))

  #_(let [game-state (assoc {}
                     (vec (for [player players]
                            (initialize-player player))))]
    (assoc game-state :current-player 0)))

With that done, I moved forward with implementing the actions a player can take on a turn, beginning with playing a card.  While beginning this code, I realized that I'd missed a case for drawing cards: what happens when there aren't enough cards in the draw pile?  Shuffle the discard pile back into the draw pile.  I quickly added a couple of tests (draw has no cards, draw has cards but not enough to cover the entire need) and the code to make the tests pass.

Then, something unfortunate happened.  I had this notion that perhaps the player portion of the game state would be better managed by color instead of by name.  I spend several hours refactoring the code to try to support that notion.  At the end of that effort, I still had a couple of broken tests, but the worst part was that when I looked at the code, it was LESS consumable than the prior version.  I reverted that work (thank you for making that painless, git!).

Let this serve as a lesson for you, kids.  When you're doing TDD and the tests don't lead you toward a design (meaning, you have a notion about a design that hasn't really emerged), don't change the design.  Wait until the test reveals the need for the design change, THEN do the necessary refactoring.

However, this surfaced a larger issue: I wasn't convinced that the way I was managing state was good enough.  I decided to fill out a sample game state map and get some feedback on it.  I have the luxury of being married to a divine software engineer, and asked her for her opinion on it.  She looked at it and gave some small but critical feedback: the board should know which pieces are on which spaces; the player shouldn't care about anything other than its cards.

Let this serve as a second lesson for you, kids.  Asking for feedback or help is ALWAYS preferable to spinning your wheels.  It's ok to try to figure something out on your own.  In fact, you SHOULD try to figure it out on your own.  However, put a time box around the effort.  For me, if I work with something for more than four hours without making tangible progress, it's time to punt.  By that point, I've not only exhausted myself, but I'm pretty crotchety about not being able to figure it out.  Maybe I should cut my time box in half so that I don't get to the "unapproachable" point...

From there, I was able to move forward with implementing the playing of a card.  This actually turned into a small series of functions, each of which was covered by an independent set of unit tests.

(defn is-open-target?
  "Returns true if the space matches the icon and has fewer than three pirates"
  [space icon]
  (and (= icon (:icon space))
       (< (count (:pirates space)) 3)))

(defn open-space-index
  "Returns the index of the first open space for the given icon after the starting index."
  [starting-index board icon]
    (some #(let [space (get board %)]
            (when (is-open-target? space icon) %))
          (range (inc starting-index) (count board)))
    (dec (count board))))

(defn play-card
  "Discards the card and moves a single pirate from the space to the next available space."
  [player icon from-space board discard-pile]
  (let [[pre-cards post-cards] (split-with #(not= icon %) (:cards player))
        [pre-pirates post-pirates] (split-with #(not= (:color player) %) (:pirates from-space))
        space-index (.indexOf board from-space)
        next-open-space-index (open-space-index space-index board icon)
        next-open-space (get board next-open-space-index)]
    {:player {:cards (concat pre-cards (rest post-cards))}
     :board-spaces (assoc board space-index (assoc from-space :pirates (vec (flatten (concat pre-pirates (rest post-pirates)))))
                         next-open-space-index (assoc next-open-space :pirates (conj (:pirates next-open-space) (:color player))))
     :discard-pile (conj discard-pile icon)}))

Even though there are a couple of spots in the play-card function that should be scrutinized further, I feel much better about this design.  I don't have any justification for that feeling aside from the fact that it emerged from the needs driven by the tests and it appears to actually work.  :)

That's it on progress for the week.  The good news is that I continue to work on it a little bit every day, and enjoy doing so (generally).  I'll try to sneak in a non-dev blog post sometime this week.  Since it's Christmas week, I don't think it'll be a problem from the content perspective...


  1. What's actually using the global game-state?

    I think its a worth while exercise to build a game without using a global atom for storing the state.

  2. Agreed, the atom is not necessary right now, and I seriously considered *not* using it and passing the entire data structure around all over the place. Eventually, I would like this to be the engine for a game that people can play over the interwebs. When that happens, it seems like I'll have to implement persistence, and could do away with the atom.

    Hmm... it smells like I should do away with the atom... :D