HomeWildfire Games

Network cleanup, fixes #3953.
Concern RaisedrP18140


Network cleanup, fixes #3953.

Don't compare for "" to identify the host, but check for a new boolean flag that is set by the client, refs #2854.
Remove an unneeded IP address conversion from u32 to string, refs #3241.

Event Timeline

elexis raised a concern with this commit.Jun 2 2018, 4:55 PM
elexis added subscribers: leper, elexis.

I don't know how letting the client chose if it wants to be the host and controller of the game could be considered a good idea.

Even when not changing the code, it is possible to connect before connected (if the client already started the join before the server was opened. Can happen in the lobby if the gamelist isn't updated quickly enough when rehosting a game or outside of the lobby when players made an appointment). (I think this has already happened in a game via internet and when playing with lag simulation in local games. In these cases the external client became assigned to player 1 and the other player is unassigned.)

We should rely on good software protocol patterns rather than an estimation of a likelihood of someone abusing this.
While the hardcoding of the IP wasn't ideal, the new code seems worse to me.

Originally this commit was done to address a concern by leper on rP17772 from 2016-03-14-QuakeNet-#0ad-dev.log:

23:44 <@leper> r17772 Why is that ipstringfunction needed at all?

But the flag wasn't his idea. From 2016-05-01-QuakeNet-#0ad-dev.log:

11:05 < Philip> elexis: For the server side, maybe send that flag from the client in one of the network setup messages, and the server can change the timeout once it sees that flag
11:09 < Philip> ((e.g. my web server thinks everybody comes from, because they're actually being proxied through Squid))

It were better to generate a secret when starting both server and initial client and have the client authenticate using that.

This commit now has outstanding concerns.Jun 2 2018, 4:55 PM
This comment was removed by vladislavbelov.