From c53472ff954cf4ea6c73e0ad6fd7c0dc7465f1d8 Mon Sep 17 00:00:00 2001 From: Isaac Levy Date: Tue, 28 Mar 2017 23:00:01 -0400 Subject: [PATCH 1/2] Tweak Clock History format Don't store 0 sec flagged times. This fixes #2863 --- modules/game/src/main/BSONHandlers.scala | 57 ++++++++++--------- modules/game/src/main/BinaryFormat.scala | 5 +- modules/game/src/main/Game.scala | 6 +- modules/game/src/main/GameDiff.scala | 4 +- .../main/java/clockencoder/VarIntEncoder.java | 6 +- 5 files changed, 42 insertions(+), 36 deletions(-) diff --git a/modules/game/src/main/BSONHandlers.scala b/modules/game/src/main/BSONHandlers.scala index 7b320735f8..728617a529 100644 --- a/modules/game/src/main/BSONHandlers.scala +++ b/modules/game/src/main/BSONHandlers.scala @@ -57,7 +57,7 @@ object BSONHandlers { ) } - implicit val gameBSONHandler = new BSON[Game] { + implicit val gameBSONHandler: BSON[Game] = new BSON[Game] { import Game.BSONFields._ import PgnImport.pgnImportBSONHandler @@ -66,7 +66,6 @@ object BSONHandlers { private val emptyPlayerBuilder = playerBSONHandler.read(BSONDocument()) def reads(r: BSON.Reader): Game = { - val nbTurns = r int turns val winC = r boolO winnerColor map Color.apply val (whiteId, blackId) = r str playerIds splitAt 4 val uids = ~r.getO[List[String]](playerUids) @@ -76,22 +75,16 @@ object BSONHandlers { val win = winC map (_ == color) builder(color)(id)(uid)(win) } - val wPlayer = player(whitePlayer, White, whiteId, whiteUid) - val bPlayer = player(blackPlayer, Black, blackId, blackUid) - val createdAtValue = r date createdAt - val realVariant = Variant(r intD variant) | chess.variant.Standard - val gameClock = r.getO[Color => Clock](clock)(clockBSONReader(createdAtValue, wPlayer.berserk, bPlayer.berserk)) map (_(Color(0 == nbTurns % 2))) - val gameId = r str id - Game( - id = gameId, - whitePlayer = wPlayer, - blackPlayer = bPlayer, + + val g = Game( + id = r str id, + whitePlayer = player(whitePlayer, White, whiteId, whiteUid), + blackPlayer = player(blackPlayer, Black, blackId, blackUid), binaryPieces = r bytes binaryPieces, binaryPgn = r bytesD binaryPgn, status = r.get[Status](status), - turns = nbTurns, + turns = r int turns, startedAtTurn = r intD startedAtTurn, - clock = gameClock, positionHashes = r.bytesD(positionHashes).value, checkCount = { val counts = r.intsD(checkCount) @@ -101,19 +94,11 @@ object BSONHandlers { unmovedRooks = r.getO[UnmovedRooks](unmovedRooks) | UnmovedRooks.default, daysPerTurn = r intO daysPerTurn, binaryMoveTimes = r bytesO moveTimes, - clockHistory = for { - clk <- gameClock - start = Centis(clk.limit * 100) - bw <- r bytesO whiteClockHistory - bb <- r bytesO blackClockHistory - history <- BinaryFormat.clockHistory.read(start, bw, bb)(gameId) - } yield history, mode = Mode(r boolD rated), - variant = realVariant, - crazyData = (realVariant == Crazyhouse) option r.get[Crazyhouse.Data](crazyData), + variant = Variant(r intD variant) | chess.variant.Standard, next = r strO next, bookmarks = r intD bookmarks, - createdAt = createdAtValue, + createdAt = r date createdAt, updatedAt = r dateO updatedAt, metadata = Metadata( source = r intO source flatMap Source.apply, @@ -124,6 +109,20 @@ object BSONHandlers { analysed = r boolD analysed ) ) + + val gameClock = r.getO[Color => Clock](clock)(clockBSONReader(g.createdAt, g.whitePlayer.berserk, g.blackPlayer.berserk)) map (_(g.turnColor)) + + g.copy( + clock = gameClock, + crazyData = (g.variant == Crazyhouse) option r.get[Crazyhouse.Data](crazyData), + clockHistory = for { + clk <- gameClock + start = Centis(clk.limit * 100) + bw <- r bytesO whiteClockHistory + bb <- r bytesO blackClockHistory + history <- BinaryFormat.clockHistory.read(start, bw, bb, g.flagged, g.id) + } yield history + ) } def writes(w: BSON.Writer, o: Game) = BSONDocument( @@ -144,8 +143,8 @@ object BSONHandlers { unmovedRooks -> o.unmovedRooks, daysPerTurn -> o.daysPerTurn, moveTimes -> o.binaryMoveTimes, - whiteClockHistory -> clockHistory(White, o.clockHistory, o.clock), - blackClockHistory -> clockHistory(Black, o.clockHistory, o.clock), + whiteClockHistory -> clockHistory(White, o.clockHistory, o.clock, o.flagged), + blackClockHistory -> clockHistory(Black, o.clockHistory, o.clock, o.flagged), rated -> w.boolO(o.mode.rated), variant -> o.variant.exotic.option(o.variant.id).map(w.int), crazyData -> o.crazyData, @@ -162,17 +161,19 @@ object BSONHandlers { ) } - private def clockHistory(color: Color, clockHistory: Option[ClockHistory], clock: Option[Clock]): Option[ByteArray] = + private def clockHistory(color: Color, clockHistory: Option[ClockHistory], clock: Option[Clock], flagged: Option[Color]) = for { clk <- clock history <- clockHistory - } yield BinaryFormat.clockHistory.writeSide(Centis(clk.limit * 100), history.get(color)) + times = history.get(color) + } yield BinaryFormat.clockHistory.writeSide(Centis(clk.limit * 100), if (flagged has color) times.dropRight(1) else times) private[game] def clockBSONReader(since: DateTime, whiteBerserk: Boolean, blackBerserk: Boolean) = new BSONReader[BSONBinary, Color => Clock] { def read(bin: BSONBinary) = BinaryFormat.clock(since).read( ByteArrayBSONHandler read bin, whiteBerserk, blackBerserk ) } + private[game] def clockBSONWrite(since: DateTime, clock: Clock) = ByteArrayBSONHandler write { BinaryFormat clock since write clock } diff --git a/modules/game/src/main/BinaryFormat.scala b/modules/game/src/main/BinaryFormat.scala index 1ada8a3a29..00a33d7d4e 100644 --- a/modules/game/src/main/BinaryFormat.scala +++ b/modules/game/src/main/BinaryFormat.scala @@ -38,8 +38,9 @@ object BinaryFormat { def readSide(start: Centis, ba: ByteArray): Vector[Centis] = ClockEncoder.decode(ba.value, start.value).map(Centis.apply)(breakOut) - def read(start: Centis, bw: ByteArray, bb: ByteArray)(gameId: String /* for logging */ ): Option[ClockHistory] = Try { - ClockHistory(readSide(start, bw), readSide(start, bb)) + def read(start: Centis, bw: ByteArray, bb: ByteArray, flagged: Option[Color], gameId: String) = Try { + val history = ClockHistory(readSide(start, bw), readSide(start, bb)) + flagged.fold(history)(c => history.update(c, _ :+ Centis(0))) }.fold( e => { logger.warn(s"Exception decoding history on game $gameId", e); none }, some diff --git a/modules/game/src/main/Game.scala b/modules/game/src/main/Game.scala index 3ee50867e2..e5e2a95499 100644 --- a/modules/game/src/main/Game.scala +++ b/modules/game/src/main/Game.scala @@ -23,7 +23,7 @@ case class Game( status: Status, turns: Int, // = ply startedAtTurn: Int, - clock: Option[Clock], + clock: Option[Clock] = None, castleLastMoveTime: CastleLastMoveTime, unmovedRooks: UnmovedRooks, daysPerTurn: Option[Int], @@ -71,7 +71,7 @@ case class Game( def firstPlayer = player(firstColor) def secondPlayer = player(!firstColor) - def turnColor = Color(0 == turns % 2) + def turnColor = Color((turns & 1) == 0) def turnOf(p: Player): Boolean = p == player def turnOf(c: Color): Boolean = c == turnColor @@ -79,6 +79,8 @@ case class Game( def playedTurns = turns - startedAtTurn + def flagged = (status == Status.Outoftime).option(turnColor) + def fullIdOf(player: Player): Option[String] = (players contains player) option s"$id${player.id}" diff --git a/modules/game/src/main/GameDiff.scala b/modules/game/src/main/GameDiff.scala index a86915c7d7..bf38be3930 100644 --- a/modules/game/src/main/GameDiff.scala +++ b/modules/game/src/main/GameDiff.scala @@ -46,7 +46,9 @@ private[game] object GameDiff { for { clk <- g.clock history <- g.clockHistory - } yield (Centis(clk.limit * 100), history.get(color)) + curColor = g.turnColor + times = history.get(color) + } yield (Centis(clk.limit * 100), if (g.flagged has color) times.dropRight(1) else times) def clockHistoryToBytes(o: Option[ClockHistorySide]) = o.map { case (x, y) => ByteArrayBSONHandler.write(BinaryFormat.clockHistory.writeSide(x, y)) diff --git a/modules/game/src/main/java/clockencoder/VarIntEncoder.java b/modules/game/src/main/java/clockencoder/VarIntEncoder.java index cab2df6753..cda6cb1852 100644 --- a/modules/game/src/main/java/clockencoder/VarIntEncoder.java +++ b/modules/game/src/main/java/clockencoder/VarIntEncoder.java @@ -29,11 +29,11 @@ public class VarIntEncoder { public static int readUnsigned(BitReader reader) { int n = reader.readBits(6); - if ((n & 0x20) != 0) { - n ^= 0x20; + if (n > 0x1F) { + n &= 0x1F; int curShift = 5; int curVal; - while (((curVal = reader.readBits(4)) & 0x08) != 0) { + while ((curVal = reader.readBits(4)) > 0x07) { n |= (curVal & 0x07) << curShift; curShift += 3; } From 61bd81108f0fe6efe529aa29db56146400df204a Mon Sep 17 00:00:00 2001 From: Isaac Levy Date: Thu, 30 Mar 2017 13:21:34 -0400 Subject: [PATCH 2/2] Clean up flagged decoder --- modules/game/src/main/BSONHandlers.scala | 2 +- modules/game/src/main/BinaryFormat.scala | 18 ++++++++++++------ modules/game/src/main/GameDiff.scala | 6 +++--- .../game/src/test/BinaryClockHistoryTest.scala | 18 +++++++++--------- 4 files changed, 25 insertions(+), 19 deletions(-) diff --git a/modules/game/src/main/BSONHandlers.scala b/modules/game/src/main/BSONHandlers.scala index 728617a529..a14d330414 100644 --- a/modules/game/src/main/BSONHandlers.scala +++ b/modules/game/src/main/BSONHandlers.scala @@ -166,7 +166,7 @@ object BSONHandlers { clk <- clock history <- clockHistory times = history.get(color) - } yield BinaryFormat.clockHistory.writeSide(Centis(clk.limit * 100), if (flagged has color) times.dropRight(1) else times) + } yield BinaryFormat.clockHistory.writeSide(Centis(clk.limit * 100), times, flagged has color) private[game] def clockBSONReader(since: DateTime, whiteBerserk: Boolean, blackBerserk: Boolean) = new BSONReader[BSONBinary, Color => Clock] { def read(bin: BSONBinary) = BinaryFormat.clock(since).read( diff --git a/modules/game/src/main/BinaryFormat.scala b/modules/game/src/main/BinaryFormat.scala index 00a33d7d4e..31308cc967 100644 --- a/modules/game/src/main/BinaryFormat.scala +++ b/modules/game/src/main/BinaryFormat.scala @@ -32,15 +32,21 @@ object BinaryFormat { object clockHistory { private val logger = lila.log("clockHistory") - def writeSide(start: Centis, times: Vector[Centis]): ByteArray = - ByteArray(ClockEncoder.encode(times.map(_.value)(breakOut), start.value)) + def writeSide(start: Centis, times: Vector[Centis], flagged: Boolean) = { + val timesToWrite = if (flagged) times.dropRight(1) else times + ByteArray(ClockEncoder.encode(timesToWrite.map(_.value)(breakOut), start.value)) + } - def readSide(start: Centis, ba: ByteArray): Vector[Centis] = - ClockEncoder.decode(ba.value, start.value).map(Centis.apply)(breakOut) + def readSide(start: Centis, ba: ByteArray, flagged: Boolean) = { + val decoded: Vector[Centis] = ClockEncoder.decode(ba.value, start.value).map(Centis.apply)(breakOut) + if (flagged) decoded :+ Centis(0) else decoded + } def read(start: Centis, bw: ByteArray, bb: ByteArray, flagged: Option[Color], gameId: String) = Try { - val history = ClockHistory(readSide(start, bw), readSide(start, bb)) - flagged.fold(history)(c => history.update(c, _ :+ Centis(0))) + ClockHistory( + readSide(start, bw, flagged has White), + readSide(start, bb, flagged has Black) + ) }.fold( e => { logger.warn(s"Exception decoding history on game $gameId", e); none }, some diff --git a/modules/game/src/main/GameDiff.scala b/modules/game/src/main/GameDiff.scala index bf38be3930..3320c08547 100644 --- a/modules/game/src/main/GameDiff.scala +++ b/modules/game/src/main/GameDiff.scala @@ -16,7 +16,7 @@ private[game] object GameDiff { type Set = BSONElement // [String, BSONValue] type Unset = BSONElement // [String, BSONBoolean] - type ClockHistorySide = (Centis, Vector[Centis]) + type ClockHistorySide = (Centis, Vector[Centis], Boolean) def apply(a: Game, b: Game): (List[Set], List[Unset]) = { @@ -48,10 +48,10 @@ private[game] object GameDiff { history <- g.clockHistory curColor = g.turnColor times = history.get(color) - } yield (Centis(clk.limit * 100), if (g.flagged has color) times.dropRight(1) else times) + } yield (Centis(clk.limit * 100), times, g.flagged has color) def clockHistoryToBytes(o: Option[ClockHistorySide]) = o.map { - case (x, y) => ByteArrayBSONHandler.write(BinaryFormat.clockHistory.writeSide(x, y)) + case (x, y, z) => ByteArrayBSONHandler.write(BinaryFormat.clockHistory.writeSide(x, y, z)) } val w = lila.db.BSON.writer diff --git a/modules/game/src/test/BinaryClockHistoryTest.scala b/modules/game/src/test/BinaryClockHistoryTest.scala index 0c70f5e636..7d32aa831b 100644 --- a/modules/game/src/test/BinaryClockHistoryTest.scala +++ b/modules/game/src/test/BinaryClockHistoryTest.scala @@ -15,13 +15,13 @@ class BinaryClockHistoryTest extends Specification { "binary clock history" should { "handle empty vectors" in { - BinaryFormat.clockHistory.writeSide(Centis(720000), Vector.empty).isEmpty must beTrue + BinaryFormat.clockHistory.writeSide(Centis(720000), Vector.empty, false).isEmpty must beTrue } "handle singleton vectors" in { val times = Vector(Centis(1234)) - val bytes = BinaryFormat.clockHistory.writeSide(Centis(12345), times) - val restored = BinaryFormat.clockHistory.readSide(Centis(12345), bytes) + val bytes = BinaryFormat.clockHistory.writeSide(Centis(12345), times, false) + val restored = BinaryFormat.clockHistory.readSide(Centis(12345), bytes, false) restored.size must_== 1 (restored(0) - times(0)).abs should be_<=(eps) @@ -32,16 +32,16 @@ class BinaryClockHistoryTest extends Specification { 0, 3, 6, 9, 12, 15, 18, 21, 24, 27, 30, 33, 36, 39, 42, 45, 48, 51, 54, 57, 60, 63, 66, 69, 72, 75, 78, 81, 84, 87, 90, 93, 96, 99, 102, 105, 108, 199, 333, 567, 666, 2000, 30 ).map(t => Centis(21000 - 10 * t)) - val bytes = BinaryFormat.clockHistory.writeSide(hour * 2, times) - val restored = BinaryFormat.clockHistory.readSide(hour * 2, bytes) + val bytes = BinaryFormat.clockHistory.writeSide(hour * 2, times, false) + val restored = BinaryFormat.clockHistory.readSide(hour * 2, bytes, false) times.size must_== restored.size (restored, times).zipped.map(_ - _).forall(_.abs <= eps) should beTrue } "restore correspondence" in { val times = Vector(118, 204, 80, 191, 75, 230, 48, 258).map(t => day * 2 - Centis(t)) - val bytes = BinaryFormat.clockHistory.writeSide(day * 2, times) - val restored = BinaryFormat.clockHistory.readSide(day * 2, bytes) + val bytes = BinaryFormat.clockHistory.writeSide(day * 2, times, false) + val restored = BinaryFormat.clockHistory.readSide(day * 2, bytes, false) times.size must_== restored.size (restored, times).zipped.map(_ - _).forall(_.abs <= eps) should beTrue } @@ -51,8 +51,8 @@ class BinaryClockHistoryTest extends Specification { var restored = Vector.empty[Centis] val start = Centis(6000) for (end <- times) { - val binary = BinaryFormat.clockHistory.writeSide(start, restored :+ end) - restored = BinaryFormat.clockHistory.readSide(start, binary) + val binary = BinaryFormat.clockHistory.writeSide(start, restored :+ end, false) + restored = BinaryFormat.clockHistory.readSide(start, binary, false) } times.size must_== restored.size (restored, times).zipped.map(_ - _).forall(_.abs <= eps) should beTrue