Feature: add experimental flag to use the mcutils library for pings #22
Conversation
- Added flag `--use-mc-utils` to enable the mcutils ping behavior - Sprinkle of debug messages as I didn't see integration tests to extend - Confirmed working on a live 1.16.5 modded forge server running Enigmatica 6 v1.5.2 - Duplicated existing pattern for legacy server list ping, but it could be more DRY
|
Hello @chloeruka, I would be happy to fix the behavior of the query package. I try to make a library that works in the greatest number of cases. Unfortunately, I can't test it against all server implementations out there. Can you please send me more informations about the behavior ? I'll fix it ASAP. Feel free to open an issue on the mcutils' repo. Thank you for using mcutils and for your feedback. |
itzg
left a comment
itzg
left a comment
There was a problem hiding this comment.
Awesome. Thanks! Would you be interested in adding a tiny section to the README? I'm on the fence, so totally up to you.
|
After this is merged and tagged can add an option to enable it in itzg/minecraft-server. |
|
Done! Thanks for the review @itzg - do the telegraf and prometheus endpoints need a change to allow the flag to be used there too? |
Good question. Apparently I didn't refactor much at all, so they each make distinct use of mcpinger. So, yes, the flags, etc would need to be repeated for those; however, they don't seem as important for now. So could be saved for another PR. |
|
This is now released in https://github.com/itzg/mc-monitor/releases/tag/0.11.0 |
|
Since there's already |
3 participants
@xrjr has made a library called mcutils that seems to be a slightly more complete implementation of the 1.7+ server list ping & FML2 protocol extension
Changes
--use-mc-utilsto enable the mcutils ping behaviorMotivation
This relates to #21 as it provides a mechanism for solving that error by using a different MC protocol API behind an experimental flag that handles FML2 protocol decorations more gracefully.
Why put this behind an experimental flag?
I don't feel comfortable (yet) suggesting mcutils as a full replacement for mc-pinger without more extensive testing. While the mcutils implementation is more complete and it's very well documented, it is lacking test coverage and
one of its packages (query) also broke against my Forge serverit works fine, this was user error. Ping seems to be all that's needed for mc-monitor, it has all the fields we normally want, and I'm not fussy about it for a cheap fork. Still, I thought it was worth socializing the fix in case it's useful to other people somehow.Edit: correction on
query