ddnet / ddnet

DDraceNetwork, a free cooperative platformer game
https://ddnet.org
Other
586 stars 416 forks source link

improve player join log line to contain more information #7876

Open jxsl13 opened 9 months ago

jxsl13 commented 9 months ago

https://github.com/ddnet/ddnet/blob/9a8541648c33f1879fbc7fc9b9f1e825bed6a591/src/engine/server/server.cpp#L1675

would be great if this log line was changed to contain as much player information as possible in order to reproduce the current server state by parsing logs / Econ.

https://github.com/teeworlds/teeworlds/issues/2475#issuecomment-944926682

reference log line

str_format(aBuf, sizeof(aBuf),"id=%d addr=%s version=%d name='%s' clan='%s' country=%d", ClientID, aClientAddr, ClientVersion, pName, pClan, Country);

example how the ddnet log line could look like:

player has entered the game. ClientID=%d addr=<{%s}> sixup=%d version=%d name='%s' clan='%s' country=%d
ChillerDragon commented 9 months ago

As a fellow log parser I like it. But the line feels a bit long :c

jxsl13 commented 9 months ago

One could shorten the prefix to something like player entered or client entered. but In wanted to keep it as backwards compatible as possible.

ChillerDragon commented 9 months ago

Yea the just appending stuff at the end is convenient. While at it to change the log format and setting something new that we would want to stay backwards compatible to. How about we make it safer to parse. As in introduce proper escaping or encoding. What if someone uses the name foo'bar and the clan ' country= then it shows up as

player has entered the game. ClientID=0 addr=<{127.0.0.1}> sixup=1 version=0 name='foo'bar' clan='' country=' country=0
jxsl13 commented 9 months ago

it does not matter for a decent regex implementation. They are (usually) greedy trying to find the longest (sub)match.

Example: https://regex101.com/r/zNPepP/2

ChillerDragon commented 9 months ago

Okay LGTM. I the chilled dragon :dragon: do hereby approve this RFC. :rocket: :heavy_check_mark:

heinrich5991 commented 9 months ago

I don't see clan and country to be all that useful as these can change without log message. Name would be nice though.

jxsl13 commented 9 months ago

There are two purposes that one might want have for that:

The first purpose is to attach an econ or log parser that is able to replicate the whole state of the server by parsing logs and allowing to visualize the state easily with languages other than C++ with e.g. a website or a discord bot, without having to poll master servers. Posting a gameover statboard in the context of ddnet-insta would be such a use case (even tho the current approach is different but ddnet-insta may not be the last pvp mod that's based on ddnet and has a statboard). Every possible information would be necessary for this as well as a log line for every state transition.

My (current) primary use case for this log line is primarily hooking into certain event log lines like

Some of those are just thoughts that might become a use case but for now it's just all of the stuff related to moderation and not necessarily trivia bots and so on.

heinrich5991 commented 9 months ago

The first purpose is to attach an econ or log parser that is able to replicate the whole state of the server by parsing logs and allowing to visualize the state easily with languages other than C++ with e.g. a website or a discord bot, without having to poll master servers.

Starting to support that sounds like a pain, especially since log message changes now become breaking…

jxsl13 commented 9 months ago

that's not my main purpose but yeah, that'd be a complete logging rework, preferrably with unique keywords describing state transitions. it's out of scope of this issue, I'd say.

jxsl13 commented 9 months ago

player has entered the game. ClientID=%d addr=<{%s}> sixup=%d version=%d name='%s' clan='%s'

jxsl13 commented 3 months ago

bump