Fix PMMP Bedrock Servers Query

[AlphaBaqpla]

I double-checked everything and fixed the error due to which the validation test did not pass

[kartashovio]

This fixes the issue #433

[PerchunPak]

Yes, tests do not pass because of (maybe) invalid poetry.lock.

[ItsDrike]

Blocked by #440, which fixes the issues with CI checks

[AlphaBaqpla]

The changes have been tested for 12 hours and there are no problems with them

[AlphaBaqpla]

hmm, 1 test failed, 2 cancelled. why did it pass all the tests in my repository

[ItsDrike]

hmm, 1 test failed, 2 cancelled. why did it pass all the tests in my repository

This is caused by an issue in our workflows that only occurs in occasionally (see: #442)

I've manually triggered a re-run on the workflows which fixed the failure.

[ItsDrike]

Seems alright to me. Thanks @AlphaBaqpla!

[PerchunPak]

Could someone also describe what exactly this change do, why it works and what did it change?

P.S. My idea is kind of change in protocol.

[AlphaBaqpla]

Right now, the wrong magic packet is being used when sending a request. There was no problem with GeyserMC and Bedrock Dedicated Server, but when working with PMMP it complained about the wrong packet and dropped the connection. I corrected it to the correct one and everything began to work correctly.

[PerchunPak]

I mean, why did it work? What bytes/information was skipped/incorrect?

[AlphaBaqpla]

https://github.com/facebookarchive/RakNet/blob/1a169895a900c9fc4841c556e16514182b75faf8/Source/RakPeer.cpp#L135
Look at this, I just modified the magic data so that it was like this.

[AlphaBaqpla]

and pmmp itself uses this to get the status of other servers

[PerchunPak]

https://github.com/facebookarchive/RakNet/blob/1a169895a900c9fc4841c556e16514182b75faf8/Source/RakPeer.cpp#L135 Look at this, I just modified the magic data so that it was like this.

I know that you just copy pasted code from some library. That is the problem. No one here (probably) doesn't understand what was changed and why it works now.

and pmmp itself uses this to get the status of other servers

Could you give a link please?

[AlphaBaqpla]

https://github.com/jasonwynn10/libpmquery/blob/master/src/libpmquery/PMQuery.php

[PerchunPak]

It is just a library for PMMP plugins with a questionable and small Git history. So it is definitely not a reliable source.

Would be much better to reverse-engineer the parsing system of PMMP itself.

[AlphaBaqpla]

Would be much better to reverse-engineer the parsing system of PMMP itself.
https://wiki.vg/Raknet_Protocol
https://github.com/pmmp/PocketMine-MP/blob/stable/tools/ping-server.php
https://github.com/pmmp/RakLib

[ItsDrike]

To address some of the confusion around this (though I'll admit I don't have a huge experience with it), this is what I've found, in collaboration with @PerchunPak.

Here's a comparison between the two, showing exactly what has changed:

The request_status_data variable is holding data of the entire packet that we will be sending. Specifically, it is the Unconnected Ping packet. The format of this packet (according to https://wiki.vg/Raknet_Protocol#Unconnected_Ping) is the following:

With the MAGIC being a constant value, of exactly these hex bytes: 00ffff00fefefefefdfdfdfd12345678.

So, these are the data this PR proposes we use:

Whereas previously, we were using this:

The time data probably didn't need to be changed, and could've stayed as 0, what's however interesting is that our original approach was clearly completely missing the last 8 bytes of data, being the client GUID. I don't know why that was the case, and I don't even know how come it was even working without them, if it's clearly a part of the packet specification, but that is what this PR actually adds.

I don't have much experience with bedrock, and I honestly don't really know what the client GUID even is, however as it is a part of the packet format, and since from my limited testing, it seems to be working fine with other servers bedrock servers, I think this can be merged.

EDIT: I missed @PerchunPak's message that he already posted (we apparently had the same idea to add some explanation here, and did so at almost the same time, so my github didn't yet update to show it). As I did include some additional details and links, to keep the gtihub conversation shorter, I've marked @PerchunPak message as hidden, and edited my message to include all of the details from both versions.

[PerchunPak]

I honestly don't really know what the client GUID even is

I investigated CloudburstMC/Network and found that they just generate random value as a client GUID.

Removing my approval until PerchunPak's review is addressed.