Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP: Stripped MotD support #92

Merged
merged 3 commits into from
Sep 11, 2021
Merged

PHP: Stripped MotD support #92

merged 3 commits into from
Sep 11, 2021

Conversation

ldilley
Copy link
Member

@ldilley ldilley commented Sep 11, 2021

Proposed Changes

@ldilley ldilley added bug Something isn't working enhancement Enhance an existing feature new feature Request something new labels Sep 11, 2021
@ldilley ldilley self-assigned this Sep 11, 2021
@ldilley ldilley added this to ToDo in MineStat via automation Sep 11, 2021
@AppVeyorBot
Copy link

Build minestat 1.0.0.78 completed (commit 4dadc0dfe6 by @ldilley)

@ldilley ldilley merged commit 4d0c780 into FragLand:master Sep 11, 2021
MineStat automation moved this from ToDo to Complete Sep 11, 2021
@mindsolve
Copy link
Collaborator

Hi @ldilley,

I found a few problems with the changes here:

  1. The json SLP protocol doesn't imply that as JSON-MOTD is used, the old format aka. a string in the description field is still valid and used by some servers (such as Hypixel):

    {
        "description": "             \u00a7aHypixel Network  \u00a7c[1.8-1.17]\n          \u00a75\u00a7lSKYBLOCK CRYSTAL HOLLOWS!"
    }

    That is the reason for my why I check the type of the passed data (object/dict vs string) in my implementations. If something like this is possible in PHP, you could completely remove the parameter to the strip_motd function.

  2. The field extra is optional in a json Chat object (example: any vanilla mc server)

    {
        "description": {"text":"This is MC \"1.16\" \u00a79\u00a7oT\u00a74E\u00a7r\u00a7lS\u00a76\u00a7o\u00a7nT"}
    }

Best regards,
~MindSolve

@ldilley
Copy link
Member Author

ldilley commented Sep 11, 2021

The Ruby implementation is going to exhibit similar symptoms then under the same condition (pointing at the equivalent server). This will need to be addressed in both languages.

@ldilley
Copy link
Member Author

ldilley commented Sep 11, 2021

PR #93 now exists to resolve the PHP issues. It was tested against both Hypixel and Mineplex. I'll await your review prior to merging.

ldilley added a commit to ldilley/minestat that referenced this pull request Jan 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement Enhance an existing feature new feature Request something new
Projects
MineStat
Complete
Development

Successfully merging this pull request may close these issues.

None yet

3 participants