チケット #45207

json protocol issue

登録: 2022-07-24 22:31 最終更新: 2022-07-26 17:22

報告者:
担当者:
チケットの種類:
状況:
完了
コンポーネント:
マイルストーン:
優先度:
5 - 中
重要度:
5 - 中
解決法:
修正済み
ファイル:
2

詳細

It seems that something has gone wrong with hrm #745593

Freeciv-web isn't updated to a git revision with that commit yet, but when I apply it as a patch on top of a commit over with which it should work, server <-> web client communication breaks. I don't know when things have broken that way - presumably with the last rebase I did for the patch, though I think I did testing with that too once already.

チケットの履歴 (14 件中 3 件表示)

2022-07-24 22:31 更新者: cazfi
  • 新しいチケット "json protocol issue" が作成されました
2022-07-24 22:40 更新者: alienvalkyrie
コメント

Heads up: If this ends up requiring changes to generate_packets.py, those will have to wait until after (the relevant part of) #45206 is done.

Also, if those changes have to do with an edge case in the non-delta versions of diff arrays (which it probably doesn't, realistically speaking), that'll get fixed along the way by itself (keeping the problem would require extra work).

2022-07-24 23:20 更新者: cazfi
  • コンポーネント(未割り当て) から General に更新されました
コメント

Reproducible with desktop server + client configured with "--enable-debug --enable-json --disable-delta-protocol"

No matter which direction you try to move units, they always move up (direction 0, I think)

You think this is something you could debug while working on generate_packets.py & related stuff, alienvalkyrie?

2022-07-24 23:53 更新者: alienvalkyrie
コメント

I'll take a look at it, tho unless it ends up being a sufficiently obvious error in the generation script, I'll likely have to take some time to familiarize myself with the dataio_json internals.

2022-07-25 01:50 更新者: alienvalkyrie
コメント

The problem is actually larger - the entire unit_order struct is zeroed, not just the dir; it just so happens that (enum unit_orders)(0) is ORDER_MOVE, so the unit still moves, and that doesn't care about any of the other fields. Generated code looks clean, and so does dio_get_unit_order_json() ~> error lies deeper.

Likely culprit: plocation_read_elem() doesn't recurse properly. It does some sanity checks, and seems to recurse into nested arrays, but not into objects inside arrays – which is exactly the case in PACKET_UNIT_ORDERS. So we're trying to reinterpret the object as (in this case) an integer, and since json_integer_value() has no way of telling us about an error, we just silently get 0.

Note that the other dio_get_<struct>_json() functions (that are actually used), i.e. requirement, worklist, and action probability, all sidestep this problem by grabbing the object and manually reading from it using dio_get_*_internal(), rather than temporarily setting a sub_location on the location it gets and using public-facing dio_get_*() functions.

2022-07-25 02:03 更新者: alienvalkyrie
  • 担当者(未割り当て) から alienvalkyrie に更新されました
  • 解決法なし から 受領 に更新されました
コメント

Attached patch makes plocation_read_elem() function analogously to plocation_read_field() (i.e. rolls back that part); tested and it seems to fix the issue. It does get rid of the checks that were there before, but I'm not convinced those checks were really necessary / sensible – if they are, we should bring them back and mirror them in plocation_read_field().

(編集済, 2022-07-25 02:35 更新者: alienvalkyrie)
2022-07-25 02:45 更新者: cazfi
コメント

With this "--enable-debug --enable-json" (delta protocol not disabled) game does not launch at all: "1: ERROR: Unable to get uint8 from location: global_advances"

Without the patch it suffered from the same problem as the deltaless json once you are in the game.

2022-07-25 03:17 更新者: alienvalkyrie
コメント

Reply To cazfi

With this "--enable-debug --enable-json" (delta protocol not disabled) game does not launch at all: "1: ERROR: Unable to get uint8 from location: global_advances" Without the patch it suffered from the same problem as the deltaless json once you are in the game.

That seems like it might be a different problem with writing diff-arrays. Do note that with --enable-json --disable-delta-protocol, the global_advances array will not have really been read at all – that's the edge case in non-delta versions of diff arrays that I mentioned before (which will be passively fixed by #45219).

Looks like JSON array-diff put produces [[index, value], [i,v], 255], when it should be [[index, value], [i, v], [255]], i.e. the terminating index 255 doesn't get wrapped in an array like it should. Since it seems like that was also introduced in hrm # 745593, should we fold that into this ticket as "fix regressions", or make it a separate one?

2022-07-25 03:29 更新者: alienvalkyrie
コメント

Decided to go the "fold both into one patch" route. Still need to test that this didn't break the non-delta-protocol again.

2022-07-25 03:48 更新者: alienvalkyrie
コメント

Both delta and non-delta appear to work correctly now (apart from the other issue I mentioned, which I've already fixed locally as part of #45219).

2022-07-26 17:22 更新者: alienvalkyrie
  • 解決法受領 から 修正済み に更新されました
2022-07-26 17:22 更新者: alienvalkyrie
  • 状況オープン から 完了 に更新されました

編集

このチケットにコメントを追加するには、ログインが必要です » ログインする