py-mine
Parse forge mods from status response
This includes forgeData key in the answer and other modded cores, if they provide this information.
forgeDataisn't a part of minecraft's official protocol, hence mcstatus doesn't support it. HoweverforgeDataseems to just be a part of the status response, as just an additional key. This means that it's already being obtained and is accessible, why would having a bool in the reader/writer classes help?Mcstatus probably isn't the place to introduce these kinds of changes, as you're not expected to use these classes in your code. These classes are considered internal, and they are not a part of the public API. This means they're only expected to be used for internal functionalities, and if they're not necessary for those, they're not needed.
If you do need this kind of change, either implement it locally within your project, or consider checking out https://github.com/py-mine/mcproto, which already supports this behavior, and where the reader/writer classes are indeed a part of the public API: see the implementation (enum def, writer and reader).
Originally posted by @ItsDrike in https://github.com/py-mine/mcstatus/issues/555#issuecomment-1565999619
forgeDataisn't a part of minecraft's official protocol, hence mcstatus doesn't support it.
We support modded server cores as well as vanilla one (#446, #433, etc). Although, they usually just follow the protocol, we sometimes change some things to fit their needs (#439, we didn't have a required part of request, but vanilla was fine with it, so why we would change it?).
However
forgeDataseems to just be a part of the status response, as just an additional key.
It's and it doesn't have documentation which makes it hard to general user to understand it (it has quite strange structure and multiple fully different versions).
These classes are considered internal, and they are not a part of the public API.
They are, but they are also often used and referenced. Both mcstatus.io and mcsrvstat.us parse forgeData property and show this information to user, so would we don't do this?
They are, but they are also often used and referenced. Both mcstatus.io and mcsrvstat.us parse forgeData property and show this information to user, so would we don't do this?}
Perhaps, however as forgeData is just a part of status, I still don't understand why a bool reader/writer is necessary. It's not like we need to read an additional packet that contains this data, we already read it as a part of status, it's essentially just another JSON object. Does it contain some bytearrays that need to be read with a buffer (or as we call it connection) reader? Or what's the reason we would need this additional bool operations?
Edit: Scratch this, I thought I was commenting within the #555 PR, not in a new issue.
It's and it doesn't have documentation which makes it hard to general user to understand it (it has quite strange structure and multiple fully different versions).
This sounds concerning, I don't necessarily love the idea of adding support for parsing something that might constantly end up changing with newer versions in major ways. But it depends on what these changes are, how many of them we'd need to support at a time, and other factors.
They are, but they are also often used and referenced. Both mcstatus.io and mcsrvstat.us parse forgeData property and show this information to user, so would we don't do this?
In this comment, I was talking about the Connection classes, and the bool reader/writers. These are fully internal and nobody should be using them. forgeData should be available as a part of raw of the status response class. This is actually a part of the public API already (that's why the raw field was necessary in the first place).
This means that users can already access forgeData obtained from status, in the JSON format. Because of the incompatibilities in standard you mentioned, that parsing may indeed be pretty annoying though, and it might be interesting to create similar parser for this data to what we recently introduced for server MOTDs, i.e. have "AST"-like classes for the individual objects in the forgeData response, for example for representing a single mod.
I've actually already written a forgeData response reader for my discord bot StatusBot (decoder module linked here), it would just be a matter of making a nice interface for it here
Why do you need to transform dict object to buffer and use bytes? Why can't you just use dict?
Why do you need to transform dict object to buffer and use bytes? Why can't you just use dict?
@PerchunPak
From the example code CoolCat467 sent, it seems that forgeData dict (originally received as JSON), contains some data under the "d" key, which while sent as string in the JSON, is actually interpreted as binary data, and you do indeed need the buffer classes to properly read it out (See this line in the linked file).
forgeDatadict (originally received as JSON), contains some data under the "d" key
I tested it with grmpixelmon.com (Forge 1.16.5, it's a just some public server that I use to test mcstatus sometimes) and didn't get any d key anywhere. I'm confused where it comes from and what it means.
Yeah, I also didn't find any good documentation for it, do you have some docs or even source code link to the forge implementation that describes this @CoolCat467?
Yes, let me try to go find it. I was meaning to find the link to this the other day but it was getting a bit late.
Here we go, this file from forge explains everything pretty well: https://github.com/MinecraftForge/MinecraftForge/blob/42115d37d6a46856e3dc914b54a1ce6d33b9872a/src/main/java/net/minecraftforge/network/ServerStatusPing.java
Oh, that makes everything much more cleaner... I will approve #555 after you will resolve the conversation.
This issue has the WIP label, but I see that no progress was done on it for 2 years, I think it's fair to say this isn't a work-in-progress at this point, unless someone wants to pick it up.
If you would like I could make a pull request adding the previously linked decoder module I've made, but I'm not promising to do anything beyond that.
If you would like I could make a pull request adding the previously linked decoder module I've made, but I'm not promising to do anything beyond that.
If it works consistently across all forge versions, then sure, that'd be great!
Oh wait it looks like that got handled as a part of https://github.com/py-mine/mcstatus/pull/578
oh lol, I thought it was somewhere, but since nothing was linked, I figured I just imagined it, guess not, in that case, this can probably just be closed.