HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

2048 game implementation in Clojure

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
implementationgameclojure2048

Problem

This is my first Clojure program. If you want to run it locally please check out https://github.com/achikin/game2048-clj

core.clj

(ns game2048.core
  (:require [nightlight.core :refer [start]])
  (:require [game2048.ui :as ui])
  (:require [lanterna.screen :as s])
  (:require [game2048.game :as g])
  (:gen-class))

(def x 1)
(def y 1)

(defn game-loop
  [scr board]
    (recur 
     scr 
     (ui/draw-board
      scr x y
      (g/game-step (s/get-key-blocking scr) board))))

(defn -main []
  (let [scr (s/get-screen) board (g/new-board)]
    (s/in-screen scr 
      (do
        (ui/draw-board scr x y board)
        (ui/draw-agenda scr x (+ y (:height g/board-size) 1) g/agenda)
        (game-loop scr board)))))


game.clj

(ns game2048.game
  (:require [game2048.board :as b]))

(def max-score 2048)

(def board-size {:width 4 :height 4})

(def agenda
  '("←↑→↓ - make move"
    "r - reset"
    "q - quit"))

(defn new-board []
  (b/add-random-tiles (b/empty-board board-size)))

(defn process-key
  "Either exit or transform board according to a key passed"
  [key board]
  (case key
    (:up :down :left :right) (b/make-move key board)
    \q (System/exit 0)
    \r (b/empty-board board-size)))

(defn check-board
  "Check for logical conditions and transform board accordingly"
  [board-before-keypress board]
  (let [board-after-rand (b/add-random-tiles board)]
    (cond
      (= board-before-keypress board) board
      (b/full? board-after-rand) (new-board)
      (b/contains-max? max-score board) (new-board)
      :else board-after-rand)))

(defn game-step
  [key board]
  (check-board board
    (process-key key board)))


ui.clj

```
(ns game2048.ui
(:require [game2048.board :as b])
(:require [lanterna.screen :as s]))

(def maxlen 5)

(defn count-digits [n]
(if (zero? n) 1
(-> n Math/log10 Math/floor long inc)))

(defn repeat-str
[n st]
(apply str (repeat n st)))

(defn pad-number
[length number]
(let [n (count-digits number)
pads (/ (-

Solution

core.clj

(def x 1)
(def y 1)


  • Use more descriptive names. These seem to be used as the origin for rendering the game, so maybe something like x-origin and y-origin would be good.



  • Add docstrings to document the meaning and purpose of each var / function



  • It might be worth-while making these dynamic vars. Then you can render the game in a different location (e.g., if you want to render two games for head-to-head competition) using bindings.



  • You could combine the two into a single origin var.



Combining these points, you'd end up with something like:

(def ^:dynamic *origin*
  "Defines the origin at which to render the game"
  [1 1])


game.clj

(def agenda
  '("←↑→↓ - make move"
    "r - reset"
    "q - quit"))


I like the way you take a data-centric approach here. It's a good pattern.

(defn process-key
  "Either exit or transform board according to a key passed"
  [key board]
  (case key
    (:up :down :left :right) (b/make-move key board)
    \q (System/exit 0)
    \r (b/empty-board board-size)))


  • You have the keys hard-coded to their actions. You could decouple this by using a data-centric approach (similar to agenda).



  • You don't have a clause in your case, so this code will throw an exception if any other key is pressed.



  • Ending the game by calling System/exit limits what you can do afterwards. For example, you can't go back to a menu of games. Avoid System/exit unless it is absolutely necessary. One way to avoid it in this case would be to return nil. Then have your main loop terminate if a nil is encountered.



Using a data-centric approach, I would do something like this:

(def ^:dynamic *actions*
  "Maps keys to action fns.  Each action is a function that takes a board state and returns a new board state."
  {:up    b/move-up
   :down  b/move-down
   :left  b/move-left
   :right b/move-right
   \q     (constantly nil)
   \r     (fn [board]
            (b/empty-board board-size))})


Then process-key can be defined as:

(defn process-key
  "Either exit or transform board according to a key passed"
  ([key board]
    (let [action (get *actions* key identity)]
      (action board))))


identity is used as the default to provide an effective no-op when the key can't be mapped.

board.clj

All the functions that manipulate board state deal directly with the internal representation of the board -- there's no abstraction. You should consider defining a protocol to define an interface for dealing with boards. There are many approaches. Here's one option:

(defprotocol IBoard
  (rows [board] [board data])
  (columns [board] [board data])


Then use deftype to provide an implementation.

Now, we can define the move- functions like:

(defn move-up [board]
  (columns board (map compress (columns board))))

(defn move-down [board]
  (columns board (map reverse (map compress (map reverse (columns board))))))


Though it would be more idiomatic to use ->>:

(defn move-left [board]
  (->> (rows board)   ;; Obtain row-centric view of the board
       (map compress) ;; "Compress" each row
       (rows board))) ;; Construct new board from compressed rows

(defn move-right [board]
  (->> (rows board)   ;;
       (map reverse)  ;; Reverse rows, so compression happens in correct order
       (map compress) ;; "Compress" each row
       (map reverse)  ;; Reverse rows back to original order
       (rows board))) ;; Construct new board from compressed rows


If desired, you could refactor these to take advantage of the shared structure.

compress takes a sequence and replaces consecutive elements with a single element of their sum. For the above functions to work, the result of compress must have the same length as its input (i.e., padded with 0s). Otherwise, you'll have to handle padding elsewhere.

I believe this design would allow you to get rid of row.clj.

ui.clj

(def maxlen 5)


I think you should avoid a hard limit here. At the very least, make maxlen dynamic. But even better, calculate it dynamically via max-length (which you currently don't use, BTW). If you do this, you might want to have a minlen, to avoid the cell size changing too often.

(defn count-digits [n] 
  (if (zero? n) 1
   (-> n Math/log10 Math/floor long inc)))


This is way too complicated. Just convert the number to a string and count the number of characters in the string:

(defn count-digits [n]
  (count (str n)))


Alternatively, use String's length property:

(defn count-digits [n]
  (. (str n) length))


(defn draw-row
  ([screen x y row]
   (if-not (empty? row)
      (do
       (s/put-string screen x y (pad-number maxlen (first row)))     
       (recur screen (+ x maxlen) y (rest row))))))


Avoid using recur if there's an alternative that fits well. In this case, I would use doseq:

```
(defn draw-row
([screen x y row]
(doseq [[i n] (zip (ra

Code Snippets

(def x 1)
(def y 1)
(def ^:dynamic *origin*
  "Defines the origin at which to render the game"
  [1 1])
(def agenda
  '("←↑→↓ - make move"
    "r - reset"
    "q - quit"))
(defn process-key
  "Either exit or transform board according to a key passed"
  [key board]
  (case key
    (:up :down :left :right) (b/make-move key board)
    \q (System/exit 0)
    \r (b/empty-board board-size)))
(def ^:dynamic *actions*
  "Maps keys to action fns.  Each action is a function that takes a board state and returns a new board state."
  {:up    b/move-up
   :down  b/move-down
   :left  b/move-left
   :right b/move-right
   \q     (constantly nil)
   \r     (fn [board]
            (b/empty-board board-size))})

Context

StackExchange Code Review Q#148151, answer score: 2

Revisions (0)

No revisions yet.