Small game written in F#

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:

• Every player has soldiers and territory

• Player can recruit soldiers, gain territory or attack another player

• 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:

[]
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:

[]
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:

```
[]
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
| Attac

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.

StackExchange Code Review Q#105937, answer score: 6