Parse forge mods from status response

[PerchunPak]

This includes forgeData key in the answer and other modded cores, if they provide this information.

[PerchunPak]

forgeData isn't a part of minecraft's official protocol, hence mcstatus doesn't support it. However forgeData seems 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 #555 (comment)

[PerchunPak]

forgeData isn'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 forgeData seems 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?

[ItsDrike]

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.

[CoolCat467]

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

[PerchunPak]

Why do you need to transform dict object to buffer and use bytes? Why can't you just use dict?

[PerchunPak]

Actually, it has quite poor documentation, but we anyway need to do our own research.

[ItsDrike]

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).

[PerchunPak]

forgeData dict (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.

[ItsDrike]

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?

[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.

[CoolCat467]

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

[PerchunPak]

Oh, that makes everything much more cleaner... I will approve #555 after you will resolve the conversation.