widelands-dev team mailing list archive

[Notabilis <notabilis27@xxxxxx>]

Thanks for the review. I have a few questions regarding your NOCOMs, though.

> You can and should use std::piecewise_construct here and get rid of the default constructor for client

I guess you mean something like (not working):
 clients_.emplace(std::piecewise_construct, id, Client::State::kConnecting);
So instead of the default constructor I will have one taking the state of the argument? That could also be achieved by
 clients_.emplace(id, Client::State::kConnecting);
I don't really see why I should get rid of the default constructor, and I guess you mean something else.

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

What exactly do you mean with this? Writing all transfered data to the syncstream? Only the commands/headers? Or just when errors occur? Desyncs are something which does not (or at least: should not) matter on this layer, the relay is happily transmitting even useless data.

> It would be nice to always get a kPong on a kPing ...

Can do so, no problem. Actually, that is what happening already (but pings are not always send). Currently, the pings are only used for the game host and not for the clients. Do you want to have pings for all participants? Kicking out slow clients is a feature I see as a responsibility of the "higher" layers, i.e., the GameHost class, and nothing done by the relay.
-- 
https://code.launchpad.net/~widelands-dev/widelands/net-relay/+merge/332386
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/net-relay into lp:widelands.