Skip to content

Make fields optional in GameEventPlayer#48

Merged
ion232 merged 2 commits intoion232:mainfrom
yzoug:zoug/fix/deserialize-gameeventplayer-ai
Jan 30, 2024
Merged

Make fields optional in GameEventPlayer#48
ion232 merged 2 commits intoion232:mainfrom
yzoug:zoug/fix/deserialize-gameeventplayer-ai

Conversation

@yzoug
Copy link
Contributor

@yzoug yzoug commented Jan 27, 2024

When describing a bot, everything is empty except the AI level field.

@yzoug yzoug changed the title fix: make fields optional in GameEventPlayer Make fields optional in GameEventPlayer Jan 27, 2024
@ion232
Copy link
Owner

ion232 commented Jan 27, 2024

Thanks @yzoug. Could you also provide an example JSON file in tests/data/response and add a unit test in response_models.rs?

yzoug added 2 commits January 28, 2024 20:50
When describing a bot, everything is empty except the AI level field.
The GameFull struct is slightly different for a game between humans and
when playing against an AI, we hence change the game_full json to
game_full_humans, and add game_full_ai describing a game against an AI.
@yzoug yzoug force-pushed the zoug/fix/deserialize-gameeventplayer-ai branch from 2118903 to a5165bf Compare January 28, 2024 20:06
@yzoug
Copy link
Contributor Author

yzoug commented Jan 28, 2024

Hello @ion232, I've added a more global test with the response GameFull against an AI (this is where it was blocking for me, since the GameEventPlayer couldn't be deserialized when receiving GameFull against an AI). I renamed the originally tested GameFull to game_full_humans.json to explicit the difference. I hope it's OK, don't hesitate if you seen any problem. Cheers!

@ion232
Copy link
Owner

ion232 commented Jan 30, 2024

Looks good 👍

@ion232 ion232 merged commit b49bf0a into ion232:main Jan 30, 2024
@yzoug
Copy link
Contributor Author

yzoug commented Jan 30, 2024

Thanks!
However it seems the test is failing :/ I've overlooked this, sorry.

The problem is that for a player, the title field is present in the answer and is null if the player has no title. However for an AI, the field is not present. So I can't just annotate with #[serde(skip_serializing_if = "Option::is_none")] (I also need that annotation on the fields I turned to options). I basically need that for a GameEventPlayer that is a bot, but not for a GameEventPlayer that is a human.

Here's an example:

$ curl -H "Authorization: Bearer TOKEN" https://lichess.org/api/board/game/stream/CLWBLVW2 -m1 |  jq '.'
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   454    0   454    0     0    453      0 --:--:--  0:00:01 --:--:--   453
curl: (28) Operation timed out after 1001 milliseconds with 454 bytes received
{
  "id": "CLWBLVW2",
  "variant": {
    "key": "standard",
    "name": "Standard",
    "short": "Std"
  },
  "speed": "correspondence",
  "perf": {
    "name": "Correspondance"
  },
  "rated": false,
  "createdAt": 1706655961028,
  "white": {
    "aiLevel": 1 # <------- No field "title".
  },
  "black": {
    "id": "saintmau46",
    "name": "saintmau46",
    "title": null,  # <----- No title, but the field is here
    "rating": 1500,
    "provisional": true
  },
  "initialFen": "startpos",
  "type": "gameFull",
  "state": {
    "type": "gameState",
    "moves": "e2e3",
    "wtime": 2147483647,
    "btime": 2147483647,
    "winc": 0,
    "binc": 0,
    "status": "started"
  }
}

It's late where I am, but I'll look into this tomorrow to fix. I don't really see how though for now, but I'll try to find an answer.

@yzoug yzoug deleted the zoug/fix/deserialize-gameeventplayer-ai branch January 30, 2024 23:17
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.

2 participants