search

Sch8ill / mcclient-lib

A lightweight Minecraft client for querying the status data of a Minecraft server
MIT License
17 stars 2 forks source link

Revision: Color Codes #15

Closed DJStompZone closed 1 year ago

DJStompZone commented 1 year ago

Updated _remove_color_codes method to strip "§g" ("Minecoin Gold", Bedrock Edition only), added optional flavor argument, defaulting to value "java" to match present behavior.

DJStompZone commented 1 year ago

@Sch8ill My bad, I didn't mean to add the Github Actions onto this PR. I can delete them or move to a new PR if you'd prefer to just merge the original changes to mcclient/response.py on this one

Sch8ill commented 1 year ago

Hey @DJStompZone , thanks for the contribution, i really appreciate it. The implementation of a regex was long over due, thanks for your help 😁 I think it would be better to move your new worflows into a different PR. I'm just going to comment a view parts of your code, i hope you are ok with that.

Best regards, Till

Sch8ill commented 1 year ago

@DJStompZone Would you mind adding the new Bedrock functionallity to the 'BedrockResponse' class?

DJStompZone commented 1 year ago

Hey @DJStompZone , thanks for the contribution, i really appreciate it. The implementation of a regex was long over due, thanks for your help 😁

You betcha!

I think it would be better to move your new worflows into a different PR.

So I'm not exceedingly fluent with the more advanced git stuff, but I think I got the workflow stuff moved out of this PR into a seperate branch, so this one should be good to go.

I'm just going to comment a view parts of your code, i hope you are ok with that.

Go for it

DJStompZone commented 1 year ago

@DJStompZone Would you mind adding the new Bedrock functionallity to the 'BedrockResponse' class?

Sure, can we do that on a new PR though? Just to be completely honest I don't want to confuse myself with all the different commits, git isn't exactly my strongest skill 😅

Sch8ill commented 1 year ago

@DJStompZone The PR is really messed up right now because i made some changes to files that you missed to fetch (my bad, i'm sorry). I think the best fix would be for you to open up a new PR containing only the changes to response.py

Sch8ill commented 1 year ago

Don't worry, i'm also not that good at git 😅. Are you using github desktop? That really helped me in the beginning

DJStompZone commented 1 year ago

@DJStompZone The PR is really messed up right now because i made some changes to files that you missed to fetch (my bad, i'm sorry). I think the best fix would be for you to open up a new PR containing only the changes to response.py

I rebased and squashed in your changes, so I think it's good to go. It's showing no conflict, but definitely have a look and make sure your latest changes are there.

DJStompZone commented 1 year ago

Are you using github desktop?

Combination of desktop, website, and CLI depending on what I'm doing at the time. GH Desktop is definitely the most intuitive though, for sure

Sch8ill commented 1 year ago

@DJStompZone The PR is really messed up right now because i made some changes to files that you missed to fetch (my bad, i'm sorry). I think the best fix would be for you to open up a new PR containing only the changes to response.py

I rebased and squashed in your changes, so I think it's good to go. It's showing no conflict, but definitely have a look and make sure your latest changes are there.

@DJStompZone the workflow files are stil in this PR Could you remove them? (: Additionally, could you take a look at the changes i suggested for _remove_color_codes ?

Sch8ill commented 1 year ago

implemented in #17