Code review comment for lp:~widelands-dev/widelands/net-relay

Revision history for this message
SirVer (sirver) wrote :

I numbered the points for easier reference:

> 1) I changed the code to my suggestion. Please revert if you disagree.

> 2) It would be nice to get syncstream data into this somehow to make it easier to debug desyncs going forward.

Sorry, that was initial excitement while reviewing, it doesn't really belong in this code review. My thoughts on this topic are not coherent yet, so I have no clear design in mind. I pulled what I have so far out into a separate bug and removed the comment.

[1] https://bugs.launchpad.net/widelands/+bug/1734671

> 3) Ping/Pong

I agree that the relay should not auto-kick users. But it is uniquely positioned to determine the actual ping values that affect gameplay. So I think it should regularly inform the players (or at least the host) how the ping to the different players is. For this a regular ping (once every 5-10 seconds) to all players would be valuable.

I pulled out a bug to design the feature: https://bugs.launchpad.net/widelands/+bug/1734673. Of course Widelands site changes and UI are not part of this MR, but I suggest to implement the regular ping for all players directly.

« Back to merge proposal