Skip to content

Conversation

@schlawg
Copy link
Contributor

@schlawg schlawg commented Feb 6, 2023

requested for mobile:

"players": [
  { "color": "white", "user": { "name": "Stockfish 8" }},
  {
    "color": "black",
    "user": {
      "name": "lizen29",
      "title": "WGM",
      "id": "lizen29"
      "rating": 2594
    },
  }
],

@benediktwerner
Copy link
Member

To at least feign the appearance of consistency, it should probably be more like

{
  "white": {},
  "black": {}
}

I guess maybe even just exactly like the game export API: https://github.com/lichess-org/lila/blob/master/modules/api/src/main/GameApiV2.scala/#L291

same as board api.  hey, they all asked for the moves.  also give times because hey free data.
@trevorbayless
Copy link
Member

trevorbayless commented Feb 7, 2023

I believe the follow up request for adding the moves list was for the "Watch TV" feature of the mobile app to be able to show the move list of the TV game. If the new moves field is only supplied in the initial stream object there still won't be enough information to continually display a move list. Reason being is that the lm field in the follow up stream objects is not valid UCI format (it's only provides the squares of the move - eg. it does not have valid castling notation, nor does it provide the promotion piece, etc). api/board/game/stream/{gameId} handles this by providing the move list on every gameState event.

@schlawg
Copy link
Contributor Author

schlawg commented Feb 7, 2023

Waiting for instructions on the fen, moves, and lastMove field wrt stream delay, but now it looks like this:

{
  "id": "yJ8p3Z8Y",
  ...
  "fen": "rnbqkbnr/pppp1ppp/4p3/8/4P3/5P2/PPPP2PP/RNBQKBNR b KQkq - 0 2",
  "player": "black",
  "turns": 4,
  ...
  "white": { "name": "Lola", "id": "lola", "rating": "1947" },
  "black": { "name": "Ana", "id": "ana", "rating": "1856" },
  "wtime": 2147483647,
  "btime": 2147483647,
  "winc": 0,
  "binc": 0,
  "moves": [
    "e2e4",
    "e7e6",
    "f2f3"
  ],
  "lastMove": "f2f3"
}

@schlawg
Copy link
Contributor Author

schlawg commented Feb 7, 2023

I don't see how the new moves field is any better than a stream building it from ndjson.

* master:
  export akka.actor.Scheduler
  New Crowdin updates (lichess-org#12325)
  read nginx stuffing header
  fix prev commit
  Fix eslint
  accuracy requires analysis
  Check whether auto comm flags are critical
  add login monitoring
  extract linkPopup and apply it to profile links
  code golf
  restore enum RateLimit.Result
  also consider logins from TOR
  iptrust rate limiter
  import cats.syntax.all.*
  More arena and team battle translations (lichess-org#12326)
  analyser test warning
  further increase hasher security
  Translate bell notification sound pref
"source" -> game.source,
"status" -> game.status,
"createdAt" -> game.createdAt,
"players" -> players(game)
Copy link
Collaborator

@ornicar ornicar Feb 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this breaks the compatibility of many endpoints

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nevermind, it just reverts a change that was made in the same PR

@ornicar
Copy link
Collaborator

ornicar commented Feb 8, 2023

lila.game.JsonView.apply is a basic JSON building block that's used by a number of other JSON producers. Many of which are enriching it with player information. The lila game page itself for example, and the game replay JSON embedded in puzzles.
I'm reviewing these callers to see how to avoid duplicating data in the produced json.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants