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

UpdateGame function for Tetris

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

Problem

I am writing a Tetris game while learning Haskell and I'm trying to make the following code more concise and easier to read. How can I improve it?

Note that the function is not finished yet (There is no check for complete lines, for instance); I currently want to improve its readability.

```
updateGame :: IORef InputsData -> IORef GameState -> IO()
updateGame keysIORef gameIORef = do
game g {time = newTime})
game g {keyCooldowns = updateCooldowns (keyCooldowns game) elapsed})
game g {currentTetromino = (currentTetromino g) {x = x (currentTetromino g) + 1}})
modifyIORef gameIORef (\g -> g {keyCooldowns = addCooldown (keyCooldowns game) (SpecialKey KeyRight) 100})
else if left keys
then when (canTetrominoMove (currentTetromino game) (Helpers.L) (tank game)) $ do
modifyIORef gameIORef (\g -> g {currentTetromino = (currentTetromino g) {x = x (currentTetromino g) - 1}})
modifyIORef gameIORef (\g -> g {keyCooldowns = addCooldown (keyCooldowns game) (SpecialKey KeyRight) 100})
else return ()
game g {currentTetromino = rotateTetromino (currentTetromino g) Helpers.R})
modifyIORef gameIORef (\g -> g {keyCooldowns = addCooldown (keyCooldowns game) (Char 'z') 200})
else if zKey keys
then when (canTetrominoRotate (currentTetromino game) (Helpers.L) (tank game)) $ do
modifyIORef gameIORef (\g -> g {currentTetromino = rotateTetromino (currentTetromino g) Helpers.L})
modifyIORef gameIORef (\g -> g {keyCooldowns = addCooldown (keyCooldowns game) (Char 'z') 200})
else return ()
game g {tetrominoFallingTime = (tetrominoFallingTime g) - elapsed})
game g {tetrominoFallingTime = (tetrominoFallingTime g) + getTetrominoFallingSpeed (difficultyLevel g)})
if canTetrominoFall (currentTetromino game) (tank game)
then do
modifyIORef gameIORef (\g -> g {currentTetromino = (currentTet

Solution

You should separate IO actions from pure code. In your case the principal task of updateGame is to create the new game state given input data, and for this no IO is necessary. The fact that the state is stored and modified in an IORef.

It's not just the matter of style and code separation. Calling modifyIORef multiple times in the function means that you can easily get race conditions, if there is another thread working with the game state. It would get or modify an inconsistent state.

So my suggestion would be to split it into two parts - one a pure function, and another that does all the necessary IO:

updateGame :: UTCTime -> InputsData -> GameState -> GameState
updateGame time input game = ...

updateGameIO :: IORef InputsData -> IORef GameState -> IO ()
updateGameIO modifyIORef gameIORef = step
  where
    step = do
      time <- getCurrentTime

      keys <- readIORef keysIORef
      modifyIORef gameIORef (updateGame time keys)

      addTimerCallback 1 step
      postRedisplay Nothing


This way, you ensure that the game state is updated atomically, and keep the main function pure.

Updating a field in a complex data type is a common task, and it can be greatly simplified using the lens library. Instead of

\g -> g {keyCooldowns = updateCooldowns (keyCooldowns game) elapsed}


you let lens generate so-called lenses for your data type, so for example keyCooldowns won't be of type GameState -> Something, but Lens' GameState Something, and then the function that modifies the field inside GameState would be

over keyCooldowns (flip updateCooldowns elapsed)


See A Little Lens Starter Tutorial

Code Snippets

updateGame :: UTCTime -> InputsData -> GameState -> GameState
updateGame time input game = ...


updateGameIO :: IORef InputsData -> IORef GameState -> IO ()
updateGameIO modifyIORef gameIORef = step
  where
    step = do
      time <- getCurrentTime

      keys <- readIORef keysIORef
      modifyIORef gameIORef (updateGame time keys)

      addTimerCallback 1 step
      postRedisplay Nothing
\g -> g {keyCooldowns = updateCooldowns (keyCooldowns game) elapsed}
over keyCooldowns (flip updateCooldowns elapsed)

Context

StackExchange Code Review Q#79852, answer score: 4

Revisions (0)

No revisions yet.