Rewrite MOTD parser #335

PerchunPak · 2022-06-23T16:30:29Z

Full rewrite of MOTD parser. I also made an API for users, so they can easily write their own parser.

Closes #204.

mcstatus/tests/test_motd.py

mcstatus/pinger.py

mcstatus/bedrock_status.py

PerchunPak · 2022-06-26T18:11:12Z

Found something strange, when I read about the protocol, found that all text in MOTD can be formatted with one tag on top level. See this. Should I implement parsing for this?

ItsDrike · 2022-06-26T18:13:31Z

Found something strange, when I read about the protocol, found that all text in MOTD can be formatted with one tag on top level. See this. Should I implement parsing for this?

That's interesting, good catch, yeah it probably should be respected, even though I don't think I've seen that before. If you are going to be adding support for that here, make sure to also add a unit-test for that. Otherwise, you can reference this comment in a new issue about this and let someone else handle it later.

ItsDrike

This is just a quick review of some issues I noticed when reading over the code for the first time, I'll make another review after inspecting the code in greater detail and doing some manual testing later, when these issues will be resolved.

This also needs addressing the failed unit tests and validation github workflows

NOTE: If this PR wasn't yet supposed to be reviewed, please mark it back as draft PR and feel free to ignore this review up until you think the PR will be ready.

mcstatus/motd.py

mcstatus/tests/test_motd.py

PerchunPak · 2022-06-29T09:21:03Z

NOTE: If this PR wasn't yet supposed to be reviewed, please mark it back as draft PR and feel free to ignore this review up until you think the PR will be ready.

This is ready to review, but not for merge. I'm keep doing ansi and xterm-256 properties.

PerchunPak · 2022-07-01T19:35:02Z

I pretty sure, few big moments here still needs more work.

PerchunPak · 2022-07-02T14:59:18Z

Hmmmmmmm... I wonder, how many things was broken in first implementation from Iapetus-11...

ItsDrike · 2022-07-02T17:48:06Z

Adding do-not-merge label, as requested.

This do-not-merge is here because this PR relies on renaming description to motd, which is currently a change that's not final and is still being discussed. To avoid the same discussion from happening here, a do-not-merge label is added until that discussion is resolved/306 gets merged.

Note that this PR can be still reviewed and approved, it just shouldn't yet be merged. But when reviewing, this blocking status should be taken into consideration, and some points on changes in variable naming or other inconsistencies with current code-base may not actually be inconsistencies, as this assumes the pending decision about this rename in 306 will end up on choosing to rename description to motd.

Merging blocked by Rewrite answer objects #306

PerchunPak · 2023-01-22T20:05:01Z

As flows out of this conversation, we no longer support ANSI 3-bit, 4-bit and 8-bit. At now, we only support 24-bit.

If you want this feature - implement it yourself, or raise a feature request. We can reconsider this.

Also, I found a public database with 130k servers and their answers. If you need to test this PR on real world - I can give a file with 45mb of raw answers. Just contact me.

mcstatus/motd/__init__.py

mcstatus/motd/components.py

mcstatus/motd/simplifies.py

tests/test_motd/test_transformers.py

ItsDrike

Looks good, is there something else that needs to be done before this can be merged? Do the simplify methods all work properly? Could you do some field testing to make sure that this works with most servers without issues?

PerchunPak · 2023-04-18T05:23:32Z

I found a database with several hundred thousand unparsed Minecraft servers' answers, and I will run all methods on them. Although, simplifies can be improved in some situations, you said that improving of them should go as separated PR, so this PR will be merged faster.

ItsDrike

Indeed, the simplifies can just be updated later, forgot I mentioned that before. In that case, after the integration testing is done, assuming it passes, IMO this can be merged.

Although I would like to see @kevinkjt2000's review too for this one, since the PR is pretty big and therefore could contain something I missed.

kevinkjt2000

This is awesome. Much wow. Much feature. Much appreciate.

PerchunPak force-pushed the add-more-methods-connected-to-motd branch 2 times, most recently from fef59cd to 223d6f2 Compare June 24, 2022 09:42

PerchunPak marked this pull request as ready for review June 25, 2022 10:07

PerchunPak force-pushed the add-more-methods-connected-to-motd branch from 43db85e to 4e2ff9e Compare June 25, 2022 10:15

PerchunPak commented Jun 25, 2022

View reviewed changes

mcstatus/tests/test_motd.py Outdated Show resolved Hide resolved

mcstatus/pinger.py Outdated Show resolved Hide resolved

mcstatus/bedrock_status.py Outdated Show resolved Hide resolved

PerchunPak force-pushed the add-more-methods-connected-to-motd branch from 4e2ff9e to 9618efb Compare June 28, 2022 14:51

ItsDrike requested changes Jun 28, 2022

View reviewed changes

ItsDrike added type: enhancement Improvement to an existing feature area: API Related to core API of the project state: waiting for author Waiting for author to address a review or respond to a comment labels Jun 28, 2022

PerchunPak force-pushed the add-more-methods-connected-to-motd branch from 9618efb to bccee99 Compare June 30, 2022 10:19

PerchunPak marked this pull request as draft July 1, 2022 19:35

PerchunPak force-pushed the add-more-methods-connected-to-motd branch 2 times, most recently from b09a9e2 to 4ca2f0d Compare July 2, 2022 12:46

PerchunPak marked this pull request as ready for review July 2, 2022 12:52

PerchunPak force-pushed the add-more-methods-connected-to-motd branch from 4ca2f0d to 5cfd1e4 Compare July 2, 2022 14:40

PerchunPak mentioned this pull request Jul 2, 2022

Status fails to process a JSON MOTD containing only a translation #319

Closed

PerchunPak force-pushed the add-more-methods-connected-to-motd branch from fbb8556 to fe247ea Compare July 2, 2022 16:24

ItsDrike added do-not-merge The PR can be reviewed but cannot be merged now labels Jul 2, 2022

PerchunPak force-pushed the add-more-methods-connected-to-motd branch from fe247ea to 622d58b Compare July 5, 2022 09:46

PerchunPak mentioned this pull request Jul 5, 2022

Add clean method to motd field #329

Closed

PerchunPak force-pushed the add-more-methods-connected-to-motd branch 2 times, most recently from 2e315db to 3f458bc Compare July 5, 2022 17:14

PerchunPak force-pushed the add-more-methods-connected-to-motd branch 3 times, most recently from 46dabec to 40774ba Compare January 15, 2023 17:39

PerchunPak force-pushed the add-more-methods-connected-to-motd branch 4 times, most recently from d6eef53 to 6e15db5 Compare January 22, 2023 20:00

PerchunPak force-pushed the add-more-methods-connected-to-motd branch from 6e15db5 to 0c41416 Compare January 22, 2023 21:14

ItsDrike requested changes Jan 22, 2023

View reviewed changes

PerchunPak force-pushed the add-more-methods-connected-to-motd branch from 0c41416 to a950e99 Compare January 24, 2023 10:32

ItsDrike removed the do-not-merge The PR can be reviewed but cannot be merged now label Jan 25, 2023

PerchunPak force-pushed the add-more-methods-connected-to-motd branch 2 times, most recently from fae1023 to 4277421 Compare February 19, 2023 21:27

ItsDrike mentioned this pull request Mar 11, 2023

Example "Ping as Java and as Bedrock in one time" didn't work #500

Closed

PerchunPak force-pushed the add-more-methods-connected-to-motd branch from 4277421 to a9e15ec Compare March 20, 2023 13:26

PerchunPak force-pushed the add-more-methods-connected-to-motd branch 2 times, most recently from 04dfac0 to ab53f18 Compare April 15, 2023 09:16

Rewrite MOTD parser

d9666e9

PerchunPak force-pushed the add-more-methods-connected-to-motd branch from ab53f18 to d9666e9 Compare April 15, 2023 09:18

ItsDrike reviewed Apr 17, 2023

View reviewed changes

ItsDrike approved these changes Apr 21, 2023

View reviewed changes

PerchunPak mentioned this pull request Apr 30, 2023

ImportError #529

Closed

kevinkjt2000 approved these changes May 3, 2023

View reviewed changes

kevinkjt2000 merged commit 9db947a into py-mine:master May 3, 2023

PerchunPak mentioned this pull request May 6, 2023

README.md documentation link open blank docs page #534

Closed

ItsDrike mentioned this pull request Jun 9, 2024

mcstatus cli: Motd is not JSON serializable #825

Closed

PerchunPak deleted the add-more-methods-connected-to-motd branch June 10, 2024 07:23

PerchunPak mentioned this pull request Jul 21, 2024

CLI: Bedrock support and misc improvements #849

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewrite MOTD parser #335

Rewrite MOTD parser #335

PerchunPak commented Jun 23, 2022 •

edited

Loading

PerchunPak commented Jun 26, 2022

ItsDrike commented Jun 26, 2022

ItsDrike left a comment •

edited

Loading

PerchunPak commented Jun 29, 2022

PerchunPak commented Jul 1, 2022

PerchunPak commented Jul 2, 2022

ItsDrike commented Jul 2, 2022 •

edited

Loading

PerchunPak commented Jan 22, 2023

ItsDrike left a comment

PerchunPak commented Apr 18, 2023

ItsDrike left a comment

kevinkjt2000 left a comment

Rewrite MOTD parser #335

Rewrite MOTD parser #335

Conversation

PerchunPak commented Jun 23, 2022 • edited Loading

PerchunPak commented Jun 26, 2022

ItsDrike commented Jun 26, 2022

ItsDrike left a comment • edited Loading

Choose a reason for hiding this comment

PerchunPak commented Jun 29, 2022

PerchunPak commented Jul 1, 2022

PerchunPak commented Jul 2, 2022

ItsDrike commented Jul 2, 2022 • edited Loading

PerchunPak commented Jan 22, 2023

ItsDrike left a comment

Choose a reason for hiding this comment

PerchunPak commented Apr 18, 2023

ItsDrike left a comment

Choose a reason for hiding this comment

kevinkjt2000 left a comment

Choose a reason for hiding this comment

PerchunPak commented Jun 23, 2022 •

edited

Loading

ItsDrike left a comment •

edited

Loading

ItsDrike commented Jul 2, 2022 •

edited

Loading