N_SERVINFO should always be replied to with N_CONNECT

[sauerbraten]

When players use the /reconnect command, gamedisconnect() doesn't run (same as vanilla), and the player1 CN is kept. As a result this bit should fail to re-send a N_CONNECT packet, since sendintro() isn't executed: https://github.com/tpoechtrager/wc-ng/blob/761b78d0507542eaa2115f72e433b9237c00c2ff/src/fpsgame/client.cpp#L1468-L1470

This is probably bad, isn't it? Players reported trouble with autoauth on Discord, and @benzomatic mentioned issues with spaghettimod, which might send multiple N_SERVINFOs with its network fences feature (related to limbo/martian stuff I believe).

[benzomatic]

To kind of add to what pix laid out above:

Autoauth doesn't seem to work at all in the latest available release builds, me and another player experienced that. It seems to successfully auto-request the authkey but then never actually claims auth. Claiming auth manually however works just fine.

Stopping sendintro() to be sent multiple times also has a side effect on spaghettimod, which in its network fences module relies on sending N_SERVINFO and receiving back N_CONNECTs multiple times per session, most notably for a module that determines when players loaded the map, to send MOTDs, set up some configuration on map start etc. The subject already came up in #4 where you swiftly fixed a related bug with your demo recorder. I don't think the sendintro() ping-pong causes negative side effects now (not in vanilla, at least) so it would be nice if the restriction was lifted again, and maybe it even helps with the autoauth issue.

[sauerbraten]

By the way, I looked through the connect/disconnect code a bit, and wonder why you don't put your mod::chat::serv[dis]connect hooks into gameconnect() and gamedisconnect()? Those seem like the "most correct" places to put them? If you need the chat server to know when the client actually joined the game, maybe add an idempotent mod::chat::ready() hook to broadcast the player's CN on the server and call that on each N_SERVINFO processing? But I don't see anything wrong with letting the client read the chat while in limbo (immediately after ENET_EVENT_TYPE_CONNECT and before getting his CN via N_SERVINFO).

[tpoechtrager]

I need the server description, that's why I put it in N_SERVINFO.

I restored the default behavior of N_SERVINFO. Does this fix the issue(s)?

[sauerbraten]

Ah I see. Don't other chat clients have the servinfo from their own copy of the master server list? Or send server description together with CN from a new ready() hook and introduce a C_SERV_JOIN message type.

Anyway, pretty sure this should fix the issue, but I can't test the fix for you. Maybe release it in a new build and ask players to try it. This change is unlikely to break anything else I would say 👍

[benzomatic]

It definitely fixes the N_SERVINFO spaghettimod issue, thank you for the quick update!

For some reason though, autoauth still seems kind of bugged between wc-ng clients and spaghettimod, so i'll run some more tests as well. I've tested autoauth on most other server mods as well as vanilla and there it seems to work perfectly fine. 👍