Small game written in F#

Question

I'm trying to wrap my head around F#. I'd like your honest opinion about this simple game source code that I wrote. Rules are simple:

1. Every player has soldiers and territory
2. Player can recruit soldiers, gain territory or attack another player
3. Player loses when he has no territory, wins when everybody else loses

My main problem is still code organization and usage of mutable fields. I believe it can be done better, but I don't really know how.

Config module:

[<AutoOpen>]
module Config

let humanPlayers = 1
let aiPlayers = 2

let aiComputationSleep = 200
let aiAttacksWhenHaveXSoldiers = 1500

let minInitialSoldiers = 750
let maxInitialSoldiers = 1000
let minInitialArea = 750
let maxInitialArea = 1000

let clearConsoleAfterEachTurn = true

Utils module:

[<AutoOpen>]
module Utils

open System

let subToZero a b = 
    match a >= b with
    | true -> a - b
    | false -> 0

let random = new Random()
let rand (a : int) (b : int) = random.Next(Math.Min(a, b), Math.Max(a, b))

let single f list = 
    let matches = list |> List.filter f
    match matches.Length with
    | 1 -> Some (List.head matches)
    | _ -> None

Types module:

[<AutoOpen>]
module Types

type PlayerType = 
    | Human
    | AI

type Player(name : string, playerType : PlayerType, soldiers : int, area : int) = 
    member this.Name = name
    member this.PlayerType = playerType
    member val Soldiers = soldiers with get, set
    member val Area = area with get, set
    member this.NameWithType = sprintf "%s(%A)" this.Name this.PlayerType
    member this.Alive() = this.Area > 0

type GameState = 
    { turn : int
      players : Player list
      winner : Player option }

type PlayerMove = 
    | Recruit
    | GainTerritory
    | Attack of Player
    | NotProvided

type AttackResult = 
    { attacker : Player
      attackerLoses : int
      defender : Player
      defenderLoses : int
      area : int }

type MoveResult = 
    | Recruted of Player * int
    | TerritoryGained of Player * int
    | Attacked of AttackResult
    | Unknown

type GetMove = Player list -> Player -> PlayerMove

type MoveDispatcher(humanMove: GetMove, aiMove: GetMove) =
    member this.HumanMove = humanMove
    member this.AIMove = aiMove
    member this.GetMove (players : Player list) (player : Player) =
        match player.PlayerType with
        | Human -> this.HumanMove players player
        | AI -> this.AIMove players player

Init module:

module Init

let initialSoldiers() = rand minInitialSoldiers maxInitialSoldiers
let initialArea() = rand minInitialArea maxInitialArea
let initPlayer (playerType : PlayerType) (num : int) = 
    Player("Player" + num.ToString(), playerType, initialSoldiers(), initialArea())

let initPlayers = 
    let human = List.init humanPlayers (initPlayer Human)
    let ai = List.init aiPlayers (initPlayer AI)
    human @ ai

let initGame = 
    { turn = 1
      players = initPlayers
      winner = None }

AI module:

module AI

open System.Threading

let getEnemyToAttackForAI (players : Player list) (player : Player) = 
    players
    |> List.except [ player ]
    |> List.maxBy (fun x -> x.Area)

let getAIMove (players : Player list) (player : Player) = 
    Thread.Sleep(aiComputationSleep)
    if player.Soldiers > aiAttacksWhenHaveXSoldiers then Attack(getEnemyToAttackForAI players player)
    elif player.Soldiers > player.Area then GainTerritory
    else Recruit

GameLogic module:

module GameLogic

open System

let recruitSoldiers (player : Player) = 
    let recruits = rand 100 player.Area
    player.Soldiers <- player.Soldiers + recruits
    Recruted(player, recruits)

let gainTerritory (player : Player) = 
    let gainedArea = rand 100 player.Soldiers
    player.Area <- player.Area + gainedArea
    TerritoryGained(player, gainedArea)

let getNextState state : GameState = 
    { state with turn = state.turn + 1
                 winner = None }

let attack (player : Player) (enemy : Player) = 
    let attackerKills = rand 0 player.Soldiers
    enemy.Soldiers <- subToZero enemy.Soldiers attackerKills
    let defenderKills = rand 0 enemy.Soldiers
    player.Soldiers <- subToZero player.Soldiers defenderKills
    let areaConquered = subToZero attackerKills defenderKills
    player.Area <- player.Area + areaConquered
    enemy.Area <- subToZero enemy.Area areaConquered
    Attacked { attacker = player
               attackerLoses = defenderKills
               defender = enemy
               defenderLoses = attackerKills
               area = areaConquered }

let changePlayerState playerMove (player : Player) = 
    match playerMove with
    | Recruit -> recruitSoldiers player
    | GainTerritory -> gainTerritory player
    | Attack enemy -> attack player enemy
    | NotProvided -> Unknown

let checkIfGameFinished (state : GameState) (players : Player list) = 
    match players |> single (fun x -> x.Alive()) with
    | Some player -> { state with winner = Some player }
    | None -> state

ConsoleUi module:

module ConsoleUi

open System
open GameLogic

let pause() = Console.Read() |> ignore
let printTitle() = printfn "SmallWars!"

let printHeader() = 
    if clearConsoleAfterEachTurn then Console.Clear()
    printTitle()
    printfn "------------------------------------------------------------------------"

let printPlayerState (player : Player) = 
    printfn "%s kingdom have %i soldiers and %i area" player.NameWithType player.Soldiers player.Area

let printState (state : GameState) = 
    match state.winner with
    | None -> 
        printfn "Turn %i" state.turn
        state.players |> List.iter printPlayerState
    | Some player -> 
        printfn "Game is finished! %s won!" player.NameWithType
        pause()

let printMoveResult = 
    function 
    | Recruted(player, soldiers) -> printfn "%s recruted %i soldiers" player.NameWithType soldiers
    | TerritoryGained(player, area) -> printfn "%s gained %i area" player.NameWithType area
    | Attacked result -> 
        printfn "%s attacked %s. Loses %i - %i. Area conquered %i" result.attacker.NameWithType 
            result.defender.NameWithType result.attackerLoses result.defenderLoses result.area
    | Unknown -> printfn ""

let printEmptyLine() = printfn ""

let getPossibleTargets (players : Player list) (player : Player) = players |> List.except [ player ]

let getEnemyToAttackForHuman (players : Player list) (player : Player) = 
    printfn "Who you want to attack?"
    let targets = getPossibleTargets players player
    targets |> List.iteri (fun i x -> printfn "%i - %s" i x.NameWithType)
    match Int32.TryParse(Console.ReadLine()) with
    | true, i when i < List.length targets -> Attack targets.[i]
    | _, _ -> NotProvided

let getHumanMove (players : Player list) (player : Player) = 
    match Console.ReadLine() with
    | "r" -> Recruit
    | "g" -> GainTerritory
    | "a" -> getEnemyToAttackForHuman players player
    | _ -> NotProvided

Program.fs - entry point:

open Init
open GameLogic
open ConsoleUi
open AI

[<EntryPoint>]
let main argv = 
    let mutable state = initGame
    let mutable moveResult = Unknown
    let moveDispatcher = MoveDispatcher(getHumanMove, getAIMove)

    let movePlayer players player = 
        printHeader()
        printMoveResult moveResult
        printEmptyLine()
        printState state
        let playerInput = moveDispatcher.GetMove players player
        moveResult <- changePlayerState playerInput player
        state <- getNextState state

    while state.winner.IsNone do
        state.players
            |> List.filter (fun x-> x.Alive())
            |> List.iter (fun x -> movePlayer state.players x)
        state <- checkIfGameFinished state state.players

    printEmptyLine()
    printMoveResult moveResult
    printState state
    pause()
    0

Answer 1

Overall, I like the code. The types are defined nicely, and the organization makes sense to me. I'm still looking in changes that could work and benefit the code, but so far, I've come up with this:

• Simplify some functions.

  For subToZero, you match on a bool. This can be simpler if you just use an if expression: let subToZero a b = if (a >= b) then a - b else 0. Even better, there is already a function that you can use from the default library: max, as in: let subToZero a b = max (a-b) 0.

  For single, you can simplify the code by not matching on the list's length, but on the list itself, using pattern matching:

    let single f list =
        match list |> List.filter f with
        | [s] -> Some s
        | _ -> None

The [s] pattern matches on a list with only a single item, the _ takes care of the rest.

• Replace "memberful" types with record types.

  Warning: For now, this will break your game, as it depends on mutable state. But you can start here, and combine it with the following points to get rid of the mutability.

  Your Player type could also be written like this:

  type Player =
      { name : string
        playerType : PlayerType
        soldiers : int
        area : int }
  

  This would still leave those fields readable as they are now (just need to switch them from PascalCase to camelCase where you access them). You would just need a way to change them, the immutable way (see next point).

  You would also need a way to keep the functionality from the methods (this.Alive() and this.NameWithType). You could do that by defining the following functions:

  let nameWithType (p : Player) = sprintf "%s(%A)" p.name p.playerType
  let alive (p : Player) = p.area > 0
  

  One additional upside from this, is that you now can rewrite things like:

  match players |> single (fun x -> x.Alive()) with (* ... *)
  

  to

  match players |> single alive with (* ... *)
  

  which, in my opinion, reads so much better :)

• Mutate data the immutable way.

  Whenever you want to change a player, you cannot do this anymore, because it is immutable. But that is okay, this is the functional way. Each function that would mutate a Player in the old case, should now return the new Player instead. Imagine a function that would halve the Player's area (which doesn't exist). The mutable way would be:

  (* Old way *)
  let halveArea (p : Player) =
      p.Area <- p.Area / 2
      (* What should it return? *)
  

  Instead, the functional way would do:

  (* New way *)
  let halveArea (p : Player) =
      { p with area = p.area / 2 }
      (* This returns a new player, based on the old one, with one change *)
  

  In the current program, this gets a bit harder, because most functions that change a Player return something else already. You could, for example, return a Recruted (which you should probably rename Recruited, by the way, in a next iteration) with the new Player and the number of recruits. But that means, that in the caller, you should also use that information to return a new GameState with that new Player instead of the old, so that you can pass that new state to the next function.

  This is a bit too much to change all here in the review, but you can see the point. In the end, the change on the Player means a change in the GameState, which should be changed in the functional way as well... This al leads to my final point:

• Use recursion instead of while.

  Yes, recursion is actually better at changing state than a mutable value. You can use it like this:

  let rec playTurn (state : GameState) =
      if (state.winner.IsSome)
          then state (* Finishes the recursion, returns final state *)
          else let nextPlayer = state.players |> List.filter alive |> List.head
               let playerMove = movePlayer state nextPlayer
               let nextGameState = getNextState state playerMove
               playTurn nextGameState
  

  This would require a rewrite of movePlayer and getNextState. movePlayer would not "change" a state, but just return the move that the nextPlayer wants to make. And getNextState would be responsible for calculating the new state, including changes to all players and their enemies, and determining the winner if there is one, returning the new GameState for the next player move.

I am sorry that I can not elaborate on that last one, because that would make a lot of changes to the existing code. But I would encourage you to play with that idea of recursion and state changing, so that you can get rid of the mutables.