Don't change player match count for bye matches

Before changing a player's match count, check to see if we're using a
bye match first. Because functions that change matches are spread across
different modules, the code is still a bit messy. We may need a more
robust strategy to update match counts to guarantee consistency.

This also changes the type for match counts from int to a natural-number
int that can't go below zero.

Fixes #83

__tests__/Scoring_test.res

@@ -42,8 +42,8 @@ test("Ratings never go below 100", () => {
   let (newRatingWhite, _) = Data.Ratings.calcNewRatings(
     ~whiteRating=100,
     ~blackRating=100,
-    ~whiteMatchCount=69,
-    ~blackMatchCount=69,
+    ~whiteMatchCount=Data.Player.NatInt.fromInt(69),
+    ~blackMatchCount=Data.Player.NatInt.fromInt(69),
     ~result=Data.Match.Result.BlackWon,
   )
   expect(newRatingWhite)->toBe(100)

src/Data/Data_Match.res

@@ -69,6 +69,8 @@ type t = {
   result: Result.t,
 }
 
+let isBye = ({whiteId, blackId, _}) => Data_Id.isDummy(whiteId) || Data_Id.isDummy(blackId)
+
 let decode = json => {
   let d = Js.Json.decodeObject(json)
   {

src/Data/Data_Match.resi

@@ -33,6 +33,8 @@ type t = {
   result: Result.t,
 }
 
+let isBye: t => bool
+
 let decode: Js.Json.t => t
 
 let encode: t => Js.Json.t

src/Data/Data_Player.res

@@ -30,11 +30,39 @@ module Type = {
   let decode = data => Js.Json.decodeString(data)->Option.getExn->fromString
 }
 
+module NatInt = {
+  type t = int
+
+  let fromInt = x =>
+    if x < 0 {
+      0
+    } else {
+      x
+    }
+
+  let toInt = x => x
+  let toString = string_of_int
+
+  let succ = x =>
+    if x < 0 {
+      0
+    } else {
+      succ(x)
+    }
+
+  let pred = x =>
+    if x < 1 {
+      0
+    } else {
+      pred(x)
+    }
+}
+
 type t = {
   firstName: string,
   id: Data_Id.t,
   lastName: string,
-  matchCount: int,
+  matchCount: NatInt.t,
   rating: int,
   type_: Type.t,
 }
@@ -47,6 +75,11 @@ let compareName = (a, b) =>
   | i => i
   }
 
+let succMatchCount = t => {...t, matchCount: NatInt.succ(t.matchCount)}
+let predMatchCount = t => {...t, matchCount: NatInt.pred(t.matchCount)}
+
+let setRating = (t, rating) => {...t, rating}
+
 let decode = json => {
   let d = Js.Json.decodeObject(json)
   {

src/Data/Data_Player.resi

@@ -6,19 +6,24 @@
   file, You can obtain one at http://mozilla.org/MPL/2.0/.
 */
 module Type: {
-  type t =
-    | Person
-    | Dummy
-    | Missing
+  type t = Person | Dummy | Missing
 
   let toString: t => string
 }
 
+module NatInt: {
+  /** A natural (non-negative) integer. */
+  type t
+  let fromInt: int => t
+  let toInt: t => int
+  let toString: t => string
+}
+
 type t = {
   firstName: string,
   id: Data_Id.t,
   lastName: string,
-  matchCount: int,
+  matchCount: NatInt.t,
   rating: int,
   type_: Type.t,
 }
@@ -27,6 +32,11 @@ let fullName: t => string
 
 let compareName: (t, t) => int
 
+let succMatchCount: t => t
+let predMatchCount: t => t
+
+let setRating: (t, int) => t
+
 let encode: t => Js.Json.t
 
 let decode: Js.Json.t => t
@@ -38,7 +48,7 @@ let dummy: t
 
 /**
  This function should always be used in components that *might* not be able to
- display current player information. This includes bye rounds with \"dummy\"
+ display current player information. This includes bye rounds with "dummy"
  players, or scoreboards where a player may have been deleted.
  */
 let getMaybe: (Data_Id.Map.t<t>, Data_Id.t) => t

src/Data/Data_Ratings.res

@@ -18,7 +18,7 @@ module EloRank = {
     ->Int.fromFloat
 
   let getKFactor = (~matchCount, ~rating) =>
-    if matchCount < 30 {
+    if Data_Player.NatInt.toInt(matchCount) < 30 {
       40
     } else if rating > 2100 {
       10

src/Data/Data_Ratings.resi

@@ -7,7 +7,7 @@
 */
 module EloRank: {
   type t = int
-  let getKFactor: (~matchCount: int, ~rating: int) => int
+  let getKFactor: (~matchCount: Data_Player.NatInt.t, ~rating: int) => int
 }
 
 /**
@@ -16,7 +16,7 @@ module EloRank: {
 let calcNewRatings: (
   ~whiteRating: EloRank.t,
   ~blackRating: EloRank.t,
-  ~whiteMatchCount: int,
-  ~blackMatchCount: int,
+  ~whiteMatchCount: Data_Player.NatInt.t,
+  ~blackMatchCount: Data_Player.NatInt.t,
   ~result: Data_Match.Result.t,
 ) => (EloRank.t, EloRank.t)

src/PagePlayers.res

@@ -12,7 +12,7 @@ module Id = Data.Id
 
 let sortName = Hooks.GetString((. x: Player.t) => x.firstName)
 let sortRating = Hooks.GetInt((. x: Player.t) => x.rating)
-let sortMatchCount = Hooks.GetInt((. x: Player.t) => x.matchCount)
+let sortMatchCount = Hooks.GetInt((. x: Player.t) => Player.NatInt.toInt(x.matchCount))
 
 /**
  * This module is a heavily-modified version of code that was generated by the
@@ -30,7 +30,7 @@ module Form = {
   }
 
   module Output = {
-    type t = {firstName: string, lastName: string, rating: int, matchCount: int}
+    type t = {firstName: string, lastName: string, rating: int, matchCount: Player.NatInt.t}
   }
 
   module Validate = {
@@ -52,13 +52,13 @@ module Form = {
     let matchCount = matchCount =>
       switch Int.fromString(matchCount) {
       | None => Error("Match count must be a number")
-      | Some(rating) => Ok(rating)
+      | Some(i) => Ok(Player.NatInt.fromInt(i))
       }
   }
 
   module FieldStatuses = {
     type t = {
-      matchCount: FormHelper.fieldStatus<int>,
+      matchCount: FormHelper.fieldStatus<Player.NatInt.t>,
       rating: FormHelper.fieldStatus<int>,
       lastName: FormHelper.fieldStatus<string>,
       firstName: FormHelper.fieldStatus<string>,
@@ -401,8 +401,8 @@ module PlayerList = {
               <td className="table__player">
                 <Link to_=Player(p.id)> {p->Player.fullName->React.string} </Link>
               </td>
-              <td className="table__number"> {p.rating->string_of_int->React.string} </td>
-              <td className="table__number"> {p.matchCount->string_of_int->React.string} </td>
+              <td className="table__number"> {p.rating->React.int} </td>
+              <td className="table__number"> {p.matchCount->Player.NatInt.toInt->React.int} </td>
               <td>
                 <button className="danger button-ghost" onClick={event => delPlayer(event, p.id)}>
                   <Icons.Trash />
@@ -619,7 +619,7 @@ module Profile = {
       firstName,
       lastName,
       rating: Int.toString(rating),
-      matchCount: Int.toString(initialMatchCount),
+      matchCount: Player.NatInt.toString(initialMatchCount),
     })
     let input = Form.input(form)
     let playerName = input.firstName ++ " " ++ input.lastName

src/PageTournament/PageRound.res

@@ -134,30 +134,32 @@ module MatchRow = {
             ~result=newResult,
           )
         }
-        let whiteOpt = Option.map(whiteOpt, white => {...white, rating: whiteNewRating})
-        let blackOpt = Option.map(blackOpt, black => {...black, rating: blackNewRating})
-        switch m.result {
-        /* If the result hasn't been scored yet, increment the matchCounts */
-        | NotSet =>
-          Option.forEach(whiteOpt, white =>
-            playersDispatch(Set(whiteId, {...white, matchCount: white.matchCount + 1}))
-          )
-          Option.forEach(blackOpt, black =>
-            playersDispatch(Set(blackId, {...black, matchCount: black.matchCount + 1}))
-          )
-        /* If the result is being un-scored, decrement the matchCounts */
-        | WhiteWon | BlackWon | Draw | Aborted | WhiteAborted | BlackAborted
-          if newResult == NotSet =>
-          Option.forEach(whiteOpt, white =>
-            playersDispatch(Set(whiteId, {...white, matchCount: white.matchCount - 1}))
-          )
-          Option.forEach(blackOpt, black =>
-            playersDispatch(Set(blackId, {...black, matchCount: black.matchCount - 1}))
-          )
-        /* Simply update the players with new ratings. */
-        | WhiteWon | BlackWon | Draw | Aborted | WhiteAborted | BlackAborted =>
-          Option.forEach(whiteOpt, white => playersDispatch(Set(whiteId, white)))
-          Option.forEach(blackOpt, black => playersDispatch(Set(blackId, black)))
+        let whiteOpt = Option.map(whiteOpt, white => Player.setRating(white, whiteNewRating))
+        let blackOpt = Option.map(blackOpt, black => Player.setRating(black, blackNewRating))
+        if !Match.isBye(m) {
+          switch m.result {
+          /* If the result hasn't been scored yet, increment the matchCounts */
+          | NotSet =>
+            Option.forEach(whiteOpt, white =>
+              playersDispatch(Set(whiteId, Player.succMatchCount(white)))
+            )
+            Option.forEach(blackOpt, black =>
+              playersDispatch(Set(blackId, Player.succMatchCount(black)))
+            )
+          /* If the result is being un-scored, decrement the matchCounts */
+          | WhiteWon | BlackWon | Draw | Aborted | WhiteAborted | BlackAborted
+            if newResult == NotSet =>
+            Option.forEach(whiteOpt, white =>
+              playersDispatch(Set(whiteId, Player.predMatchCount(white)))
+            )
+            Option.forEach(blackOpt, black =>
+              playersDispatch(Set(blackId, Player.predMatchCount(black)))
+            )
+          /* Simply update the players with new ratings. */
+          | WhiteWon | BlackWon | Draw | Aborted | WhiteAborted | BlackAborted =>
+            Option.forEach(whiteOpt, white => playersDispatch(Set(whiteId, white)))
+            Option.forEach(blackOpt, black => playersDispatch(Set(blackId, black)))
+          }
         }
         let newMatch = {...m, result: newResult, whiteNewRating, blackNewRating}
         roundList
@@ -374,23 +376,17 @@ module Round = {
 
     let unMatch = (matchId, round) => {
       switch round->Rounds.Round.getMatchById(matchId) {
-      /* checks if the match has been scored yet & resets the players'
-       records */
-      | Some(match) if match.result != NotSet =>
-        [
-          (match.whiteId, match.whiteOrigRating),
-          (match.blackId, match.blackOrigRating),
-        ]->Array.forEach(((id, rating)) =>
+      /* checks if the match has been scored yet & resets the players' records */
+      | Some(match) if match.result != NotSet && !Match.isBye(match) =>
+        let reset = (id, rating) =>
           switch players->Map.get(id) {
-          /* If there was a dummy player or a deleted player then bail
-           on the dispatch. */
-          | None => ()
           | Some(player) =>
-            playersDispatch(Set(player.id, {...player, rating, matchCount: player.matchCount - 1}))
+            playersDispatch(Set(player.id, player->Player.setRating(rating)->Player.predMatchCount))
+          | None => () /* Don't try to set dummy or deleted players */
           }
-        )
-      | None
-      | Some(_) => ()
+        reset(match.whiteId, match.whiteOrigRating)
+        reset(match.blackId, match.blackOrigRating)
+      | None | Some(_) => ()
       }
       let newRound = round->Rounds.Round.removeMatchById(matchId)
       switch roundList->Rounds.set(roundId, newRound) {

src/PageTournament/PageTourney.res

@@ -102,27 +102,18 @@ module Sidebar = {
           ->Rounds.Round.toArray
           ->Array.forEach(match_ => {
             let {result, whiteId, blackId, whiteOrigRating, blackOrigRating, _} = match_
-            /* Don't change players who haven't scored. */
-            switch result {
-            | NotSet => ()
-            | WhiteWon
-            | BlackWon
-            | Draw
-            | Aborted
-            | WhiteAborted
-            | BlackAborted =>
-              [(whiteId, whiteOrigRating), (blackId, blackOrigRating)]->Array.forEach(((
-                id,
-                rating,
-              )) =>
+            if result != NotSet && !Match.isBye(match_) {
+              /* Don't change players who haven't scored. */
+              let reset = (id, rating) =>
                 switch players->Map.get(id) {
                 | Some(player) =>
-                  let matchCount = player.matchCount - 1
-                  playersDispatch(Set(player.id, {...player, matchCount, rating}))
-                /* Don't try to set dummy or deleted players */
-                | None => ()
+                  playersDispatch(
+                    Set(player.id, player->Player.setRating(rating)->Player.predMatchCount),
+                  )
+                | None => () /* Don't try to set dummy or deleted players */
                 }
-              )
+              reset(whiteId, whiteOrigRating)
+              reset(blackId, blackOrigRating)
             }
           })
         }

src/PageTournament/PairPicker.res

@@ -215,6 +215,7 @@ module Stage = {
     | Some(player) => player->getPlayer->Player.fullName
     }
 
+    /* TODO: should this increment match counts? */
     let match = _ =>
       switch (state.p1, state.p2) {
       | (Some(white), Some(black)) =>